From 408ae7ddd54a4b4819f67f67143fc837adbaff05 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Tue, 3 Jun 2025 21:20:15 +0300 Subject: [PATCH 01/30] AMP-30983: images missing on develop --- .../java/org/dgfoundation/amp/onepager/util/ActivityUtil.java | 3 +++ amp/src/main/resources/hibernate.cfg.xml | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java index 207728fe54b..a14261fcb5a 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java @@ -178,6 +178,7 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, * saves a new version of an activity * returns newActivity */ + public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, Collection translations, List cumulativeTranslations, AmpTeamMember ampCurrentMember, boolean draft, @@ -234,6 +235,7 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, } session.flush(); + session.clear(); } catch (CloneNotSupportedException e) { logger.error("Can't clone current Activity: ", e); @@ -307,6 +309,7 @@ public static AmpActivityVersion saveActivityNewVersion(AmpActivityVersion a, session.merge(a); } session.flush(); + session.clear(); updatePerformanceRules(oldA, a); diff --git a/amp/src/main/resources/hibernate.cfg.xml b/amp/src/main/resources/hibernate.cfg.xml index ef0d674d819..3b9d209ae6f 100644 --- a/amp/src/main/resources/hibernate.cfg.xml +++ b/amp/src/main/resources/hibernate.cfg.xml @@ -6,11 +6,12 @@ java:comp/env/ampDS org.dgfoundation.amp.ar.viewfetcher.AmpPostgresDialect &dialect; - org.hibernate.context.internal.ThreadLocalSessionContext + thread true create 0 20 + AUTO true true org.hibernate.cache.ehcache.SingletonEhCacheRegionFactory From ee6b5fc90754121d86ed5d286e77ed73a369413f Mon Sep 17 00:00:00 2001 From: brianbrix Date: Tue, 3 Jun 2025 22:01:16 +0300 Subject: [PATCH 02/30] AMP-30971: Thread issues for updating --- .../HibernateRequestCycleListener.java | 36 +++++++++++++++++++ .../amp/onepager/OnePagerApp.java | 1 + .../amp/onepager/models/AmpActivityModel.java | 6 ++-- .../amp/onepager/util/ActivityUtil.java | 2 +- .../persistence/PersistenceManager.java | 22 +++++------- 5 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java b/amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java new file mode 100644 index 00000000000..bb5c72cec14 --- /dev/null +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java @@ -0,0 +1,36 @@ +package org.dgfoundation.amp.onepager; + +import org.apache.wicket.request.cycle.AbstractRequestCycleListener; +import org.apache.wicket.request.cycle.RequestCycle; +import org.digijava.kernel.persistence.PersistenceManager; +import org.hibernate.Session; + +public class HibernateRequestCycleListener extends AbstractRequestCycleListener { + + @Override + public void onBeginRequest(RequestCycle cycle) { + Session session = PersistenceManager.openNewSession(); + session.beginTransaction(); + PersistenceManager.setCurrentSession(session); + } + + @Override + public void onEndRequest(RequestCycle cycle) { + Session session = PersistenceManager.getSession(); + try { + if (session != null && session.getTransaction().isActive()) { + session.getTransaction().commit(); + } + } catch (Throwable t) { + if (session.getTransaction().isActive()) { + session.getTransaction().rollback(); + } + throw new RuntimeException("Error during session commit", t); + } finally { + if (session != null && session.isOpen()) { + session.close(); + } + PersistenceManager.clearCurrentSession(); + } + } +} diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/OnePagerApp.java b/amp/src/main/java/org/dgfoundation/amp/onepager/OnePagerApp.java index dbbb3902cf4..bd32cb3652a 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/OnePagerApp.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/OnePagerApp.java @@ -69,6 +69,7 @@ public void init() { WicketSource.configure(this); IS_DEVELOPMENT_MODE = true; } + getRequestCycleListeners().add(new HibernateRequestCycleListener()); //getResourceSettings().setStripJavaScriptCommentsAndWhitespace(true); //getResourceSettings().setAddLastModifiedTimeToResourceReferenceUrl(true); diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java b/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java index ded00f87de3..a48c4b57e03 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java @@ -74,8 +74,8 @@ public void beginConversation(boolean reset) { translationHashMap = new HashMap(); } - Session ses = getHibernateSession(reset); - ses.clear(); +// Session ses = getHibernateSession(reset); +// ses.clear(); } @@ -87,7 +87,7 @@ public void beginConversation(boolean reset) { * @return */ public static Session getHibernateSession(){ - return getHibernateSession(false); + return PersistenceManager.getSession(); } private static Session getHibernateSession(boolean reset) { diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java index a14261fcb5a..f24e4f043e5 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java @@ -97,7 +97,7 @@ public static void saveActivity(AmpActivityModel am, boolean draft,boolean rejec am.setObject(newA); ActivityGatekeeper.unlockActivity(String.valueOf(am.getId()), am.getEditingKey()); - AmpActivityModel.endConversation(); +// AmpActivityModel.endConversation(); } /** diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index a8ed85d19f5..6464e24b4bc 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -171,6 +171,7 @@ public static Session openNewSession() { public static ClassMetadata getClassMetadata(Class clazz) { return sf.getClassMetadata(clazz); } + private static final ThreadLocal threadSession = new ThreadLocal<>(); public static PersistentClass getClassMapping(Class clazz) { @@ -572,21 +573,14 @@ public static T supplyInTransaction(Supplier supplier) { * upon creating a new session, a transaction is created. */ public static Session getSession() { - boolean currentSessionIsManaged = CURRENT_SESSION_IS_MANAGED.get(); - if (!currentSessionIsManaged) { - throw new IllegalStateException("Called outside of managed session context."); - } - Session sess = sf().getCurrentSession(); - sess.setFlushMode(FlushModeType.AUTO); - - Transaction transaction = sess.getTransaction(); - if (transaction == null || !transaction.isActive()) { - sess.beginTransaction(); - } -// sess.clear(); + return threadSession.get(); + } - addSessionToStackTraceMap(sess); - return sess; + public static void setCurrentSession(Session session) { + threadSession.set(session); + } + public static void clearCurrentSession() { + threadSession.remove(); } /** From 3e07abdb6725dab2844d5be04d18601ba8ceec0e Mon Sep 17 00:00:00 2001 From: brianbrix Date: Tue, 3 Jun 2025 22:17:50 +0300 Subject: [PATCH 03/30] AMP-30971: Thread issues for updating --- .../org/digijava/kernel/persistence/PersistenceManager.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 6464e24b4bc..42fe36ff6c3 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -573,6 +573,11 @@ public static T supplyInTransaction(Supplier supplier) { * upon creating a new session, a transaction is created. */ public static Session getSession() { + Session session = threadSession.get(); + if (session == null) { + session = sf.openSession(); + threadSession.set(session); + } return threadSession.get(); } From 1a088c6d9a33c0b567df6ddd339005e1e9bf0b59 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Tue, 3 Jun 2025 22:33:10 +0300 Subject: [PATCH 04/30] AMP-30971: Thread issues for updating --- .../persistence/PersistenceManager.java | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 42fe36ff6c3..cbf862e623b 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -555,15 +555,40 @@ public static void inTransaction(Runnable runnable) { } public static T supplyInTransaction(Supplier supplier) { + Session session = getSession(); + boolean isSessionProvided = (session != null); boolean prevManagedFlag = CURRENT_SESSION_IS_MANAGED.get(); + try { CURRENT_SESSION_IS_MANAGED.set(true); - return supplier.get(); + + if (!isSessionProvided) { + // Start a new session and bind it to ThreadLocal + session = sf().openSession(); + session.beginTransaction(); + setCurrentSession(session); + } + + T result = supplier.get(); + + if (!isSessionProvided) { + session.getTransaction().commit(); + } + + return result; + } catch (Throwable e) { - PersistenceManager.rollbackCurrentSessionTx(); - throw e; + if (!isSessionProvided && session != null && session.getTransaction().isActive()) { + session.getTransaction().rollback(); + } + throw new RuntimeException("Transaction failed", e); + } finally { - PersistenceManager.endSessionLifecycle(); + if (!isSessionProvided && session != null && session.isOpen()) { + session.close(); + clearCurrentSession(); + } + CURRENT_SESSION_IS_MANAGED.set(prevManagedFlag); } } From bf1f33f9dcc01e49902f25a48d350e402c4dd523 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 00:34:40 +0300 Subject: [PATCH 05/30] AMP-30971: Thread issues for updating --- .../org/digijava/kernel/persistence/PersistenceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index cbf862e623b..882adbfbb67 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -562,7 +562,7 @@ public static T supplyInTransaction(Supplier supplier) { try { CURRENT_SESSION_IS_MANAGED.set(true); - if (!isSessionProvided) { + if (!isSessionProvided || !session.isOpen()) { // Start a new session and bind it to ThreadLocal session = sf().openSession(); session.beginTransaction(); From 08037881de40bccd08846cff49356b9566adaef9 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 16:37:04 +0300 Subject: [PATCH 06/30] AMP-30971: Thread issues for updating --- .../HibernateRequestCycleListener.java | 79 +- .../persistence/PersistenceManager.java | 770 +++++------------- 2 files changed, 257 insertions(+), 592 deletions(-) diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java b/amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java index bb5c72cec14..13b67883f1b 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/HibernateRequestCycleListener.java @@ -1,36 +1,89 @@ package org.dgfoundation.amp.onepager; +import org.apache.wicket.request.IRequestHandler; import org.apache.wicket.request.cycle.AbstractRequestCycleListener; import org.apache.wicket.request.cycle.RequestCycle; import org.digijava.kernel.persistence.PersistenceManager; import org.hibernate.Session; +import org.hibernate.Transaction; +/** + * Wicket request cycle listener that manages Hibernate session lifecycle + * for each web request. + */ public class HibernateRequestCycleListener extends AbstractRequestCycleListener { + private static final org.apache.log4j.Logger logger = org.apache.log4j.Logger.getLogger(HibernateRequestCycleListener.class); @Override public void onBeginRequest(RequestCycle cycle) { - Session session = PersistenceManager.openNewSession(); - session.beginTransaction(); - PersistenceManager.setCurrentSession(session); + if (PersistenceManager.isSessionManaged()) { + logger.warn("Session already exists when starting new request - possible nesting issue"); + return; + } + + try { + Session session = PersistenceManager.getSession(); + if (!session.getTransaction().isActive()) { + session.beginTransaction(); + } + logger.debug("Started new session and transaction for request"); + } catch (Exception e) { + logger.error("Failed to start Hibernate session for request", e); + throw new RuntimeException("Failed to initialize Hibernate session", e); + } } @Override public void onEndRequest(RequestCycle cycle) { - Session session = PersistenceManager.getSession(); try { - if (session != null && session.getTransaction().isActive()) { - session.getTransaction().commit(); + Session session = PersistenceManager.getSession(); + if (session == null || !session.isOpen()) { + logger.debug("No active session to close at end of request"); + return; } - } catch (Throwable t) { - if (session.getTransaction().isActive()) { - session.getTransaction().rollback(); + + Transaction transaction = session.getTransaction(); + try { + if (transaction.isActive()) { + if (transaction.getStatus().canRollback()) { + transaction.commit(); + logger.debug("Successfully committed transaction"); + } else { + logger.warn("Transaction in unexpected state: " + transaction.getStatus()); + } + } + } catch (Exception e) { + logger.error("Error during transaction commit - attempting rollback", e); + try { + if (transaction.isActive()) { + transaction.rollback(); + logger.debug("Rolled back transaction after error"); + } + } catch (Exception rollbackEx) { + logger.error("Failed to rollback transaction", rollbackEx); + } + throw new RuntimeException("Failed to commit transaction", e); } - throw new RuntimeException("Error during session commit", t); } finally { - if (session != null && session.isOpen()) { - session.close(); + try { + PersistenceManager.cleanupThread(); + logger.debug("Cleaned up thread resources"); + } catch (Exception e) { + logger.error("Error during thread cleanup", e); } - PersistenceManager.clearCurrentSession(); } } + + @Override + public void onDetach(RequestCycle cycle) { + // Ensure cleanup even if request processing fails + PersistenceManager.cleanupThread(); + } + + @Override + public IRequestHandler onException(RequestCycle cycle, Exception ex) { + logger.error("Exception occurred during request processing - cleaning up session", ex); + PersistenceManager.cleanupThread(); + return null; + } } diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 882adbfbb67..ba2e74ef858 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -56,245 +56,23 @@ import java.sql.SQLException; import java.util.*; import java.util.Map.Entry; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; public class PersistenceManager { + private static final Logger logger = I18NHelper.getKernelLogger(PersistenceManager.class); private static SessionFactory sf; private static Configuration cfg; - private static Logger logger = I18NHelper.getKernelLogger(PersistenceManager.class); + private static final AtomicInteger activeSessions = new AtomicInteger(); - public static String PRECACHE_REGION = + public static final String PRECACHE_REGION = "org.digijava.kernel.persistence.PersistenceManager.precache_region"; - - /** - * The maximum allowed life for an opened hibernate session, in miliseconds - */ - public static final long MAX_HIBERNATE_SESSION_LIFE_MILLIS=60*60*1000; - - private static final HashMap sessionStackTraceMap = new HashMap<>(); - - /** - * Invoked at the end of each request. Iterates and removes Hibernate closed sessions from the trace map. - * It also checks the long running sessions and forces closure on ones that are longer than - * The {@link HashMap} is synchronized to prevent concurrency issues between HTTP threads - */ - public static void checkClosedOrLongSessionsFromTraceMap() { - // remove closed sessions - synchronized (sessionStackTraceMap) { - Iterator iterator = PersistenceManager.sessionStackTraceMap - .keySet().iterator(); - while (iterator.hasNext()) { - Session session = (Session) iterator.next(); - - - // force closure of long running sessions - Long millis = (Long) sessionStackTraceMap.get(session)[0]; - if (session.isOpen() && ( System.currentTimeMillis() - millis > MAX_HIBERNATE_SESSION_LIFE_MILLIS )) { - StackTraceElement[] stackTrace = (StackTraceElement[]) sessionStackTraceMap - .get(session)[1]; - logger.info("Forcing closure and removal of hibernate session " - + session.hashCode() - + " because it ran for longer than " - + MAX_HIBERNATE_SESSION_LIFE_MILLIS - / 1000 - + " seconds"); - logger.info("Please review the code that generated the following recorded stack trace and ensure this session is closed properly: "); - for (int i = 0; i < stackTrace.length && i < 8; i++) logger.info(stackTrace[i].toString()); - - - try { - session.getTransaction().commit(); - } catch (Throwable e) { - e.printStackTrace(); - } - - try { - session.clear(); - } catch (Throwable e) { - e.printStackTrace(); - } - - - try { - session.close(); - } catch (Throwable e) { - e.printStackTrace(); - } - } - - // remove closed sessions - if (!session.isOpen()) iterator.remove(); - - - } - } - } - /** - * Removes only the closed sessions from the map that tracks opened sessions. - * No other check is being done. - */ - private static void removeClosedSessionsFromMap() { - int count = 0; - synchronized (sessionStackTraceMap) { - Iterator> iterator = PersistenceManager.sessionStackTraceMap.entrySet().iterator(); - while (iterator.hasNext()) { - Session sess = iterator.next().getKey(); - if ( !sess.isOpen() ) { - iterator.remove(); - count++; - } - } - } - logger.debug( count + " closed sessions were removed from 'sessionStackTraceMap' "); - } - - /** - * Opens a new Hibernate session. Use this with caution. - * For servlets you will not require to use this, use {@link #getSession()} instead! - * This method returns you an unmanaged Hibernate session, that you need to close yourself! - * @return - */ - public static Session openNewSession() { - org.hibernate.Session openSession = sf.openSession(); - return openSession; - } - - /** - * Returns the metadata for the given class from session factory - * @param clazz - * @return - */ - public static ClassMetadata getClassMetadata(Class clazz) { - return sf.getClassMetadata(clazz); - } - private static final ThreadLocal threadSession = new ThreadLocal<>(); - - public static PersistentClass getClassMapping(Class clazz) - { - - StandardServiceRegistry registry = new StandardServiceRegistryBuilder().applySettings(cfg.getProperties()).build(); - MetadataSources sources = new MetadataSources(registry); - Metadata metadata = sources.buildMetadata(); - return metadata.getEntityBinding(clazz.getName()); -// return cfg.getClassMapping(clazz.getName()); - } - - /** - * Gets a RAW jdbc connection to the database. Use with caution ! Close it yourself manually when done! - * @return - * @throws SQLException - */ -// public static Connection getJdbcConnection() throws SQLException { -// SessionFactoryImplementor sfi = (SessionFactoryImplementor) sf; -// return sfi.getConnectionProvider().getConnection(); -// } - - public static Connection getJdbcConnection() throws SQLException { - SessionFactoryImplementor sfi = (SessionFactoryImplementor) sf; - return sfi.getServiceRegistry().getService(ConnectionProvider.class).getConnection(); - } - - - - /** - * Shows the open remaining hibernate sessions upon AMP server shutdown. It displays the - * {@link Thread#currentThread#getStackTrace()} for the unclosed sessions. - * It will attempt to force {@link Session#clear()} and {@link Session#close()} on them - * This needs to be invoked (as Hibernate's APIs suggests) before {@link SessionFactory#close()} is invoked at the very end of AMP lifecycle - */ - public static void closeUnclosedSessionsFromTraceMap() { - // print open sessions - boolean found=false; - synchronized (sessionStackTraceMap) { - Iterator iterator = PersistenceManager.sessionStackTraceMap.keySet().iterator(); - while (iterator.hasNext()) { - Session session = (Session) iterator.next(); - if(session.isOpen()) { - found=true; - Object o[] = sessionStackTraceMap.get(session); - StackTraceElement[] stackTraceElements = (StackTraceElement[]) o[1]; - logger.info("Session opened "+(System.currentTimeMillis()-(Long)o[0])+" miliseconds ago is still open. Will force closure, recorded stack trace: "); - for (int i = 3; i < stackTraceElements.length && i < 8; i++) logger.info(stackTraceElements[i].toString()); - logger.info("Forcing Hibernate session close..."); - try { - session.clear(); - session.close(); - logger.info("Hibernate Session Close succeeded"); - } catch (Throwable t) { - logger.info("Error while forcing Hibernate session close:"); - logger.error(t.getMessage(), t); - } - logger.error("----------------------------------"); - } - } - if(found) logger.info("Check the code around the above stack traces, commit transactions only if not using HTTP threads:"); - } - } - - /** - * initialize PersistenceManager - * @param precache if true, do "empty" query for precached objects to put - * them in the cache - */ - public static void initialize(boolean precache) { - initialize(precache, null); - } - - public static synchronized void initialize(boolean precache, String target) { - - DigiConfig config = null; - HashMap modulesConfig = null; - try { - config = DigiConfigManager.getConfig(); - - logger.debug("Initializing persistence manager"); - - - // load kernel hibernate classes - HibernateClassLoader.initialize(config); - - if (target != null) { - if (!target.equalsIgnoreCase("kernel")) { - Object modConfig = DigiConfigManager.getModulesConfig().get(target); - if (modConfig != null) { - modulesConfig = new HashMap(); - modulesConfig.put(target, modConfig); - } - } - } - else { - modulesConfig = DigiConfigManager.getModulesConfig(); - } - - // load module hibernate classes - if (modulesConfig != null) { - HibernateClassLoader.initialize(modulesConfig); - } - - HibernateClassLoader.buildHibernateSessionFactory(); - - sf = HibernateClassLoader.getSessionFactory(); - cfg = HibernateClassLoader.getConfiguration(); - - if (precache) { - precache(); - } - } - catch (Exception ex) { - //String errKey = "PersistenceManager.intialize.error"; - //logger.l7dlog(Level.FATAL, errKey, null, ex); - logger.fatal("Unable to initialize PersistenceManager", ex); - - } - - } - /** - * precache hibernate classes + * The maximum allowed life for an opened hibernate session, in milliseconds */ private static void precache() throws DgException { DigiConfig config = DigiConfigManager.getConfig(); @@ -446,106 +224,161 @@ private static void precacheToRegion(Session session, " is already filled, skiping precache"); } } + public static final long MAX_HIBERNATE_SESSION_LIFE_MILLIS = 60 * 60 * 1000; + + private static final HashMap sessionStackTraceMap = new HashMap<>(); + private static final ThreadLocal threadSession = new ThreadLocal<>(); + private static final ThreadLocal CURRENT_SESSION_IS_MANAGED = ThreadLocal.withInitial(() -> false); /** - * Gets the Hibernate configuration used to build the immutable SessionFactory. - * Invoke this after initialize() - * @return the Hibernate Configuration object + * Initialize PersistenceManager */ - public static synchronized Configuration getHibernateConfiguration() { - return cfg; + public static void initialize(boolean precache) { + initialize(precache, null); } - private PersistenceManager() { - } + public static synchronized void initialize(boolean precache, String target) { + DigiConfig config = null; + HashMap modulesConfig = null; + try { + config = DigiConfigManager.getConfig(); + logger.debug("Initializing persistence manager"); + // load kernel hibernate classes + HibernateClassLoader.initialize(config); + + if (target != null && !target.equalsIgnoreCase("kernel")) { + Object modConfig = DigiConfigManager.getModulesConfig().get(target); + if (modConfig != null) { + modulesConfig = new HashMap(); + modulesConfig.put(target, modConfig); + } + } else { + modulesConfig = DigiConfigManager.getModulesConfig(); + } + + // load module hibernate classes + if (modulesConfig != null) { + HibernateClassLoader.initialize(modulesConfig); + } + + HibernateClassLoader.buildHibernateSessionFactory(); + + sf = HibernateClassLoader.getSessionFactory(); + cfg = HibernateClassLoader.getConfiguration(); + + if (precache) { + precache(); + } + } catch (Exception ex) { + logger.fatal("Unable to initialize PersistenceManager", ex); + } + } /** - * Close session factory + * Clean up resources */ public static void cleanup() { logger.debug("cleanup() called"); if (sf != null) { try { + closeUnclosedSessionsFromTraceMap(); sf.close(); - } - catch (HibernateException ex) { + } catch (HibernateException ex) { logger.error("Error cleaning up persistence manager", ex); } } + } - + /** + * Clean up thread-local resources + */ + public static void cleanupThread() { + Session session = threadSession.get(); + if (session != null) { + closeSession(session); + } + threadSession.remove(); + CURRENT_SESSION_IS_MANAGED.remove(); } + // [Previous methods like precache(), precacheToRegion(), etc. remain unchanged] + // [... existing methods ...] /** - * Loads all objects of T from database, using request (thread) session. - * @param - * @param object - * @return - * @throws DgException + * Open a new unmanaged Hibernate session */ - public static Collection loadAll(Class object) throws DgException{ - return loadAll(object, getRequestDBSession()); + public static Session openNewSession() { + Session session = sf.openSession(); + activeSessions.incrementAndGet(); + addSessionToStackTraceMap(session); + logger.debug("Opened new unmanaged session. Active sessions: " + activeSessions.get()); + return session; } /** - * Loads all objects of T from database. - * Client should care about opening and releasing session which is passed as parameter to this method. - * @param - * @param object class object of T - * @param session database session. Client should handle session - opening and releasing, including transactions if required. - * @return - * @throws DgException + * Close an unmanaged Hibernate session */ - @SuppressWarnings("unchecked") - public static Collection loadAll(Class object, Session session) throws DgException{ - Collection col = null; - String queryString = null; + public static void closeSession(Session session) { + if (session == null) return; + try { - queryString = "from " + object.getName(); - Query qry = session.createQuery(queryString); - col = qry.list(); - } catch (Exception e) { - logger.error("cannot execute query: "+queryString, e); - throw new DgException(e); + if (session.isOpen()) { + if (session.getTransaction().isActive()) { + try { + session.getTransaction().rollback(); + } catch (HibernateException e) { + logger.error("Failed to rollback transaction during close", e); + } + } + session.close(); + activeSessions.decrementAndGet(); + logger.debug("Closed unmanaged session. Active sessions: " + activeSessions.get()); + } + } catch (HibernateException e) { + logger.error("Failed to close session", e); + } finally { + removeSessionFromMap(session); + if (session.equals(threadSession.get())) { + clearCurrentSession(); + } } - return col; } /** - * + * Get current session or create new one if none exists */ - public static void rollbackCurrentSessionTx() { - if (sf.getCurrentSession().getTransaction().getStatus().equals(TransactionStatus.ACTIVE) - && sf.getCurrentSession().isOpen() - && sf.getCurrentSession().isConnected()) { - logger.info("Trying to rollback database transaction after exception"); - sf.getCurrentSession().getTransaction().rollback(); + public static Session getSession() { + Session session = threadSession.get(); + if (session == null || !session.isOpen()) { + session = sf.openSession(); + session.setHibernateFlushMode(FlushMode.AUTO); + threadSession.set(session); + addSessionToStackTraceMap(session); + activeSessions.incrementAndGet(); + logger.debug("Opened new managed session. Active sessions: " + activeSessions.get()); } + return session; } - /** - * A flag set to true when current session is managed. I.e. there are guarantees that the session will be closed - * after some arbitrary work is done. - */ - private static final ThreadLocal CURRENT_SESSION_IS_MANAGED = ThreadLocal.withInitial(() -> false); + public static Session getRequestDBSession() { + return getSession(); + } + + public static void setCurrentSession(Session session) { + threadSession.set(session); + } + + public static void clearCurrentSession() { + threadSession.remove(); + } + + public static boolean isSessionManaged() { + return CURRENT_SESSION_IS_MANAGED.get(); + } /** - * Execute runnable and ensures that if an active hibernate transaction exists it is committed or rolled back. - * Will always close the current session before returning. - * - *

If the runnable throws an exception, then transaction is rolled back and same exception is thrown again.

- * - *

Transaction is not created before calling the runnable. For actual semantics when the transaction is created - * please check {@link #getSession()}.

- * - *

Neither active session nor transaction are nested. Calling this method recursively will ensure that active - * transaction and session are closed upon exiting the method. This is not very useful nor a good way to reason - * about nested transaction semantics but this is how it worked before. Known to be used by - * {@link HibernateSessionRequestFilter} which is itself invoked recursively during error processing.

- * - * @param runnable the runnable to execute with open session in view context + * Execute work in transaction */ public static void inTransaction(Runnable runnable) { supplyInTransaction(() -> { @@ -563,7 +396,6 @@ public static T supplyInTransaction(Supplier supplier) { CURRENT_SESSION_IS_MANAGED.set(true); if (!isSessionProvided || !session.isOpen()) { - // Start a new session and bind it to ThreadLocal session = sf().openSession(); session.beginTransaction(); setCurrentSession(session); @@ -582,327 +414,107 @@ public static T supplyInTransaction(Supplier supplier) { session.getTransaction().rollback(); } throw new RuntimeException("Transaction failed", e); - } finally { if (!isSessionProvided && session != null && session.isOpen()) { session.close(); clearCurrentSession(); } - CURRENT_SESSION_IS_MANAGED.set(prevManagedFlag); } } - /** - * Returns the current Session. If there is none, creates one and returns it - * upon creating a new session, a transaction is created. - */ - public static Session getSession() { - Session session = threadSession.get(); - if (session == null) { - session = sf.openSession(); - threadSession.set(session); - } - return threadSession.get(); - } - - public static void setCurrentSession(Session session) { - threadSession.set(session); - } - public static void clearCurrentSession() { - threadSession.remove(); - } - - /** - * an alias for {@link #getSession()} - */ - public static Session getRequestDBSession() { - return getSession(); - } - - /** - * Adds this session to the stack trace map, so its closing can be tracked later - * @param sess - */ - public static void addSessionToStackTraceMap(Session sess) { - synchronized (sessionStackTraceMap){ - if(sessionStackTraceMap.get(sess)==null) - //logger.error(String.format("Thread #%d: storing new Session %d", Thread.currentThread().getId(), System.identityHashCode(sess))); - sessionStackTraceMap.put(sess,new Object[] {System.currentTimeMillis(),Thread.currentThread().getStackTrace()}); - } - } - - /** - * Removes object from Hibernate second-level cache, loads it from database - * and initializes - * @param objectClass target class - * @param primaryKey primary key for object - * @throws DgException - */ - public static void refreshCachedObject(Class objectClass, - Serializable primaryKey) throws - DgException { - Session session = null; - - try { -// sf.evict(objectClass, primaryKey); - - session = getSession(); - session.evict(primaryKey); - Object obj = session.load(objectClass, primaryKey); - Hibernate.initialize(obj); - } - catch (Exception ex) { - throw new DgException( - "Unable to refresh object into cache", ex); + // [Session tracking methods] + private static void addSessionToStackTraceMap(Session sess) { + synchronized (sessionStackTraceMap) { + if (!sessionStackTraceMap.containsKey(sess)) { + sessionStackTraceMap.put(sess, new Object[] { + System.currentTimeMillis(), + Thread.currentThread().getStackTrace(), + new Exception("Session creation point") + }); + } } } - /** - * closes a JDBC connection, swallowing any exceptions which have appeared in the meantime - * @param connection - */ - public static void closeQuietly(AutoCloseable connection) - { - if (connection == null) - return; - try - { - connection.close(); - } - catch(Exception e) - { - logger.error(e.getMessage(), e); + private static void removeSessionFromMap(Session session) { + synchronized (sessionStackTraceMap) { + sessionStackTraceMap.remove(session); } } - /** - * extracts a Long from an object - * @param obj - * @return - */ - public final static Long getLong(Object obj) - { - if (obj == null) - return null; - if (obj instanceof Long) - return (Long) obj; - if (obj instanceof Number) - return ((Number) obj).longValue(); - if (obj instanceof String) - return Long.parseLong((String) obj); - throw new RuntimeException("cannot convert object of class " + obj.getClass().getName() + " to long"); - } - - /** - * extracts an Integer from an object - * @param obj - * @return - */ - public final static Integer getInteger(Object obj) - { - if (obj == null) - return null; - if (obj instanceof Integer) - return (Integer) obj; - if (obj instanceof Number) - return ((Number) obj).intValue(); - if (obj instanceof String) - return Integer.parseInt((String) obj); - throw new RuntimeException("cannot convert object of class " + obj.getClass().getName() + " to int"); - } - - /** - * extracts a String from an object - * @param obj - * @return - */ - public final static String getString(Object obj) - { - if (obj == null) - return null; - if (obj instanceof String) - return (String) obj; - return obj.toString(); - } - - /** - * extracts a Double from an object - * @param obj - * @return - */ - public final static Double getDouble(Object obj) - { - if (obj == null) - return null; - if (obj instanceof Double) - return (Double) obj; - if (obj instanceof Number) - return ((Number) obj).doubleValue(); - if (obj instanceof String) - return Double.parseDouble((String) obj); - throw new RuntimeException("cannot convert object of class " + obj.getClass().getName() + " to double"); - } - - public final static Boolean getBoolean(Object obj) { - if (obj == null) - return null; - if (obj instanceof Boolean) - return (Boolean) obj; - if (obj.toString().equals("true") || obj.toString().equals("yes")) - return true; - if (obj.toString().equals("false") || obj.toString().equals("no")) - return false; - throw new RuntimeException("cannot convert object " + obj + " to boolean"); - } - - public static StatelessSession openNewStatelessSession() { - return sf.openStatelessSession(); - } - - public static SessionFactory sf() { - return sf; - } - - /** - * cleanly closes an unmanaged Hibernate session - * ONLY USE IT FOR CONNECTIONS CREATED WITH {@link #openNewSession()} - * @param session - */ - public static void closeSession(Session session) { - if (session == null) return; - try { - flushAndCommit(session); - } catch (HibernateException e) { - // logging the error since finally may throw another exception and this one will be lost - logger.error("Failed to commit.", e); - throw e; - } finally { - if (session.isOpen()) { - session.close(); - } - } - } + public static void checkClosedOrLongSessionsFromTraceMap() { + synchronized (sessionStackTraceMap) { + Iterator iterator = sessionStackTraceMap.keySet().iterator(); + while (iterator.hasNext()) { + Session session = iterator.next(); - /** - * Flushes the session and commits the transaction. It will not close the session and allows to rollback if an - * exception is raised here (as opposed to {@link #closeSession(Session)} which does not allow rollback). - */ - public static void flushAndCommit(Session session) { - Transaction transaction = session.getTransaction(); - if (transaction != null) { - if (transaction.isActive()) { -// try { -// // note: flushing is needed only if session uses FlushMode.MANUAL -// session.flush(); -// } catch (HibernateException e) { -// // logging the error since finally may throw another exception and this one will be lost -// logger.error("Failed to flush the session.", e); -// throw e; -// } finally { -// // do we really want to attempt commit if flushing fails? - //session will be flushed automatically on transaction commit since we set the FlusmodeType as AUTO - transaction.commit(); -// } - } - } - } + if (session.isOpen() && (System.currentTimeMillis() - (Long) sessionStackTraceMap.get(session)[0] > MAX_HIBERNATE_SESSION_LIFE_MILLIS)) { + StackTraceElement[] stackTrace = (StackTraceElement[]) sessionStackTraceMap.get(session)[1]; + logger.warn("Forcing closure of long-running session " + session.hashCode()); + for (int i = 0; i < stackTrace.length && i < 8; i++) { + logger.warn(stackTrace[i].toString()); + } - /** - * This is a lifecycle management function
- * this function should ONLY be called at the end of a Session's lifecycle, being the last a "cycle" sees. - * UNDER NO CIRCUMSTANCES CALL IT IN A "USER" (non-framework, non-job) ENVIRONMENT - * @see #cleanupSession(Session) - */ - public static void endSessionLifecycle() { - cleanupSession(getSession()); - } + try { + if (session.getTransaction().isActive()) { + session.getTransaction().rollback(); + } + session.close(); + } catch (Throwable e) { + logger.error("Error forcing session closure", e); + } + } - /** - * This is a lifecycle management function
- * commits & closes a session and then removes it from the SessionStackTraceMap - * this function should ONLY be called at the end of a Session's lifecycle, being the last a "cycle" sees. - * UNDER NO CIRCUMSTANCES CALL IT IN A "USER" (non-framework, non-job) ENVIRONMENT - * @see PersistenceManager#endSessionLifecycle() - * @param session - */ - public static void cleanupSession(Session session) { - try { - closeSession(session); - } finally { - try { - removeClosedSessionsFromMap(); - } catch (Exception e) { - // ignore exceptions - } - try { - removeSessionFromMap(session); - } catch (Exception e) { - // ignore exceptions + if (!session.isOpen()) { + iterator.remove(); + } } } } - private static void removeSessionFromMap(Session session) { + public static void closeUnclosedSessionsFromTraceMap() { synchronized (sessionStackTraceMap) { - if (sessionStackTraceMap.containsKey(session)) { - //logger.error(String.format("Thread #%d: removing Session %d", Thread.currentThread().getId(), System.identityHashCode(session))); - sessionStackTraceMap.remove(session); - } else { - //logger.error(String.format("Thread #%d: trying to cleanup nonexisting Session %d", Thread.currentThread().getId(), System.identityHashCode(session))); + Iterator iterator = sessionStackTraceMap.keySet().iterator(); + while (iterator.hasNext()) { + Session session = iterator.next(); + if (session.isOpen()) { + Object[] o = sessionStackTraceMap.get(session); + logger.warn("Force closing session opened " + (System.currentTimeMillis() - (Long) o[0]) + "ms ago"); + try { + if (session.getTransaction().isActive()) { + session.getTransaction().rollback(); + } + session.close(); + } catch (Throwable t) { + logger.error("Error closing session", t); + } + } + iterator.remove(); } } } - /** - * Execute Work in a new session and wrapped by a transaction. - * Useful for accessing db outside of http request. - * @param work work to be executed - */ - public static void doWorkInTransaction(Work work) { - doInTransaction(session -> { - session.doWork(work); - return Void.class; - }); + // [Additional helper methods] + public static Connection getJdbcConnection() throws SQLException { + SessionFactoryImplementor sfi = (SessionFactoryImplementor) sf; + return sfi.getServiceRegistry().getService(ConnectionProvider.class).getConnection(); } - /** - * Execute ReturningWork in a new session and wrapped by a transaction. - * Useful for accessing db outside of http request. - * @param returningWork returning work to be executed - */ - public static T doReturningWorkInTransaction(ReturningWork returningWork) { - return doInTransaction(session -> { - return session.doReturningWork(returningWork); - }); + public static ClassMetadata getClassMetadata(Class clazz) { + return sf.getClassMetadata(clazz); } - /** - * Execute a block of code in new session. - */ - public static void doInTransaction(Consumer consumer) { - doInTransaction(s -> { - consumer.accept(s); - return Void.class; - }); + public static PersistentClass getClassMapping(Class clazz) { + StandardServiceRegistry registry = new StandardServiceRegistryBuilder() + .applySettings(cfg.getProperties()).build(); + MetadataSources sources = new MetadataSources(registry); + Metadata metadata = sources.buildMetadata(); + return metadata.getEntityBinding(clazz.getName()); } - /** - * Execute a function inside a transaction. - * @param fn takes as input hibernate session - * @param return type - * @return result of the function - */ - public static R doInTransaction(Function fn) { - Session session = PersistenceManager.openNewSession(); - Transaction tx = session.beginTransaction(); - try { - return fn.apply(session); - } catch (Throwable e) { - tx.rollback(); - throw e; - } finally { - PersistenceManager.closeSession(session); - } + public static SessionFactory sf() { + return sf; } + + // [Other existing methods...] } From d12cd73a14908d13d62fa4ef7486620b91821424 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 16:42:22 +0300 Subject: [PATCH 07/30] AMP-30971: Thread issues for updating --- .../dgfoundation/amp/onepager/models/AmpActivityModel.java | 4 ++-- .../java/org/dgfoundation/amp/onepager/util/ActivityUtil.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java b/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java index a48c4b57e03..2e39acdc3df 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/models/AmpActivityModel.java @@ -74,8 +74,8 @@ public void beginConversation(boolean reset) { translationHashMap = new HashMap(); } -// Session ses = getHibernateSession(reset); -// ses.clear(); + Session ses = getHibernateSession(reset); + ses.clear(); } diff --git a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java index f24e4f043e5..a14261fcb5a 100644 --- a/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java +++ b/amp/src/main/java/org/dgfoundation/amp/onepager/util/ActivityUtil.java @@ -97,7 +97,7 @@ public static void saveActivity(AmpActivityModel am, boolean draft,boolean rejec am.setObject(newA); ActivityGatekeeper.unlockActivity(String.valueOf(am.getId()), am.getEditingKey()); -// AmpActivityModel.endConversation(); + AmpActivityModel.endConversation(); } /** From b2352c641a04254ff1b539d948d9ff8b03498b26 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 16:53:36 +0300 Subject: [PATCH 08/30] AMP-30971: Thread issues for updating --- .../persistence/PersistenceManager.java | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index ba2e74ef858..81ddeb80829 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -275,6 +275,81 @@ public static synchronized void initialize(boolean precache, String target) { } } + public static T doReturningWorkInTransaction(ReturningWork returningWork) { + return doInTransaction(session -> { + return session.doReturningWork(returningWork); + }); + } + + /** + * Executes a function within a transactional context with proper session management + * + * @param The return type + * @param function The function to execute within transaction + * @return The result of the function execution + * @throws RuntimeException if any error occurs during execution + */ + public static R doInTransaction(Function function) { + boolean isExistingSession = PersistenceManager.isSessionManaged(); + Session session = null; + Transaction transaction = null; + + try { + // Get or create session based on current context + session = isExistingSession ? PersistenceManager.getSession() : PersistenceManager.openNewSession(); + + // Begin transaction if needed + if (!session.getTransaction().isActive()) { + transaction = session.beginTransaction(); + logger.debug("Started new transaction for doInTransaction"); + } + + // Execute the business logic + R result = function.apply(session); + + // Commit if we started the transaction + if (transaction != null && transaction.isActive()) { + transaction.commit(); + logger.debug("Successfully committed transaction in doInTransaction"); + } + + return result; + + } catch (Exception e) { + // Handle transaction rollback + if (transaction != null && transaction.isActive()) { + try { + transaction.rollback(); + logger.debug("Rolled back transaction due to error", e); + } catch (HibernateException rollbackEx) { + logger.error("Failed to rollback transaction", rollbackEx); + } + } + throw new RuntimeException("Transaction failed in doInTransaction", e); + + } finally { + // Clean up resources if we created them + if (!isExistingSession && session != null && session.isOpen()) { + try { + session.close(); + logger.debug("Closed unmanaged session in doInTransaction"); + } catch (HibernateException e) { + logger.error("Failed to close session", e); + } + } + } + } + + /** + * Convenience method for void operations + */ + public static void doInTransaction(Consumer consumer) { + doInTransaction(session -> { + consumer.accept(session); + return null; + }); + } + /** * Clean up resources */ From 41cb2426c61e2a470ef57826c595579cccf914d4 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 17:18:43 +0300 Subject: [PATCH 09/30] AMP-30971: Thread issues for updating --- .../persistence/PersistenceManager.java | 800 ++++++++++++------ 1 file changed, 550 insertions(+), 250 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 81ddeb80829..5f0b3cffcce 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -63,16 +63,213 @@ public class PersistenceManager { - private static final Logger logger = I18NHelper.getKernelLogger(PersistenceManager.class); private static SessionFactory sf; private static Configuration cfg; + private static Logger logger = I18NHelper.getKernelLogger(PersistenceManager.class); private static final AtomicInteger activeSessions = new AtomicInteger(); - public static final String PRECACHE_REGION = + + private static final HashMap sessionStackTraceMap = new HashMap<>(); + + public static String PRECACHE_REGION = "org.digijava.kernel.persistence.PersistenceManager.precache_region"; + + /** + * The maximum allowed life for an opened hibernate session, in miliseconds + */ + public static final long MAX_HIBERNATE_SESSION_LIFE_MILLIS=60*60*1000; + + + /** + * Invoked at the end of each request. Iterates and removes Hibernate closed sessions from the trace map. + * It also checks the long running sessions and forces closure on ones that are longer than + * The {@link HashMap} is synchronized to prevent concurrency issues between HTTP threads + */ + public static void checkClosedOrLongSessionsFromTraceMap() { + synchronized (sessionStackTraceMap) { + Iterator iterator = sessionStackTraceMap.keySet().iterator(); + while (iterator.hasNext()) { + Session session = iterator.next(); + + if (session.isOpen() && (System.currentTimeMillis() - (Long) sessionStackTraceMap.get(session)[0] > MAX_HIBERNATE_SESSION_LIFE_MILLIS)) { + StackTraceElement[] stackTrace = (StackTraceElement[]) sessionStackTraceMap.get(session)[1]; + logger.warn("Forcing closure of long-running session " + session.hashCode()); + for (int i = 0; i < stackTrace.length && i < 8; i++) { + logger.warn(stackTrace[i].toString()); + } + + try { + if (session.getTransaction().isActive()) { + session.getTransaction().rollback(); + } + session.close(); + } catch (Throwable e) { + logger.error("Error forcing session closure", e); + } + } + + if (!session.isOpen()) { + iterator.remove(); + } + } + } + } + /** + * Removes only the closed sessions from the map that tracks opened sessions. + * No other check is being done. + */ + private static void removeClosedSessionsFromMap() { + int count = 0; + synchronized (sessionStackTraceMap) { + Iterator> iterator = PersistenceManager.sessionStackTraceMap.entrySet().iterator(); + while (iterator.hasNext()) { + Session sess = iterator.next().getKey(); + if ( !sess.isOpen() ) { + iterator.remove(); + count++; + } + } + } + logger.debug( count + " closed sessions were removed from 'sessionStackTraceMap' "); + } + + public static void cleanupThread() { + Session session = threadSession.get(); + if (session != null) { + closeSession(session); + } + threadSession.remove(); + CURRENT_SESSION_IS_MANAGED.remove(); + } + + /** + * Opens a new Hibernate session. Use this with caution. + * For servlets you will not require to use this, use {@link #getSession()} instead! + * This method returns you an unmanaged Hibernate session, that you need to close yourself! + * @return + */ + public static Session openNewSession() { + Session session = sf.openSession(); + activeSessions.incrementAndGet(); + addSessionToStackTraceMap(session); + logger.debug("Opened new unmanaged session. Active sessions: " + activeSessions.get()); + return session; + } + + + /** + * Returns the metadata for the given class from session factory + * @param clazz + * @return + */ + public static ClassMetadata getClassMetadata(Class clazz) { + return sf.getClassMetadata(clazz); + } + private static final ThreadLocal threadSession = new ThreadLocal<>(); + + public static PersistentClass getClassMapping(Class clazz) { + StandardServiceRegistry registry = new StandardServiceRegistryBuilder() + .applySettings(cfg.getProperties()).build(); + MetadataSources sources = new MetadataSources(registry); + Metadata metadata = sources.buildMetadata(); + return metadata.getEntityBinding(clazz.getName()); + } + + /** + * Gets a RAW jdbc connection to the database. Use with caution ! Close it yourself manually when done! + * @return + * @throws SQLException + */ +// public static Connection getJdbcConnection() throws SQLException { +// SessionFactoryImplementor sfi = (SessionFactoryImplementor) sf; +// return sfi.getConnectionProvider().getConnection(); +// } + + public static Connection getJdbcConnection() throws SQLException { + SessionFactoryImplementor sfi = (SessionFactoryImplementor) sf; + return sfi.getServiceRegistry().getService(ConnectionProvider.class).getConnection(); + } + + + + /** + * Shows the open remaining hibernate sessions upon AMP server shutdown. It displays the + * {@link Thread#currentThread#getStackTrace()} for the unclosed sessions. + * It will attempt to force {@link Session#clear()} and {@link Session#close()} on them + * This needs to be invoked (as Hibernate's APIs suggests) before {@link SessionFactory#close()} is invoked at the very end of AMP lifecycle + */ + public static void closeUnclosedSessionsFromTraceMap() { + synchronized (sessionStackTraceMap) { + Iterator iterator = sessionStackTraceMap.keySet().iterator(); + while (iterator.hasNext()) { + Session session = iterator.next(); + if (session.isOpen()) { + Object[] o = sessionStackTraceMap.get(session); + logger.warn("Force closing session opened " + (System.currentTimeMillis() - (Long) o[0]) + "ms ago"); + try { + if (session.getTransaction().isActive()) { + session.getTransaction().rollback(); + } + session.close(); + } catch (Throwable t) { + logger.error("Error closing session", t); + } + } + iterator.remove(); + } + } + } + + /** + * initialize PersistenceManager + * @param precache if true, do "empty" query for precached objects to put + * them in the cache + */ + public static void initialize(boolean precache) { + initialize(precache, null); + } + + public static synchronized void initialize(boolean precache, String target) { + DigiConfig config = null; + HashMap modulesConfig = null; + try { + config = DigiConfigManager.getConfig(); + logger.debug("Initializing persistence manager"); + + // load kernel hibernate classes + HibernateClassLoader.initialize(config); + + if (target != null && !target.equalsIgnoreCase("kernel")) { + Object modConfig = DigiConfigManager.getModulesConfig().get(target); + if (modConfig != null) { + modulesConfig = new HashMap(); + modulesConfig.put(target, modConfig); + } + } else { + modulesConfig = DigiConfigManager.getModulesConfig(); + } + + // load module hibernate classes + if (modulesConfig != null) { + HibernateClassLoader.initialize(modulesConfig); + } + + HibernateClassLoader.buildHibernateSessionFactory(); + + sf = HibernateClassLoader.getSessionFactory(); + cfg = HibernateClassLoader.getConfiguration(); + + if (precache) { + precache(); + } + } catch (Exception ex) { + logger.fatal("Unable to initialize PersistenceManager", ex); + } + } + /** - * The maximum allowed life for an opened hibernate session, in milliseconds + * precache hibernate classes */ private static void precache() throws DgException { DigiConfig config = DigiConfigManager.getConfig(); @@ -224,134 +421,22 @@ private static void precacheToRegion(Session session, " is already filled, skiping precache"); } } - public static final long MAX_HIBERNATE_SESSION_LIFE_MILLIS = 60 * 60 * 1000; - - private static final HashMap sessionStackTraceMap = new HashMap<>(); - private static final ThreadLocal threadSession = new ThreadLocal<>(); - private static final ThreadLocal CURRENT_SESSION_IS_MANAGED = ThreadLocal.withInitial(() -> false); /** - * Initialize PersistenceManager + * Gets the Hibernate configuration used to build the immutable SessionFactory. + * Invoke this after initialize() + * @return the Hibernate Configuration object */ - public static void initialize(boolean precache) { - initialize(precache, null); - } - - public static synchronized void initialize(boolean precache, String target) { - DigiConfig config = null; - HashMap modulesConfig = null; - try { - config = DigiConfigManager.getConfig(); - logger.debug("Initializing persistence manager"); - - // load kernel hibernate classes - HibernateClassLoader.initialize(config); - - if (target != null && !target.equalsIgnoreCase("kernel")) { - Object modConfig = DigiConfigManager.getModulesConfig().get(target); - if (modConfig != null) { - modulesConfig = new HashMap(); - modulesConfig.put(target, modConfig); - } - } else { - modulesConfig = DigiConfigManager.getModulesConfig(); - } - - // load module hibernate classes - if (modulesConfig != null) { - HibernateClassLoader.initialize(modulesConfig); - } - - HibernateClassLoader.buildHibernateSessionFactory(); - - sf = HibernateClassLoader.getSessionFactory(); - cfg = HibernateClassLoader.getConfiguration(); - - if (precache) { - precache(); - } - } catch (Exception ex) { - logger.fatal("Unable to initialize PersistenceManager", ex); - } - } - - public static T doReturningWorkInTransaction(ReturningWork returningWork) { - return doInTransaction(session -> { - return session.doReturningWork(returningWork); - }); + public static synchronized Configuration getHibernateConfiguration() { + return cfg; } - /** - * Executes a function within a transactional context with proper session management - * - * @param The return type - * @param function The function to execute within transaction - * @return The result of the function execution - * @throws RuntimeException if any error occurs during execution - */ - public static R doInTransaction(Function function) { - boolean isExistingSession = PersistenceManager.isSessionManaged(); - Session session = null; - Transaction transaction = null; - - try { - // Get or create session based on current context - session = isExistingSession ? PersistenceManager.getSession() : PersistenceManager.openNewSession(); - - // Begin transaction if needed - if (!session.getTransaction().isActive()) { - transaction = session.beginTransaction(); - logger.debug("Started new transaction for doInTransaction"); - } - - // Execute the business logic - R result = function.apply(session); - - // Commit if we started the transaction - if (transaction != null && transaction.isActive()) { - transaction.commit(); - logger.debug("Successfully committed transaction in doInTransaction"); - } - - return result; - - } catch (Exception e) { - // Handle transaction rollback - if (transaction != null && transaction.isActive()) { - try { - transaction.rollback(); - logger.debug("Rolled back transaction due to error", e); - } catch (HibernateException rollbackEx) { - logger.error("Failed to rollback transaction", rollbackEx); - } - } - throw new RuntimeException("Transaction failed in doInTransaction", e); - - } finally { - // Clean up resources if we created them - if (!isExistingSession && session != null && session.isOpen()) { - try { - session.close(); - logger.debug("Closed unmanaged session in doInTransaction"); - } catch (HibernateException e) { - logger.error("Failed to close session", e); - } - } - } + private PersistenceManager() { } - /** - * Convenience method for void operations - */ - public static void doInTransaction(Consumer consumer) { - doInTransaction(session -> { - consumer.accept(session); - return null; - }); - } /** - * Clean up resources + * Close session factory */ public static void cleanup() { logger.debug("cleanup() called"); @@ -365,95 +450,75 @@ public static void cleanup() { } } - /** - * Clean up thread-local resources - */ - public static void cleanupThread() { - Session session = threadSession.get(); - if (session != null) { - closeSession(session); - } - threadSession.remove(); - CURRENT_SESSION_IS_MANAGED.remove(); - } - - // [Previous methods like precache(), precacheToRegion(), etc. remain unchanged] - // [... existing methods ...] /** - * Open a new unmanaged Hibernate session + * Loads all objects of T from database, using request (thread) session. + * @param + * @param object + * @return + * @throws DgException */ - public static Session openNewSession() { - Session session = sf.openSession(); - activeSessions.incrementAndGet(); - addSessionToStackTraceMap(session); - logger.debug("Opened new unmanaged session. Active sessions: " + activeSessions.get()); - return session; + public static Collection loadAll(Class object) throws DgException{ + return loadAll(object, getRequestDBSession()); } /** - * Close an unmanaged Hibernate session + * Loads all objects of T from database. + * Client should care about opening and releasing session which is passed as parameter to this method. + * @param + * @param object class object of T + * @param session database session. Client should handle session - opening and releasing, including transactions if required. + * @return + * @throws DgException */ - public static void closeSession(Session session) { - if (session == null) return; - + @SuppressWarnings("unchecked") + public static Collection loadAll(Class object, Session session) throws DgException{ + Collection col = null; + String queryString = null; try { - if (session.isOpen()) { - if (session.getTransaction().isActive()) { - try { - session.getTransaction().rollback(); - } catch (HibernateException e) { - logger.error("Failed to rollback transaction during close", e); - } - } - session.close(); - activeSessions.decrementAndGet(); - logger.debug("Closed unmanaged session. Active sessions: " + activeSessions.get()); - } - } catch (HibernateException e) { - logger.error("Failed to close session", e); - } finally { - removeSessionFromMap(session); - if (session.equals(threadSession.get())) { - clearCurrentSession(); - } + queryString = "from " + object.getName(); + Query qry = session.createQuery(queryString); + col = qry.list(); + } catch (Exception e) { + logger.error("cannot execute query: "+queryString, e); + throw new DgException(e); } + return col; } /** - * Get current session or create new one if none exists + * */ - public static Session getSession() { - Session session = threadSession.get(); - if (session == null || !session.isOpen()) { - session = sf.openSession(); - session.setHibernateFlushMode(FlushMode.AUTO); - threadSession.set(session); - addSessionToStackTraceMap(session); - activeSessions.incrementAndGet(); - logger.debug("Opened new managed session. Active sessions: " + activeSessions.get()); + public static void rollbackCurrentSessionTx() { + if (sf.getCurrentSession().getTransaction().getStatus().equals(TransactionStatus.ACTIVE) + && sf.getCurrentSession().isOpen() + && sf.getCurrentSession().isConnected()) { + logger.info("Trying to rollback database transaction after exception"); + sf.getCurrentSession().getTransaction().rollback(); } - return session; - } - - public static Session getRequestDBSession() { - return getSession(); - } - - public static void setCurrentSession(Session session) { - threadSession.set(session); } - public static void clearCurrentSession() { - threadSession.remove(); - } - - public static boolean isSessionManaged() { - return CURRENT_SESSION_IS_MANAGED.get(); - } + /** + * A flag set to true when current session is managed. I.e. there are guarantees that the session will be closed + * after some arbitrary work is done. + */ + private static final ThreadLocal CURRENT_SESSION_IS_MANAGED = ThreadLocal.withInitial(() -> false); /** - * Execute work in transaction + * Execute runnable and ensures that if an active hibernate transaction exists it is committed or rolled back. + * Will always close the current session before returning. + * + *

If the runnable throws an exception, then transaction is rolled back and same exception is thrown again.

+ * + *

Transaction is not created before calling the runnable. For actual semantics when the transaction is created + * please check {@link #getSession()}.

+ * + *

Neither active session nor transaction are nested. Calling this method recursively will ensure that active + * transaction and session are closed upon exiting the method. This is not very useful nor a good way to reason + * about nested transaction semantics but this is how it worked before. Known to be used by + * {@link HibernateSessionRequestFilter} which is itself invoked recursively during error processing.

+ * + * @param runnable the runnable to execute with open session in view context */ public static void inTransaction(Runnable runnable) { supplyInTransaction(() -> { @@ -498,7 +563,45 @@ public static T supplyInTransaction(Supplier supplier) { } } - // [Session tracking methods] + /** + * Returns the current Session. If there is none, creates one and returns it + * upon creating a new session, a transaction is created. + */ + public static Session getSession() { + Session session = threadSession.get(); + if (session == null || !session.isOpen()) { + session = sf.openSession(); + session.setHibernateFlushMode(FlushMode.AUTO); + threadSession.set(session); + addSessionToStackTraceMap(session); + activeSessions.incrementAndGet(); + logger.debug("Opened new managed session. Active sessions: " + activeSessions.get()); + } + return session; + } + + public static void setCurrentSession(Session session) { + threadSession.set(session); + } + public static void clearCurrentSession() { + threadSession.remove(); + } + + public static boolean isSessionManaged() { + return CURRENT_SESSION_IS_MANAGED.get(); + } + + /** + * an alias for {@link #getSession()} + */ + public static Session getRequestDBSession() { + return getSession(); + } + + /** + * Adds this session to the stack trace map, so its closing can be tracked later + * @param sess + */ private static void addSessionToStackTraceMap(Session sess) { synchronized (sessionStackTraceMap) { if (!sessionStackTraceMap.containsKey(sess)) { @@ -510,86 +613,283 @@ private static void addSessionToStackTraceMap(Session sess) { } } } + /** + * Removes object from Hibernate second-level cache, loads it from database + * and initializes + * @param objectClass target class + * @param primaryKey primary key for object + * @throws DgException + */ + public static void refreshCachedObject(Class objectClass, + Serializable primaryKey) throws + DgException { + Session session = null; - private static void removeSessionFromMap(Session session) { - synchronized (sessionStackTraceMap) { - sessionStackTraceMap.remove(session); + try { +// sf.evict(objectClass, primaryKey); + + session = getSession(); + session.evict(primaryKey); + Object obj = session.load(objectClass, primaryKey); + Hibernate.initialize(obj); + } + catch (Exception ex) { + throw new DgException( + "Unable to refresh object into cache", ex); } } - public static void checkClosedOrLongSessionsFromTraceMap() { - synchronized (sessionStackTraceMap) { - Iterator iterator = sessionStackTraceMap.keySet().iterator(); - while (iterator.hasNext()) { - Session session = iterator.next(); + /** + * closes a JDBC connection, swallowing any exceptions which have appeared in the meantime + * @param connection + */ + public static void closeQuietly(AutoCloseable connection) + { + if (connection == null) + return; + try + { + connection.close(); + } + catch(Exception e) + { + logger.error(e.getMessage(), e); + } + } - if (session.isOpen() && (System.currentTimeMillis() - (Long) sessionStackTraceMap.get(session)[0] > MAX_HIBERNATE_SESSION_LIFE_MILLIS)) { - StackTraceElement[] stackTrace = (StackTraceElement[]) sessionStackTraceMap.get(session)[1]; - logger.warn("Forcing closure of long-running session " + session.hashCode()); - for (int i = 0; i < stackTrace.length && i < 8; i++) { - logger.warn(stackTrace[i].toString()); - } + /** + * extracts a Long from an object + * @param obj + * @return + */ + public final static Long getLong(Object obj) + { + if (obj == null) + return null; + if (obj instanceof Long) + return (Long) obj; + if (obj instanceof Number) + return ((Number) obj).longValue(); + if (obj instanceof String) + return Long.parseLong((String) obj); + throw new RuntimeException("cannot convert object of class " + obj.getClass().getName() + " to long"); + } + /** + * extracts an Integer from an object + * @param obj + * @return + */ + public final static Integer getInteger(Object obj) + { + if (obj == null) + return null; + if (obj instanceof Integer) + return (Integer) obj; + if (obj instanceof Number) + return ((Number) obj).intValue(); + if (obj instanceof String) + return Integer.parseInt((String) obj); + throw new RuntimeException("cannot convert object of class " + obj.getClass().getName() + " to int"); + } + + /** + * extracts a String from an object + * @param obj + * @return + */ + public final static String getString(Object obj) + { + if (obj == null) + return null; + if (obj instanceof String) + return (String) obj; + return obj.toString(); + } + + /** + * extracts a Double from an object + * @param obj + * @return + */ + public final static Double getDouble(Object obj) + { + if (obj == null) + return null; + if (obj instanceof Double) + return (Double) obj; + if (obj instanceof Number) + return ((Number) obj).doubleValue(); + if (obj instanceof String) + return Double.parseDouble((String) obj); + throw new RuntimeException("cannot convert object of class " + obj.getClass().getName() + " to double"); + } + + public final static Boolean getBoolean(Object obj) { + if (obj == null) + return null; + if (obj instanceof Boolean) + return (Boolean) obj; + if (obj.toString().equals("true") || obj.toString().equals("yes")) + return true; + if (obj.toString().equals("false") || obj.toString().equals("no")) + return false; + throw new RuntimeException("cannot convert object " + obj + " to boolean"); + } + + public static StatelessSession openNewStatelessSession() { + return sf.openStatelessSession(); + } + + public static SessionFactory sf() { + return sf; + } + + /** + * cleanly closes an unmanaged Hibernate session + * ONLY USE IT FOR CONNECTIONS CREATED WITH {@link #openNewSession()} + * @param session + */ + public static void closeSession(Session session) { + if (session == null) return; + + try { + if (session.isOpen()) { + if (session.getTransaction().isActive()) { try { - if (session.getTransaction().isActive()) { - session.getTransaction().rollback(); - } - session.close(); - } catch (Throwable e) { - logger.error("Error forcing session closure", e); + session.getTransaction().rollback(); + } catch (HibernateException e) { + logger.error("Failed to rollback transaction during close", e); } } + session.close(); + activeSessions.decrementAndGet(); + logger.debug("Closed unmanaged session. Active sessions: " + activeSessions.get()); + } + } catch (HibernateException e) { + logger.error("Failed to close session", e); + } finally { + removeSessionFromMap(session); + if (session.equals(threadSession.get())) { + clearCurrentSession(); + } + } + } - if (!session.isOpen()) { - iterator.remove(); - } + /** + * Flushes the session and commits the transaction. It will not close the session and allows to rollback if an + * exception is raised here (as opposed to {@link #closeSession(Session)} which does not allow rollback). + */ + public static void flushAndCommit(Session session) { + Transaction transaction = session.getTransaction(); + if (transaction != null) { + if (transaction.isActive()) { +// try { +// // note: flushing is needed only if session uses FlushMode.MANUAL +// session.flush(); +// } catch (HibernateException e) { +// // logging the error since finally may throw another exception and this one will be lost +// logger.error("Failed to flush the session.", e); +// throw e; +// } finally { +// // do we really want to attempt commit if flushing fails? + //session will be flushed automatically on transaction commit since we set the FlusmodeType as AUTO + transaction.commit(); +// } } } } - public static void closeUnclosedSessionsFromTraceMap() { - synchronized (sessionStackTraceMap) { - Iterator iterator = sessionStackTraceMap.keySet().iterator(); - while (iterator.hasNext()) { - Session session = iterator.next(); - if (session.isOpen()) { - Object[] o = sessionStackTraceMap.get(session); - logger.warn("Force closing session opened " + (System.currentTimeMillis() - (Long) o[0]) + "ms ago"); - try { - if (session.getTransaction().isActive()) { - session.getTransaction().rollback(); - } - session.close(); - } catch (Throwable t) { - logger.error("Error closing session", t); - } - } - iterator.remove(); + /** + * This is a lifecycle management function
+ * this function should ONLY be called at the end of a Session's lifecycle, being the last a "cycle" sees. + * UNDER NO CIRCUMSTANCES CALL IT IN A "USER" (non-framework, non-job) ENVIRONMENT + * @see #cleanupSession(Session) + */ + public static void endSessionLifecycle() { + cleanupSession(getSession()); + } + + /** + * This is a lifecycle management function
+ * commits & closes a session and then removes it from the SessionStackTraceMap + * this function should ONLY be called at the end of a Session's lifecycle, being the last a "cycle" sees. + * UNDER NO CIRCUMSTANCES CALL IT IN A "USER" (non-framework, non-job) ENVIRONMENT + * @see PersistenceManager#endSessionLifecycle() + * @param session + */ + public static void cleanupSession(Session session) { + try { + closeSession(session); + } finally { + try { + removeClosedSessionsFromMap(); + } catch (Exception e) { + // ignore exceptions + } + try { + removeSessionFromMap(session); + } catch (Exception e) { + // ignore exceptions } } } - // [Additional helper methods] - public static Connection getJdbcConnection() throws SQLException { - SessionFactoryImplementor sfi = (SessionFactoryImplementor) sf; - return sfi.getServiceRegistry().getService(ConnectionProvider.class).getConnection(); + private static void removeSessionFromMap(Session session) { + synchronized (sessionStackTraceMap) { + sessionStackTraceMap.remove(session); + } } - public static ClassMetadata getClassMetadata(Class clazz) { - return sf.getClassMetadata(clazz); + /** + * Execute Work in a new session and wrapped by a transaction. + * Useful for accessing db outside of http request. + * @param work work to be executed + */ + public static void doWorkInTransaction(Work work) { + doInTransaction(session -> { + session.doWork(work); + return Void.class; + }); } - public static PersistentClass getClassMapping(Class clazz) { - StandardServiceRegistry registry = new StandardServiceRegistryBuilder() - .applySettings(cfg.getProperties()).build(); - MetadataSources sources = new MetadataSources(registry); - Metadata metadata = sources.buildMetadata(); - return metadata.getEntityBinding(clazz.getName()); + /** + * Execute ReturningWork in a new session and wrapped by a transaction. + * Useful for accessing db outside of http request. + * @param returningWork returning work to be executed + */ + public static T doReturningWorkInTransaction(ReturningWork returningWork) { + return doInTransaction(session -> { + return session.doReturningWork(returningWork); + }); } - public static SessionFactory sf() { - return sf; + /** + * Execute a block of code in new session. + */ + public static void doInTransaction(Consumer consumer) { + doInTransaction(s -> { + consumer.accept(s); + return Void.class; + }); } - // [Other existing methods...] + /** + * Execute a function inside a transaction. + * @param fn takes as input hibernate session + * @param return type + * @return result of the function + */ + public static R doInTransaction(Function fn) { + Session session = PersistenceManager.openNewSession(); + Transaction tx = session.beginTransaction(); + try { + return fn.apply(session); + } catch (Throwable e) { + tx.rollback(); + throw e; + } finally { + PersistenceManager.closeSession(session); + } + } } From f11eaf09d0581585fd0f78cbac238198004ae79b Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 17:31:33 +0300 Subject: [PATCH 10/30] AMP-30971: Thread issues for updating --- .../persistence/PersistenceManager.java | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 5f0b3cffcce..1b58424ae87 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -529,40 +529,46 @@ public static void inTransaction(Runnable runnable) { public static T supplyInTransaction(Supplier supplier) { Session session = getSession(); - boolean isSessionProvided = (session != null); - boolean prevManagedFlag = CURRENT_SESSION_IS_MANAGED.get(); + boolean existingTransaction = session.getTransaction().isActive(); + Transaction transaction = null; try { - CURRENT_SESSION_IS_MANAGED.set(true); - - if (!isSessionProvided || !session.isOpen()) { - session = sf().openSession(); - session.beginTransaction(); - setCurrentSession(session); + if (!existingTransaction) { + transaction = session.beginTransaction(); } T result = supplier.get(); - if (!isSessionProvided) { - session.getTransaction().commit(); + if (!existingTransaction && transaction != null) { + transaction.commit(); } return result; - - } catch (Throwable e) { - if (!isSessionProvided && session != null && session.getTransaction().isActive()) { - session.getTransaction().rollback(); + } catch (Exception e) { + if (!existingTransaction && transaction != null && transaction.isActive()) { + try { + transaction.rollback(); + } catch (HibernateException he) { + logger.error("Failed to rollback transaction", he); + } } throw new RuntimeException("Transaction failed", e); } finally { - if (!isSessionProvided && session != null && session.isOpen()) { - session.close(); + if (!existingTransaction && session != null && session.isOpen()) { + try { + if (session.getTransaction().isActive()) { + session.getTransaction().rollback(); + } + session.close(); + } catch (HibernateException e) { + logger.error("Failed to close session", e); + } clearCurrentSession(); } - CURRENT_SESSION_IS_MANAGED.set(prevManagedFlag); } } + /** * Returns the current Session. If there is none, creates one and returns it * upon creating a new session, a transaction is created. @@ -571,12 +577,16 @@ public static Session getSession() { Session session = threadSession.get(); if (session == null || !session.isOpen()) { session = sf.openSession(); - session.setHibernateFlushMode(FlushMode.AUTO); + session.setFlushMode(FlushModeType.AUTO); threadSession.set(session); addSessionToStackTraceMap(session); - activeSessions.incrementAndGet(); - logger.debug("Opened new managed session. Active sessions: " + activeSessions.get()); } + + // Ensure we have an active transaction for write operations + if (!session.getTransaction().isActive()) { + session.beginTransaction(); + } + return session; } From de718c12c4e67267e09ae44ec87ffc359f01d223 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 17:55:46 +0300 Subject: [PATCH 11/30] AMP-30971: Thread issues when updating activity via airflow/API --- .../persistence/PersistenceManager.java | 52 +++++++++++++++++++ .../message/jobs/ConnectionCleaningJob.java | 6 +-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 1b58424ae87..b193dc7fd8a 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -81,6 +81,58 @@ public class PersistenceManager { public static final long MAX_HIBERNATE_SESSION_LIFE_MILLIS=60*60*1000; + /** + * Special execution method for Quartz jobs and other background tasks + */ + public static void executeInJobContext(Runnable jobTask) { + executeInJobContext(() -> { + jobTask.run(); + return null; + }); + } + + public static T executeInJobContext(Supplier jobTask) { + Session session = null; + Transaction transaction = null; + boolean success = false; + + try { + session = openNewSession(); + transaction = session.beginTransaction(); + + T result = jobTask.get(); + + transaction.commit(); + success = true; + return result; + } catch (Exception e) { + if (transaction != null && transaction.isActive()) { + try { + transaction.rollback(); + } catch (HibernateException he) { + logger.error("Failed to rollback job transaction", he); + } + } + throw new RuntimeException("Job execution failed", e); + } finally { + if (session != null && session.isOpen()) { + try { + if (!success && transaction != null && transaction.isActive()) { + transaction.rollback(); + } + session.close(); + } catch (HibernateException e) { + logger.error("Failed to close job session", e); + } + } + } + } + private static boolean isQuartzJobThread() { + String threadName = Thread.currentThread().getName(); + return threadName != null && threadName.contains("QuartzScheduler"); + } + + /** * Invoked at the end of each request. Iterates and removes Hibernate closed sessions from the trace map. * It also checks the long running sessions and forces closure on ones that are longer than diff --git a/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java b/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java index e1b512e1d98..6e9e1dcecf6 100644 --- a/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java +++ b/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java @@ -6,11 +6,11 @@ import org.quartz.JobExecutionException; public abstract class ConnectionCleaningJob implements Job { - + @Override public final void execute(JobExecutionContext context) throws JobExecutionException { try { if (shouldExecuteInTransaction()) { - PersistenceManager.inTransaction(() -> { + PersistenceManager.executeInJobContext(() -> { try { executeInternal(context); } catch (JobExecutionException e) { @@ -28,7 +28,7 @@ public abstract class ConnectionCleaningJob implements Job { } } } - + public abstract void executeInternal(JobExecutionContext context) throws JobExecutionException; public boolean shouldExecuteInTransaction() { From 8db0521c893f925fea166a774d40e3aad30856b2 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 18:14:21 +0300 Subject: [PATCH 12/30] AMP-30971: Thread issues when updating activity via airflow/API --- .../persistence/PersistenceManager.java | 51 +------------------ .../message/jobs/ConnectionCleaningJob.java | 2 +- 2 files changed, 2 insertions(+), 51 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index b193dc7fd8a..a6265501ba0 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -81,56 +81,6 @@ public class PersistenceManager { public static final long MAX_HIBERNATE_SESSION_LIFE_MILLIS=60*60*1000; - /** - * Special execution method for Quartz jobs and other background tasks - */ - public static void executeInJobContext(Runnable jobTask) { - executeInJobContext(() -> { - jobTask.run(); - return null; - }); - } - - public static T executeInJobContext(Supplier jobTask) { - Session session = null; - Transaction transaction = null; - boolean success = false; - - try { - session = openNewSession(); - transaction = session.beginTransaction(); - - T result = jobTask.get(); - - transaction.commit(); - success = true; - return result; - } catch (Exception e) { - if (transaction != null && transaction.isActive()) { - try { - transaction.rollback(); - } catch (HibernateException he) { - logger.error("Failed to rollback job transaction", he); - } - } - throw new RuntimeException("Job execution failed", e); - } finally { - if (session != null && session.isOpen()) { - try { - if (!success && transaction != null && transaction.isActive()) { - transaction.rollback(); - } - session.close(); - } catch (HibernateException e) { - logger.error("Failed to close job session", e); - } - } - } - } - private static boolean isQuartzJobThread() { - String threadName = Thread.currentThread().getName(); - return threadName != null && threadName.contains("QuartzScheduler"); - } /** @@ -604,6 +554,7 @@ public static T supplyInTransaction(Supplier supplier) { logger.error("Failed to rollback transaction", he); } } + rollbackCurrentSessionTx(); throw new RuntimeException("Transaction failed", e); } finally { if (!existingTransaction && session != null && session.isOpen()) { diff --git a/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java b/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java index 6e9e1dcecf6..56237868e23 100644 --- a/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java +++ b/amp/src/main/java/org/digijava/module/message/jobs/ConnectionCleaningJob.java @@ -10,7 +10,7 @@ public abstract class ConnectionCleaningJob implements Job { @Override public final void execute(JobExecutionContext context) throws JobExecutionException { try { if (shouldExecuteInTransaction()) { - PersistenceManager.executeInJobContext(() -> { + PersistenceManager.inTransaction(() -> { try { executeInternal(context); } catch (JobExecutionException e) { From 3ea3bf4c953e4dd2b0895d2edd99601b5f259e16 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 18:34:12 +0300 Subject: [PATCH 13/30] AMP-30971: Thread issues when updating activity via airflow/API --- .../org/digijava/kernel/persistence/PersistenceManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index a6265501ba0..74d1b667811 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -549,12 +549,12 @@ public static T supplyInTransaction(Supplier supplier) { } catch (Exception e) { if (!existingTransaction && transaction != null && transaction.isActive()) { try { - transaction.rollback(); + rollbackCurrentSessionTx(); + } catch (HibernateException he) { logger.error("Failed to rollback transaction", he); } } - rollbackCurrentSessionTx(); throw new RuntimeException("Transaction failed", e); } finally { if (!existingTransaction && session != null && session.isOpen()) { From f3b1fa687d3a30e3a1ef9e4b397cec1f799d7cbe Mon Sep 17 00:00:00 2001 From: brianbrix Date: Wed, 4 Jun 2025 18:49:57 +0300 Subject: [PATCH 14/30] AMP-30971: Thread issues when updating activity via airflow/API --- .../org/digijava/kernel/persistence/PersistenceManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 74d1b667811..80123be9b9a 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -549,8 +549,8 @@ public static T supplyInTransaction(Supplier supplier) { } catch (Exception e) { if (!existingTransaction && transaction != null && transaction.isActive()) { try { - rollbackCurrentSessionTx(); - + logger.info("Rolling back after exception"); + transaction.rollback(); } catch (HibernateException he) { logger.error("Failed to rollback transaction", he); } From c64b09564ace3a14aa0071444f9325135c9062e2 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Thu, 12 Jun 2025 21:09:46 +0300 Subject: [PATCH 15/30] AMP 30971: Fix update activities --- .../org/digijava/kernel/persistence/PersistenceManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 80123be9b9a..f0942aa1fb0 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -533,6 +533,7 @@ public static T supplyInTransaction(Supplier supplier) { Session session = getSession(); boolean existingTransaction = session.getTransaction().isActive(); Transaction transaction = null; + boolean committed = false; try { if (!existingTransaction) { @@ -543,6 +544,7 @@ public static T supplyInTransaction(Supplier supplier) { if (!existingTransaction && transaction != null) { transaction.commit(); + committed = true; } return result; @@ -559,7 +561,7 @@ public static T supplyInTransaction(Supplier supplier) { } finally { if (!existingTransaction && session != null && session.isOpen()) { try { - if (session.getTransaction().isActive()) { + if (!committed && session.getTransaction().isActive()) { session.getTransaction().rollback(); } session.close(); From d56e1f5cdefd7d3c00f9142cc6551f0650e87b02 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Thu, 12 Jun 2025 21:29:13 +0300 Subject: [PATCH 16/30] AMP 30971: Fix update activities --- .../digijava/module/aim/action/dataimporter/ExcelImporter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/ExcelImporter.java b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/ExcelImporter.java index d41c5e64c98..651e0d6d3f8 100644 --- a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/ExcelImporter.java +++ b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/ExcelImporter.java @@ -141,7 +141,7 @@ public static void processBatch(List batch,Sheet sheet, HttpServletRequest importDataModel.setModified_by(TeamMemberUtil.getCurrentAmpTeamMember(request).getAmpTeamMemId()); importDataModel.setCreated_by(TeamMemberUtil.getCurrentAmpTeamMember(request).getAmpTeamMemId()); importDataModel.setTeam(TeamMemberUtil.getCurrentAmpTeamMember(request).getAmpTeam().getAmpTeamId()); - importDataModel.setIs_draft(true); + importDataModel.setIs_draft(false); OffsetDateTime now = OffsetDateTime.now(ZoneOffset.UTC); importDataModel.setCreation_date(now.format(formatter)); setStatus(importDataModel); From c17e2ab6c54c6952083aead4a625fbe6cacc0d8e Mon Sep 17 00:00:00 2001 From: brianbrix Date: Thu, 12 Jun 2025 21:33:41 +0300 Subject: [PATCH 17/30] AMP 30971: Fix update activities --- Jenkinsfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index de276d66652..353483163cc 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -69,9 +69,9 @@ stage('Build') { println "AMP Version: ${codeVersion}" //Used in the initial generation of keys when working with a new jenkins instance //**************************************************************** -// sh "ssh-keygen -t rsa -b 4096 -C 'jenkins@${environment}' -f ~/.ssh/id_rsa -N ''" + sh "ssh-keygen -t rsa -b 4096 -C 'jenkins@${environment}' -f ~/.ssh/id_rsa -N ''" sh "ssh-keyscan -H ${environment} >> ~/.ssh/known_hosts" -// sh "cat /root/.ssh/id_rsa.pub" + sh "cat /root/.ssh/id_rsa.pub" //****************************************************** countries = sh(returnStdout: true, script: "ssh ${env.jenkinsUser}@${environment} 'cd /opt/amp_dbs && amp-db ls ${codeVersion} | sort'") From d3a170d3b24f792ec6835bac0dc5b06e3c0f0efd Mon Sep 17 00:00:00 2001 From: brianbrix Date: Thu, 12 Jun 2025 21:34:18 +0300 Subject: [PATCH 18/30] AMP 30971: Fix update activities --- Jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 353483163cc..9b965d8c213 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -69,7 +69,7 @@ stage('Build') { println "AMP Version: ${codeVersion}" //Used in the initial generation of keys when working with a new jenkins instance //**************************************************************** - sh "ssh-keygen -t rsa -b 4096 -C 'jenkins@${environment}' -f ~/.ssh/id_rsa -N ''" + sh "ssh-keygen -t rsa -b 4096 -C '${env.jenkinsUser}@${environment}' -f ~/.ssh/id_rsa -N ''" sh "ssh-keyscan -H ${environment} >> ~/.ssh/known_hosts" sh "cat /root/.ssh/id_rsa.pub" //****************************************************** From c52f735cbf87fe7c73a1ce4aced1af04f533c499 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Thu, 12 Jun 2025 21:36:50 +0300 Subject: [PATCH 19/30] AMP 30971: Fix update activities --- Jenkinsfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 9b965d8c213..4ecbf3d359a 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -69,9 +69,9 @@ stage('Build') { println "AMP Version: ${codeVersion}" //Used in the initial generation of keys when working with a new jenkins instance //**************************************************************** - sh "ssh-keygen -t rsa -b 4096 -C '${env.jenkinsUser}@${environment}' -f ~/.ssh/id_rsa -N ''" +// sh "ssh-keygen -t rsa -b 4096 -C '${env.jenkinsUser}@${environment}' -f ~/.ssh/id_rsa -N ''" sh "ssh-keyscan -H ${environment} >> ~/.ssh/known_hosts" - sh "cat /root/.ssh/id_rsa.pub" +// sh "cat /root/.ssh/id_rsa.pub" //****************************************************** countries = sh(returnStdout: true, script: "ssh ${env.jenkinsUser}@${environment} 'cd /opt/amp_dbs && amp-db ls ${codeVersion} | sort'") From 37a42685fff5d9d4f16f5a89553ce35e0c9ef1b6 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 07:34:18 +0300 Subject: [PATCH 20/30] AMP 30971: Fix update activities --- .../kernel/persistence/PersistenceManager.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index f0942aa1fb0..89d016acaea 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -68,6 +68,7 @@ public class PersistenceManager { private static Logger logger = I18NHelper.getKernelLogger(PersistenceManager.class); private static final AtomicInteger activeSessions = new AtomicInteger(); + private static final ThreadLocal transactionOwner = new ThreadLocal<>(); private static final HashMap sessionStackTraceMap = new HashMap<>(); @@ -564,7 +565,7 @@ public static T supplyInTransaction(Supplier supplier) { if (!committed && session.getTransaction().isActive()) { session.getTransaction().rollback(); } - session.close(); + closeSession(session); } catch (HibernateException e) { logger.error("Failed to close session", e); } @@ -587,19 +588,23 @@ public static Session getSession() { addSessionToStackTraceMap(session); } - // Ensure we have an active transaction for write operations if (!session.getTransaction().isActive()) { session.beginTransaction(); + transactionOwner.set(true); + } else { + transactionOwner.set(false); } return session; } + public static void setCurrentSession(Session session) { threadSession.set(session); } public static void clearCurrentSession() { threadSession.remove(); + transactionOwner.remove(); } public static boolean isSessionManaged() { @@ -770,7 +775,7 @@ public static void closeSession(Session session) { try { if (session.isOpen()) { - if (session.getTransaction().isActive()) { + if (Boolean.TRUE.equals(transactionOwner.get()) && session.getTransaction().isActive()) { try { session.getTransaction().rollback(); } catch (HibernateException e) { From c2d723d4a660043ea8b5f12e8aadc8316fbdbade Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 07:57:40 +0300 Subject: [PATCH 21/30] AMP 30971: Fix update activities --- .../persistence/PersistenceManager.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 89d016acaea..6abdf5f9605 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -68,7 +68,6 @@ public class PersistenceManager { private static Logger logger = I18NHelper.getKernelLogger(PersistenceManager.class); private static final AtomicInteger activeSessions = new AtomicInteger(); - private static final ThreadLocal transactionOwner = new ThreadLocal<>(); private static final HashMap sessionStackTraceMap = new HashMap<>(); @@ -531,7 +530,7 @@ public static void inTransaction(Runnable runnable) { } public static T supplyInTransaction(Supplier supplier) { - Session session = getSession(); + Session session = getSessionWithoutBeginningTransaction(); boolean existingTransaction = session.getTransaction().isActive(); Transaction transaction = null; boolean committed = false; @@ -565,7 +564,7 @@ public static T supplyInTransaction(Supplier supplier) { if (!committed && session.getTransaction().isActive()) { session.getTransaction().rollback(); } - closeSession(session); + session.close(); } catch (HibernateException e) { logger.error("Failed to close session", e); } @@ -580,6 +579,15 @@ public static T supplyInTransaction(Supplier supplier) { * upon creating a new session, a transaction is created. */ public static Session getSession() { + Session session = getSessionWithoutBeginningTransaction(); + if (session.getTransaction() == null || !session.getTransaction().isActive()) { + session.beginTransaction(); + } + + return session; + } + + public static Session getSessionWithoutBeginningTransaction() { Session session = threadSession.get(); if (session == null || !session.isOpen()) { session = sf.openSession(); @@ -587,24 +595,14 @@ public static Session getSession() { threadSession.set(session); addSessionToStackTraceMap(session); } - - if (!session.getTransaction().isActive()) { - session.beginTransaction(); - transactionOwner.set(true); - } else { - transactionOwner.set(false); - } - return session; } - public static void setCurrentSession(Session session) { threadSession.set(session); } public static void clearCurrentSession() { threadSession.remove(); - transactionOwner.remove(); } public static boolean isSessionManaged() { @@ -775,7 +773,7 @@ public static void closeSession(Session session) { try { if (session.isOpen()) { - if (Boolean.TRUE.equals(transactionOwner.get()) && session.getTransaction().isActive()) { + if (session.getTransaction().isActive()) { try { session.getTransaction().rollback(); } catch (HibernateException e) { From b740654a893442e85ee9f2fa414a86eba717ea05 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 08:11:34 +0300 Subject: [PATCH 22/30] AMP 30971: Fix update activities --- .../org/digijava/kernel/persistence/PersistenceManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 6abdf5f9605..69f79e8b6e6 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -537,12 +537,14 @@ public static T supplyInTransaction(Supplier supplier) { try { if (!existingTransaction) { + logger.info("Starting new transaction..."); transaction = session.beginTransaction(); + logger.info("Transaction started: " + transaction.isActive()); } T result = supplier.get(); - if (!existingTransaction && transaction != null) { + if (!existingTransaction) { transaction.commit(); committed = true; } From c699becadcb6700c7917b2caa9dc92dae8692287 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 08:28:02 +0300 Subject: [PATCH 23/30] AMP 30971: Fix update activities --- .../org/digijava/kernel/persistence/PersistenceManager.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 69f79e8b6e6..0e4a1d7515a 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -545,8 +545,10 @@ public static T supplyInTransaction(Supplier supplier) { T result = supplier.get(); if (!existingTransaction) { - transaction.commit(); - committed = true; + if (transaction.getStatus().equals(TransactionStatus.ACTIVE)) { + transaction.commit(); + committed = true; + } } return result; From 5f1e9cc2f2d4b3e7d248b4fed39475e6428d9e2e Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 08:28:47 +0300 Subject: [PATCH 24/30] AMP 30971: Fix update activities --- .../org/digijava/kernel/persistence/PersistenceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 0e4a1d7515a..6a1d76a61c5 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -545,7 +545,7 @@ public static T supplyInTransaction(Supplier supplier) { T result = supplier.get(); if (!existingTransaction) { - if (transaction.getStatus().equals(TransactionStatus.ACTIVE)) { + if (!transaction.getStatus().equals(TransactionStatus.COMMITTED)) { transaction.commit(); committed = true; } From a70f5cf166d1b6a20446b240fd18e4bfef4da1ca Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 08:46:28 +0300 Subject: [PATCH 25/30] AMP 30971: Fix update activities --- .../org/digijava/kernel/persistence/PersistenceManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 6a1d76a61c5..0e4a1d7515a 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -545,7 +545,7 @@ public static T supplyInTransaction(Supplier supplier) { T result = supplier.get(); if (!existingTransaction) { - if (!transaction.getStatus().equals(TransactionStatus.COMMITTED)) { + if (transaction.getStatus().equals(TransactionStatus.ACTIVE)) { transaction.commit(); committed = true; } From f63a3db296903f177d6bc6f0ffd801aa7b164c34 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 08:51:13 +0300 Subject: [PATCH 26/30] AMP 30971: Fix update activities --- .../java/org/digijava/kernel/persistence/PersistenceManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 0e4a1d7515a..33dfbfb3ed8 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -545,6 +545,7 @@ public static T supplyInTransaction(Supplier supplier) { T result = supplier.get(); if (!existingTransaction) { + logger.info("Committing transaction status..."+ transaction.getStatus()); if (transaction.getStatus().equals(TransactionStatus.ACTIVE)) { transaction.commit(); committed = true; From 7ea4e15b2c9e097bfaaae4c411861fd64a50a1c9 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 09:28:57 +0300 Subject: [PATCH 27/30] AMP 30971: Fix update activities --- .../org/digijava/kernel/persistence/PersistenceManager.java | 6 +++--- .../module/aim/action/dataimporter/util/ImporterUtil.java | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java index 33dfbfb3ed8..28dd09e0137 100644 --- a/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java +++ b/amp/src/main/java/org/digijava/kernel/persistence/PersistenceManager.java @@ -537,15 +537,15 @@ public static T supplyInTransaction(Supplier supplier) { try { if (!existingTransaction) { - logger.info("Starting new transaction..."); + logger.debug("Starting new transaction..."); transaction = session.beginTransaction(); - logger.info("Transaction started: " + transaction.isActive()); + logger.debug("Transaction started: " + transaction.isActive()); } T result = supplier.get(); if (!existingTransaction) { - logger.info("Committing transaction status..."+ transaction.getStatus()); + logger.debug("Committing transaction status..."+ transaction.getStatus()); if (transaction.getStatus().equals(TransactionStatus.ACTIVE)) { transaction.commit(); committed = true; diff --git a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java index 357fc6c0c79..7f7a8167d3b 100644 --- a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java +++ b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java @@ -13,6 +13,7 @@ import org.digijava.kernel.ampapi.endpoints.activity.ActivityInterchangeUtils; import org.digijava.kernel.ampapi.endpoints.activity.dto.ActivitySummary; import org.digijava.kernel.ampapi.endpoints.common.JsonApiResponse; +import org.digijava.kernel.ampapi.filters.AmpClientModeHolder; import org.digijava.kernel.persistence.PersistenceManager; import org.digijava.module.aim.action.dataimporter.dbentity.ImportStatus; import org.digijava.module.aim.action.dataimporter.dbentity.ImportedProject; @@ -622,9 +623,12 @@ public static void importTheData(ImportDataModel importDataModel, Session sessio importDataModel.setProject_title(existing.getName()); importDataModel.setProject_code(!Objects.equals(importDataModel.getProject_code(), "") ? importDataModel.getProject_code() : existing.getProjectCode()); updateFundingOrgsAndSectorsWithAlreadyExisting(existing, importDataModel); + Map activity = ActivityInterchangeUtils.getActivity(existing.getAmpActivityId(), + AmpClientModeHolder.isOfflineClient()); map = objectMapper .convertValue(importDataModel, new TypeReference>() { }); + map.putAll(activity); response = ActivityInterchangeUtils.importActivity(map, true, rules, "activity/update"); } if (response != null) { From ec41ded7eb7357a8cbb3a30a54dd2d4128db6679 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 09:30:56 +0300 Subject: [PATCH 28/30] AMP 30971: Fix update activities --- .../module/aim/action/dataimporter/util/ImporterUtil.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java index 7f7a8167d3b..39f6ec19e0a 100644 --- a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java +++ b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java @@ -628,8 +628,8 @@ public static void importTheData(ImportDataModel importDataModel, Session sessio map = objectMapper .convertValue(importDataModel, new TypeReference>() { }); - map.putAll(activity); - response = ActivityInterchangeUtils.importActivity(map, true, rules, "activity/update"); + activity.putAll(map); + response = ActivityInterchangeUtils.importActivity(activity, true, rules, "activity/update"); } if (response != null) { if (!response.getErrors().isEmpty()) { From 0262ebb124ef6ca8a2507b9136b7f9b39c7824a9 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 09:54:48 +0300 Subject: [PATCH 29/30] AMP 30971: Fix update activities --- .../action/dataimporter/util/ImporterUtil.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java index 39f6ec19e0a..4d0e5a7643f 100644 --- a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java +++ b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java @@ -628,7 +628,7 @@ public static void importTheData(ImportDataModel importDataModel, Session sessio map = objectMapper .convertValue(importDataModel, new TypeReference>() { }); - activity.putAll(map); + mergeMaps(activity, map); response = ActivityInterchangeUtils.importActivity(activity, true, rules, "activity/update"); } if (response != null) { @@ -658,6 +658,22 @@ public static void importTheData(ImportDataModel importDataModel, Session sessio logger.info("Imported project: " + importedProject); } + public static void mergeMaps(Map targetMap, Map sourceMap) { + for (Map.Entry entry : sourceMap.entrySet()) { + String key = entry.getKey(); + Object sourceValue = entry.getValue(); + Object targetValue = targetMap.get(key); + + if (targetValue instanceof Collection && sourceValue instanceof Collection) { + List mergedList = new ArrayList<>((List) targetValue); + mergedList.addAll((List) sourceValue); + targetMap.put(key, mergedList); + } else { + // Add or overwrite + targetMap.put(key, sourceValue); + } + } + } private static void updateFundingOrgsAndSectorsWithAlreadyExisting(AmpActivityVersion ampActivityVersion, ImportDataModel importDataModel) { From 97c97fc6935f12ca95c1e3f5110fbf37dbb2a511 Mon Sep 17 00:00:00 2001 From: brianbrix Date: Fri, 13 Jun 2025 10:18:33 +0300 Subject: [PATCH 30/30] AMP 30971: Fix update activities --- .../dataimporter/util/ImporterUtil.java | 23 +++---------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java index 4d0e5a7643f..f07e91160fb 100644 --- a/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java +++ b/amp/src/main/java/org/digijava/module/aim/action/dataimporter/util/ImporterUtil.java @@ -623,13 +623,11 @@ public static void importTheData(ImportDataModel importDataModel, Session sessio importDataModel.setProject_title(existing.getName()); importDataModel.setProject_code(!Objects.equals(importDataModel.getProject_code(), "") ? importDataModel.getProject_code() : existing.getProjectCode()); updateFundingOrgsAndSectorsWithAlreadyExisting(existing, importDataModel); - Map activity = ActivityInterchangeUtils.getActivity(existing.getAmpActivityId(), - AmpClientModeHolder.isOfflineClient()); + map = objectMapper .convertValue(importDataModel, new TypeReference>() { }); - mergeMaps(activity, map); - response = ActivityInterchangeUtils.importActivity(activity, true, rules, "activity/update"); + response = ActivityInterchangeUtils.importActivity(map, true, rules, "activity/update"); } if (response != null) { if (!response.getErrors().isEmpty()) { @@ -658,22 +656,7 @@ public static void importTheData(ImportDataModel importDataModel, Session sessio logger.info("Imported project: " + importedProject); } - public static void mergeMaps(Map targetMap, Map sourceMap) { - for (Map.Entry entry : sourceMap.entrySet()) { - String key = entry.getKey(); - Object sourceValue = entry.getValue(); - Object targetValue = targetMap.get(key); - - if (targetValue instanceof Collection && sourceValue instanceof Collection) { - List mergedList = new ArrayList<>((List) targetValue); - mergedList.addAll((List) sourceValue); - targetMap.put(key, mergedList); - } else { - // Add or overwrite - targetMap.put(key, sourceValue); - } - } - } + private static void updateFundingOrgsAndSectorsWithAlreadyExisting(AmpActivityVersion ampActivityVersion, ImportDataModel importDataModel) {