Feature 296385 caching 1

[markopoulou]

Summary
Adds caching improvements for Dataschema service.

Changes

• Improved response performance for fetching dataset schema id.

Testing

• Verified locally
• Ran existing test suite
• Confirmed cache invalidation works as expected

Notes
Please review especially the caching strategy and invalidation logic.

[apboutos-trasys]

Since declaring self variables is not a thing we do in Java, please add a comment here to document this declaration, so it is obvious you are declaring a self variable to make Spring AOP work for self invocations of @transactional and @cachable, and @lazy to avoid circular dependancy error on startup. Intent comments are always welcome!

[apboutos-trasys]

Where is the cache for {@code uniqueConstraints}? Delete the javadoc reference if not needed.

[markopoulou]

These are leftovers after splitting the implementation. Thanks for noticing.

[apboutos-trasys]

Same here

[markopoulou]

These are leftovers after splitting the implementation. Thanks for noticing.

[apboutos-trasys]

Transaction will not work for the invocation of this method on lines 2998 and 3390 of this class as they are not called through the self reference. Is this intentional? We don't want it to work there?

[markopoulou]

This is surely not intentional. We do want @Transactional to work there.
The two internal callers are:

• updateImportFieldSchema calls updateFieldSchema(...) directly via this
• processToModifyTheFK calls updateFieldSchema(...) directly via this

Both are private methods, and neither routes through self, so the @Transactional annotation will be silently ignored for those two call paths.

Those calls should be routed through self otherwise:

• In updateImportFieldSchema: There are multiple operations happen in sequence. If updateFieldSchema fails midway, without a transaction boundary those preceding side effects won't be rolled back, leaving the schema in an inconsistent state.
• In processToModifyTheFK: We have multiple field updates loop. A failure mid-loop would leave partially updated FK relations.

[apboutos-trasys]

@transactional here will only work if an exception is thrown from schemasRepository.updateReferenceDataset() on line 2606. updateTableSchema() on line 2614 can fail to update any of the tables and the transaction will still be considered valid, as the exception thrown by updateTableSchema() is swallowed. Let's discuss this with Fotis.

[markopoulou]

You are correct. We have two problems here:

1. The swallowed exception: If updateTableSchema fails for any or all tables in the loop, the caught EEAException is only logged. No exception propagates out, so Spring's transaction manager sees a clean return and commits. The schemasRepository.updateReferenceDataset() change persists even if none of the tables were actually updated, leaving the schema in an inconsistent state.
2. The this invocation: Even if the exception was allowed to propagate, updateTableSchema is called via this rather than self, meaning its own @Transactional annotation is also bypassed, and more critically the call won't participate correctly in the outer transaction anyway.

I see two options here:

1. Remove the try-catch and let it propagate(either all tables update or none):

// no try-catch, exception propagates and triggers rollback
self.updateTableSchema(datasetId, table, true);

2. Keep the try-catch but acknowledge that @Transactional is misleading here and remove it, since it provides no real protection anyway.

try {
    self.updateTableSchema(datasetId, table, true);
} catch (EEAException e) {
    LOG.error(...);
}