Muli Salem has uploaded a new change for review. Change subject: engine: Network Command Validation ......................................................................
engine: Network Command Validation This patch extracts repetitive code to the NetworkCommon class, and cleans up canDoActions in both AddNetwork and UpdateNetwork commands, to use the Validation framework given by the CommandBase. Longer description using lines' length under 72 chars. With multiple paragraphs if necessary. Change-Id: I4bb5bafd2029b3a8729bcf78d359804c6f587ff7 Bug-Url: https://bugzilla.redhat.com/?????? Signed-off-by: Muli Salem <msa...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/AddNetworkCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/NetworkCommon.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java 3 files changed, 101 insertions(+), 61 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/42/10542/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/AddNetworkCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/AddNetworkCommand.java index 736dd36..3945dac 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/AddNetworkCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/AddNetworkCommand.java @@ -5,6 +5,7 @@ import org.ovirt.engine.core.bll.MultiLevelAdministrationHandler; import org.ovirt.engine.core.bll.PredefinedRoles; +import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; @@ -52,29 +53,13 @@ return false; } - // check that network name not start with 'bond' - if (getNetworkName().toLowerCase().startsWith("bond")) { - addCanDoActionMessage(VdcBllMessages.NETWORK_CANNOT_CONTAIN_BOND_NAME); + if (!validate(networkNameStartsWithoutBond())) { return false; } - // we return ok only if the network not exists - List<Network> all; - if (getNetwork().getstorage_pool_id() != null - && !getNetwork().getstorage_pool_id().getValue().equals(Guid.Empty)) { - all = getNetworkDAO().getAllForDataCenter(getNetwork().getstorage_pool_id().getValue()); - } else { - all = getNetworkDAO().getAll(); - } - boolean exists = null != LinqUtils.firstOrNull(all, new Predicate<Network>() { - @Override - public boolean eval(Network net) { - return net.getname().equals(getNetworkName()); - } - }); - if (exists) { - getReturnValue().getCanDoActionMessages() - .add(VdcBllMessages.NETWORK_NAME_ALREADY_EXISTS.toString()); + List<Network> all = getNetworks(); + + if (!validate(networkDoesNotExist(all))) { return false; } @@ -120,4 +105,28 @@ perms.setrole_id(role.getId()); MultiLevelAdministrationHandler.addPermission(perms); } + + private List<Network> getNetworks() { + if (getNetwork().getstorage_pool_id() != null + && !getNetwork().getstorage_pool_id().getValue().equals(Guid.Empty)) { + return getNetworkDAO().getAllForDataCenter(getNetwork().getstorage_pool_id().getValue()); + } else { + return getNetworkDAO().getAll(); + } + } + + private ValidationResult networkDoesNotExist(List<Network> networks) { + return getNetworkByName(networks) == null + ? ValidationResult.VALID + : new ValidationResult(VdcBllMessages.NETWORK_NAME_ALREADY_EXISTS); + } + + private Network getNetworkByName(List<Network> networks) { + return LinqUtils.firstOrNull(networks, new Predicate<Network>() { + @Override + public boolean eval(Network network) { + return network.getname().equals(getNetworkName()); + } + }); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/NetworkCommon.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/NetworkCommon.java index 3238fdc..ca43acf 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/NetworkCommon.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/NetworkCommon.java @@ -67,7 +67,6 @@ } } } - return ValidationResult.VALID; } @@ -77,6 +76,12 @@ : new ValidationResult(VdcBllMessages.NETWORK_CLUSTER_NETWORK_IN_USE); } + protected ValidationResult networkNameStartsWithoutBond() { + return getNetworkName().toLowerCase().startsWith("bond") + ? new ValidationResult(VdcBllMessages.NETWORK_CANNOT_CONTAIN_BOND_NAME) + : ValidationResult.VALID; + } + @Override public List<PermissionSubject> getPermissionCheckSubjects() { Network network = getNetwork(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java index d40f3da..8ee61bd 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java @@ -2,6 +2,7 @@ import java.util.List; +import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.network.cluster.NetworkClusterHelper; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.AddNetworkStoragePoolParameters; @@ -41,10 +42,7 @@ @Override protected boolean canDoAction() { - List<Network> networks = getNetworkDAO().getAll(); - - if (getStoragePool() == null) { - addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST); + if (!validate(storagePoolExists())) { return false; } @@ -60,58 +58,31 @@ return false; } - // check that network name not start with 'bond' - if (getNetworkName().toLowerCase().startsWith("bond")) { - addCanDoActionMessage(VdcBllMessages.NETWORK_CANNOT_CONTAIN_BOND_NAME); + if (!validate(networkNameStartsWithoutBond())) { return false; } + + List<Network> networks = getNetworkDAO().getAll(); if (!validate(vlanIsFree(networks))) { return false; } - // check that network not exsits - Network oldNetwork = LinqUtils.firstOrNull(networks, new Predicate<Network>() { - @Override - public boolean eval(Network n) { - return n.getId().equals(getNetwork().getId()); - } - }); - if (oldNetwork == null) { - addCanDoActionMessage(VdcBllMessages.NETWORK_NOT_EXISTS); + Network oldNetwork = getNetworkById(networks); + if (!validate(networkExists(oldNetwork))) { return false; } - // check defalut network name is not renamed - String defaultNetwork = Config.<String> GetValue(ConfigValues.ManagementNetwork); - if (oldNetwork.getname().equals(defaultNetwork) && - !getNetworkName().equals(defaultNetwork)) { - addCanDoActionMessage(VdcBllMessages.NETWORK_CAN_NOT_REMOVE_DEFAULT_NETWORK); + if (!validate(notDefaultNetwork(oldNetwork))) { return false; } - Network net = LinqUtils.firstOrNull(networks, new Predicate<Network>() { - @Override - public boolean eval(Network n) { - return n.getname().trim().toLowerCase() - .equals(getNetworkName().trim().toLowerCase()) - && !n.getId().equals(getNetwork().getId()) - && getNetwork().getstorage_pool_id().equals(n.getstorage_pool_id()); - } - }); - if (net != null) { - addCanDoActionMessage(VdcBllMessages.NETWORK_IN_USE); + if (!validate(networkNameNotUsed(networks))) { return false; } - // check if the network in use with running vm - clusters = getVdsGroupDAO().getAllForStoragePool(getStoragePool().getId()); - for (VDSGroup cluster : clusters) { - List<VmStatic> vms = getVmStaticDAO().getAllByGroupAndNetworkName(cluster.getId(), getNetworkName()); - if (vms.size() > 0) { - addCanDoActionMessage(VdcBllMessages.NETWORK_INTERFACE_IN_USE_BY_VM); - return false; - } + if (!validate(networkNotUsedByRunningVm())) { + return false; } return validate(networkNotAttachedToCluster(oldNetwork)); @@ -127,4 +98,59 @@ addValidationGroup(UpdateEntity.class); return super.getValidationGroups(); } + + private Network getNetworkById(List<Network> networks) { + return LinqUtils.firstOrNull(networks, new Predicate<Network>() { + @Override + public boolean eval(Network network) { + return network.getId().equals(getNetwork().getId()); + } + }); + } + + private ValidationResult storagePoolExists() { + return getStoragePool() == null + ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST) + : ValidationResult.VALID; + } + + private ValidationResult networkExists(Network oldNetwork) { + return oldNetwork == null + ? new ValidationResult(VdcBllMessages.NETWORK_NOT_EXISTS) + : ValidationResult.VALID; + } + + private ValidationResult networkNameNotUsed(List<Network> networks) { + Network networkWithSameName = LinqUtils.firstOrNull(networks, new Predicate<Network>() { + @Override + public boolean eval(Network network) { + return network.getname().trim().toLowerCase() + .equals(getNetworkName().trim().toLowerCase()) + && !network.getId().equals(getNetwork().getId()) + && getNetwork().getstorage_pool_id().equals(network.getstorage_pool_id()); + } + }); + + return networkWithSameName != null + ? new ValidationResult(VdcBllMessages.NETWORK_IN_USE) + : ValidationResult.VALID; + } + + private ValidationResult networkNotUsedByRunningVm() { + for (VDSGroup cluster : getVdsGroupDAO().getAllForStoragePool(getStoragePool().getId())) { + List<VmStatic> vms = getVmStaticDAO().getAllByGroupAndNetworkName(cluster.getId(), getNetworkName()); + if (vms.size() > 0) { + return new ValidationResult(VdcBllMessages.NETWORK_INTERFACE_IN_USE_BY_VM); + } + } + return ValidationResult.VALID; + } + + private ValidationResult notDefaultNetwork(Network oldNetwork) { + String defaultNetwork = Config.<String> GetValue(ConfigValues.ManagementNetwork); + return oldNetwork.getname().equals(defaultNetwork) && + !getNetworkName().equals(defaultNetwork) + ? new ValidationResult(VdcBllMessages.NETWORK_CAN_NOT_REMOVE_DEFAULT_NETWORK) + : ValidationResult.VALID; + } } -- To view, visit http://gerrit.ovirt.org/10542 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4bb5bafd2029b3a8729bcf78d359804c6f587ff7 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Muli Salem <msa...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
