From 0cfc1a98bf2a77804f13e4aedadde2d77fc332bc Mon Sep 17 00:00:00 2001 From: Neil McErlean Date: Tue, 24 Mar 2015 21:15:22 +0000 Subject: [PATCH] First updates following code review on RM-1945 & RM-1946. Thanks Roy & Tom! Renamed a public service method to getClassificationLevels. Removed a redundant test method. Code tidying. Various internal renames to help readability. Slight javadoc improvements. Also some trivial changes like fixing typos and copyright years etc. Removing warnings from within AuthenticationUtil blocks allows IntelliJ to fold them to Java 8 closure format. (fistpump). git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@100020 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../org_alfresco_module_rm/module-context.xml | 2 +- .../rm-classified-records-context.xml | 2 +- .../classification/ClassificationService.java | 6 ++-- .../ClassificationServiceImpl.java | 30 ++++++++-------- .../classification/Configuration.java | 7 ++-- .../ClassificationServiceImplUnitTest.java | 35 ++++--------------- 6 files changed, 33 insertions(+), 49 deletions(-) diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/module-context.xml b/rm-server/config/alfresco/module/org_alfresco_module_rm/module-context.xml index 220ea33aea..c5d91d36dd 100644 --- a/rm-server/config/alfresco/module/org_alfresco_module_rm/module-context.xml +++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/module-context.xml @@ -46,7 +46,7 @@ - + diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-classified-records-context.xml b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-classified-records-context.xml index 3784b28d00..88a727cdc9 100644 --- a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-classified-records-context.xml +++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-classified-records-context.xml @@ -42,7 +42,7 @@ diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationService.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationService.java index 52b86c0521..3e93c95943 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationService.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationService.java @@ -32,7 +32,9 @@ public interface ClassificationService { /** * Returns an immutable list of the defined classification levels. - * @return classification levels in descending order from highest to lowest. + * @return classification levels in descending order from highest to lowest + * (where fewer users have access to the highest classification levels + * and therefore access to the most restricted documents). */ - public List getApplicableLevels(); + public List getClassificationLevels(); } diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java index affc7b89cc..bef1fc9b4f 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImpl.java @@ -38,9 +38,9 @@ public class ClassificationServiceImpl extends ServiceBaseImpl { private static Log logger = LogFactory.getLog(ClassificationServiceImpl.class); - private static final String[] ATTRIBUTE_KEYS = new String[] { "org.alfresco", - "module.org_alfresco_module_rm", - "classification.levels" }; + private static final String[] LEVELS_KEY = new String[] { "org.alfresco", + "module.org_alfresco_module_rm", + "classification.levels" }; public static final String DEFAULT_CONFIG_LOCATION = "/alfresco/module/org_alfresco_module_rm/classification/rm-classification-levels.json"; @@ -56,24 +56,24 @@ public class ClassificationServiceImpl extends ServiceBaseImpl void initConfiguredClassificationLevels() { - final List allPersistedLevels = getPersistedLevels(); - final List configuredLevels = getConfiguredLevels(); + final List allPersistedLevels = getPersistedLevels(); + final List configurationLevels = getConfigurationLevels(); if (logger.isDebugEnabled()) { // Note! We cannot log the level names or even the size of these lists for security reasons. logger.debug("Persisted classification levels: " + loggableStatusOf(allPersistedLevels)); - logger.debug("Configured classification levels: " + loggableStatusOf(configuredLevels)); + logger.debug("Classpath classification levels: " + loggableStatusOf(configurationLevels)); } - if (configuredLevels == null || configuredLevels.isEmpty()) + if (configurationLevels == null || configurationLevels.isEmpty()) { throw new MissingConfiguration("Classification level configuration is missing."); } - else if ( !configuredLevels.equals(allPersistedLevels)) + else if ( !configurationLevels.equals(allPersistedLevels)) { - attributeService.setAttribute((Serializable) configuredLevels, ATTRIBUTE_KEYS); - this.configuredLevels = configuredLevels; + attributeService.setAttribute((Serializable) configurationLevels, LEVELS_KEY); + this.configuredLevels = configurationLevels; } else { @@ -96,21 +96,23 @@ public class ClassificationServiceImpl extends ServiceBaseImpl List getPersistedLevels() { return authenticationUtil.runAsSystem(new AuthenticationUtil.RunAsWork>() { - @Override public List doWork() throws Exception + @Override + @SuppressWarnings("unchecked") + public List doWork() throws Exception { - return (List) attributeService.getAttribute(ATTRIBUTE_KEYS); + return (List) attributeService.getAttribute(LEVELS_KEY); } }); } /** Gets the list (in descending order) of classification levels - as defined in the system configuration. */ - List getConfiguredLevels() + List getConfigurationLevels() { return config.getConfiguredLevels(); } @Override - public List getApplicableLevels() + public List getClassificationLevels() { return configuredLevels == null ? Collections.emptyList() : Collections.unmodifiableList(configuredLevels); diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/Configuration.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/Configuration.java index 4906b2898b..f048805e8c 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/Configuration.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/Configuration.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2014 Alfresco Software Limited. + * Copyright (C) 2005-2015 Alfresco Software Limited. * * This file is part of Alfresco * @@ -57,7 +57,7 @@ class Configuration List result; try (final InputStream in = this.getClass().getResourceAsStream(configLocation)) { - if ( in == null) { result = Collections.emptyList(); } + if (in == null) { result = Collections.emptyList(); } else { final String jsonString = IOUtils.toString(in); @@ -65,7 +65,8 @@ class Configuration result = new ArrayList<>(jsonArray.length()); - for (int i = 0; i < jsonArray.length(); i++) { + for (int i = 0; i < jsonArray.length(); i++) + { final JSONObject nextObj = jsonArray.getJSONObject(i); final String name = nextObj.getString("name"); final String displayLabelKey = nextObj.getString("displayLabel"); diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java index d9b3e85f1b..b31eb83806 100644 --- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java +++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/ClassificationServiceImplUnitTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2014 Alfresco Software Limited. + * Copyright (C) 2005-2015 Alfresco Software Limited. * * This file is part of Alfresco * @@ -31,7 +31,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; - /** * Unit tests for {@link ClassificationServiceImpl}. * @@ -63,21 +62,11 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest ClassificationLevel.class.getSimpleName(), idsAndLabels.length)); } - final int resultLength = idsAndLabels.length / 2; - final String[] ids = new String[resultLength]; - final String[] labels = new String[resultLength]; + final List levels = new ArrayList<>(idsAndLabels.length / 2); - for (int i = 0; i < idsAndLabels.length; i++) + for (int i = 0; i < idsAndLabels.length; i += 2) { - if (i % 2 == 0) { ids[i / 2] = idsAndLabels[i]; } - else { labels[(i - 1) / 2] = idsAndLabels[i]; } - } - - final List levels = new ArrayList<>(resultLength); - - for (int i = 0; i < resultLength; i++) - { - levels.add(new ClassificationLevel(ids[i], labels[i])); + levels.add(new ClassificationLevel(idsAndLabels[i], idsAndLabels[i+1])); } return levels; @@ -94,17 +83,7 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest classificationService.initConfiguredClassificationLevels(); - assertEquals(DEFAULT_CLASSIFICATION_LEVELS, classificationService.getApplicableLevels()); - } - - @Test public void alternativeConfigurationVanillaSystem() - { - classificationService = new TestClassificationService(null, ALT_CLASSIFICATION_LEVELS); - classificationService.setAttributeService(mockedAttributeService); - - classificationService.initConfiguredClassificationLevels(); - - assertEquals(ALT_CLASSIFICATION_LEVELS, classificationService.getApplicableLevels()); + assertEquals(DEFAULT_CLASSIFICATION_LEVELS, classificationService.getClassificationLevels()); } @Test public void alternativeConfigurationPreviouslyStartedSystem() @@ -114,7 +93,7 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest classificationService.initConfiguredClassificationLevels(); - assertEquals(ALT_CLASSIFICATION_LEVELS, classificationService.getApplicableLevels()); + assertEquals(ALT_CLASSIFICATION_LEVELS, classificationService.getClassificationLevels()); } @Test (expected=MissingConfiguration.class) @@ -141,6 +120,6 @@ public class ClassificationServiceImplUnitTest extends BaseUnitTest } @Override List getPersistedLevels() { return persisted; } - @Override List getConfiguredLevels() { return configured; } + @Override List getConfigurationLevels() { return configured; } } }