refactor: cascade delete snapshots and comments when post or page deleted (#2601)

#### What type of PR is this?
/kind bug
/area core
/milestone 2.0

#### What this PR does / why we need it:
当删除文章或自定义页面时级联删除内容快照和评论及计数器

see also #2602
#### Which issue(s) this PR fixes:

Fixes #2599

#### Special notes for your reviewer:
how to test it?
1. 新建一篇文章并创建一些评论(需要安装评论插件 [plugin-comment-widget](https://github.com/halo-sigs/plugin-comment-widget))
2. 逻辑删除文章时评论不会被删除,真实删除则会
3. 新建一个文章,再删除它,然后查询 snaphost 模型看关联文章的数据有没有被删除
4. 自定义页面同上
5. 删除文章或自定义页面时会删除对应的 Counter 记录(`GET /apis/metrics.halo.run/v1alpha1/counters`)

/cc @halo-dev/sig-halo 
#### Does this PR introduce a user-facing change?

```release-note
修复删除文章或自定义页面时没有级联删除内容快照和评论的问题
```
pull/2623/head
guqing 2022-10-24 15:44:10 +08:00 committed by GitHub
parent d765c2c329
commit 7b4dd59c58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 161 additions and 21 deletions

View File

@ -55,6 +55,7 @@ import run.halo.app.extension.router.ExtensionCompositeRouterFunction;
import run.halo.app.infra.ExternalUrlSupplier; import run.halo.app.infra.ExternalUrlSupplier;
import run.halo.app.infra.SystemConfigurableEnvironmentFetcher; import run.halo.app.infra.SystemConfigurableEnvironmentFetcher;
import run.halo.app.infra.properties.HaloProperties; import run.halo.app.infra.properties.HaloProperties;
import run.halo.app.metrics.CounterService;
import run.halo.app.plugin.ExtensionComponentsFinder; import run.halo.app.plugin.ExtensionComponentsFinder;
import run.halo.app.plugin.HaloPluginManager; import run.halo.app.plugin.HaloPluginManager;
import run.halo.app.plugin.resources.ReverseProxyRouterFunctionRegistry; import run.halo.app.plugin.resources.ReverseProxyRouterFunctionRegistry;
@ -147,9 +148,10 @@ public class ExtensionConfiguration {
@Bean @Bean
Controller postController(ExtensionClient client, ContentService contentService, Controller postController(ExtensionClient client, ContentService contentService,
PostPermalinkPolicy postPermalinkPolicy) { PostPermalinkPolicy postPermalinkPolicy, CounterService counterService) {
return new ControllerBuilder("post-controller", client) return new ControllerBuilder("post-controller", client)
.reconciler(new PostReconciler(client, contentService, postPermalinkPolicy)) .reconciler(new PostReconciler(client, contentService, postPermalinkPolicy,
counterService))
.extension(new Post()) .extension(new Post())
.build(); .build();
} }
@ -195,10 +197,10 @@ public class ExtensionConfiguration {
@Bean @Bean
Controller singlePageController(ExtensionClient client, ContentService contentService, Controller singlePageController(ExtensionClient client, ContentService contentService,
ApplicationContext applicationContext) { ApplicationContext applicationContext, CounterService counterService) {
return new ControllerBuilder("single-page-controller", client) return new ControllerBuilder("single-page-controller", client)
.reconciler(new SinglePageReconciler(client, contentService, .reconciler(new SinglePageReconciler(client, contentService,
applicationContext) applicationContext, counterService)
) )
.extension(new SinglePage()) .extension(new SinglePage())
.build(); .build();

View File

@ -12,13 +12,18 @@ import org.jsoup.Jsoup;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import run.halo.app.content.ContentService; import run.halo.app.content.ContentService;
import run.halo.app.content.permalinks.PostPermalinkPolicy; import run.halo.app.content.permalinks.PostPermalinkPolicy;
import run.halo.app.core.extension.Comment;
import run.halo.app.core.extension.Post; import run.halo.app.core.extension.Post;
import run.halo.app.core.extension.Snapshot; import run.halo.app.core.extension.Snapshot;
import run.halo.app.extension.ExtensionClient; import run.halo.app.extension.ExtensionClient;
import run.halo.app.extension.ExtensionOperator;
import run.halo.app.extension.Ref;
import run.halo.app.extension.controller.Reconciler; import run.halo.app.extension.controller.Reconciler;
import run.halo.app.infra.Condition; import run.halo.app.infra.Condition;
import run.halo.app.infra.ConditionStatus; import run.halo.app.infra.ConditionStatus;
import run.halo.app.infra.utils.JsonUtils; import run.halo.app.infra.utils.JsonUtils;
import run.halo.app.metrics.CounterService;
import run.halo.app.metrics.MeterUtils;
/** /**
* <p>Reconciler for {@link Post}.</p> * <p>Reconciler for {@link Post}.</p>
@ -37,22 +42,29 @@ public class PostReconciler implements Reconciler<Reconciler.Request> {
private final ExtensionClient client; private final ExtensionClient client;
private final ContentService contentService; private final ContentService contentService;
private final PostPermalinkPolicy postPermalinkPolicy; private final PostPermalinkPolicy postPermalinkPolicy;
private final CounterService counterService;
public PostReconciler(ExtensionClient client, ContentService contentService, public PostReconciler(ExtensionClient client, ContentService contentService,
PostPermalinkPolicy postPermalinkPolicy) { PostPermalinkPolicy postPermalinkPolicy, CounterService counterService) {
this.client = client; this.client = client;
this.contentService = contentService; this.contentService = contentService;
this.postPermalinkPolicy = postPermalinkPolicy; this.postPermalinkPolicy = postPermalinkPolicy;
this.counterService = counterService;
} }
@Override @Override
public Result reconcile(Request request) { public Result reconcile(Request request) {
client.fetch(Post.class, request.name()) client.fetch(Post.class, request.name())
.ifPresent(post -> { .ifPresent(post -> {
if (isDeleted(post)) { if (ExtensionOperator.isDeleted(post)) {
cleanUpResourcesAndRemoveFinalizer(request.name()); cleanUpResourcesAndRemoveFinalizer(request.name());
return; return;
} }
if (Objects.equals(true, post.getSpec().getDeleted())) {
// remove permalink from permalink indexer
postPermalinkPolicy.onPermalinkDelete(post);
return;
}
addFinalizerIfNecessary(post); addFinalizerIfNecessary(post);
reconcileMetadata(request.name()); reconcileMetadata(request.name());
@ -198,6 +210,23 @@ public class PostReconciler implements Reconciler<Reconciler.Request> {
private void cleanUpResources(Post post) { private void cleanUpResources(Post post) {
// remove permalink from permalink indexer // remove permalink from permalink indexer
postPermalinkPolicy.onPermalinkDelete(post); postPermalinkPolicy.onPermalinkDelete(post);
// clean up snapshots
Snapshot.SubjectRef subjectRef =
Snapshot.SubjectRef.of(Post.KIND, post.getMetadata().getName());
client.list(Snapshot.class,
snapshot -> subjectRef.equals(snapshot.getSpec().getSubjectRef()), null)
.forEach(client::delete);
// clean up comments
Ref ref = Ref.of(post);
client.list(Comment.class, comment -> comment.getSpec().getSubjectRef().equals(ref),
null)
.forEach(client::delete);
// delete counter
counterService.deleteByName(MeterUtils.nameOf(Post.class, post.getMetadata().getName()))
.block();
} }
private Map<String, String> getLabelsOrDefault(Post post) { private Map<String, String> getLabelsOrDefault(Post post) {
@ -224,9 +253,4 @@ public class PostReconciler implements Reconciler<Reconciler.Request> {
private boolean isPublished(Post post) { private boolean isPublished(Post post) {
return Objects.equals(true, post.getSpec().getPublished()); return Objects.equals(true, post.getSpec().getPublished());
} }
private boolean isDeleted(Post post) {
return Objects.equals(true, post.getSpec().getDeleted())
|| post.getMetadata().getDeletionTimestamp() != null;
}
} }

View File

@ -16,16 +16,21 @@ import org.springframework.context.ApplicationContext;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import run.halo.app.content.ContentService; import run.halo.app.content.ContentService;
import run.halo.app.content.permalinks.ExtensionLocator; import run.halo.app.content.permalinks.ExtensionLocator;
import run.halo.app.core.extension.Comment;
import run.halo.app.core.extension.Post; import run.halo.app.core.extension.Post;
import run.halo.app.core.extension.SinglePage; import run.halo.app.core.extension.SinglePage;
import run.halo.app.core.extension.Snapshot; import run.halo.app.core.extension.Snapshot;
import run.halo.app.extension.ExtensionClient; import run.halo.app.extension.ExtensionClient;
import run.halo.app.extension.ExtensionOperator;
import run.halo.app.extension.GroupVersionKind; import run.halo.app.extension.GroupVersionKind;
import run.halo.app.extension.Ref;
import run.halo.app.extension.controller.Reconciler; import run.halo.app.extension.controller.Reconciler;
import run.halo.app.infra.Condition; import run.halo.app.infra.Condition;
import run.halo.app.infra.ConditionStatus; import run.halo.app.infra.ConditionStatus;
import run.halo.app.infra.utils.JsonUtils; import run.halo.app.infra.utils.JsonUtils;
import run.halo.app.infra.utils.PathUtils; import run.halo.app.infra.utils.PathUtils;
import run.halo.app.metrics.CounterService;
import run.halo.app.metrics.MeterUtils;
import run.halo.app.theme.router.PermalinkIndexAddCommand; import run.halo.app.theme.router.PermalinkIndexAddCommand;
import run.halo.app.theme.router.PermalinkIndexDeleteCommand; import run.halo.app.theme.router.PermalinkIndexDeleteCommand;
@ -47,12 +52,14 @@ public class SinglePageReconciler implements Reconciler<Reconciler.Request> {
private final ExtensionClient client; private final ExtensionClient client;
private final ContentService contentService; private final ContentService contentService;
private final ApplicationContext applicationContext; private final ApplicationContext applicationContext;
private final CounterService counterService;
public SinglePageReconciler(ExtensionClient client, ContentService contentService, public SinglePageReconciler(ExtensionClient client, ContentService contentService,
ApplicationContext applicationContext) { ApplicationContext applicationContext, CounterService counterService) {
this.client = client; this.client = client;
this.contentService = contentService; this.contentService = contentService;
this.applicationContext = applicationContext; this.applicationContext = applicationContext;
this.counterService = counterService;
} }
@Override @Override
@ -60,10 +67,16 @@ public class SinglePageReconciler implements Reconciler<Reconciler.Request> {
client.fetch(SinglePage.class, request.name()) client.fetch(SinglePage.class, request.name())
.ifPresent(singlePage -> { .ifPresent(singlePage -> {
SinglePage oldPage = JsonUtils.deepCopy(singlePage); SinglePage oldPage = JsonUtils.deepCopy(singlePage);
if (isDeleted(oldPage)) { if (ExtensionOperator.isDeleted(singlePage)) {
cleanUpResourcesAndRemoveFinalizer(request.name()); cleanUpResourcesAndRemoveFinalizer(request.name());
return; return;
} }
if (Objects.equals(true, singlePage.getSpec().getDeleted())) {
// remove permalink from permalink indexer
permalinkOnDelete(singlePage);
return;
}
addFinalizerIfNecessary(oldPage); addFinalizerIfNecessary(oldPage);
reconcileStatus(request.name()); reconcileStatus(request.name());
@ -92,6 +105,24 @@ public class SinglePageReconciler implements Reconciler<Reconciler.Request> {
private void cleanUpResources(SinglePage singlePage) { private void cleanUpResources(SinglePage singlePage) {
// remove permalink from permalink indexer // remove permalink from permalink indexer
permalinkOnDelete(singlePage); permalinkOnDelete(singlePage);
// clean up snapshot
Snapshot.SubjectRef subjectRef =
Snapshot.SubjectRef.of(SinglePage.KIND, singlePage.getMetadata().getName());
client.list(Snapshot.class,
snapshot -> subjectRef.equals(snapshot.getSpec().getSubjectRef()), null)
.forEach(client::delete);
// clean up comments
Ref ref = Ref.of(singlePage);
client.list(Comment.class, comment -> comment.getSpec().getSubjectRef().equals(ref),
null)
.forEach(client::delete);
// delete counter for single page
counterService.deleteByName(
MeterUtils.nameOf(SinglePage.class, singlePage.getMetadata().getName()))
.block();
} }
private void cleanUpResourcesAndRemoveFinalizer(String pageName) { private void cleanUpResourcesAndRemoveFinalizer(String pageName) {

View File

@ -95,4 +95,9 @@ public interface ExtensionOperator {
static <T extends ExtensionOperator> Predicate<T> isNotDeleted() { static <T extends ExtensionOperator> Predicate<T> isNotDeleted() {
return ext -> ext.getMetadata().getDeletionTimestamp() == null; return ext -> ext.getMetadata().getDeletionTimestamp() == null;
} }
static boolean isDeleted(ExtensionOperator extension) {
return extension.getMetadata() != null
&& extension.getMetadata().getDeletionTimestamp() != null;
}
} }

View File

@ -1,5 +1,6 @@
package run.halo.app.metrics; package run.halo.app.metrics;
import reactor.core.publisher.Mono;
import run.halo.app.core.extension.Counter; import run.halo.app.core.extension.Counter;
/** /**
@ -9,4 +10,6 @@ import run.halo.app.core.extension.Counter;
public interface CounterService { public interface CounterService {
Counter getByName(String counterName); Counter getByName(String counterName);
Mono<Counter> deleteByName(String counterName);
} }

View File

@ -4,8 +4,10 @@ import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tag;
import java.util.Collection; import java.util.Collection;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import reactor.core.publisher.Mono;
import run.halo.app.core.extension.Counter; import run.halo.app.core.extension.Counter;
import run.halo.app.extension.Metadata; import run.halo.app.extension.Metadata;
import run.halo.app.extension.ReactiveExtensionClient;
/** /**
* Counter service implementation. * Counter service implementation.
@ -17,24 +19,39 @@ import run.halo.app.extension.Metadata;
public class CounterServiceImpl implements CounterService { public class CounterServiceImpl implements CounterService {
private final MeterRegistry meterRegistry; private final MeterRegistry meterRegistry;
private final ReactiveExtensionClient client;
public CounterServiceImpl(MeterRegistry meterRegistry) { public CounterServiceImpl(MeterRegistry meterRegistry, ReactiveExtensionClient client) {
this.meterRegistry = meterRegistry; this.meterRegistry = meterRegistry;
this.client = client;
} }
@Override @Override
public Counter getByName(String counterName) { public Counter getByName(String counterName) {
Tag commonTag = MeterUtils.METRICS_COMMON_TAG; Collection<io.micrometer.core.instrument.Counter> counters =
Collection<io.micrometer.core.instrument.Counter> counters = meterRegistry.find(counterName) findCounters(counterName);
.tag(commonTag.getKey(),
valueMatch -> commonTag.getValue().equals(valueMatch))
.counters();
Counter counter = emptyCounter(counterName); Counter counter = emptyCounter(counterName);
counter.populateFrom(counters); counter.populateFrom(counters);
return counter; return counter;
} }
private Collection<io.micrometer.core.instrument.Counter> findCounters(String counterName) {
Tag commonTag = MeterUtils.METRICS_COMMON_TAG;
return meterRegistry.find(counterName)
.tag(commonTag.getKey(),
valueMatch -> commonTag.getValue().equals(valueMatch))
.counters();
}
@Override
public Mono<Counter> deleteByName(String counterName) {
return client.fetch(Counter.class, counterName)
.flatMap(counter -> client.delete(counter)
.doOnNext(deleted -> findCounters(counterName).forEach(meterRegistry::remove))
.thenReturn(counter));
}
private Counter emptyCounter(String name) { private Counter emptyCounter(String name) {
Counter counter = new Counter(); Counter counter = new Counter();
counter.setMetadata(new Metadata()); counter.setMetadata(new Metadata());

View File

@ -30,6 +30,7 @@ import run.halo.app.core.extension.Snapshot;
import run.halo.app.extension.ExtensionClient; import run.halo.app.extension.ExtensionClient;
import run.halo.app.extension.Metadata; import run.halo.app.extension.Metadata;
import run.halo.app.extension.controller.Reconciler; import run.halo.app.extension.controller.Reconciler;
import run.halo.app.metrics.CounterService;
import run.halo.app.theme.router.PermalinkIndexAddCommand; import run.halo.app.theme.router.PermalinkIndexAddCommand;
import run.halo.app.theme.router.PermalinkIndexDeleteCommand; import run.halo.app.theme.router.PermalinkIndexDeleteCommand;
import run.halo.app.theme.router.PermalinkIndexUpdateCommand; import run.halo.app.theme.router.PermalinkIndexUpdateCommand;
@ -50,11 +51,15 @@ class SinglePageReconcilerTest {
@Mock @Mock
private ApplicationContext applicationContext; private ApplicationContext applicationContext;
@Mock
private CounterService counterService;
private SinglePageReconciler singlePageReconciler; private SinglePageReconciler singlePageReconciler;
@BeforeEach @BeforeEach
void setUp() { void setUp() {
singlePageReconciler = new SinglePageReconciler(client, contentService, applicationContext); singlePageReconciler = new SinglePageReconciler(client, contentService, applicationContext,
counterService);
} }
@Test @Test

View File

@ -22,4 +22,20 @@ class ExtensionOperatorTest {
when(metadata.getDeletionTimestamp()).thenReturn(Instant.now()); when(metadata.getDeletionTimestamp()).thenReturn(Instant.now());
assertFalse(ExtensionOperator.isNotDeleted().test(ext)); assertFalse(ExtensionOperator.isNotDeleted().test(ext));
} }
@Test
void testIsDeleted() {
var ext = mock(ExtensionOperator.class);
when(ext.getMetadata()).thenReturn(null);
assertFalse(ExtensionOperator.isDeleted(ext));
var metadata = mock(Metadata.class);
when(ext.getMetadata()).thenReturn(metadata);
when(metadata.getDeletionTimestamp()).thenReturn(null);
assertFalse(ExtensionOperator.isDeleted(ext));
when(metadata.getDeletionTimestamp()).thenReturn(Instant.now());
assertTrue(ExtensionOperator.isDeleted(ext));
}
} }

View File

@ -1,11 +1,24 @@
package run.halo.app.metrics; package run.halo.app.metrics;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import run.halo.app.core.extension.Counter;
import run.halo.app.core.extension.Post; import run.halo.app.core.extension.Post;
import run.halo.app.extension.Metadata;
import run.halo.app.extension.ReactiveExtensionClient;
/** /**
* Tests for {@link CounterServiceImpl}. * Tests for {@link CounterServiceImpl}.
@ -13,20 +26,32 @@ import run.halo.app.core.extension.Post;
* @author guqing * @author guqing
* @since 2.0.0 * @since 2.0.0
*/ */
@ExtendWith(MockitoExtension.class)
class CounterServiceImplTest { class CounterServiceImplTest {
@Mock
private ReactiveExtensionClient client;
private CounterServiceImpl counterService; private CounterServiceImpl counterService;
@BeforeEach @BeforeEach
void setUp() { void setUp() {
SimpleMeterRegistry simpleMeterRegistry = new SimpleMeterRegistry(); SimpleMeterRegistry simpleMeterRegistry = new SimpleMeterRegistry();
counterService = new CounterServiceImpl(simpleMeterRegistry); counterService = new CounterServiceImpl(simpleMeterRegistry, client);
String counterName = MeterUtils.nameOf(Post.class, "fake-post"); String counterName = MeterUtils.nameOf(Post.class, "fake-post");
MeterUtils.visitCounter(simpleMeterRegistry, MeterUtils.visitCounter(simpleMeterRegistry,
counterName).increment(); counterName).increment();
MeterUtils.approvedCommentCounter(simpleMeterRegistry, counterName) MeterUtils.approvedCommentCounter(simpleMeterRegistry, counterName)
.increment(2); .increment(2);
Counter counter = new Counter();
counter.setMetadata(new Metadata());
counter.getMetadata().setName(counterName);
counter.setVisit(2);
lenient().when(client.delete(any())).thenReturn(Mono.just(counter));
lenient().when(client.fetch(eq(Counter.class), eq(counterName)))
.thenReturn(Mono.just(counter));
} }
@Test @Test
@ -38,4 +63,16 @@ class CounterServiceImplTest {
assertThat(counter.getTotalComment()).isEqualTo(0); assertThat(counter.getTotalComment()).isEqualTo(0);
assertThat(counter.getApprovedComment()).isEqualTo(2); assertThat(counter.getApprovedComment()).isEqualTo(2);
} }
@Test
void deleteByName() {
String counterName = MeterUtils.nameOf(Post.class, "fake-post");
counterService.deleteByName(counterName)
.as(StepVerifier::create)
.consumeNextWith(counter -> {
verify(client, times(1)).delete(any(Counter.class));
assertThat(counterService.getByName(counterName).getVisit()).isEqualTo(0);
})
.verifyComplete();
}
} }