From ecf65d221463a9ccb3fd6061c1e6f3d95b86c625 Mon Sep 17 00:00:00 2001 From: Halo Dev Bot <87291978+halo-dev-bot@users.noreply.github.com> Date: Wed, 8 Mar 2023 17:50:12 +0800 Subject: [PATCH] [release-2.3] Fix the problem of failure to create and publish post (#3488) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an automated cherry-pick of #3441 /assign JohnNiang ```release-note 解决文章创建和发布经常失败的问题 ``` --- .../app/content/impl/PostServiceImpl.java | 48 ++++++------- .../content/impl/SinglePageServiceImpl.java | 48 ++++++------- .../core/extension/endpoint/PostEndpoint.java | 68 +++++++++---------- .../extension/endpoint/PostEndpointTest.java | 59 ++++++++++++++-- 4 files changed, 134 insertions(+), 89 deletions(-) diff --git a/src/main/java/run/halo/app/content/impl/PostServiceImpl.java b/src/main/java/run/halo/app/content/impl/PostServiceImpl.java index e60394015b..ea1cd77e26 100644 --- a/src/main/java/run/halo/app/content/impl/PostServiceImpl.java +++ b/src/main/java/run/halo/app/content/impl/PostServiceImpl.java @@ -268,25 +268,25 @@ public Mono draftPost(PostRequest postRequest) { private Mono waitForPostToDraftConcludingWork(String postName, ContentWrapper contentWrapper) { - return client.fetch(Post.class, postName) - .flatMap(post -> { - post.getSpec().setBaseSnapshot(contentWrapper.getSnapshotName()); - post.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); - if (Objects.equals(true, post.getSpec().getPublish())) { - post.getSpec().setReleaseSnapshot(post.getSpec().getHeadSnapshot()); - } - Condition condition = Condition.builder() - .type(Post.PostPhase.DRAFT.name()) - .reason("DraftedSuccessfully") - .message("Drafted post successfully.") - .status(ConditionStatus.TRUE) - .lastTransitionTime(Instant.now()) - .build(); - Post.PostStatus status = post.getStatusOrDefault(); - status.setPhase(Post.PostPhase.DRAFT.name()); - status.getConditionsOrDefault().addAndEvictFIFO(condition); - return client.update(post); - }) + return Mono.defer(() -> client.fetch(Post.class, postName) + .flatMap(post -> { + post.getSpec().setBaseSnapshot(contentWrapper.getSnapshotName()); + post.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); + if (Objects.equals(true, post.getSpec().getPublish())) { + post.getSpec().setReleaseSnapshot(post.getSpec().getHeadSnapshot()); + } + Condition condition = Condition.builder() + .type(Post.PostPhase.DRAFT.name()) + .reason("DraftedSuccessfully") + .message("Drafted post successfully.") + .status(ConditionStatus.TRUE) + .lastTransitionTime(Instant.now()) + .build(); + Post.PostStatus status = post.getStatusOrDefault(); + status.setPhase(Post.PostPhase.DRAFT.name()); + status.getConditionsOrDefault().addAndEvictFIFO(condition); + return client.update(post); + })) .retryWhen(Retry.backoff(5, Duration.ofMillis(100)) .filter(OptimisticLockingFailureException.class::isInstance)); } @@ -306,11 +306,11 @@ public Mono updatePost(PostRequest postRequest) { return client.update(post); }); } - return updateContent(baseSnapshot, postRequest.contentRequest()) - .flatMap(contentWrapper -> { - post.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); - return client.update(post); - }) + return Mono.defer(() -> updateContent(baseSnapshot, postRequest.contentRequest()) + .flatMap(contentWrapper -> { + post.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); + return client.update(post); + })) .retryWhen(Retry.backoff(5, Duration.ofMillis(100)) .filter(throwable -> throwable instanceof OptimisticLockingFailureException)); } diff --git a/src/main/java/run/halo/app/content/impl/SinglePageServiceImpl.java b/src/main/java/run/halo/app/content/impl/SinglePageServiceImpl.java index 5cbbf079cf..66128992ee 100644 --- a/src/main/java/run/halo/app/content/impl/SinglePageServiceImpl.java +++ b/src/main/java/run/halo/app/content/impl/SinglePageServiceImpl.java @@ -128,25 +128,25 @@ public Mono draft(SinglePageRequest pageRequest) { private Mono waitForPageToDraftConcludingWork(String pageName, ContentWrapper contentWrapper) { - return client.fetch(SinglePage.class, pageName) - .flatMap(page -> { - page.getSpec().setBaseSnapshot(contentWrapper.getSnapshotName()); - page.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); - if (Objects.equals(true, page.getSpec().getPublish())) { - page.getSpec().setReleaseSnapshot(page.getSpec().getHeadSnapshot()); - } - Condition condition = Condition.builder() - .type(Post.PostPhase.DRAFT.name()) - .reason("DraftedSuccessfully") - .message("Drafted page successfully") - .status(ConditionStatus.TRUE) - .lastTransitionTime(Instant.now()) - .build(); - SinglePage.SinglePageStatus status = page.getStatusOrDefault(); - status.getConditionsOrDefault().addAndEvictFIFO(condition); - status.setPhase(Post.PostPhase.DRAFT.name()); - return client.update(page); - }) + return Mono.defer(() -> client.fetch(SinglePage.class, pageName) + .flatMap(page -> { + page.getSpec().setBaseSnapshot(contentWrapper.getSnapshotName()); + page.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); + if (Objects.equals(true, page.getSpec().getPublish())) { + page.getSpec().setReleaseSnapshot(page.getSpec().getHeadSnapshot()); + } + Condition condition = Condition.builder() + .type(Post.PostPhase.DRAFT.name()) + .reason("DraftedSuccessfully") + .message("Drafted page successfully") + .status(ConditionStatus.TRUE) + .lastTransitionTime(Instant.now()) + .build(); + SinglePage.SinglePageStatus status = page.getStatusOrDefault(); + status.getConditionsOrDefault().addAndEvictFIFO(condition); + status.setPhase(Post.PostPhase.DRAFT.name()); + return client.update(page); + })) .retryWhen(Retry.backoff(5, Duration.ofMillis(100)) .filter(OptimisticLockingFailureException.class::isInstance) ); @@ -167,11 +167,11 @@ public Mono update(SinglePageRequest pageRequest) { return client.update(page); }); } - return updateContent(baseSnapshot, pageRequest.contentRequest()) - .flatMap(contentWrapper -> { - page.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); - return client.update(page); - }) + return Mono.defer(() -> updateContent(baseSnapshot, pageRequest.contentRequest()) + .flatMap(contentWrapper -> { + page.getSpec().setHeadSnapshot(contentWrapper.getSnapshotName()); + return client.update(page); + })) .retryWhen(Retry.backoff(5, Duration.ofMillis(100)) .filter(throwable -> throwable instanceof OptimisticLockingFailureException)); } 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 c6373588db..dea7f0637b 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 @@ -7,6 +7,7 @@ import io.swagger.v3.oas.annotations.enums.ParameterIn; import java.time.Duration; +import java.util.Objects; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springdoc.core.fn.builders.schema.Builder; @@ -19,7 +20,8 @@ import org.springframework.web.reactive.function.server.RouterFunction; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.function.server.ServerResponse; -import org.thymeleaf.util.StringUtils; +import org.springframework.web.server.ServerErrorException; +import reactor.core.Exceptions; import reactor.core.publisher.Mono; import reactor.util.retry.Retry; import run.halo.app.content.ContentWrapper; @@ -229,39 +231,35 @@ Mono publishPost(ServerRequest request) { ) .retryWhen(Retry.backoff(5, Duration.ofMillis(100)) .filter(t -> t instanceof OptimisticLockingFailureException)) - .flatMap(post -> { - if (asyncPublish) { - return Mono.just(post); - } - return client.fetch(Post.class, name) - .map(latest -> { - String latestReleasedSnapshotName = - ExtensionUtil.nullSafeAnnotations(latest) - .get(Post.LAST_RELEASED_SNAPSHOT_ANNO); - if (StringUtils.equals(latestReleasedSnapshotName, - latest.getSpec().getReleaseSnapshot())) { - return latest; - } - throw new RetryException("Post publishing status is not as expected"); - }) - .retryWhen(Retry.fixedDelay(10, Duration.ofMillis(200)) - .filter(t -> t instanceof RetryException)) - .doOnError(IllegalStateException.class, err -> { - log.error("Failed to publish post [{}]", name, err); - throw new IllegalStateException("Publishing wait timeout."); - }); - }) + .filter(post -> asyncPublish) + .switchIfEmpty(Mono.defer(() -> awaitPostPublished(name))) + .onErrorMap(Exceptions::isRetryExhausted, err -> new ServerErrorException( + "Post publishing failed, please try again later.", err)) .flatMap(publishResult -> ServerResponse.ok().bodyValue(publishResult)); } + private Mono awaitPostPublished(String postName) { + return Mono.defer(() -> client.get(Post.class, postName) + .filter(post -> { + var releasedSnapshot = ExtensionUtil.nullSafeAnnotations(post) + .get(Post.LAST_RELEASED_SNAPSHOT_ANNO); + var expectReleaseSnapshot = post.getSpec().getReleaseSnapshot(); + return Objects.equals(releasedSnapshot, expectReleaseSnapshot); + }) + .switchIfEmpty(Mono.error( + () -> new RetryException("Retry to check post publish status")))) + .retryWhen(Retry.fixedDelay(10, Duration.ofMillis(200)) + .filter(t -> t instanceof RetryException)); + } + private Mono unpublishPost(ServerRequest request) { var name = request.pathVariable("name"); - return client.get(Post.class, name) - .doOnNext(post -> { - var spec = post.getSpec(); - spec.setPublish(false); - }) - .flatMap(client::update) + return Mono.defer(() -> client.get(Post.class, name) + .doOnNext(post -> { + var spec = post.getSpec(); + spec.setPublish(false); + }) + .flatMap(client::update)) .retryWhen(Retry.backoff(3, Duration.ofMillis(100)) .filter(t -> t instanceof OptimisticLockingFailureException)) // TODO Fire unpublished event in reconciler in the future @@ -272,12 +270,12 @@ private Mono unpublishPost(ServerRequest request) { private Mono recyclePost(ServerRequest request) { var name = request.pathVariable("name"); - return client.get(Post.class, name) - .doOnNext(post -> { - var spec = post.getSpec(); - spec.setDeleted(true); - }) - .flatMap(client::update) + return Mono.defer(() -> client.get(Post.class, name) + .doOnNext(post -> { + var spec = post.getSpec(); + spec.setDeleted(true); + }) + .flatMap(client::update)) .retryWhen(Retry.backoff(3, Duration.ofMillis(100)) .filter(t -> t instanceof OptimisticLockingFailureException)) // TODO Fire recycled event in reconciler in the future 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 a6c7290b36..c39792a7c7 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 @@ -7,6 +7,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -21,6 +22,7 @@ import run.halo.app.content.PostService; import run.halo.app.content.TestPost; import run.halo.app.core.extension.content.Post; +import run.halo.app.core.extension.content.Post.PostSpec; import run.halo.app.extension.Metadata; import run.halo.app.extension.ReactiveExtensionClient; @@ -85,7 +87,7 @@ void publishRetryOnOptimisticLockingFailure() { var post = new Post(); post.setMetadata(new Metadata()); post.getMetadata().setName("post-1"); - post.setSpec(new Post.PostSpec()); + post.setSpec(new PostSpec()); when(client.get(eq(Post.class), eq("post-1"))).thenReturn(Mono.just(post)); when(client.update(any(Post.class))) @@ -108,9 +110,19 @@ 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()); + post.setSpec(new PostSpec()); + + var publishedPost = new Post(); + var publishedMetadata = new Metadata(); + publishedMetadata.setAnnotations(Map.of(Post.LAST_RELEASED_SNAPSHOT_ANNO, "my-release")); + publishedPost.setMetadata(publishedMetadata); + var publishedPostSpec = new PostSpec(); + publishedPostSpec.setReleaseSnapshot("my-release"); + publishedPost.setSpec(publishedPostSpec); + + when(client.get(eq(Post.class), eq("post-1"))) + .thenReturn(Mono.just(post)) + .thenReturn(Mono.just(publishedPost)); when(client.update(any(Post.class))) .thenReturn(Mono.just(post)); @@ -123,8 +135,43 @@ void publishSuccess() { .is2xxSuccessful(); // Verify WebClient retry behavior - verify(client, times(1)).get(eq(Post.class), eq("post-1")); - verify(client, times(1)).update(any(Post.class)); + verify(client, times(2)).get(eq(Post.class), eq("post-1")); + verify(client).update(any(Post.class)); + } + + @Test + void shouldFailIfWaitTimeoutForPublishedStatus() { + var post = new Post(); + post.setMetadata(new Metadata()); + post.getMetadata().setName("post-1"); + post.setSpec(new PostSpec()); + + var publishedPost = new Post(); + var publishedMetadata = new Metadata(); + publishedMetadata.setAnnotations( + Map.of(Post.LAST_RELEASED_SNAPSHOT_ANNO, "old-my-release")); + publishedPost.setMetadata(publishedMetadata); + var publishedPostSpec = new PostSpec(); + publishedPostSpec.setReleaseSnapshot("my-release"); + publishedPost.setSpec(publishedPostSpec); + + when(client.get(eq(Post.class), eq("post-1"))) + .thenReturn(Mono.just(post)) + .thenReturn(Mono.just(publishedPost)); + + when(client.update(any(Post.class))) + .thenReturn(Mono.just(post)); + + // Send request + webTestClient.put() + .uri("/posts/{name}/publish?async=false", "post-1") + .exchange() + .expectStatus() + .is5xxServerError(); + + // Verify WebClient retry behavior + verify(client, times(12)).get(eq(Post.class), eq("post-1")); + verify(client).update(any(Post.class)); } PostRequest postRequest(Post post) {