Allon Mureinik has uploaded a new change for review. Change subject: core: ImagesHandler.getAllStorageIdsForImageIds ......................................................................
core: ImagesHandler.getAllStorageIdsForImageIds Extracted a method to get all the relevant storage domains IDs from a collection of DiskImages, and added some tests to it. This method will be used in subsequent patches in the effort to decouple ImagesHandler.PerformImagesChecks, but in the meanwhile it's main benefit is the added unit test coverage. Change-Id: If1b4d7231a03ea5a6d991609a65951fe4e279ef5 Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java 2 files changed, 47 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/31/12131/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java index c1a44c0..4c38b49 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -516,14 +517,13 @@ ArrayList<DiskImage> irsImages = new ArrayList<DiskImage>(); if (diskSpaceCheck || checkStorageDomain) { - Set<Guid> domainsIds = new HashSet<Guid>(); + Set<Guid> domainsIds; if (!Guid.Empty.equals(storageDomainId)) { - domainsIds.add(storageDomainId); + domainsIds = Collections.singleton(storageDomainId); } else { - for (DiskImage image : images) { - domainsIds.addAll(image.getstorage_ids()); - } + domainsIds = getAllStorageIdsForImageIds(images); } + for (Guid domainId : domainsIds) { StorageDomain domain = DbFacade.getInstance().getStorageDomainDao().getForStoragePool( domainId, storagePoolId); @@ -556,6 +556,18 @@ } /** + * @return A unique {@link Set} of all the storage domain IDs relevant to all the given images + * @param images The images to get the storage domain IDs for + */ + public static Set<Guid> getAllStorageIdsForImageIds(Collection<DiskImage> images) { + Set<Guid> domainsIds = new HashSet<Guid>(); + for (DiskImage image : images) { + domainsIds.addAll(image.getstorage_ids()); + } + return domainsIds; + } + + /** * Returns whether the storage domain is within the threshold * * @param storageDomain diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java index d276ac6..0d61a88 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java @@ -1,10 +1,16 @@ package org.ovirt.engine.core.bll; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Set; import org.junit.Before; import org.junit.Test; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.compat.Guid; /** A test case for {@link ImagesHandler} */ public class ImagesHandlerTest { @@ -12,12 +18,14 @@ /** The prefix to use for all tests */ private static final String prefix = "PREFIX"; - /** The disk to use for testing */ - private DiskImage disk; + /** The disks to use for testing */ + private DiskImage disk1; + private DiskImage disk2; @Before public void setUp() { - disk = new DiskImage(); + disk1 = new DiskImage(); + disk2 = new DiskImage(); } @Test @@ -29,17 +37,31 @@ @Test public void testGetSuggestedDiskAliasNullAliasDisk() { - disk.setDiskAlias(null); + disk1.setDiskAlias(null); assertEquals("disk with null alias does not give the default name", prefix + ImagesHandler.DISK + ImagesHandler.DefaultDriveName, - ImagesHandler.getSuggestedDiskAlias(disk, prefix, 1)); + ImagesHandler.getSuggestedDiskAlias(disk1, prefix, 1)); } @Test public void testGetSuggestedDiskAliasNotNullAliasDisk() { - disk.setDiskAlias("someAlias"); + disk1.setDiskAlias("someAlias"); assertEquals("a new alias was generated instead of returning the pre-defined one", - disk.getDiskAlias(), - ImagesHandler.getSuggestedDiskAlias(disk, prefix, 1)); + disk1.getDiskAlias(), + ImagesHandler.getSuggestedDiskAlias(disk1, prefix, 1)); + } + + public void testGetAllStorageIdsForImageIds() { + Guid sdIdShared = Guid.NewGuid(); + Guid sdId1 = Guid.NewGuid(); + Guid sdId2 = Guid.NewGuid(); + + disk1.setstorage_ids(new ArrayList<Guid>(Arrays.asList(sdId1, sdIdShared))); + disk2.setstorage_ids(new ArrayList<Guid>(Arrays.asList(sdId2, sdIdShared))); + + Set<Guid> result = ImagesHandler.getAllStorageIdsForImageIds(Arrays.asList(disk1, disk2)); + + assertEquals("Wrong number of Guids returned", 3, result.size()); + assertTrue("Wrong Guids returned", result.containsAll(Arrays.asList(sdId1, sdId2, sdIdShared))); } } -- To view, visit http://gerrit.ovirt.org/12131 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If1b4d7231a03ea5a6d991609a65951fe4e279ef5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
