From 05da18073191f235fde99dd06371ed5c874c31ff Mon Sep 17 00:00:00 2001 From: Tom Page Date: Mon, 10 Aug 2015 13:45:44 +0000 Subject: [PATCH] RM-2500 Enable editing of classification scheme. Allow adding and removing classification levels, classification reasons and exemption categories after the server has been started for the first time. It is also possible to alter the display text associated with an existing level, reason or category. All changes require a server restart. +review RM git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@109854 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../ClassificationServiceBootstrap.java | 23 +++-- ...lassificationServiceBootstrapUnitTest.java | 84 ++++--------------- 2 files changed, 27 insertions(+), 80 deletions(-) diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrap.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrap.java index d9171ec1cd..9018507fdf 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrap.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrap.java @@ -171,24 +171,21 @@ public class ClassificationServiceBootstrap extends AbstractLifecycleBean implem validator.validate(classpathValues, clazz.getSimpleName()); - if (isEmpty(persistedValues)) + if (isEmpty(classpathValues)) { - if (isEmpty(classpathValues)) - { - throw new MissingConfiguration(clazz.getSimpleName() + " configuration is missing."); - } - attributeService.setAttribute((Serializable) classpathValues, key); - return classpathValues; + throw new MissingConfiguration(clazz.getSimpleName() + " configuration is missing."); } - else + if (classpathValues.equals(persistedValues)) { - if (isEmpty(classpathValues) || !classpathValues.equals(persistedValues)) - { - LOGGER.warn(clazz.getSimpleName() + " data configured in classpath does not match data stored in Alfresco. " - + "Alfresco will use the unchanged values stored in the database."); - } return persistedValues; } + if (!isEmpty(persistedValues)) + { + LOGGER.warn("{} configuration changed. This may result in unpredictable results if the classification scheme is already in use.", + clazz.getSimpleName()); + } + attributeService.setAttribute((Serializable) classpathValues, key); + return classpathValues; } /** diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrapUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrapUnitTest.java index 5f3af461ba..c4c4c0398c 100644 --- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrapUnitTest.java +++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceBootstrapUnitTest.java @@ -21,7 +21,6 @@ package org.alfresco.module.org_alfresco_module_rm.classification; import static java.util.Arrays.asList; import static org.apache.commons.collections.ListUtils.union; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; @@ -33,7 +32,6 @@ import static org.mockito.Mockito.when; import java.io.Serializable; import java.util.ArrayList; import java.util.List; -import java.util.stream.Stream; import com.google.common.collect.ImmutableList; import org.alfresco.module.org_alfresco_module_rm.classification.ClassificationException.MissingConfiguration; @@ -46,8 +44,6 @@ import org.alfresco.module.org_alfresco_module_rm.test.util.MockAuthenticationUt import org.alfresco.module.org_alfresco_module_rm.util.AuthenticationUtil; import org.alfresco.service.cmr.attributes.AttributeService; import org.apache.log4j.Appender; -import org.apache.log4j.Level; -import org.apache.log4j.spi.LoggingEvent; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -116,7 +112,6 @@ public class ClassificationServiceBootstrapUnitTest implements ClassifiedContent @Mock ClearanceLevelManager mockClearanceLevelManager; /** Using a mock appender in the class logger so that we can verify some of the logging requirements. */ @Mock Appender mockAppender; - @Captor ArgumentCaptor loggingEventCaptor; @Captor ArgumentCaptor> clearanceLevelCaptor; private ClassificationLevelFieldsValidator classificationLevelFieldsValidator = new ClassificationLevelFieldsValidator(); @@ -146,7 +141,7 @@ public class ClassificationServiceBootstrapUnitTest implements ClassifiedContent anyString(), anyString(), anyString()); } - @Test public void alternativeLevelsConfigurationPreviouslyStartedSystem() + @Test public void levelsConfigurationChanged() { when(mockClassificationServiceDAO.getConfiguredValues(ClassificationLevel.class)).thenReturn(ALT_CLASSIFICATION_LEVELS); when(mockAttributeService.getAttribute(anyString(), anyString(), anyString())) @@ -154,8 +149,7 @@ public class ClassificationServiceBootstrapUnitTest implements ClassifiedContent List entities = classificationServiceBootstrap.getConfiguredSchemeEntities(ClassificationLevel.class, LEVELS_KEY, classificationLevelValidator); - // TODO Check that changing the behaviour here is ok. - assertEquals(DEFAULT_CLASSIFICATION_LEVELS, entities); + assertEquals(ALT_CLASSIFICATION_LEVELS, entities); } @Test (expected=MissingConfiguration.class) @@ -192,44 +186,21 @@ public class ClassificationServiceBootstrapUnitTest implements ClassifiedContent anyString(), anyString(), anyString()); } - /** - * Check that if the reasons supplied on the classpath differ from those already persisted then a warning is logged - * and no change is made to the persisted reasons. - *

- * This test uses the underlying log4j implementation to insert a mock Appender, and tests this for the warning - * message. If the underlying logging framework is changed then this unit test will fail, and it may not be - * possible to/worth fixing. - */ - @Test public void previouslyStartedSystemShouldWarnIfConfiguredReasonsHaveChanged() + /** Check that if the reasons supplied on the classpath differ from those already persisted then they are changed. */ + @Test + public void changeConfiguredReasons() { // The classification reasons stored are different from those found on the classpath. when(mockAttributeService.getAttribute(anyString(), anyString(), anyString())).thenReturn( (Serializable) PLACEHOLDER_CLASSIFICATION_REASONS); when(mockClassificationServiceDAO.getConfiguredValues(ClassificationReason.class)).thenReturn(ALTERNATIVE_CLASSIFICATION_REASONS); - // Put the mock Appender into the log4j logger and allow warning messages to be received. - org.apache.log4j.Logger log4jLogger = org.apache.log4j.Logger.getLogger(ClassificationServiceBootstrap.class); - log4jLogger.addAppender(mockAppender); - Level normalLevel = log4jLogger.getLevel(); - log4jLogger.setLevel(Level.WARN); - // Call the method under test. - classificationServiceBootstrap.getConfiguredSchemeEntities(ClassificationReason.class, REASONS_KEY, classificationReasonValidator); + List actual = classificationServiceBootstrap.getConfiguredSchemeEntities(ClassificationReason.class, REASONS_KEY, classificationReasonValidator); - // Reset the logging level for other tests. - log4jLogger.setLevel(normalLevel); - - // Check the persisted values weren't changed. - verify(mockAttributeService, never()).setAttribute(any(Serializable.class), - anyString(), anyString(), anyString()); - - // Check that the warning message was logged. - verify(mockAppender).doAppend(loggingEventCaptor.capture()); - List loggingEvents = loggingEventCaptor.getAllValues(); - Stream messages = loggingEvents.stream() - .map(LoggingEvent::getRenderedMessage); - String expectedMessage = "ClassificationReason data configured in classpath does not match data stored in Alfresco. Alfresco will use the unchanged values stored in the database."; - assertTrue("Warning message not found in log.", messages.anyMatch(message -> expectedMessage.equals(message))); + assertEquals(ALTERNATIVE_CLASSIFICATION_REASONS, actual); + // Check the persisted values were changed. + verify(mockAttributeService).setAttribute((Serializable) ALTERNATIVE_CLASSIFICATION_REASONS, REASONS_KEY); } @Test(expected = MissingConfiguration.class) @@ -267,20 +238,16 @@ public class ClassificationServiceBootstrapUnitTest implements ClassifiedContent } /** - * Check that if the exemption categories supplied on the classpath differ from those already persisted then a - * warning is logged and no change is made to the persisted categories. + * Check that if the exemption categories supplied on the classpath differ from those already persisted then the new + * values are persisted. *

- * RM-2322

+     * RM-2337
      * Given that I have already started the system once
      * When I stop the system
      * And modify the exemption categories
      * And restart the system
-     * Then I am warned that the exemption categories have been changed
-     * And they are not altered
+     * Then the new exemption categories are loaded.
      * 

-     * This test uses the underlying log4j implementation to insert a mock Appender, and tests this for the warning
-     * message. If the underlying logging framework is changed then this unit test will fail, and it may not be possible
-     * to/worth fixing.
      */
     @Test
     public void testInitConfiguredExemptionCategories_changedCategories()
@@ -289,29 +256,12 @@ public class ClassificationServiceBootstrapUnitTest implements ClassifiedContent
                     .thenReturn((Serializable) EXEMPTION_CATEGORIES);
         when(mockClassificationServiceDAO.getConfiguredValues(ExemptionCategory.class)).thenReturn(CHANGED_EXEMPTION_CATEGORIES);
 
-        // Put the mock Appender into the log4j logger and allow warning messages to be received.
-        org.apache.log4j.Logger log4jLogger = org.apache.log4j.Logger.getLogger(ClassificationServiceBootstrap.class);
-        log4jLogger.addAppender(mockAppender);
-        Level normalLevel = log4jLogger.getLevel();
-        log4jLogger.setLevel(Level.WARN);
-
         // Call the method under test.
-        classificationServiceBootstrap.getConfiguredSchemeEntities(ExemptionCategory.class, EXEMPTION_CATEGORIES_KEY, exemptionCategoryValidator);
+        List actual = classificationServiceBootstrap.getConfiguredSchemeEntities(ExemptionCategory.class, EXEMPTION_CATEGORIES_KEY, exemptionCategoryValidator);
 
-        // Reset the logging level for other tests.
-        log4jLogger.setLevel(normalLevel);
-
-        // Check the persisted values weren't changed.
-        verify(mockAttributeService, never()).setAttribute(any(Serializable.class),
-                anyString(), anyString(), anyString());
-
-        // Check that the warning message was logged.
-        verify(mockAppender).doAppend(loggingEventCaptor.capture());
-        List loggingEvents = loggingEventCaptor.getAllValues();
-        Stream messages = loggingEvents.stream()
-                                               .map(LoggingEvent::getRenderedMessage);
-        String expectedMessage = "ExemptionCategory data configured in classpath does not match data stored in Alfresco. Alfresco will use the unchanged values stored in the database.";
-        assertTrue("Warning message not found in log.", messages.anyMatch(message -> expectedMessage.equals(message)));
+        assertEquals(CHANGED_EXEMPTION_CATEGORIES, actual);
+        // Check the persisted values were changed.
+        verify(mockAttributeService).setAttribute((Serializable) CHANGED_EXEMPTION_CATEGORIES, EXEMPTION_CATEGORIES_KEY);
     }
 
     @Test(expected = MissingConfiguration.class)