From dbc57451edb388eb54ef216ad6aff50173dbb903 Mon Sep 17 00:00:00 2001 From: Tom Page Date: Fri, 5 Jan 2018 15:43:49 +0000 Subject: [PATCH] Fix some issues raised by Sonar. Avoid NPE if calculateListOfEmptyFolders returns null in ScheduleXRecordLoaders. Fix equals method of a few classes to check against the other instance. Make synchonisation consistent in AppliedSourceServiceImpl and also remove a redundant null check. Use Arrays.toString to make a more readable string representation of an array. Combine a few if statement branches that do the same thing. --- .../RecordsManagementAdminServiceImpl.java | 12 +++----- .../capability/RMAfterInvocationProvider.java | 28 ++++++++----------- .../MayBeScheduledCapabilityCondition.java | 13 ++------- .../property/DispositionProperty.java | 20 ++----------- .../model/rma/type/RecordFolderType.java | 21 ++++---------- 5 files changed, 25 insertions(+), 69 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/admin/RecordsManagementAdminServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/admin/RecordsManagementAdminServiceImpl.java index 62a90bbc74..c37234c425 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/admin/RecordsManagementAdminServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/admin/RecordsManagementAdminServiceImpl.java @@ -876,15 +876,11 @@ public class RecordsManagementAdminServiceImpl extends RecordsManagementAdminBas } String lovConstraintQNameAsString = newLovConstraint.toPrefixString(getNamespaceService()); - // Add the constraint - if it isn't already there. - String refOfExistingConstraint = null; + // Add the constraint - if it isn't already there (there should only be one constraint). + String refOfExistingConstraint = (targetProp.getConstraints().isEmpty() ? + null : + targetProp.getConstraints().get(0).getRef()); - for (M2Constraint c : targetProp.getConstraints()) - { - // There should only be one constraint. - refOfExistingConstraint = c.getRef(); - break; - } if (refOfExistingConstraint != null) { targetProp.removeConstraintRef(refOfExistingConstraint); diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java index 5bf6e0cbfc..30dc3dfca3 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMAfterInvocationProvider.java @@ -37,6 +37,13 @@ import java.util.Map; import java.util.Set; import java.util.StringTokenizer; +import net.sf.acegisecurity.AccessDeniedException; +import net.sf.acegisecurity.Authentication; +import net.sf.acegisecurity.ConfigAttribute; +import net.sf.acegisecurity.ConfigAttributeDefinition; +import net.sf.acegisecurity.afterinvocation.AfterInvocationProvider; +import net.sf.acegisecurity.vote.AccessDecisionVoter; + import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; import org.alfresco.repo.search.SimpleResultSetMetaData; @@ -62,13 +69,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.InitializingBean; -import net.sf.acegisecurity.AccessDeniedException; -import net.sf.acegisecurity.Authentication; -import net.sf.acegisecurity.ConfigAttribute; -import net.sf.acegisecurity.ConfigAttributeDefinition; -import net.sf.acegisecurity.afterinvocation.AfterInvocationProvider; -import net.sf.acegisecurity.vote.AccessDecisionVoter; - /** * RM After Invocation Provider */ @@ -285,11 +285,8 @@ public class RMAfterInvocationProvider extends RMSecurityCommon { for (ConfigAttributeDefintion cad : supportedDefinitions) { - if (cad.parent && parentResult == AccessDecisionVoter.ACCESS_DENIED) - { - throw new AccessDeniedException("Access Denied"); - } - else if (!cad.parent && childResult == AccessDecisionVoter.ACCESS_DENIED) + if ((cad.parent && parentResult == AccessDecisionVoter.ACCESS_DENIED) + || (!cad.parent && childResult == AccessDecisionVoter.ACCESS_DENIED)) { throw new AccessDeniedException("Access Denied"); } @@ -358,11 +355,8 @@ public class RMAfterInvocationProvider extends RMSecurityCommon continue; } - if (cad.parent && parentReadCheck != AccessDecisionVoter.ACCESS_GRANTED) - { - throw new AccessDeniedException("Access Denied"); - } - else if (childReadCheck != AccessDecisionVoter.ACCESS_GRANTED) + if ((cad.parent && parentReadCheck != AccessDecisionVoter.ACCESS_GRANTED) + || (childReadCheck != AccessDecisionVoter.ACCESS_GRANTED)) { throw new AccessDeniedException("Access Denied"); } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/declarative/condition/MayBeScheduledCapabilityCondition.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/declarative/condition/MayBeScheduledCapabilityCondition.java index efaf39ca8b..a844db8efb 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/declarative/condition/MayBeScheduledCapabilityCondition.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/capability/declarative/condition/MayBeScheduledCapabilityCondition.java @@ -94,17 +94,8 @@ public class MayBeScheduledCapabilityCondition extends AbstractCapabilityConditi */ private boolean checkDispositionLevel(NodeRef nodeRef, DispositionSchedule dispositionSchedule) { - boolean result = false; boolean isRecordLevelDisposition = dispositionSchedule.isRecordLevelDisposition(); - if (recordService.isRecord(nodeRef) && isRecordLevelDisposition) - { - result = true; - } - else if (recordFolderService.isRecordFolder(nodeRef) && !isRecordLevelDisposition) - - { - result = true; - } - return result; + return (recordService.isRecord(nodeRef) && isRecordLevelDisposition) + || (recordFolderService.isRecordFolder(nodeRef) && !isRecordLevelDisposition); } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/disposition/property/DispositionProperty.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/disposition/property/DispositionProperty.java index 9f5b70b8eb..880872110f 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/disposition/property/DispositionProperty.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/disposition/property/DispositionProperty.java @@ -258,25 +258,11 @@ public class DispositionProperty extends BaseBehaviourBean */ private boolean isPropertyUpdated(Map before, Map after) { - boolean result = false; - Serializable beforeValue = before.get(propertyName); Serializable afterValue = after.get(propertyName); - if (beforeValue == null && afterValue != null) - { - result = true; - } - else if (beforeValue != null && afterValue == null) - { - result = true; - } - else if (beforeValue != null && afterValue != null && - !beforeValue.equals(afterValue)) - { - result = true; - } - - return result; + return ((beforeValue == null && afterValue != null) + || (beforeValue != null && afterValue == null) + || (beforeValue != null && afterValue != null && !beforeValue.equals(afterValue))); } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/RecordFolderType.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/RecordFolderType.java index 242416567a..c6100cfe58 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/RecordFolderType.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/type/RecordFolderType.java @@ -27,12 +27,9 @@ package org.alfresco.module.org_alfresco_module_rm.model.rma.type; -import static org.alfresco.module.org_alfresco_module_rm.record.RecordUtils.generateRecordIdentifier; - import java.io.Serializable; import java.util.Map; -import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.identifier.IdentifierService; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; @@ -203,18 +200,10 @@ public class RecordFolderType extends AbstractDisposableItem @Override public boolean getMustCopy(QName classQName, CopyDetails copyDetails) { - boolean result = true; - - if (nodeService.getType(copyDetails.getTargetParentNodeRef()).equals(TYPE_RECORD_FOLDER)) - { - result = false; - } - else if (ArrayUtils.contains(unwantedAspects, classQName)) - { - result = false; - } - - return result; + boolean targetParentIsRecordFolder = nodeService.getType(copyDetails.getTargetParentNodeRef()) + .equals(TYPE_RECORD_FOLDER); + boolean containsUnwantedAspect = ArrayUtils.contains(unwantedAspects, classQName); + return !(targetParentIsRecordFolder || containsUnwantedAspect); } }; } @@ -253,7 +242,7 @@ public class RecordFolderType extends AbstractDisposableItem } } - /** + /** * On transaction commit * * @see org.alfresco.repo.node.NodeServicePolicies.OnCreateChildAssociationPolicy#onCreateChildAssociation(org.alfresco.service.cmr.repository.ChildAssociationRef, boolean)