[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-10344?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16413769#comment-16413769
 ] 

ASF GitHub Bot commented on CLOUDSTACK-10344:
---------------------------------------------

rafaelweingartner commented on a change in pull request #2511: 
[CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511#discussion_r177070454
 
 

 ##########
 File path: 
server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
 ##########
 @@ -922,4 +925,170 @@ public NetworkACL updateNetworkACL(final Long id, final 
String customId, final B
         return _networkACLDao.findById(id);
     }
 
+    @Override
+    public NetworkACLItem 
moveNetworkAclRuleToNewPosition(MoveNetworkAclItemCmd moveNetworkAclItemCmd) {
+        String uuidRuleBeingMoved = 
moveNetworkAclItemCmd.getUuidRuleBeingMoved();
+        String nextAclRuleUuid = moveNetworkAclItemCmd.getNextAclRuleUuid();
+        String previousAclRuleUuid = 
moveNetworkAclItemCmd.getPreviousAclRuleUuid();
+
+        if (StringUtils.isBlank(previousAclRuleUuid) && 
StringUtils.isBlank(nextAclRuleUuid)) {
+            throw new InvalidParameterValueException("Both previous and next 
ACL rule IDs cannot be blank.");
+        }
+
+        NetworkACLItemVO ruleBeingMoved = 
_networkACLItemDao.findByUuid(uuidRuleBeingMoved);
+        if (ruleBeingMoved == null) {
+            throw new InvalidParameterValueException(String.format("Could not 
find a rule with ID[%s]", uuidRuleBeingMoved));
+        }
+        NetworkACLItemVO previousRule = 
retrieveAndValidateAclRule(previousAclRuleUuid);
+        NetworkACLItemVO nextRule = 
retrieveAndValidateAclRule(nextAclRuleUuid);
+
+        validateMoveAclRulesData(ruleBeingMoved, previousRule, nextRule);
+
+        List<NetworkACLItemVO> allAclRules = 
getAllAclRulesSortedByNumber(ruleBeingMoved.getAclId());
+        if (previousRule == null) {
+            return moveRuleToTheTop(ruleBeingMoved, allAclRules);
+        }
+        if (nextRule == null) {
+            return moveRuleToTheBottom(ruleBeingMoved, allAclRules);
+        }
+
+        return moveRuleBetweenAclRules(ruleBeingMoved, allAclRules, 
previousRule, nextRule);
+    }
+
+    /**
+     * Loads all ACL rules from given network ACL list. Then, the ACL rules 
will be sorted according to the 'number' field in ascending order.
+     */
+    protected List<NetworkACLItemVO> getAllAclRulesSortedByNumber(long aclId) {
+        List<NetworkACLItemVO> allAclRules = 
_networkACLItemDao.listByACL(aclId);
+        Collections.sort(allAclRules, new Comparator<NetworkACLItemVO>() {
+            @Override
+            public int compare(NetworkACLItemVO o1, NetworkACLItemVO o2) {
+                return o1.number - o2.number;
+            }
+        });
+        return allAclRules;
+    }
+
+    /**
+     * Mover an ACL to the space between to other rules. If there is already 
enough room to accommodate the ACL rule being moved, we simply get the 'number' 
field from the previous ACL rule and add one, and then define this new value as 
the 'number' value for the ACL rule being moved.
 
 Review comment:
   Done

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Sometimes a bug happens when moving ACL rules (changing their order with drag 
> and drop) 
> ----------------------------------------------------------------------------------------
>
>                 Key: CLOUDSTACK-10344
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10344
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>            Reporter: Rafael Weingärtner
>            Assignee: Rafael Weingärtner
>            Priority: Major
>             Fix For: 4.12
>
>
> An error is happening in certain conditions, such as when you have only 2 ACL 
> rules and you move the last one to the top. There are other conditions, for 
> instance, when moving ACLs that are in a sequence of numbers without gaps.
> Example, rules:
> number | rule
> 1 - rule A
> 2 - rule D
> 3 - rule B
> 4 - rule C
> 5 - rule E
> It is not possible to move "rule C" in position 2, 1, and 3.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to