diff --git a/component/api/src/main/java/org/exoplatform/commons/api/persistence/ExoTransactional.java b/component/api/src/main/java/org/exoplatform/commons/api/persistence/ExoTransactional.java index e43135e597..161d50134e 100644 --- a/component/api/src/main/java/org/exoplatform/commons/api/persistence/ExoTransactional.java +++ b/component/api/src/main/java/org/exoplatform/commons/api/persistence/ExoTransactional.java @@ -28,6 +28,13 @@ * REQUIRED. Support a current transaction, create a new one if none exists. * Analogous to EJB or Spring transaction attribute of the same name. * + * When a call that failed marked the transaction that the method began + * rollback-only, the transaction is rolled back: a method that returned then + * throws a jakarta.persistence.RollbackException, and a method that threw + * keeps its exception, unless ending the transaction fails. A method that + * joined an active transaction never throws it: the method that began the + * transaction does. + * * This annotation is handled in org.exoplatform.commons.persistence.impl.ExoTransactionalAspect. * */ diff --git a/component/api/src/main/java/org/exoplatform/portal/config/model/PortalConfig.java b/component/api/src/main/java/org/exoplatform/portal/config/model/PortalConfig.java index 13462c5b93..647df8bfbb 100644 --- a/component/api/src/main/java/org/exoplatform/portal/config/model/PortalConfig.java +++ b/component/api/src/main/java/org/exoplatform/portal/config/model/PortalConfig.java @@ -30,8 +30,10 @@ import org.exoplatform.portal.mop.SiteType; import org.exoplatform.portal.pom.data.PortalData; +import lombok.AccessLevel; import lombok.Data; import lombok.EqualsAndHashCode; +import lombok.Setter; @Data @EqualsAndHashCode(callSuper = true) @@ -88,6 +90,14 @@ public class PortalConfig extends ModelObject implements Cloneable { private long bannerFileId; + /** + * True when the stored layout of the site couldn't be read because its root + * container is missing: a save keeps the stored layout, unless a layout is + * set with {@link #setPortalLayout(Container)}. + */ + @Setter(AccessLevel.NONE) + private boolean layoutUnresolved; + public PortalConfig() { this(PORTAL_TYPE); } @@ -127,6 +137,7 @@ public PortalConfig(PortalData data) { this.displayed = data.isDisplayed(); this.displayOrder = data.getDisplayOrder(); this.bannerFileId = data.getBannerFileId(); + this.layoutUnresolved = data.isLayoutUnresolved(); } public long getId() { @@ -139,6 +150,7 @@ public long getId() { public void setPortalLayout(Container container) { portalLayout = container == null ? new SiteLayout() : new SiteLayout(container); + layoutUnresolved = false; } public String getProperty(String name) { @@ -216,21 +228,23 @@ private static Container initDefaultLayout() { } public PortalData build() { - return new PortalData(storageId, - name, - type, - locale, - label, - description, - accessPermissions == null ? Collections.emptyList() : Arrays.asList(accessPermissions), - editPermission, - properties == null ? Collections.emptyMap() : new Properties(properties), - skin, - portalLayout.build(), - defaultLayout, - displayed, - displayOrder, - bannerFileId); + PortalData portalData = new PortalData(storageId, + name, + type, + locale, + label, + description, + accessPermissions == null ? Collections.emptyList() : Arrays.asList(accessPermissions), + editPermission, + properties == null ? Collections.emptyMap() : new Properties(properties), + skin, + portalLayout.build(), + defaultLayout, + displayed, + displayOrder, + bannerFileId); + portalData.setLayoutUnresolved(layoutUnresolved); + return portalData; } @Override diff --git a/component/api/src/main/java/org/exoplatform/portal/pom/data/PortalData.java b/component/api/src/main/java/org/exoplatform/portal/pom/data/PortalData.java index 3e1b9cc897..9e44da4bd8 100644 --- a/component/api/src/main/java/org/exoplatform/portal/pom/data/PortalData.java +++ b/component/api/src/main/java/org/exoplatform/portal/pom/data/PortalData.java @@ -69,6 +69,12 @@ public class PortalData extends ModelData { private long bannerFileId; + /** + * True when the stored layout of the site couldn't be read because its root + * container is missing: a save of this data keeps the stored layout. + */ + private boolean layoutUnresolved; + private PortalData() { super(null, null); this.key = null; diff --git a/component/common/src/main/java/io/meeds/common/ContainerTransactional.java b/component/common/src/main/java/io/meeds/common/ContainerTransactional.java index c562d8b0b0..d6e2c353ff 100644 --- a/component/common/src/main/java/io/meeds/common/ContainerTransactional.java +++ b/component/common/src/main/java/io/meeds/common/ContainerTransactional.java @@ -26,11 +26,35 @@ /** * Describe a method as transactional. This annotation will allow to: * - Set current container to PortalContainer - * - Start a transaction for IDM and JPA sessions + * - Start the request lifecycle of the container: the IDM session transaction + * and the JPA EntityManager. The JPA writes made through kernel DAOs commit + * one DAO call at a time, unless {@link #atomic()} is set. * - * This annotation is handled in io.meeds.commons.persistence.impl.ContainerTransactionalAspect. + * This annotation is handled in io.meeds.common.ContainerTransactionalAspect. */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) public @interface ContainerTransactional { + + /** + * @return true to run the method in one JPA transaction, committed when it + * returns and rolled back when it throws. The writes made through + * the kernel EntityManager (kernel DAOs, ExoTransactional methods) + * join it. When the method begins the transaction, the Spring + * transactional methods, Spring Data repositories included, join it + * too, unless a Spring transaction of its own is active on the + * thread, and the callbacks they register run once it is committed + * or rolled back, unless a Spring transaction synchronization is + * already active on the thread: their callbacks are then left to + * that synchronization. A Spring cache eviction (CacheEvict) made + * inside the method takes effect at once, before the commit. When a + * call that failed marked the transaction rollback-only, a method + * that returned throws a RollbackException after the rollback, and + * a method that threw keeps its exception, unless the rollback + * fails. When a JPA transaction is already + * active on the thread, the method joins it and marks it + * rollback-only when it throws. + */ + boolean atomic() default false; + } diff --git a/component/common/src/main/java/io/meeds/common/ContainerTransactionalAspect.java b/component/common/src/main/java/io/meeds/common/ContainerTransactionalAspect.java index ec5e0a5115..9e41d726cc 100644 --- a/component/common/src/main/java/io/meeds/common/ContainerTransactionalAspect.java +++ b/component/common/src/main/java/io/meeds/common/ContainerTransactionalAspect.java @@ -21,21 +21,37 @@ import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.reflect.MethodSignature; +import org.springframework.transaction.support.TransactionSynchronization; +import org.exoplatform.commons.persistence.impl.EntityManagerService; import org.exoplatform.container.ExoContainer; import org.exoplatform.container.ExoContainerContext; import org.exoplatform.container.PortalContainer; import org.exoplatform.container.RootContainer; import org.exoplatform.container.component.RequestLifeCycle; +import org.exoplatform.services.log.ExoLogger; +import org.exoplatform.services.log.Log; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityTransaction; +import jakarta.persistence.RollbackException; /** * An aspect processing point that will allow to encapsulate two behavior: * - Set current container to PortalContainer - * - Start a transaction for IDM and JPA sessions + * - Start the request lifecycle of the container, and a JPA transaction when + * the annotation is {@link ContainerTransactional#atomic()} */ @Aspect public class ContainerTransactionalAspect { + private static final Log LOG = ExoLogger.getLogger(ContainerTransactionalAspect.class); + + /** + * The advice keeps this signature: the classes an addon wove at its own build + * call it as is. + */ @Around("execution(* *(..)) && @annotation(io.meeds.common.ContainerTransactional)") public Object around(ProceedingJoinPoint point) throws Throwable { ExoContainer container = ExoContainerContext.getCurrentContainer(); @@ -46,7 +62,11 @@ public Object around(ProceedingJoinPoint point) throws Throwable { } RequestLifeCycle.begin(transactionContainer); try { - return point.proceed(); + if (isAtomic(point)) { + return proceedAtomically(point, transactionContainer.getComponentInstanceOfType(EntityManagerService.class)); + } else { + return point.proceed(); + } } finally { RequestLifeCycle.end(); if (transactionContainer != container) { @@ -55,4 +75,95 @@ public Object around(ProceedingJoinPoint point) throws Throwable { } } + /** + * Runs the advised method in the active JPA transaction of the thread, or in + * a new one that is committed when the method returns and rolled back when it + * throws. The kernel DAOs annotated with ExoTransactional join it without + * committing it. When the method begins the transaction, the Spring + * transactional methods (Spring Data repositories included) join it too, + * unless a Spring transaction of its own is active on the thread. + */ + Object proceedAtomically(ProceedingJoinPoint point, EntityManagerService entityManagerService) throws Throwable { + if (entityManagerService == null) { + // No JPA persistence in the container: there is no JPA write to group + return point.proceed(); + } + EntityManager entityManager = entityManagerService.getEntityManager(); + if (entityManager == null) { + throw new IllegalStateException("No JPA EntityManager is started on the current thread: the method can't run atomically"); + } + EntityTransaction transaction = entityManager.getTransaction(); + if (transaction.isActive()) { + return proceedInActiveTransaction(point, transaction); + } else { + return proceedInNewTransaction(point, + transaction, + SpringTransactionScope.open(entityManagerService.getEntityManagerFactory(), entityManager)); + } + } + + private Object proceedInActiveTransaction(ProceedingJoinPoint point, EntityTransaction transaction) throws Throwable { + try { + return point.proceed(); + } catch (Throwable e) { // NOSONAR the enclosing transaction can't be committed whatever the error + if (transaction.isActive()) { + transaction.setRollbackOnly(); + // The method that began the transaction fails with a RollbackException + // once it returns over it: this log names the call that marked it + LOG.warn("{} failed in a transaction it joined, which is marked rollback-only: none of its writes is committed. Cause: {}", + point.getSignature(), + e.toString()); + } + throw e; + } + } + + private Object proceedInNewTransaction(ProceedingJoinPoint point, + EntityTransaction transaction, + SpringTransactionScope springScope) throws Throwable { + int status = TransactionSynchronization.STATUS_UNKNOWN; + boolean committing = false; + try { + transaction.begin(); + Object result = point.proceed(); + if (transaction.getRollbackOnly()) { + springScope.beforeCompletion(); + springScope.close(); + transaction.rollback(); + status = TransactionSynchronization.STATUS_ROLLED_BACK; + throw new RollbackException("The transaction of " + point.getSignature() + + " was marked rollback-only by a call that failed: none of its writes is committed"); + } + springScope.beforeCommit(); + springScope.close(); + committing = true; + transaction.commit(); + status = TransactionSynchronization.STATUS_COMMITTED; + springScope.afterCommit(); + return result; + } catch (Throwable e) { // NOSONAR none of the method writes is committed whatever the error + if (transaction.isActive()) { + springScope.beforeCompletion(); + springScope.close(); + transaction.rollback(); + status = TransactionSynchronization.STATUS_ROLLED_BACK; + } else if (committing && status == TransactionSynchronization.STATUS_UNKNOWN && e instanceof RollbackException) { + // The JPA provider rolls the transaction back before throwing + status = TransactionSynchronization.STATUS_ROLLED_BACK; + } + throw e; + } finally { + springScope.end(status); + } + } + + boolean isAtomic(ProceedingJoinPoint point) { + if (point.getSignature() instanceof MethodSignature methodSignature) { + ContainerTransactional containerTransactional = methodSignature.getMethod().getAnnotation(ContainerTransactional.class); + return containerTransactional != null && containerTransactional.atomic(); + } else { + return false; + } + } + } diff --git a/component/common/src/main/java/io/meeds/common/SpringTransactionScope.java b/component/common/src/main/java/io/meeds/common/SpringTransactionScope.java new file mode 100644 index 0000000000..4e2df52155 --- /dev/null +++ b/component/common/src/main/java/io/meeds/common/SpringTransactionScope.java @@ -0,0 +1,127 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package io.meeds.common; + +import java.util.List; + +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; +import org.springframework.transaction.support.TransactionSynchronizationUtils; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; + +/** + * Spring transactional methods, Spring Data repositories included, write + * through the EntityManager that Spring's transaction synchronization holds + * for the factory. The scope holds the thread's EntityManager there, as a + * transaction already active, so that they join the transaction instead of + * committing their own. When no Spring transaction synchronization is + * active on the thread, the scope opens one and collects the callbacks they + * register (a TransactionalEventListener event) so that they run with the + * outcome of the transaction. The before-commit and before-completion + * callbacks run inside the transaction. The EntityManager is then released + * and the synchronization closed, before the transaction is committed or + * rolled back: a Spring transactional method called by a callback of the + * commit or of the rollback (a TransactionCallbacks callback), or by an + * after-commit or after-completion callback, runs in a Spring transaction of + * its own, with callbacks of its own, and one called while the commit + * flushes the writes (an entity listener) doesn't join the transaction. When a + * synchronization is already active, as in a Spring transaction without an + * EntityManager of its own, the scope collects none of their callbacks: they + * are left to that synchronization, which runs them with its own outcome and, + * without an actual transaction, skips a TransactionalEventListener event + * unless its fallback execution runs it at once. + */ +final class SpringTransactionScope { + + private static final SpringTransactionScope NONE = new SpringTransactionScope(null, false); + + private final EntityManagerFactory boundFactory; + + private final boolean synchronizing; + + private boolean completing; + + private List synchronizations; + + private SpringTransactionScope(EntityManagerFactory boundFactory, boolean synchronizing) { + this.boundFactory = boundFactory; + this.synchronizing = synchronizing; + } + + static SpringTransactionScope open(EntityManagerFactory entityManagerFactory, EntityManager entityManager) { + if (entityManagerFactory == null || TransactionSynchronizationManager.hasResource(entityManagerFactory)) { + // A Spring transaction of its own holds an EntityManager for the + // factory on the thread: the Spring writes stay in it + return NONE; + } + TransactionSynchronizationManager.bindResource(entityManagerFactory, new ThreadEntityManagerHolder(entityManager)); + boolean synchronizing = !TransactionSynchronizationManager.isSynchronizationActive(); + if (synchronizing) { + TransactionSynchronizationManager.initSynchronization(); + TransactionSynchronizationManager.setActualTransactionActive(true); + } + return new SpringTransactionScope(entityManagerFactory, synchronizing); + } + + void beforeCommit() { + if (synchronizing) { + TransactionSynchronizationUtils.triggerBeforeCommit(false); + beforeCompletion(); + } + } + + void beforeCompletion() { + if (synchronizing && !completing) { + completing = true; + TransactionSynchronizationUtils.triggerBeforeCompletion(); + } + } + + void close() { + release(); + if (synchronizing && synchronizations == null) { + synchronizations = TransactionSynchronizationManager.getSynchronizations(); + TransactionSynchronizationManager.clearSynchronization(); + TransactionSynchronizationManager.setActualTransactionActive(false); + } + } + + void afterCommit() { + close(); + if (synchronizing) { + TransactionSynchronizationUtils.invokeAfterCommit(synchronizations); + } + } + + void end(int status) { + close(); + if (synchronizing) { + TransactionSynchronizationUtils.invokeAfterCompletion(synchronizations, status); + } + } + + private void release() { + if (boundFactory != null) { + TransactionSynchronizationManager.unbindResourceIfPossible(boundFactory); + } + } + +} diff --git a/component/common/src/main/java/io/meeds/common/ThreadEntityManagerHolder.java b/component/common/src/main/java/io/meeds/common/ThreadEntityManagerHolder.java new file mode 100644 index 0000000000..7591fe2466 --- /dev/null +++ b/component/common/src/main/java/io/meeds/common/ThreadEntityManagerHolder.java @@ -0,0 +1,39 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package io.meeds.common; + +import org.springframework.orm.jpa.EntityManagerHolder; + +import jakarta.persistence.EntityManager; + +class ThreadEntityManagerHolder extends EntityManagerHolder { + + ThreadEntityManagerHolder(EntityManager entityManager) { + super(entityManager); + // The EntityManager is given to the repositories as is, without joining + // it to a transaction nor closing it with one + setSynchronizedWithTransaction(true); + } + + @Override + protected boolean isTransactionActive() { + return getEntityManager().getTransaction().isActive(); + } + +} diff --git a/component/common/src/main/java/io/meeds/common/TransactionCallbacks.java b/component/common/src/main/java/io/meeds/common/TransactionCallbacks.java new file mode 100644 index 0000000000..c35f6e3a83 --- /dev/null +++ b/component/common/src/main/java/io/meeds/common/TransactionCallbacks.java @@ -0,0 +1,81 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package io.meeds.common; + +import org.hibernate.Transaction; + +import org.exoplatform.commons.persistence.impl.EntityManagerService; +import org.exoplatform.container.ExoContainerContext; +import org.exoplatform.services.log.ExoLogger; +import org.exoplatform.services.log.Log; + +import jakarta.persistence.EntityManager; +import jakarta.transaction.Synchronization; + +/** + * Callbacks run once the JPA transaction of the current thread completes + */ +public class TransactionCallbacks { + + private static final Log LOG = ExoLogger.getLogger(TransactionCallbacks.class); + + private TransactionCallbacks() { + // Utility class + } + + /** + * Registers a callback run once the JPA transaction active on the current + * thread completes, whether it's committed or rolled back. A cache eviction + * made inside a transaction is repeated this way: a concurrent read made + * before the commit caches the previous state again. + * + * @param callback the callback to run, its errors are logged + * @return true when the callback is registered, false when no JPA + * transaction is active on the thread: the callback isn't run then + */ + public static boolean afterCompletion(Runnable callback) { + return afterCompletion(ExoContainerContext.getService(EntityManagerService.class), callback); + } + + static boolean afterCompletion(EntityManagerService entityManagerService, Runnable callback) { + EntityManager entityManager = entityManagerService == null ? null : entityManagerService.getEntityManager(); + if (entityManager == null + || !(entityManager.getTransaction() instanceof Transaction transaction) + || !transaction.isActive()) { + return false; + } + transaction.registerSynchronization(new Synchronization() { + @Override + public void beforeCompletion() { + // Nothing to run before the transaction completes + } + + @Override + public void afterCompletion(int status) { + try { + callback.run(); + } catch (RuntimeException e) { + LOG.warn("Error while running a callback after the completion of a transaction", e); + } + } + }); + return true; + } + +} diff --git a/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java b/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java index 1742aa811c..3ae2738822 100644 --- a/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java +++ b/component/common/src/main/java/org/exoplatform/commons/cache/future/FutureCache.java @@ -92,8 +92,9 @@ public void removeFuture(K key) { * {@link #removeFuture(Object)} applied to a whole-cache clear. */ public void clearFutures() { - futureEntries.values().forEach(Retrieval::invalidate); - futureEntries.clear(); + // Each load leaves the map with its invalidation, so that a load + // registered meanwhile is never dropped from the map without it + futureEntries.keySet().forEach(this::removeFuture); } /** diff --git a/component/common/src/main/java/org/exoplatform/commons/persistence/impl/ExoTransactionalAspect.java b/component/common/src/main/java/org/exoplatform/commons/persistence/impl/ExoTransactionalAspect.java index 4b47c1b557..6da4c57684 100644 --- a/component/common/src/main/java/org/exoplatform/commons/persistence/impl/ExoTransactionalAspect.java +++ b/component/common/src/main/java/org/exoplatform/commons/persistence/impl/ExoTransactionalAspect.java @@ -21,6 +21,7 @@ import jakarta.persistence.EntityManager; import jakarta.persistence.EntityTransaction; import jakarta.persistence.PersistenceException; +import jakarta.persistence.RollbackException; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; @@ -45,12 +46,22 @@ public class ExoTransactionalAspect { @Around("execution(* *(..)) && @annotation(org.exoplatform.commons.api.persistence.ExoTransactional)") public Object around(ProceedingJoinPoint point) throws Throwable { + return proceed(point, getService()); + } + + /** + * Runs the advised method in the active transaction of the thread, or in a + * new one that is committed once the method returns or throws, unless a call + * that failed marked it rollback-only: it is then rolled back, and a method + * that returned throws a RollbackException, so that the writes it lost don't + * go unnoticed. + */ + Object proceed(ProceedingJoinPoint point, EntityManagerService service) throws Throwable { EntityManager entityManager; boolean begunTx = false; boolean emStarted = false; // Do we need to start EntityManager ? - EntityManagerService service = getService(); entityManager = service.getEntityManager(); if (entityManager == null) { LOG.debug("Injecting new PersistenceContext"); @@ -81,36 +92,56 @@ public Object around(ProceedingJoinPoint point) throws Throwable { } begunTx = true; } + Object result; + try { + result = point.proceed(); + } catch (Throwable e) { // NOSONAR the transaction ends whatever the error + endTransaction(service, entityManager, emStarted, tx, begunTx); + throw e; + } + if (endTransaction(service, entityManager, emStarted, tx, begunTx)) { + throw new RollbackException("The transaction of " + point.getSignature() + + " was marked rollback-only by a call that failed: none of its writes is committed"); + } + return result; + } + + /** + * @return true when the transaction that the advice began was marked + * rollback-only, and is rolled back + */ + private boolean endTransaction(EntityManagerService service, + EntityManager entityManager, + boolean emStarted, + EntityTransaction tx, + boolean begunTx) { try { - return point.proceed(); - } finally { // Do we need to end Transaction ? - try { - if ((begunTx) && (tx.isActive())) { - if (!tx.getRollbackOnly()) { - LOG.debug("Committing current transaction"); - tx.commit(); - } else { - LOG.debug("Rollback current transaction set as RollbackOnly"); - tx.rollback(); - } + if ((begunTx) && (tx.isActive())) { + if (!tx.getRollbackOnly()) { + LOG.debug("Committing current transaction"); + tx.commit(); + } else { + LOG.debug("Rollback current transaction set as RollbackOnly"); + tx.rollback(); + return true; } - } catch (RuntimeException ex) { - try { - if (tx != null && tx.isActive()) { - tx.rollback(); - } - } catch (RuntimeException rbEx) { - LOG.error("Could not roll back transaction", rbEx); - } - throw ex; - } finally { - // Do we need to close EntityManager ? - if (emStarted && (entityManager != null)) { - service.closeEntityManager(); + } + return false; + } catch (RuntimeException ex) { + try { + if (tx != null && tx.isActive()) { + tx.rollback(); } + } catch (RuntimeException rbEx) { + LOG.error("Could not roll back transaction", rbEx); + } + throw ex; + } finally { + // Do we need to close EntityManager ? + if (emStarted && (entityManager != null)) { + service.closeEntityManager(); } - } } diff --git a/component/common/src/test/java/io/meeds/common/ContainerTransactionalAspectTest.java b/component/common/src/test/java/io/meeds/common/ContainerTransactionalAspectTest.java new file mode 100644 index 0000000000..f56447a04f --- /dev/null +++ b/component/common/src/test/java/io/meeds/common/ContainerTransactionalAspectTest.java @@ -0,0 +1,444 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package io.meeds.common; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import java.util.List; + +import org.aspectj.lang.ProceedingJoinPoint; +import org.aspectj.lang.Signature; +import org.aspectj.lang.reflect.MethodSignature; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.orm.jpa.EntityManagerHolder; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; + +import org.exoplatform.commons.persistence.impl.EntityManagerService; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; +import jakarta.persistence.EntityTransaction; +import jakarta.persistence.RollbackException; + +@ExtendWith(MockitoExtension.class) +class ContainerTransactionalAspectTest { + + private static final Object RESULT = new Object(); + + @Mock + private ProceedingJoinPoint point; + + @Mock + private EntityManagerService entityManagerService; + + @Mock + private EntityManagerFactory entityManagerFactory; + + @Mock + private EntityManager entityManager; + + @Mock + private EntityTransaction transaction; + + private ContainerTransactionalAspect aspect = new ContainerTransactionalAspect(); + + @BeforeEach + void setUp() { + lenient().when(entityManagerService.getEntityManager()).thenReturn(entityManager); + lenient().when(entityManagerService.getEntityManagerFactory()).thenReturn(entityManagerFactory); + lenient().when(entityManager.getTransaction()).thenReturn(transaction); + } + + private boolean isSpringScopeClosed() { + return !TransactionSynchronizationManager.hasResource(entityManagerFactory) + && !TransactionSynchronizationManager.isSynchronizationActive() + && !TransactionSynchronizationManager.isActualTransactionActive(); + } + + @AfterEach + void tearDown() { + TransactionSynchronizationManager.unbindResourceIfPossible(entityManagerFactory); + if (TransactionSynchronizationManager.isSynchronizationActive()) { + TransactionSynchronizationManager.clearSynchronization(); + } + TransactionSynchronizationManager.setActualTransactionActive(false); + } + + @Test + void testCommitWhenTheMethodReturns() throws Throwable { + when(point.proceed()).thenReturn(RESULT); + + assertSame(RESULT, aspect.proceedAtomically(point, entityManagerService)); + + InOrder order = inOrder(transaction, point); + order.verify(transaction).begin(); + order.verify(point).proceed(); + order.verify(transaction).commit(); + verify(transaction, never()).rollback(); + assertTrue(isSpringScopeClosed()); + } + + @Test + void testRollbackWhenTheMethodThrows() throws Throwable { + IllegalStateException error = new IllegalStateException("write failed"); + when(point.proceed()).thenAnswer(invocation -> { + when(transaction.isActive()).thenReturn(true); + throw error; + }); + + assertSame(error, assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService))); + + verify(transaction).begin(); + verify(transaction).rollback(); + verify(transaction, never()).commit(); + assertTrue(isSpringScopeClosed()); + } + + @Test + void testCloseTheSpringScopeWhenTheTransactionCantBegin() throws Throwable { + IllegalStateException error = new IllegalStateException("begin failed"); + doThrow(error).when(transaction).begin(); + + assertSame(error, assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService))); + + verify(point, never()).proceed(); + verify(transaction, never()).commit(); + verify(transaction, never()).rollback(); + assertTrue(isSpringScopeClosed()); + } + + @Test + void testRollbackAndThrowWhenTheTransactionIsMarkedRollbackOnly() throws Throwable { + when(point.proceed()).thenAnswer(invocation -> { + when(transaction.isActive()).thenReturn(true); + when(transaction.getRollbackOnly()).thenReturn(true); + return RESULT; + }); + boolean[] closedBeforeRollback = new boolean[1]; + doAnswer(invocation -> { + closedBeforeRollback[0] = isSpringScopeClosed(); + return when(transaction.isActive()).thenReturn(false); + }).when(transaction).rollback(); + + assertThrows(RollbackException.class, () -> aspect.proceedAtomically(point, entityManagerService)); + + verify(transaction).rollback(); + verify(transaction, never()).commit(); + assertTrue(closedBeforeRollback[0]); + } + + @Test + void testJoinTheActiveTransaction() throws Throwable { + when(transaction.isActive()).thenReturn(true); + when(point.proceed()).thenReturn(RESULT); + + assertSame(RESULT, aspect.proceedAtomically(point, entityManagerService)); + + verify(transaction, never()).begin(); + verify(transaction, never()).commit(); + verify(transaction, never()).rollback(); + verify(transaction, never()).setRollbackOnly(); + } + + @Test + void testMarkTheJoinedTransactionRollbackOnlyWhenTheMethodThrows() throws Throwable { + IllegalStateException error = new IllegalStateException("write failed"); + when(transaction.isActive()).thenReturn(true); + when(point.proceed()).thenThrow(error); + + assertSame(error, assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService))); + + verify(transaction).setRollbackOnly(); + verify(transaction, never()).begin(); + verify(transaction, never()).commit(); + verify(transaction, never()).rollback(); + } + + @Test + void testHoldTheEntityManagerForSpringRepositoriesDuringTheMethod() throws Throwable { + when(point.proceed()).thenAnswer(invocation -> { + EntityManagerHolder holder = (EntityManagerHolder) TransactionSynchronizationManager.getResource(entityManagerFactory); + assertSame(entityManager, holder.getEntityManager()); + assertTrue(holder.isSynchronizedWithTransaction()); + throw new IllegalStateException("write failed"); + }); + + assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService)); + + assertFalse(TransactionSynchronizationManager.hasResource(entityManagerFactory)); + } + + @Test + void testKeepTheEntityManagerOfASpringTransaction() throws Throwable { + EntityManagerHolder springHolder = new EntityManagerHolder(mock(EntityManager.class)); + TransactionSynchronizationManager.bindResource(entityManagerFactory, springHolder); + when(point.proceed()).thenAnswer(invocation -> { + assertSame(springHolder, TransactionSynchronizationManager.getResource(entityManagerFactory)); + return RESULT; + }); + + assertSame(RESULT, aspect.proceedAtomically(point, entityManagerService)); + + assertSame(springHolder, TransactionSynchronizationManager.getResource(entityManagerFactory)); + } + + @Test + void testRunTheSpringCallbacksOnceTheTransactionIsCommitted() throws Throwable { + TransactionSynchronization synchronization = mock(TransactionSynchronization.class); + when(point.proceed()).thenAnswer(invocation -> { + assertTrue(TransactionSynchronizationManager.isActualTransactionActive()); + TransactionSynchronizationManager.registerSynchronization(synchronization); + return RESULT; + }); + + assertSame(RESULT, aspect.proceedAtomically(point, entityManagerService)); + + InOrder order = inOrder(synchronization, transaction); + order.verify(synchronization).beforeCommit(false); + order.verify(synchronization).beforeCompletion(); + order.verify(transaction).commit(); + order.verify(synchronization).afterCommit(); + order.verify(synchronization).afterCompletion(TransactionSynchronization.STATUS_COMMITTED); + assertFalse(TransactionSynchronizationManager.isSynchronizationActive()); + assertFalse(TransactionSynchronizationManager.isActualTransactionActive()); + } + + @Test + void testRunTheSpringCallbacksOnceTheTransactionIsRolledBack() throws Throwable { + TransactionSynchronization synchronization = mock(TransactionSynchronization.class); + IllegalStateException error = new IllegalStateException("write failed"); + when(point.proceed()).thenAnswer(invocation -> { + when(transaction.isActive()).thenReturn(true); + TransactionSynchronizationManager.registerSynchronization(synchronization); + throw error; + }); + + assertSame(error, assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService))); + + InOrder order = inOrder(synchronization, transaction); + order.verify(synchronization).beforeCompletion(); + order.verify(transaction).rollback(); + order.verify(synchronization).afterCompletion(TransactionSynchronization.STATUS_ROLLED_BACK); + verify(synchronization, never()).afterCommit(); + assertFalse(TransactionSynchronizationManager.isSynchronizationActive()); + } + + @Test + void testReleaseTheEntityManagerBeforeTheSpringCallbacks() throws Throwable { + boolean[] releasedBeforeAfterCommit = new boolean[1]; + boolean[] scopeClosedBeforeAfterCommit = new boolean[1]; + boolean[] scopeClosedBeforeAfterCompletion = new boolean[1]; + when(point.proceed()).thenAnswer(invocation -> { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + releasedBeforeAfterCommit[0] = !TransactionSynchronizationManager.hasResource(entityManagerFactory); + scopeClosedBeforeAfterCommit[0] = !TransactionSynchronizationManager.isSynchronizationActive() + && !TransactionSynchronizationManager.isActualTransactionActive(); + } + + @Override + public void afterCompletion(int status) { + scopeClosedBeforeAfterCompletion[0] = !TransactionSynchronizationManager.isSynchronizationActive(); + } + }); + return RESULT; + }); + + aspect.proceedAtomically(point, entityManagerService); + + assertTrue(releasedBeforeAfterCommit[0]); + assertTrue(scopeClosedBeforeAfterCommit[0], "A Spring transactional call of an after-commit callback has callbacks of its own"); + assertTrue(scopeClosedBeforeAfterCompletion[0]); + } + + @Test + void testReleaseTheEntityManagerBeforeTheSpringCallbacksOfARollback() throws Throwable { + boolean[] releasedBeforeAfterCompletion = new boolean[1]; + boolean[] scopeClosedBeforeAfterCompletion = new boolean[1]; + when(point.proceed()).thenAnswer(invocation -> { + when(transaction.isActive()).thenReturn(true); + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCompletion(int status) { + releasedBeforeAfterCompletion[0] = !TransactionSynchronizationManager.hasResource(entityManagerFactory); + scopeClosedBeforeAfterCompletion[0] = !TransactionSynchronizationManager.isSynchronizationActive() + && !TransactionSynchronizationManager.isActualTransactionActive(); + } + }); + throw new IllegalStateException("write failed"); + }); + + assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService)); + + assertTrue(releasedBeforeAfterCompletion[0]); + assertTrue(scopeClosedBeforeAfterCompletion[0], "A Spring transactional call of an after-completion callback has callbacks of its own"); + } + + @Test + void testLeaveTheActiveSpringSynchronizationToItsTransaction() throws Throwable { + TransactionSynchronization outerSynchronization = mock(TransactionSynchronization.class); + TransactionSynchronizationManager.initSynchronization(); + TransactionSynchronizationManager.registerSynchronization(outerSynchronization); + when(point.proceed()).thenAnswer(invocation -> { + EntityManagerHolder holder = (EntityManagerHolder) TransactionSynchronizationManager.getResource(entityManagerFactory); + assertSame(entityManager, holder.getEntityManager()); + return RESULT; + }); + + assertSame(RESULT, aspect.proceedAtomically(point, entityManagerService)); + + verify(transaction).commit(); + assertFalse(TransactionSynchronizationManager.hasResource(entityManagerFactory)); + assertTrue(TransactionSynchronizationManager.isSynchronizationActive()); + assertEquals(List.of(outerSynchronization), TransactionSynchronizationManager.getSynchronizations()); + verifyNoInteractions(outerSynchronization); + } + + @Test + void testCloseTheSpringScopeBeforeTheCommit() throws Throwable { + boolean[] closedBeforeCommit = new boolean[1]; + when(point.proceed()).thenReturn(RESULT); + doAnswer(invocation -> closedBeforeCommit[0] = isSpringScopeClosed()).when(transaction).commit(); + + assertSame(RESULT, aspect.proceedAtomically(point, entityManagerService)); + + assertTrue(closedBeforeCommit[0], "A Spring transactional call of a callback of the commit runs in a Spring transaction of its own"); + } + + @Test + void testCloseTheSpringScopeBeforeTheRollback() throws Throwable { + boolean[] closedBeforeRollback = new boolean[1]; + when(point.proceed()).thenAnswer(invocation -> { + when(transaction.isActive()).thenReturn(true); + throw new IllegalStateException("write failed"); + }); + doAnswer(invocation -> closedBeforeRollback[0] = isSpringScopeClosed()).when(transaction).rollback(); + + assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService)); + + assertTrue(closedBeforeRollback[0], "A Spring transactional call of a callback of the rollback runs in a Spring transaction of its own"); + } + + @Test + void testReportARollbackWhenTheCommitFails() throws Throwable { + TransactionSynchronization synchronization = mock(TransactionSynchronization.class); + RollbackException error = new RollbackException("flush failed"); + when(point.proceed()).thenAnswer(invocation -> { + TransactionSynchronizationManager.registerSynchronization(synchronization); + return RESULT; + }); + doThrow(error).when(transaction).commit(); + + assertSame(error, assertThrows(RollbackException.class, () -> aspect.proceedAtomically(point, entityManagerService))); + + verify(synchronization).afterCompletion(TransactionSynchronization.STATUS_ROLLED_BACK); + verify(synchronization, never()).afterCommit(); + } + + @Test + void testOpenNoSpringScopeInAJoinedTransaction() throws Throwable { + when(transaction.isActive()).thenReturn(true); + when(point.proceed()).thenAnswer(invocation -> { + assertFalse(TransactionSynchronizationManager.hasResource(entityManagerFactory)); + assertFalse(TransactionSynchronizationManager.isSynchronizationActive()); + return RESULT; + }); + + assertSame(RESULT, aspect.proceedAtomically(point, entityManagerService)); + } + + @Test + void testProceedWithoutJpaPersistence() throws Throwable { + when(point.proceed()).thenReturn(RESULT); + + assertSame(RESULT, aspect.proceedAtomically(point, null)); + } + + @Test + void testRefuseToRunWithoutEntityManager() throws Throwable { + when(entityManagerService.getEntityManager()).thenReturn(null); + + assertThrows(IllegalStateException.class, () -> aspect.proceedAtomically(point, entityManagerService)); + + verify(point, never()).proceed(); + } + + @Test + void testIsAtomic() throws Exception { + assertTrue(aspect.isAtomic(pointOf("atomicMethod"))); + assertFalse(aspect.isAtomic(pointOf("containerMethod"))); + assertFalse(aspect.isAtomic(pointOf("plainMethod"))); + ProceedingJoinPoint constructorPoint = mock(ProceedingJoinPoint.class); + when(constructorPoint.getSignature()).thenReturn(mock(Signature.class)); + assertFalse(aspect.isAtomic(constructorPoint)); + } + + @Test + void testAtomicIsOptIn() throws Exception { + assertEquals(Boolean.FALSE, ContainerTransactional.class.getMethod("atomic").getDefaultValue()); + } + + private ProceedingJoinPoint pointOf(String methodName) throws NoSuchMethodException { + MethodSignature signature = mock(MethodSignature.class); + when(signature.getMethod()).thenReturn(AnnotatedMethods.class.getDeclaredMethod(methodName)); + ProceedingJoinPoint methodPoint = mock(ProceedingJoinPoint.class); + when(methodPoint.getSignature()).thenReturn(signature); + return methodPoint; + } + + @SuppressWarnings("unused") + private static class AnnotatedMethods { + + @ContainerTransactional(atomic = true) + void atomicMethod() { + // Annotation holder + } + + @ContainerTransactional + void containerMethod() { + // Annotation holder + } + + void plainMethod() { + // Annotation holder + } + } + +} diff --git a/component/common/src/test/java/io/meeds/common/ContainerTransactionalRepositoryTest.java b/component/common/src/test/java/io/meeds/common/ContainerTransactionalRepositoryTest.java new file mode 100644 index 0000000000..940931adb7 --- /dev/null +++ b/component/common/src/test/java/io/meeds/common/ContainerTransactionalRepositoryTest.java @@ -0,0 +1,155 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package io.meeds.common; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; + +import org.aspectj.lang.ProceedingJoinPoint; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + +import org.exoplatform.commons.persistence.impl.EntityManagerService; +import org.exoplatform.container.component.RequestLifeCycle; +import org.exoplatform.jpa.CommonsDAOJPAImplTest; +import org.exoplatform.settings.jpa.dao.SettingContextDAO; +import org.exoplatform.settings.jpa.entity.ContextEntity; + +import io.meeds.spring.module.dao.TestJpaDao; +import io.meeds.spring.module.entity.TestJpaEntity; + +import jakarta.persistence.EntityManager; + +@SpringJUnitConfig(CommonsDAOJPAImplTest.class) +public class ContainerTransactionalRepositoryTest extends CommonsDAOJPAImplTest { // NOSONAR + + @Autowired + private TestJpaDao testJpaDao; + + @Test + public void repositoryWritesRollBackWithTheAtomicMethod() throws Throwable { + long entitiesCount = testJpaDao.count(); + IllegalStateException error = new IllegalStateException("Failure after the repository write"); + ProceedingJoinPoint point = mock(ProceedingJoinPoint.class); + when(point.proceed()).thenAnswer(invocation -> { + testJpaDao.save(new TestJpaEntity(null, "rolled back")); + throw error; + }); + + assertSame(error, assertThrows(IllegalStateException.class, () -> proceedAtomically(point))); + + assertEquals(entitiesCount, testJpaDao.count()); + } + + @Test + public void repositoryWritesCommitWithTheAtomicMethod() throws Throwable { + long entitiesCount = testJpaDao.count(); + ProceedingJoinPoint point = mock(ProceedingJoinPoint.class); + when(point.proceed()).thenAnswer(invocation -> testJpaDao.save(new TestJpaEntity(null, "committed"))); + + proceedAtomically(point); + + assertEquals(entitiesCount + 1, testJpaDao.count()); + } + + @Test + public void kernelWritesAfterARepositoryWriteCommitWithTheAtomicMethod() throws Throwable { + SettingContextDAO settingContextDAO = getService(SettingContextDAO.class); + long entitiesCount = testJpaDao.count(); + long contextsCount = countContexts(settingContextDAO); + ProceedingJoinPoint point = mock(ProceedingJoinPoint.class); + when(point.proceed()).thenAnswer(invocation -> { + EntityManager entityManager = getService(EntityManagerService.class).getEntityManager(); + TestJpaEntity entity = testJpaDao.save(new TestJpaEntity(null, "committed with a kernel write")); + assertTrue(entityManager.isOpen()); + assertTrue(entityManager.getTransaction().isActive()); + ContextEntity context = settingContextDAO.create(new ContextEntity().setType("atomic").setName("committed")); + return List.of(entity, context); + }); + + List written = (List) proceedAtomically(point); + + assertEquals(entitiesCount + 1, testJpaDao.count()); + assertEquals(contextsCount + 1, countContexts(settingContextDAO)); + testJpaDao.delete((TestJpaEntity) written.get(0)); + RequestLifeCycle.begin(getContainer()); + try { + settingContextDAO.delete((ContextEntity) written.get(1)); + } finally { + RequestLifeCycle.end(); + } + } + + @Test + public void kernelWritesAfterARepositoryWriteRollBackWithTheAtomicMethod() throws Throwable { + SettingContextDAO settingContextDAO = getService(SettingContextDAO.class); + long entitiesCount = testJpaDao.count(); + long contextsCount = countContexts(settingContextDAO); + IllegalStateException error = new IllegalStateException("Failure after the kernel write"); + ProceedingJoinPoint point = mock(ProceedingJoinPoint.class); + when(point.proceed()).thenAnswer(invocation -> { + testJpaDao.save(new TestJpaEntity(null, "rolled back with a kernel write")); + settingContextDAO.create(new ContextEntity().setType("atomic").setName("rolled back")); + throw error; + }); + + assertSame(error, assertThrows(IllegalStateException.class, () -> proceedAtomically(point))); + + assertEquals(entitiesCount, testJpaDao.count()); + assertEquals(contextsCount, countContexts(settingContextDAO)); + } + + @Test + public void repositoryWritesOfATransactionCallbackCommitInATransactionOfTheirOwn() throws Throwable { + long entitiesCount = testJpaDao.count(); + ProceedingJoinPoint point = mock(ProceedingJoinPoint.class); + when(point.proceed()).thenAnswer(invocation -> TransactionCallbacks.afterCompletion(getService(EntityManagerService.class), + () -> testJpaDao.save(new TestJpaEntity(null, + "saved by a callback")))); + + assertEquals(Boolean.TRUE, proceedAtomically(point)); + + assertEquals(entitiesCount + 1, testJpaDao.count()); + } + + private long countContexts(SettingContextDAO settingContextDAO) { + RequestLifeCycle.begin(getContainer()); + try { + return settingContextDAO.count(); + } finally { + RequestLifeCycle.end(); + } + } + + private Object proceedAtomically(ProceedingJoinPoint point) throws Throwable { + RequestLifeCycle.begin(getContainer()); + try { + return new ContainerTransactionalAspect().proceedAtomically(point, getService(EntityManagerService.class)); + } finally { + RequestLifeCycle.end(); + } + } + +} diff --git a/component/common/src/test/java/io/meeds/common/TransactionCallbacksTest.java b/component/common/src/test/java/io/meeds/common/TransactionCallbacksTest.java new file mode 100644 index 0000000000..463dc241f1 --- /dev/null +++ b/component/common/src/test/java/io/meeds/common/TransactionCallbacksTest.java @@ -0,0 +1,129 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package io.meeds.common; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.hibernate.Transaction; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import org.exoplatform.commons.persistence.impl.EntityManagerService; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityTransaction; +import jakarta.transaction.Status; +import jakarta.transaction.Synchronization; + +@ExtendWith(MockitoExtension.class) +class TransactionCallbacksTest { + + @Mock + private EntityManagerService entityManagerService; + + @Mock + private EntityManager entityManager; + + @Mock + private Transaction transaction; + + private AtomicInteger callbackRuns = new AtomicInteger(); + + private Runnable callback = callbackRuns::incrementAndGet; + + @BeforeEach + void setUp() { + lenient().when(entityManagerService.getEntityManager()).thenReturn(entityManager); + lenient().when(entityManager.getTransaction()).thenReturn(transaction); + } + + @Test + void testRunTheCallbackOnceTheActiveTransactionCompletes() { + when(transaction.isActive()).thenReturn(true); + + assertTrue(TransactionCallbacks.afterCompletion(entityManagerService, callback)); + assertEquals(0, callbackRuns.get(), "The callback runs once the transaction completes"); + + ArgumentCaptor synchronization = ArgumentCaptor.forClass(Synchronization.class); + verify(transaction).registerSynchronization(synchronization.capture()); + synchronization.getValue().beforeCompletion(); + assertEquals(0, callbackRuns.get()); + synchronization.getValue().afterCompletion(Status.STATUS_COMMITTED); + assertEquals(1, callbackRuns.get()); + synchronization.getValue().afterCompletion(Status.STATUS_ROLLEDBACK); + assertEquals(2, callbackRuns.get()); + } + + @Test + void testLogTheCallbackErrors() { + when(transaction.isActive()).thenReturn(true); + + assertTrue(TransactionCallbacks.afterCompletion(entityManagerService, () -> { + throw new IllegalStateException("eviction failed"); + })); + + ArgumentCaptor synchronization = ArgumentCaptor.forClass(Synchronization.class); + verify(transaction).registerSynchronization(synchronization.capture()); + synchronization.getValue().afterCompletion(Status.STATUS_COMMITTED); + } + + @Test + void testNoCallbackWithoutActiveTransaction() { + when(transaction.isActive()).thenReturn(false); + + assertFalse(TransactionCallbacks.afterCompletion(entityManagerService, callback)); + + verify(transaction, never()).registerSynchronization(any()); + assertEquals(0, callbackRuns.get()); + } + + @Test + void testNoCallbackWithoutEntityManager() { + when(entityManagerService.getEntityManager()).thenReturn(null); + + assertFalse(TransactionCallbacks.afterCompletion(entityManagerService, callback)); + assertFalse(TransactionCallbacks.afterCompletion(null, callback)); + assertEquals(0, callbackRuns.get()); + } + + @Test + void testNoCallbackWithAnotherTransactionImplementation() { + EntityTransaction jpaTransaction = mock(EntityTransaction.class); + lenient().when(jpaTransaction.isActive()).thenReturn(true); + when(entityManager.getTransaction()).thenReturn(jpaTransaction); + + assertFalse(TransactionCallbacks.afterCompletion(entityManagerService, callback)); + assertEquals(0, callbackRuns.get()); + } + +} diff --git a/component/common/src/test/java/org/exoplatform/commons/cache/future/FutureCacheTest.java b/component/common/src/test/java/org/exoplatform/commons/cache/future/FutureCacheTest.java new file mode 100644 index 0000000000..5320ae9cf5 --- /dev/null +++ b/component/common/src/test/java/org/exoplatform/commons/cache/future/FutureCacheTest.java @@ -0,0 +1,57 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.exoplatform.commons.cache.future; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.Test; + +class FutureCacheTest { + + @Test + void testClearFuturesInvalidatesTheLoadsInFlight() throws Exception { + CountDownLatch loading = new CountDownLatch(1); + CountDownLatch cleared = new CountDownLatch(1); + FutureMap futureCache = new FutureMap<>((context, key) -> { + loading.countDown(); + cleared.await(10, TimeUnit.SECONDS); + return "previous value"; + }); + AtomicReference loadedValue = new AtomicReference<>(); + Thread reader = new Thread(() -> loadedValue.set(futureCache.get(null, "key"))); + reader.start(); + try { + assertTrue(loading.await(10, TimeUnit.SECONDS)); + futureCache.clearFutures(); + } finally { + cleared.countDown(); + reader.join(10000); + } + + assertEquals("previous value", loadedValue.get(), "The load in flight returns its value to its reader"); + assertNull(futureCache.get("key"), "The load in flight caches its value after the clear"); + } + +} diff --git a/component/common/src/test/java/org/exoplatform/commons/persistence/impl/ExoTransactionalAspectTest.java b/component/common/src/test/java/org/exoplatform/commons/persistence/impl/ExoTransactionalAspectTest.java new file mode 100644 index 0000000000..dcce129d2e --- /dev/null +++ b/component/common/src/test/java/org/exoplatform/commons/persistence/impl/ExoTransactionalAspectTest.java @@ -0,0 +1,139 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.exoplatform.commons.persistence.impl; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.aspectj.lang.ProceedingJoinPoint; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityTransaction; +import jakarta.persistence.RollbackException; + +@ExtendWith(MockitoExtension.class) +class ExoTransactionalAspectTest { + + private static final Object RESULT = new Object(); + + @Mock + private ProceedingJoinPoint point; + + @Mock + private EntityManagerService entityManagerService; + + @Mock + private EntityManager entityManager; + + @Mock + private EntityTransaction transaction; + + private ExoTransactionalAspect aspect = new ExoTransactionalAspect(); + + @BeforeEach + void setUp() { + lenient().when(entityManagerService.getEntityManager()).thenReturn(entityManager); + lenient().when(entityManager.getTransaction()).thenReturn(transaction); + } + + @Test + void testCommitWhenTheMethodReturns() throws Throwable { + when(transaction.isActive()).thenReturn(false, true); + when(point.proceed()).thenReturn(RESULT); + + assertSame(RESULT, aspect.proceed(point, entityManagerService)); + + verify(transaction).begin(); + verify(transaction).commit(); + verify(transaction, never()).rollback(); + } + + @Test + void testCommitWhenTheMethodThrows() throws Throwable { + IllegalStateException error = new IllegalStateException("failure"); + when(transaction.isActive()).thenReturn(false, true); + when(point.proceed()).thenThrow(error); + + assertSame(error, assertThrows(IllegalStateException.class, () -> aspect.proceed(point, entityManagerService))); + + verify(transaction).commit(); + verify(transaction, never()).rollback(); + } + + @Test + void testRollbackAndThrowWhenTheMethodReturnsAndTheTransactionIsMarkedRollbackOnly() throws Throwable { + when(transaction.isActive()).thenReturn(false, true); + when(transaction.getRollbackOnly()).thenReturn(true); + when(point.proceed()).thenReturn(RESULT); + + assertThrows(RollbackException.class, () -> aspect.proceed(point, entityManagerService)); + + verify(transaction).rollback(); + verify(transaction, never()).commit(); + } + + @Test + void testRollbackAndKeepTheErrorWhenTheMethodThrowsAndTheTransactionIsMarkedRollbackOnly() throws Throwable { + IllegalStateException error = new IllegalStateException("failure"); + when(transaction.isActive()).thenReturn(false, true); + when(transaction.getRollbackOnly()).thenReturn(true); + when(point.proceed()).thenThrow(error); + + assertSame(error, assertThrows(IllegalStateException.class, () -> aspect.proceed(point, entityManagerService))); + + verify(transaction).rollback(); + verify(transaction, never()).commit(); + } + + @Test + void testLeaveAJoinedTransactionToTheMethodThatBeganIt() throws Throwable { + when(transaction.isActive()).thenReturn(true); + lenient().when(transaction.getRollbackOnly()).thenReturn(true); + when(point.proceed()).thenReturn(RESULT); + + assertSame(RESULT, aspect.proceed(point, entityManagerService)); + + verify(transaction, never()).begin(); + verify(transaction, never()).commit(); + verify(transaction, never()).rollback(); + } + + @Test + void testCloseTheEntityManagerItStartedWhenTheTransactionIsRolledBack() throws Throwable { + when(entityManagerService.getEntityManager()).thenReturn(null); + when(entityManagerService.createEntityManager()).thenReturn(entityManager); + when(transaction.isActive()).thenReturn(false, true); + when(transaction.getRollbackOnly()).thenReturn(true); + when(point.proceed()).thenReturn(RESULT); + + assertThrows(RollbackException.class, () -> aspect.proceed(point, entityManagerService)); + + verify(entityManagerService).closeEntityManager(); + } + +} diff --git a/component/portal/src/main/java/org/exoplatform/portal/config/NewPortalConfigListener.java b/component/portal/src/main/java/org/exoplatform/portal/config/NewPortalConfigListener.java index 05355e4a40..867de2bc06 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/config/NewPortalConfigListener.java +++ b/component/portal/src/main/java/org/exoplatform/portal/config/NewPortalConfigListener.java @@ -27,6 +27,7 @@ import java.util.Locale; import java.util.Objects; import java.util.Set; +import java.util.function.Consumer; import java.util.regex.Pattern; import org.apache.commons.collections4.CollectionUtils; @@ -53,9 +54,11 @@ import org.exoplatform.portal.mop.SiteType; import org.exoplatform.portal.mop.importer.ImportMode; import org.exoplatform.portal.mop.importer.NavigationImporter; +import org.exoplatform.portal.mop.importer.NavigationOrphanRemover; import org.exoplatform.portal.mop.importer.PageImporter; import org.exoplatform.portal.mop.importer.PortalConfigImporter; import org.exoplatform.portal.mop.importer.Status; +import org.exoplatform.portal.mop.page.PageContext; import org.exoplatform.portal.mop.service.LayoutService; import org.exoplatform.portal.mop.service.NavigationService; import org.exoplatform.portal.mop.storage.DescriptionStorage; @@ -227,17 +230,64 @@ public void run() throws Exception { for (NewPortalConfig ele : configs) { initPortalConfigDB(ele, prepareImport); } + // A site that one of its configurations overwrites is imported from all + // its configurations, as an overwrite restore of the site does, so that a + // configuration doesn't remove the pages and nodes of the others + Set overwrittenSites = getOverwrittenSites(prepareImport); for (NewPortalConfig ele : configs) { - initPageDB(ele, prepareImport); + initPageDB(withoutSites(ele, overwrittenSites), prepareImport); + } + for (SiteKey site : overwrittenSites) { + importSitePages(getPortalConfigs(site.getTypeName(), site.getName()), site.getName(), ImportMode.OVERWRITE); } for (NewPortalConfig ele : configs) { - initPageNavigationDB(ele, prepareImport); + initPageNavigationDB(withoutSites(ele, overwrittenSites), prepareImport); + } + for (SiteKey site : overwrittenSites) { + importSiteNavigation(getPortalConfigs(site.getTypeName(), site.getName()), site.getName(), ImportMode.OVERWRITE); } // touchImport(); } + private Set getOverwrittenSites(boolean prepareImport) { + Set sites = new HashSet<>(); + for (NewPortalConfig config : configs) { + try { + if ((config.getOverrideMode() || prepareImport) && getRightMode(config.getImportMode()) == ImportMode.OVERWRITE) { + for (String owner : config.getPredefinedOwner()) { + if (createdOwners.contains(owner)) { + sites.add(new SiteKey(config.getOwnerType(), owner)); + } + } + } + } catch (RuntimeException e) { + // The import of the configuration logs its unknown import mode or site + // type + LOG.debug("The configuration of sites {} is imported on its own: its import mode or site type is unknown", + config.getPredefinedOwner(), + e); + } + } + return sites; + } + + /** + * @return the configuration without the owners of the given sites + */ + private NewPortalConfig withoutSites(NewPortalConfig config, Set sites) { + HashSet owners = new HashSet<>(config.getPredefinedOwner()); + if (owners.removeIf(owner -> sites.stream() + .anyMatch(site -> site.getTypeName().equalsIgnoreCase(config.getOwnerType()) + && site.getName().equals(owner)))) { + NewPortalConfig otherSitesConfig = config.clone(); + otherSitesConfig.setPredefinedOwner(owners); + return otherSitesConfig; + } + return config; + } + public NewPortalConfig getPortalConfig(String type, String name) { return CollectionUtils.isNotEmpty(configs) ? configs.stream() .filter(c -> StringUtils.equalsIgnoreCase(c.getOwnerType(), type) @@ -307,10 +357,21 @@ public void mergePlugin(NewPortalConfigListener other) { } public void initPortalConfigDB(NewPortalConfig ele, boolean prepareImport) { + initPortalConfigDB(ele, prepareImport, null); + } + + /** + * @param ele the sites configuration of an extension + * @param prepareImport true to import the sites even when the configuration + * doesn't override them + * @param beforeImport applied to each site configuration before it's + * imported, null to import it as is + */ + public void initPortalConfigDB(NewPortalConfig ele, boolean prepareImport, Consumer beforeImport) { RequestLifeCycle.begin(PortalContainer.getInstance()); try { if (ele.getOverrideMode() || prepareImport) { - initPortalConfigDB(ele); + initPortalConfigDB(ele, beforeImport); } } catch (Exception e) { log.error("NewPortalConfig error: " + e.getMessage(), e); @@ -320,8 +381,12 @@ public void initPortalConfigDB(NewPortalConfig ele, boolean prepareImport) { } public void initPortalConfigDB(NewPortalConfig config) { + initPortalConfigDB(config, (Consumer) null); + } + + private void initPortalConfigDB(NewPortalConfig config, Consumer beforeImport) { for (String owner : config.getPredefinedOwner()) { - if (createPortalConfig(config, owner)) { + if (createPortalConfig(config, owner, beforeImport)) { LOG.info("Importing Site of type '{}' with name '{}' from location '{}' wth mode '{}'", config.getOwnerType(), owner, @@ -372,7 +437,162 @@ public void initPageNavigationDB(NewPortalConfig config) { } } + /** + * Imports the site layout of each configuration of a site with the given + * import mode. + * + * @param configs the configurations of the site + * @param owner the site name + * @param importMode the import mode + * @param beforeImport applied to each site configuration before it's + * imported, null to import it as is + */ + public void importSiteLayout(List configs, + String owner, + ImportMode importMode, + Consumer beforeImport) { + for (NewPortalConfig config : configs) { + initPortalConfigDB(getSiteConfig(config, owner, importMode), true, beforeImport); + } + } + + /** + * Imports the pages of each configuration of a site with the given import + * mode. An overwrite removes the pages that no configuration defines first, + * then the pages of each configuration are imported in place, so that a + * configuration doesn't remove the pages of the others, and the pages keep + * the navigation nodes of other sites that reference them. The pages are + * imported whether or not the site was imported at startup. + * + * @param configs the configurations of the site + * @param owner the site name + * @param importMode the import mode + */ + public void importSitePages(List configs, String owner, ImportMode importMode) { + boolean overwrite = importMode == ImportMode.OVERWRITE; + if (overwrite) { + removePageOrphans(configs, owner); + } + for (NewPortalConfig config : configs) { + try { + createPage(getSiteConfig(config, owner, overwrite ? ImportMode.MERGE : importMode), owner); + } catch (Exception e) { + log.error("NewPortalConfig error: " + e.getMessage(), e); + } + } + } + + /** + * Imports the navigation of each configuration of a site with the given + * import mode. The navigation of each configuration is imported without + * removing any node, then the nodes that no configuration defines are + * removed as the import mode does, so that a configuration doesn't remove + * the nodes of the others. + * + * @param configs the configurations of the site + * @param owner the site name + * @param importMode the import mode + */ + public void importSiteNavigation(List configs, String owner, ImportMode importMode) { + boolean removeOrphans = NavigationOrphanRemover.removesOrphans(importMode); + for (NewPortalConfig config : configs) { + initPageNavigationDB(getSiteConfig(config, owner, removeOrphans ? ImportMode.MERGE : importMode), true); + } + if (removeOrphans) { + removeNavigationOrphans(configs, owner, importMode); + } + } + + private NewPortalConfig getSiteConfig(NewPortalConfig config, String owner, ImportMode importMode) { + NewPortalConfig siteConfig = config.clone(); + siteConfig.setImportMode(importMode.name()); + siteConfig.setOverrideMode(true); + siteConfig.setPredefinedOwner(new HashSet<>(Set.of(owner))); + return siteConfig; + } + + /** + * Removes the navigation nodes of a site that none of its configurations + * defines, as the import mode removes orphan nodes. Nothing is removed when + * the navigation file of a configuration can't be parsed, since the nodes it + * defines can't be told apart from the orphan ones then. A configuration + * whose navigation file can't be opened defines no node. + * + * @param configs the configurations of the site + * @param owner the site name + * @param importMode the import mode requested for the site navigation + */ + private void removeNavigationOrphans(List configs, String owner, ImportMode importMode) { + RequestLifeCycle.begin(PortalContainer.getInstance()); + try { + List navigations = new ArrayList<>(); + for (NewPortalConfig config : configs) { + UnmarshalledObject navigation = getConfig(config, owner, "navigation", PageNavigation.class); + if (navigation != null) { + navigations.add(navigation.getObject()); + } + } + new NavigationOrphanRemover(navigationService, navigations, importMode).perform(); + } catch (Exception e) { + log.error("Error while removing the orphan navigation nodes of site {}, no node is removed", owner, e); + } finally { + RequestLifeCycle.end(); + } + } + + /** + * Removes the pages of a site that none of its configurations defines. + * Nothing is removed when the pages file of a configuration can't be parsed, + * since the pages it defines can't be told apart from the orphan ones then, + * nor from a user site, whose pages an import doesn't remove. A + * configuration whose pages file can't be opened defines no page. The + * orphans are selected once, then removed one by one, each page row read + * again at its removal: a page removed since the selection is skipped, and a + * page created since then is kept, unless it is named after an orphan. + * + * @param configs the configurations of the site + * @param owner the site name + */ + private void removePageOrphans(List configs, String owner) { + if (configs.isEmpty()) { + return; + } + SiteKey siteKey = new SiteKey(configs.get(0).getOwnerType(), owner); + if (siteKey.getType().equals(SiteType.USER)) { + return; + } + RequestLifeCycle.begin(PortalContainer.getInstance()); + try { + Set pageNames = new HashSet<>(); + for (NewPortalConfig config : configs) { + UnmarshalledObject pageSet = getConfig(config, owner, "pages", PageSet.class); + if (pageSet != null) { + pageSet.getObject().getPages().forEach(page -> pageNames.add(page.getName())); + } + } + for (PageContext page : layoutService.findPages(siteKey)) { + if (!pageNames.contains(page.getKey().getName())) { + try { + layoutService.remove(page.getKey()); + } catch (RuntimeException e) { + // A page removed since the orphans were selected, for instance, + // doesn't stop the removal of the others + log.warn("Error while removing the orphan page {}", page.getKey(), e); + } + } + } + } catch (Exception e) { + log.error("Error while removing the orphan pages of site {}", owner, e); + } finally { + RequestLifeCycle.end(); + } + } + public boolean createPortalConfig(NewPortalConfig config, String owner) { + return createPortalConfig(config, owner, null); + } + + private boolean createPortalConfig(NewPortalConfig config, String owner, Consumer beforeImport) { String type = config.getOwnerType(); String fixedOwnerName = fixOwnerName(type, owner); @@ -416,6 +636,9 @@ public boolean createPortalConfig(NewPortalConfig config, String owner) { // If XML configuration of PortalConfig exists or PortalConfig doesn't exist // in storage, import PortalConfig switch default ImportMode + if (beforeImport != null) { + beforeImport.accept(pConfig); + } ImportMode importMode = getRightMode(config.getImportMode()); PortalConfigImporter portalImporter = new PortalConfigImporter(importMode, pConfig, layoutService); try { diff --git a/component/portal/src/main/java/org/exoplatform/portal/config/UserPortalConfigService.java b/component/portal/src/main/java/org/exoplatform/portal/config/UserPortalConfigService.java index 6a8764c27b..0ba158f553 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/config/UserPortalConfigService.java +++ b/component/portal/src/main/java/org/exoplatform/portal/config/UserPortalConfigService.java @@ -21,7 +21,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; @@ -173,37 +172,21 @@ public boolean restoreSite(String type, if (!canRestore(type, name)) { return false; } + PortalConfig previousSite = restoreSiteLayout ? layoutService.getPortalConfig(type, name) : null; List configs = newPortalConfigListener.getPortalConfigs(type, name); - for(NewPortalConfig config : configs) { - config = config.clone(); - config.setImportMode(importMode.name()); - config.setOverrideMode(true); - HashSet ownerName = new HashSet<>(); - ownerName.add(name); - config.setPredefinedOwner(ownerName); - if (restoreSiteLayout) { - PortalConfig previousSite = layoutService.getPortalConfig(type, name); - newPortalConfigListener.initPortalConfigDB(config, true); - PortalConfig site = layoutService.getPortalConfig(type, name); - if (previousSite != null) { - // Preserve previous version or properties - site.setLabel(previousSite.getLabel()); - site.setDescription(previousSite.getDescription()); - site.setAccessPermissions(previousSite.getAccessPermissions()); - site.setEditPermission(previousSite.getEditPermission()); - site.setDisplayed(previousSite.isDisplayed()); - site.setDisplayOrder(previousSite.getDisplayOrder()); - site.setIcon(previousSite.getIcon()); - site.setProperties(previousSite.getProperties()); - layoutService.save(site); - } - } - if (restorePages) { - newPortalConfigListener.initPageDB(config, true); - } - if (restoreNavigationTree) { - newPortalConfigListener.initPageNavigationDB(config, true); - } + if (restoreSiteLayout) { + // The imported site carries the settings of the site it replaces, so + // that it is saved once, without being read again + newPortalConfigListener.importSiteLayout(configs, + name, + importMode, + importedSite -> keepSiteSettings(importedSite, previousSite)); + } + if (restorePages) { + newPortalConfigListener.importSitePages(configs, name, importMode); + } + if (restoreNavigationTree) { + newPortalConfigListener.importSiteNavigation(configs, name, importMode); } return true; } @@ -591,6 +574,10 @@ public UserNode getSiteNodeOrGlobalNode(String portalType, // NOSONAR } } + NewPortalConfigListener getNewPortalConfigListener() { + return newPortalConfigListener; + } + @Synchronized public void initListener(ComponentPlugin listener) { if (listener instanceof NewPortalConfigListener portalConfigListener) { @@ -713,6 +700,19 @@ public void setMetaPortalConfig(PortalConfig metaPortalConfig) { this.metaPortalConfig = metaPortalConfig; } + private void keepSiteSettings(PortalConfig importedSite, PortalConfig previousSite) { + if (previousSite != null) { + importedSite.setLabel(previousSite.getLabel()); + importedSite.setDescription(previousSite.getDescription()); + importedSite.setAccessPermissions(previousSite.getAccessPermissions()); + importedSite.setEditPermission(previousSite.getEditPermission()); + importedSite.setDisplayed(previousSite.isDisplayed()); + importedSite.setDisplayOrder(previousSite.getDisplayOrder()); + importedSite.setIcon(previousSite.getIcon()); + importedSite.setProperties(previousSite.getProperties()); + } + } + private PageContext getPage(PageKey pageRef, Identity userAclIdentity) { if (pageRef == null) { return null; diff --git a/component/portal/src/main/java/org/exoplatform/portal/jdbc/entity/PageEntity.java b/component/portal/src/main/java/org/exoplatform/portal/jdbc/entity/PageEntity.java index b24e83d335..426f962c6a 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/jdbc/entity/PageEntity.java +++ b/component/portal/src/main/java/org/exoplatform/portal/jdbc/entity/PageEntity.java @@ -22,6 +22,7 @@ import java.util.LinkedList; import java.util.List; +import org.hibernate.annotations.DynamicUpdate; import org.json.simple.JSONArray; import org.json.simple.JSONObject; @@ -44,8 +45,13 @@ import lombok.Getter; import lombok.Setter; +/** + * A page row has two writers, its properties save and its layout save: each + * writes only the columns it changed, so neither writes back the other's. + */ @Entity(name = "GateInPage") @Table(name = "PORTAL_PAGES") +@DynamicUpdate @NamedQuery(name = "PageEntity.deleteByOwner", query = "DELETE GateInPage p WHERE p.owner.id = :ownerId") @NamedQuery(name = "PageEntity.findByKey", query = "SELECT p FROM GateInPage p WHERE p.name = :name AND p.owner.siteType = :ownerType AND p.owner.name = :ownerId") public class PageEntity extends ComponentEntity implements Serializable { diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAO.java b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAO.java index 1942c402c6..f19879f9cf 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAO.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAO.java @@ -28,6 +28,16 @@ public interface PageDAO extends GenericDAO { PageEntity findByKey(PageKey pageKey); + /** + * Reads the page row again from the database, bypassing the state already + * loaded in the current persistence context + * + * @param entity a page entity loaded by this DAO, detached from the + * persistence context by this call + * @return the entity read again, null when the page was removed meanwhile + */ + PageEntity refresh(PageEntity entity); + ListAccess findByQuery(SiteType siteType, String siteName, String name, int offset, int limit); void deleteByOwner(long id); diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAOImpl.java b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAOImpl.java index ccef7ca6eb..55bf993a9c 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAOImpl.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/PageDAOImpl.java @@ -67,6 +67,17 @@ public PageEntity findByKey(PageKey pageKey) { } } + @Override + public PageEntity refresh(PageEntity entity) { + EntityManager entityManager = getEntityManager(); + if (entityManager.contains(entity)) { + // The page row is then read from the database instead of the + // persistence context + entityManager.detach(entity); + } + return find(entity.getId()); + } + @Override @ExoTransactional public void deleteByOwner(long id) { diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAO.java b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAO.java index cd1f124f30..dfc88f2824 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAO.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAO.java @@ -30,6 +30,16 @@ public interface SiteDAO extends GenericDAO { SiteEntity findByKey(SiteKey siteKey); + /** + * Reads the site row again from the database, bypassing the state already + * loaded in the current persistence context + * + * @param entity a site entity loaded by this DAO, detached from the + * persistence context by this call + * @return the entity read again, null when the site was removed meanwhile + */ + SiteEntity refresh(SiteEntity entity); + List findByType(SiteType siteType); List findSiteKey(SiteType siteType); diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAOImpl.java b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAOImpl.java index c8f4fa3656..00fded6e11 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAOImpl.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/dao/SiteDAOImpl.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; +import jakarta.persistence.EntityManager; import jakarta.persistence.NoResultException; import jakarta.persistence.TypedQuery; @@ -98,6 +99,17 @@ public SiteEntity findByKey(SiteKey siteKey) { } } + @Override + public SiteEntity refresh(SiteEntity entity) { + EntityManager entityManager = getEntityManager(); + if (entityManager.contains(entity)) { + // The site row is then read from the database instead of the + // persistence context + entityManager.detach(entity); + } + return find(entity.getId()); + } + @Override public List findByType(SiteType siteType) { TypedQuery query = getEntityManager().createNamedQuery("SiteEntity.findByType", SiteEntity.class); diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/importer/DefinedNavigationNode.java b/component/portal/src/main/java/org/exoplatform/portal/mop/importer/DefinedNavigationNode.java new file mode 100644 index 0000000000..44e5b10a5b --- /dev/null +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/importer/DefinedNavigationNode.java @@ -0,0 +1,86 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.exoplatform.portal.mop.importer; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.exoplatform.portal.config.model.PageNode; + +/** + * A navigation node that the configurations of a site define, attach a + * navigation fragment to, or name in the path of a fragment attachment point, + * with the child nodes the configurations define under it. + */ +class DefinedNavigationNode { + + private final Map children = new HashMap<>(); + + /** + * True when the configurations define the node children: the node is + * defined by a configuration, or is the attachment point of a fragment. The + * children of a node that is only in the path of an attachment point are + * not defined by the configurations. + */ + private boolean childrenDefined; + + /** + * True when the node is the attachment point of a fragment, or a top-level + * node of a fragment: a child node that no configuration defines under it is + * a top-level section, not a node nested in a default node. + */ + private boolean topLevelParent; + + DefinedNavigationNode getChild(String name) { + return children.get(name); + } + + DefinedNavigationNode getOrAddPathNode(String name) { + return children.computeIfAbsent(name, key -> new DefinedNavigationNode()); + } + + void setAttachmentPoint() { + childrenDefined = true; + topLevelParent = true; + } + + void addTopLevelNode(PageNode node) { + addNode(node).topLevelParent = true; + } + + boolean isChildrenDefined() { + return childrenDefined; + } + + boolean isTopLevelParent() { + return topLevelParent; + } + + private DefinedNavigationNode addNode(PageNode node) { + DefinedNavigationNode definedNode = getOrAddPathNode(node.getName()); + definedNode.childrenDefined = true; + List nodes = node.getNodes(); + if (nodes != null) { + nodes.forEach(definedNode::addNode); + } + return definedNode; + } + +} diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/importer/NavigationOrphanRemover.java b/component/portal/src/main/java/org/exoplatform/portal/mop/importer/NavigationOrphanRemover.java new file mode 100644 index 0000000000..d4160649e2 --- /dev/null +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/importer/NavigationOrphanRemover.java @@ -0,0 +1,132 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.exoplatform.portal.mop.importer; + +import java.util.List; + +import org.exoplatform.portal.config.model.NavigationFragment; +import org.exoplatform.portal.config.model.PageNavigation; +import org.exoplatform.portal.mop.SiteKey; +import org.exoplatform.portal.mop.navigation.NavigationContext; +import org.exoplatform.portal.mop.navigation.NodeContext; +import org.exoplatform.portal.mop.navigation.NodeModel; +import org.exoplatform.portal.mop.navigation.Scope; +import org.exoplatform.portal.mop.service.NavigationService; + +/** + * Removes the nodes of a site navigation that none of the navigation + * configurations of the site defines, as the import mode removes orphan nodes: + * every node with {@link ImportMode#OVERWRITE}, the nodes nested in a default + * node with {@link ImportMode#RESTORE_DEFAULTS}. The configurations are + * imported first without removing any node, so that a configuration doesn't + * remove the nodes another configuration of the site defines. As the import + * of a fragment, the removal keeps the child nodes of a node that is only in + * the path of a fragment attachment point. + */ +public class NavigationOrphanRemover { + + private final NavigationService navigationService; + + private final List navigations; + + private final ImportMode importMode; + + /** + * @param navigationService the navigation service + * @param navigations the navigation configurations of one site + * @param importMode the import mode requested for the site + */ + public NavigationOrphanRemover(NavigationService navigationService, List navigations, ImportMode importMode) { + this.navigationService = navigationService; + this.navigations = navigations; + this.importMode = importMode; + } + + /** + * @param importMode an import mode + * @return true when the import mode removes the orphan nodes of a navigation + */ + public static boolean removesOrphans(ImportMode importMode) { + return importMode.config.destroyOrphan || importMode.config.destroyNestedOrphan; + } + + public void perform() { + if (navigations.isEmpty() || !removesOrphans(importMode)) { + return; + } + PageNavigation siteNavigation = navigations.get(0); + NavigationContext navigation = navigationService.loadNavigation(new SiteKey(siteNavigation.getOwnerType(), + siteNavigation.getOwnerId())); + if (navigation == null) { + return; + } + NodeContext rootNode = navigationService.loadNode(NodeModel.SELF_MODEL, navigation, Scope.ALL, null); + if (removeOrphans(rootNode, getDefinedRootNode())) { + navigationService.saveNode(rootNode, null); + } + } + + private DefinedNavigationNode getDefinedRootNode() { + DefinedNavigationNode rootNode = new DefinedNavigationNode(); + for (PageNavigation navigation : navigations) { + if (navigation.getFragments() != null) { + for (NavigationFragment fragment : navigation.getFragments()) { + DefinedNavigationNode attachmentNode = getAttachmentNode(rootNode, fragment.getParentURI()); + attachmentNode.setAttachmentPoint(); + if (fragment.getNodes() != null) { + fragment.getNodes().forEach(attachmentNode::addTopLevelNode); + } + } + } + } + return rootNode; + } + + private DefinedNavigationNode getAttachmentNode(DefinedNavigationNode rootNode, String parentURI) { + DefinedNavigationNode attachmentNode = rootNode; + if (parentURI != null) { + for (String name : parentURI.split("/")) { + if (!name.isEmpty()) { + attachmentNode = attachmentNode.getOrAddPathNode(name); + } + } + } + return attachmentNode; + } + + private boolean removeOrphans(NodeContext node, DefinedNavigationNode definedNode) { + boolean removed = false; + NodeContext child = node.getFirst(); + while (child != null) { + NodeContext nextChild = child.getNext(); + DefinedNavigationNode definedChild = definedNode.getChild(child.getName()); + if (definedChild != null) { + removed |= removeOrphans(child, definedChild); + } else if (definedNode.isChildrenDefined() + && (importMode.config.destroyOrphan + || (importMode.config.destroyNestedOrphan && !definedNode.isTopLevelParent()))) { + child.removeNode(); + removed = true; + } + child = nextChild; + } + return removed; + } + +} diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/PageStorageImpl.java b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/PageStorageImpl.java index 91ed6bbe43..92cb772e38 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/PageStorageImpl.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/PageStorageImpl.java @@ -53,6 +53,8 @@ import org.exoplatform.services.log.ExoLogger; import org.exoplatform.services.log.Log; +import io.meeds.common.ContainerTransactional; + public class PageStorageImpl implements PageStorage { private static final Log LOG = ExoLogger.getLogger(PageStorageImpl.class); @@ -141,6 +143,12 @@ public boolean savePage(PageContext page) { } PageEntity entity = pageDAO.findByKey(page.getKey()); + if (entity != null) { + // The update writes the columns that differ from the loaded state: they're + // compared with the row as stored, not as an earlier read of this request + // left it + entity = pageDAO.refresh(entity); + } boolean created = false; if (entity == null) { entity = new PageEntity(); @@ -173,19 +181,11 @@ public boolean destroyPage(PageKey key) { if (key == null) { throw new IllegalArgumentException("PageKey is mandatory"); } - - PageEntity page = pageDAO.findByKey(key); - if (page != null) { - String pageBody = page.getPageBody(); - JSONArray children = parseJsonArray(pageBody); - layoutStorage.deleteChildren(children); - layoutStorage.deletePermissions(PageEntity.class.getName(), page.getId()); - pageDAO.delete(page); - + boolean destroyed = deletePage(key); + if (destroyed) { broadcastEvent(EventType.PAGE_DESTROYED, key); - return true; } - return false; + return destroyed; } @Override @@ -205,6 +205,9 @@ public PageContext clone(PageKey srcPageKey, PageKey dstPageKey) { } PageEntity pageSrc = pageDAO.findByKey(srcPageKey); + if (pageSrc != null) { + pageSrc = pageDAO.refresh(pageSrc); + } if (pageSrc == null) { throw new PageServiceException(PageError.CLONE_NO_SRC_PAGE, String.format("Could not clone non existing page %s from site of type %s with id %s", @@ -262,23 +265,7 @@ public QueryResult findPages(int from, @Override public void save(PageData page) { - SiteKey siteKey = new SiteKey(page.getKey().getType(), page.getKey().getId()); - org.exoplatform.portal.mop.page.PageKey mopKey = - new org.exoplatform.portal.mop.page.PageKey(siteKey, page.getKey().getName()); - - PageEntity dst = pageDAO.findByKey(mopKey); - if (dst == null) { - throw new NoSuchDataException("The page " + page.getKey() + " not found"); - } - List children = page.getChildren(); - - JSONArray pageBodyJson = parseJsonArray(dst.getPageBody()); - List newPageBody = layoutStorage.saveChildren(pageBodyJson, children); - dst.setChildren(newPageBody); - dst.setPageBody(((JSONArray) dst.toJSON().get("children")).toJSONString()); - - pageDAO.update(dst); - broadcastEvent(EventType.PAGE_UPDATED, mopKey); + broadcastEvent(EventType.PAGE_UPDATED, savePageLayout(page)); } @Override @@ -328,6 +315,60 @@ protected PageData buildPageData(PageEntity entity) { entity.getLink()); } + /** + * Removes the page layout, permissions and row in one transaction, so that a + * page whose removal fails keeps its layout. The layout removed is the one of + * the page row as stored, not as the current persistence context may hold it + * since an earlier read of the same request. That row is read without a lock: + * a layout another request commits on the page between that read and the + * deletion isn't removed. + */ + @ContainerTransactional(atomic = true) + private boolean deletePage(PageKey key) { + PageEntity page = pageDAO.findByKey(key); + if (page != null) { + page = pageDAO.refresh(page); + } + if (page == null) { + return false; + } + layoutStorage.deleteChildren(parseJsonArray(page.getPageBody())); + layoutStorage.deletePermissions(PageEntity.class.getName(), page.getId()); + pageDAO.delete(page); + return true; + } + + /** + * Replaces the page layout in one transaction, so that a concurrent reader + * never reads a page whose removed windows are deleted while its new layout + * isn't committed yet, and a page whose update fails keeps its layout. The + * layout replaced is the one of the page row as stored, not as the current + * persistence context may hold it since an earlier read of the same request. + */ + @ContainerTransactional(atomic = true) + private org.exoplatform.portal.mop.page.PageKey savePageLayout(PageData page) { + SiteKey siteKey = new SiteKey(page.getKey().getType(), page.getKey().getId()); + org.exoplatform.portal.mop.page.PageKey mopKey = + new org.exoplatform.portal.mop.page.PageKey(siteKey, page.getKey().getName()); + + PageEntity dst = pageDAO.findByKey(mopKey); + if (dst != null) { + dst = pageDAO.refresh(dst); + } + if (dst == null) { + throw new NoSuchDataException("The page " + page.getKey() + " not found"); + } + List children = page.getChildren(); + + JSONArray pageBodyJson = parseJsonArray(dst.getPageBody()); + List newPageBody = layoutStorage.saveChildren(pageBodyJson, children); + dst.setChildren(newPageBody); + dst.setPageBody(((JSONArray) dst.toJSON().get("children")).toJSONString()); + + pageDAO.update(dst); + return mopKey; + } + protected void broadcastEvent(String eventName, org.exoplatform.portal.mop.page.PageKey pageKey) { try { listenerService.broadcast(eventName, this, pageKey); diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/SiteStorageImpl.java b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/SiteStorageImpl.java index 60fbf22b3d..e2a935b811 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/SiteStorageImpl.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/SiteStorageImpl.java @@ -72,6 +72,8 @@ import org.exoplatform.upload.UploadResource; import org.exoplatform.upload.UploadService; +import io.meeds.common.ContainerTransactional; + public class SiteStorageImpl implements SiteStorage { private static final String DEFAULT_SHAREDLAYOUT_PATH = "war:/conf/portal/portal/sharedlayout.xml"; // NOSONAR @@ -126,9 +128,10 @@ public void create(PortalConfig config) { } @Override + @ContainerTransactional(atomic = true) public void create(PortalData config) { SiteEntity entity = new SiteEntity(); - buildSiteEntity(entity, config); + buildSiteEntity(entity, config, false); entity = siteDAO.create(entity); savePermissions(entity.getId(), config); } @@ -142,14 +145,28 @@ public void save(PortalConfig config) { save(config.build()); } + /** + * Replaces the site layout and its attributes in one transaction, so that a + * concurrent reader never reads a site whose layout containers are deleted + * while its new ones aren't committed yet. The layout replaced is the one of + * the site row as stored, not as the current persistence context may hold it + * since an earlier read of the same request. + */ @Override + @ContainerTransactional(atomic = true) public void save(PortalData config) { SiteKey siteKey = new SiteKey(config.getKey().getType(), config.getKey().getId()); SiteEntity entity = siteDAO.findByKey(siteKey); + if (entity != null) { + entity = siteDAO.refresh(entity); + } if (entity == null) { throw new IllegalStateException("Cannot update portal " + config.getName() + " that does not exist"); } - buildSiteEntity(entity, config); + if (config.isLayoutUnresolved()) { + LOG.warn("The layout of site {} wasn't resolved when the site was read, the stored layout is kept", siteKey); + } + buildSiteEntity(entity, config, config.isLayoutUnresolved()); entity = siteDAO.update(entity); savePermissions(entity.getId(), config); } @@ -165,11 +182,25 @@ public void remove(PortalData config) { remove(siteKey); } + /** + * Removes the site layout, pages, navigation and row in one transaction, so + * that a concurrent reader never reads a site whose layout containers are + * already deleted. The layout removed is the one of the site row as stored, + * not as the current persistence context may hold it since an earlier read + * of the same request. + */ @Override + @ContainerTransactional(atomic = true) public void remove(SiteKey siteKey) { PortalData config = getPortalConfig(siteKey); if (config != null && config.isRemovable()) { SiteEntity entity = siteDAO.findByKey(siteKey); + if (entity != null) { + entity = siteDAO.refresh(entity); + } + if (entity == null) { + throw new NoSuchDataException("Could not remove non existing portal " + siteKey); + } String siteBody = entity.getSiteBody(); JSONArray children = parseJsonArray(siteBody); layoutStorage.deleteChildren(children); @@ -199,18 +230,12 @@ public PortalData getPortalConfig(PortalKey key) { @Override public PortalData getPortalConfig(SiteKey siteKey) { - SiteEntity entity = siteDAO.findByKey(siteKey); - if (entity != null) { - return buildPortalData(entity); - } else { - return null; - } + return buildResolvedPortalData(siteDAO.findByKey(siteKey)); } @Override public PortalData getPortalConfig(long siteId) { - SiteEntity entity = siteDAO.find(siteId); - return buildPortalData(entity); + return buildResolvedPortalData(siteDAO.find(siteId)); } @Override @@ -284,7 +309,7 @@ public List getSitesKeys(SiteFilter siteFilter) { } - private void buildSiteEntity(SiteEntity entity, PortalData config) { + private void buildSiteEntity(SiteEntity entity, PortalData config, boolean keepStoredLayout) { entity.setDescription(config.getDescription()); entity.setLabel(config.getLabel()); entity.setLocale(config.getLocale()); @@ -299,18 +324,38 @@ private void buildSiteEntity(SiteEntity entity, PortalData config) { } entity.setProperties(propertiesString); - List children = new ArrayList<>(); - children.add(config.getPortalLayout()); + if (!keepStoredLayout) { + List children = new ArrayList<>(); + children.add(config.getPortalLayout()); - JSONArray siteBody = parseJsonArray(entity.getSiteBody()); - List newSiteBody = layoutStorage.saveChildren(siteBody, children); - entity.setChildren(newSiteBody); - entity.setSiteBody(((JSONArray) entity.toJSON().get("children")).toJSONString()); + JSONArray siteBody = parseJsonArray(entity.getSiteBody()); + List newSiteBody = layoutStorage.saveChildren(siteBody, children); + entity.setChildren(newSiteBody); + entity.setSiteBody(((JSONArray) entity.toJSON().get("children")).toJSONString()); + } entity.setDisplayed(config.isDisplayed()); entity.setDisplayOrder(config.getDisplayOrder()); entity.setBannerFileId(config.getBannerFileId()); } + /** + * A site row read while a concurrent save replaces its layout can reference + * root containers that this save already deleted: the row is then read again + * to get the committed layout. + */ + private PortalData buildResolvedPortalData(SiteEntity entity) { + PortalData portalData = buildPortalData(entity); + if (portalData != null && portalData.isLayoutUnresolved()) { + portalData = buildPortalData(siteDAO.refresh(entity)); + if (portalData != null && portalData.isLayoutUnresolved()) { + LOG.warn("The root container of the layout of site {}:{} doesn't exist, the site is read with an empty layout", + portalData.getType(), + portalData.getName()); + } + } + return portalData; + } + @SuppressWarnings("unchecked") private PortalData buildPortalData(SiteEntity entity) { if (entity == null) { @@ -341,21 +386,23 @@ private PortalData buildPortalData(SiteEntity entity) { }); } - return new PortalData("site_" + entity.getId(), - entity.getName(), - entity.getSiteType().getName(), - entity.getLocale(), - entity.getLabel(), - entity.getDescription(), - access, - editPermission, - Collections.unmodifiableMap(properties), - entity.getSkin(), - rootContainer, - entity.isDefaultSiteBody(), - entity.isDisplayed(), - entity.getDisplayOrder(), - entity.getBannerFileId()); + PortalData portalData = new PortalData("site_" + entity.getId(), + entity.getName(), + entity.getSiteType().getName(), + entity.getLocale(), + entity.getLabel(), + entity.getDescription(), + access, + editPermission, + Collections.unmodifiableMap(properties), + entity.getSkin(), + rootContainer, + entity.isDefaultSiteBody(), + entity.isDisplayed(), + entity.getDisplayOrder(), + entity.getBannerFileId()); + portalData.setLayoutUnresolved(rootContainer == null && siteBody != null && !siteBody.isEmpty()); + return portalData; } private void savePermissions(Long id, PortalData config) { diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheDescriptionStorage.java b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheDescriptionStorage.java index f0fd3676d4..c5915e42d1 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheDescriptionStorage.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheDescriptionStorage.java @@ -41,6 +41,8 @@ import io.meeds.portal.mop.storage.cache.model.DescriptionLocaleListCache; +import io.meeds.common.TransactionCallbacks; + public class CacheDescriptionStorage extends DescriptionStorageImpl { private static final Log LOG = @@ -91,7 +93,7 @@ public void setDescription(String id, Locale locale, State description) { try { super.setDescription(id, locale, description); } finally { - clearCacheEntries(id); + removeFromCache(id); } } @@ -100,7 +102,7 @@ public void setDescription(String id, State description) { try { super.setDescription(id, description); } finally { - clearCacheEntries(id); + removeFromCache(id); } } @@ -109,7 +111,7 @@ public void setDescriptions(String id, Map descriptions) { try { super.setDescriptions(id, descriptions); } finally { - clearCacheEntries(id); + removeFromCache(id); } } @@ -147,4 +149,20 @@ private void clearCacheEntries(String id) { } } + /** + * Inside a transaction, the cache entries are cleared again once the + * transaction completes: a concurrent read made before the commit caches the + * previous descriptions again. The loads in flight are invalidated first so + * that they don't cache them either, unless one is already writing its + * result: the cache expiry is then the backstop. + */ + private void removeFromCache(String id) { + Runnable eviction = () -> { + descriptionFutureCache.clearFutures(); + descriptionLocaleListFutureCache.clearFutures(); + clearCacheEntries(id); + }; + eviction.run(); + TransactionCallbacks.afterCompletion(eviction); + } } diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheLayoutStorage.java b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheLayoutStorage.java index 3e4a591877..c05c2346c1 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheLayoutStorage.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheLayoutStorage.java @@ -30,6 +30,8 @@ import org.exoplatform.services.cache.CacheService; import org.exoplatform.services.cache.ExoCache; +import io.meeds.common.TransactionCallbacks; + public class CacheLayoutStorage extends LayoutStorage { public static final String PORTLET_PREFERENCES_CACHE_NAME = "portal.PortletPreferences"; @@ -74,7 +76,7 @@ public WindowData retrieve(Object context, Long siteKey) throws Exception { protected WindowEntity createWindow(WindowEntity dstW) { WindowEntity window = super.createWindow(dstW); if (window != null && window.getId() != null) { - portletPreferencesCache.remove(window.getId()); + removeFromCache(window.getId()); } return window; } @@ -84,7 +86,7 @@ protected WindowEntity updateWindow(WindowEntity window) { try { return super.updateWindow(window); } finally { - portletPreferencesCache.remove(window.getId()); + removeFromCache(window.getId()); } } @@ -99,7 +101,7 @@ protected void deleteWindow(WindowEntity window) { try { super.deleteWindow(window); } finally { - portletPreferencesCache.remove(window.getId()); + removeFromCache(window.getId()); } } @@ -108,7 +110,7 @@ protected void deleteWindowById(Long id) { try { super.deleteWindowById(id); } finally { - portletPreferencesCache.remove(id); + removeFromCache(id); } } @@ -130,4 +132,20 @@ private WindowEntity toEntity(WindowData window) { windowEntity.setCustomization(window.getCustomization()); return windowEntity; } + + /** + * Inside a transaction, the window is removed from the cache again once the + * transaction completes: a concurrent read made before the commit caches the + * previous state of the window again. The load in flight is invalidated + * first so that it doesn't cache it either, unless it's already writing its + * result: the cache expiry is then the backstop. + */ + private void removeFromCache(Long windowId) { + Runnable eviction = () -> { + portletPreferencesFutureCache.removeFuture(windowId); + portletPreferencesCache.remove(windowId); + }; + eviction.run(); + TransactionCallbacks.afterCompletion(eviction); + } } diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheNavigationStorage.java b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheNavigationStorage.java index 37180e2879..740599dad8 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheNavigationStorage.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheNavigationStorage.java @@ -38,6 +38,8 @@ import org.exoplatform.services.log.ExoLogger; import org.exoplatform.services.log.Log; +import io.meeds.common.TransactionCallbacks; + public class CacheNavigationStorage extends NavigationStorageImpl { private static final Log LOG = @@ -85,7 +87,7 @@ public NodeData[] createNode(Long parentId, Long previousId, String name, NodeSt try { return super.createNode(parentId, previousId, name, state, index); } finally { - this.nodeFutureCache.remove(parentId); + evict(() -> this.nodeFutureCache.remove(parentId)); } } @@ -95,7 +97,7 @@ public NodeData destroyNode(Long targetId) { try { return super.destroyNode(targetId); } finally { - clearNodeCache(nodeData); + evict(() -> clearNodeCache(nodeData)); } } @@ -112,9 +114,11 @@ public NodeData[] moveNode(Long targetId, Long fromId, Long toId, Long previousI try { return super.moveNode(targetId, fromId, toId, previousId); } finally { - clearNodeCache(nodeData); - clearNodeCache(fromNodeData); - clearNodeCache(toNodeData); + evict(() -> { + clearNodeCache(nodeData); + clearNodeCache(fromNodeData); + clearNodeCache(toNodeData); + }); } } @@ -125,8 +129,10 @@ public NodeData[] renameNode(Long targetId, Long parentId, String name) { try { return super.renameNode(targetId, parentId, name); } finally { - clearNodeCache(nodeData); - clearNodeCache(parentNodeData); + evict(() -> { + clearNodeCache(nodeData); + clearNodeCache(parentNodeData); + }); } } @@ -136,7 +142,7 @@ public NodeData updateNode(Long targetId, NodeState state) { try { return super.updateNode(targetId, state); } finally { - clearNodeCache(nodeData); + evict(() -> clearNodeCache(nodeData)); } } @@ -151,8 +157,10 @@ public void saveNavigation(SiteKey key, NavigationState state) { try { super.saveNavigation(key, state); } finally { - clearNavigationByKey(key); - clearNodeCache(key); + evict(() -> { + clearNavigationByKey(key); + clearNodeCache(key); + }); } } @@ -161,8 +169,10 @@ public boolean destroyNavigation(SiteKey siteKey) { try { return super.destroyNavigation(siteKey); } finally { - clearNavigationByKey(siteKey); - clearNodeCache(siteKey); + evict(() -> { + clearNavigationByKey(siteKey); + clearNodeCache(siteKey); + }); } } @@ -171,14 +181,16 @@ public boolean destroyNavigation(NavigationData data) { try { return super.destroyNavigation(data); } finally { - if (data != null && data.getSiteKey() != null) { - SiteKey siteKey = data.getSiteKey(); - clearNavigationByKey(siteKey); - clearNodeCache(siteKey); - if (data.getRootId() != null) { - clearNodeCache(Long.parseLong(data.getRootId())); + evict(() -> { + if (data != null && data.getSiteKey() != null) { + SiteKey siteKey = data.getSiteKey(); + clearNavigationByKey(siteKey); + clearNodeCache(siteKey); + if (data.getRootId() != null) { + clearNodeCache(Long.parseLong(data.getRootId())); + } } - } + }); } } @@ -226,4 +238,20 @@ public void clearNodeCache(NodeData nodeData) { } } + /** + * Inside a transaction, the eviction is run again once the transaction + * completes: a concurrent read made before the commit caches the previous + * navigation state again. The loads in flight are invalidated first so that + * they don't cache it either, unless one is already writing its result: the + * cache expiry is then the backstop. + */ + private void evict(Runnable eviction) { + Runnable invalidatingEviction = () -> { + navigationFutureCache.clearFutures(); + nodeFutureCache.clearFutures(); + eviction.run(); + }; + invalidatingEviction.run(); + TransactionCallbacks.afterCompletion(invalidatingEviction); + } } diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CachePageStorage.java b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CachePageStorage.java index 50046cd45f..da162f7f19 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CachePageStorage.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CachePageStorage.java @@ -31,6 +31,8 @@ import org.exoplatform.services.cache.ExoCache; import org.exoplatform.services.listener.ListenerService; +import io.meeds.common.TransactionCallbacks; + public class CachePageStorage extends PageStorageImpl { public static final String PAGE_CACHE_NAME = "portal.PageService"; @@ -62,8 +64,8 @@ public PageContext clone(PageKey srcPageKey, PageKey dstPageKey) { try { return super.clone(srcPageKey, dstPageKey); } finally { - pageFutureCache.remove(srcPageKey); - pageFutureCache.remove(dstPageKey); + removeFromCache(srcPageKey); + removeFromCache(dstPageKey); } } @@ -72,7 +74,7 @@ public boolean savePage(PageContext page) { try { return super.savePage(page); } finally { - pageFutureCache.remove(page.getKey()); + removeFromCache(page.getKey()); } } @@ -81,7 +83,7 @@ public void save(PageData page) { try { super.save(page); } finally { - pageFutureCache.remove(page.getKey().toMopPageKey()); + removeFromCache(page.getKey().toMopPageKey()); } } @@ -90,7 +92,7 @@ public boolean destroyPage(PageKey key) { try { return super.destroyPage(key); } finally { - pageFutureCache.remove(key); + removeFromCache(key); } } @@ -110,4 +112,19 @@ protected PageKey getPageKey(long id) { return pageKey; } + /** + * Inside a transaction, the page is removed from the cache again once the + * transaction completes: a concurrent read made before the commit caches the + * previous state of the page again. The load in flight is invalidated first + * so that it doesn't cache it either, unless it's already writing its + * result: the cache expiry is then the backstop. + */ + private void removeFromCache(PageKey pageKey) { + Runnable eviction = () -> { + pageFutureCache.removeFuture(pageKey); + pageFutureCache.remove(pageKey); + }; + eviction.run(); + TransactionCallbacks.afterCompletion(eviction); + } } diff --git a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheSiteStorage.java b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheSiteStorage.java index bcf747e6cb..6bf6b42399 100644 --- a/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheSiteStorage.java +++ b/component/portal/src/main/java/org/exoplatform/portal/mop/storage/cache/CacheSiteStorage.java @@ -38,6 +38,8 @@ import java.util.List; +import io.meeds.common.TransactionCallbacks; + public class CacheSiteStorage extends SiteStorageImpl { public static final String SITE_CACHE_NAME = "portal.SiteService"; @@ -105,8 +107,7 @@ public void create(PortalData config) { try { super.create(config); } finally { - siteFutureCache.remove(getSiteKey(config.getKey())); - clearSiteKeysByFilterFutureCache(); + removeFromCache(getSiteKey(config.getKey())); } } @@ -115,18 +116,16 @@ public void save(PortalData config) { try { super.save(config); } finally { - siteFutureCache.remove(getSiteKey(config.getKey())); - clearSiteKeysByFilterFutureCache(); + removeFromCache(getSiteKey(config.getKey())); } } @Override - public void remove(PortalData config) { + public void remove(SiteKey siteKey) { try { - super.remove(config); + super.remove(siteKey); } finally { - siteFutureCache.remove(getSiteKey(config.getKey())); - clearSiteKeysByFilterFutureCache(); + removeFromCache(siteKey); } } @@ -152,6 +151,24 @@ private SiteKey getSiteKey(PortalKey portalKey) { return new SiteKey(portalKey.getType(), portalKey.getId()); } + /** + * Inside a transaction, the site is removed from the cache again once the + * transaction completes: a concurrent read made before the commit caches the + * previous state of the site again. The loads in flight are invalidated + * first so that they don't cache it either, unless one is already writing + * its result: the cache expiry is then the backstop. + */ + private void removeFromCache(SiteKey siteKey) { + Runnable eviction = () -> { + siteFutureCache.removeFuture(siteKey); + siteFutureCache.remove(siteKey); + siteKeysByFilterFutureCache.clearFutures(); + clearSiteKeysByFilterFutureCache(); + }; + eviction.run(); + TransactionCallbacks.afterCompletion(eviction); + } + private void clearSiteKeysByFilterFutureCache() { this.siteKeysByFilterFutureCache.clear(); } diff --git a/component/portal/src/test/java/org/exoplatform/portal/config/NewPortalConfigListenerTest.java b/component/portal/src/test/java/org/exoplatform/portal/config/NewPortalConfigListenerTest.java index 58cb86bae9..ff9ebf0ff8 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/config/NewPortalConfigListenerTest.java +++ b/component/portal/src/test/java/org/exoplatform/portal/config/NewPortalConfigListenerTest.java @@ -23,7 +23,9 @@ import org.exoplatform.container.xml.ValueParam; import org.exoplatform.portal.config.model.PortalConfig; import org.exoplatform.portal.mop.SiteKey; +import org.exoplatform.portal.mop.importer.ImportMode; import org.exoplatform.portal.mop.page.PageContext; +import org.exoplatform.portal.mop.page.PageKey; import org.exoplatform.portal.mop.service.LayoutService; import org.exoplatform.portal.mop.service.NavigationService; import org.exoplatform.portal.mop.storage.DescriptionStorage; @@ -112,4 +114,34 @@ public void testInitPageDB() { verify(layoutService, times(0)).removePages(any()); } + + @Test + public void testOverwriteRemovesTheOtherOrphanPagesWhenOneCantBeRemoved() { + ValueParam valueParam = new ValueParam(); + valueParam.setName("meta.portal"); + valueParam.setValue("classic"); + when(initParams.getValueParam("meta.portal")).thenReturn(valueParam); + NewPortalConfigListener newPortalConfigListener = new NewPortalConfigListener(owner, + layoutService, + configurationManager, + initParams, + navigationService, + descriptionStorage, + userACL, + localeConfigService); + NewPortalConfig newPortalConfig = mock(NewPortalConfig.class); + when(newPortalConfig.getOwnerType()).thenReturn(PortalConfig.PORTAL_TYPE); + when(newPortalConfig.clone()).thenReturn(newPortalConfig); + SiteKey siteKey = SiteKey.portal("global"); + PageKey removedPageKey = new PageKey(siteKey, "removed"); + PageKey orphanPageKey = new PageKey(siteKey, "orphan"); + when(layoutService.findPages(siteKey)).thenReturn(List.of(new PageContext(removedPageKey, null), + new PageContext(orphanPageKey, null))); + // The page was removed since the orphans were selected + doThrow(NullPointerException.class).when(layoutService).remove(removedPageKey); + + newPortalConfigListener.importSitePages(List.of(newPortalConfig), "global", ImportMode.OVERWRITE); + + verify(layoutService).remove(orphanPageKey); + } } diff --git a/component/portal/src/test/java/org/exoplatform/portal/config/TestImportNavigationOverwrite.java b/component/portal/src/test/java/org/exoplatform/portal/config/TestImportNavigationOverwrite.java index ba71462a5e..d44e4ea0f6 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/config/TestImportNavigationOverwrite.java +++ b/component/portal/src/test/java/org/exoplatform/portal/config/TestImportNavigationOverwrite.java @@ -30,9 +30,17 @@ protected ImportMode getMode() { @Override protected final void afterOnePhaseBoot(NodeContext root) { - assertState(root); - assertEquals(2, root.get("baz").getIndex()); + // A configuration that overwrites the site keeps the nodes of the other + // configurations of the site + assertEquals(4, root.getNodeCount()); + NodeContext foo = root.get("foo"); + assertEquals("foo_icon_2", foo.getState().getIcon()); + assertEquals(1, foo.getNodeCount()); + assertEquals("bar_icon", root.get("bar").getState().getIcon()); + assertEquals("daa_icon", root.get("daa").getState().getIcon()); assertEquals(0, root.get("foo").getIndex()); + assertEquals(2, root.get("baz").getIndex()); + assertEquals(3, root.get("daa").getIndex()); } @Override diff --git a/component/portal/src/test/java/org/exoplatform/portal/config/TestRestoreSiteConfigurations.java b/component/portal/src/test/java/org/exoplatform/portal/config/TestRestoreSiteConfigurations.java new file mode 100644 index 0000000000..ad353b1fa0 --- /dev/null +++ b/component/portal/src/test/java/org/exoplatform/portal/config/TestRestoreSiteConfigurations.java @@ -0,0 +1,406 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.exoplatform.portal.config; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; + +import org.exoplatform.component.test.ConfigurationUnit; +import org.exoplatform.component.test.ConfiguredBy; +import org.exoplatform.component.test.ContainerScope; +import org.exoplatform.container.ExoContainerContext; +import org.exoplatform.container.PortalContainer; +import org.exoplatform.container.component.RequestLifeCycle; +import org.exoplatform.portal.config.model.Container; +import org.exoplatform.portal.config.model.Page; +import org.exoplatform.portal.config.model.PortalConfig; +import org.exoplatform.portal.mop.SiteKey; +import org.exoplatform.portal.mop.dao.ContainerDAO; +import org.exoplatform.portal.mop.dao.PageDAO; +import org.exoplatform.portal.mop.importer.ImportMode; +import org.exoplatform.portal.mop.navigation.NavigationContext; +import org.exoplatform.portal.mop.navigation.NodeContext; +import org.exoplatform.portal.mop.navigation.NodeModel; +import org.exoplatform.portal.mop.navigation.Scope; +import org.exoplatform.portal.mop.page.PageContext; +import org.exoplatform.portal.mop.page.PageKey; +import org.exoplatform.portal.mop.service.LayoutService; +import org.exoplatform.portal.mop.service.NavigationService; +import org.exoplatform.portal.mop.storage.PageStorage; +import org.exoplatform.services.listener.Event; +import org.exoplatform.services.listener.Listener; +import org.exoplatform.services.listener.ListenerService; + +/** + * Restores the site "contributed", defined by three configurations: the first + * one holds pages and no navigation, the two others hold pages and the nodes + * home > section > node2 and home > section > node3, the second one also holds + * home > section > node23, which references a page of the third one, and the + * third one attaches home > attached3 > leaf3 to the home node. The site "partial" is defined by a + * configuration and by another one whose pages and navigation can't be parsed. + * The site "attached" is defined by a configuration that attaches section6 > + * node6 to the node section6, which no configuration defines. The site + * "startup" is defined by a configuration imported in insert mode and by + * another one that overwrites the site at each startup. + */ +@ConfiguredBy({ + @ConfigurationUnit(scope = ContainerScope.PORTAL, path = "conf/portal/configuration.xml"), + @ConfigurationUnit(scope = ContainerScope.PORTAL, path = "conf/exo.portal.component.portal-configuration-local.xml"), + @ConfigurationUnit(scope = ContainerScope.PORTAL, path = "org/exoplatform/portal/config/conf/configuration.xml"), + @ConfigurationUnit(scope = ContainerScope.PORTAL, path = "org/exoplatform/portal/config/restore/configuration.xml") }) +public class TestRestoreSiteConfigurations extends AbstractConfigTest { + + private static final String SITE_NAME = "contributed"; + + private static final SiteKey SITE_KEY = SiteKey.portal(SITE_NAME); + + private static final String PARTIAL_SITE_NAME = "partial"; + + private static final String ATTACHED_SITE_NAME = "attached"; + + private static final String STARTUP_SITE_NAME = "startup"; + + private static final String CUSTOM_NAME = "custom"; + + private static final String HOME = "home"; + + private static final String SECTION = "section"; + + private static final String ATTACHED = "attached3"; + + private UserPortalConfigService userPortalConfigService; + + private NavigationService navigationService; + + private PageStorage pageStorage; + + private ContainerDAO containerDAO; + + private PageDAO pageDAO; + + private ListenerService listenerService; + + @Override + protected void setUp() throws Exception { + super.setUp(); + PortalContainer container = getContainer(); + userPortalConfigService = container.getComponentInstanceOfType(UserPortalConfigService.class); + navigationService = container.getComponentInstanceOfType(NavigationService.class); + pageStorage = container.getComponentInstanceOfType(PageStorage.class); + containerDAO = container.getComponentInstanceOfType(ContainerDAO.class); + pageDAO = container.getComponentInstanceOfType(PageDAO.class); + listenerService = container.getComponentInstanceOfType(ListenerService.class); + } + + public void testOverwriteRestoresThePagesAndNavigationOfEveryConfiguration() { + addCustomPageAndNodes(); + pageStorage.destroyPage(new PageKey(SITE_KEY, "page3")); + SiteKey attachedSiteKey = SiteKey.portal(ATTACHED_SITE_NAME); + addCustomNode(getRootNode(attachedSiteKey), new PageKey(SITE_KEY, "page1")); + try { + assertTrue(userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, SITE_NAME, ImportMode.OVERWRITE, true, true, true)); + restartTransaction(); + + assertEquals(Set.of("page1", "page2", "page3"), getPageNames(SITE_KEY)); + NodeContext rootNode = getRootNode(SITE_KEY); + assertEquals(Set.of(HOME), getNodeNames(rootNode)); + assertEquals(Set.of(SECTION, ATTACHED), getNodeNames(rootNode.get(HOME))); + NodeContext sectionNode = rootNode.get(HOME).get(SECTION); + assertEquals(Set.of("node2", "node23", "node3"), getNodeNames(sectionNode)); + assertEquals(new PageKey(SITE_KEY, "page2"), sectionNode.get("node2").getState().getPageRef()); + assertEquals("A node references the page of a later configuration, imported before any navigation", + new PageKey(SITE_KEY, "page3"), + sectionNode.get("node23").getState().getPageRef()); + assertEquals("A node references the removed default page that the restore creates again", + new PageKey(SITE_KEY, "page3"), + sectionNode.get("node3").getState().getPageRef()); + NodeContext attachedNode = rootNode.get(HOME).get(ATTACHED); + assertEquals(Set.of("leaf3"), getNodeNames(attachedNode)); + assertEquals(new PageKey(SITE_KEY, "page3"), attachedNode.get("leaf3").getState().getPageRef()); + assertEquals("A node of another site keeps the restored page it references", + new PageKey(SITE_KEY, "page1"), + getRootNode(attachedSiteKey).get(CUSTOM_NAME).getState().getPageRef()); + } finally { + removeCustomNode(attachedSiteKey); + } + } + + public void testRestorePagesImportsThePagesOfASiteThatTheStartupDidntImport() { + PageKey pageKey = new PageKey(SITE_KEY, "page3"); + pageStorage.destroyPage(pageKey); + restartTransaction(); + // A startup that imports no configuration of the site, as a restart does + // when none of them overrides it + Set createdOwners = userPortalConfigService.getNewPortalConfigListener().createdOwners; + Set startupOwners = new HashSet<>(createdOwners); + createdOwners.clear(); + try { + assertTrue(userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, + SITE_NAME, + ImportMode.RESTORE_DEFAULTS, + false, + true, + true)); + restartTransaction(); + + assertNotNull(pageStorage.getPage(pageKey)); + } finally { + createdOwners.addAll(startupOwners); + } + } + + public void testRestoreDefaultsKeepsTheDefaultNodesOfEveryConfiguration() { + addCustomPageAndNodes(); + try { + assertTrue(userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, + SITE_NAME, + ImportMode.RESTORE_DEFAULTS, + true, + true, + true)); + restartTransaction(); + + assertEquals("A restore of the default pages keeps the custom pages", + Set.of("page1", "page2", "page3", CUSTOM_NAME), + getPageNames(SITE_KEY)); + NodeContext rootNode = getRootNode(SITE_KEY); + assertEquals("A custom node of a fragment attachment point is kept", + Set.of(HOME, CUSTOM_NAME), + getNodeNames(rootNode)); + assertEquals("A custom node of a top-level node of a fragment is kept", + Set.of(SECTION, ATTACHED, CUSTOM_NAME), + getNodeNames(rootNode.get(HOME))); + assertEquals("The nodes of each configuration are kept and the custom nested node is removed", + Set.of("node2", "node23", "node3"), + getNodeNames(rootNode.get(HOME).get(SECTION))); + assertEquals("A custom node of a top-level node of a fragment attached to another node is kept", + Set.of("leaf3", CUSTOM_NAME), + getNodeNames(rootNode.get(HOME).get(ATTACHED))); + } finally { + userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, SITE_NAME, ImportMode.OVERWRITE, false, true, true); + restartTransaction(); + } + } + + public void testOverwriteKeepsTheNodesWhenANavigationConfigurationCantBeRead() { + SiteKey partialSiteKey = SiteKey.portal(PARTIAL_SITE_NAME); + addCustomNode(getRootNode(partialSiteKey), null); + try { + assertTrue(userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, + PARTIAL_SITE_NAME, + ImportMode.OVERWRITE, + false, + false, + true)); + restartTransaction(); + + assertEquals(Set.of("node4", CUSTOM_NAME), getNodeNames(getRootNode(partialSiteKey))); + } finally { + removeCustomNode(partialSiteKey); + } + } + + public void testOverwriteKeepsThePagesWhenAPagesConfigurationCantBeRead() { + PageKey customPageKey = new PageKey(SiteKey.portal(PARTIAL_SITE_NAME), CUSTOM_NAME); + pageStorage.savePage(new PageContext(customPageKey, null)); + restartTransaction(); + try { + assertTrue(userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, + PARTIAL_SITE_NAME, + ImportMode.OVERWRITE, + false, + true, + false)); + restartTransaction(); + + assertEquals(Set.of("page4", CUSTOM_NAME), getPageNames(customPageKey.getSite())); + } finally { + pageStorage.destroyPage(customPageKey); + restartTransaction(); + } + } + + public void testOverwriteRemovesTheLayoutSavedOnAnOrphanPageSinceTheRequestReadIt() throws Exception { + PageKey firstOrphanKey = new PageKey(SITE_KEY, "orphan1"); + PageKey secondOrphanKey = new PageKey(SITE_KEY, "orphan2"); + pageStorage.savePage(new PageContext(firstOrphanKey, null)); + pageStorage.savePage(new PageContext(secondOrphanKey, null)); + restartTransaction(); + // The request reads the orphan pages before the restore, as a node load + // reads the pages its nodes reference + pageDAO.findByKey(firstOrphanKey); + pageDAO.findByKey(secondOrphanKey); + AtomicBoolean saveOnRemoval = new AtomicBoolean(true); + AtomicReference savedContainerId = new AtomicReference<>(); + listenerService.addListener(LayoutService.PAGE_REMOVED, new Listener() { + @Override + public void onEvent(Event event) throws Exception { + PageKey removedKey = event.getData().getPageKey(); + if ((removedKey.equals(firstOrphanKey) || removedKey.equals(secondOrphanKey)) + && saveOnRemoval.compareAndSet(true, false)) { + // The other orphan is saved by another request between two removals + savedContainerId.set(saveContainerInAnotherRequest(removedKey.equals(firstOrphanKey) ? secondOrphanKey : + firstOrphanKey)); + } + } + }); + try { + assertTrue(userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, SITE_NAME, ImportMode.OVERWRITE, false, true, false)); + restartTransaction(); + + assertNotNull(savedContainerId.get()); + assertNull(pageStorage.loadPage(firstOrphanKey)); + assertNull(pageStorage.loadPage(secondOrphanKey)); + assertNull("The container saved on an orphan page since the request read it is removed with the page", + containerDAO.find(Long.parseLong(savedContainerId.get()))); + } finally { + saveOnRemoval.set(false); + pageStorage.destroyPage(firstOrphanKey); + pageStorage.destroyPage(secondOrphanKey); + restartTransaction(); + } + } + + public void testStartupOverwriteImportsTheSiteFromEveryConfiguration() { + SiteKey startupSiteKey = SiteKey.portal(STARTUP_SITE_NAME); + assertEquals("An overwrite at startup keeps the pages of the other configurations", + Set.of("pageA", "pageB"), + getPageNames(startupSiteKey)); + NodeContext startupRootNode = getRootNode(startupSiteKey); + assertEquals("An overwrite at startup keeps the nodes of the other configurations", + Set.of("nodeA", "nodeB"), + getNodeNames(startupRootNode)); + assertEquals(new PageKey(startupSiteKey, "pageA"), startupRootNode.get("nodeA").getState().getPageRef()); + assertEquals(new PageKey(startupSiteKey, "pageB"), startupRootNode.get("nodeB").getState().getPageRef()); + + PageKey customPageKey = new PageKey(startupSiteKey, CUSTOM_NAME); + pageStorage.savePage(new PageContext(customPageKey, null)); + addCustomNode(getRootNode(startupSiteKey), customPageKey); + SiteKey attachedSiteKey = SiteKey.portal(ATTACHED_SITE_NAME); + addCustomNode(getRootNode(attachedSiteKey), new PageKey(startupSiteKey, "pageB")); + try { + userPortalConfigService.start(); + restartTransaction(); + + assertEquals("An overwrite at startup removes the pages that no configuration defines only", + Set.of("pageA", "pageB"), + getPageNames(startupSiteKey)); + assertEquals("An overwrite at startup removes the nodes that no configuration defines only", + Set.of("nodeA", "nodeB"), + getNodeNames(getRootNode(startupSiteKey))); + assertEquals("A node of another site keeps the page that an overwrite at startup imports", + new PageKey(startupSiteKey, "pageB"), + getRootNode(attachedSiteKey).get(CUSTOM_NAME).getState().getPageRef()); + } finally { + removeCustomNode(attachedSiteKey); + } + } + + public void testOverwriteKeepsTheNodesOutsideTheFragmentAttachmentPoints() { + SiteKey attachedSiteKey = SiteKey.portal(ATTACHED_SITE_NAME); + addCustomNode(getRootNode(attachedSiteKey), null); + addCustomNode(getRootNode(attachedSiteKey).get("section6"), null); + try { + assertTrue(userPortalConfigService.restoreSite(PortalConfig.PORTAL_TYPE, + ATTACHED_SITE_NAME, + ImportMode.OVERWRITE, + false, + false, + true)); + restartTransaction(); + + NodeContext rootNode = getRootNode(attachedSiteKey); + assertEquals("A node outside the fragment attachment point is kept", + Set.of("section6", CUSTOM_NAME), + getNodeNames(rootNode)); + assertEquals(Set.of("node6"), getNodeNames(rootNode.get("section6"))); + } finally { + removeCustomNode(attachedSiteKey); + } + } + + private void addCustomPageAndNodes() { + PageKey customPageKey = new PageKey(SITE_KEY, CUSTOM_NAME); + pageStorage.savePage(new PageContext(customPageKey, null)); + addCustomNode(getRootNode(SITE_KEY), customPageKey); + addCustomNode(getRootNode(SITE_KEY).get(HOME), customPageKey); + addCustomNode(getRootNode(SITE_KEY).get(HOME).get(SECTION), customPageKey); + addCustomNode(getRootNode(SITE_KEY).get(HOME).get(ATTACHED), customPageKey); + } + + private String saveContainerInAnotherRequest(PageKey pageKey) throws Exception { + PortalContainer container = getContainer(); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + return executor.submit(() -> { + ExoContainerContext.setCurrentContainer(container); + RequestLifeCycle.begin(container); + try { + Page page = pageStorage.getPage(pageKey); + page.setChildren(new ArrayList<>(List.of(new Container()))); + pageStorage.save(page.build()); + return pageStorage.getPage(pageKey).getChildren().get(0).getStorageId(); + } finally { + RequestLifeCycle.end(); + ExoContainerContext.setCurrentContainer(null); + } + }).get(30, TimeUnit.SECONDS); + } finally { + executor.shutdownNow(); + } + } + + private void addCustomNode(NodeContext parentNode, PageKey pageKey) { + NodeContext customNode = parentNode.add(null, CUSTOM_NAME); + customNode.setState(customNode.getState().builder().pageRef(pageKey).build()); + navigationService.saveNode(parentNode, null); + restartTransaction(); + } + + private void removeCustomNode(SiteKey siteKey) { + NodeContext rootNode = getRootNode(siteKey); + if (rootNode.get(CUSTOM_NAME) != null) { + rootNode.get(CUSTOM_NAME).removeNode(); + navigationService.saveNode(rootNode, null); + } + restartTransaction(); + } + + private NodeContext getRootNode(SiteKey siteKey) { + NavigationContext navigation = navigationService.loadNavigation(siteKey); + return navigationService.loadNode(NodeModel.SELF_MODEL, navigation, Scope.ALL, null); + } + + private Set getNodeNames(NodeContext parentNode) { + return parentNode.getNodes().stream().map(node -> ((NodeContext) node).getName()).collect(Collectors.toSet()); + } + + private Set getPageNames(SiteKey siteKey) { + return pageStorage.loadPages(siteKey).stream().map(page -> page.getKey().getName()).collect(Collectors.toSet()); + } + +} diff --git a/component/portal/src/test/java/org/exoplatform/portal/config/TestSiteDataImportOverwrite.java b/component/portal/src/test/java/org/exoplatform/portal/config/TestSiteDataImportOverwrite.java index 68581fda84..a70f088c43 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/config/TestSiteDataImportOverwrite.java +++ b/component/portal/src/test/java/org/exoplatform/portal/config/TestSiteDataImportOverwrite.java @@ -46,7 +46,8 @@ protected void afterOneBootWithExtention(PortalContainer container) throws Excep Application layoutPortlet = (Application) layout.getChildren().get(0); assertEquals("site2/layout", layoutService.getId(layoutPortlet.getState())); PageContext page = pageStorage.loadPage(PageKey.parse("portal::classic::home")); - assertNull(page); + assertNotNull(page); + assertEquals("site 1", page.getState().getDisplayName()); page = pageStorage.loadPage(PageKey.parse("portal::classic::page1")); assertNotNull(page); assertEquals("site 2", page.getState().getDisplayName()); diff --git a/component/portal/src/test/java/org/exoplatform/portal/config/TestUserPortalConfigService.java b/component/portal/src/test/java/org/exoplatform/portal/config/TestUserPortalConfigService.java index 0ae7f0bed9..03fad9b368 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/config/TestUserPortalConfigService.java +++ b/component/portal/src/test/java/org/exoplatform/portal/config/TestUserPortalConfigService.java @@ -37,9 +37,11 @@ import org.exoplatform.container.configuration.ConfigurationManager; import org.exoplatform.container.xml.InitParams; import org.exoplatform.portal.config.model.Application; +import org.exoplatform.portal.config.model.Container; import org.exoplatform.portal.config.model.Page; import org.exoplatform.portal.config.model.PageBody; import org.exoplatform.portal.config.model.PortalConfig; +import org.exoplatform.portal.config.model.Properties; import org.exoplatform.portal.mop.EventType; import org.exoplatform.portal.mop.SiteKey; import org.exoplatform.portal.mop.SiteType; @@ -673,6 +675,48 @@ public void testRestore() { assertFalse(userPortalConfigService.restoreSite("portal", customSiteName, ImportMode.OVERWRITE, false, true, false)); } + public void testRestoreSiteLayoutKeepsTheSiteSettings() { + PortalConfig originalSite = layoutService.getPortalConfig(PortalConfig.PORTAL_TYPE, "classic"); + List> siteUpdates = new ArrayList<>(); + listenerService.addListener(LayoutService.PORTAL_CONFIG_UPDATED, new Listener() { + @Override + public void onEvent(Event event) { + siteUpdates.add(event); + } + }); + try { + PortalConfig site = layoutService.getPortalConfig(PortalConfig.PORTAL_TYPE, "classic"); + site.setLabel("Customized label"); + site.setDescription("Customized description"); + site.setDisplayed(false); + site.setDisplayOrder(7); + site.setProperties(new Properties(Map.of("customized", "true"))); + site.setPortalLayout(new Container()); + layoutService.save(site); + siteUpdates.clear(); + + assertTrue(userPortalConfigService.restoreSite("portal", "classic", ImportMode.OVERWRITE, true, false, false)); + + assertEquals("The restored site is saved once, with the settings it replaces", 1, siteUpdates.size()); + PortalConfig restoredSite = layoutService.getPortalConfig(PortalConfig.PORTAL_TYPE, "classic"); + assertEquals("Customized label", restoredSite.getLabel()); + assertEquals("Customized description", restoredSite.getDescription()); + assertFalse(restoredSite.isDisplayed()); + assertEquals(7, restoredSite.getDisplayOrder()); + assertEquals("true", restoredSite.getProperty("customized")); + assertEquals(originalSite.getPortalLayout().getChildren().size(), restoredSite.getPortalLayout().getChildren().size()); + assertFalse(restoredSite.getPortalLayout().getChildren().isEmpty()); + } finally { + PortalConfig site = layoutService.getPortalConfig(PortalConfig.PORTAL_TYPE, "classic"); + site.setLabel(originalSite.getLabel()); + site.setDescription(originalSite.getDescription()); + site.setDisplayed(originalSite.isDisplayed()); + site.setDisplayOrder(originalSite.getDisplayOrder()); + site.setProperties(originalSite.getProperties()); + layoutService.save(site); + } + } + public void testRootGetPage() { new UnitTest() { public void execute() throws Exception { diff --git a/component/portal/src/test/java/org/exoplatform/portal/mop/dao/SiteDAOTest.java b/component/portal/src/test/java/org/exoplatform/portal/mop/dao/SiteDAOTest.java index 33800fa8a4..fc835883af 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/mop/dao/SiteDAOTest.java +++ b/component/portal/src/test/java/org/exoplatform/portal/mop/dao/SiteDAOTest.java @@ -20,6 +20,9 @@ import java.util.List; +import jakarta.persistence.EntityManager; + +import org.exoplatform.commons.persistence.impl.EntityManagerService; import org.exoplatform.portal.jdbc.entity.SiteEntity; import org.exoplatform.portal.mop.SiteFilter; import org.exoplatform.portal.mop.SiteKey; @@ -152,6 +155,40 @@ public void testFind() { siteDAO.delete(siteEntity2); } + public void testRefreshReadsTheCommittedRow() { + SiteEntity siteEntity = createSiteEntity("refreshed", SiteType.PORTAL); + siteDAO.create(siteEntity); + restartTransaction(); + + SiteEntity loadedEntity = siteDAO.find(siteEntity.getId()); + // A bulk update bypasses the persistence context, as a concurrent writer + // does: the entity loaded on this thread keeps the previous body + executeInTransaction("UPDATE GateInSite s SET s.siteBody = 'committedBody' WHERE s.id = " + siteEntity.getId()); + assertEquals("refreshedBody", siteDAO.find(siteEntity.getId()).getSiteBody()); + + SiteEntity refreshedEntity = siteDAO.refresh(loadedEntity); + assertNotNull(refreshedEntity); + assertEquals("committedBody", refreshedEntity.getSiteBody()); + } + + public void testRefreshRemovedSite() { + SiteEntity siteEntity = createSiteEntity("removed", SiteType.PORTAL); + siteDAO.create(siteEntity); + restartTransaction(); + + SiteEntity loadedEntity = siteDAO.find(siteEntity.getId()); + executeInTransaction("DELETE FROM GateInSite s WHERE s.id = " + siteEntity.getId()); + + assertNull(siteDAO.refresh(loadedEntity)); + } + + private void executeInTransaction(String statement) { + EntityManager entityManager = getContainer().getComponentInstanceOfType(EntityManagerService.class).getEntityManager(); + entityManager.getTransaction().begin(); + entityManager.createQuery(statement).executeUpdate(); + entityManager.getTransaction().commit(); + } + private void assertSite(SiteEntity expected, SiteEntity siteEntity) { assertEquals(expected.getDescription(), siteEntity.getDescription()); assertEquals(expected.getId(), siteEntity.getId()); diff --git a/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemoryPageDAO.java b/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemoryPageDAO.java index 05e5aceddd..ae957cf7f3 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemoryPageDAO.java +++ b/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemoryPageDAO.java @@ -44,6 +44,12 @@ public PageEntity findByKey(PageKey pageKey) { .orElse(null); } + @Override + public PageEntity refresh(PageEntity entity) { + // No persistence context in memory: the stored entity is the current one + return find(entity.getId()); + } + @Override public InMemoryListAccess findByQuery(SiteType siteType, String siteName, diff --git a/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemorySiteDAO.java b/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemorySiteDAO.java index 83930bf9b7..43312a07c5 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemorySiteDAO.java +++ b/component/portal/src/test/java/org/exoplatform/portal/mop/dao/mock/InMemorySiteDAO.java @@ -32,6 +32,12 @@ public class InMemorySiteDAO extends AbstractInMemoryDAO implements SiteDAO { + @Override + public SiteEntity refresh(SiteEntity entity) { + // No persistence context in memory: the stored entity is the current one + return find(entity.getId()); + } + @Override public SiteEntity findByKey(SiteKey siteKey) { return entities.values() diff --git a/component/portal/src/test/java/org/exoplatform/portal/mop/storage/TestDataStorage.java b/component/portal/src/test/java/org/exoplatform/portal/mop/storage/TestDataStorage.java index fb42397285..9f77824846 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/mop/storage/TestDataStorage.java +++ b/component/portal/src/test/java/org/exoplatform/portal/mop/storage/TestDataStorage.java @@ -20,17 +20,38 @@ import static org.junit.Assert.assertThrows; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedList; +import java.util.List; import java.util.Random; - +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import org.json.simple.JSONArray; +import org.json.simple.JSONObject; +import org.json.simple.parser.JSONParser; + +import org.exoplatform.commons.api.settings.SettingService; import org.exoplatform.commons.exception.ObjectNotFoundException; +import org.exoplatform.commons.file.services.FileService; +import org.exoplatform.commons.persistence.impl.EntityManagerService; import org.exoplatform.component.test.AbstractKernelTest; import org.exoplatform.component.test.ConfigurationUnit; import org.exoplatform.component.test.ConfiguredBy; import org.exoplatform.component.test.ContainerScope; +import org.exoplatform.container.ExoContainerContext; import org.exoplatform.container.PortalContainer; +import org.exoplatform.container.component.RequestLifeCycle; +import org.exoplatform.container.configuration.ConfigurationManager; +import org.exoplatform.portal.config.StaleModelException; import org.exoplatform.portal.config.UserACL; import org.exoplatform.portal.config.model.Application; import org.exoplatform.portal.config.model.ApplicationState; @@ -40,13 +61,19 @@ import org.exoplatform.portal.config.model.PersistentApplicationState; import org.exoplatform.portal.config.model.PortalConfig; import org.exoplatform.portal.config.model.TransientApplicationState; +import org.exoplatform.portal.jdbc.entity.PageEntity; +import org.exoplatform.portal.jdbc.entity.SiteEntity; import org.exoplatform.portal.mop.EventType; import org.exoplatform.portal.mop.QueryResult; import org.exoplatform.portal.mop.SiteKey; import org.exoplatform.portal.mop.SiteType; +import org.exoplatform.portal.mop.dao.ContainerDAO; +import org.exoplatform.portal.mop.dao.PageDAO; +import org.exoplatform.portal.mop.dao.SiteDAO; import org.exoplatform.portal.mop.navigation.NavigationContext; import org.exoplatform.portal.mop.navigation.NavigationState; import org.exoplatform.portal.mop.navigation.NodeContext; +import org.exoplatform.portal.mop.navigation.NodeData; import org.exoplatform.portal.mop.navigation.NodeModel; import org.exoplatform.portal.mop.navigation.Scope; import org.exoplatform.portal.mop.page.PageContext; @@ -54,16 +81,27 @@ import org.exoplatform.portal.mop.page.PageState; import org.exoplatform.portal.mop.service.LayoutService; import org.exoplatform.portal.mop.service.NavigationService; +import org.exoplatform.portal.mop.storage.cache.CacheLayoutStorage; +import org.exoplatform.portal.mop.storage.cache.CacheNavigationStorage; +import org.exoplatform.portal.mop.storage.cache.CachePageStorage; +import org.exoplatform.portal.mop.storage.cache.CacheSiteStorage; +import org.exoplatform.portal.pom.data.ComponentData; import org.exoplatform.portal.pom.data.ContainerData; import org.exoplatform.portal.pom.data.PortalData; import org.exoplatform.portal.pom.spi.portlet.Portlet; import org.exoplatform.portal.pom.spi.portlet.PortletBuilder; +import org.exoplatform.services.cache.CacheService; +import org.exoplatform.services.cache.ExoCache; import org.exoplatform.services.listener.Event; import org.exoplatform.services.listener.Listener; import org.exoplatform.services.listener.ListenerService; import org.exoplatform.services.organization.Group; import org.exoplatform.services.organization.GroupHandler; import org.exoplatform.services.organization.OrganizationService; +import org.exoplatform.upload.UploadService; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityTransaction; /** * Created by The eXo Platform SARL Author : Tung Pham thanhtungty@gmail.com Nov @@ -76,6 +114,8 @@ }) public class TestDataStorage extends AbstractKernelTest { + private static final String MISSING_ROOT_CONTAINER_BODY = "[{\"type\":\"CONTAINER\",\"id\":" + Long.MAX_VALUE + "}]"; + /** . */ private LayoutService storage_; @@ -782,6 +822,657 @@ public void testGroupLayout() throws Exception { assertNull(group); } + public void testSaveSiteKeepsTheStoredLayoutWhenTheNewLayoutFails() throws Exception { + String siteName = "failingLayoutSave"; + createSite(SiteType.PORTAL, siteName); + PortalData site = modelStorage.getPortalConfig(SiteKey.portal(siteName)); + String rootContainerId = site.getPortalLayout().getStorageId(); + + PortalData failingSite = new PortalData(site.getStorageId(), + siteName, + site.getType(), + site.getLocale(), + "Changed label", + site.getDescription(), + site.getAccessPermissions(), + site.getEditPermission(), + site.getProperties(), + site.getSkin(), + newContainer(siteName, List.of(new UnsupportedComponentData())), + site.isDisplayed(), + site.getDisplayOrder(), + site.getBannerFileId()); + assertThrows(StaleModelException.class, () -> modelStorage.save(failingSite)); + restartTransaction(); + + PortalData storedSite = modelStorage.getPortalConfig(SiteKey.portal(siteName)); + assertFalse(storedSite.isLayoutUnresolved()); + assertEquals(rootContainerId, storedSite.getPortalLayout().getStorageId()); + assertEquals(site.getLabel(), storedSite.getLabel()); + } + + public void testCreateSiteStoresNothingWhenTheLayoutFails() throws Exception { + String siteName = "failingLayoutCreate"; + ContainerDAO containerDAO = PortalContainer.getInstance().getComponentInstanceOfType(ContainerDAO.class); + long containersCount = containerDAO.count(); + + PortalData failingSite = new PortalData(null, + siteName, + SiteType.PORTAL.getName(), + null, + null, + null, + new ArrayList<>(), + null, + null, + null, + newContainer(siteName, List.of(new UnsupportedComponentData())), + true, + 5, + 0); + assertThrows(StaleModelException.class, () -> modelStorage.create(failingSite)); + restartTransaction(); + + assertNull(modelStorage.getPortalConfig(SiteKey.portal(siteName))); + assertEquals(containersCount, containerDAO.count().longValue()); + } + + public void testSaveSiteReadWithAMissingRootContainerKeepsTheStoredLayout() throws Exception { + String siteName = "unresolvedLayout"; + createSite(SiteType.PORTAL, siteName); + SiteDAO siteDAO = PortalContainer.getInstance().getComponentInstanceOfType(SiteDAO.class); + SiteEntity siteEntity = siteDAO.findByKey(SiteKey.portal(siteName)); + String siteBody = siteEntity.getSiteBody(); + String rootContainerId = getRootContainerId(siteBody); + + // The site row references a root container that a concurrent save deleted + // and replaced without having committed yet + siteEntity.setSiteBody(MISSING_ROOT_CONTAINER_BODY); + siteDAO.update(siteEntity); + restartTransaction(); + + PortalData unresolvedSite = modelStorage.getPortalConfig(SiteKey.portal(siteName)); + assertTrue(unresolvedSite.isLayoutUnresolved()); + + // The concurrent save commits the site row + siteEntity = siteDAO.findByKey(SiteKey.portal(siteName)); + siteEntity.setSiteBody(siteBody); + siteDAO.update(siteEntity); + restartTransaction(); + + PortalConfig unresolvedConfig = new PortalConfig(unresolvedSite); + unresolvedConfig.setLabel("Changed label"); + storage_.save(unresolvedConfig); + restartTransaction(); + + PortalData savedSite = modelStorage.getPortalConfig(SiteKey.portal(siteName)); + assertEquals("Changed label", savedSite.getLabel()); + assertFalse(savedSite.isLayoutUnresolved()); + assertEquals(rootContainerId, savedSite.getPortalLayout().getStorageId()); + } + + public void testReadSiteAgainWhenItsRootContainerIsMissing() throws Exception { + String siteName = "reReadLayout"; + createSite(SiteType.PORTAL, siteName); + PortalContainer container = PortalContainer.getInstance(); + SiteDAO siteDAO = container.getComponentInstanceOfType(SiteDAO.class); + SiteEntity siteEntity = siteDAO.findByKey(SiteKey.portal(siteName)); + String siteBody = siteEntity.getSiteBody(); + String rootContainerId = getRootContainerId(siteBody); + siteEntity.setSiteBody(MISSING_ROOT_CONTAINER_BODY); + siteDAO.update(siteEntity); + restartTransaction(); + + // The concurrent save commits the site row right after this thread read it + SiteDAO concurrentlySavedSiteDAO = (SiteDAO) Proxy.newProxyInstance(SiteDAO.class.getClassLoader(), + new Class[] { SiteDAO.class }, + (proxy, method, args) -> { + Object result = invoke(siteDAO, method, args); + if ("findByKey".equals(method.getName())) { + executeInTransaction("UPDATE GateInSite s SET s.siteBody = '" + + siteBody + "' WHERE s.id = " + + ((SiteEntity) result).getId()); + } + return result; + }); + SiteStorageImpl siteStorage = newSiteStorage(pageService, concurrentlySavedSiteDAO); + + PortalData site = siteStorage.getPortalConfig(SiteKey.portal(siteName)); + assertFalse(site.isLayoutUnresolved()); + assertEquals(rootContainerId, site.getPortalLayout().getStorageId()); + } + + public void testRemoveSiteKeepsTheSiteWhenTheRemovalFails() throws Exception { + String siteName = "failingRemoval"; + createSite(SiteType.PORTAL, siteName); + String rootContainerId = modelStorage.getPortalConfig(SiteKey.portal(siteName)).getPortalLayout().getStorageId(); + + PageStorage failingPageStorage = (PageStorage) Proxy.newProxyInstance(PageStorage.class.getClassLoader(), + new Class[] { PageStorage.class }, + (proxy, method, args) -> { + if ("destroyPages".equals(method.getName())) { + throw new IllegalStateException("Pages removal failure"); + } + return invoke(pageService, method, args); + }); + SiteStorageImpl siteStorage = newSiteStorage(failingPageStorage, + PortalContainer.getInstance().getComponentInstanceOfType(SiteDAO.class)); + assertThrows(IllegalStateException.class, () -> siteStorage.remove(SiteKey.portal(siteName))); + restartTransaction(); + + PortalData storedSite = siteStorage.getPortalConfig(SiteKey.portal(siteName)); + assertNotNull(storedSite); + assertFalse(storedSite.isLayoutUnresolved()); + assertEquals(rootContainerId, storedSite.getPortalLayout().getStorageId()); + assertNotNull(navService.loadNavigation(SiteKey.portal(siteName))); + } + + public void testDestroyPageKeepsThePageLayoutWhenThePageRemovalFails() throws Exception { + PageKey pageKey = createPageWithWindow("failingPageRemoval"); + PageStorageImpl pageStorage = newPageStorageFailingOn("delete"); + + assertThrows(IllegalStateException.class, () -> pageStorage.destroyPage(pageKey)); + restartTransaction(); + + Page storedPage = pageStorage.getPage(pageKey); + assertNotNull(storedPage); + assertEquals(1, storedPage.getChildren().size()); + assertEquals(1, ((Container) storedPage.getChildren().get(0)).getChildren().size()); + } + + public void testSavePageKeepsThePageLayoutWhenThePageUpdateFails() throws Exception { + PageKey pageKey = createPageWithWindow("failingPageUpdate"); + PageStorageImpl pageStorage = newPageStorageFailingOn("update"); + Page page = pageStorage.getPage(pageKey); + ((Container) page.getChildren().get(0)).setChildren(new ArrayList<>()); + + assertThrows(IllegalStateException.class, () -> pageStorage.save(page.build())); + restartTransaction(); + + Page storedPage = pageStorage.getPage(pageKey); + assertEquals(1, storedPage.getChildren().size()); + assertEquals(1, ((Container) storedPage.getChildren().get(0)).getChildren().size()); + } + + public void testCachesAreClearedAgainOnceTheTransactionCompletes() throws Exception { + PortalContainer container = PortalContainer.getInstance(); + CacheService cacheService = container.getComponentInstanceOfType(CacheService.class); + ExoCache windowCache = cacheService.getCacheInstance(CacheLayoutStorage.PORTLET_PREFERENCES_CACHE_NAME); + ExoCache siteCache = cacheService.getCacheInstance(CacheSiteStorage.SITE_CACHE_NAME); + SiteKey siteKey = SiteKey.portal("evictedCaches"); + PortalConfig newSite = new PortalConfig(siteKey.getTypeName(), siteKey.getName()); + newSite.setAccessPermissions(new String[] { UserACL.EVERYONE }); + Application newWindow = new Application(); + newWindow.setState(new TransientApplicationState("test/test", null)); + Container layout = new Container(); + layout.getChildren().add(newWindow); + newSite.setPortalLayout(layout); + storage_.create(newSite); + restartTransaction(); + + PortalConfig site = storage_.getPortalConfig(siteKey.getTypeName(), siteKey.getName()); + Application window = getFirstApplication(site.getPortalLayout()); + assertNotNull(window); + Long windowId = Long.valueOf(window.getStorageId()); + container.getComponentInstanceOfType(LayoutStorage.class).findWindow(windowId); + Object cachedWindow = windowCache.get(windowId); + Object cachedSite = siteCache.get(siteKey); + assertNotNull(cachedWindow); + assertNotNull(cachedSite); + + EntityTransaction transaction = beginTransaction(); + try { + storage_.save(site); + // Reads made by other threads before the commit cache the previous + // states again + windowCache.put(windowId, cachedWindow); + siteCache.put(siteKey, cachedSite); + transaction.commit(); + } finally { + if (transaction.isActive()) { + transaction.rollback(); + } + } + + assertNull(windowCache.get(windowId)); + assertNull(siteCache.get(siteKey)); + } + + public void testRemovedSiteCachesAreClearedAgainOnceTheTransactionCompletes() throws Exception { + String siteName = "removedSiteCaches"; + createSite(SiteType.PORTAL, siteName); + SiteKey siteKey = SiteKey.portal(siteName); + PageKey pageKey = siteKey.page("removedPage"); + pageService.savePage(new PageContext(pageKey, null)); + restartTransaction(); + + PortalContainer container = PortalContainer.getInstance(); + CacheService cacheService = container.getComponentInstanceOfType(CacheService.class); + ExoCache pageCache = cacheService.getCacheInstance(CachePageStorage.PAGE_CACHE_NAME); + ExoCache navigationCache = cacheService.getCacheInstance(CacheNavigationStorage.NAVIGATION_CACHE_NAME); + ExoCache siteCache = cacheService.getCacheInstance(CacheSiteStorage.SITE_CACHE_NAME); + assertNotNull(pageService.loadPage(pageKey)); + assertNotNull(navService.loadNavigation(siteKey)); + assertNotNull(modelStorage.getPortalConfig(siteKey)); + Object cachedPage = pageCache.get(pageKey); + Object cachedNavigation = navigationCache.get(siteKey); + Object cachedSite = siteCache.get(siteKey); + assertNotNull(cachedPage); + assertNotNull(cachedNavigation); + assertNotNull(cachedSite); + + EntityTransaction transaction = beginTransaction(); + try { + modelStorage.remove(siteKey); + // Reads made by other threads before the commit cache the removed site, + // page and navigation again + pageCache.put(pageKey, cachedPage); + navigationCache.put(siteKey, cachedNavigation); + siteCache.put(siteKey, cachedSite); + transaction.commit(); + } finally { + if (transaction.isActive()) { + transaction.rollback(); + } + } + + assertNull(pageCache.get(pageKey)); + assertNull(navigationCache.get(siteKey)); + assertNull(siteCache.get(siteKey)); + } + + public void testUpdatedNodeCacheIsClearedAgainOnceTheTransactionCompletes() throws Exception { + String siteName = "updatedNodeCache"; + createSite(SiteType.PORTAL, siteName); + NavigationContext navigation = navService.loadNavigation(SiteKey.portal(siteName)); + NodeContext rootNode = navService.loadNode(NodeModel.SELF_MODEL, navigation, Scope.CHILDREN, null); + rootNode.add(null, "updatedNode"); + navService.saveNode(rootNode, null); + restartTransaction(); + + PortalContainer container = PortalContainer.getInstance(); + NavigationStorage navigationStorage = container.getComponentInstanceOfType(NavigationStorage.class); + ExoCache nodeCache = container.getComponentInstanceOfType(CacheService.class) + .getCacheInstance(CacheNavigationStorage.NODE_CACHE_NAME); + Long nodeId = Long.valueOf(rootNode.get("updatedNode").getId()); + NodeData nodeData = navigationStorage.loadNode(nodeId); + Object cachedNode = nodeCache.get(nodeId); + assertNotNull(cachedNode); + + EntityTransaction transaction = beginTransaction(); + try { + navigationStorage.updateNode(nodeId, nodeData.getState().builder().label("Updated").build()); + // A read made by another thread before the commit caches the node again + nodeCache.put(nodeId, cachedNode); + transaction.commit(); + } finally { + if (transaction.isActive()) { + transaction.rollback(); + } + } + + assertNull(nodeCache.get(nodeId)); + } + + public void testSiteLoadInFlightDuringASaveIsNotCached() throws Exception { + String siteName = "loadInFlight"; + createSite(SiteType.PORTAL, siteName); + SiteKey siteKey = SiteKey.portal(siteName); + PortalContainer container = PortalContainer.getInstance(); + CacheService cacheService = container.getComponentInstanceOfType(CacheService.class); + ExoCache siteCache = cacheService.getCacheInstance(CacheSiteStorage.SITE_CACHE_NAME); + PortalConfig savedSite = new PortalConfig(modelStorage.getPortalConfig(siteKey)); + siteCache.remove(siteKey); + + // The reader thread reads the site row, then waits until the save commits + // before building and caching the site + SiteDAO siteDAO = container.getComponentInstanceOfType(SiteDAO.class); + Thread testThread = Thread.currentThread(); + CountDownLatch siteRowRead = new CountDownLatch(1); + CountDownLatch siteSaved = new CountDownLatch(1); + SiteDAO slowReaderSiteDAO = (SiteDAO) Proxy.newProxyInstance(SiteDAO.class.getClassLoader(), + new Class[] { SiteDAO.class }, + (proxy, method, args) -> { + Object result = invoke(siteDAO, method, args); + if ("findByKey".equals(method.getName()) + && Thread.currentThread() != testThread) { + siteRowRead.countDown(); + siteSaved.await(10, TimeUnit.SECONDS); + } + return result; + }); + CacheSiteStorage siteStorage = new CacheSiteStorage(cacheService, + container.getComponentInstanceOfType(SettingService.class), + container.getComponentInstanceOfType(ConfigurationManager.class), + container.getComponentInstanceOfType(NavigationStorage.class), + pageService, + container.getComponentInstanceOfType(LayoutStorage.class), + slowReaderSiteDAO, + container.getComponentInstanceOfType(UploadService.class), + container.getComponentInstanceOfType(FileService.class)); + AtomicReference readSite = new AtomicReference<>(); + Thread reader = new Thread(() -> { + ExoContainerContext.setCurrentContainer(container); + RequestLifeCycle.begin(container); + try { + readSite.set(siteStorage.getPortalConfig(siteKey)); + } finally { + RequestLifeCycle.end(); + } + }); + reader.start(); + try { + assertTrue(siteRowRead.await(10, TimeUnit.SECONDS)); + savedSite.setLabel("Saved label"); + siteStorage.save(savedSite); + } finally { + siteSaved.countDown(); + reader.join(10000); + } + + assertNotNull(readSite.get()); + assertNull("The site read before the save is cached", siteCache.get(siteKey)); + assertEquals("Saved label", siteStorage.getPortalConfig(siteKey).getLabel()); + } + + public void testSavePageLayoutRemovesTheLayoutSavedSinceTheRequestReadThePage() throws Exception { + PageKey pageKey = createPageWithWindow("staleLayoutSave"); + getComponent(PageDAO.class).findByKey(pageKey); + String savedContainerId = addPageContainerInAnotherRequest(pageKey); + + Page page = new Page(); + page.setOwnerType(PortalConfig.PORTAL_TYPE); + page.setOwnerId(pageKey.getSite().getName()); + page.setName(pageKey.getName()); + page.setChildren(new ArrayList<>(List.of(new Container()))); + pageService.save(page.build()); + restartTransaction(); + + assertNull("The container saved since the request read the page is replaced with the rest of its layout", + getComponent(ContainerDAO.class).find(Long.parseLong(savedContainerId))); + } + + public void testSavePagePropertiesKeepsTheLayoutSavedSinceTheRequestReadThePage() throws Exception { + PageKey pageKey = createPageWithWindow("stalePropertiesSave"); + getComponent(PageDAO.class).findByKey(pageKey); + String savedContainerId = addPageContainerInAnotherRequest(pageKey); + + PageContext pageContext = pageService.loadPage(pageKey); + pageService.savePage(new PageContext(pageKey, pageContext.getState().builder().displayName("Changed").build())); + restartTransaction(); + + Page savedPage = pageService.getPage(pageKey); + assertEquals("Changed", pageService.loadPage(pageKey).getState().getDisplayName()); + assertEquals("The properties save keeps the layout saved since the request read the page", + savedContainerId, + savedPage.getChildren().get(savedPage.getChildren().size() - 1).getStorageId()); + } + + public void testSavePagePropertiesWritesBackAPropertyChangedSinceTheRequestReadThePage() throws Exception { + PageKey pageKey = createPageWithWindow("stalePropertyValue"); + PageState originalState = pageService.loadPage(pageKey).getState().builder().displayName("Original").build(); + pageService.savePage(new PageContext(pageKey, originalState)); + restartTransaction(); + PageDAO pageDAO = getComponent(PageDAO.class); + pageDAO.findByKey(pageKey); + inAnotherRequest(() -> { + PageState state = pageService.loadPage(pageKey).getState(); + pageService.savePage(new PageContext(pageKey, state.builder().displayName("Other").build())); + assertEquals("The other request's property is stored", "Other", pageDAO.findByKey(pageKey).getDisplayName()); + return null; + }); + + pageService.savePage(new PageContext(pageKey, originalState)); + restartTransaction(); + + assertEquals("A property saved back to the value the request first read is written", + "Original", + pageDAO.findByKey(pageKey).getDisplayName()); + } + + public void testPageRowUpdateWritesOnlyTheColumnsItChanged() throws Exception { + PageKey pageKey = createPageWithWindow("dynamicUpdate"); + PageDAO pageDAO = getComponent(PageDAO.class); + PageEntity page = pageDAO.findByKey(pageKey); + String savedContainerId = addPageContainerInAnotherRequest(pageKey); + + page.setDisplayName("Changed"); + pageDAO.update(page); + restartTransaction(); + + PageEntity storedPage = pageDAO.findByKey(pageKey); + assertEquals("Changed", storedPage.getDisplayName()); + JSONArray storedBody = (JSONArray) new JSONParser().parse(storedPage.getPageBody()); + assertEquals("An update of the page properties writes no layout column", + savedContainerId, + String.valueOf(((JSONObject) storedBody.get(storedBody.size() - 1)).get("id"))); + } + + public void testClonePageCopiesTheLayoutSavedSinceTheRequestReadThePage() throws Exception { + PageKey pageKey = createPageWithWindow("staleClone"); + getComponent(PageDAO.class).findByKey(pageKey); + addPageContainerInAnotherRequest(pageKey); + + PageKey cloneKey = new PageKey(pageKey.getSite(), "fooClone"); + pageService.clone(pageKey, cloneKey); + restartTransaction(); + + assertEquals("The clone copies the layout saved since the request read the page", + 2, + pageService.getPage(cloneKey).getChildren().size()); + } + + public void testSaveSiteRemovesTheLayoutSavedSinceTheRequestReadTheSite() throws Exception { + SiteKey siteKey = SiteKey.portal("staleSiteSave"); + createSite(SiteType.PORTAL, siteKey.getName()); + restartTransaction(); + getComponent(SiteDAO.class).findByKey(siteKey); + String savedContainerId = addSiteContainerInAnotherRequest(siteKey); + + PortalConfig site = new PortalConfig(modelStorage.getPortalConfig(siteKey)); + site.getPortalLayout().setChildren(new ArrayList<>()); + storage_.save(site); + restartTransaction(); + + assertNull("The container saved since the request read the site is replaced with the rest of its layout", + getComponent(ContainerDAO.class).find(Long.parseLong(savedContainerId))); + } + + public void testRemoveSiteRemovesTheLayoutSavedSinceTheRequestReadTheSite() throws Exception { + SiteKey siteKey = SiteKey.portal("staleSiteRemove"); + createSite(SiteType.PORTAL, siteKey.getName()); + restartTransaction(); + getComponent(SiteDAO.class).findByKey(siteKey); + String savedContainerId = addSiteContainerInAnotherRequest(siteKey); + + modelStorage.remove(siteKey); + restartTransaction(); + + assertNull(modelStorage.getPortalConfig(siteKey)); + assertNull("The container saved since the request read the site is removed with the site", + getComponent(ContainerDAO.class).find(Long.parseLong(savedContainerId))); + } + + private String addPageContainerInAnotherRequest(PageKey pageKey) throws Exception { + return inAnotherRequest(() -> { + Page page = pageService.getPage(pageKey); + ArrayList children = new ArrayList<>(page.getChildren()); + children.add(new Container()); + page.setChildren(children); + pageService.save(page.build()); + List savedChildren = pageService.getPage(pageKey).getChildren(); + assertEquals("The other request's layout is stored", children.size(), savedChildren.size()); + return savedChildren.get(savedChildren.size() - 1).getStorageId(); + }); + } + + private String addSiteContainerInAnotherRequest(SiteKey siteKey) throws Exception { + return inAnotherRequest(() -> { + PortalConfig site = new PortalConfig(modelStorage.getPortalConfig(siteKey)); + ArrayList children = site.getPortalLayout().getChildren() == null ? new ArrayList<>() : + new ArrayList<>(site.getPortalLayout() + .getChildren()); + children.add(new Container()); + site.getPortalLayout().setChildren(children); + storage_.save(site); + List savedChildren = new PortalConfig(modelStorage.getPortalConfig(siteKey)).getPortalLayout().getChildren(); + assertEquals("The other request's layout is stored", children.size(), savedChildren.size()); + return savedChildren.get(savedChildren.size() - 1).getStorageId(); + }); + } + + private T inAnotherRequest(Callable task) throws Exception { + PortalContainer container = PortalContainer.getInstance(); + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + return executor.submit(() -> { + ExoContainerContext.setCurrentContainer(container); + RequestLifeCycle.begin(container); + try { + return task.call(); + } finally { + RequestLifeCycle.end(); + ExoContainerContext.setCurrentContainer(null); + } + }).get(30, TimeUnit.SECONDS); + } finally { + executor.shutdownNow(); + } + } + + private T getComponent(Class type) { + return PortalContainer.getInstance().getComponentInstanceOfType(type); + } + + private PageKey createPageWithWindow(String siteName) throws Exception { + createSite(SiteType.PORTAL, siteName); + Application application = new Application(); + application.setState(new TransientApplicationState("test/test", null)); + Container container = new Container(); + container.setTemplate("test"); + container.setChildren(new ArrayList<>(Collections.singletonList(application))); + Page page = new Page(); + page.setOwnerType(PortalConfig.PORTAL_TYPE); + page.setOwnerId(siteName); + page.setName("foo"); + page.setChildren(new ArrayList<>(Collections.singletonList(container))); + pageService.savePage(new PageContext(page.getPageKey(), null)); + pageService.save(page.build()); + restartTransaction(); + return page.getPageKey(); + } + + private PageStorageImpl newPageStorageFailingOn(String pageDAOMethodName) { + PortalContainer container = PortalContainer.getInstance(); + PageDAO pageDAO = container.getComponentInstanceOfType(PageDAO.class); + PageDAO failingPageDAO = (PageDAO) Proxy.newProxyInstance(PageDAO.class.getClassLoader(), + new Class[] { PageDAO.class }, + (proxy, method, args) -> { + if (pageDAOMethodName.equals(method.getName())) { + throw new IllegalStateException("Page DAO failure"); + } + return invoke(pageDAO, method, args); + }); + return new PageStorageImpl(listenerService, + container.getComponentInstanceOfType(LayoutStorage.class), + container.getComponentInstanceOfType(SiteDAO.class), + failingPageDAO); + } + + private SiteStorageImpl newSiteStorage(PageStorage pageStorage, SiteDAO siteDAO) { + PortalContainer container = PortalContainer.getInstance(); + return new SiteStorageImpl(container.getComponentInstanceOfType(SettingService.class), + container.getComponentInstanceOfType(ConfigurationManager.class), + container.getComponentInstanceOfType(NavigationStorage.class), + pageStorage, + container.getComponentInstanceOfType(LayoutStorage.class), + siteDAO, + container.getComponentInstanceOfType(UploadService.class), + container.getComponentInstanceOfType(FileService.class)); + } + + private Application getFirstApplication(Container container) { + for (ModelObject child : container.getChildren()) { + if (child instanceof Application application) { + return application; + } else if (child instanceof Container childContainer) { + Application application = getFirstApplication(childContainer); + if (application != null) { + return application; + } + } + } + return null; + } + + private static Object invoke(Object target, Method method, Object[] args) throws Throwable { + try { + return method.invoke(target, args); + } catch (InvocationTargetException e) { + throw e.getCause(); + } + } + + private void executeInTransaction(String statement) { + EntityManager entityManager = getEntityManager(); + EntityTransaction transaction = entityManager.getTransaction(); + boolean begun = !transaction.isActive(); + if (begun) { + transaction.begin(); + } + entityManager.createQuery(statement).executeUpdate(); + if (begun) { + transaction.commit(); + } + } + + /** + * The test drives the transaction boundary: the transaction that the test + * class may have begun is committed first + */ + private EntityTransaction beginTransaction() { + EntityTransaction transaction = getEntityManager().getTransaction(); + if (transaction.isActive()) { + transaction.commit(); + } + transaction.begin(); + return transaction; + } + + private EntityManager getEntityManager() { + return PortalContainer.getInstance().getComponentInstanceOfType(EntityManagerService.class).getEntityManager(); + } + + private String getRootContainerId(String siteBody) throws Exception { + JSONArray children = (JSONArray) new JSONParser().parse(siteBody); + return String.valueOf(((JSONObject) children.get(0)).get("id")); + } + + private ContainerData newContainer(String siteName, List children) { + return new ContainerData(null, + "testcontainer_" + siteName, + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + null, + null, + Collections.emptyList(), + children); + } + + private static class UnsupportedComponentData extends ComponentData { + + private static final long serialVersionUID = 1L; + + } + protected void createSite(SiteType type, String siteName) throws Exception { ContainerData container = new ContainerData(null, "testcontainer_" + siteName, diff --git a/component/portal/src/test/java/org/exoplatform/portal/test/InitContainer10TestSuite.java b/component/portal/src/test/java/org/exoplatform/portal/test/InitContainer10TestSuite.java new file mode 100644 index 0000000000..d6c9fe9538 --- /dev/null +++ b/component/portal/src/test/java/org/exoplatform/portal/test/InitContainer10TestSuite.java @@ -0,0 +1,33 @@ +/** + * This file is part of the Meeds project (https://meeds.io/). + * + * Copyright (C) 2026 Meeds Association contact@meeds.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.exoplatform.portal.test; + +import org.junit.runner.RunWith; +import org.junit.runners.Suite; +import org.junit.runners.Suite.SuiteClasses; + +import org.exoplatform.portal.config.TestRestoreSiteConfigurations; + +@RunWith(Suite.class) +@SuiteClasses({ + TestRestoreSiteConfigurations.class, +}) +public class InitContainer10TestSuite { + +} diff --git a/component/portal/src/test/java/org/exoplatform/portal/test/InitContainer1TestSuite.java b/component/portal/src/test/java/org/exoplatform/portal/test/InitContainer1TestSuite.java index 14ff4a1885..10efc03688 100644 --- a/component/portal/src/test/java/org/exoplatform/portal/test/InitContainer1TestSuite.java +++ b/component/portal/src/test/java/org/exoplatform/portal/test/InitContainer1TestSuite.java @@ -47,6 +47,7 @@ import org.exoplatform.portal.mop.dao.PermissionDAOTest; import org.exoplatform.portal.mop.dao.SiteDAOTest; import org.exoplatform.portal.mop.dao.WindowDAOTest; +import org.exoplatform.portal.mop.importer.TestNavigationFragmentImporterRestoreDefaults; import org.exoplatform.portal.mop.rest.NavigationRestTest; import org.exoplatform.portal.mop.storage.LayoutStorageTest; import org.exoplatform.portal.mop.storage.TestDescriptionService; @@ -100,6 +101,7 @@ TestDescriptionService.class, TestNavigationServiceRebase.class, TestNavigationService.class, + TestNavigationFragmentImporterRestoreDefaults.class, TestNavigationInsertAtIndex.class, TestNavigationServiceSave.class, TestNavigationServiceUpdate.class, diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/configuration.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/configuration.xml new file mode 100644 index 0000000000..3722a0779e --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/configuration.xml @@ -0,0 +1,208 @@ + + + + + org.exoplatform.portal.config.UserPortalConfigService + + new.portal.config.user.listener.contributed + initListener + org.exoplatform.portal.config.NewPortalConfigListener + + + contribution1 + + + + + contributed + + + + + portal + + + classpath:/org/exoplatform/portal/config/restore/contribution1 + + + + + contribution2 + + + + + contributed + + + + + portal + + + classpath:/org/exoplatform/portal/config/restore/contribution2 + + + + + contribution3 + + + + + contributed + + + + + portal + + + classpath:/org/exoplatform/portal/config/restore/contribution3 + + + + + contribution4 + + + + + partial + + + + + portal + + + classpath:/org/exoplatform/portal/config/restore/contribution4 + + + + + contribution5 + + + + + partial + + + + + portal + + + classpath:/org/exoplatform/portal/config/restore/contribution5 + + + + + contribution6 + + + + + attached + + + + + portal + + + classpath:/org/exoplatform/portal/config/restore/contribution6 + + + + + contribution7 + + + + + startup + + + + + portal + + + insert + + + classpath:/org/exoplatform/portal/config/restore/contribution7 + + + + + contribution8 + + + + + startup + + + + + portal + + + overwrite + + + true + + + classpath:/org/exoplatform/portal/config/restore/contribution8 + + + + + contribution9 + A site type that doesn't exist, overwriting a site of the same name: the other sites are imported all the same + + + + + startup + + + + + unknown + + + overwrite + + + classpath:/org/exoplatform/portal/config/restore/contribution4 + + + + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution1/portal/contributed/pages.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution1/portal/contributed/pages.xml new file mode 100644 index 0000000000..ebcbf4285f --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution1/portal/contributed/pages.xml @@ -0,0 +1,32 @@ + + + + + + page1 + Page 1 + Everyone + *:/platform/administrators + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution1/portal/contributed/portal.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution1/portal/contributed/portal.xml new file mode 100644 index 0000000000..564c85fa70 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution1/portal/contributed/portal.xml @@ -0,0 +1,32 @@ + + + + contributed + + en + Everyone + *:/platform/administrators + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution2/portal/contributed/navigation.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution2/portal/contributed/navigation.xml new file mode 100644 index 0000000000..8b37c47467 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution2/portal/contributed/navigation.xml @@ -0,0 +1,46 @@ + + + + 1 + + + + home + + + section + + + node2 + + portal::contributed::page2 + + + node23 + + portal::contributed::page3 + + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution2/portal/contributed/pages.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution2/portal/contributed/pages.xml new file mode 100644 index 0000000000..e3730a1297 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution2/portal/contributed/pages.xml @@ -0,0 +1,32 @@ + + + + + + page2 + Page 2 + Everyone + *:/platform/administrators + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution3/portal/contributed/navigation.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution3/portal/contributed/navigation.xml new file mode 100644 index 0000000000..1c43ada28c --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution3/portal/contributed/navigation.xml @@ -0,0 +1,54 @@ + + + + 1 + + + + home + + + section + + + node3 + + portal::contributed::page3 + + + + + + + home + + attached3 + + + leaf3 + + portal::contributed::page3 + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution3/portal/contributed/pages.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution3/portal/contributed/pages.xml new file mode 100644 index 0000000000..9bf6239e3c --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution3/portal/contributed/pages.xml @@ -0,0 +1,32 @@ + + + + + + page3 + Page 3 + Everyone + *:/platform/administrators + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/navigation.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/navigation.xml new file mode 100644 index 0000000000..85fdd60192 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/navigation.xml @@ -0,0 +1,32 @@ + + + + 1 + + + + node4 + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/pages.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/pages.xml new file mode 100644 index 0000000000..7811073292 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/pages.xml @@ -0,0 +1,32 @@ + + + + + + page4 + Page 4 + Everyone + *:/platform/administrators + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/portal.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/portal.xml new file mode 100644 index 0000000000..1bf0cd2686 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution4/portal/partial/portal.xml @@ -0,0 +1,32 @@ + + + + partial + + en + Everyone + *:/platform/administrators + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution5/portal/partial/navigation.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution5/portal/partial/navigation.xml new file mode 100644 index 0000000000..236e15aa90 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution5/portal/partial/navigation.xml @@ -0,0 +1,25 @@ + + + + + + + node5 + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution5/portal/partial/pages.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution5/portal/partial/pages.xml new file mode 100644 index 0000000000..caacccd2f0 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution5/portal/partial/pages.xml @@ -0,0 +1,24 @@ + + + + + + page5 + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution6/portal/attached/navigation.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution6/portal/attached/navigation.xml new file mode 100644 index 0000000000..adc6e3b448 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution6/portal/attached/navigation.xml @@ -0,0 +1,33 @@ + + + + 1 + + + section6 + + node6 + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution6/portal/attached/portal.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution6/portal/attached/portal.xml new file mode 100644 index 0000000000..5fe269c0f4 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution6/portal/attached/portal.xml @@ -0,0 +1,32 @@ + + + + attached + + en + Everyone + *:/platform/administrators + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/navigation.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/navigation.xml new file mode 100644 index 0000000000..b4adacef22 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/navigation.xml @@ -0,0 +1,33 @@ + + + + 1 + + + + nodeA + + portal::startup::pageA + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/pages.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/pages.xml new file mode 100644 index 0000000000..fbbe547394 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/pages.xml @@ -0,0 +1,32 @@ + + + + + + pageA + Page A + Everyone + *:/platform/administrators + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/portal.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/portal.xml new file mode 100644 index 0000000000..bee7c85623 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution7/portal/startup/portal.xml @@ -0,0 +1,32 @@ + + + + startup + + en + Everyone + *:/platform/administrators + + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution8/portal/startup/navigation.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution8/portal/startup/navigation.xml new file mode 100644 index 0000000000..b5584637e1 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution8/portal/startup/navigation.xml @@ -0,0 +1,33 @@ + + + + 1 + + + + nodeB + + portal::startup::pageB + + + diff --git a/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution8/portal/startup/pages.xml b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution8/portal/startup/pages.xml new file mode 100644 index 0000000000..fd58fbe979 --- /dev/null +++ b/component/portal/src/test/resources/org/exoplatform/portal/config/restore/contribution8/portal/startup/pages.xml @@ -0,0 +1,32 @@ + + + + + + pageB + Page B + Everyone + *:/platform/administrators + + +