Moti Asayag has posted comments on this change.
Change subject: engine: Refactored code into NetworkValidator class
......................................................................
Patch Set 4: (2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/AddNetworkCommand.java
Line 61: }
Line 62:
Line 63: @Override
Line 64: public List<PermissionSubject> getPermissionCheckSubjects() {
Line 65: NGuid dataCenterId = getNetwork().getDataCenterId();
getNetwork() implemented as getParameters().getNetwork() - we since
networkExists() is performed after the validation, i think we should add a
check that getParameters().getNetwork() returns not null.
The result will be the same: the command will fail, however instead of "oVirt
internal error" and a long stacktrace to the log the user get a messag of:
"User is not authorized to perform this action".
Line 66: return Collections.singletonList(new
PermissionSubject(dataCenterId == null ? null
Line 67: : dataCenterId.getValue(),
Line 68: VdcObjectType.StoragePool,
getActionType().getActionGroup()));
Line 69: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java
Line 58: && validate(validatorOld.networkNotUsedByVms())
Line 59: && validate(validatorOld.networkNotUsedByTemplates())
Line 60: && validate(validatorOld.networkNotUsedByHosts());
Line 61: }
Line 62:
I imagined it as an extension of NetworkValidator:
protected class UpdateNetworkValidator extends NetworkValidator () {
public UpdateNetworkValidator(Network oldNetwork){..}
public ValidationResult notChangingManagementNetwork(String
newNetworkName) {...}
}
What do you think ?
Please move the class definition to the bottom of the class.
If you don't plan adding test for this class, please change its visibility to
private.
Line 63: protected class UpdateNetworkValidator {
Line 64:
Line 65: protected final Network oldNetwork, newNetwork;
Line 66:
--
To view, visit http://gerrit.ovirt.org/10940
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf40d81f0481c6b6dec141a71363888dc9e9a941
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
