Tal Nisan has uploaded a new change for review. Change subject: core: Change existing disk validation CDA message in case of null disk alias ......................................................................
core: Change existing disk validation CDA message in case of null disk alias When try to import a VM which one or more of it's disks already exist on the system there's a CDA message containing the disk aliases of the existing disks, the aliases are taken from the disks in the export domian but in case the alias is null (mainly in backups of versions <V3.0, P2V, V2V etc..) the alias was displayed as a blank string, the CDA message was change and in case the alias is null the alias will be taken from the disk in the system Change-Id: I07b794b0fe9bdc5c83a62a74b82653ce209327b7 Signed-off-by: Tal Nisan <tni...@redhat.com> Bug-url: https://bugzilla.redhat.com/928864 --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java 2 files changed, 32 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/76/24276/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java index 59591c7..d808c1a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskImagesValidator.java @@ -49,6 +49,10 @@ return diskImagesNotInStatus(ImageStatus.LOCKED, VdcBllMessages.ACTION_TYPE_FAILED_DISKS_LOCKED); } + protected DiskImage getExistingDisk(Guid id) { + return getDbFacade().getDiskImageDao().get(id); + } + protected boolean isDiskExists(Guid id) { return DbFacade.getInstance().getBaseDiskDao().exists(id); } @@ -62,8 +66,9 @@ List<String> existingDisksAliases = new ArrayList<String>(); for (DiskImage diskImage : diskImages) { - if (isDiskExists(diskImage.getId())) { - existingDisksAliases.add(diskImage.getDiskAlias()); + DiskImage existingDisk = getExistingDisk(diskImage.getId()); + if (existingDisk != null) { + existingDisksAliases.add(diskImage.getDiskAlias() == null ? existingDisk.getDiskAlias() : diskImage.getDiskAlias()); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java index 81a6292..a62b4bf 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskImagesValidatorTest.java @@ -122,16 +122,37 @@ @Test public void diskImagesAlreadyExistBothExist() { - doReturn(true).when(validator).isDiskExists(any(Guid.class)); + doReturn(new DiskImage()).when(validator).getExistingDisk(any(Guid.class)); assertThat(validator.diskImagesAlreadyExist(), both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST)).and(replacements (hasItem(createAliasReplacements(disk1, disk2))))); } + /** + * Test a case when the two validated disks exists and have a null disk alias, in that case the disk aliases in + * the CDA message should be taken from the disks existing on the setup + */ + @Test + public void diskImagesAlreadyDiskInImportWithNullAlias() { + disk1.setDiskAlias(null); + disk2.setDiskAlias(null); + DiskImage existingImage1 = new DiskImage(); + existingImage1.setDiskAlias("existingDiskAlias1"); + DiskImage existingImage2 = new DiskImage(); + existingImage2.setDiskAlias("existingDiskAlias2"); + + doReturn(existingImage1).when(validator).getExistingDisk(disk1.getId()); + doReturn(existingImage2).when(validator).getExistingDisk(disk2.getId()); + assertThat(validator.diskImagesAlreadyExist(), + both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST)).and(replacements + (hasItem(createAliasReplacements(existingImage1, existingImage2))))); + } + + @Test public void diskImagesAlreadyExistOneExist() { - doReturn(true).when(validator).isDiskExists(disk1.getId()); - doReturn(false).when(validator).isDiskExists(disk2.getId()); + doReturn(new DiskImage()).when(validator).getExistingDisk(disk1.getId()); + doReturn(null).when(validator).getExistingDisk(disk2.getId()); assertThat(validator.diskImagesAlreadyExist(), both(failsWith(VdcBllMessages.ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST)).and(replacements (hasItem(createAliasReplacements(disk1))))); @@ -139,7 +160,7 @@ @Test public void diskImagesAlreadyExistBothDoesntExist() { - doReturn(false).when(validator).isDiskExists(any(Guid.class)); + doReturn(null).when(validator).getExistingDisk(any(Guid.class)); assertEquals(validator.diskImagesAlreadyExist(), ValidationResult.VALID); } -- To view, visit http://gerrit.ovirt.org/24276 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I07b794b0fe9bdc5c83a62a74b82653ce209327b7 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <tni...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
