Allon Mureinik has uploaded a new change for review. Change subject: core: Extract disk alias generation methods ......................................................................
core: Extract disk alias generation methods Extracted the methods used to generate disk alias from ImagesHandler to a new DiskUtils class. This was done for two reasons: * These methods were in the wrong place - disk aliases are used for all disks, not just images. * To allow these methods to be used outside of the bll package, e.g., in by the OVF handling classes (see next patch). This patch contains: * The new DiskUtils class with the getSuggestedDiskAlias and getDefaultDiskAlias moved to it, along with some constants required by them. * Reducing the visibility of said constants compared to their visibility under ImagesHandler. * Some minor improvements in the methods' documentation. * Refactoring of all the usages of the original methods to now use them from their new location. * A unit test for these methods. Change-Id: Iaf2a1147c022f4cdb7c5658818667af4b51e6aac Relates-To: https://bugzilla.redhat.com/show_bug.cgi?id=872506 Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java A backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/disks/DiskUtils.java A backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/disks/DiskUtilsTest.java 6 files changed, 109 insertions(+), 36 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/73/9273/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java index fe50ca0..6a1a738 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java @@ -2,6 +2,8 @@ import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.queries.GetAllDisksByVmIdParameters; +import org.ovirt.engine.core.utils.disks.DiskUtils; + public class GetNextAvailableDiskAliasNameByVMIdQuery<P extends GetAllDisksByVmIdParameters> extends QueriesCommandBase<P> { @@ -19,7 +21,7 @@ if (vm != null) { updateDisksFromDb(vm); suggestedDiskName = - ImagesHandler.getDefaultDiskAlias(vm.getvm_name(), Integer.toString(vm.getDiskMapCount() + 1)); + DiskUtils.getDefaultDiskAlias(vm.getvm_name(), Integer.toString(vm.getDiskMapCount() + 1)); } getQueryReturnValue().setReturnValue(suggestedDiskName); } 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 95f028d..9d28aa7 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 @@ -48,14 +48,14 @@ import org.ovirt.engine.core.compat.backendcompat.Path; import org.ovirt.engine.core.dal.VdcBllMessages; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.utils.disks.DiskUtils; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; + public final class ImagesHandler { - private static final String DISK = "_Disk"; public static final Guid BlankImageTemplateId = new Guid("00000000-0000-0000-0000-000000000000"); - public static final String DefaultDriveName = "1"; - protected static final Log log = LogFactory.getLog(ImagesHandler.class); + private static final Log log = LogFactory.getLog(ImagesHandler.class); /** * The following method will find all images and storages where they located for provide template and will fill an @@ -117,41 +117,12 @@ vmName = vm.getvm_name(); count = vm.getDiskMapCount() + 1; } - disk.setDiskAlias(getSuggestedDiskAlias(disk, vmName, count)); + disk.setDiskAlias(DiskUtils.getSuggestedDiskAlias(disk, vmName, count)); return true; } else { log.errorFormat("Disk object is null"); return false; } - } - - /** - * Retrieve disk alias name, if the alias name is null returns the default alias name. - * - * @param disk - * - Disk which disk alias is being initialized in. - * @param diskPrefix - * - The prefix for disk alias if needs to be initialized. - */ - public static String getSuggestedDiskAlias(BaseDisk disk, String diskPrefix, int count) { - String diskAlias; - if (disk == null) { - diskAlias = getDefaultDiskAlias(diskPrefix, DefaultDriveName); - log.warnFormat("Disk object is null, the suggested default disk alias to be used is {0}", - diskAlias); - } else { - diskAlias = disk.getDiskAlias(); - if (StringUtils.isEmpty(diskAlias)) { - diskAlias = getDefaultDiskAlias(diskPrefix, String.valueOf(count)); - log.infoFormat("Disk alias retrieved from the client is null or empty, the suggested default disk alias to be used is {0}", - diskAlias); - } - } - return diskAlias; - } - - public static String getDefaultDiskAlias(String prefix, String suffix) { - return prefix + DISK + suffix; } public static Map<Guid, List<DiskImage>> buildStorageToDiskMap(Collection<DiskImage> images, diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java index 523d644..77849fd 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java @@ -52,8 +52,10 @@ import org.ovirt.engine.core.dal.VdcBllMessages; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.StorageDomainStaticDAO; +import org.ovirt.engine.core.utils.disks.DiskUtils; import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; + @DisableInPrepareMode @NonTransactiveCommandAttribute(forceCompensation = true) @@ -338,7 +340,7 @@ getCompensationContext().snapshotNewEntity(image.getImage()); getCompensationContext().snapshotNewEntity(map); if (!DbFacade.getInstance().getBaseDiskDao().exists(image.getId())) { - image.setDiskAlias(ImagesHandler.getSuggestedDiskAlias(image, getVmTemplateName(), count)); + image.setDiskAlias(DiskUtils.getSuggestedDiskAlias(image, getVmTemplateName(), count)); count++; DbFacade.getInstance().getBaseDiskDao().save(image); getCompensationContext().snapshotNewEntity(image); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java index 8426886..d80cc60 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java @@ -36,11 +36,13 @@ import org.ovirt.engine.core.dao.VmNetworkInterfaceDAO; import org.ovirt.engine.core.dao.VmStaticDAO; import org.ovirt.engine.core.dao.VmTemplateDAO; +import org.ovirt.engine.core.utils.disks.DiskUtils; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; import org.ovirt.engine.core.utils.ovf.OvfManager; import org.ovirt.engine.core.utils.ovf.OvfReaderException; import org.ovirt.engine.core.utils.ovf.VMStaticOvfLogHandler; + /** * The {@link Snapshot} manager is used to easily add/update/remove snapshots. @@ -362,7 +364,7 @@ } ImagesHandler.addDiskToVm(diskImage, vmId); } - diskImage.setDiskAlias(ImagesHandler.getSuggestedDiskAlias(diskImage, vmName, count)); + diskImage.setDiskAlias(DiskUtils.getSuggestedDiskAlias(diskImage, vmName, count)); count++; } removeDisksNotInSnapshot(vmId, diskIdsFromSnapshot); diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/disks/DiskUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/disks/DiskUtils.java new file mode 100644 index 0000000..b4d4a00 --- /dev/null +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/disks/DiskUtils.java @@ -0,0 +1,51 @@ +package org.ovirt.engine.core.utils.disks; + +import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.common.businessentities.BaseDisk; +import org.ovirt.engine.core.utils.log.Log; +import org.ovirt.engine.core.utils.log.LogFactory; + +/** A utility class for disk related functions */ +public class DiskUtils { + + /** The logging category */ + private static final Log log = LogFactory.getLog(DiskUtils.class); + + /** The default drive name */ + private static final String DefaultDriveName = "1"; + + /** The default string to use between a disk alias' prefix and suffix */ + public static final String DISK = "_Disk"; + + /** + * Suggests an alias for a disk. + * If the disk does not already have an alias, one will be generated for it. + * + * @param disk + * - Disk which disk alias is being initialized in. + * @param diskPrefix + * - The prefix for disk alias if needs to be initialized. + * @return The suggested alias + */ + public static String getSuggestedDiskAlias(BaseDisk disk, String diskPrefix, int count) { + String diskAlias; + if (disk == null) { + diskAlias = getDefaultDiskAlias(diskPrefix, DefaultDriveName); + log.warnFormat("Disk object is null, the suggested default disk alias to be used is {0}", + diskAlias); + } else { + diskAlias = disk.getDiskAlias(); + if (StringUtils.isEmpty(diskAlias)) { + diskAlias = getDefaultDiskAlias(diskPrefix, String.valueOf(count)); + log.infoFormat("Disk alias retrieved from the client is null or empty, the suggested default disk alias to be used is {0}", + diskAlias); + } + } + return diskAlias; + } + + public static String getDefaultDiskAlias(String prefix, String suffix) { + return prefix + DISK + suffix; + } + +} diff --git a/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/disks/DiskUtilsTest.java b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/disks/DiskUtilsTest.java new file mode 100644 index 0000000..24ef74b --- /dev/null +++ b/backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/disks/DiskUtilsTest.java @@ -0,0 +1,45 @@ +package org.ovirt.engine.core.utils.disks; + +import static org.junit.Assert.assertEquals; + +import org.junit.Before; +import org.junit.Test; +import org.ovirt.engine.core.common.businessentities.DiskImage; + +/** A test case for {@link DiskUtils} */ +public class DiskUtilsTest { + + /** The prefix to use for all tests */ + private static final String prefix = "PREFIX"; + + /** The disk to use for testing */ + private DiskImage disk; + + @Before + public void setUp() { + disk = new DiskImage(); + } + + @Test + public void testNullDisk() { + assertEquals("null disk does not give the default name", + prefix + "_Disk1", + DiskUtils.getSuggestedDiskAlias(null, prefix, 1)); + } + + @Test + public void testNullAliasDisk() { + disk.setDiskAlias(null); + assertEquals("null disk does not give the default name", + prefix + "_Disk1", + DiskUtils.getSuggestedDiskAlias(disk, prefix, 1)); + } + + @Test + public void testNotNullAliasDisk() { + disk.setDiskAlias("someAlias"); + assertEquals("null disk does not give the default name", + disk.getDiskAlias(), + DiskUtils.getSuggestedDiskAlias(disk, prefix, 1)); + } +} -- To view, visit http://gerrit.ovirt.org/9273 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iaf2a1147c022f4cdb7c5658818667af4b51e6aac 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
