diff --git a/modules/core/core-api/src/main/java/com/enonic/xp/node/DeleteSnapshotsResult.java b/modules/core/core-api/src/main/java/com/enonic/xp/node/DeleteSnapshotsResult.java index 5741f1e491e..2ddcaa5e88a 100644 --- a/modules/core/core-api/src/main/java/com/enonic/xp/node/DeleteSnapshotsResult.java +++ b/modules/core/core-api/src/main/java/com/enonic/xp/node/DeleteSnapshotsResult.java @@ -9,6 +9,7 @@ import com.enonic.xp.annotation.PublicApi; import com.enonic.xp.support.AbstractImmutableEntitySet; +@Deprecated @PublicApi public class DeleteSnapshotsResult extends AbstractImmutableEntitySet diff --git a/modules/core/core-api/src/main/java/com/enonic/xp/node/RemoveSnapshotsResult.java b/modules/core/core-api/src/main/java/com/enonic/xp/node/RemoveSnapshotsResult.java new file mode 100644 index 00000000000..e8e7432a842 --- /dev/null +++ b/modules/core/core-api/src/main/java/com/enonic/xp/node/RemoveSnapshotsResult.java @@ -0,0 +1,60 @@ +package com.enonic.xp.node; + +import java.util.Set; + +import com.google.common.collect.ImmutableSet; + +import com.enonic.xp.annotation.PublicApi; + +@PublicApi +public final class RemoveSnapshotsResult +{ + private final Set snapshotNames; + + private final Set failedSnapshotNames; + + private RemoveSnapshotsResult( final Builder builder ) + { + this.snapshotNames = builder.snapshotNames.build(); + this.failedSnapshotNames = builder.failedSnapshotNames.build(); + } + + public Set getSnapshotNames() + { + return snapshotNames; + } + + public Set getFailedSnapshotNames() + { + return failedSnapshotNames; + } + + public static Builder create() + { + return new Builder(); + } + + public static final class Builder + { + private final ImmutableSet.Builder snapshotNames = ImmutableSet.builder(); + + private final ImmutableSet.Builder failedSnapshotNames = ImmutableSet.builder(); + + public Builder addProcessed( final String snapshotName ) + { + this.snapshotNames.add( snapshotName ); + return this; + } + + public Builder addFailed( final String snapshotName ) + { + this.failedSnapshotNames.add( snapshotName ); + return this; + } + + public RemoveSnapshotsResult build() + { + return new RemoveSnapshotsResult( this ); + } + } +} diff --git a/modules/core/core-api/src/main/java/com/enonic/xp/snapshot/SnapshotService.java b/modules/core/core-api/src/main/java/com/enonic/xp/snapshot/SnapshotService.java index 2e1cd661333..92c4652fc43 100644 --- a/modules/core/core-api/src/main/java/com/enonic/xp/snapshot/SnapshotService.java +++ b/modules/core/core-api/src/main/java/com/enonic/xp/snapshot/SnapshotService.java @@ -2,6 +2,7 @@ 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; @@ -14,7 +15,10 @@ public interface SnapshotService RestoreResult restore( RestoreParams restoreParams ); + @Deprecated DeleteSnapshotsResult delete( DeleteSnapshotParams params ); + RemoveSnapshotsResult remove( DeleteSnapshotParams params ); + SnapshotResults list(); } diff --git a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/elasticsearch/snapshot/SnapshotServiceImpl.java b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/elasticsearch/snapshot/SnapshotServiceImpl.java index 3d20215a347..7dbe879c61d 100644 --- a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/elasticsearch/snapshot/SnapshotServiceImpl.java +++ b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/elasticsearch/snapshot/SnapshotServiceImpl.java @@ -2,7 +2,6 @@ import java.nio.file.Path; import java.time.Instant; -import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Set; @@ -33,6 +32,7 @@ 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,8 +197,18 @@ 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 ) ); } @@ -310,52 +320,60 @@ private SnapshotInfo getSnapshot( final String snapshotName ) } } - private DeleteSnapshotsResult doDelete( final DeleteSnapshotParams params ) + private RemoveSnapshotsResult doDelete( final DeleteSnapshotParams params ) { - final DeleteSnapshotsResult.Builder builder = DeleteSnapshotsResult.create(); + final RemoveSnapshotsResult.Builder builder = RemoveSnapshotsResult.create(); if ( !params.getSnapshotNames().isEmpty() ) { - builder.addAll( deleteByName( params.getSnapshotNames() ) ); + deleteByName( builder, params.getSnapshotNames() ); } if ( params.getBefore() != null ) { - builder.addAll( deleteByBefore( params.getBefore() ) ); + deleteByBefore( builder, params.getBefore() ); } return builder.build(); } - private Set deleteByBefore( final Instant before ) + private void deleteByBefore( final RemoveSnapshotsResult.Builder builder, final Instant before ) { - final Set deleted = new HashSet<>(); - final SnapshotResults snapshotResults = doListSnapshots(); for ( final SnapshotResult snapshotResult : snapshotResults ) { if ( snapshotResult.getTimestamp().isBefore( before ) ) { - doDeleteSnapshot( snapshotResult.getName() ); - deleted.add( snapshotResult.getName() ); + try + { + doDeleteSnapshot( snapshotResult.getName() ); + builder.addProcessed( snapshotResult.getName() ); + } + catch ( Exception e ) + { + LOG.error( "Snapshot delete failed: {}", snapshotResult.getName(), e ); + builder.addFailed( snapshotResult.getName() ); + } } } - - return deleted; } - private Set deleteByName( final Set snapshotNames ) + private void deleteByName( final RemoveSnapshotsResult.Builder builder, final Set snapshotNames ) { - final Set deletedNames = new HashSet<>(); - for ( final String name : snapshotNames ) { - doDeleteSnapshot( name ); - deletedNames.add( name ); + try + { + doDeleteSnapshot( name ); + builder.addProcessed( name ); + } + catch ( Exception e ) + { + LOG.error( "Snapshot delete failed: {}", name, e ); + builder.addFailed( name ); + } } - - return deletedNames; } private void doDeleteSnapshot( final String snapshotName ) diff --git a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTask.java b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTask.java index 80bf076a0ef..1e77d98a2ba 100644 --- a/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTask.java +++ b/modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTask.java @@ -5,11 +5,9 @@ import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.enonic.xp.node.DeleteSnapshotParams; -import com.enonic.xp.node.DeleteSnapshotsResult; +import com.enonic.xp.node.RemoveSnapshotsResult; import com.enonic.xp.repo.impl.vacuum.VacuumTask; import com.enonic.xp.repo.impl.vacuum.VacuumTaskParams; import com.enonic.xp.snapshot.SnapshotService; @@ -19,8 +17,6 @@ public class SnapshotsVacuumTask implements VacuumTask { - private static final Logger LOG = LoggerFactory.getLogger( SnapshotsVacuumTask.class ); - private static final int ORDER = 400; private static final String NAME = "SnapshotsVacuumTask"; @@ -42,20 +38,14 @@ public VacuumTaskResult execute( final VacuumTaskParams params ) } final VacuumTaskResult.Builder builder = VacuumTaskResult.create().taskName( NAME ); - try - { - final DeleteSnapshotsResult deleteSnapshotsResult = snapshotService.delete( - DeleteSnapshotParams.create().before( Instant.now().minusMillis( params.getAgeThreshold() ) ).build() ); - deleteSnapshotsResult.stream().forEach( snapshot -> builder.processed() ); + final RemoveSnapshotsResult deleteSnapshotsResult = + snapshotService.remove( DeleteSnapshotParams.create().before( Instant.now().minusMillis( params.getAgeThreshold() ) ).build() ); - return builder.build(); - } - catch ( Exception e ) - { - LOG.error( "Failed to vacuum snapshots", e ); - return builder.failed().build(); - } + deleteSnapshotsResult.getSnapshotNames().forEach( snapshot -> builder.processed() ); + deleteSnapshotsResult.getFailedSnapshotNames().forEach( snapshot -> builder.failed() ); + + return builder.build(); } @Override diff --git a/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTaskTest.java b/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTaskTest.java index 3d7f7bb3243..d461bb2327b 100644 --- a/modules/core/core-repo/src/test/java/com/enonic/xp/repo/impl/vacuum/snapshots/SnapshotsVacuumTaskTest.java +++ b/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.DeleteSnapshotsResult; +import com.enonic.xp.node.RemoveSnapshotsResult; 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.delete( any( DeleteSnapshotParams.class ) ) ).thenReturn( - DeleteSnapshotsResult.create().add( "snapshot" ).build() ); + when( snapshotService.remove( any( DeleteSnapshotParams.class ) ) ).thenReturn( + RemoveSnapshotsResult.create().addProcessed( "snapshot" ).build() ); SnapshotsVacuumTask instance = new SnapshotsVacuumTask( snapshotService ); @@ -62,25 +62,6 @@ public void processed( final long count ) assertEquals( 0, result.getDeleted() ); assertEquals( 0, result.getFailed() ); - verify( snapshotService, times( 1 ) ).delete( any( DeleteSnapshotParams.class ) ); - } - - @Test - void testFailed() - { - SnapshotService snapshotService = mock( SnapshotService.class ); - when( snapshotService.delete( any( DeleteSnapshotParams.class ) ) ).thenThrow( new RuntimeException() ); - - SnapshotsVacuumTask instance = new SnapshotsVacuumTask( snapshotService ); - - VacuumTaskResult result = instance.execute( VacuumTaskParams.create().ageThreshold( 60 * 1000 ).build() ); - - assertEquals( "SnapshotsVacuumTask", result.getTaskName() ); - assertEquals( 0, result.getProcessed() ); - assertEquals( 0, result.getInUse() ); - assertEquals( 0, result.getDeleted() ); - assertEquals( 1, result.getFailed() ); - - verify( snapshotService, times( 1 ) ).delete( any( DeleteSnapshotParams.class ) ); + verify( snapshotService, times( 1 ) ).remove( any( DeleteSnapshotParams.class ) ); } } diff --git a/modules/itest/itest-core/src/test/java/com/enonic/xp/core/snapshot/SnapshotServiceImplTest.java b/modules/itest/itest-core/src/test/java/com/enonic/xp/core/snapshot/SnapshotServiceImplTest.java index 783ecc1e3fc..abbef06ec66 100644 --- a/modules/itest/itest-core/src/test/java/com/enonic/xp/core/snapshot/SnapshotServiceImplTest.java +++ b/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.delete( DeleteSnapshotParams.create().add( "my-snapshot" ).build() ); + this.snapshotService.remove( DeleteSnapshotParams.create().add( "my-snapshot" ).build() ); this.repositoryService.deleteRepository( DeleteRepositoryParams.from( newRepoId ) );