No error/warning on failed snapshot deletion #10676

modules/core/core-api/src/main/java/com/enonic/xp/node/DeleteSnapshotsResult.java

@@ -1,22 +1,32 @@
 package com.enonic.xp.node;
 
-import java.util.Collection;
-import java.util.HashSet;
 import java.util.Set;
 
 import com.google.common.collect.ImmutableSet;
 
 import com.enonic.xp.annotation.PublicApi;
-import com.enonic.xp.support.AbstractImmutableEntitySet;
 
-@Deprecated
 @PublicApi
 public class DeleteSnapshotsResult
-    extends AbstractImmutableEntitySet<String>
 {
+    private final Set<String> deletedSnapshots;
+
+    private final Set<String> failedSnapshots;
+
     private DeleteSnapshotsResult( final Builder builder )
     {
-        super( ImmutableSet.copyOf( builder.snapshotNames ) );
+        this.deletedSnapshots = builder.deletedSnapshots.build();
+        this.failedSnapshots = builder.failedSnapshots.build();
+    }
+
+    public Set<String> getDeletedSnapshots()
+    {
+        return deletedSnapshots;
+    }
+
+    public Set<String> getFailedSnapshots()
+    {
+        return failedSnapshots;
     }
 
     public static Builder create()
@@ -26,18 +36,19 @@ public static Builder create()
 
     public static class Builder
     {
-        private final Set<String> snapshotNames = new HashSet<>();
+        private final ImmutableSet.Builder<String> deletedSnapshots = ImmutableSet.builder();
 
+        private final ImmutableSet.Builder<String> failedSnapshots = ImmutableSet.builder();
 
         public Builder add( final String snapshotName )
         {
-            this.snapshotNames.add( snapshotName );
+            this.deletedSnapshots.add( snapshotName );
             return this;
         }
 
-        public Builder addAll( final Collection<String> snapshotNames )
+        public Builder addFailed( final String snapshotName )
         {
-            this.snapshotNames.addAll( snapshotNames );
+            this.failedSnapshots.add( snapshotName );
             return this;
         }
 

modules/core/core-api/src/main/java/com/enonic/xp/snapshot/SnapshotService.java

@@ -2,7 +2,6 @@
 
 import com.enonic.xp.node.DeleteSnapshotParams;
 import com.enonic.xp.node.DeleteSnapshotsResult;
-import com.enonic.xp.node.RemoveSnapshotsResult;
 import com.enonic.xp.node.RestoreParams;
 import com.enonic.xp.node.RestoreResult;
 import com.enonic.xp.node.SnapshotParams;
@@ -15,10 +14,7 @@ public interface SnapshotService
 
     RestoreResult restore( RestoreParams restoreParams );
 
-    @Deprecated
     DeleteSnapshotsResult delete( DeleteSnapshotParams params );
 
-    RemoveSnapshotsResult remove( DeleteSnapshotParams params );
-
     SnapshotResults list();
 }

modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/elasticsearch/snapshot/SnapshotServiceImpl.java

@@ -32,7 +32,6 @@
 import com.enonic.xp.event.EventPublisher;
 import com.enonic.xp.node.DeleteSnapshotParams;
 import com.enonic.xp.node.DeleteSnapshotsResult;
-import com.enonic.xp.node.RemoveSnapshotsResult;
 import com.enonic.xp.node.RestoreParams;
 import com.enonic.xp.node.RestoreResult;
 import com.enonic.xp.node.SnapshotParams;
@@ -197,18 +196,8 @@ private SnapshotResults doListSnapshots()
         return SnapshotResultsFactory.create( getSnapshotsResponse );
     }
 
-    @Deprecated
     @Override
     public DeleteSnapshotsResult delete( final DeleteSnapshotParams params )
-    {
-        return NodeHelper.runAsAdmin( () -> {
-            final RemoveSnapshotsResult result = doDelete( params );
-            return DeleteSnapshotsResult.create().addAll( result.getSnapshotNames() ).build();
-        } );
-    }
-
-    @Override
-    public RemoveSnapshotsResult remove( final DeleteSnapshotParams params )
     {
         return NodeHelper.runAsAdmin( () -> doDelete( params ) );
     }
@@ -320,9 +309,9 @@ private SnapshotInfo getSnapshot( final String snapshotName )
         }
     }
 
