From 3ce49b9a4757dfcb50decc6d0bdfe741fe87d955 Mon Sep 17 00:00:00 2001 From: ffalqui Date: Mon, 9 Feb 2026 17:23:18 +0100 Subject: [PATCH] - import developments ESB-805 fix multitenancy keygen duplicate key error - added global-lock --- .github/build.sh | 1 + .../system/BaseInterceptorMadMax.java | 11 +- .../system/ActionLoggerInterceptor.java | 15 ++ .../system/MDCStrutsTenantInterceptor.java | 17 +- .../system/MultitenancyStrutsInterceptor.java | 14 +- .../agiletec/aps/system/ApsSystemUtils.java | 9 + .../keygenerator/IKeyGeneratorDAO.java | 6 + .../keygenerator/KeyGeneratorDAO.java | 81 +++++-- .../keygenerator/KeyGeneratorManager.java | 79 ++++++- .../KeyGeneratorManagerCacheWrapper.java | 18 ++ .../actionlog/ActionLogAppenderThread.java | 6 + .../services/sync/GlobalLockManager.java | 34 +++ .../services/sync/IGlobalLockManager.java | 34 +++ .../exception/GlobalLockEntException.java | 15 ++ .../aps/managers/baseManagersConfig.xml | 6 +- .../keygenerator/KeyGeneratorDAOTest.java | 151 ++++++++++++ .../keygenerator/KeyGeneratorManagerTest.java | 121 ++++++++++ .../services/sync/GlobalLockManagerTest.java | 69 ++++++ .../system/redis/sync/GlobalLockManager.java | 172 ++++++++++++++ .../sync/GlobalLockManagerFactoryBean.java | 32 +++ .../jpredis/aps/baseManagersConfig.xml | 2 + .../redis/sync/GlobalLockManagerTest.java | 223 ++++++++++++++++++ 22 files changed, 1076 insertions(+), 40 deletions(-) create mode 100644 engine/src/main/java/org/entando/entando/aps/system/services/sync/GlobalLockManager.java create mode 100644 engine/src/main/java/org/entando/entando/aps/system/services/sync/IGlobalLockManager.java create mode 100644 engine/src/main/java/org/entando/entando/aps/system/services/sync/exception/GlobalLockEntException.java create mode 100644 engine/src/test/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorDAOTest.java create mode 100644 engine/src/test/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManagerTest.java create mode 100644 engine/src/test/java/org/entando/entando/aps/system/services/sync/GlobalLockManagerTest.java create mode 100644 redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManager.java create mode 100644 redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerFactoryBean.java create mode 100644 redis-plugin/src/test/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerTest.java diff --git a/.github/build.sh b/.github/build.sh index 73029599e9..363adf2fab 100755 --- a/.github/build.sh +++ b/.github/build.sh @@ -1,4 +1,5 @@ #!/bin/bash +set -e mvn -B clean diff --git a/admin-console/src/main/java/com/agiletec/apsadmin/system/BaseInterceptorMadMax.java b/admin-console/src/main/java/com/agiletec/apsadmin/system/BaseInterceptorMadMax.java index 2bdec121ba..b99076f1eb 100644 --- a/admin-console/src/main/java/com/agiletec/apsadmin/system/BaseInterceptorMadMax.java +++ b/admin-console/src/main/java/com/agiletec/apsadmin/system/BaseInterceptorMadMax.java @@ -17,6 +17,8 @@ import java.util.Iterator; import java.util.Set; +import com.agiletec.aps.system.ApsSystemUtils; +import com.agiletec.aps.util.ApsTenantApplicationUtils; import jakarta.servlet.http.HttpSession; import org.apache.struts2.ServletActionContext; @@ -44,6 +46,9 @@ public abstract class BaseInterceptorMadMax extends AbstractInterceptor { public String intercept(ActionInvocation invocation) throws Exception { boolean isAuthorized = false; try { + // ESB-805 - Adding logs for tenant + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - intercept - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); HttpSession session = ServletActionContext.getRequest().getSession(); UserDetails currentUser = (UserDetails) session.getAttribute(SystemConstants.SESSIONPARAM_CURRENT_USER); IAuthorizationManager authManager = (IAuthorizationManager) ApsWebApplicationUtils.getBean(SystemConstants.AUTHORIZATION_SERVICE, ServletActionContext.getRequest()); @@ -60,7 +65,11 @@ public String intercept(ActionInvocation invocation) throws Exception { } } if (isAuthorized) { - return this.invoke(invocation); + String result = this.invoke(invocation); + // ESB-805 - Adding logs for tenant + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - invoked - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); + return result; } } catch (Throwable t) { _logger.error("Error occurred verifying authority of current user", t); diff --git a/admin-console/src/main/java/org/entando/entando/apsadmin/system/ActionLoggerInterceptor.java b/admin-console/src/main/java/org/entando/entando/apsadmin/system/ActionLoggerInterceptor.java index de83e367a8..a53d23dba9 100644 --- a/admin-console/src/main/java/org/entando/entando/apsadmin/system/ActionLoggerInterceptor.java +++ b/admin-console/src/main/java/org/entando/entando/apsadmin/system/ActionLoggerInterceptor.java @@ -18,6 +18,8 @@ import java.util.Map; import java.util.Map.Entry; +import com.agiletec.aps.system.ApsSystemUtils; +import com.agiletec.aps.util.ApsTenantApplicationUtils; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpSession; @@ -53,8 +55,15 @@ public String intercept(ActionInvocation invocation) throws Exception { ActionLogRecord actionRecord = null; String result = null; try { + // ESB-805 - Adding logs for tenant + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - intercept - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); actionRecord = this.buildActionRecord(invocation); result = invocation.invoke(); + // ESB-805 - Adding logs for tenant + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - invoked - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); + List asiList = null; Object actionObject = invocation.getAction(); if (actionObject instanceof BaseAction) { @@ -62,6 +71,9 @@ public String intercept(ActionInvocation invocation) throws Exception { asiList = action.getActivityStreamInfos(); } this.includeActionProperties(actionRecord, actionObject); + // CDP - ESB-805 - Adding logs for tenant + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - saving log - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); if (null == asiList || asiList.isEmpty()) { this.getActionLoggerManager().addActionRecord(actionRecord); } else { @@ -71,6 +83,9 @@ public String intercept(ActionInvocation invocation) throws Exception { this.getActionLoggerManager().addActionRecord(clone); } } + ApsTenantApplicationUtils.removeTenant(); + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - log saved - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); } catch (Throwable t) { _logger.error("error in intercept", t); } diff --git a/admin-console/src/main/java/org/entando/entando/apsadmin/system/MDCStrutsTenantInterceptor.java b/admin-console/src/main/java/org/entando/entando/apsadmin/system/MDCStrutsTenantInterceptor.java index 5a234456ff..b93ac90c0a 100644 --- a/admin-console/src/main/java/org/entando/entando/apsadmin/system/MDCStrutsTenantInterceptor.java +++ b/admin-console/src/main/java/org/entando/entando/apsadmin/system/MDCStrutsTenantInterceptor.java @@ -1,5 +1,6 @@ package org.entando.entando.apsadmin.system; +import com.agiletec.aps.system.ApsSystemUtils; import com.agiletec.aps.util.ApsTenantApplicationUtils; import org.apache.struts2.ActionInvocation; import org.apache.struts2.interceptor.AbstractInterceptor; @@ -11,11 +12,15 @@ public class MDCStrutsTenantInterceptor extends AbstractInterceptor { @Override public String intercept(ActionInvocation invocation) throws Exception { - try { - MDC.put(MDC_KEY_TENANT, ApsTenantApplicationUtils.getTenant().orElse("")); - return invocation.invoke(); - } finally { - MDC.remove(MDC_KEY_TENANT); - } + //try { + // ESB-805 - Adding logs for tenant + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - intercept - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); + MDC.put(MDC_KEY_TENANT, ApsTenantApplicationUtils.getTenant().orElse("")); + return invocation.invoke(); + //} finally { + // ESB-805 - Removed to preserve tenant + // MDC.remove(MDC_KEY_TENANT); + //} } } diff --git a/admin-console/src/main/java/org/entando/entando/apsadmin/system/MultitenancyStrutsInterceptor.java b/admin-console/src/main/java/org/entando/entando/apsadmin/system/MultitenancyStrutsInterceptor.java index a6a6edb5e0..ee6352a44a 100644 --- a/admin-console/src/main/java/org/entando/entando/apsadmin/system/MultitenancyStrutsInterceptor.java +++ b/admin-console/src/main/java/org/entando/entando/apsadmin/system/MultitenancyStrutsInterceptor.java @@ -13,6 +13,7 @@ */ package org.entando.entando.apsadmin.system; +import com.agiletec.aps.system.ApsSystemUtils; import com.agiletec.aps.system.EntThreadLocal; import com.agiletec.aps.util.ApsTenantApplicationUtils; import org.apache.struts2.ActionInvocation; @@ -25,7 +26,11 @@ public class MultitenancyStrutsInterceptor extends AbstractInterceptor { @Override public String intercept(ActionInvocation invocation) throws Exception { - try { + //try { + // ESB-805 - Adding logs for tenant + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - intercept - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); + EntThreadLocal.clear(); HttpServletRequest request = ServletActionContext.getRequest(); @@ -34,8 +39,9 @@ public String intercept(ActionInvocation invocation) throws Exception { return invocation.invoke(); - } finally { - ApsTenantApplicationUtils.removeTenant(); - } + //} finally { + // ESB-805 - Removed to preserve tenant + // ApsTenantApplicationUtils.removeTenant(); + //} } } \ No newline at end of file diff --git a/engine/src/main/java/com/agiletec/aps/system/ApsSystemUtils.java b/engine/src/main/java/com/agiletec/aps/system/ApsSystemUtils.java index 7aa338d76c..16ae58f1d5 100644 --- a/engine/src/main/java/com/agiletec/aps/system/ApsSystemUtils.java +++ b/engine/src/main/java/com/agiletec/aps/system/ApsSystemUtils.java @@ -14,12 +14,14 @@ package com.agiletec.aps.system; import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.lang3.StringUtils; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; import org.slf4j.Marker; import org.slf4j.MarkerFactory; import java.util.Enumeration; +import java.util.Optional; /** * Utility class for system logger @@ -209,6 +211,13 @@ public static String getEnv(String name, String def) { return (res != null) ? res : def; } + public static Integer getEnv(String name, Integer defaultValue) { + return Optional.ofNullable(System.getenv(name)) + .filter(StringUtils::isNotBlank) + .map(Integer::parseInt) + .orElse(defaultValue); + } + public static boolean getEnvFlag(String name, boolean def) { String res = System.getenv(name); return res != null ? Boolean.parseBoolean(res) : def; diff --git a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/IKeyGeneratorDAO.java b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/IKeyGeneratorDAO.java index 292cfb6b78..ed5a0845b4 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/IKeyGeneratorDAO.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/IKeyGeneratorDAO.java @@ -19,6 +19,12 @@ */ public interface IKeyGeneratorDAO { + /** + * Incrementa la chiave presente nel db. + * @return La chiave incrementata. + */ + public int getNextUniqueKey(); + /** * Estrae la chiave presente nel db. * Il metodo viene chiamato solo in fase di inizializzazione. diff --git a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorDAO.java b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorDAO.java index da5fc3f2f8..f3ca1c869c 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorDAO.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorDAO.java @@ -13,10 +13,7 @@ */ package com.agiletec.aps.system.services.keygenerator; -import java.sql.Connection; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.Statement; +import java.sql.*; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; @@ -30,7 +27,30 @@ public class KeyGeneratorDAO extends AbstractDAO implements IKeyGeneratorDAO { private static final EntLogger _logger = EntLogFactory.getSanitizedLogger(KeyGeneratorDAO.class); - + + @Override + public int getNextUniqueKey() { + Connection conn = null; + int nextKey = 0; + Statement stat = null; + ResultSet res = null; + try { + conn = this.getConnection(); + conn.setAutoCommit(false); + nextKey = this.getUniqueKey(conn) + 1; + this.updateKey(nextKey, conn); + conn.commit(); + } catch (Throwable t) { + this.executeRollback(conn); + _logger.error("Error while getting the unique key", t); + throw new RuntimeException("Error while getting the unique key", t); + //processDaoException(e, "Error while getting the unique key", "getUniqueKey"); + } finally { + closeConnection(conn); + } + return nextKey; + } + /** * Estrae la chiave presente nel db. * Il metodo viene chiamato solo in fase di inizializzazione. @@ -39,21 +59,15 @@ public class KeyGeneratorDAO extends AbstractDAO implements IKeyGeneratorDAO { public int getUniqueKey() { Connection conn = null; int currentKey = 0; - Statement stat = null; - ResultSet res = null; try { conn = this.getConnection(); - stat = conn.createStatement(); - res = stat.executeQuery(EXTRACT_KEY); - if (res.next()) { - currentKey = res.getInt(1); - } + currentKey = this.getUniqueKey(conn); } catch (Throwable t) { _logger.error("Error while getting the unique key", t); throw new RuntimeException("Error while getting the unique key", t); //processDaoException(e, "Error while getting the unique key", "getUniqueKey"); } finally { - closeDaoResources(res, stat, conn); + closeConnection(conn); } return currentKey; } @@ -64,21 +78,46 @@ public int getUniqueKey() { */ public synchronized void updateKey(int currentKey) { Connection conn = null; - PreparedStatement stat = null; try { conn = this.getConnection(); - conn.setAutoCommit(false); - stat = conn.prepareStatement(UPDATE_KEY); - stat.setInt(1, currentKey); - stat.executeUpdate(); - conn.commit(); + this.updateKey(currentKey, conn); } catch (Throwable t) { - this.executeRollback(conn); _logger.error("Error while updating a key", t); throw new RuntimeException("Error while updating a key", t); //processDaoException(e, "Error while updating a key", "getUpdateKey"); } finally { - closeDaoResources(null, stat, conn); + closeConnection(conn); + } + } + + protected int getUniqueKey(Connection conn) throws SQLException { + int currentKey = 0; + Statement stat = null; + ResultSet res = null; + try { + stat = conn.createStatement(); + res = stat.executeQuery(EXTRACT_KEY); + if (res.next()) { + currentKey = res.getInt(1); + } + } finally { + closeDaoResources(res, stat); + } + return currentKey; + } + + /** + * Aggiorna la chiave univoca nel db. + * @param currentKey Il valore della chiave corrente. + */ + protected void updateKey(int currentKey, Connection conn) throws SQLException { + PreparedStatement stat = null; + try { + stat = conn.prepareStatement(UPDATE_KEY); + stat.setInt(1, currentKey); + stat.executeUpdate(); + } finally { + closeDaoResources(null, stat); } } diff --git a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManager.java b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManager.java index 4e82a063f7..de531179c9 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManager.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManager.java @@ -13,13 +13,23 @@ */ package com.agiletec.aps.system.services.keygenerator; +import com.agiletec.aps.system.ApsSystemUtils; import com.agiletec.aps.system.common.AbstractService; +import com.agiletec.aps.util.ApsTenantApplicationUtils; +import org.entando.entando.aps.system.services.IFeatureFlag; +import org.entando.entando.aps.system.services.sync.IGlobalLockManager; +import org.entando.entando.aps.system.services.sync.exception.GlobalLockEntException; import org.entando.entando.aps.system.services.tenants.RefreshableBeanTenantAware; import org.entando.entando.ent.exception.EntException; import com.agiletec.aps.system.services.keygenerator.cache.IKeyGeneratorManagerCacheWrapper; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import java.time.Duration; +import java.util.function.Supplier; + +import static com.agiletec.aps.system.ApsSystemUtils.getEnv; + /** * Servizio gestore di sequenze univoche. * @@ -29,15 +39,26 @@ public class KeyGeneratorManager extends AbstractService implements IKeyGenerato private final EntLogger logger = EntLogFactory.getSanitizedLogger(getClass()); + private static boolean KEYGEN_LOCK_ENABLED = IFeatureFlag.readEnablementStatus("KEYGEN_LOCK"); + + private static Duration KEYGEN_LOCK_DURATION = Duration.ofMinutes(getEnv("KEYGEN_LOCK_DURATION", 5)); + + private static Duration KEYGEN_LOCK_MAX_WAIT = Duration.ofMinutes(getEnv("KEYGEN_LOCK_MAX_WAIT_MILLIS", 250)); + private IKeyGeneratorDAO keyGeneratorDao; private IKeyGeneratorManagerCacheWrapper cacheWrapper; + private IGlobalLockManager globalLockManager; + @Override public void init() throws Exception { initTenantAware(); - logger.debug("{} ready. : last loaded key {}", this.getClass().getName(), this.getCacheWrapper().getUniqueKeyCurrentValue()); - } + if (this.isLockEnabled()) { + logger.debug("{} ready. Lock feature is enabled.", this.getClass().getName(), this.getCacheWrapper().getUniqueKeyCurrentValue()); + } else { + logger.debug("{} ready. : last loaded key {}", this.getClass().getName()); + } } @Override protected void release() { @@ -47,14 +68,17 @@ protected void release() { @Override public void initTenantAware() throws Exception { - this.getCacheWrapper().initCache(this.getKeyGeneratorDAO()); + if (this.isLockEnabled()) { + this.getCacheWrapper().initCache(this.getKeyGeneratorDAO()); + } } @Override public void releaseTenantAware() { - this.getCacheWrapper().release(); + if (this.isLockEnabled()) { + this.getCacheWrapper().release(); + } } - /** * Restituisce la chiave univoca corrente. * @@ -64,7 +88,41 @@ public void releaseTenantAware() { */ @Override public int getUniqueKeyCurrentValue() throws EntException { - return this.getCacheWrapper().getAndIncrementUniqueKeyCurrentValue(this.getKeyGeneratorDAO()); + IKeyGeneratorDAO keyGeneratorDAO = this.getKeyGeneratorDAO(); + if (this.isLockEnabled()) { + return this.doOnLock(() -> + this.getCacheWrapper().getAndIncrementUniqueKeyCurrentValue(keyGeneratorDAO)); + } else { + return keyGeneratorDAO.getNextUniqueKey(); + } + } + + + private T doOnLock(Supplier supplier) throws GlobalLockEntException { + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - opening cache lock - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); + IGlobalLockManager globalLockManager = this.getGlobalLockManager(); + String lockKey = this.getLockKey(); + String token = globalLockManager.lock(lockKey, "system", + KEYGEN_LOCK_DURATION, + KEYGEN_LOCK_MAX_WAIT); + + T result = supplier.get(); + + globalLockManager.unlock(lockKey, token); + + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - closing cache lock - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); + return result; + } + + private String getLockKey() { + String tenant = ApsTenantApplicationUtils.getTenant().orElse("primary"); + return tenant + "_keygen"; + } + + private boolean isLockEnabled() { + return KEYGEN_LOCK_ENABLED; } protected IKeyGeneratorDAO getKeyGeneratorDAO() { @@ -83,4 +141,13 @@ public void setCacheWrapper(IKeyGeneratorManagerCacheWrapper cacheWrapper) { this.cacheWrapper = cacheWrapper; } + protected IGlobalLockManager getGlobalLockManager() { + return globalLockManager; + } + + public void setGlobalLockManager(IGlobalLockManager globalLockManager) { + this.globalLockManager = globalLockManager; + } + + } diff --git a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/cache/KeyGeneratorManagerCacheWrapper.java b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/cache/KeyGeneratorManagerCacheWrapper.java index 3e807fa9ad..85797bad8a 100644 --- a/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/cache/KeyGeneratorManagerCacheWrapper.java +++ b/engine/src/main/java/com/agiletec/aps/system/services/keygenerator/cache/KeyGeneratorManagerCacheWrapper.java @@ -13,8 +13,10 @@ */ package com.agiletec.aps.system.services.keygenerator.cache; +import com.agiletec.aps.system.ApsSystemUtils; import com.agiletec.aps.system.common.AbstractCacheWrapper; import com.agiletec.aps.system.services.keygenerator.IKeyGeneratorDAO; +import com.agiletec.aps.util.ApsTenantApplicationUtils; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.ent.util.EntLogging.EntLogFactory; import org.springframework.cache.Cache; @@ -26,6 +28,8 @@ public class KeyGeneratorManagerCacheWrapper extends AbstractCacheWrapper implem @Override public void release() { Cache cache = this.getCache(); + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - release - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); cache.evict(IKeyGeneratorManagerCacheWrapper.CURRENT_KEY); } @@ -38,30 +42,44 @@ protected String getCacheName() { public void initCache(IKeyGeneratorDAO keyGeneratorDAO) { Integer value = keyGeneratorDAO.getUniqueKey(); Cache cache = this.getCache(); + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - initCache - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); this.insertObjectsOnCache(cache, value); } @Override public synchronized int getAndIncrementUniqueKeyCurrentValue(IKeyGeneratorDAO keyGeneratorDAO) { Cache cache = this.getCache(); + // apro il lock + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - start getAndIncrement - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); Integer currentValue = this.get(cache, CURRENT_KEY, Integer.class); Integer nextValue = currentValue + 1; this.insertObjectsOnCache(cache, nextValue); keyGeneratorDAO.updateKey(nextValue); + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - end getAndIncrement - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); + // chiudo il lock return nextValue; } @Override public int getUniqueKeyCurrentValue() { + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - getUniqueKey - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); return this.get(this.getCache(), CURRENT_KEY, Integer.class); } @Override public void updateCurrentKey(int value) { + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - updateCurrentKey - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); this.insertObjectsOnCache(this.getCache(), value); } private void insertObjectsOnCache(Cache cache, Integer value) { + ApsSystemUtils.ApsDeepDebug.print("CACHE:TENANT", String.format("%s - insertObjectsOnCache - value %s - tenant %s", + this.getClass().getSimpleName(), value, ApsTenantApplicationUtils.getTenant().orElse("primary"))); cache.put(IKeyGeneratorManagerCacheWrapper.CURRENT_KEY, value); logger.trace("current key is now {}", value); } diff --git a/engine/src/main/java/org/entando/entando/aps/system/services/actionlog/ActionLogAppenderThread.java b/engine/src/main/java/org/entando/entando/aps/system/services/actionlog/ActionLogAppenderThread.java index a38e3c3724..9b3fdfbfb8 100644 --- a/engine/src/main/java/org/entando/entando/aps/system/services/actionlog/ActionLogAppenderThread.java +++ b/engine/src/main/java/org/entando/entando/aps/system/services/actionlog/ActionLogAppenderThread.java @@ -13,6 +13,8 @@ */ package org.entando.entando.aps.system.services.actionlog; +import com.agiletec.aps.system.ApsSystemUtils; +import com.agiletec.aps.util.ApsTenantApplicationUtils; import org.entando.entando.aps.system.services.actionlog.model.ActionLogRecord; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,7 +38,11 @@ public ActionLogAppenderThread(ActionLogRecord actionRecordToAdd, @Override public void run() { try { + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - start - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); this.actionLogManager.addActionRecordByThread(this.actionRecordToAdd); + ApsSystemUtils.ApsDeepDebug.print("TENANT", String.format("%s - end - tenant %s", + this.getClass().getSimpleName(), ApsTenantApplicationUtils.getTenant().orElse("primary"))); } catch (Throwable t) { logger.error("error in run", t); } diff --git a/engine/src/main/java/org/entando/entando/aps/system/services/sync/GlobalLockManager.java b/engine/src/main/java/org/entando/entando/aps/system/services/sync/GlobalLockManager.java new file mode 100644 index 0000000000..b8c211fb24 --- /dev/null +++ b/engine/src/main/java/org/entando/entando/aps/system/services/sync/GlobalLockManager.java @@ -0,0 +1,34 @@ +package org.entando.entando.aps.system.services.sync; + +import org.entando.entando.aps.system.services.sync.exception.GlobalLockEntException; + +import java.time.Duration; +import java.util.Map; +import java.util.Optional; + +/** + * Default no-op implementation. Overridden by the redis-plugin's + * GlobalLockManagerFactoryBean via XML bean definition when Redis is active. + */ +public class GlobalLockManager implements IGlobalLockManager { + + @Override + public String tryLock(String key, String property, Duration duration) throws GlobalLockEntException { + return ""; + } + + @Override + public String lock(String key, String property, Duration duration, Duration maxWait) throws GlobalLockEntException { + return ""; + } + + @Override + public boolean unlock(String key, String token) throws GlobalLockEntException { + return false; + } + + @Override + public Optional> verifyLock(String key) throws GlobalLockEntException { + return Optional.empty(); + } +} diff --git a/engine/src/main/java/org/entando/entando/aps/system/services/sync/IGlobalLockManager.java b/engine/src/main/java/org/entando/entando/aps/system/services/sync/IGlobalLockManager.java new file mode 100644 index 0000000000..fcb63a4c2e --- /dev/null +++ b/engine/src/main/java/org/entando/entando/aps/system/services/sync/IGlobalLockManager.java @@ -0,0 +1,34 @@ +package org.entando.entando.aps.system.services.sync; + +import org.entando.entando.aps.system.services.sync.exception.GlobalLockEntException; + +import java.time.Duration; +import java.util.Map; +import java.util.Optional; + +public interface IGlobalLockManager { + + String BEAN_ID = "GlobalLockManager"; + + /** + * Lock property: creation data + */ + String LOCk_PROP_CREATED_AT = "createdAt"; + /** + * Lock property:: general purpose data + */ + String LOCK_PROP_DATA = "gpData"; + /** + * UUID of the lock, returned when the lock is created successfully + */ + String LOCK_PROP_TOKEN = "token"; + + String tryLock(String key, String property, Duration duration) throws GlobalLockEntException; + + String lock(String key, String property, Duration duration, Duration maxWait) throws GlobalLockEntException; + + boolean unlock(String key, String token) throws GlobalLockEntException; + + Optional> verifyLock(String key) throws GlobalLockEntException; + +} diff --git a/engine/src/main/java/org/entando/entando/aps/system/services/sync/exception/GlobalLockEntException.java b/engine/src/main/java/org/entando/entando/aps/system/services/sync/exception/GlobalLockEntException.java new file mode 100644 index 0000000000..7ed0fa0428 --- /dev/null +++ b/engine/src/main/java/org/entando/entando/aps/system/services/sync/exception/GlobalLockEntException.java @@ -0,0 +1,15 @@ +package org.entando.entando.aps.system.services.sync.exception; + +import org.entando.entando.ent.exception.EntException; + +public class GlobalLockEntException extends EntException { + + public GlobalLockEntException(String message) { + super(message); + } + + public GlobalLockEntException(String message, Throwable cause) { + super(message, cause); + } + +} diff --git a/engine/src/main/resources/spring/aps/managers/baseManagersConfig.xml b/engine/src/main/resources/spring/aps/managers/baseManagersConfig.xml index 432eb0fb0d..4837304832 100644 --- a/engine/src/main/resources/spring/aps/managers/baseManagersConfig.xml +++ b/engine/src/main/resources/spring/aps/managers/baseManagersConfig.xml @@ -15,12 +15,13 @@ http://www.springframework.org/schema/task http://www.springframework.org/schema/task/spring-task.xsd"> - + - + + @@ -359,6 +360,7 @@ + dao.getNextUniqueKey()); + + verify(connection).setAutoCommit(false); + verify(connection).rollback(); + verify(connection, never()).commit(); + } + + @Test + void getNextUniqueKey_shouldCloseConnection() throws Exception { + when(connection.createStatement()).thenReturn(statement); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(true); + when(resultSet.getInt(1)).thenReturn(5); + when(connection.prepareStatement(anyString())).thenReturn(preparedStatement); + + dao.getNextUniqueKey(); + + verify(connection).close(); + } + + @Test + void getNextUniqueKey_shouldCloseConnectionOnError() throws Exception { + when(connection.createStatement()).thenReturn(statement); + when(statement.executeQuery(anyString())).thenThrow(new SQLException("DB error")); + + assertThrows(RuntimeException.class, () -> dao.getNextUniqueKey()); + + verify(connection).close(); + } + + @Test + void getNextUniqueKey_shouldIncrementKeyByOne() throws Exception { + when(connection.createStatement()).thenReturn(statement); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(true); + when(resultSet.getInt(1)).thenReturn(42); + when(connection.prepareStatement(anyString())).thenReturn(preparedStatement); + + int result = dao.getNextUniqueKey(); + + assertEquals(43, result); + verify(preparedStatement).setInt(1, 43); + verify(preparedStatement).executeUpdate(); + } + + @Test + void getNextUniqueKey_shouldReturnOneWhenTableEmpty() throws Exception { + when(connection.createStatement()).thenReturn(statement); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + when(connection.prepareStatement(anyString())).thenReturn(preparedStatement); + + int result = dao.getNextUniqueKey(); + + assertEquals(1, result); + } + + @Test + void getUniqueKey_shouldReturnCurrentKey() throws Exception { + when(connection.createStatement()).thenReturn(statement); + when(statement.executeQuery(anyString())).thenReturn(resultSet); + when(resultSet.next()).thenReturn(true); + when(resultSet.getInt(1)).thenReturn(99); + + int result = dao.getUniqueKey(); + + assertEquals(99, result); + verify(connection).close(); + } + + @Test + void updateKey_shouldUpdateAndCloseConnection() throws Exception { + when(connection.prepareStatement(anyString())).thenReturn(preparedStatement); + + dao.updateKey(55); + + verify(preparedStatement).setInt(1, 55); + verify(preparedStatement).executeUpdate(); + verify(connection).close(); + } +} \ No newline at end of file diff --git a/engine/src/test/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManagerTest.java b/engine/src/test/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManagerTest.java new file mode 100644 index 0000000000..29434bb34e --- /dev/null +++ b/engine/src/test/java/com/agiletec/aps/system/services/keygenerator/KeyGeneratorManagerTest.java @@ -0,0 +1,121 @@ +package com.agiletec.aps.system.services.keygenerator; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Field; + +import com.agiletec.aps.system.services.keygenerator.cache.IKeyGeneratorManagerCacheWrapper; +import org.entando.entando.aps.system.services.sync.IGlobalLockManager; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class KeyGeneratorManagerTest { + + @Mock + private IKeyGeneratorDAO keyGeneratorDAO; + @Mock + private IKeyGeneratorManagerCacheWrapper cacheWrapper; + @Mock + private IGlobalLockManager globalLockManager; + + private KeyGeneratorManager manager; + private boolean originalLockEnabled; + + @BeforeEach + void setUp() throws Exception { + originalLockEnabled = getStaticField(); + manager = new KeyGeneratorManager(); + manager.setKeyGeneratorDAO(keyGeneratorDAO); + manager.setCacheWrapper(cacheWrapper); + manager.setGlobalLockManager(globalLockManager); + } + + @AfterEach + void tearDown() throws Exception { + setStaticField(originalLockEnabled); + } + + @Test + void getUniqueKeyCurrentValue_lockDisabled_shouldUseDaoDirectly() throws Exception { + setStaticField(false); + when(keyGeneratorDAO.getNextUniqueKey()).thenReturn(42); + + int result = manager.getUniqueKeyCurrentValue(); + + assertEquals(42, result); + verify(keyGeneratorDAO).getNextUniqueKey(); + verify(cacheWrapper, never()).getAndIncrementUniqueKeyCurrentValue(any()); + } + + @Test + void getUniqueKeyCurrentValue_lockEnabled_shouldUseCacheWithLock() throws Exception { + setStaticField(true); + when(globalLockManager.lock(anyString(), anyString(), any(), any())).thenReturn("token123"); + when(cacheWrapper.getAndIncrementUniqueKeyCurrentValue(keyGeneratorDAO)).thenReturn(99); + + int result = manager.getUniqueKeyCurrentValue(); + + assertEquals(99, result); + verify(globalLockManager).lock(anyString(), anyString(), any(), any()); + verify(cacheWrapper).getAndIncrementUniqueKeyCurrentValue(keyGeneratorDAO); + verify(globalLockManager).unlock(anyString(), anyString()); + } + + @Test + void initTenantAware_lockDisabled_shouldNotInitCache() throws Exception { + setStaticField(false); + + manager.initTenantAware(); + + verify(cacheWrapper, never()).initCache(any()); + } + + @Test + void initTenantAware_lockEnabled_shouldInitCache() throws Exception { + setStaticField(true); + + manager.initTenantAware(); + + verify(cacheWrapper).initCache(keyGeneratorDAO); + } + + @Test + void releaseTenantAware_lockDisabled_shouldNotReleaseCache() throws Exception { + setStaticField(false); + + manager.releaseTenantAware(); + + verify(cacheWrapper, never()).release(); + } + + @Test + void releaseTenantAware_lockEnabled_shouldReleaseCache() throws Exception { + setStaticField(true); + + manager.releaseTenantAware(); + + verify(cacheWrapper).release(); + } + + private static boolean getStaticField() throws Exception { + Field field = KeyGeneratorManager.class.getDeclaredField("KEYGEN_LOCK_ENABLED"); + field.setAccessible(true); + return (boolean) field.get(null); + } + + private static void setStaticField(boolean value) throws Exception { + Field field = KeyGeneratorManager.class.getDeclaredField("KEYGEN_LOCK_ENABLED"); + field.setAccessible(true); + field.set(null, value); + } +} \ No newline at end of file diff --git a/engine/src/test/java/org/entando/entando/aps/system/services/sync/GlobalLockManagerTest.java b/engine/src/test/java/org/entando/entando/aps/system/services/sync/GlobalLockManagerTest.java new file mode 100644 index 0000000000..7c40373eb0 --- /dev/null +++ b/engine/src/test/java/org/entando/entando/aps/system/services/sync/GlobalLockManagerTest.java @@ -0,0 +1,69 @@ +package org.entando.entando.aps.system.services.sync; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.util.Map; +import java.util.Optional; + +import org.entando.entando.aps.system.services.sync.exception.GlobalLockEntException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class GlobalLockManagerTest { + + private GlobalLockManager lockManager; + + @BeforeEach + void setUp() { + lockManager = new GlobalLockManager(); + } + + @Test + void tryLock_shouldReturnEmptyString() throws GlobalLockEntException { + String result = lockManager.tryLock("testKey", "testProp", Duration.ofMinutes(1)); + assertEquals("", result); + } + + @Test + void lock_shouldReturnEmptyString() throws GlobalLockEntException { + String result = lockManager.lock("testKey", "testProp", Duration.ofMinutes(1), Duration.ofSeconds(5)); + assertEquals("", result); + } + + @Test + void unlock_shouldReturnFalse() throws GlobalLockEntException { + boolean result = lockManager.unlock("testKey", "testToken"); + assertFalse(result); + } + + @Test + void verifyLock_shouldReturnEmpty() throws GlobalLockEntException { + Optional> result = lockManager.verifyLock("testKey"); + assertTrue(result.isEmpty()); + } + + @Test + void tryLock_withNullKey_shouldReturnEmptyString() throws GlobalLockEntException { + String result = lockManager.tryLock(null, "prop", Duration.ofMinutes(1)); + assertEquals("", result); + } + + @Test + void lock_withNullKey_shouldReturnEmptyString() throws GlobalLockEntException { + String result = lockManager.lock(null, null, Duration.ofMinutes(1), Duration.ofSeconds(1)); + assertEquals("", result); + } + + @Test + void unlock_withNullArgs_shouldReturnFalse() throws GlobalLockEntException { + assertFalse(lockManager.unlock(null, null)); + } + + @Test + void verifyLock_withNullKey_shouldReturnEmpty() throws GlobalLockEntException { + assertTrue(lockManager.verifyLock(null).isEmpty()); + } +} \ No newline at end of file diff --git a/redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManager.java b/redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManager.java new file mode 100644 index 0000000000..22f74e5fb2 --- /dev/null +++ b/redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManager.java @@ -0,0 +1,172 @@ +package org.entando.entando.plugins.jpredis.aps.system.redis.sync; + +import com.agiletec.aps.system.ApsSystemUtils.ApsDeepDebug; +import io.lettuce.core.RedisClient; +import io.lettuce.core.RedisException; +import io.lettuce.core.ScriptOutputType; +import io.lettuce.core.api.StatefulRedisConnection; +import io.lettuce.core.api.sync.RedisCommands; +import org.apache.commons.lang3.StringUtils; +import org.entando.entando.aps.system.services.sync.IGlobalLockManager; +import org.entando.entando.aps.system.services.sync.exception.GlobalLockEntException; +import org.entando.entando.ent.util.EntLogging.EntLogger; +import org.entando.entando.ent.util.EntLogging.EntLogFactory; + + +import java.security.SecureRandom; +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ThreadLocalRandom; + +public class GlobalLockManager implements IGlobalLockManager { + + private static final EntLogger log = EntLogFactory.getSanitizedLogger((GlobalLockManager.class)); + + public static final String ENTANDO_GLOBAL_LOCK = "Entando_GlobalLock::"; + public static final int LOCK_TOKEN_SIZE = 16; + + private final RedisClient redisClient; + private final SecureRandom rnd = new SecureRandom(); + + private static final String LOCK_LUA = + "local created = redis.call('HSETNX', KEYS[1], '" + LOCK_PROP_TOKEN + "', ARGV[1]) " + + "if created == 1 then " + + "redis.call('PEXPIRE', KEYS[1], ARGV[4]) " + + "redis.call('HSET', KEYS[1], '" + LOCk_PROP_CREATED_AT + "', ARGV[2]) " + + "redis.call('HSET', KEYS[1], '" + LOCK_PROP_DATA + "', ARGV[3]) " + + "return ARGV[1] " + + "end " + + "return created "; + + private static final String UNLOCK_LUA = + "if redis.call('hget', KEYS[1], 'token') == ARGV[1] then " + + " return redis.call('del', KEYS[1]) " + + "else return 0 end"; + + public GlobalLockManager(RedisClient redisClient) { + this.redisClient = redisClient; + ApsDeepDebug.print("global-lock", "Redis global-lock enabled"); + } + + private StatefulRedisConnection redisSyncConnection() { + return redisClient.connect(); + } + + private RedisCommands redisCommands(StatefulRedisConnection conn) { + return conn.sync(); + } + + private RedisCommands getSyncCommand() { + StatefulRedisConnection conn = redisSyncConnection(); + return redisCommands(conn); + } + + @Override + public String tryLock(String key, String property, Duration duration) throws GlobalLockEntException { + try { + if (StringUtils.isNotBlank(key)) { + final String token = generateToken(); + final RedisCommands cmd = getSyncCommand(); + + key = formatLockName(key); + String res = cmd.eval(LOCK_LUA, ScriptOutputType.VALUE, new String[]{key}, + token, + Instant.now().toString(), + property, + String.valueOf(duration.toMillis()) + ); + return res; + +// Boolean created = cmd.hsetnx(key, LOCK_PROP_TOKEN, token); +// +// if (created) { +// cmd.hset(key, Map.of( +// LOCk_PROP_CREATED_AT, Instant.now().toString(), +// LOCK_PROP_DATA, property)); +// cmd.pexpire(key, duration.toMillis()); +// return token; +// } + } + return null; + } catch (RedisException e ) { + throw new GlobalLockEntException("communication error with Redis detected while locking", e.getCause()); + } + } + + @Override + public String lock(String key, String property, Duration duration, Duration maxWait) throws GlobalLockEntException { + long end = System.nanoTime() + maxWait.toNanos(); + + while (System.nanoTime() < end) { + final String token = tryLock(key, property, duration); + + if (StringUtils.isNotBlank(token)) { + return token; + } + try { + // retry with jitter + Thread.sleep(ThreadLocalRandom.current().nextLong(20, 61)); + } catch (InterruptedException e) { + log.warn("Thread interrupted exception!", e); + } + } + return null; + } + + @Override + public boolean unlock(String key, String token) throws GlobalLockEntException { + try { + if (StringUtils.isNotBlank(key) + && StringUtils.isNotBlank(token)) { + final RedisCommands cmd = getSyncCommand(); + + key = formatLockName(key); + Long res = cmd.eval(UNLOCK_LUA, ScriptOutputType.INTEGER, new String[]{key}, token); + ApsDeepDebug.print("global-lock", "releasing lock " + key + ": status " + (res != null && res == 1L)); + return (res != null && res == 1L); + } + return false; + } catch(RedisException e ) { + throw new GlobalLockEntException("communication error with Redis detected while unlocking", e.getCause()); + } + } + + @Override + public Optional> verifyLock(String key) throws GlobalLockEntException { + try { + if (StringUtils.isNotBlank(key)) { + key = formatLockName(key); + + final RedisCommands cmd = getSyncCommand(); + final Map fields = cmd.hgetall(key); + + if (fields == null || fields.isEmpty()) { + return Optional.empty(); + } + fields.remove(LOCK_PROP_TOKEN); + + ApsDeepDebug.print("global-lock-verify", "verifying lock " + key + ": has payload " + !fields.isEmpty()); + return Optional.of(fields); + } + return Optional.empty(); + } catch (RedisException e ) { + throw new GlobalLockEntException("communication error with Redis detected while verifying lock", e.getCause()); + } + } + + private String generateToken() { + byte[] bytes = new byte[LOCK_TOKEN_SIZE]; + + rnd.nextBytes(bytes); + StringBuilder sb = new StringBuilder(32); + for (byte x : bytes) sb.append(String.format("%02x", x)); + return sb.toString(); + } + + private String formatLockName(String key) { + return ENTANDO_GLOBAL_LOCK + key; + } + +} diff --git a/redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerFactoryBean.java b/redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerFactoryBean.java new file mode 100644 index 0000000000..273f2e16a5 --- /dev/null +++ b/redis-plugin/src/main/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerFactoryBean.java @@ -0,0 +1,32 @@ +package org.entando.entando.plugins.jpredis.aps.system.redis.sync; + +import io.lettuce.core.RedisClient; +import org.entando.entando.aps.system.services.cache.RedisEnvironmentVariables; +import org.entando.entando.aps.system.services.sync.IGlobalLockManager; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.FactoryBean; + +public class GlobalLockManagerFactoryBean implements FactoryBean, BeanFactoryAware { + + private BeanFactory beanFactory; + + @Override + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + this.beanFactory = beanFactory; + } + + @Override + public IGlobalLockManager getObject() { + if (RedisEnvironmentVariables.active()) { + return new GlobalLockManager(beanFactory.getBean(RedisClient.class)); + } + return new org.entando.entando.aps.system.services.sync.GlobalLockManager(); + } + + @Override + public Class getObjectType() { + return IGlobalLockManager.class; + } +} \ No newline at end of file diff --git a/redis-plugin/src/main/resources/spring/plugins/jpredis/aps/baseManagersConfig.xml b/redis-plugin/src/main/resources/spring/plugins/jpredis/aps/baseManagersConfig.xml index d385c887be..914bc01a37 100644 --- a/redis-plugin/src/main/resources/spring/plugins/jpredis/aps/baseManagersConfig.xml +++ b/redis-plugin/src/main/resources/spring/plugins/jpredis/aps/baseManagersConfig.xml @@ -15,5 +15,7 @@ + + diff --git a/redis-plugin/src/test/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerTest.java b/redis-plugin/src/test/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerTest.java new file mode 100644 index 0000000000..e73924526c --- /dev/null +++ b/redis-plugin/src/test/java/org/entando/entando/plugins/jpredis/aps/system/redis/sync/GlobalLockManagerTest.java @@ -0,0 +1,223 @@ +package org.entando.entando.plugins.jpredis.aps.system.redis.sync; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +import io.lettuce.core.RedisClient; +import io.lettuce.core.RedisException; +import io.lettuce.core.ScriptOutputType; +import io.lettuce.core.api.StatefulRedisConnection; +import io.lettuce.core.api.sync.RedisCommands; +import org.entando.entando.aps.system.services.sync.IGlobalLockManager; +import org.entando.entando.aps.system.services.sync.exception.GlobalLockEntException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class GlobalLockManagerTest { + + @Mock + private RedisClient redisClient; + @Mock + private StatefulRedisConnection connection; + @Mock + private RedisCommands commands; + + private GlobalLockManager lockManager; + + @BeforeEach + void setUp() { + lockManager = new GlobalLockManager(redisClient); + } + + private void stubRedisConnection() { + when(redisClient.connect()).thenReturn(connection); + when(connection.sync()).thenReturn(commands); + } + + @Test + void tryLock_shouldEvalLuaScriptAndReturnToken() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.VALUE), any(String[].class), + any())).thenReturn("generated-token"); + + String result = lockManager.tryLock("myKey", "myProp", Duration.ofMinutes(5)); + + assertEquals("generated-token", result); + verify(commands).eval(any(String.class), eq(ScriptOutputType.VALUE), + any(String[].class), any()); + } + + @Test + void tryLock_withBlankKey_shouldReturnNull() throws GlobalLockEntException { + assertNull(lockManager.tryLock("", "prop", Duration.ofMinutes(1))); + assertNull(lockManager.tryLock(null, "prop", Duration.ofMinutes(1))); + } + + @Test + void tryLock_shouldPrefixKeyWithGlobalLockNamespace() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.VALUE), any(String[].class), + any())).thenReturn("token"); + + lockManager.tryLock("myKey", "prop", Duration.ofMinutes(1)); + + verify(commands).eval(any(String.class), eq(ScriptOutputType.VALUE), + eq(new String[]{GlobalLockManager.ENTANDO_GLOBAL_LOCK + "myKey"}), + any()); + } + + @Test + void tryLock_onRedisException_shouldThrowGlobalLockEntException() { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.VALUE), any(String[].class), + any())).thenThrow(new RedisException("connection lost")); + + assertThrows(GlobalLockEntException.class, + () -> lockManager.tryLock("key", "prop", Duration.ofMinutes(1))); + } + + @Test + void lock_shouldReturnTokenOnFirstSuccessfulTryLock() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.VALUE), any(String[].class), + any())).thenReturn("token-abc"); + + String result = lockManager.lock("key", "prop", Duration.ofMinutes(5), Duration.ofSeconds(1)); + + assertEquals("token-abc", result); + } + + @Test + void lock_shouldReturnNullWhenTimeoutExceeded() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.VALUE), any(String[].class), + any())).thenReturn(null); + + String result = lockManager.lock("key", "prop", Duration.ofMinutes(5), Duration.ofMillis(50)); + + assertNull(result); + } + + @Test + void unlock_shouldEvalUnlockLuaScript() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.INTEGER), any(String[].class), + any())).thenReturn(1L); + + boolean result = lockManager.unlock("myKey", "myToken"); + + assertTrue(result); + verify(commands).eval(any(String.class), eq(ScriptOutputType.INTEGER), + eq(new String[]{GlobalLockManager.ENTANDO_GLOBAL_LOCK + "myKey"}), + eq("myToken")); + } + + @Test + void unlock_shouldReturnFalseWhenRedisReturnsZero() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.INTEGER), any(String[].class), + any())).thenReturn(0L); + + assertFalse(lockManager.unlock("key", "wrong-token")); + } + + @Test + void unlock_shouldReturnFalseWhenRedisReturnsNull() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.INTEGER), any(String[].class), + any())).thenReturn(null); + + assertFalse(lockManager.unlock("key", "token")); + } + + @Test + void unlock_withBlankKey_shouldReturnFalse() throws GlobalLockEntException { + assertFalse(lockManager.unlock("", "token")); + assertFalse(lockManager.unlock(null, "token")); + } + + @Test + void unlock_withBlankToken_shouldReturnFalse() throws GlobalLockEntException { + assertFalse(lockManager.unlock("key", "")); + assertFalse(lockManager.unlock("key", null)); + } + + @Test + void unlock_onRedisException_shouldThrowGlobalLockEntException() { + stubRedisConnection(); + when(commands.eval(any(String.class), eq(ScriptOutputType.INTEGER), any(String[].class), + any())).thenThrow(new RedisException("connection lost")); + + assertThrows(GlobalLockEntException.class, + () -> lockManager.unlock("key", "token")); + } + + @Test + void verifyLock_shouldReturnFieldsWithoutToken() throws GlobalLockEntException { + stubRedisConnection(); + Map fields = new HashMap<>(); + fields.put(IGlobalLockManager.LOCK_PROP_TOKEN, "secret-token"); + fields.put(IGlobalLockManager.LOCk_PROP_CREATED_AT, "2026-01-01T00:00:00Z"); + fields.put(IGlobalLockManager.LOCK_PROP_DATA, "system"); + + when(commands.hgetall(GlobalLockManager.ENTANDO_GLOBAL_LOCK + "myKey")).thenReturn(fields); + + Optional> result = lockManager.verifyLock("myKey"); + + assertTrue(result.isPresent()); + assertFalse(result.get().containsKey(IGlobalLockManager.LOCK_PROP_TOKEN)); + assertEquals("2026-01-01T00:00:00Z", result.get().get(IGlobalLockManager.LOCk_PROP_CREATED_AT)); + assertEquals("system", result.get().get(IGlobalLockManager.LOCK_PROP_DATA)); + } + + @Test + void verifyLock_shouldReturnEmptyWhenNoLockExists() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.hgetall(any())).thenReturn(new HashMap<>()); + + Optional> result = lockManager.verifyLock("noSuchKey"); + + assertTrue(result.isEmpty()); + } + + @Test + void verifyLock_shouldReturnEmptyWhenNullFields() throws GlobalLockEntException { + stubRedisConnection(); + when(commands.hgetall(any())).thenReturn(null); + + Optional> result = lockManager.verifyLock("key"); + + assertTrue(result.isEmpty()); + } + + @Test + void verifyLock_withBlankKey_shouldReturnEmpty() throws GlobalLockEntException { + assertTrue(lockManager.verifyLock("").isEmpty()); + assertTrue(lockManager.verifyLock(null).isEmpty()); + } + + @Test + void verifyLock_onRedisException_shouldThrowGlobalLockEntException() { + stubRedisConnection(); + when(commands.hgetall(any())).thenThrow(new RedisException("timeout")); + + assertThrows(GlobalLockEntException.class, + () -> lockManager.verifyLock("key")); + } +} \ No newline at end of file