[
https://issues.apache.org/jira/browse/CLOUDSTACK-10344?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16439325#comment-16439325
]
ASF GitHub Bot commented on CLOUDSTACK-10344:
---------------------------------------------
DaanHoogland closed pull request #2511: [CLOUDSTACK-10344] bug when moving ACL
rules (change order with drag and drop)
URL: https://github.com/apache/cloudstack/pull/2511
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java
b/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java
index 378b15ce940..7c4e8b45333 100644
--- a/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java
+++ b/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java
@@ -21,6 +21,7 @@
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
+import org.apache.cloudstack.api.command.user.network.MoveNetworkAclItemCmd;
import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLListCmd;
@@ -28,6 +29,7 @@
import com.cloud.utils.Pair;
public interface NetworkACLService {
+
/**
* Creates Network ACL for the specified VPC
*/
@@ -90,4 +92,8 @@
NetworkACL updateNetworkACL(UpdateNetworkACLListCmd
updateNetworkACLListCmd);
-}
+ /**
+ * Updates a network item ACL to a new position. This method allows users
to inform between which ACLs the given ACL will be placed. Therefore, the
'number' field will be filled out by the system in the best way possible to
place the ACL accordingly.
+ */
+ NetworkACLItem moveNetworkAclRuleToNewPosition(MoveNetworkAclItemCmd
moveNetworkAclItemCmd);
+}
\ No newline at end of file
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index 7825bb4fc93..56d5957f022 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -152,6 +152,8 @@
public static final String ICMP_TYPE = "icmptype";
public static final String ID = "id";
public static final String IDS = "ids";
+ public static final String PREVIOUS_ACL_RULE_ID = "previousaclruleid";
+ public static final String NEXT_ACL_RULE_ID = "nextaclruleid";
public static final String INTERNAL_DNS1 = "internaldns1";
public static final String INTERNAL_DNS2 = "internaldns2";
public static final String INTERVAL_TYPE = "intervaltype";
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/network/MoveNetworkAclItemCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/network/MoveNetworkAclItemCmd.java
new file mode 100644
index 00000000000..aaa9c185526
--- /dev/null
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/network/MoveNetworkAclItemCmd.java
@@ -0,0 +1,96 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.network;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseAsyncCustomIdCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.NetworkACLItemResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.event.EventTypes;
+import com.cloud.network.vpc.NetworkACLItem;
+import com.cloud.user.Account;
+
+@APICommand(name = "moveNetworkAclItem", description = "Move an ACL rule to a
position bettwen two other ACL rules of the same ACL network list",
responseObject = NetworkACLItemResponse.class, requestHasSensitiveInfo = false,
responseHasSensitiveInfo = false)
+public class MoveNetworkAclItemCmd extends BaseAsyncCustomIdCmd {
+
+ public static final Logger s_logger =
Logger.getLogger(MoveNetworkAclItemCmd.class.getName());
+ private static final String s_name = "moveNetworkAclItemResponse";
+
+ @Parameter(name = ApiConstants.ID, type = CommandType.STRING, required =
true, description = "The ID of the network ACL rule that is being moved to a
new position.")
+ private String uuidRuleBeingMoved;
+
+ @Parameter(name = ApiConstants.PREVIOUS_ACL_RULE_ID, type =
CommandType.STRING, description = "The ID of the first rule that is right
before the new position where the rule being moved is going to be placed. This
value can be 'NULL' if the rule is being moved to the first position of the
network ACL list.")
+ private String previousAclRuleUuid;
+
+ @Parameter(name = ApiConstants.NEXT_ACL_RULE_ID, type =
CommandType.STRING, description = "The ID of the rule that is right after the
new position where the rule being moved is going to be placed. This value can
be 'NULL' if the rule is being moved to the last position of the network ACL
list.")
+ private String nextAclRuleUuid;
+
+ @Override
+ public void execute() {
+ CallContext.current().setEventDetails(getEventDescription());
+
+ NetworkACLItem aclItem =
_networkACLService.moveNetworkAclRuleToNewPosition(this);
+
+ NetworkACLItemResponse aclResponse =
_responseGenerator.createNetworkACLItemResponse(aclItem);
+ setResponseObject(aclResponse);
+ aclResponse.setResponseName(getCommandName());
+ }
+
+ public String getUuidRuleBeingMoved() {
+ return uuidRuleBeingMoved;
+ }
+
+ public String getPreviousAclRuleUuid() {
+ return previousAclRuleUuid;
+ }
+
+ public String getNextAclRuleUuid() {
+ return nextAclRuleUuid;
+ }
+
+ @Override
+ public void checkUuid() {
+ if (this.getCustomId() != null) {
+ _uuidMgr.checkUuid(this.getCustomId(), NetworkACLItem.class);
+ }
+ }
+
+ @Override
+ public String getEventType() {
+ return EventTypes.EVENT_NETWORK_ACL_ITEM_UPDATE;
+ }
+
+ @Override
+ public String getEventDescription() {
+ return String.format("Placing network ACL item [%s] between [%s] and
[%s].", uuidRuleBeingMoved, previousAclRuleUuid, nextAclRuleUuid);
+ }
+
+ @Override
+ public String getCommandName() {
+ return s_name;
+ }
+
+ @Override
+ public long getEntityOwnerId() {
+ Account caller = CallContext.current().getCallingAccount();
+ return caller.getAccountId();
+ }
+}
diff --git
a/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemDao.java
b/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemDao.java
index 21794cba649..59031724bcb 100644
--- a/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemDao.java
+++ b/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemDao.java
@@ -20,7 +20,7 @@
import com.cloud.utils.db.GenericDao;
-/*
+/**
* Data Access Object for network_acl_item table
*/
public interface NetworkACLItemDao extends GenericDao<NetworkACLItemVO, Long> {
@@ -36,4 +36,12 @@
NetworkACLItemVO findByAclAndNumber(long aclId, int number);
void loadCidrs(NetworkACLItemVO item);
+
+ /**
+ * Updated the network ACL item 'number' field.
+ *
+ * @param networkItemId is the ID of the network ACL rule that will have
its 'number' field updated.
+ * @param newNumberValue is the new value that will be assigned to the
'number' field.
+ */
+ void updateNumberFieldNetworkItem(long networkItemId, int newNumberValue);
}
diff --git
a/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemCidrsDaoImpl.java
b/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemCidrsDaoImpl.java
index 395c9b20f80..4501f1493b5 100644
---
a/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemCidrsDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemCidrsDaoImpl.java
@@ -21,7 +21,6 @@
import java.util.ArrayList;
import java.util.List;
-
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
diff --git
a/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java
b/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java
index eb957a944ba..b88cc0d05f9 100644
---
a/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java
@@ -16,12 +16,12 @@
// under the License.
package com.cloud.network.vpc.dao;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
import java.util.List;
import javax.inject.Inject;
-import com.google.common.collect.Lists;
-import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
import com.cloud.network.vpc.NetworkACLItem.State;
@@ -35,11 +35,12 @@
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.SearchCriteria.Op;
import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.collect.Lists;
-@Component
@DB()
+@Component
public class NetworkACLItemDaoImpl extends GenericDaoBase<NetworkACLItemVO,
Long> implements NetworkACLItemDao {
- private static final Logger s_logger =
Logger.getLogger(NetworkACLItemDaoImpl.class);
protected final SearchBuilder<NetworkACLItemVO> AllFieldsSearch;
protected final SearchBuilder<NetworkACLItemVO> NotRevokedSearch;
@@ -115,12 +116,14 @@ public boolean revoke(NetworkACLItemVO rule) {
@Override
public List<NetworkACLItemVO> listByACL(Long aclId) {
- if (aclId == null) return Lists.newArrayList();
+ if (aclId == null) {
+ return Lists.newArrayList();
+ }
SearchCriteria<NetworkACLItemVO> sc = AllFieldsSearch.create();
sc.setParameters("aclId", aclId);
List<NetworkACLItemVO> list = listBy(sc);
- for(NetworkACLItemVO item :list) {
+ for (NetworkACLItemVO item : list) {
loadCidrs(item);
}
return list;
@@ -140,7 +143,7 @@ public NetworkACLItemVO findByAclAndNumber(long aclId, int
number) {
sc.setParameters("aclId", aclId);
sc.setParameters("number", number);
NetworkACLItemVO vo = findOneBy(sc);
- if(vo != null) {
+ if (vo != null) {
loadCidrs(vo);
}
return vo;
@@ -172,4 +175,19 @@ public void loadCidrs(NetworkACLItemVO item) {
List<String> cidrs = _networkACLItemCidrsDao.getCidrs(item.getId());
item.setSourceCidrList(cidrs);
}
-}
+
+ private String sqlUpdateNumberFieldNetworkItem = "UPDATE network_acl_item
SET number = ? where id =?";
+
+ @Override
+ public void updateNumberFieldNetworkItem(long networkItemId, int
newNumberValue) {
+ try (TransactionLegacy txn = TransactionLegacy.currentTxn();
+ PreparedStatement pstmt =
txn.prepareAutoCloseStatement(sqlUpdateNumberFieldNetworkItem)) {
+ pstmt.setLong(1, newNumberValue);
+ pstmt.setLong(2, networkItemId);
+ pstmt.executeUpdate();
+ txn.commit();
+ } catch (SQLException e) {
+ throw new CloudRuntimeException(e);
+ }
+ }
+}
\ No newline at end of file
diff --git
a/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql
b/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql
index 79ecbfc5a06..d5e6d61ea71 100644
--- a/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql
+++ b/engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql
@@ -26,4 +26,10 @@ ALTER TABLE `cloud`.`network_acl_item` ADD COLUMN `reason`
VARCHAR(2500) AFTER `
ALTER TABLE `cloud`.`alert` ADD COLUMN `content` VARCHAR(5000);
-- Fix the name of the column used to hold IPv4 range in 'vlan' table.
-ALTER TABLE `vlan` CHANGE `description` `ip4_range` varchar(255);
\ No newline at end of file
+ALTER TABLE `vlan` CHANGE `description` `ip4_range` varchar(255);
+
+-- [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and
drop)
+-- We are only adding the permission to the default rules. Any custom rule
must be configured by the root admin.
+INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`,
`permission`, `sort_order`) values (UUID(), 2, 'moveNetworkAclItem', 'ALLOW',
100) ON DUPLICATE KEY UPDATE rule=rule;
+INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`,
`permission`, `sort_order`) values (UUID(), 3, 'moveNetworkAclItem', 'ALLOW',
302) ON DUPLICATE KEY UPDATE rule=rule;
+INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`,
`permission`, `sort_order`) values (UUID(), 4, 'moveNetworkAclItem', 'ALLOW',
260) ON DUPLICATE KEY UPDATE rule=rule;
\ No newline at end of file
diff --git
a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
index 18d3f8da121..674910d8d1e 100644
--- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
+++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
@@ -17,6 +17,8 @@
package com.cloud.network.vpc;
import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
import java.util.List;
import java.util.Map;
@@ -27,6 +29,7 @@
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
+import org.apache.cloudstack.api.command.user.network.MoveNetworkAclItemCmd;
import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLListCmd;
import org.apache.cloudstack.context.CallContext;
@@ -936,4 +939,170 @@ public NetworkACL
updateNetworkACL(UpdateNetworkACLListCmd updateNetworkACLListC
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;
+ }
+
+ /**
+ * Moves 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.
+ * Otherwise, we will need to make room. This process is executed via
{@link #updateAclRuleToNewPositionAndExecuteShiftIfNecessary(NetworkACLItemVO,
int, List, int)}, which will create the space between ACL rules if necessary.
This involves shifting ACL rules to accommodate the rule being moved.
+ */
+ protected NetworkACLItem moveRuleBetweenAclRules(NetworkACLItemVO
ruleBeingMoved, List<NetworkACLItemVO> allAclRules, NetworkACLItemVO
previousRule, NetworkACLItemVO nextRule) {
+ if (previousRule.getNumber() + 1 != nextRule.getNumber()) {
+ int newNumberFieldValue = previousRule.getNumber() + 1;
+ for (NetworkACLItemVO networkACLItemVO : allAclRules) {
+ if (networkACLItemVO.getNumber() == newNumberFieldValue) {
+ throw new InvalidParameterValueException("There are some
inconsistencies with the data you sent. The new position calculated already has
a ACL rule on it.");
+ }
+ }
+ ruleBeingMoved.setNumber(newNumberFieldValue);
+
_networkACLItemDao.updateNumberFieldNetworkItem(ruleBeingMoved.getId(),
newNumberFieldValue);
+ return _networkACLItemDao.findById(ruleBeingMoved.getId());
+ }
+ int positionToStartProcessing = 0;
+ for (int i = 0; i < allAclRules.size(); i++) {
+ if (allAclRules.get(i).getId() == previousRule.getId()) {
+ positionToStartProcessing = i + 1;
+ break;
+ }
+ }
+ return
updateAclRuleToNewPositionAndExecuteShiftIfNecessary(ruleBeingMoved,
previousRule.getNumber() + 1, allAclRules, positionToStartProcessing);
+ }
+
+ /**
+ * Moves a network ACL rule to the bottom of the list. This is executed
by getting the 'number' field of the last ACL rule from the ACL list, and
incrementing one.
+ * This new value is assigned to the network ACL being moved and updated
in the database using {@link
NetworkACLItemDao#updateNumberFieldNetworkItem(long, int)}.
+ */
+ protected NetworkACLItem moveRuleToTheBottom(NetworkACLItemVO
ruleBeingMoved, List<NetworkACLItemVO> allAclRules) {
+ NetworkACLItemVO lastAclRule = allAclRules.get(allAclRules.size() - 1);
+
+ int newNumberFieldValue = lastAclRule.getNumber() + 1;
+ ruleBeingMoved.setNumber(newNumberFieldValue);
+
+
_networkACLItemDao.updateNumberFieldNetworkItem(ruleBeingMoved.getId(),
newNumberFieldValue);
+ return _networkACLItemDao.findById(ruleBeingMoved.getId());
+ }
+
+ /**
+ * Move the rule to the top of the ACL rule list. This means that the ACL
rule being moved will receive the position '1'.
+ * Also, if necessary other ACL rules will have their 'number' field
updated to create room for the new top rule.
+ */
+ protected NetworkACLItem moveRuleToTheTop(NetworkACLItemVO ruleBeingMoved,
List<NetworkACLItemVO> allAclRules) {
+ return
updateAclRuleToNewPositionAndExecuteShiftIfNecessary(ruleBeingMoved, 1,
allAclRules, 0);
+ }
+
+ /**
+ * Updates the ACL rule number executing the shift on subsequent ACL rules
if necessary.
+ * For example, if we have the following ACL rules:
+ * <ul>
+ * <li> ACL A - number 1
+ * <li> ACL B - number 2
+ * <li> ACL C - number 3
+ * <li> ACL D - number 12
+ * </ul>
+ * If we move 'ACL D' to a place between 'ACL A' and 'ACL B', this method
will execute the shift needded to create the space for 'ACL D'.
+ * After applying this method, we will have the following condition.
+ * <ul>
+ * <li> ACL A - number 1
+ * <li> ACL D - number 2
+ * <li> ACL B - number 3
+ * <li> ACL C - number 4
+ * </ul>
+ */
+ protected NetworkACLItem
updateAclRuleToNewPositionAndExecuteShiftIfNecessary(NetworkACLItemVO
ruleBeingMoved, int newNumberFieldValue, List<NetworkACLItemVO> allAclRules,
+ int indexToStartProcessing) {
+ ruleBeingMoved.setNumber(newNumberFieldValue);
+ for (int i = indexToStartProcessing; i < allAclRules.size(); i++) {
+ NetworkACLItemVO networkACLItemVO = allAclRules.get(i);
+ if (networkACLItemVO.getId() == ruleBeingMoved.getId()) {
+ continue;
+ }
+ if (newNumberFieldValue != networkACLItemVO.getNumber()) {
+ break;
+ }
+ int newNumberFieldValueNextAclRule = newNumberFieldValue + 1;
+
updateAclRuleToNewPositionAndExecuteShiftIfNecessary(networkACLItemVO,
newNumberFieldValueNextAclRule, allAclRules, i);
+ }
+
_networkACLItemDao.updateNumberFieldNetworkItem(ruleBeingMoved.getId(),
newNumberFieldValue);
+ return _networkACLItemDao.findById(ruleBeingMoved.getId());
+ }
+
+ /**
+ * Searches in the database for an ACL rule by its UUID.
+ * An {@link InvalidParameterValueException} is thrown if no ACL rule is
found with the given UUID.
+ */
+ protected NetworkACLItemVO retrieveAndValidateAclRule(String aclRuleUuid) {
+ if (StringUtils.isBlank(aclRuleUuid)) {
+ return null;
+ }
+ NetworkACLItemVO aclRule = _networkACLItemDao.findByUuid(aclRuleUuid);
+ if (aclRule == null) {
+ throw new InvalidParameterValueException(String.format("Could not
find rule with ID [%s]", aclRuleUuid));
+ }
+ return aclRule;
+ }
+
+ /**
+ * Validates if the data provided to move the ACL rule is supported by
this implementation. The user needs to provide a valid ACL UUID, and at least
one of the previous or the next ACL rule.
+ * The validation is as follows:
+ * <ul>
+ * <li> If both ACL rules 'previous' and 'next' are invalid, we throw
an {@link InvalidParameterValueException};
+ * <li> informed previous and next ACL rules must have the same ACL
ID as the rule being moved; otherwise, an {@link
InvalidParameterValueException} is thrown;
+ * <li> then we check if the user trying to move ACL rules has access
to the VPC, where the ACL rules are being applied.
+ * </ul>
+ */
+ protected void validateMoveAclRulesData(NetworkACLItemVO ruleBeingMoved,
NetworkACLItemVO previousRule, NetworkACLItemVO nextRule) {
+ if (nextRule == null && previousRule == null) {
+ throw new InvalidParameterValueException("Both previous and next
ACL rule IDs cannot be invalid.");
+ }
+ long aclId = ruleBeingMoved.getAclId();
+
+ if ((nextRule != null && nextRule.getAclId() != aclId) ||
(previousRule != null && previousRule.getAclId() != aclId)) {
+ throw new InvalidParameterValueException("Cannot use ACL rules
from differenting ACLs. Rule being moved.");
+ }
+ NetworkACLVO acl = _networkACLDao.findById(aclId);
+ Vpc vpc = _entityMgr.findById(Vpc.class, acl.getVpcId());
+ Account caller = CallContext.current().getCallingAccount();
+ _accountMgr.checkAccess(caller, null, true, vpc);
+ }
}
\ No newline at end of file
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index 3ced3d603cc..3082f1dfa72 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -382,6 +382,7 @@
import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworkOfferingsCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworksCmd;
+import org.apache.cloudstack.api.command.user.network.MoveNetworkAclItemCmd;
import org.apache.cloudstack.api.command.user.network.ReplaceNetworkACLListCmd;
import org.apache.cloudstack.api.command.user.network.RestartNetworkCmd;
import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
@@ -2332,7 +2333,6 @@ public String getConsoleAccessUrlRoot(final long vmId) {
return new Pair<String, Integer>(null, -1);
}
-
@Override
public Pair<List<? extends Alert>, Integer> searchForAlerts(final
ListAlertsCmd cmd) {
final Filter searchFilter = new Filter(AlertVO.class, "lastSent",
false, cmd.getStartIndex(), cmd.getPageSizeVal());
@@ -2958,6 +2958,7 @@ public long getMemoryOrCpuCapacityByHost(final Long
hostId, final short capacity
cmdList.add(ListNetworkACLListsCmd.class);
cmdList.add(ReplaceNetworkACLListCmd.class);
cmdList.add(UpdateNetworkACLItemCmd.class);
+ cmdList.add(MoveNetworkAclItemCmd.class);
cmdList.add(CleanVMReservationsCmd.class);
cmdList.add(UpgradeRouterTemplateCmd.class);
cmdList.add(UploadSslCertCmd.class);
diff --git
a/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
b/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
index 540a104a573..598057402f2 100644
--- a/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
+++ b/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
@@ -17,14 +17,20 @@
package com.cloud.network.vpc;
+import static org.mockito.Mockito.times;
+
import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
+import org.apache.cloudstack.api.command.user.network.MoveNetworkAclItemCmd;
import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLListCmd;
import org.apache.cloudstack.context.CallContext;
+import org.apache.commons.lang.StringUtils;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -63,7 +69,7 @@
@Mock
private NetworkModel networkModelMock;
@Mock
- private NetworkACLManager networkAclManager;
+ private NetworkACLManager networkAclManagerMock;
@Mock
private NetworkACLItemDao networkAclItemDaoMock;
@Mock
@@ -80,7 +86,7 @@
@Mock
private Network networkMock;
@Mock
- private NetworkACL networkAclMock;
+ private NetworkACLVO networkAclMock;
@Mock
private NetworkACLItemVO networkAclItemVoMock;
@Mock
@@ -93,6 +99,20 @@
private Long networkMockVpcMockId = 3L;
private long networkAclListId = 1l;
+ @Mock
+ private MoveNetworkAclItemCmd moveNetworkAclItemCmdMock;
+ @Mock
+ private NetworkACLItemVO aclRuleBeingMovedMock;
+ private String uuidAclRuleBeingMoved = "uuidRuleBeingMoved";
+
+ @Mock
+ private NetworkACLItemVO previousAclRuleMock;
+ private String previousAclRuleUuid = "uuidPreviousAclRule";
+
+ @Mock
+ private NetworkACLItemVO nextAclRuleMock;
+ private String nextAclRuleUuid = "uuidNextAclRule";
+
@Before
public void befoteTest() {
PowerMockito.mockStatic(CallContext.class);
@@ -104,6 +124,16 @@ public void befoteTest() {
Mockito.when(networkMock.getNetworkOfferingId()).thenReturn(networkOfferingMockId);
Mockito.when(networkMock.getVpcId()).thenReturn(networkMockVpcMockId);
+
+
Mockito.when(moveNetworkAclItemCmdMock.getUuidRuleBeingMoved()).thenReturn(uuidAclRuleBeingMoved);
+
+
Mockito.when(aclRuleBeingMovedMock.getUuid()).thenReturn(uuidAclRuleBeingMoved);
+
Mockito.when(aclRuleBeingMovedMock.getAclId()).thenReturn(networkAclMockId);
+
+
Mockito.when(previousAclRuleMock.getUuid()).thenReturn(previousAclRuleUuid);
+ Mockito.when(nextAclRuleMock.getUuid()).thenReturn(nextAclRuleUuid);
+
+
Mockito.when(networkAclMock.getVpcId()).thenReturn(networkMockVpcMockId);
}
@Test
@@ -120,7 +150,7 @@ private void
createNetworkACLItemTestForNumberAndExecuteTest(Integer number) {
Mockito.when(createNetworkAclCmdMock.getNumber()).thenReturn(number);
Mockito.doReturn(networkAclMockId).when(networkAclServiceImpl).createAclListIfNeeded(createNetworkAclCmdMock);
-
Mockito.when(networkAclManager.getNetworkACL(networkAclMockId)).thenReturn(networkAclMock);
+
Mockito.when(networkAclManagerMock.getNetworkACL(networkAclMockId)).thenReturn(networkAclMock);
Mockito.doNothing().when(networkAclServiceImpl).validateAclRuleNumber(createNetworkAclCmdMock,
networkAclMock);
Mockito.doNothing().when(networkAclServiceImpl).validateNetworkAcl(networkAclMock);
@@ -129,7 +159,7 @@ private void
createNetworkACLItemTestForNumberAndExecuteTest(Integer number) {
Mockito.when(networkAclItemDaoMock.getMaxNumberByACL(networkAclMockId)).thenReturn(5);
Mockito.doNothing().when(networkAclServiceImpl).validateNetworkACLItem(Mockito.any(NetworkACLItemVO.class));
-
Mockito.when(networkAclManager.createNetworkACLItem(Mockito.any(NetworkACLItemVO.class))).thenAnswer(new
Answer<NetworkACLItemVO>() {
+
Mockito.when(networkAclManagerMock.createNetworkACLItem(Mockito.any(NetworkACLItemVO.class))).thenAnswer(new
Answer<NetworkACLItemVO>() {
@Override
public NetworkACLItemVO answer(InvocationOnMock invocation) throws
Throwable {
return (NetworkACLItemVO)invocation.getArguments()[0];
@@ -140,15 +170,15 @@ public NetworkACLItemVO answer(InvocationOnMock
invocation) throws Throwable {
Assert.assertEquals(number == null ? 6 : number,
netowkrAclRuleCreated.getNumber());
- InOrder inOrder = Mockito.inOrder(networkAclManager,
networkAclServiceImpl, networkAclItemDaoMock);
+ InOrder inOrder = Mockito.inOrder(networkAclManagerMock,
networkAclServiceImpl, networkAclItemDaoMock);
inOrder.verify(networkAclServiceImpl).createAclListIfNeeded(createNetworkAclCmdMock);
- inOrder.verify(networkAclManager).getNetworkACL(networkAclMockId);
+ inOrder.verify(networkAclManagerMock).getNetworkACL(networkAclMockId);
inOrder.verify(networkAclServiceImpl).validateNetworkAcl(networkAclMock);
inOrder.verify(networkAclServiceImpl).validateAclRuleNumber(createNetworkAclCmdMock,
networkAclMock);
inOrder.verify(networkAclServiceImpl).validateAndCreateNetworkAclRuleAction(Mockito.anyString());
inOrder.verify(networkAclItemDaoMock, Mockito.times(number == null ? 1
: 0)).getMaxNumberByACL(networkAclMockId);
inOrder.verify(networkAclServiceImpl).validateNetworkACLItem(Mockito.any(NetworkACLItemVO.class));
-
inOrder.verify(networkAclManager).createNetworkACLItem(Mockito.any(NetworkACLItemVO.class));
+
inOrder.verify(networkAclManagerMock).createNetworkACLItem(Mockito.any(NetworkACLItemVO.class));
}
@Test
@@ -238,7 +268,7 @@ public void
createAclListForNetworkAndReturnAclListIdTestServicesSupportedByNetw
public void
createAclListForNetworkAndReturnAclListIdTestCreateNetworkAclReturnsNull() {
Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId,
Network.Service.NetworkACL);
Mockito.doReturn(Mockito.mock(Vpc.class)).when(entityManagerMock).findById(Vpc.class,
networkMockVpcMockId);
-
Mockito.doReturn(null).when(networkAclManager).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+
Mockito.doReturn(null).when(networkAclManagerMock).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
networkAclServiceImpl.createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock,
networkMock);
}
@@ -247,8 +277,8 @@ public void
createAclListForNetworkAndReturnAclListIdTestCreateNetworkAclReturns
public void
createAclListForNetworkAndReturnAclListIdTestAclNetworkIsCreatedButNotApplied()
throws ResourceUnavailableException {
Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId,
Network.Service.NetworkACL);
Mockito.doReturn(Mockito.mock(Vpc.class)).when(entityManagerMock).findById(Vpc.class,
networkMockVpcMockId);
-
Mockito.doReturn(Mockito.mock(NetworkACL.class)).when(networkAclManager).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
-
Mockito.doReturn(false).when(networkAclManager).replaceNetworkACL(Mockito.any(NetworkACL.class),
Mockito.any(NetworkVO.class));
+
Mockito.doReturn(Mockito.mock(NetworkACL.class)).when(networkAclManagerMock).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+
Mockito.doReturn(false).when(networkAclManagerMock).replaceNetworkACL(Mockito.any(NetworkACL.class),
Mockito.any(NetworkVO.class));
NetworkVO networkVoMock = new NetworkVO();
networkVoMock.setNetworkOfferingId(networkOfferingMockId);
@@ -261,9 +291,9 @@ public void
createAclListForNetworkAndReturnAclListIdTestAclNetworkIsCreatedButN
public void
createAclListForNetworkAndReturnAclListIdTestAclNetworkIsCreatedButNotAppliedWithException()
throws ResourceUnavailableException {
Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId,
Network.Service.NetworkACL);
Mockito.doReturn(Mockito.mock(Vpc.class)).when(entityManagerMock).findById(Vpc.class,
networkMockVpcMockId);
-
Mockito.doReturn(Mockito.mock(NetworkACL.class)).when(networkAclManager).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+
Mockito.doReturn(Mockito.mock(NetworkACL.class)).when(networkAclManagerMock).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
-
Mockito.doThrow(ResourceUnavailableException.class).when(networkAclManager).replaceNetworkACL(Mockito.any(NetworkACL.class),
Mockito.any(NetworkVO.class));
+
Mockito.doThrow(ResourceUnavailableException.class).when(networkAclManagerMock).replaceNetworkACL(Mockito.any(NetworkACL.class),
Mockito.any(NetworkVO.class));
NetworkVO networkVoMock = new NetworkVO();
networkVoMock.setNetworkOfferingId(networkOfferingMockId);
@@ -280,9 +310,9 @@ public void
createAclListForNetworkAndReturnAclListIdTestAclIsCreatedAndAppliedW
NetworkACL networkAclMock = Mockito.mock(NetworkACL.class);
Long expectedNetworkAclId = 5L;
Mockito.when(networkAclMock.getId()).thenReturn(expectedNetworkAclId);
-
Mockito.doReturn(networkAclMock).when(networkAclManager).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+
Mockito.doReturn(networkAclMock).when(networkAclManagerMock).createNetworkACL(Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
-
Mockito.doReturn(true).when(networkAclManager).replaceNetworkACL(Mockito.any(NetworkACL.class),
Mockito.any(NetworkVO.class));
+
Mockito.doReturn(true).when(networkAclManagerMock).replaceNetworkACL(Mockito.any(NetworkACL.class),
Mockito.any(NetworkVO.class));
NetworkVO networkVoMock = new NetworkVO();
networkVoMock.setNetworkOfferingId(networkOfferingMockId);
@@ -666,21 +696,21 @@ public void
validateIcmpTypeAndCodeTestIcmpTypeValidAndIcmpCodeValid() {
public void updateNetworkACLItemTest() throws ResourceUnavailableException
{
Mockito.when(networkAclItemVoMock.getAclId()).thenReturn(networkAclMockId);
Mockito.doReturn(networkAclItemVoMock).when(networkAclServiceImpl).validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmdMock);
-
Mockito.doReturn(networkAclMock).when(networkAclManager).getNetworkACL(networkAclMockId);
+
Mockito.doReturn(networkAclMock).when(networkAclManagerMock).getNetworkACL(networkAclMockId);
Mockito.doNothing().when(networkAclServiceImpl).validateNetworkAcl(Mockito.eq(networkAclMock));
Mockito.doNothing().when(networkAclServiceImpl).transferDataToNetworkAclRulePojo(Mockito.eq(updateNetworkACLItemCmdMock),
Mockito.eq(networkAclItemVoMock), Mockito.eq(networkAclMock));
Mockito.doNothing().when(networkAclServiceImpl).validateNetworkACLItem(networkAclItemVoMock);
-
Mockito.doReturn(networkAclItemVoMock).when(networkAclManager).updateNetworkACLItem(networkAclItemVoMock);
+
Mockito.doReturn(networkAclItemVoMock).when(networkAclManagerMock).updateNetworkACLItem(networkAclItemVoMock);
networkAclServiceImpl.updateNetworkACLItem(updateNetworkACLItemCmdMock);
- InOrder inOrder = Mockito.inOrder(networkAclServiceImpl,
networkAclManager);
+ InOrder inOrder = Mockito.inOrder(networkAclServiceImpl,
networkAclManagerMock);
inOrder.verify(networkAclServiceImpl).validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmdMock);
- inOrder.verify(networkAclManager).getNetworkACL(networkAclMockId);
+ inOrder.verify(networkAclManagerMock).getNetworkACL(networkAclMockId);
inOrder.verify(networkAclServiceImpl).validateNetworkAcl(networkAclMock);
inOrder.verify(networkAclServiceImpl).transferDataToNetworkAclRulePojo(Mockito.eq(updateNetworkACLItemCmdMock),
Mockito.eq(networkAclItemVoMock), Mockito.eq(networkAclMock));
inOrder.verify(networkAclServiceImpl).validateNetworkACLItem(networkAclItemVoMock);
-
inOrder.verify(networkAclManager).updateNetworkACLItem(networkAclItemVoMock);
+
inOrder.verify(networkAclManagerMock).updateNetworkACLItem(networkAclItemVoMock);
}
@Test(expected = InvalidParameterValueException.class)
@@ -852,6 +882,169 @@ public void updateNetworkACLTestParametersNotNull() {
inOrder.verify(networkAclDaoMock).findById(networkAclListId);
}
+ @Test(expected = InvalidParameterValueException.class)
+ public void
moveNetworkAclRuleToNewPositionTestBothPreviousAndNextAclRuleIdsNull() {
+ configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand(null, null);
+
+
networkAclServiceImpl.moveNetworkAclRuleToNewPosition(moveNetworkAclItemCmdMock);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void
moveNetworkAclRuleToNewPositionTestBothPreviousAndNextAclRuleIdsEmpty() {
+ configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand("", "");
+
+
networkAclServiceImpl.moveNetworkAclRuleToNewPosition(moveNetworkAclItemCmdMock);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void
moveNetworkAclRuleToNewPositionTestBothPreviousAndNextAclRuleIdsBlank() {
+ configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand(" ", "
");
+
+
networkAclServiceImpl.moveNetworkAclRuleToNewPosition(moveNetworkAclItemCmdMock);
+ }
+
+ private void
configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand(String
nextAclRuleUuid, String previousAclRuleUuid) {
+
Mockito.when(moveNetworkAclItemCmdMock.getNextAclRuleUuid()).thenReturn(nextAclRuleUuid);
+
Mockito.when(moveNetworkAclItemCmdMock.getPreviousAclRuleUuid()).thenReturn(previousAclRuleUuid);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void moveNetworkAclRuleToNewPositionTestAclRuleBeingMovedNotFound()
{
+
configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand(nextAclRuleUuid,
previousAclRuleUuid);
+
+
Mockito.doReturn(null).when(networkAclItemDaoMock).findByUuid(uuidAclRuleBeingMoved);
+
+
networkAclServiceImpl.moveNetworkAclRuleToNewPosition(moveNetworkAclItemCmdMock);
+ }
+
+ @Test
+ public void moveNetworkAclRuleToNewPositionTestMoveRuleToTop() {
+
configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand(nextAclRuleUuid,
previousAclRuleUuid);
+
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclItemDaoMock).findByUuid(uuidAclRuleBeingMoved);
+
+
Mockito.doReturn(null).when(networkAclServiceImpl).retrieveAndValidateAclRule(previousAclRuleUuid);
+
Mockito.doReturn(nextAclRuleMock).when(networkAclServiceImpl).retrieveAndValidateAclRule(nextAclRuleUuid);
+
+
Mockito.doNothing().when(networkAclServiceImpl).validateMoveAclRulesData(aclRuleBeingMovedMock,
null, nextAclRuleMock);
+
+ configureMoveMethodsToDoNothing();
+
+
networkAclServiceImpl.moveNetworkAclRuleToNewPosition(moveNetworkAclItemCmdMock);
+
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(1)).moveRuleToTheTop(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(0)).moveRuleToTheBottom(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(0)).moveRuleBetweenAclRules(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class), Mockito.eq(previousAclRuleMock),
+ Mockito.eq(nextAclRuleMock));
+ }
+
+ @Test
+ public void moveNetworkAclRuleToNewPositionTestMoveRuleToBottom() {
+
configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand(nextAclRuleUuid,
previousAclRuleUuid);
+
+
Mockito.doNothing().when(networkAclServiceImpl).validateMoveAclRulesData(aclRuleBeingMovedMock,
previousAclRuleMock, null);
+
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclItemDaoMock).findByUuid(uuidAclRuleBeingMoved);
+
+
Mockito.doReturn(previousAclRuleMock).when(networkAclServiceImpl).retrieveAndValidateAclRule(previousAclRuleUuid);
+
Mockito.doReturn(null).when(networkAclServiceImpl).retrieveAndValidateAclRule(nextAclRuleUuid);
+
+ configureMoveMethodsToDoNothing();
+
+
networkAclServiceImpl.moveNetworkAclRuleToNewPosition(moveNetworkAclItemCmdMock);
+
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(0)).moveRuleToTheTop(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(1)).moveRuleToTheBottom(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(0)).moveRuleBetweenAclRules(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class), Mockito.eq(previousAclRuleMock),
+ Mockito.eq(nextAclRuleMock));
+ }
+
+ @Test
+ public void moveNetworkAclRuleToNewPositionTestMoveBetweenAclRules() {
+
configureNextAndPreviousAclRuleUuidsForMoveAclRuleCommand(nextAclRuleUuid,
previousAclRuleUuid);
+
+
Mockito.doNothing().when(networkAclServiceImpl).validateMoveAclRulesData(aclRuleBeingMovedMock,
previousAclRuleMock, nextAclRuleMock);
+
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclItemDaoMock).findByUuid(uuidAclRuleBeingMoved);
+
+
Mockito.doReturn(previousAclRuleMock).when(networkAclServiceImpl).retrieveAndValidateAclRule(previousAclRuleUuid);
+
Mockito.doReturn(nextAclRuleMock).when(networkAclServiceImpl).retrieveAndValidateAclRule(nextAclRuleUuid);
+
+ configureMoveMethodsToDoNothing();
+
+
networkAclServiceImpl.moveNetworkAclRuleToNewPosition(moveNetworkAclItemCmdMock);
+
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(0)).moveRuleToTheTop(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(0)).moveRuleToTheBottom(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(1)).moveRuleBetweenAclRules(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class), Mockito.eq(previousAclRuleMock),
+ Mockito.eq(nextAclRuleMock));
+ }
+
+ private void configureMoveMethodsToDoNothing() {
+ Mockito.doReturn(new
ArrayList<>()).when(networkAclServiceImpl).getAllAclRulesSortedByNumber(networkAclMockId);
+
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclServiceImpl).moveRuleToTheTop(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclServiceImpl).moveRuleToTheBottom(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class));
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclServiceImpl).moveRuleBetweenAclRules(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyListOf(NetworkACLItemVO.class),
+ Mockito.eq(previousAclRuleMock), Mockito.eq(nextAclRuleMock));
+ }
+
+ @Test
+ public void retrieveAndValidateAclRuleTestUuidNull() {
+ NetworkACLItemVO networkACLItemVOReceived =
networkAclServiceImpl.retrieveAndValidateAclRule(null);
+
+ Assert.assertNull(networkACLItemVOReceived);
+ }
+
+ @Test
+ public void retrieveAndValidateAclRuleTestUuidEmpty() {
+ NetworkACLItemVO networkACLItemVOReceived =
networkAclServiceImpl.retrieveAndValidateAclRule(StringUtils.EMPTY);
+
+ Assert.assertNull(networkACLItemVOReceived);
+ }
+
+ @Test
+ public void retrieveAndValidateAclRuleTestUuidBlank() {
+ NetworkACLItemVO networkACLItemVOReceived =
networkAclServiceImpl.retrieveAndValidateAclRule(" ");
+
+ Assert.assertNull(networkACLItemVOReceived);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void retrieveAndValidateAclRuleTestAclRuleNotFound() {
+
Mockito.doReturn(null).when(networkAclItemDaoMock).findByUuid(nextAclRuleUuid);
+
+ networkAclServiceImpl.retrieveAndValidateAclRule(nextAclRuleUuid);
+ }
+
+ @Test
+ public void retrieveAndValidateAclRuleTestAclRuleFound() {
+
Mockito.doReturn(nextAclRuleMock).when(networkAclItemDaoMock).findByUuid(nextAclRuleUuid);
+
+ NetworkACLItemVO networkACLItemVOReceived =
networkAclServiceImpl.retrieveAndValidateAclRule(nextAclRuleUuid);
+
+ Assert.assertEquals(nextAclRuleMock, networkACLItemVOReceived);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void validateMoveAclRulesDataTestInvalidPreviousAndNextAclRules() {
+ networkAclServiceImpl.validateMoveAclRulesData(aclRuleBeingMovedMock,
null, null);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void validateMoveAclRulesDataTestPreviousRuleWithDifferentAclId() {
+ Mockito.when(previousAclRuleMock.getAclId()).thenReturn(99L);
+
+ networkAclServiceImpl.validateMoveAclRulesData(aclRuleBeingMovedMock,
previousAclRuleMock, null);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void validateMoveAclRulesDataTestNextRuleWithDifferentAclId() {
+ Mockito.when(nextAclRuleMock.getAclId()).thenReturn(99L);
+
+ networkAclServiceImpl.validateMoveAclRulesData(aclRuleBeingMovedMock,
null, nextAclRuleMock);
+ }
+
@Test
@PrepareForTest(CallContext.class)
public void updateNetworkACLTestParametersWithNullValues() {
@@ -878,4 +1071,214 @@ public void
updateNetworkACLTestParametersWithNullValues() {
inOrder.verify(networkAclDaoMock).findById(networkAclListId);
}
-}
+ @Test
+ public void validateMoveAclRulesDataTestSuccesfullExecution() {
+ Mockito.when(nextAclRuleMock.getAclId()).thenReturn(networkAclMockId);
+
Mockito.when(previousAclRuleMock.getAclId()).thenReturn(networkAclMockId);
+
+
Mockito.doReturn(networkAclMock).when(networkAclDaoMock).findById(networkAclMockId);
+
Mockito.doReturn(Mockito.mock(Vpc.class)).when(entityManagerMock).findById(Vpc.class,
networkMockVpcMockId);
+
+ CallContext callContextMock = Mockito.mock(CallContext.class);
+
Mockito.doReturn(Mockito.mock(Account.class)).when(callContextMock).getCallingAccount();
+
+ PowerMockito.mockStatic(CallContext.class);
+ PowerMockito.when(CallContext.current()).thenReturn(callContextMock);
+
+
Mockito.doNothing().when(accountManagerMock).checkAccess(Mockito.any(Account.class),
Mockito.isNull(AccessType.class), Mockito.eq(true), Mockito.any(Vpc.class));
+
+ networkAclServiceImpl.validateMoveAclRulesData(aclRuleBeingMovedMock,
previousAclRuleMock, nextAclRuleMock);
+
+ Mockito.verify(networkAclDaoMock).findById(networkAclMockId);
+ Mockito.verify(entityManagerMock).findById(Vpc.class,
networkMockVpcMockId);
+
Mockito.verify(accountManagerMock).checkAccess(Mockito.any(Account.class),
Mockito.isNull(AccessType.class), Mockito.eq(true), Mockito.any(Vpc.class));
+ }
+
+ @Test
+ public void getAllAclRulesSortedByNumberTest() {
+ List<NetworkACLItemVO> networkAclItemVos = new ArrayList<>();
+
+ NetworkACLItemVO networkACLItemVO1 = new NetworkACLItemVO();
+ networkACLItemVO1.setNumber(1);
+
+ NetworkACLItemVO networkACLItemVO2 = new NetworkACLItemVO();
+ networkACLItemVO2.setNumber(2);
+
+ NetworkACLItemVO networkACLItemVO3 = new NetworkACLItemVO();
+ networkACLItemVO3.setNumber(3);
+
+ NetworkACLItemVO networkACLItemVO4 = new NetworkACLItemVO();
+ networkACLItemVO4.setNumber(4);
+
+ networkAclItemVos.add(networkACLItemVO1);
+ networkAclItemVos.add(networkACLItemVO2);
+ networkAclItemVos.add(networkACLItemVO3);
+ networkAclItemVos.add(networkACLItemVO4);
+
+ Collections.shuffle(networkAclItemVos);
+
+
Mockito.doReturn(networkAclItemVos).when(networkAclItemDaoMock).listByACL(networkAclMockId);
+
+ List<NetworkACLItemVO> allAclRulesSortedByNumber =
networkAclServiceImpl.getAllAclRulesSortedByNumber(networkAclMockId);
+
+ Assert.assertEquals(networkAclItemVos.size(),
allAclRulesSortedByNumber.size());
+ Assert.assertEquals(networkACLItemVO1, networkAclItemVos.get(0));
+ Assert.assertEquals(networkACLItemVO2, networkAclItemVos.get(1));
+ Assert.assertEquals(networkACLItemVO3, networkAclItemVos.get(2));
+ Assert.assertEquals(networkACLItemVO4, networkAclItemVos.get(3));
+
+ }
+
+ @Test
+ public void moveRuleToTheTopTest() {
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclServiceImpl).updateAclRuleToNewPositionAndExecuteShiftIfNecessary(Mockito.eq(aclRuleBeingMovedMock),
Mockito.anyInt(),
+ Mockito.anyListOf(NetworkACLItemVO.class), Mockito.anyInt());
+
+ networkAclServiceImpl.moveRuleToTheTop(aclRuleBeingMovedMock, new
ArrayList<>());
+
+
Mockito.verify(networkAclServiceImpl).updateAclRuleToNewPositionAndExecuteShiftIfNecessary(Mockito.eq(aclRuleBeingMovedMock),
Mockito.eq(1), Mockito.anyListOf(NetworkACLItemVO.class),
+ Mockito.eq(0));
+ }
+
+ @Test
+ public void moveRuleToTheBottomTest() {
+ ArrayList<NetworkACLItemVO> allAclRules = new ArrayList<>();
+
+ NetworkACLItemVO networkACLItemVO1 = new NetworkACLItemVO();
+ networkACLItemVO1.setNumber(100);
+
+ allAclRules.add(networkACLItemVO1);
+ Mockito.when(aclRuleBeingMovedMock.getId()).thenReturn(99l);
+
+
Mockito.doNothing().when(networkAclItemDaoMock).updateNumberFieldNetworkItem(Mockito.anyLong(),
Mockito.anyInt());
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclItemDaoMock).findById(99l);
+
+ networkAclServiceImpl.moveRuleToTheBottom(aclRuleBeingMovedMock,
allAclRules);
+
+ Mockito.verify(aclRuleBeingMovedMock).setNumber(101);
+
Mockito.verify(networkAclItemDaoMock).updateNumberFieldNetworkItem(99l, 101);
+ Mockito.verify(networkAclItemDaoMock).findById(99l);
+ }
+
+ @Test(expected = InvalidParameterValueException.class)
+ public void
moveRuleBetweenAclRulesTestThereIsSpaceBetweenPreviousRuleAndNextRuleToAccomodateTheNewRuleWithOtherruleColliding()
{
+ Mockito.when(previousAclRuleMock.getNumber()).thenReturn(10);
+ Mockito.when(nextAclRuleMock.getNumber()).thenReturn(15);
+
+ ArrayList<NetworkACLItemVO> allAclRules = new ArrayList<>();
+ NetworkACLItemVO networkACLItemVO1 = new NetworkACLItemVO();
+ networkACLItemVO1.setNumber(11);
+
+ allAclRules.add(previousAclRuleMock);
+ allAclRules.add(networkACLItemVO1);
+ allAclRules.add(nextAclRuleMock);
+
+ networkAclServiceImpl.moveRuleBetweenAclRules(aclRuleBeingMovedMock,
allAclRules, previousAclRuleMock, nextAclRuleMock);
+ }
+
+ @Test
+ public void
moveRuleBetweenAclRulesTestThereIsSpaceBetweenPreviousRuleAndNextRuleToAccomodateTheNewRule()
{
+ Mockito.when(previousAclRuleMock.getNumber()).thenReturn(10);
+ Mockito.when(nextAclRuleMock.getNumber()).thenReturn(11);
+ Mockito.when(aclRuleBeingMovedMock.getNumber()).thenReturn(50);
+ Mockito.when(aclRuleBeingMovedMock.getId()).thenReturn(1l);
+
+ ArrayList<NetworkACLItemVO> allAclRules = new ArrayList<>();
+ NetworkACLItemVO networkACLItemVO12 = new NetworkACLItemVO();
+ networkACLItemVO12.setNumber(12);
+ NetworkACLItemVO networkACLItemVO13 = new NetworkACLItemVO();
+ networkACLItemVO13.setNumber(13);
+ NetworkACLItemVO networkACLItemVO14 = new NetworkACLItemVO();
+ networkACLItemVO14.setNumber(14);
+
+ allAclRules.add(previousAclRuleMock);
+ allAclRules.add(nextAclRuleMock);
+ allAclRules.add(networkACLItemVO12);
+ allAclRules.add(networkACLItemVO13);
+ allAclRules.add(networkACLItemVO14);
+ allAclRules.add(aclRuleBeingMovedMock);
+
+
Mockito.doNothing().when(networkAclItemDaoMock).updateNumberFieldNetworkItem(Mockito.anyLong(),
Mockito.anyInt());
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclItemDaoMock).findById(1l);
+
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclServiceImpl).updateAclRuleToNewPositionAndExecuteShiftIfNecessary(Mockito.any(NetworkACLItemVO.class),
Mockito.anyInt(),
+ Mockito.anyListOf(NetworkACLItemVO.class), Mockito.anyInt());
+
+ networkAclServiceImpl.moveRuleBetweenAclRules(aclRuleBeingMovedMock,
allAclRules, previousAclRuleMock, nextAclRuleMock);
+
+ Mockito.verify(networkAclItemDaoMock,
times(0)).updateNumberFieldNetworkItem(aclRuleBeingMovedMock.getId(), 11);
+ Mockito.verify(networkAclItemDaoMock, times(0)).findById(1l);
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(1)).updateAclRuleToNewPositionAndExecuteShiftIfNecessary(Mockito.any(NetworkACLItemVO.class),
Mockito.eq(11),
+ Mockito.anyListOf(NetworkACLItemVO.class), Mockito.eq(1));
+ }
+
+ @Test
+ public void
moveRuleBetweenAclRulesTestThereIsNoSpaceBetweenPreviousRuleAndNextRuleToAccomodateTheNewRule()
{
+ Mockito.when(previousAclRuleMock.getNumber()).thenReturn(10);
+ Mockito.when(nextAclRuleMock.getNumber()).thenReturn(15);
+ Mockito.when(aclRuleBeingMovedMock.getNumber()).thenReturn(50);
+ Mockito.when(aclRuleBeingMovedMock.getId()).thenReturn(1l);
+
+ ArrayList<NetworkACLItemVO> allAclRules = new ArrayList<>();
+
+ allAclRules.add(previousAclRuleMock);
+ allAclRules.add(nextAclRuleMock);
+ allAclRules.add(aclRuleBeingMovedMock);
+
+
Mockito.doNothing().when(networkAclItemDaoMock).updateNumberFieldNetworkItem(Mockito.anyLong(),
Mockito.anyInt());
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclItemDaoMock).findById(1l);
+
+
Mockito.doReturn(aclRuleBeingMovedMock).when(networkAclServiceImpl).updateAclRuleToNewPositionAndExecuteShiftIfNecessary(Mockito.any(NetworkACLItemVO.class),
Mockito.anyInt(),
+ Mockito.anyListOf(NetworkACLItemVO.class), Mockito.anyInt());
+
+ networkAclServiceImpl.moveRuleBetweenAclRules(aclRuleBeingMovedMock,
allAclRules, previousAclRuleMock, nextAclRuleMock);
+
+
Mockito.verify(networkAclItemDaoMock).updateNumberFieldNetworkItem(aclRuleBeingMovedMock.getId(),
11);
+ Mockito.verify(networkAclItemDaoMock).findById(1l);
+ Mockito.verify(networkAclServiceImpl,
Mockito.times(0)).updateAclRuleToNewPositionAndExecuteShiftIfNecessary(Mockito.any(NetworkACLItemVO.class),
Mockito.anyInt(),
+ Mockito.anyListOf(NetworkACLItemVO.class), Mockito.anyInt());
+ }
+
+ @Test
+ public void updateAclRuleToNewPositionAndExecuteShiftIfNecessaryTest() {
+ Mockito.when(previousAclRuleMock.getNumber()).thenReturn(10);
+
+ Mockito.when(nextAclRuleMock.getNumber()).thenReturn(11);
+ Mockito.when(nextAclRuleMock.getId()).thenReturn(50l);
+
+ Mockito.when(aclRuleBeingMovedMock.getNumber()).thenReturn(50);
+ Mockito.when(aclRuleBeingMovedMock.getId()).thenReturn(1l);
+
+ ArrayList<NetworkACLItemVO> allAclRules = new ArrayList<>();
+ NetworkACLItemVO networkACLItemVO12 = new NetworkACLItemVO();
+ networkACLItemVO12.setNumber(12);
+ networkACLItemVO12.id = 12;
+ NetworkACLItemVO networkACLItemVO13 = new NetworkACLItemVO();
+ networkACLItemVO13.id = 13;
+ networkACLItemVO13.setNumber(13);
+ NetworkACLItemVO networkACLItemVO14 = new NetworkACLItemVO();
+ networkACLItemVO14.setNumber(14);
+ networkACLItemVO14.id = 14;
+
+ allAclRules.add(previousAclRuleMock);
+ allAclRules.add(nextAclRuleMock);
+ allAclRules.add(networkACLItemVO12);
+ allAclRules.add(networkACLItemVO13);
+ allAclRules.add(networkACLItemVO14);
+ allAclRules.add(aclRuleBeingMovedMock);
+
+
Mockito.doNothing().when(networkAclItemDaoMock).updateNumberFieldNetworkItem(Mockito.anyLong(),
Mockito.anyInt());
+
Mockito.doReturn(null).when(networkAclItemDaoMock).findById(Mockito.anyLong());
+
+
networkAclServiceImpl.updateAclRuleToNewPositionAndExecuteShiftIfNecessary(aclRuleBeingMovedMock,
11, allAclRules, 1);
+
+ Mockito.verify(aclRuleBeingMovedMock).setNumber(11);
+ Mockito.verify(nextAclRuleMock).setNumber(12);
+ Mockito.verify(networkAclItemDaoMock).updateNumberFieldNetworkItem(1l,
11);
+
Mockito.verify(networkAclItemDaoMock).updateNumberFieldNetworkItem(50l, 12);
+
+ Assert.assertEquals(13, networkACLItemVO12.getNumber());
+ Assert.assertEquals(14, networkACLItemVO13.getNumber());
+ Assert.assertEquals(15, networkACLItemVO14.getNumber());
+ }
+}
\ No newline at end of file
diff --git a/ui/scripts/vpc.js b/ui/scripts/vpc.js
index d7980c9ca46..9fea0e80a13 100644
--- a/ui/scripts/vpc.js
+++ b/ui/scripts/vpc.js
@@ -535,31 +535,22 @@
moveDrag: {
action: function(args) {
var rule = args.context.multiRule[0];
- var number = 0;
- var prevItem = args.prevItem ? args.prevItem.number : null;
- var nextItem = args.nextItem ? args.nextItem.number : null;
-
- if (!nextItem) { // Last item
- number = prevItem + 100;
- } else {
- if (nextItem - prevItem <= 10) {
- number = nextItem - parseInt(((nextItem -
prevItem) / 2));
- } else {
- number = nextItem > 1 ? nextItem - 10 : 1;
- }
- }
-
+
+ var previousRuleId = args.prevItem ? args.prevItem.id :
undefined;
+ var nextRuleId = args.nextItem ? args.nextItem.id :
undefined;
+
$.ajax({
- url: createURL('updateNetworkACLItem'),
+ url: createURL('moveNetworkAclItem'),
data: {
id: rule.id,
- number: number
+ previousaclruleid: previousRuleId,
+ nextaclruleid: nextRuleId
},
success: function(json) {
var pollTimer = setInterval(function() {
pollAsyncJobResult({
_custom: {
- jobId:
json.createnetworkaclresponse.jobid
+ jobId:
json.moveNetworkAclItemResponse.jobid
},
complete: function() {
clearInterval(pollTimer);
----------------------------------------------------------------
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:
[email protected]
> 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)
