Michael Pasternak has uploaded a new change for review. Change subject: restapi: Create ISCSI LUN Disk: Connection details should be mandatory #864840 ......................................................................
restapi: Create ISCSI LUN Disk: Connection details should be mandatory #864840 extra validation according to [1] [1] https://bugzilla.redhat.com/show_bug.cgi?id=864840#c7 Change-Id: Icfd508f225a1386f6a588c9d58cc1df286a82cbb Signed-off-by: Michael Pasternak <mpast...@redhat.com> --- M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDisksResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java 3 files changed, 53 insertions(+), 15 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/97/8997/1 diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDisksResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDisksResource.java index f884b23..612bb6d 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDisksResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDisksResource.java @@ -10,6 +10,8 @@ import org.ovirt.engine.api.model.StorageType; import org.ovirt.engine.api.resource.DiskResource; import org.ovirt.engine.api.resource.DisksResource; +import org.ovirt.engine.api.restapi.logging.Messages; +import org.ovirt.engine.api.restapi.resource.BaseBackendResource.WebFaultException; import org.ovirt.engine.api.restapi.resource.utils.DiskResourceUtils; import org.ovirt.engine.core.common.action.AddDiskParameters; import org.ovirt.engine.core.common.action.RemoveDiskParameters; @@ -48,15 +50,21 @@ } protected void validateDiskForCreation(Disk disk) { - validateParameters(disk, "format", "interface"); + validateParameters(disk, 3, "format", "interface"); if (DiskResourceUtils.isLunDisk(disk)) { - validateParameters(disk.getLunStorage(), "type"); // when creating a LUN disk, user must specify type. + validateParameters(disk.getLunStorage(), 3, "type"); // when creating a LUN disk, user must specify type. StorageType storageType = StorageType.fromValue(disk.getLunStorage().getType()); - if (storageType!=null && storageType==StorageType.ISCSI) { - validateParameters(disk.getLunStorage().getLogicalUnits().get(0), "address", "target", "port"); + if (storageType != null && storageType == StorageType.ISCSI) { + validateParameters(disk.getLunStorage().getLogicalUnits().get(0), 3, "address", "target", "port", "id"); } + } else if (disk.isSetLunStorage() && disk.getLunStorage().getLogicalUnits().isEmpty()) { + // TODO: Implement nested entity existence validation infra for validateParameters() + throw new WebFaultException(null, + localize(Messages.INCOMPLETE_PARAMS_REASON), + localize(Messages.INCOMPLETE_PARAMS_DETAIL_TEMPLATE, "LogicalUnit", "", "add"), + Response.Status.BAD_REQUEST); } else { - validateParameters(disk, "provisionedSize|size"); // Non lun disks require size + validateParameters(disk, 3, "provisionedSize|size"); // Non lun disks require size } validateEnums(Disk.class, disk); } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java index 5412280..650a93c 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java @@ -27,7 +27,9 @@ import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.api.restapi.logging.Messages; import org.ovirt.engine.api.restapi.resource.AbstractBackendSubResource.ParametersProvider; +import org.ovirt.engine.api.restapi.resource.BaseBackendResource.WebFaultException; import org.ovirt.engine.api.restapi.resource.utils.DiskResourceUtils; public class BackendVmDisksResource @@ -184,15 +186,21 @@ } protected void validateDiskForCreation(Disk disk) { - validateParameters(disk, "format", "interface"); + validateParameters(disk, 3, "format", "interface"); if (DiskResourceUtils.isLunDisk(disk)) { - validateParameters(disk.getLunStorage(), "type"); // when creating a LUN disk, user must specify type. + validateParameters(disk.getLunStorage(), 3, "type"); // when creating a LUN disk, user must specify type. StorageType storageType = StorageType.fromValue(disk.getLunStorage().getType()); if (storageType != null && storageType == StorageType.ISCSI) { - validateParameters(disk.getLunStorage().getLogicalUnits().get(0), "address", "target", "port"); + validateParameters(disk.getLunStorage().getLogicalUnits().get(0), 3, "address", "target", "port", "id"); } + } else if (disk.isSetLunStorage() && disk.getLunStorage().getLogicalUnits().isEmpty()) { + // TODO: Implement nested entity existence validation infra for validateParameters() + throw new WebFaultException(null, + localize(Messages.INCOMPLETE_PARAMS_REASON), + localize(Messages.INCOMPLETE_PARAMS_DETAIL_TEMPLATE, "LogicalUnit", "", "add"), + Response.Status.BAD_REQUEST); } else { - validateParameters(disk, "provisionedSize|size"); // Non lun disks require size + validateParameters(disk, 3, "provisionedSize|size"); // Non lun disks require size } validateEnums(Disk.class, disk); } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java index 0107981..4b27754 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java @@ -343,7 +343,8 @@ collection.add(model); fail("expected WebApplicationException on incomplete parameters"); } catch (WebApplicationException wae) { - verifyIncompleteException(wae, "Disk", "validateDiskForCreation", "format", "interface"); + // Because of extra frame offset used current method name in test, while in real world used "add" method name + verifyIncompleteException(wae, "Disk", "testAddIncompleteParameters", "format", "interface"); } } @@ -356,7 +357,8 @@ collection.add(model); fail("expected WebApplicationException on incomplete parameters"); } catch (WebApplicationException wae) { - verifyIncompleteException(wae, "Disk", "validateDiskForCreation", "provisionedSize|size"); + // Because of extra frame offset used current method name in test, while in real world used "add" method name + verifyIncompleteException(wae, "Disk", "testAddIncompleteParameters_2", "provisionedSize|size"); } } @@ -370,7 +372,23 @@ collection.add(model); fail("expected WebApplicationException on incomplete parameters"); } catch (WebApplicationException wae) { - verifyIncompleteException(wae, "Storage", "validateDiskForCreation", "type"); + // Because of extra frame offset used current method name in test, while in real world used "add" method name + verifyIncompleteException(wae, "Storage", "testAddLunDisk_MissingType", "type"); + } + } + + @Test + public void testAddLunDisk_MissingId() { + Disk model = createIscsiLunDisk(); + model.getLunStorage().getLogicalUnits().get(0).setId(null); + setUriInfo(setUpBasicUriExpectations()); + control.replay(); + try { + collection.add(model); + fail("expected WebApplicationException on incomplete parameters"); + } catch (WebApplicationException wae) { + // Because of extra frame offset used current method name in test, while in real world used "add" method name + verifyIncompleteException(wae, "LogicalUnit", "testAddLunDisk_MissingId", "id"); } } @@ -384,7 +402,8 @@ collection.add(model); fail("expected WebApplicationException on incomplete parameters"); } catch (WebApplicationException wae) { - verifyIncompleteException(wae, "LogicalUnit", "validateDiskForCreation", "address"); + // Because of extra frame offset used current method name in test, while in real world used "add" method name + verifyIncompleteException(wae, "LogicalUnit", "testAddIscsiLunDisk_IncompleteParameters_ConnectionAddress", "address"); } } @@ -398,7 +417,8 @@ collection.add(model); fail("expected WebApplicationException on incomplete parameters"); } catch (WebApplicationException wae) { - verifyIncompleteException(wae, "LogicalUnit", "validateDiskForCreation", "target"); + // Because of extra frame offset used current method name in test, while in real world used "add" method name + verifyIncompleteException(wae, "LogicalUnit", "testAddIscsiLunDisk_IncompleteParameters_ConnectionTarget", "target"); } } @@ -412,7 +432,8 @@ collection.add(model); fail("expected WebApplicationException on incomplete parameters"); } catch (WebApplicationException wae) { - verifyIncompleteException(wae, "LogicalUnit", "validateDiskForCreation", "port"); + // Because of extra frame offset used current method name in test, while in real world used "add" method name + verifyIncompleteException(wae, "LogicalUnit", "testAddIscsiLunDisk_IncompleteParameters_ConnectionPort", "port"); } } @@ -421,6 +442,7 @@ model.setLunStorage(new Storage()); model.getLunStorage().setType("iscsi"); model.getLunStorage().getLogicalUnits().add(new LogicalUnit()); + model.getLunStorage().getLogicalUnits().get(0).setId(GUIDS[0].toString()); model.getLunStorage().getLogicalUnits().get(0).setAddress(ISCSI_SERVER_ADDRESS); model.getLunStorage().getLogicalUnits().get(0).setTarget(ISCSI_SERVER_TARGET); model.getLunStorage().getLogicalUnits().get(0).setPort(ISCSI_SERVER_CONNECTION_PORT); -- To view, visit http://gerrit.ovirt.org/8997 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icfd508f225a1386f6a588c9d58cc1df286a82cbb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Pasternak <mpast...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
