Skip to content

Feature 296385 caching 1 #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
markopoulou wants to merge 3 commits into
base: SandboxEnv
Choose a base branch
from
Open

Feature 296385 caching 1 #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1fc4643
Fixed self invocation problem for getDatasetSchemaId. Refs #296385
markopoulou Apr 8, 2026
cfd74fc
Documentation added. Refs #296385
markopoulou May 21, 2026
23aeb6a
Self routing calls for @Transactional. Refs #296385
markopoulou May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions ...es/src/main/java/org/eea/security/jwt/configuration/CacheClientSecurityConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
package org.eea.security.jwt.configuration;

import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import org.eea.security.jwt.data.CacheTokenVO;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.data.redis.cache.RedisCacheConfiguration;
import org.springframework.data.redis.cache.RedisCacheManager;
import org.springframework.data.redis.connection.RedisSentinelConfiguration;
import org.springframework.data.redis.connection.RedisStandaloneConfiguration;
import org.springframework.data.redis.connection.jedis.JedisClientConfiguration;
import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.data.redis.serializer.GenericJackson2JsonRedisSerializer;
import org.springframework.data.redis.serializer.Jackson2JsonRedisSerializer;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.serializer.StringRedisSerializer;
import redis.clients.jedis.JedisPoolConfig;

