You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/04/16 11:45:31 UTC
[GitHub] DaanHoogland closed pull request #2511: [CLOUDSTACK-10344] bug when
moving ACL rules (change order with drag and drop)
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:
users@infra.apache.org
With regards,
Apache Git Services