Skip to content

FINERACT-1724: Making global configuration caching transaction bound #3041

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

Merged
galovics merged 1 commit into from
Mar 14, 2023
Merged

FINERACT-1724: Making global configuration caching transaction bound #3041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion ...ct-provider/src/main/java/org/apache/fineract/infrastructure/cache/CacheApiConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ private CacheApiConstants() {
}

public static final String RESOURCE_NAME = "CACHE";
public static final String cacheTypeParameter = "cacheType";
public static final String CACHE_TYPE_PARAMETER = "cacheType";

}
6 changes: 3 additions & 3 deletions ...main/java/org/apache/fineract/infrastructure/cache/command/UpdateCacheCommandHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
public class UpdateCacheCommandHandler implements NewCommandSourceHandler {

private final CacheWritePlatformService cacheService;
private static final Set<String> REQUEST_DATA_PARAMETERS = new HashSet<>(Arrays.asList(CacheApiConstants.cacheTypeParameter));
private static final Set<String> REQUEST_DATA_PARAMETERS = new HashSet<>(Arrays.asList(CacheApiConstants.CACHE_TYPE_PARAMETER));

@Autowired
public UpdateCacheCommandHandler(final CacheWritePlatformService cacheService) {
Expand All @@ -72,8 +72,8 @@ public CommandProcessingResult processCommand(final JsonCommand command) {
final DataValidatorBuilder baseDataValidator = new DataValidatorBuilder(dataValidationErrors)
.resource(CacheApiConstants.RESOURCE_NAME.toLowerCase());

final int cacheTypeEnum = command.integerValueSansLocaleOfParameterNamed(CacheApiConstants.cacheTypeParameter);
baseDataValidator.reset().parameter(CacheApiConstants.cacheTypeParameter).value(Integer.valueOf(cacheTypeEnum)).notNull()
final int cacheTypeEnum = command.integerValueSansLocaleOfParameterNamed(CacheApiConstants.CACHE_TYPE_PARAMETER);
baseDataValidator.reset().parameter(CacheApiConstants.CACHE_TYPE_PARAMETER).value(Integer.valueOf(cacheTypeEnum)).notNull()
.isOneOfTheseValues(Integer.valueOf(1), Integer.valueOf(2), Integer.valueOf(3));

if (!dataValidationErrors.isEmpty()) {
Expand Down
84 changes: 37 additions & 47 deletions .../java/org/apache/fineract/infrastructure/cache/service/RuntimeDelegatingCacheManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.infrastructure.cache.CacheApiConstants;
import org.apache.fineract.infrastructure.cache.CacheEnumerations;
import org.apache.fineract.infrastructure.cache.data.CacheData;
import org.apache.fineract.infrastructure.cache.domain.CacheType;
import org.apache.fineract.infrastructure.core.data.EnumOptionData;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.cache.Cache;
import org.springframework.cache.CacheManager;
import org.springframework.cache.jcache.JCacheCacheManager;
import org.springframework.cache.support.NoOpCacheManager;
import org.springframework.stereotype.Component;

Expand All @@ -43,92 +43,82 @@
* database on startup and allow user to switch implementation through UI/API
*/
@Component(value = "runtimeDelegatingCacheManager")
public class RuntimeDelegatingCacheManager implements CacheManager {

private static final Logger LOG = LoggerFactory.getLogger(RuntimeDelegatingCacheManager.class);

private final CacheManager cacheManager;
private final CacheManager noOpCacheManager = new NoOpCacheManager();
@RequiredArgsConstructor
@Slf4j
public class RuntimeDelegatingCacheManager implements CacheManager, InitializingBean {

@Qualifier("ehCacheManager")
private final CacheManager ehCacheManager;
@Qualifier("defaultCacheManager")
private final CacheManager defaultCacheManager;
private CacheManager currentCacheManager;

@Autowired
public RuntimeDelegatingCacheManager(final JCacheCacheManager cacheManager) {
this.cacheManager = cacheManager;
this.currentCacheManager = this.noOpCacheManager;
@Override
public void afterPropertiesSet() throws Exception {
currentCacheManager = defaultCacheManager;
}

@Override
public Cache getCache(final String name) {
return this.currentCacheManager.getCache(name);
return currentCacheManager.getCache(name);
}

@Override
public Collection<String> getCacheNames() {
return this.currentCacheManager.getCacheNames();
return currentCacheManager.getCacheNames();
}

public Collection<CacheData> retrieveAll() {

final boolean noCacheEnabled = this.currentCacheManager instanceof NoOpCacheManager;
final boolean ehcacheEnabled = this.currentCacheManager instanceof JCacheCacheManager;

// final boolean distributedCacheEnabled = false;
final boolean noCacheEnabled = currentCacheManager == defaultCacheManager;
final boolean ehCacheEnabled = currentCacheManager == ehCacheManager;

final EnumOptionData noCacheType = CacheEnumerations.cacheType(CacheType.NO_CACHE);
final EnumOptionData singleNodeCacheType = CacheEnumerations.cacheType(CacheType.SINGLE_NODE);
// final EnumOptionData multiNodeCacheType =
// CacheEnumerations.cacheType(CacheType.MULTI_NODE);

final CacheData noCache = CacheData.instance(noCacheType, noCacheEnabled);
final CacheData singleNodeCache = CacheData.instance(singleNodeCacheType, ehcacheEnabled);
// final CacheData distributedCache =
// CacheData.instance(multiNodeCacheType, distributedCacheEnabled);
final CacheData singleNodeCache = CacheData.instance(singleNodeCacheType, ehCacheEnabled);

final Collection<CacheData> caches = Arrays.asList(noCache, singleNodeCache);
return caches;
return Arrays.asList(noCache, singleNodeCache);
}

public Map<String, Object> switchToCache(final boolean ehcacheEnabled, final CacheType toCacheType) {

@vidakovic vidakovic Mar 14, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably I missed (a lot of) the conversation around this feature... but is there really a use case where you have to switch cache implementation during runtime? That seems to me such an important architectural decision that I would say it is done way before any instance of Fineract is running, at the least you would decide if EH Cache is enough or if you need it at all (I'd say the answer here is always yes... but not sure if you guys discussed a use case where no cache is desirable).
BTW: multi-node cache... Redis is your friend... 1st class support in Spring Boot and works really great (read: performant).

@adamsaghy adamsaghy Mar 14, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For multi node cache for defo we should use Redis


final Map<String, Object> changes = new HashMap<>();

final boolean noCacheEnabled = !ehcacheEnabled;
final boolean distributedCacheEnabled = !ehcacheEnabled;

switch (toCacheType) {
case INVALID:
break;
case NO_CACHE:
case INVALID -> {
log.warn("Invalid cache type used");
}
case NO_CACHE -> {
if (!noCacheEnabled) {
changes.put(CacheApiConstants.cacheTypeParameter, toCacheType.getValue());
changes.put(CacheApiConstants.CACHE_TYPE_PARAMETER, toCacheType.getValue());
}
this.currentCacheManager = this.noOpCacheManager;
break;
case SINGLE_NODE:
currentCacheManager = defaultCacheManager;
}
case SINGLE_NODE -> {
if (!ehcacheEnabled) {
changes.put(CacheApiConstants.cacheTypeParameter, toCacheType.getValue());
changes.put(CacheApiConstants.CACHE_TYPE_PARAMETER, toCacheType.getValue());
clearEhCache();
}
this.currentCacheManager = this.cacheManager;
currentCacheManager = ehCacheManager;

if (this.currentCacheManager.getCacheNames().size() == 0) {
LOG.error("No caches configured for activated CacheManager {}", this.currentCacheManager);
}
break;
case MULTI_NODE:
if (!distributedCacheEnabled) {
changes.put(CacheApiConstants.cacheTypeParameter, toCacheType.getValue());
if (currentCacheManager.getCacheNames().size() == 0) {
log.error("No caches configured for activated CacheManager {}", currentCacheManager);
}
break;
}
case MULTI_NODE -> throw new UnsupportedOperationException("Multi node cache is not supported");
}

return changes;
}

private void clearEhCache() {
Iterable<String> cacheNames = cacheManager.getCacheNames();
Iterable<String> cacheNames = ehCacheManager.getCacheNames();
for (String cacheName : cacheNames) {
cacheManager.getCache(cacheName).clear();
ehCacheManager.getCache(cacheName).clear();
}
}
}
33 changes: 7 additions & 26 deletions ...rg/apache/fineract/infrastructure/configuration/domain/ConfigurationDomainServiceJpa.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,22 @@
package org.apache.fineract.infrastructure.configuration.domain;

import java.time.LocalDate;
import java.util.HashMap;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;
import org.apache.fineract.infrastructure.cache.domain.CacheType;
import org.apache.fineract.infrastructure.cache.domain.PlatformCache;
import org.apache.fineract.infrastructure.cache.domain.PlatformCacheRepository;
import org.apache.fineract.infrastructure.configuration.data.GlobalConfigurationPropertyData;
import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil;
import org.apache.fineract.useradministration.domain.Permission;
import org.apache.fineract.useradministration.domain.PermissionRepository;
import org.apache.fineract.useradministration.exception.PermissionNotFoundException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Service
@RequiredArgsConstructor
public class ConfigurationDomainServiceJpa implements ConfigurationDomainService {

@vidakovic vidakovic Mar 14, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

... see my comment on global configuration service... pretty much the same here.


public static final String ENABLE_BUSINESS_DATE = "enable_business_date";
Expand All @@ -55,15 +54,6 @@ public class ConfigurationDomainServiceJpa implements ConfigurationDomainService
private final PermissionRepository permissionRepository;
private final GlobalConfigurationRepositoryWrapper globalConfigurationRepository;
private final PlatformCacheRepository cacheTypeRepository;
private static Map<String, GlobalConfigurationPropertyData> configurations = new HashMap<>();

@Autowired
public ConfigurationDomainServiceJpa(final PermissionRepository permissionRepository,
final GlobalConfigurationRepositoryWrapper globalConfigurationRepository, final PlatformCacheRepository cacheTypeRepository) {
this.permissionRepository = permissionRepository;
this.globalConfigurationRepository = globalConfigurationRepository;
this.cacheTypeRepository = cacheTypeRepository;
}

@Override
public boolean isMakerCheckerEnabledForTask(final String taskPermissionCode) {
Expand Down Expand Up @@ -321,9 +311,7 @@ public Long getDailyTPTLimit() {

@Override
public void removeGlobalConfigurationPropertyDataFromCache(final String propertyName) {
String identifier = ThreadLocalContextUtil.getTenant().getTenantIdentifier();
String key = identifier + "_" + propertyName;
configurations.remove(key);
globalConfigurationRepository.removeFromCache(propertyName);
}

@Override
Expand Down Expand Up @@ -389,15 +377,8 @@ public Long retrieveRelaxingDaysConfigForPivotDate() {
return property.getValue();
}

@Cacheable(value = "configByName", key = "T(org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil).getTenant().getTenantIdentifier().concat(#propertyName)")
public GlobalConfigurationPropertyData getGlobalConfigurationPropertyData(final String propertyName) {
String identifier = ThreadLocalContextUtil.getTenant().getTenantIdentifier();
String key = identifier + "_" + propertyName;
if (!configurations.containsKey(key)) {
GlobalConfigurationProperty configuration = this.globalConfigurationRepository.findOneByNameWithNotFoundDetection(propertyName);
configurations.put(key, configuration.toData());
}
return configurations.get(key);
private GlobalConfigurationPropertyData getGlobalConfigurationPropertyData(final String propertyName) {
return globalConfigurationRepository.findOneByNameWithNotFoundDetection(propertyName).toData();
}

@Override
Expand Down
9 changes: 9 additions & 0 deletions ...he/fineract/infrastructure/configuration/domain/GlobalConfigurationRepositoryWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
*/
package org.apache.fineract.infrastructure.configuration.domain;

import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.infrastructure.configuration.exception.GlobalConfigurationPropertyNotFoundException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.stereotype.Service;

/**
Expand All @@ -28,6 +31,7 @@
* </p>
*/
@Service
@Slf4j
public class GlobalConfigurationRepositoryWrapper {

private final GlobalConfigurationRepository repository;
Expand All @@ -37,6 +41,7 @@ public GlobalConfigurationRepositoryWrapper(final GlobalConfigurationRepository
this.repository = repository;
}

@Cacheable(value = "configByName", key = "T(org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil).getTenant().getTenantIdentifier().concat(#propertyName)")

@vidakovic vidakovic Mar 14, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just in general: I would try to eliminate this service all together... as can be seen with this requirement to add caching... this thing is just getting in the way and it's not really dev-ops friendly (as you need a database in the first place to be able to do your configuration... instead of simple files)... just saying it again: I think reloadable configurations are a solved problem in Spring/Boot... my 2 cents here: this service is creating more head-aches than it provides solutions. But maybe a discussion for another day.

public GlobalConfigurationProperty findOneByNameWithNotFoundDetection(final String propertyName) {
final GlobalConfigurationProperty property = this.repository.findOneByName(propertyName);
if (property == null) {
Expand All @@ -61,4 +66,8 @@ public void delete(final GlobalConfigurationProperty globalConfigurationProperty
this.repository.delete(globalConfigurationProperty);
}

@CacheEvict(value = "configByName", key = "T(org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil).getTenant().getTenantIdentifier().concat(#propertyName)")
public void removeFromCache(String propertyName) {
log.debug("Cache entry evicted {}", propertyName);
}
}
7 changes: 7 additions & 0 deletions ...ider/src/main/java/org/apache/fineract/infrastructure/core/config/FineractProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public class FineractProperties {
private FineractJobProperties job;

private FineractTemplateProperties template;
private FineractJpaProperties jpa;

@Getter
@Setter
Expand Down Expand Up @@ -248,4 +249,10 @@ public static class FineractTemplateProperties {
private List<String> regexWhitelist;
}

@Getter
@Setter
public static class FineractJpaProperties {

private boolean statementLoggingEnabled;
}
}
19 changes: 16 additions & 3 deletions ...frastructure/core/config/CacheConfig.java → ...ucture/core/config/cache/CacheConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

package org.apache.fineract.infrastructure.core.config;
package org.apache.fineract.infrastructure.core.config.cache;

import java.time.Duration;
import javax.cache.CacheManager;
Expand All @@ -28,20 +28,32 @@
import org.ehcache.config.builders.ResourcePoolsBuilder;
import org.ehcache.jsr107.Eh107Configuration;
import org.springframework.cache.jcache.JCacheCacheManager;
import org.springframework.cache.support.NoOpCacheManager;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
public class CacheConfig {

public static final String CONFIG_BY_NAME_CACHE_NAME = "configByName";

@Bean
public TransactionBoundCacheManager defaultCacheManager(JCacheCacheManager ehCacheManager) {
SpecifiedCacheSupportingCacheManager cacheManager = new SpecifiedCacheSupportingCacheManager();
cacheManager.setNoOpCacheManager(new NoOpCacheManager());
cacheManager.setDelegateCacheManager(ehCacheManager);
cacheManager.setSupportedCaches(CONFIG_BY_NAME_CACHE_NAME);
return new TransactionBoundCacheManager(cacheManager);
}

@Bean
public JCacheCacheManager ehCacheManager() {
JCacheCacheManager jCacheCacheManager = new JCacheCacheManager();
jCacheCacheManager.setCacheManager(getCustomCacheManager());
jCacheCacheManager.setCacheManager(getInternalEhCacheManager());
return jCacheCacheManager;
}

private CacheManager getCustomCacheManager() {
private CacheManager getInternalEhCacheManager() {
CachingProvider provider = Caching.getCachingProvider();
CacheManager cacheManager = provider.getCacheManager();

Expand All @@ -61,6 +73,7 @@ private CacheManager getCustomCacheManager() {
cacheManager.createCache("codes", defaultTemplate);
cacheManager.createCache("hooks", defaultTemplate);
cacheManager.createCache("tfConfig", defaultTemplate);
cacheManager.createCache(CONFIG_BY_NAME_CACHE_NAME, defaultTemplate);

javax.cache.configuration.Configuration<Object, Object> accessTokenTemplate = Eh107Configuration.fromEhcacheCacheConfiguration(
CacheConfigurationBuilder.newCacheConfigurationBuilder(Object.class, Object.class, ResourcePoolsBuilder.heap(10000))
Expand Down
Loading