-    private RemoveSnapshotsResult doDelete( final DeleteSnapshotParams params )
+    private DeleteSnapshotsResult doDelete( final DeleteSnapshotParams params )
     {
-        final RemoveSnapshotsResult.Builder builder = RemoveSnapshotsResult.create();
+        final DeleteSnapshotsResult.Builder builder = DeleteSnapshotsResult.create();
 
         if ( !params.getSnapshotNames().isEmpty() )
         {
@@ -337,7 +326,7 @@ private RemoveSnapshotsResult doDelete( final DeleteSnapshotParams params )
         return builder.build();
     }
 
-    private void deleteByBefore( final RemoveSnapshotsResult.Builder builder, final Instant before )
+    private void deleteByBefore( final DeleteSnapshotsResult.Builder builder, final Instant before )
     {
         final SnapshotResults snapshotResults = doListSnapshots();
 
@@ -348,7 +337,7 @@ private void deleteByBefore( final RemoveSnapshotsResult.Builder builder, final
                 try
                 {
                     doDeleteSnapshot( snapshotResult.getName() );
-                    builder.addProcessed( snapshotResult.getName() );
+                    builder.add( snapshotResult.getName() );
                 }
                 catch ( Exception e )
                 {
@@ -359,14 +348,14 @@ private void deleteByBefore( final RemoveSnapshotsResult.Builder builder, final
         }
     }
 
-    private void deleteByName( final RemoveSnapshotsResult.Builder builder, final Set<String> snapshotNames )
+    private void deleteByName( final DeleteSnapshotsResult.Builder builder, final Set<String> snapshotNames )
     {
         for ( final String name : snapshotNames )
         {
             try
             {
                 doDeleteSnapshot( name );
-                builder.addProcessed( name );
+                builder.add( name );
             }
             catch ( Exception e )
             {

modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTask.java

@@ -7,7 +7,7 @@
 import org.osgi.service.component.annotations.Reference;
 
 import com.enonic.xp.node.DeleteSnapshotParams;
-import com.enonic.xp.node.RemoveSnapshotsResult;
+import com.enonic.xp.node.DeleteSnapshotsResult;
 import com.enonic.xp.repo.impl.vacuum.VacuumTask;
 import com.enonic.xp.repo.impl.vacuum.VacuumTaskParams;
 import com.enonic.xp.snapshot.SnapshotService;
@@ -39,11 +39,11 @@ public VacuumTaskResult execute( final VacuumTaskParams params )
 
         final VacuumTaskResult.Builder builder = VacuumTaskResult.create().taskName( NAME );
 
-        final RemoveSnapshotsResult deleteSnapshotsResult =
-            snapshotService.remove( DeleteSnapshotParams.create().before( Instant.now().minusMillis( params.getAgeThreshold() ) ).build() );
+        final DeleteSnapshotsResult deleteSnapshotsResult =
+            snapshotService.delete( DeleteSnapshotParams.create().before( Instant.now().minusMillis( params.getAgeThreshold() ) ).build() );
 
-        deleteSnapshotsResult.getSnapshotNames().forEach( snapshot -> builder.processed() );
-        deleteSnapshotsResult.getFailedSnapshotNames().forEach( snapshot -> builder.failed() );
+        deleteSnapshotsResult.getDeletedSnapshots().forEach( snapshot -> builder.processed() );
+        deleteSnapshotsResult.getFailedSnapshots().forEach( snapshot -> builder.failed() );
 
         return builder.build();
     }

modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/elasticsearch/snapshot/SnapshotServiceImplTest.java

@@ -24,7 +24,7 @@
 
 import com.enonic.xp.event.EventPublisher;
 import com.enonic.xp.node.DeleteSnapshotParams;
-import com.enonic.xp.node.RemoveSnapshotsResult;
+import com.enonic.xp.node.DeleteSnapshotsResult;
 import com.enonic.xp.repo.impl.config.RepoConfiguration;
 import com.enonic.xp.repo.impl.index.IndexServiceInternal;
 import com.enonic.xp.repo.impl.repository.RepositoryEntryService;
@@ -62,7 +62,7 @@ void setUp()
     }
 
     @Test
-    void testRemove()
+    void testDelete()
     {
         final RepositoryMetaData repositoryMetaData = new RepositoryMetaData( "enonic-xp-snapshot-repo", "fs", Settings.settingsBuilder()
             .put( "compress", true )
@@ -96,15 +96,15 @@ void testRemove()
             argThat( request -> "snapshot2".equals( request.snapshot() ) || "snapshot4".equals( request.snapshot() ) ) ) ).thenThrow(
             new ElasticsearchException( "Failed to delete snapshot" ) );
 
-        final RemoveSnapshotsResult result = instance.remove(
+        final DeleteSnapshotsResult result = instance.delete(
             DeleteSnapshotParams.create().add( "snapshot1" ).add( "snapshot2" ).before( now.minus( 2, ChronoUnit.HOURS ) ).build() );
 
-        assertEquals( 2, result.getSnapshotNames().size() );
-        assertTrue( result.getSnapshotNames().contains( "snapshot1" ) );
-        assertTrue( result.getSnapshotNames().contains( "snapshot3" ) );
-        assertEquals( 2, result.getFailedSnapshotNames().size() );
-        assertTrue( result.getFailedSnapshotNames().contains( "snapshot2" ) );
-        assertTrue( result.getFailedSnapshotNames().contains( "snapshot4" ) );
+        assertEquals( 2, result.getDeletedSnapshots().size() );
+        assertTrue( result.getDeletedSnapshots().contains( "snapshot1" ) );
+        assertTrue( result.getDeletedSnapshots().contains( "snapshot3" ) );
+        assertEquals( 2, result.getFailedSnapshots().size() );
+        assertTrue( result.getFailedSnapshots().contains( "snapshot2" ) );
+        assertTrue( result.getFailedSnapshots().contains( "snapshot4" ) );
     }
 
     private static SnapshotInfo mockSnapshot( String name, SnapshotState state, Instant endTime )

modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTaskTest.java

@@ -3,7 +3,7 @@
 import org.junit.jupiter.api.Test;
 
 import com.enonic.xp.node.DeleteSnapshotParams;
-import com.enonic.xp.node.RemoveSnapshotsResult;
+import com.enonic.xp.node.DeleteSnapshotsResult;
 import com.enonic.xp.repo.impl.vacuum.VacuumTaskParams;
 import com.enonic.xp.snapshot.SnapshotService;
 import com.enonic.xp.vacuum.VacuumListener;
@@ -22,8 +22,8 @@ public class SnapshotsVacuumTaskTest
     void test()
     {
         SnapshotService snapshotService = mock( SnapshotService.class );
-        when( snapshotService.remove( any( DeleteSnapshotParams.class ) ) ).thenReturn(
-            RemoveSnapshotsResult.create().addProcessed( "snapshot" ).build() );
+        when( snapshotService.delete( any( DeleteSnapshotParams.class ) ) ).thenReturn(
+            DeleteSnapshotsResult.create().add( "snapshot" ).build() );
 
         SnapshotsVacuumTask instance = new SnapshotsVacuumTask( snapshotService );
 
@@ -62,6 +62,6 @@ public void processed( final long count )
         assertEquals( 0, result.getDeleted() );
         assertEquals( 0, result.getFailed() );
 
-        verify( snapshotService, times( 1 ) ).remove( any( DeleteSnapshotParams.class ) );
+        verify( snapshotService, times( 1 ) ).delete( any( DeleteSnapshotParams.class ) );
     }
 }

modules/itest/itest-core/src/test/java/com/enonic/xp/core/snapshot/SnapshotServiceImplTest.java

@@ -252,7 +252,7 @@ private void doRestoreInvalidSnapshot()
 
         this.snapshotService.snapshot( SnapshotParams.create().snapshotName( "my-snapshot" ).build() );
 
-        this.snapshotService.remove( DeleteSnapshotParams.create().add( "my-snapshot" ).build() );
+        this.snapshotService.delete( DeleteSnapshotParams.create().add( "my-snapshot" ).build() );
 
         this.repositoryService.deleteRepository( DeleteRepositoryParams.from( newRepoId ) );
 

modules/server/server-rest/src/main/java/com/enonic/xp/impl/server/rest/model/DeleteSnapshotsResultJson.java

@@ -8,18 +8,26 @@ public class DeleteSnapshotsResultJson
 {
     private final Set<String> deletedSnapshots;
 
-    private DeleteSnapshotsResultJson( final Set<String> deletedSnapshots )
+    private final Set<String> failedSnapshots;
+
+    private DeleteSnapshotsResultJson( final Set<String> deletedSnapshots, final Set<String> failedSnapshots )
     {
         this.deletedSnapshots = deletedSnapshots;
+        this.failedSnapshots = failedSnapshots;
     }
 
     public static DeleteSnapshotsResultJson from( final DeleteSnapshotsResult result )
     {
-        return new DeleteSnapshotsResultJson( result.getSet() );
+        return new DeleteSnapshotsResultJson( result.getDeletedSnapshots(), result.getFailedSnapshots() );
     }
 
     public Set<String> getDeletedSnapshots()
     {
         return deletedSnapshots;
     }
+
+    public Set<String> getFailedSnapshots()
+    {
+        return failedSnapshots;
+    }
 }

modules/server/server-rest/src/test/resources/com/enonic/xp/impl/server/rest/delete.json

@@ -1,5 +1,6 @@
 {
   "deletedSnapshots": [
     "snapshot1"
-  ]
-}
+  ],
+  "failedSnapshots" : [ ]
+}