From aba151f54c645caf47fffc27d2018d72d6ac30e8 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Tue, 28 Feb 2023 23:30:18 +0800 Subject: [PATCH] refactor: add retry operation to single page publishing (#3422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core /milestone 2.3.x #### What this PR does / why we need it: 修复初始化时自定义页面会发布失败的问题 #### Which issue(s) this PR fixes: Fixes #3279 #### Does this PR introduce a user-facing change? ```release-note None ``` --- .../core/extension/endpoint/PostEndpoint.java | 29 +++--- .../endpoint/SinglePageEndpoint.java | 24 +++-- .../extension/endpoint/PostEndpointTest.java | 52 +++++++++++ .../endpoint/SinglePageEndpointTest.java | 92 +++++++++++++++++++ 4 files changed, 173 insertions(+), 24 deletions(-) create mode 100644 src/test/java/run/halo/app/core/extension/endpoint/SinglePageEndpointTest.java diff --git a/src/main/java/run/halo/app/core/extension/endpoint/PostEndpoint.java b/src/main/java/run/halo/app/core/extension/endpoint/PostEndpoint.java index 853be7c94..c6373588d 100644 --- a/src/main/java/run/halo/app/core/extension/endpoint/PostEndpoint.java +++ b/src/main/java/run/halo/app/core/extension/endpoint/PostEndpoint.java @@ -214,19 +214,20 @@ public class PostEndpoint implements CustomEndpoint { boolean asyncPublish = request.queryParam("async") .map(Boolean::parseBoolean) .orElse(false); - return client.get(Post.class, name) - .doOnNext(post -> { - var spec = post.getSpec(); - request.queryParam("headSnapshot").ifPresent(spec::setHeadSnapshot); - spec.setPublish(true); - if (spec.getHeadSnapshot() == null) { - spec.setHeadSnapshot(spec.getBaseSnapshot()); - } - // TODO Provide release snapshot query param to control - spec.setReleaseSnapshot(spec.getHeadSnapshot()); - }) - .flatMap(client::update) - .retryWhen(Retry.backoff(3, Duration.ofMillis(100)) + return Mono.defer(() -> client.get(Post.class, name) + .doOnNext(post -> { + var spec = post.getSpec(); + request.queryParam("headSnapshot").ifPresent(spec::setHeadSnapshot); + spec.setPublish(true); + if (spec.getHeadSnapshot() == null) { + spec.setHeadSnapshot(spec.getBaseSnapshot()); + } + // TODO Provide release snapshot query param to control + spec.setReleaseSnapshot(spec.getHeadSnapshot()); + }) + .flatMap(client::update) + ) + .retryWhen(Retry.backoff(5, Duration.ofMillis(100)) .filter(t -> t instanceof OptimisticLockingFailureException)) .flatMap(post -> { if (asyncPublish) { @@ -243,7 +244,7 @@ public class PostEndpoint implements CustomEndpoint { } throw new RetryException("Post publishing status is not as expected"); }) - .retryWhen(Retry.fixedDelay(10, Duration.ofMillis(100)) + .retryWhen(Retry.fixedDelay(10, Duration.ofMillis(200)) .filter(t -> t instanceof RetryException)) .doOnError(IllegalStateException.class, err -> { log.error("Failed to publish post [{}]", name, err); diff --git a/src/main/java/run/halo/app/core/extension/endpoint/SinglePageEndpoint.java b/src/main/java/run/halo/app/core/extension/endpoint/SinglePageEndpoint.java index aa4f61b6b..6cad62ecf 100644 --- a/src/main/java/run/halo/app/core/extension/endpoint/SinglePageEndpoint.java +++ b/src/main/java/run/halo/app/core/extension/endpoint/SinglePageEndpoint.java @@ -11,6 +11,7 @@ import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springdoc.core.fn.builders.schema.Builder; import org.springdoc.webflux.core.fn.SpringdocRouteBuilder; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.http.MediaType; import org.springframework.retry.RetryException; import org.springframework.stereotype.Component; @@ -189,16 +190,19 @@ public class SinglePageEndpoint implements CustomEndpoint { boolean asyncPublish = request.queryParam("async") .map(Boolean::parseBoolean) .orElse(false); - return client.fetch(SinglePage.class, name) - .flatMap(singlePage -> { - SinglePage.SinglePageSpec spec = singlePage.getSpec(); - spec.setPublish(true); - if (spec.getHeadSnapshot() == null) { - spec.setHeadSnapshot(spec.getBaseSnapshot()); - } - spec.setReleaseSnapshot(spec.getHeadSnapshot()); - return client.update(singlePage); - }) + return Mono.defer(() -> client.get(SinglePage.class, name) + .flatMap(singlePage -> { + SinglePage.SinglePageSpec spec = singlePage.getSpec(); + spec.setPublish(true); + if (spec.getHeadSnapshot() == null) { + spec.setHeadSnapshot(spec.getBaseSnapshot()); + } + spec.setReleaseSnapshot(spec.getHeadSnapshot()); + return client.update(singlePage); + }) + ) + .retryWhen(Retry.backoff(5, Duration.ofMillis(100)) + .filter(t -> t instanceof OptimisticLockingFailureException)) .flatMap(post -> { if (asyncPublish) { return Mono.just(post); diff --git a/src/test/java/run/halo/app/core/extension/endpoint/PostEndpointTest.java b/src/test/java/run/halo/app/core/extension/endpoint/PostEndpointTest.java index 22bff140e..a6c7290b3 100644 --- a/src/test/java/run/halo/app/core/extension/endpoint/PostEndpointTest.java +++ b/src/test/java/run/halo/app/core/extension/endpoint/PostEndpointTest.java @@ -2,6 +2,9 @@ package run.halo.app.core.extension.endpoint; 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.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.junit.jupiter.api.BeforeEach; @@ -11,12 +14,14 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.test.web.reactive.server.WebTestClient; import reactor.core.publisher.Mono; import run.halo.app.content.PostRequest; import run.halo.app.content.PostService; import run.halo.app.content.TestPost; import run.halo.app.core.extension.content.Post; +import run.halo.app.extension.Metadata; import run.halo.app.extension.ReactiveExtensionClient; /** @@ -75,6 +80,53 @@ class PostEndpointTest { .value(post -> assertThat(post).isEqualTo(TestPost.postV1())); } + @Test + void publishRetryOnOptimisticLockingFailure() { + var post = new Post(); + post.setMetadata(new Metadata()); + post.getMetadata().setName("post-1"); + post.setSpec(new Post.PostSpec()); + when(client.get(eq(Post.class), eq("post-1"))).thenReturn(Mono.just(post)); + + when(client.update(any(Post.class))) + .thenReturn(Mono.error(new OptimisticLockingFailureException("fake-error"))); + + // Send request + webTestClient.put() + .uri("/posts/{name}/publish?async=false", "post-1") + .exchange() + .expectStatus() + .is5xxServerError(); + + // Verify WebClient retry behavior + verify(client, times(6)).get(eq(Post.class), eq("post-1")); + verify(client, times(6)).update(any(Post.class)); + } + + @Test + void publishSuccess() { + var post = new Post(); + post.setMetadata(new Metadata()); + post.getMetadata().setName("post-1"); + post.setSpec(new Post.PostSpec()); + when(client.get(eq(Post.class), eq("post-1"))).thenReturn(Mono.just(post)); + when(client.fetch(eq(Post.class), eq("post-1"))).thenReturn(Mono.empty()); + + when(client.update(any(Post.class))) + .thenReturn(Mono.just(post)); + + // Send request + webTestClient.put() + .uri("/posts/{name}/publish?async=false", "post-1") + .exchange() + .expectStatus() + .is2xxSuccessful(); + + // Verify WebClient retry behavior + verify(client, times(1)).get(eq(Post.class), eq("post-1")); + verify(client, times(1)).update(any(Post.class)); + } + PostRequest postRequest(Post post) { return new PostRequest(post, new PostRequest.Content("B", "

B

", "MARKDOWN")); } diff --git a/src/test/java/run/halo/app/core/extension/endpoint/SinglePageEndpointTest.java b/src/test/java/run/halo/app/core/extension/endpoint/SinglePageEndpointTest.java new file mode 100644 index 000000000..9e3fcd873 --- /dev/null +++ b/src/test/java/run/halo/app/core/extension/endpoint/SinglePageEndpointTest.java @@ -0,0 +1,92 @@ +package run.halo.app.core.extension.endpoint; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.dao.OptimisticLockingFailureException; +import org.springframework.test.web.reactive.server.WebTestClient; +import reactor.core.publisher.Mono; +import run.halo.app.core.extension.content.SinglePage; +import run.halo.app.extension.Metadata; +import run.halo.app.extension.ReactiveExtensionClient; + +/** + * Tests for @{@link SinglePageEndpoint}. + * + * @author guqing + * @since 2.3.0 + */ +@ExtendWith(MockitoExtension.class) +class SinglePageEndpointTest { + + @Mock + private ReactiveExtensionClient client; + + @InjectMocks + SinglePageEndpoint singlePageEndpoint; + + WebTestClient webTestClient; + + @BeforeEach + void setUp() { + webTestClient = WebTestClient + .bindToRouterFunction(singlePageEndpoint.endpoint()) + .build(); + } + + @Test + void publishRetryOnOptimisticLockingFailure() { + var page = new SinglePage(); + page.setMetadata(new Metadata()); + page.getMetadata().setName("page-1"); + page.setSpec(new SinglePage.SinglePageSpec()); + when(client.get(eq(SinglePage.class), eq("page-1"))).thenReturn(Mono.just(page)); + + when(client.update(any(SinglePage.class))) + .thenReturn(Mono.error(new OptimisticLockingFailureException("fake-error"))); + + // Send request + webTestClient.put() + .uri("/singlepages/{name}/publish?async=false", "page-1") + .exchange() + .expectStatus() + .is5xxServerError(); + + // Verify WebClient retry behavior + verify(client, times(6)).get(eq(SinglePage.class), eq("page-1")); + verify(client, times(6)).update(any(SinglePage.class)); + } + + @Test + void publishSuccess() { + var page = new SinglePage(); + page.setMetadata(new Metadata()); + page.getMetadata().setName("page-1"); + page.setSpec(new SinglePage.SinglePageSpec()); + + when(client.get(eq(SinglePage.class), eq("page-1"))).thenReturn(Mono.just(page)); + when(client.fetch(eq(SinglePage.class), eq("page-1"))).thenReturn(Mono.empty()); + + when(client.update(any(SinglePage.class))).thenReturn(Mono.just(page)); + + // Send request + webTestClient.put() + .uri("/singlepages/{name}/publish?async=false", "page-1") + .exchange() + .expectStatus() + .is2xxSuccessful(); + + // Verify WebClient retry behavior + verify(client, times(1)).get(eq(SinglePage.class), eq("page-1")); + verify(client, times(1)).update(any(SinglePage.class)); + } +} \ No newline at end of file