snapshot: look up a duplicate snapshot name by volume and name

Query for an active snapshot with the given name on the volume instead of
listing every snapshot of the volume and comparing names, as asked in review.

Author: rootnomad

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java

@@ -53,6 +53,8 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long>, StateDao<Snap
 
     List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... status);
 
+    SnapshotVO findByVolumeIdAndNameNotInStatus(long volumeId, String name, Snapshot.State... status);
+
     /**
      * Retrieves a list of snapshots filtered by ids.
      * @param ids Snapshot ids.

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java

@@ -65,6 +65,7 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
     private SearchBuilder<SnapshotVO> InstanceIdSearch;
     private SearchBuilder<SnapshotVO> StatusSearch;
     private SearchBuilder<SnapshotVO> notInStatusSearch;
+    private SearchBuilder<SnapshotVO> volumeIdNameNotInStatusSearch;
     private GenericSearchBuilder<SnapshotVO, Long> CountSnapshotsByAccount;
     @Inject
     ResourceTagDao _tagsDao;
@@ -159,6 +160,12 @@ protected void init() {
         notInStatusSearch.and("status", notInStatusSearch.entity().getState(), SearchCriteria.Op.NOTIN);
         notInStatusSearch.done();
 
+        volumeIdNameNotInStatusSearch = createSearchBuilder();
+        volumeIdNameNotInStatusSearch.and("volumeId", volumeIdNameNotInStatusSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
+        volumeIdNameNotInStatusSearch.and("name", volumeIdNameNotInStatusSearch.entity().getName(), SearchCriteria.Op.EQ);
+        volumeIdNameNotInStatusSearch.and("status", volumeIdNameNotInStatusSearch.entity().getState(), SearchCriteria.Op.NOTIN);
+        volumeIdNameNotInStatusSearch.done();
+
         CountSnapshotsByAccount = createSearchBuilder(Long.class);
         CountSnapshotsByAccount.select(null, Func.COUNT, null);
         CountSnapshotsByAccount.and("account", CountSnapshotsByAccount.entity().getAccountId(), SearchCriteria.Op.EQ);
@@ -295,6 +302,15 @@ public List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... statu
         return listBy(sc, null);
     }
 
+    @Override
+    public SnapshotVO findByVolumeIdAndNameNotInStatus(long volumeId, String name, Snapshot.State... status) {
+        SearchCriteria<SnapshotVO> sc = volumeIdNameNotInStatusSearch.create();
+        sc.setParameters("volumeId", volumeId);
+        sc.setParameters("name", name);
+        sc.setParameters("status", (Object[]) status);
+        return findOneBy(sc);
+    }
+
     @Override
     public List<SnapshotVO> searchByVolumes(List<Long> volumeIds) {
         if (CollectionUtils.isEmpty(volumeIds)) {

server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java

@@ -1721,10 +1721,8 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName,
         if (snapshotName == null)
             snapshotName = vmDisplayName + "_" + volume.getName() + "_" + timeString;
 
-        for (SnapshotVO existingSnapshot : _snapshotDao.listByStatusNotIn(volumeId, Snapshot.State.Destroyed, Snapshot.State.Error)) {
-            if (snapshotName.equals(existingSnapshot.getName())) {
-                throw new InvalidParameterValueException(String.format("A snapshot with name [%s] already exists for volume %s.", snapshotName, volume));
-            }
+        if (_snapshotDao.findByVolumeIdAndNameNotInStatus(volumeId, snapshotName, Snapshot.State.Destroyed, Snapshot.State.Error) != null) {
+            throw new InvalidParameterValueException(String.format("A snapshot with name [%s] already exists for volume %s.", snapshotName, volume));
         }
 
         HypervisorType hypervisorType = HypervisorType.None;

server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java

@@ -121,10 +121,8 @@ public void testAllocSnapshotRejectsDuplicateNameForVolume() {
             Mockito.when(volume.getInstanceId()).thenReturn(null);
             Mockito.when(volume.getAccountId()).thenReturn(2L);
 
-            SnapshotVO existing = Mockito.mock(SnapshotVO.class);
-            Mockito.when(existing.getName()).thenReturn("dup");
-            Mockito.when(snapshotDao.listByStatusNotIn(volumeId, Snapshot.State.Destroyed, Snapshot.State.Error))
-                    .thenReturn(List.of(existing));
+            Mockito.when(snapshotDao.findByVolumeIdAndNameNotInStatus(volumeId, "dup", Snapshot.State.Destroyed, Snapshot.State.Error))
+                    .thenReturn(Mockito.mock(SnapshotVO.class));
 
             Assert.assertThrows(InvalidParameterValueException.class, () ->
                     snapshotManager.allocSnapshot(volumeId, Snapshot.MANUAL_POLICY_ID, "dup", null));