This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 685cc72 [CLOUDSTACK-10344] bug when moving ACL rules (change order
with drag and drop) (#2511)
685cc72 is described below
commit 685cc7210c2f6396beb649cd1fb789957a6cdb66
Author: Rafael Weingärtner <rafaelweingart...@gmail.com>
AuthorDate: Mon Apr 16 08:45:29 2018 -0300
[CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and
drop) (#2511)
* Add permission to 'moveNetworkAclItem' API method in default roles
---
.../com/cloud/network/vpc/NetworkACLService.java | 8 +-
.../org/apache/cloudstack/api/ApiConstants.java | 2 +
.../user/network/MoveNetworkAclItemCmd.java | 96 +++++
.../com/cloud/network/vpc/NetworkACLItemDao.java | 10 +-
.../vpc/dao/NetworkACLItemCidrsDaoImpl.java | 1 -
.../network/vpc/dao/NetworkACLItemDaoImpl.java | 34 +-
.../resources/META-INF/db/schema-41110to41200.sql | 8 +-
.../cloud/network/vpc/NetworkACLServiceImpl.java | 169 ++++++++
.../com/cloud/server/ManagementServerImpl.java | 3 +-
.../network/vpc/NetworkACLServiceImplTest.java | 443 ++++++++++++++++++++-
ui/scripts/vpc.js | 25 +-
11 files changed, 749 insertions(+), 50 deletions(-)
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 378b15c..7c4e8b4 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 java.util.List;
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.exception.ResourceUnavailableException;
import com.cloud.utils.Pair;
public interface NetworkACLService {
+
/**
* Creates Network ACL for the specified VPC
*/
@@ -90,4 +92,8 @@ public interface NetworkACLService {
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 7825bb4..56d5957 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 class ApiConstants {
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 0000000..aaa9c18
--- /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 21794cb..5903172 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 java.util.List;
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 @@ public interface NetworkACLItemDao extends
GenericDao<NetworkACLItemVO, Long> {
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 395c9b2..4501f14 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 @@ package com.cloud.network.vpc.dao;
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 eb957a9..b88cc0d 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.SearchBuilder;
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 class NetworkACLItemDaoImpl extends
GenericDaoBase<NetworkACLItemVO, Long
@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 class NetworkACLItemDaoImpl extends
GenericDaoBase<NetworkACLItemVO, Long
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 class NetworkACLItemDaoImpl extends
GenericDaoBase<NetworkACLItemVO, Long
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 79ecbfc..d5e6d61 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 18d3f8d..674910d 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.ServerApiException;
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 class NetworkACLServiceImpl extends ManagerBase
implements NetworkACLServ
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 3ced3d6..3082f1d 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.ListNetworkACLListsCmd;
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 class ManagementServerImpl extends ManagerBase
implements ManagementServe
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 class ManagementServerImpl extends ManagerBase
implements ManagementServe
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 540a104..5980574 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 @@ public class NetworkACLServiceImplTest {
@Mock
private NetworkModel networkModelMock;
@Mock
- private NetworkACLManager networkAclManager;
+ private NetworkACLManager networkAclManagerMock;
@Mock
private NetworkACLItemDao networkAclItemDaoMock;
@Mock
@@ -80,7 +86,7 @@ public class NetworkACLServiceImplTest {
@Mock
private Network networkMock;
@Mock
- private NetworkACL networkAclMock;
+ private NetworkACLVO networkAclMock;
@Mock
private NetworkACLItemVO networkAclItemVoMock;
@Mock
@@ -93,6 +99,20 @@ public class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 @@ public class NetworkACLServiceImplTest {
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 @@ public class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 class NetworkACLServiceImplTest {
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 25baf6c..8a0bb96 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);
--
To stop receiving notification emails like this one, please contact
d...@apache.org.
