Martin Mucha has uploaded a new change for review.
Change subject: userportal,webadmin: refactored badly overgrown method to more
understandable parts
......................................................................
userportal,webadmin: refactored badly overgrown method to more understandable
parts
• method broken into three methods:
◦ method checking if there's valid operation for first operand.
◦ method handling unary operation applied on first operand
◦ method handling binary operations.
• removed unnecessary elses to reduce indentation
• overloaded operationFor methods moved close together
• 'instanceof BondNetworkInterfaceModel' is performed on closer
superclass (NetworkInterfaceModel) instead of NetworkItemModel
to allow move casting to caller method (handleBinaryOperation) and
altered one (binaryOperationWithNetworkInterfaceModelOperand) can deal
only with NetworkInterfaceModel and not with NetworkItemModel.
Change-Id: Ie84e1c7fb7324279b5a8687d1ad6ffc605f921a0
Signed-off-by: Martin Mucha <mmu...@redhat.com>
---
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java
1 file changed, 175 insertions(+), 120 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/41/37641/1
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java
index c4272d9..de7fd96 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/NetworkOperationFactory.java
@@ -14,7 +14,12 @@
* The Factory also generates Menu Items for these Operations.
*
*/
+@SuppressWarnings("ChainOfInstanceofChecks")
public class NetworkOperationFactory {
+
+ public static NetworkOperation operationFor(NetworkItemModel<?> op1,
NetworkItemModel<?> op2) {
+ return operationFor(op1, op2, false);
+ }
/**
* Gets the valid Operation involving the two operands.<BR>
@@ -25,139 +30,189 @@
* @return
*/
public static NetworkOperation operationFor(NetworkItemModel<?> op1,
NetworkItemModel<?> op2, boolean isDrag) {
- // no valid operation for external networks or networks attached via
label
- if (op1 instanceof LogicalNetworkModel) {
- LogicalNetworkModel network = (LogicalNetworkModel) op1;
- if (network.getEntity().isExternal() ||
network.isAttachedViaLabel()) {
+ if (noValidOperationForFirstOperand(op1)) {
+ return NetworkOperation.NULL_OPERATION;
+ }
+
+ boolean unaryOperation = op2 == null;
+ if (unaryOperation) {
+ return handleUnaryOperation(op1, isDrag);
+ } else {
+ return handleBinaryOperation(op1, op2);
+ }
+ }
+
+ private static NetworkOperation handleUnaryOperation(NetworkItemModel<?>
op1, boolean isDrag) {
+ // unary operation dragging op1 to nowhere
+
+ // op1 is a bond, break it
+ if (op1 instanceof BondNetworkInterfaceModel) {
+ return NetworkOperation.BREAK_BOND;
+ }
+
+ // op1 is an interface, if it's bonded remove from bond
+ if (op1 instanceof NetworkInterfaceModel) {
+ NetworkInterfaceModel nic = (NetworkInterfaceModel) op1;
+ if (nic.isBonded()) {
+ return NetworkOperation.REMOVE_FROM_BOND;
+ } else {
return NetworkOperation.NULL_OPERATION;
}
}
- // no valid operation for network labels
- if (op1 instanceof NetworkLabelModel) {
- return NetworkOperation.NULL_OPERATION;
- }
-
- // unary operation dragging op1 to nowhere
- if (op2 == null) {
-
- // op1 is a bond, break it
- if (op1 instanceof BondNetworkInterfaceModel) {
- return NetworkOperation.BREAK_BOND;
- }
- // op1 is an interface, if it's bonded remove from bond
- else if (op1 instanceof NetworkInterfaceModel) {
- NetworkInterfaceModel nic = (NetworkInterfaceModel) op1;
- if (nic.isBonded()) {
- return NetworkOperation.REMOVE_FROM_BOND;
- }
- }
- // op1 is a network, detach it if already attached to a NIC
- else if (op1 instanceof LogicalNetworkModel) {
- LogicalNetworkModel network = (LogicalNetworkModel) op1;
- if (network.isAttached()) {
- if (!network.isManaged()) {
- if (isDrag) {
- return NetworkOperation.NULL_OPERATION_UNMANAGED;
- } else {
- return NetworkOperation.REMOVE_UNMANAGED_NETWORK;
- }
+ // op1 is a network, detach it if already attached to a NIC
+ if (op1 instanceof LogicalNetworkModel) {
+ LogicalNetworkModel network = (LogicalNetworkModel) op1;
+ if (network.isAttached()) {
+ if (!network.isManaged()) {
+ if (isDrag) {
+ return NetworkOperation.NULL_OPERATION_UNMANAGED;
+ } else {
+ return NetworkOperation.REMOVE_UNMANAGED_NETWORK;
}
+ } else {
return NetworkOperation.DETACH_NETWORK;
}
- }
- }
- // binary operation joining items together - in most cases valid iff
their networks comply
- else if (op2 instanceof NetworkInterfaceModel) {
- NetworkInterfaceModel dst = (NetworkInterfaceModel) op2;
-
- // first collect the networks into one set
- Set<LogicalNetworkModel> networks = new
HashSet<LogicalNetworkModel>();
- networks.addAll(dst.getItems());
-
- // op1 is a NIC, verify that it isn't already part of a bond or
dragged unto itself
- if (op1 instanceof NetworkInterfaceModel) {
- NetworkInterfaceModel src = (NetworkInterfaceModel) op1;
- if (src.isBonded() || src.equals(dst)) {
- return NetworkOperation.NULL_OPERATION;
- }
- networks.addAll(((NetworkInterfaceModel) op1).getItems());
- }
- // op1 is a network, verify that it isn't dragged unto the NIC
already containing it
- else if (op1 instanceof LogicalNetworkModel) {
- if (!networks.add((LogicalNetworkModel) op1)) {
- return NetworkOperation.NULL_OPERATION;
- }
- }
-
- // go over the networks and check whether they comply, if not -
the reason is important
- boolean vlanFound = false;
- String nonVlanVmNetwork = null;
- int nonVlanCounter = 0;
- for (LogicalNetworkModel network : networks) {
- if (!network.isManaged()) {
- if (op1 instanceof LogicalNetworkModel) {
- return NetworkOperation.NULL_OPERATION_UNMANAGED;
- } else if (op1 instanceof NetworkInterfaceModel) {
- dst.setCulpritNetwork(network.getName());
- return NetworkOperation.NULL_OPERATION_BOND_UNMANAGED;
- }
- } else if (!network.isInSync()) {
- if (op1 instanceof LogicalNetworkModel) {
- return NetworkOperation.NULL_OPERATION_OUT_OF_SYNC;
- } else if (op1 instanceof NetworkInterfaceModel) {
- dst.setCulpritNetwork(network.getName());
- return
NetworkOperation.NULL_OPERATION_BOND_OUT_OF_SYNC;
- }
- }
- if (network.hasVlan()) {
- vlanFound = true;
- } else if (network.getEntity().isVmNetwork()) {
- nonVlanVmNetwork = network.getName();
- ++nonVlanCounter;
- } else {
- ++nonVlanCounter;
- }
- if (nonVlanCounter > 1) {
- if (op1 instanceof LogicalNetworkModel) {
- return
NetworkOperation.NULL_OPERATION_TOO_MANY_NON_VLANS;
- } else if (op1 instanceof NetworkInterfaceModel) {
- dst.setCulpritNetwork(network.getName());
- return
NetworkOperation.NULL_OPERATION_BOND_TOO_MANY_NON_VLANS;
- }
- } else if (nonVlanVmNetwork != null && vlanFound) {
- if (op1 instanceof LogicalNetworkModel) {
- return NetworkOperation.NULL_OPERATION_VM_WITH_VLANS;
- } else if (op1 instanceof NetworkInterfaceModel) {
- dst.setCulpritNetwork(nonVlanVmNetwork);
- return
NetworkOperation.NULL_OPERATION_BOND_VM_WITH_VLANS;
- }
- }
- }
-
- // networks comply, all that's left is to return the correct
operation
- if (op1 instanceof LogicalNetworkModel) {
- return NetworkOperation.ATTACH_NETWORK;
- } else if (op1 instanceof BondNetworkInterfaceModel) {
- if (op2 instanceof BondNetworkInterfaceModel) {
- return NetworkOperation.JOIN_BONDS;
- } else {
- return NetworkOperation.EXTEND_BOND_WITH;
- }
- } else if (op1 instanceof NetworkInterfaceModel) {
- if (op2 instanceof BondNetworkInterfaceModel) {
- return NetworkOperation.ADD_TO_BOND;
- } else {
- return NetworkOperation.BOND_WITH;
- }
+ } else {
+ return NetworkOperation.NULL_OPERATION;
}
}
return NetworkOperation.NULL_OPERATION;
}
- public static NetworkOperation operationFor(NetworkItemModel<?> op1,
NetworkItemModel<?> op2) {
- return operationFor(op1, op2, false);
+ private static NetworkOperation handleBinaryOperation(NetworkItemModel<?>
op1, NetworkItemModel<?> op2) {
+ // binary operation joining items together - in most cases valid iff
their networks comply
+ if (op2 instanceof NetworkInterfaceModel) {
+ return
binaryOperationWithNetworkInterfaceModelAsSecondOperand(op1,
(NetworkInterfaceModel) op2);
+ }
+
+ return NetworkOperation.NULL_OPERATION;
+ }
+
+ private static NetworkOperation
binaryOperationWithNetworkInterfaceModelAsSecondOperand(NetworkItemModel<?> op1,
+ NetworkInterfaceModel dst) {
+
+ // first collect the networks into one set
+ Set<LogicalNetworkModel> networks = new HashSet<>();
+ networks.addAll(dst.getItems());
+
+ // op1 is a NIC, verify that it isn't already part of a bond or
dragged unto itself
+ if (op1 instanceof NetworkInterfaceModel) {
+ NetworkInterfaceModel src = (NetworkInterfaceModel) op1;
+ if (src.isBonded() || src.equals(dst)) { //TODO MM: is there a
place where 'A op A' is valid?
+ return NetworkOperation.NULL_OPERATION;
+ } else {
+ networks.addAll(((NetworkInterfaceModel) op1).getItems());
+ }
+ } else {
+ // op1 is a network, verify that it isn't dragged unto the NIC
already containing it
+ if (op1 instanceof LogicalNetworkModel) {
+ if (!networks.add((LogicalNetworkModel) op1)) {
+ return NetworkOperation.NULL_OPERATION;
+ }
+ }
+ }
+
+ // go over the networks and check whether they comply, if not - the
reason is important
+ boolean vlanFound = false;
+ String nonVlanVmNetwork = null;
+ int nonVlanCounter = 0;
+ for (LogicalNetworkModel network : networks) {
+ if (!network.isManaged()) {
+ if (op1 instanceof LogicalNetworkModel) {
+ return NetworkOperation.NULL_OPERATION_UNMANAGED;
+ }
+
+ if (op1 instanceof NetworkInterfaceModel) {
+ dst.setCulpritNetwork(network.getName());
+ return NetworkOperation.NULL_OPERATION_BOND_UNMANAGED;
+ }
+ } else {
+ if (!network.isInSync()) {
+ if (op1 instanceof LogicalNetworkModel) {
+ return NetworkOperation.NULL_OPERATION_OUT_OF_SYNC;
+ }
+
+ if (op1 instanceof NetworkInterfaceModel) {
+ dst.setCulpritNetwork(network.getName());
+ return
NetworkOperation.NULL_OPERATION_BOND_OUT_OF_SYNC;
+ }
+ }
+ }
+
+ if (network.hasVlan()) {
+ vlanFound = true;
+ } else {
+ ++nonVlanCounter;
+ if (network.getEntity().isVmNetwork()) {
+ nonVlanVmNetwork = network.getName();
+ }
+ }
+
+ if (nonVlanCounter > 1) {
+ if (op1 instanceof LogicalNetworkModel) {
+ return NetworkOperation.NULL_OPERATION_TOO_MANY_NON_VLANS;
+ }
+
+ if (op1 instanceof NetworkInterfaceModel) {
+ dst.setCulpritNetwork(network.getName());
+ return
NetworkOperation.NULL_OPERATION_BOND_TOO_MANY_NON_VLANS;
+ }
+ } else {
+ if (nonVlanVmNetwork != null && vlanFound) {
+ if (op1 instanceof LogicalNetworkModel) {
+ return NetworkOperation.NULL_OPERATION_VM_WITH_VLANS;
+ }
+
+ if (op1 instanceof NetworkInterfaceModel) {
+ dst.setCulpritNetwork(nonVlanVmNetwork);
+ return
NetworkOperation.NULL_OPERATION_BOND_VM_WITH_VLANS;
+ }
+ }
+ }
+ }
+
+ // networks comply, all that's left is to return the correct operation
+ if (op1 instanceof LogicalNetworkModel) {
+ return NetworkOperation.ATTACH_NETWORK;
+ }
+
+ if (op1 instanceof BondNetworkInterfaceModel) {
+ if (dst instanceof BondNetworkInterfaceModel) {
+ return NetworkOperation.JOIN_BONDS;
+ } else {
+ return NetworkOperation.EXTEND_BOND_WITH;
+ }
+ }
+
+ if (op1 instanceof NetworkInterfaceModel) {
+ if (dst instanceof BondNetworkInterfaceModel) {
+ return NetworkOperation.ADD_TO_BOND;
+ } else {
+ return NetworkOperation.BOND_WITH;
+ }
+ }
+
+ return NetworkOperation.NULL_OPERATION;
+
+ }
+
+ private static boolean noValidOperationForFirstOperand(NetworkItemModel<?>
op1) {
+ // no valid operation for external networks or networks attached via
label
+ if (op1 instanceof LogicalNetworkModel) {
+ LogicalNetworkModel network = (LogicalNetworkModel) op1;
+ if (network.getEntity().isExternal() ||
network.isAttachedViaLabel()) {
+ return true;
+ }
+ }
+
+ // no valid operation for network labels
+ if (op1 instanceof NetworkLabelModel) {
+ return true;
+ }
+
+ return false;
}
private final List<LogicalNetworkModel> allNetworks;
--
To view, visit http://gerrit.ovirt.org/37641
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie84e1c7fb7324279b5a8687d1ad6ffc605f921a0
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
