diff --git a/.github/workflows/ga-publication.yml b/.github/workflows/ga-publication.yml index 12552a7..8a5260e 100644 --- a/.github/workflows/ga-publication.yml +++ b/.github/workflows/ga-publication.yml @@ -41,7 +41,7 @@ jobs: --id "CHECKOUT FOR GA PUBLICATION" \ --lcd "$LOCAL_CLONE_DIR" - name: "Cache Maven packages" - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }} @@ -63,4 +63,4 @@ jobs: env: MAVEN_USERNAME: ${{ secrets.MAVEN_USERNAME }} MAVEN_PASSWORD: ${{ secrets.MAVEN_PASSWORD }} - MAVEN_GPG_PASSPHRASE: ${{ secrets.MAVEN_GPG_PASSPHRASE }} \ No newline at end of file + MAVEN_GPG_PASSPHRASE: ${{ secrets.MAVEN_GPG_PASSPHRASE }} diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 6d1fcfe..1890fa8 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -95,7 +95,7 @@ jobs: java-version: 11 #~ MAVEN CACHE - name: "Cache Maven packages" - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2 @@ -103,13 +103,13 @@ jobs: #~ BUILD CACHE - name: "Cache Build Dir" id: build-cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: "${{ env.LOCAL_CLONE_DIR }}/target/" key: ${{ runner.os }}-build-${{ env.BUILD_CACHE_KEY }} #~ SONAR CACHE - name: Cache SonarCloud packages - uses: actions/cache@v1 + uses: actions/cache@v4 with: path: ~/.sonar/cache key: ${{ runner.os }}-sonar @@ -151,7 +151,7 @@ jobs: #~ MAVEN CACHE - name: "Cache Maven packages" id: maven-cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2-matrix-${{ matrix.scan-type }} @@ -162,7 +162,7 @@ jobs: #~ BUILD CACHE - name: "Cache Build Dir" id: build-cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: "${{ env.LOCAL_CLONE_DIR}}/target/" key: ${{ runner.os }}-build-${{ env.BUILD_CACHE_KEY }} @@ -188,4 +188,4 @@ jobs: export ENTANDO_OPT_TEST_TLS_KEY="${{ secrets.ENTANDO_OPT_TEST_TLS_KEY }}" ;; esac - ~/ppl-run generic "$SCAN_TYPE" mvn --id "$SCAN_TYPE" --lcd "$LOCAL_CLONE_DIR" \ No newline at end of file + ~/ppl-run generic "$SCAN_TYPE" mvn --id "$SCAN_TYPE" --lcd "$LOCAL_CLONE_DIR" diff --git a/.github/workflows/publication.yml b/.github/workflows/publication.yml index d3ba911..7834d68 100644 --- a/.github/workflows/publication.yml +++ b/.github/workflows/publication.yml @@ -56,7 +56,7 @@ jobs: java-version: 11 #~ MAVEN CACHE - name: "Cache Maven packages" - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2 @@ -64,7 +64,7 @@ jobs: #~ BUILD CACHE - name: "Cache Build Dir" id: build-cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: "${{ env.LOCAL_CLONE_DIR }}/target/" key: ${{ runner.os }}-build-${{ env.BUILD_CACHE_KEY }} @@ -131,7 +131,7 @@ jobs: java-version: 11 #~ MAVEN CACHE - name: "Cache Maven packages" - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2 @@ -139,7 +139,7 @@ jobs: #~ BUILD CACHE - name: "Cache Build Dir" id: build-cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: "${{ env.LOCAL_CLONE_DIR }}/target/" key: ${{ runner.os }}-build-${{ env.BUILD_CACHE_KEY }} @@ -177,7 +177,7 @@ jobs: java-version: 11 #~ MAVEN CACHE - name: "Cache Maven packages" - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.m2 key: ${{ runner.os }}-m2 @@ -185,7 +185,7 @@ jobs: #~ BUILD CACHE - name: "Cache Build Dir" id: build-cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: "${{ env.LOCAL_CLONE_DIR }}/target/" key: ${{ runner.os }}-build-${{ env.BUILD_CACHE_KEY }} @@ -200,4 +200,4 @@ jobs: export ENTANDO_OPT_TEST_TLS_CRT="${{ secrets.ENTANDO_OPT_TEST_TLS_CRT }}" export ENTANDO_OPT_TEST_TLS_KEY="${{ secrets.ENTANDO_OPT_TEST_TLS_KEY }}" - ~/ppl-run generic "POST-DEP-TESTS" --id "POST_DEP_TESTS" --lcd "$LOCAL_CLONE_DIR" \ No newline at end of file + ~/ppl-run generic "POST-DEP-TESTS" --id "POST_DEP_TESTS" --lcd "$LOCAL_CLONE_DIR" diff --git a/pom.xml b/pom.xml index 0be6722..8cb9933 100644 --- a/pom.xml +++ b/pom.xml @@ -5,12 +5,12 @@ org.entando entando-core-parent - 6.5.0-ENG-5128-PR-156 + 6.5.0 entando-plugin-jpversioning org.entando.entando.plugins war - 6.5.1 + 6.5.2 Entando Plugin: Content Versioning Manages the versioning of portal content, storing the history of performed operations @@ -99,7 +99,7 @@ org.entando entando-core-bom - 6.5.0-ENG-5128 + 6.5.1 pom import @@ -111,6 +111,7 @@ org.entando.entando.plugins entando-plugin-jacms classes + 6.5.4-ESB-827-PR-240 org.entando.entando @@ -146,6 +147,7 @@ org.entando.entando.plugins entando-plugin-jacms test-jar + 6.5.4-ESB-827-PR-240 org.springframework @@ -163,6 +165,16 @@ org.mockito mockito-junit-jupiter test + + + org.mockito + mockito-core + + + + + org.mockito + mockito-inline org.assertj @@ -201,6 +213,5 @@ 2.14.1 jar - diff --git a/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/IFDeferredVersioning.java b/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/IFDeferredVersioning.java new file mode 100644 index 0000000..9df9654 --- /dev/null +++ b/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/IFDeferredVersioning.java @@ -0,0 +1,37 @@ +package com.agiletec.plugins.jpversioning.aps.system.services.versioning; + +import com.agiletec.aps.system.ApsSystemUtils.ApsDeepDebug; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.function.Supplier; +import org.entando.entando.aps.system.services.IFeatureFlag; + +public interface IFDeferredVersioning extends IFeatureFlag { + boolean DEFERRED_VERSIONING_ENABLED = checkEnabled(); + + default boolean isEnabled() { + return DEFERRED_VERSIONING_ENABLED; + } + + static boolean checkEnabled() { + return IFeatureFlag.readEnablementStatus("DEFERRED_VERSIONING"); + } + + static CompletableFuture possiblyDeferred(Executor executor, Supplier action, String method) { + if (checkEnabled()) { + ApsDeepDebug.print("deferred-versioning", "running versioning task in a separate thread: " + method); + + return CompletableFuture.runAsync(() -> action.get(), executor); + } else { + try { + action.get(); + return CompletableFuture.completedFuture(null); + } catch (Exception e) { + CompletableFuture failed = new CompletableFuture<>(); + failed.completeExceptionally(e); + return failed; + } + } + } + +} diff --git a/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/IVersioningManager.java b/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/IVersioningManager.java index 0506ca7..a14b295 100644 --- a/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/IVersioningManager.java +++ b/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/IVersioningManager.java @@ -24,6 +24,8 @@ import com.agiletec.aps.system.common.entity.IEntityManager; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; import org.entando.entando.ent.exception.EntException; import com.agiletec.plugins.jacms.aps.system.services.content.model.Content; @@ -47,8 +49,8 @@ public interface IVersioningManager { public ContentVersion getVersion(long id) throws EntException; public ContentVersion getLastVersion(String contentId) throws EntException; - - public void saveContentVersion(String contentId) throws EntException; + + public void saveContentVersion(String contentId) throws EntException; public void deleteVersion(long versionid) throws EntException; @@ -56,4 +58,4 @@ public interface IVersioningManager { public Content getContent(ContentVersion contentVersion) throws EntException; -} \ No newline at end of file +} diff --git a/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/VersioningManager.java b/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/VersioningManager.java index 32c2980..15989ea 100644 --- a/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/VersioningManager.java +++ b/src/main/java/com/agiletec/plugins/jpversioning/aps/system/services/versioning/VersioningManager.java @@ -21,19 +21,6 @@ */ package com.agiletec.plugins.jpversioning.aps.system.services.versioning; -import java.io.StringReader; -import java.util.List; - -import javax.xml.parsers.SAXParser; - -import org.aspectj.lang.annotation.Aspect; -import org.aspectj.lang.annotation.Before; -import org.entando.entando.ent.exception.EntException; -import org.entando.entando.ent.util.EntLogging.EntLogger; -import org.entando.entando.ent.util.EntLogging.EntLogFactory; -import org.entando.entando.ent.util.EntSafeXmlUtils; -import org.xml.sax.InputSource; - import com.agiletec.aps.system.common.AbstractService; import com.agiletec.aps.system.common.entity.parse.EntityHandler; import com.agiletec.aps.system.services.baseconfig.ConfigInterface; @@ -43,8 +30,19 @@ import com.agiletec.plugins.jacms.aps.system.services.content.model.ContentRecordVO; import com.agiletec.plugins.jpversioning.aps.system.JpversioningSystemConstants; import java.io.IOException; +import java.io.StringReader; +import java.util.List; +import java.util.concurrent.Executor; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.parsers.SAXParser; import org.apache.commons.lang3.StringUtils; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.entando.entando.ent.exception.EntException; +import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.EntLogging.EntLogger; +import org.entando.entando.ent.util.EntSafeXmlUtils; +import org.xml.sax.InputSource; import org.xml.sax.SAXException; /** @@ -55,6 +53,7 @@ public class VersioningManager extends AbstractService implements IVersioningMan private static final EntLogger _logger = EntLogFactory.getSanitizedLogger(VersioningManager.class); + @Override public void init() throws Exception { String deleteMidVersions = this.getConfigManager().getParam(JpversioningSystemConstants.CONFIG_PARAM_DELETE_MID_VERSIONS); @@ -62,51 +61,51 @@ public void init() throws Exception { _logger.debug("{} ready", this.getClass().getName()); } + @Before("execution(* com.agiletec.plugins.jacms.aps.system.services.content.IContentManager.saveContent(..)) && args(content)") public void onSaveContent(Content content) { - try { - if (!this.hasToVersionContent(content)) { - return; - } - this.saveContentVersion(content.getId()); - } catch (Exception e) { - _logger.error("error in onSaveContent", e); - } + this.processDeferredVersioning(content, "onSaveContent"); } @Before("execution(* com.agiletec.plugins.jacms.aps.system.services.content.IContentManager.insertOnLineContent(..)) && args(content)") public void onInsertOnLineContent(Content content) { - try { - if (!this.hasToVersionContent(content)) { - return; - } - this.saveContentVersion(content.getId()); - } catch (Exception e) { - _logger.error("error in onInsertOnLineContent", e); - } + this.processDeferredVersioning(content, "onInsertOnLineContent"); } @Before("execution(* com.agiletec.plugins.jacms.aps.system.services.content.IContentManager.removeOnLineContent(..)) && args(content)") public void onRemoveOnLineContent(Content content) { - try { - if (!this.hasToVersionContent(content)) { - return; - } - this.saveContentVersion(content.getId()); - } catch (Exception e) { - _logger.error("error in onRemoveOnLineContent", e); - } + this.processDeferredVersioning(content, "onRemoveOnLineContent"); } @Before("execution(* com.agiletec.plugins.jacms.aps.system.services.content.IContentManager.deleteContent(..)) && args(content)") public void onDeleteContent(Content content) { + this.processDeferredVersioning(content, "onDeleteContent"); + } + + private void processDeferredVersioning(Content content, String methodName) { try { if (!this.hasToVersionContent(content)) { return; } - this.saveContentVersion(content.getId()); + // this is kept sequential + final ContentRecordVO contentRecordVO = this.getContentManager().loadContentVO(content.getId()); + // the remainder of the process can be safely deferred + IFDeferredVersioning.possiblyDeferred(_executor, + () -> { + try { + saveContentVersion(contentRecordVO); + } catch (EntException ex) { + _logger.error("error in (deferred) {}", methodName, ex); + } + return null; + }, methodName + ", " + content.getId()) + .whenComplete((result, ex) -> { + if (ex != null) { + _logger.error("error in possiblyDeferred {}", methodName, ex); + } + }); } catch (Exception e) { - _logger.error("error in onDeleteContent", e); + _logger.error("error in {}", methodName, e); } } @@ -168,7 +167,7 @@ public ContentVersion getLastVersion(String contentId) throws EntException { return this.getVersioningDAO().getLastVersion(contentId); } catch (Exception e) { _logger.error("Error loading last version for content {}", contentId, e); - throw new EntException("Error loading last version for content" + contentId); + throw new EntException("Error loading last version for content " + contentId); } } @@ -176,20 +175,29 @@ public ContentVersion getLastVersion(String contentId) throws EntException { public void saveContentVersion(String contentId) throws EntException { try { if (contentId != null) { - ContentRecordVO record = this.getContentManager().loadContentVO(contentId); - if (record != null) { - ContentVersion versionRecord = this.createContentVersion(record); - //CANCELLAZIONE VERSIONE WORK OBSOLETE - if (versionRecord.isApproved()) { - int onlineVersionsToDelete = versionRecord.getOnlineVersion() - 1; - this.deleteWorkVersions(versionRecord.getContentId(), onlineVersionsToDelete); - } - this.getVersioningDAO().addContentVersion(versionRecord); - } + final ContentRecordVO contentRecordVO = this.getContentManager().loadContentVO(contentId); + saveContentVersion(contentRecordVO); } } catch (Exception e) { _logger.error("error in Error saving version for content {}", contentId, e); - throw new EntException("Error saving version for content" + contentId); + throw new EntException("Error saving version for content " + contentId); + } + } + + protected void saveContentVersion(final ContentRecordVO recordVO) throws EntException { + try { + if (recordVO != null) { + ContentVersion versionRecord = this.createContentVersion(recordVO); + //CANCELLAZIONE VERSIONE WORK OBSOLETE + if (versionRecord.isApproved()) { + int onlineVersionsToDelete = versionRecord.getOnlineVersion() - 1; + this.deleteWorkVersions(versionRecord.getContentId(), onlineVersionsToDelete); + } + this.getVersioningDAO().addContentVersion(versionRecord); + } + } catch (Exception e) { + _logger.error("error in Error saving version for content {}", recordVO.getId(), e); + throw new EntException("Error saving version for content " + recordVO.getId()); } } @@ -217,7 +225,7 @@ public Content getContent(ContentVersion contentVersion) throws EntException { /** * Crea un'entità specifica valorizzata in base alla sua definizione in xml - * ed al tipo. + * e al tipo. * * @param entityTypeCode Il codice del tipo di entità. * @param xml L'xml dell'entità specifica. @@ -277,7 +285,7 @@ protected String getXmlAttributeRootElementName() { /** * Setta il nome dell'attributo della root dell'xml rappresentante la - * singola entità. Il metodo è ad uso della definizione del servizio + * singola entità. Il metodo è a uso della definizione del servizio * nell'xml di configurazione di spring. Di default, la definizione del * servizio astratto nella configurazione di spring presenta una un nome * base "entity"; questa definizione và sostituita nella definizione del @@ -352,6 +360,14 @@ public void setConfigManager(ConfigInterface configManager) { this._configManager = configManager; } + public Executor getExecutor() { + return _executor; + } + + public void setExecutor(Executor executor) { + this._executor = executor; + } + private boolean _deleteMidVersions; private EntityHandler _entityHandler; @@ -363,5 +379,6 @@ public void setConfigManager(ConfigInterface configManager) { private IContentManager _contentManager; private ICategoryManager _categoryManager; private ConfigInterface _configManager; + private transient Executor _executor; } diff --git a/src/main/resources/spring/plugins/jpversioning/aps/versioningManagersConfig.xml b/src/main/resources/spring/plugins/jpversioning/aps/versioningManagersConfig.xml index 1431987..0d5ed83 100644 --- a/src/main/resources/spring/plugins/jpversioning/aps/versioningManagersConfig.xml +++ b/src/main/resources/spring/plugins/jpversioning/aps/versioningManagersConfig.xml @@ -10,6 +10,15 @@ + + + + + + + + + @@ -24,6 +33,7 @@ + versions = this.versioningManager.getVersions("CNG12"); assertNull(versions); @@ -66,6 +69,7 @@ void testGetVersions() throws Throwable { this.checkVersionIds(new long[]{1, 2, 3}, versions); } + @Test void testGetLastVersions() throws Throwable { List versions = this.versioningManager.getLastVersions("CNG", null); assertTrue(versions.isEmpty()); @@ -74,6 +78,7 @@ void testGetLastVersions() throws Throwable { this.checkVersionIds(new long[]{3}, versions); } + @Test void testGetVersion() throws Throwable { ContentVersion contentVersion = this.versioningManager.getVersion(10000); assertNull(contentVersion); @@ -92,6 +97,7 @@ void testGetVersion() throws Throwable { assertEquals("admin", contentVersion.getUsername()); } + @Test void testGetLastVersion() throws Throwable { ContentVersion contentVersion = this.versioningManager.getLastVersion("CNG12"); assertNull(contentVersion); @@ -109,8 +115,9 @@ void testGetLastVersion() throws Throwable { assertEquals("mainEditor", contentVersion.getUsername()); } + @Test void testSaveGetDeleteVersion() throws Throwable { - ((VersioningManager) this.versioningManager).saveContentVersion("ART102"); + this.versioningManager.saveContentVersion("ART102"); ContentVersion contentVersion = this.versioningManager.getLastVersion("ART102"); assertEquals(4, contentVersion.getId()); assertEquals("ART102", contentVersion.getContentId()); @@ -128,28 +135,46 @@ void testSaveGetDeleteVersion() throws Throwable { assertNull(this.versioningManager.getLastVersion("ART102")); } - public void deleteWorkVersions() throws Throwable { - List versions = this.versioningManager.getVersions("ART1"); - this.checkVersionIds(new long[]{1, 2, 3}, versions); - this.versioningManager.deleteWorkVersions("ART1", 0); - versions = this.versioningManager.getVersions("ART1"); - this.checkVersionIds(new long[]{1, 3}, versions); + @Test + void testDeleteWorkVersions() throws Throwable { + String contentId = "ART1"; + VersioningManager versioningManagerImpl = (VersioningManager) this.versioningManager; + try { + versioningManagerImpl.setDeleteMidVersions(true); + List versions = this.versioningManager.getVersions(contentId); + this.checkVersionIds(new long[]{1, 2, 3}, versions); + this.versioningManager.deleteWorkVersions(contentId, 0); + versions = this.versioningManager.getVersions(contentId); + this.checkVersionIds(new long[]{1, 3}, versions); + + this.helper.initContentVersions(); + versioningManagerImpl.setDeleteMidVersions(false); + versions = this.versioningManager.getVersions(contentId); + this.checkVersionIds(new long[]{1, 2, 3}, versions); + this.versioningManager.deleteWorkVersions(contentId, 0); + versions = this.versioningManager.getVersions(contentId); + this.checkVersionIds(new long[]{1, 2, 3}, versions); + } finally { + versioningManagerImpl.setDeleteMidVersions(true); + } } private void checkVersionIds(long[] expected, List received) { assertEquals(expected.length, received.size()); for (long current : expected) { - if (!received.contains(new Long(current))) { + if (!received.contains(current)) { fail("Expected " + current + " - Not found"); } } } + @Test void testContentVersionToIgnore_1() throws Exception { this.testContentVersionToIgnore(false, true); this.testContentVersionToIgnore(true, true); } + @Test void testContentVersionToIgnore_2() throws Exception { this.testContentVersionToIgnore(false, false); this.testContentVersionToIgnore(true, false); @@ -186,8 +211,6 @@ protected void testContentVersionToIgnore(boolean includeVersion, boolean isType } else { assertTrue(null == versions || versions.isEmpty()); } - } catch (Exception e) { - throw e; } finally { if (null != versions) { for (Long version : versions) { @@ -206,6 +229,65 @@ protected void testContentVersionToIgnore(boolean includeVersion, boolean isType } } + @Test + void testGetContent_Exception() { + ContentVersion contentVersion = new ContentVersion(); + contentVersion.setContentType("ART"); + contentVersion.setXml(""); + assertThrows(org.entando.entando.ent.exception.EntException.class, () -> { + this.versioningManager.getContent(contentVersion); + }); + + contentVersion.setXml(""); + contentVersion.setContentType("INVALID_TYPE"); + assertThrows(org.entando.entando.ent.exception.EntException.class, () -> { + this.versioningManager.getContent(contentVersion); + }); + } + + @Test + void testSaveContentVersion_Exception() { + // We test that it does NOT throw exception for null ID, as per implementation + try { + this.versioningManager.saveContentVersion((String) null); + } catch (Exception e) { + fail("Should not throw exception for null contentId"); + } + } + + @Test + void testSaveContentVersionVO_Exception() { + com.agiletec.plugins.jacms.aps.system.services.content.model.ContentRecordVO record = new com.agiletec.plugins.jacms.aps.system.services.content.model.ContentRecordVO(); + record.setId("ART1"); + record.setVersion("invalid.version"); // This will cause NumberFormatException in createContentVersion + + assertThrows(org.entando.entando.ent.exception.EntException.class, () -> { + ((VersioningManager) this.versioningManager).saveContentVersion(record); + }); + + record.setVersion(null); // This will cause NullPointerException in createContentVersion + assertThrows(org.entando.entando.ent.exception.EntException.class, () -> { + ((VersioningManager) this.versioningManager).saveContentVersion(record); + }); + } + + @Test + void testSaveContentVersionString_Exception() throws Exception { + IContentManager originalContentManager = ((VersioningManager) this.versioningManager).getContentManager(); + IContentManager mockContentManager = mock(IContentManager.class); + when(mockContentManager.loadContentVO(anyString())).thenThrow(new RuntimeException("Test exception")); + + try { + ((VersioningManager) this.versioningManager).setContentManager(mockContentManager); + assertThrows(org.entando.entando.ent.exception.EntException.class, () -> { + this.versioningManager.saveContentVersion("ART1"); + }); + } finally { + ((VersioningManager) this.versioningManager).setContentManager(originalContentManager); + } + } + + private void updateConfigItem(String paramKey, String paramValue) throws Exception { Map params = new HashMap<>(); params.put(paramKey, paramValue); @@ -215,7 +297,7 @@ private void updateConfigItem(String paramKey, String paramValue) throws Excepti } @BeforeEach - private void init() throws Exception { + public void init() throws Exception { this.versioningManager = (IVersioningManager) this.getService(JpversioningSystemConstants.VERSIONING_MANAGER); this.configManager = (ConfigInterface) this.getService(SystemConstants.BASE_CONFIG_MANAGER); this.contentManager = (IContentManager) this.getService(JacmsSystemConstants.CONTENT_MANAGER); @@ -225,7 +307,7 @@ private void init() throws Exception { } @AfterEach - private void dispose() throws Exception { + public void dispose() throws Exception { this.helper.cleanContentVersions(); } diff --git a/src/test/java/com/agiletec/plugins/jpversioning/apsadmin/versioning/TestVersionAction.java b/src/test/java/com/agiletec/plugins/jpversioning/apsadmin/versioning/TestVersionAction.java index 57fdc4d..375872f 100644 --- a/src/test/java/com/agiletec/plugins/jpversioning/apsadmin/versioning/TestVersionAction.java +++ b/src/test/java/com/agiletec/plugins/jpversioning/apsadmin/versioning/TestVersionAction.java @@ -28,21 +28,20 @@ import com.agiletec.apsadmin.ApsAdminBaseTestCase; import com.agiletec.apsadmin.system.ApsAdminSystemConstants; -import java.util.List; - -import javax.sql.DataSource; - -import com.agiletec.plugins.jpversioning.util.JpversioningTestHelper; - import com.agiletec.plugins.jacms.aps.system.JacmsSystemConstants; import com.agiletec.plugins.jacms.aps.system.services.content.IContentManager; import com.agiletec.plugins.jacms.aps.system.services.content.model.Content; -import com.agiletec.plugins.jacms.apsadmin.content.ContentActionConstants; import com.agiletec.plugins.jacms.apsadmin.content.AbstractContentAction; +import com.agiletec.plugins.jacms.apsadmin.content.ContentActionConstants; import com.agiletec.plugins.jpversioning.aps.system.JpversioningSystemConstants; import com.agiletec.plugins.jpversioning.aps.system.services.versioning.ContentVersion; import com.agiletec.plugins.jpversioning.aps.system.services.versioning.IVersioningManager; +import com.agiletec.plugins.jpversioning.aps.system.services.versioning.VersioningManager; +import com.agiletec.plugins.jpversioning.util.JpversioningTestHelper; import com.opensymphony.xwork2.Action; +import java.util.List; +import java.util.concurrent.Executor; +import javax.sql.DataSource; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -95,6 +94,9 @@ void testEntryRecover() throws Throwable { void testRecoverAction() throws Throwable { String contentId = null; try { + // force sequential execution by injecting a synchronized runnable + Executor syncExecutor = Runnable::run; + ((VersioningManager)versioningManager).setExecutor(syncExecutor); Content contentToVersion = this.contentManager.loadContent("ART187", false); contentToVersion.setId(null); for (int i = 0; i < 15; i++) { @@ -206,4 +208,4 @@ protected void dispose() throws Exception { private IContentManager contentManager; private IVersioningManager versioningManager; -} \ No newline at end of file +} diff --git a/src/test/java/org/entando/entando/jpversioning/web/content/ContentVersioningControllerIntegrationTest.java b/src/test/java/org/entando/entando/jpversioning/web/content/ContentVersioningControllerIntegrationTest.java index e76613d..e1679bb 100644 --- a/src/test/java/org/entando/entando/jpversioning/web/content/ContentVersioningControllerIntegrationTest.java +++ b/src/test/java/org/entando/entando/jpversioning/web/content/ContentVersioningControllerIntegrationTest.java @@ -25,10 +25,10 @@ import java.util.Map; import java.util.Optional; import org.entando.entando.ent.exception.EntException; +import org.entando.entando.ent.util.EntLogging.EntLogFactory; +import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.web.AbstractControllerIntegrationTest; import org.entando.entando.web.utils.OAuth2TestUtils; -import org.entando.entando.ent.util.EntLogging.EntLogger; -import org.entando.entando.ent.util.EntLogging.EntLogFactory; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -191,6 +191,7 @@ void testPaginationListContentVersion() throws Exception { contentManager.saveContent(newContent); contentManager.saveContent(newContent); + listContentVersions(user, newContentId) .andExpect(jsonPath("$.payload.size()", is(5))) .andExpect(jsonPath("$.metaData.page", is(1))) @@ -311,6 +312,7 @@ void testGetContentVersion() throws Exception { newContentId = saveContent("json/1_POST_content_with_boolean_attribute.json", accessToken); updateContent("json/1_PUT_content_with_boolean_attribute.json", newContentId, accessToken); + final ContentVersion lastVersion = versioningManager.getLastVersion(newContentId); Assertions.assertNotNull(lastVersion); final Long versionId = lastVersion.getId(); diff --git a/src/test/java/org/entando/entando/jpversioning/web/content/IFDeferredVersioningTest.java b/src/test/java/org/entando/entando/jpversioning/web/content/IFDeferredVersioningTest.java new file mode 100644 index 0000000..0ef6b0f --- /dev/null +++ b/src/test/java/org/entando/entando/jpversioning/web/content/IFDeferredVersioningTest.java @@ -0,0 +1,155 @@ +package org.entando.entando.jpversioning.web.content; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.agiletec.plugins.jpversioning.aps.system.services.versioning.IFDeferredVersioning; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; +import java.util.function.Supplier; +import org.entando.entando.web.AbstractControllerIntegrationTest; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +public class IFDeferredVersioningTest extends AbstractControllerIntegrationTest { + + + @Test + void possiblyDeferred_enabled() throws Exception { + + Executor executorMock = mock(Executor.class); + Supplier actionMock = mock(Supplier.class); + String methodName = "saveContent"; + + try (MockedStatic mock = mockStatic(IFDeferredVersioning.class)) { + mock.when(IFDeferredVersioning::checkEnabled).thenReturn(true); + mock.when(() -> IFDeferredVersioning.possiblyDeferred( + any(Executor.class), + any(Supplier.class), + anyString() + )).thenCallRealMethod(); + + IFDeferredVersioning.possiblyDeferred(executorMock, actionMock, methodName); + // the executor gets called!!! + verify(executorMock, times(1)).execute(any(Runnable.class)); + } + } + + @Test + void possiblyDeferred_disabled() { + Executor executorMock = mock(Executor.class); + Supplier actionMock = mock(Supplier.class); + String methodName = "saveContent"; + + try (MockedStatic mocked = mockStatic(IFDeferredVersioning.class)) { + // Feature flag disabled + mocked.when(IFDeferredVersioning::checkEnabled).thenReturn(false); + + mocked.when(() -> IFDeferredVersioning.possiblyDeferred( + any(Executor.class), + any(Supplier.class), + anyString()) + ).thenCallRealMethod(); + + IFDeferredVersioning.possiblyDeferred(executorMock, actionMock, methodName); + + verify(actionMock, times(1)).get(); + // Executor never gets called!!! + verify(executorMock, never()).execute(any()); + } + } + + @Test + void possiblyDeferred_exception_enabled() throws Exception { + Executor executorMock = mock(Executor.class); + Supplier actionMock = mock(Supplier.class); + String methodName = "saveContent"; + RuntimeException exception = new RuntimeException("Test Exception"); + + when(actionMock.get()).thenThrow(exception); + + // Simulo l'esecuzione immediata nel thread corrente quando viene chiamato l'executor + doAnswer(invocation -> { + Runnable runnable = invocation.getArgument(0); + runnable.run(); + return null; + }).when(executorMock).execute(any(Runnable.class)); + + try (MockedStatic mock = mockStatic(IFDeferredVersioning.class)) { + mock.when(IFDeferredVersioning::checkEnabled).thenReturn(true); + mock.when(() -> IFDeferredVersioning.possiblyDeferred( + any(Executor.class), + any(Supplier.class), + anyString() + )).thenCallRealMethod(); + + CompletableFuture result = IFDeferredVersioning.possiblyDeferred(executorMock, actionMock, methodName); + + assertNotNull(result); + assertTrue(result.isCompletedExceptionally()); + + ExecutionException ex = assertThrows(ExecutionException.class, result::get); + assertEquals(exception, ex.getCause()); + } + } + + @Test + void possiblyDeferred_exception_disabled() throws Exception { + Executor executorMock = mock(Executor.class); + Supplier actionMock = mock(Supplier.class); + String methodName = "saveContent"; + RuntimeException exception = new RuntimeException("Test Exception"); + + when(actionMock.get()).thenThrow(exception); + + try (MockedStatic mocked = mockStatic(IFDeferredVersioning.class)) { + mocked.when(IFDeferredVersioning::checkEnabled).thenReturn(false); + mocked.when(() -> IFDeferredVersioning.possiblyDeferred( + any(Executor.class), + any(Supplier.class), + anyString()) + ).thenCallRealMethod(); + + CompletableFuture result = IFDeferredVersioning.possiblyDeferred(executorMock, actionMock, methodName); + + assertNotNull(result); + assertTrue(result.isCompletedExceptionally()); + + ExecutionException ex = assertThrows(ExecutionException.class, result::get); + assertEquals(exception, ex.getCause()); + } + } + + @Test + void possiblyDeferred_should_propagate_exception_when_enabled() throws Exception { + Executor executor = Runnable::run; // Immediate execution + Supplier action = () -> { + throw new RuntimeException("Async Exception"); + }; + + try (MockedStatic mocked = mockStatic(IFDeferredVersioning.class)) { + mocked.when(IFDeferredVersioning::checkEnabled).thenReturn(true); + mocked.when(() -> IFDeferredVersioning.possiblyDeferred(any(), any(), anyString())).thenCallRealMethod(); + + CompletableFuture result = IFDeferredVersioning.possiblyDeferred(executor, action, "testMethod"); + + assertNotNull(result); + assertTrue(result.isCompletedExceptionally()); + ExecutionException ex = assertThrows(ExecutionException.class, result::get); + assertEquals("Async Exception", ex.getCause().getMessage()); + } + } + +}