Mike Kolesnik has submitted this change and it was merged. Change subject: engine: Clear syntax for writing validations ...................................................................... engine: Clear syntax for writing validations Normally when using ValidationResult to do validations the code looks like the following examples: 1. Check positive case (using ternary operator): return FeatureSupported.nonVmNetwork(getDataCenter().getcompatibility_version()) ? ValidationResult.VALID : new ValidationResult(VdcBllMessages.NON_VM_NETWORK_NOT_SUPPORTED_FOR_POOL_LEVEL); 2. Check negative case (using if): VDSStatus hostStatus = getVds().getStatus(); if (hostStatus != VDSStatus.Maintenance && hostStatus != VDSStatus.Up && hostStatus != VDSStatus.NonOperational) { return new ValidationResult( VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL, VdcBllMessages.VAR__HOST_STATUS__UP_MAINTENANCE_OR_NON_OPERATIONAL.name()); } return ValidationResult.VALID; 3. Check negative case (using ternary operator): return getVds() == null ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NOT_EXIST) : ValidationResult.VALID; And so forth.. The suggested syntax would enable simpler checking that is focused on a fluent syntax that's easy to write and read. To check a positive case you would write (Instead of example #1): return ValidationResult.failWith(VdcBllMessages.NON_VM_NETWORK_NOT_SUPPORTED_FOR_POOL_LEVEL) .unless(FeatureSupported.nonVmNetwork(getDataCenter().getcompatibility_version())); To check a negative case you would write (Instead of example #3): return ValidationResult.failWith(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NOT_EXIST) .when(getVds() == null); Example #2 could be rewritten in a similar matter (the condition could be simplified/extracted). The new syntax presents a more comprehensible way to read and write the validation logic, and abstracts the repetitive decision of returning the valid or invalid result. In the next patch there will be a small refactor to demonstrate the usage. In the future, the ValidationResult constructors could be hidden in favor of this API (after refactoring their usages). Change-Id: I2e40ce1bf69d2df45d3136a9f2db0afc00a02159 Signed-off-by: Mike Kolesnik <mkolesni@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ValidationResult.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ValidationResultTest.java 2 files changed, 112 insertions(+), 0 deletions(-) Approvals: Mike Kolesnik: Verified Moti Asayag: Looks good to me, approved Yair Zaslavsky: Looks good to me, but someone else must approve Oved Ourfali: Looks good to me, but someone else must approve -- To view, visit http://gerrit.ovirt.org/29414 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2e40ce1bf69d2df45d3136a9f2db0afc00a02159 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <mkolesni@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkolesni@redhat.com> Gerrit-Reviewer: Moti Asayag <masayag@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofrenkel@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourfali@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzaslavs@redhat.com> Gerrit-Reviewer: automation@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server