Expand Down Expand Up @@ -40,6 +47,8 @@ public class CacheClientSecurityConfiguration {
private Integer minEvitableIdleTimeMillis;
@Value("${spring.redis.jedis.pool.max-wait}")
private Integer maxWaitMillis;
@Value("${cache.datasetSchemaId.ttl:120}")
private long datasetSchemaIdTtl;

@Value("${spring.cloud.consul.discovery.instanceId}")
private String serviceInstanceId;
Expand Down Expand Up @@ -111,4 +120,64 @@ private JedisPoolConfig createPoolConfig() {
return poolConfig;
}

/**
* Redis Cache Manager for local profile.
* Uses a standalone Redis connection to manage caches for
* {@code datasetSchemaId}.
* TTLs are configurable via Consul keys {@code cache.datasetSchemaId.ttl}
* and {@code cache.uniqueConstraints.ttl} (in minutes).
*
* @param jedisConnectionFactory the standalone Redis connection factory
* @return the Redis cache manager
*/
@Bean
@Profile("local")
public RedisCacheManager localCacheManager(JedisConnectionFactory jedisConnectionFactory) {
RedisCacheConfiguration defaultConfig = RedisCacheConfiguration.defaultCacheConfig()
.serializeKeysWith(
RedisSerializationContext.SerializationPair.fromSerializer(
new StringRedisSerializer()))
.serializeValuesWith(
RedisSerializationContext.SerializationPair.fromSerializer(
new GenericJackson2JsonRedisSerializer()));

Map<String, RedisCacheConfiguration> cacheConfigs = new HashMap<>();
cacheConfigs.put("datasetSchemaId", defaultConfig.entryTtl(Duration.ofMinutes(datasetSchemaIdTtl)));

return RedisCacheManager.builder(jedisConnectionFactory)
.cacheDefaults(defaultConfig)
.withInitialCacheConfigurations(cacheConfigs)
.build();
}

/**
* Redis Cache Manager for non-local profiles (dev, staging, production).
* Uses a Redis Sentinel connection for high availability to manage caches for
* {@code datasetSchemaId}.
* TTLs are configurable via Consul keys {@code cache.datasetSchemaId.ttl}
* and {@code cache.uniqueConstraints.ttl} (in minutes).
*
* @param jedisSentinelConnectionFactory the Redis Sentinel connection factory
* @return the Redis cache manager
*/
@Bean
@Profile("!local")
public RedisCacheManager sentinelCacheManager(JedisConnectionFactory jedisSentinelConnectionFactory) {
RedisCacheConfiguration defaultConfig = RedisCacheConfiguration.defaultCacheConfig()
.serializeKeysWith(
RedisSerializationContext.SerializationPair.fromSerializer(
new StringRedisSerializer()))
.serializeValuesWith(
RedisSerializationContext.SerializationPair.fromSerializer(
new GenericJackson2JsonRedisSerializer()));

Map<String, RedisCacheConfiguration> cacheConfigs = new HashMap<>();
cacheConfigs.put("datasetSchemaId", defaultConfig.entryTtl(Duration.ofMinutes(datasetSchemaIdTtl)));

return RedisCacheManager.builder(jedisSentinelConnectionFactory)
.cacheDefaults(defaultConfig)
.withInitialCacheConfigurations(cacheConfigs)
.build();
}

}
68 changes: 44 additions & 24 deletions dataset-service/src/main/java/org/eea/dataset/service/impl/DataschemaServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.context.annotation.Lazy;
import org.springframework.http.HttpStatus;
import org.springframework.scheduling.annotation.Async;
import org.springframework.security.core.context.SecurityContextHolder;
Expand All @@ -92,6 +95,18 @@
@Service("dataschemaService")
public class DataschemaServiceImpl implements DatasetSchemaService {

/**
* Self-referencing proxy of this service bean, injected to enable Spring AOP features
* (e.g., {@link Cacheable}, {@link CacheEvict}) on internal method calls within the same class.
* Direct {@code this.method()} calls bypass the Spring proxy and would not trigger cache
* interception; calling {@code self.method()} routes through the proxy instead.
*
* The {@link Lazy} annotation prevents circular dependency issues during bean initialization.
*/
@Autowired
@Lazy
private DatasetSchemaService self;

Copy link
Copy Markdown

@apboutos-trasys apboutos-trasys May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

/** The Constant REGEX_NAME: {@value}. */
private static final String REGEX_NAME = "[a-zA-Z0-9\\s_-]+";

Expand Down Expand Up @@ -333,21 +348,6 @@ public ObjectId createEmptyDataSetSchema(Long dataflowId) throws EEAException {
return idDataSetSchema;
}

/**
* Delete group and remove user.
*
* @param datasetId the dataset id
* @param resourceTypeEnum the resource type enum
*/
@Override
public void deleteGroup(Long datasetId, ResourceTypeEnum resourceTypeEnum) {
// We find all types of data of this schema and delete it
List<ResourceInfoVO> resourceCustodian = resourceManagementControllerZull
.getGroupsByIdResourceType(datasetId, ResourceTypeEnum.DATA_SCHEMA);
resourceManagementControllerZull.deleteResource(resourceCustodian);
LOG.info("Deleted group for datasetId {}", datasetId);
}

/**
* Gets the data schema by id.
*
Expand All @@ -370,6 +370,21 @@ public DataSetSchemaVO getDataSchemaById(String dataschemaId) {
return dataSchemaVO;
}

/**
* Delete group and remove user.
*
* @param datasetId the dataset id
* @param resourceTypeEnum the resource type enum
*/
@Override
public void deleteGroup(Long datasetId, ResourceTypeEnum resourceTypeEnum) {
// We find all types of data of this schema and delete it
List<ResourceInfoVO> resourceCustodian = resourceManagementControllerZull
.getGroupsByIdResourceType(datasetId, ResourceTypeEnum.DATA_SCHEMA);
resourceManagementControllerZull.deleteResource(resourceCustodian);
LOG.info("Deleted group for datasetId {}", datasetId);
}

/**
* Find the dataschema per idDataFlow.
*
Expand Down Expand Up @@ -420,6 +435,7 @@ private void setNameSchema(String schemaId, DataSetSchemaVO dataschemaVO) {
* @throws EEAException the EEA exception
*/
@Override
@Cacheable(value = "datasetSchemaId", key = "#datasetId")
public String getDatasetSchemaId(Long datasetId) throws EEAException {
return obtainDatasetMetabase(datasetId).getDatasetSchema();
}
Expand Down Expand Up @@ -450,6 +466,7 @@ private DataSetMetabase obtainDatasetMetabase(final Long datasetId) throws EEAEx
*/
@Override
@Transactional
@CacheEvict(value = "datasetSchemaId", key = "#datasetId")
public void deleteDatasetSchema(String schemaId, Long datasetId) {
// we delete the integrity rules associated with this dataset and delete the integrity in mongo
rulesControllerZuul.deleteDatasetRuleAndIntegrityByDatasetSchemaId(schemaId, datasetId);
Expand Down Expand Up @@ -568,7 +585,7 @@ public TableSchemaVO createTableSchema(String id, TableSchemaVO tableSchemaVO, L
@Override
public void updateTableSchema(Long datasetId, TableSchemaVO tableSchemaVO, Boolean updateMaterializedViews) throws EEAException {

String datasetSchemaId = getDatasetSchemaId(datasetId);
String datasetSchemaId = self.getDatasetSchemaId(datasetId);

try {
Document tableSchema =
Expand Down Expand Up @@ -777,6 +794,7 @@ public String createFieldSchema(String datasetSchemaId, FieldSchemaVO fieldSchem
* @throws EEAException the EEA exception
*/
@Override
@Transactional
Copy link
Copy Markdown

@apboutos-trasys apboutos-trasys May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Author

@markopoulou markopoulou May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

public DataType updateFieldSchema(String datasetSchemaId, FieldSchemaVO fieldSchemaVO,
Long datasetId, boolean cloningOrImporting) throws EEAException {

Expand Down Expand Up @@ -1156,6 +1174,7 @@ public void updateDatasetSchemaDescription(String datasetSchemaId, String descri
* @param availableInPublic the available in public
*/
@Override
@Transactional
public void updateDatasetSchemaExportable(String datasetSchemaId, boolean availableInPublic) {
schemasRepository.updateDatasetSchemaExportable(datasetSchemaId, availableInPublic);
}
Expand Down Expand Up @@ -2152,7 +2171,7 @@ public void copyUniqueConstraintsCatalogue(List<String> originDatasetSchemaIds,
*/
@Override
public SimpleDatasetSchemaVO getSimpleSchema(Long datasetId) throws EEAException {
String schemaId = getDatasetSchemaId(datasetId);
String schemaId = self.getDatasetSchemaId(datasetId);
if (schemaId != null) {
LOG.info("Getting schema from id {}", schemaId);
Optional<DesignDataset> designDataset =
Expand Down Expand Up @@ -2249,7 +2268,7 @@ public Boolean checkClearAttachments(Long datasetId, String datasetSchemaId,
*/
private void createNotEmptyRule(String tableSchemaId, Long datasetId) throws EEAException {
// retieve default level error if any
String datasetSchemaId = getDatasetSchemaId(datasetId);
String datasetSchemaId = self.getDatasetSchemaId(datasetId);
RulesSchema rulesSchema = rulesRepository.findByIdDatasetSchema(new ObjectId(datasetSchemaId));

ErrorTypeEnum automaticQCDefaultLevelError =
Expand Down Expand Up @@ -2570,7 +2589,7 @@ public void importSchemas(Long dataflowId, InputStream is, String fileName)
*/
@Override
public List<TableSchemaIdNameVO> getTableSchemasIds(Long datasetId) throws EEAException {
String datasetschemaId = getDatasetSchemaId(datasetId);
String datasetschemaId = self.getDatasetSchemaId(datasetId);
DataSetSchema schema = schemasRepository.findByIdDataSetSchema(new ObjectId(datasetschemaId));
List<TableSchemaIdNameVO> tableSchemasVOList = new ArrayList<>();
for (TableSchema table : schema.getTableSchemas()) {
Expand All @@ -2588,6 +2607,7 @@ public List<TableSchemaIdNameVO> getTableSchemasIds(Long datasetId) throws EEAEx
* @param referenceDataset the reference dataset
*/
@Override
@Transactional
Copy link
Copy Markdown

@apboutos-trasys apboutos-trasys May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Copy Markdown
Author

@markopoulou markopoulou May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
  1. 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(...);
}

public void updateReferenceDataset(Long datasetId, String datasetSchemaId,
boolean referenceDataset) {

Expand Down Expand Up @@ -2982,7 +3002,7 @@ private void updateImportFieldSchema(FieldSchemaVO fieldSchemaVO, List<FieldSche
datasetService.deleteAttachmentByFieldSchemaId(datasetId, fieldSchemaVO.getId());
}

DataType type = updateFieldSchema(datasetSchema.getIdDataSetSchema().toString(),
DataType type = self.updateFieldSchema(datasetSchema.getIdDataSetSchema().toString(),
fieldSchemaVO, datasetId, false);

// Create query view
Expand Down Expand Up @@ -3371,10 +3391,10 @@ private void processToModifyTheFK(Map<String, String> dictionaryOriginTargetObje
// update all the stuff
// related to the PK/FK
try {
String datasetSchemaId = getDatasetSchemaId(datasetId);
String datasetSchemaId = self.getDatasetSchemaId(datasetId);
updateForeignRelation(datasetId, fieldSchemaNoRulesMapper.entityToClass(field),
datasetSchemaId);
DataType type = updateFieldSchema(datasetSchemaId,
DataType type = self.updateFieldSchema(datasetSchemaId,
fieldSchemaNoRulesMapper.entityToClass(field), datasetId, true);
propagateRulesAfterUpdateSchema(datasetSchemaId,
fieldSchemaNoRulesMapper.entityToClass(field), type, datasetId);
Expand Down Expand Up @@ -3633,7 +3653,7 @@ public String getFieldName(String datasetSchemaId, String tableSchemaId, List<St
@Override
public String getFieldSchemaIdByDatasetIdTableNameAndFieldName(Long datasetId, String tableSchemaName, String fieldName){
try {
String datasetSchemaId = getDatasetSchemaId(datasetId);
String datasetSchemaId = self.getDatasetSchemaId(datasetId);
DataSetSchemaVO datasetSchema = getDataSchemaById(datasetSchemaId);
if (datasetSchema != null) {
String finalTableSchemaName = tableSchemaName;
Expand Down Expand Up @@ -3666,7 +3686,7 @@ public String getFieldSchemaIdByDatasetIdTableNameAndFieldName(Long datasetId, S
*/
@Override
public void updateManuallyEditableByDatasetId(Long datasetId, Boolean manuallyEditable) throws EEAException {
String datasetSchemaId = getDatasetSchemaId(datasetId);
String datasetSchemaId = self.getDatasetSchemaId(datasetId);
List<TableSchemaIdNameVO> tableSchemaIds = getTableSchemasIds(datasetId);
for(TableSchemaIdNameVO tableSchemaIdNameVO: tableSchemaIds){
TableSchemaVO tableSchemaVO = getTableSchemaVO(tableSchemaIdNameVO.getIdTableSchema(), datasetSchemaId);
Expand Down