Mike Kolesnik has posted comments on this change. Change subject: engine: Introduce VnicProfileValidator ......................................................................
Patch Set 22: (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVnicProfileCommand.java Line 18: protected boolean canDoAction() { Line 19: VnicProfileValidator validator = new VnicProfileValidator(getVnicProfile()); Line 20: return validate(validator.vnicProfileIsSet()) Line 21: && validate(validator.vnicProfileExists()) Line 22: && validate(validator.networkExists()) Why is it necessary? If you validate that the profile exists, and that the network didn't change, then what good would checking if the network exists would bring here? Line 23: && validate(validator.vnicProfileNameNotUsed()) Line 24: && validate(validator.networkNotChanged()); Line 25: } Line 26: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VnicProfileValidator.java Line 40: ? new ValidationResult(VdcBllMessages.VNIC_PROFILE_NOT_EXISTS) Line 41: : ValidationResult.VALID; Line 42: } Line 43: Line 44: public ValidationResult networkExists() { This is part of NetworkValidator?! Line 45: return getNetwork() == null Line 46: ? new ValidationResult(VdcBllMessages.NETWORK_NOT_EXISTS) Line 47: : ValidationResult.VALID; Line 48: } Line 109: } Line 110: Line 111: protected List<VM> getVmsUsingProfile() { Line 112: if (vms == null) { Line 113: vms = getDbFacade().getVmDao().getAllForVnicProfile(vnicProfile.getId()); Why are you caching this list? You don't seem to be reusing it... Also you're not caching the templates list so I'm not following the consistency here.. Line 114: } Line 115: Line 116: return vms; Line 117: } -- To view, visit http://gerrit.ovirt.org/16887 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6af74a398d1d7ffb97a1802305c56ad536acb346 Gerrit-PatchSet: 22 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches