From 5118434db252e36afe1fb1ea4e755ab747552cfd Mon Sep 17 00:00:00 2001 From: John Niang Date: Tue, 6 Sep 2022 10:50:11 +0800 Subject: [PATCH] Refine menu item reconciler to sync permalinks of other refs (#2380) #### What type of PR is this? /kind improvement /area core /milestone 2.0 #### What this PR does / why we need it: 1. Synchronize permalink and display name of category every 1min 2. Synchronize permalink and display name of tag every 1min 3. Synchronize permalink and display name of post every 1min Please note that we don't handle the synchronization of `Page` because we don't have the extension yet. #### Which issue(s) this PR fixes: See https://github.com/halo-dev/halo/pull/2303 for more. #### Special notes for your reviewer: **How to test?** 1. Create a Category/Tag/Post and check the permalink 2. Create a menu and a menu item 3. Set `spec.categoryRef.name` of menu item with the extension name we just created 5. Update the menu item and check the permalink 6. Update slug name of Category/Tag/Post and check the permalink 7. Wait for 1min and check the permalink of menu item #### Does this PR introduce a user-facing change? ```release-note None ``` --- .../run/halo/app/core/extension/MenuItem.java | 17 +-- .../reconciler/MenuItemReconciler.java | 79 +++++++++--- .../reconciler/MenuItemReconcilerTest.java | 112 +++++++++++++++--- 3 files changed, 165 insertions(+), 43 deletions(-) diff --git a/src/main/java/run/halo/app/core/extension/MenuItem.java b/src/main/java/run/halo/app/core/extension/MenuItem.java index 5d29870b1..862f6208a 100644 --- a/src/main/java/run/halo/app/core/extension/MenuItem.java +++ b/src/main/java/run/halo/app/core/extension/MenuItem.java @@ -8,6 +8,7 @@ import lombok.EqualsAndHashCode; import lombok.ToString; import run.halo.app.extension.AbstractExtension; import run.halo.app.extension.GVK; +import run.halo.app.extension.Ref; @Data @ToString(callSuper = true) @@ -40,24 +41,16 @@ public class MenuItem extends AbstractExtension { private LinkedHashSet children; @Schema(description = "Category reference.") - private MenuItemRef categoryRef; + private Ref categoryRef; @Schema(description = "Tag reference.") - private MenuItemRef tagRef; + private Ref tagRef; @Schema(description = "Post reference.") - private MenuItemRef postRef; + private Ref postRef; @Schema(description = "Page reference.") - private MenuItemRef pageRef; - - } - - @Data - public static class MenuItemRef { - - @Schema(description = "Reference name.", required = true) - private String name; + private Ref pageRef; } diff --git a/src/main/java/run/halo/app/core/extension/reconciler/MenuItemReconciler.java b/src/main/java/run/halo/app/core/extension/reconciler/MenuItemReconciler.java index c4d3823fe..6c9c3b010 100644 --- a/src/main/java/run/halo/app/core/extension/reconciler/MenuItemReconciler.java +++ b/src/main/java/run/halo/app/core/extension/reconciler/MenuItemReconciler.java @@ -1,9 +1,16 @@ package run.halo.app.core.extension.reconciler; +import java.time.Duration; +import java.util.Objects; import org.springframework.util.StringUtils; +import run.halo.app.core.extension.Category; import run.halo.app.core.extension.MenuItem; +import run.halo.app.core.extension.MenuItem.MenuItemSpec; import run.halo.app.core.extension.MenuItem.MenuItemStatus; +import run.halo.app.core.extension.Post; +import run.halo.app.core.extension.Tag; import run.halo.app.extension.ExtensionClient; +import run.halo.app.extension.Ref; import run.halo.app.extension.controller.Reconciler; import run.halo.app.extension.controller.Reconciler.Request; @@ -17,7 +24,7 @@ public class MenuItemReconciler implements Reconciler { @Override public Result reconcile(Request request) { - client.fetch(MenuItem.class, request.name()).ifPresent(menuItem -> { + return client.fetch(MenuItem.class, request.name()).map(menuItem -> { final var spec = menuItem.getSpec(); if (menuItem.getStatus() == null) { @@ -25,25 +32,69 @@ public class MenuItemReconciler implements Reconciler { } var status = menuItem.getStatus(); if (spec.getCategoryRef() != null) { - // TODO resolve permalink from category. + return handleCategoryRef(request.name(), status, spec.getCategoryRef()); } else if (spec.getTagRef() != null) { - // TODO resolve permalink from tag. + return handleTagRef(request.name(), status, spec.getTagRef()); } else if (spec.getPageRef() != null) { - // TODO resolve permalink from page. + // TODO resolve permalink from page. At present, we don't have Page extension. + return new Result(false, null); } else if (spec.getPostRef() != null) { - // TODO resolve permalink from post. + return handlePostRef(request.name(), status, spec.getPostRef()); } else { - // at last, we resolve href and display name from spec. - if (spec.getHref() == null || !StringUtils.hasText(spec.getDisplayName())) { - throw new IllegalArgumentException("Both href and displayName are required"); - } - status.setHref(spec.getHref()); - status.setDisplayName(spec.getDisplayName()); - // update status - client.update(menuItem); + return handleMenuSpec(request.name(), status, spec); } - }); + }).orElseGet(() -> new Result(false, null)); + } + + private Result handleCategoryRef(String menuItemName, MenuItemStatus status, Ref categoryRef) { + client.fetch(Category.class, categoryRef.getName()) + .filter(category -> category.getStatus() != null) + .filter(category -> StringUtils.hasText(category.getStatus().getPermalink())) + .ifPresent(category -> { + status.setHref(category.getStatus().getPermalink()); + status.setDisplayName(category.getSpec().getDisplayName()); + updateStatus(menuItemName, status); + }); + return new Result(true, Duration.ofMinutes(1)); + } + + private Result handleTagRef(String menuItemName, MenuItemStatus status, Ref tagRef) { + client.fetch(Tag.class, tagRef.getName()).filter(tag -> tag.getStatus() != null) + .filter(tag -> StringUtils.hasText(tag.getStatus().getPermalink())).ifPresent(tag -> { + status.setHref(tag.getStatus().getPermalink()); + status.setDisplayName(tag.getSpec().getDisplayName()); + updateStatus(menuItemName, status); + }); + return new Result(true, Duration.ofMinutes(1)); + } + + private Result handlePostRef(String menuItemName, MenuItemStatus status, Ref postRef) { + client.fetch(Post.class, postRef.getName()).filter(post -> post.getStatus() != null) + .filter(post -> StringUtils.hasText(post.getStatus().getPermalink())) + .ifPresent(post -> { + status.setHref(post.getStatus().getPermalink()); + status.setDisplayName(post.getSpec().getTitle()); + updateStatus(menuItemName, status); + }); + return new Result(true, Duration.ofMinutes(1)); + } + + private Result handleMenuSpec(String menuItemName, MenuItemStatus status, MenuItemSpec spec) { + if (spec.getHref() != null && StringUtils.hasText(spec.getDisplayName())) { + status.setHref(spec.getHref()); + status.setDisplayName(spec.getDisplayName()); + updateStatus(menuItemName, status); + } return new Result(false, null); } + private void updateStatus(String menuItemName, MenuItemStatus status) { + client.fetch(MenuItem.class, menuItemName) + .filter(menuItem -> !Objects.deepEquals(menuItem.getStatus(), status)) + .ifPresent(menuItem -> { + menuItem.setStatus(status); + client.update(menuItem); + }); + } + } diff --git a/src/test/java/run/halo/app/core/extension/reconciler/MenuItemReconcilerTest.java b/src/test/java/run/halo/app/core/extension/reconciler/MenuItemReconcilerTest.java index ee5ac3a9e..7d0a01550 100644 --- a/src/test/java/run/halo/app/core/extension/reconciler/MenuItemReconcilerTest.java +++ b/src/test/java/run/halo/app/core/extension/reconciler/MenuItemReconcilerTest.java @@ -1,23 +1,31 @@ package run.halo.app.core.extension.reconciler; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.time.Duration; import java.util.Optional; import java.util.function.Consumer; +import java.util.function.Supplier; import org.junit.jupiter.api.Nested; 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 run.halo.app.core.extension.Category; import run.halo.app.core.extension.MenuItem; import run.halo.app.core.extension.MenuItem.MenuItemSpec; import run.halo.app.extension.ExtensionClient; import run.halo.app.extension.Metadata; +import run.halo.app.extension.Ref; import run.halo.app.extension.controller.Reconciler.Request; @ExtendWith(MockitoExtension.class) @@ -30,33 +38,96 @@ class MenuItemReconcilerTest { MenuItemReconciler reconciler; @Nested - class WhenOtherRefNotSet { - + class WhenCategoryRefSet { @Test - void shouldReEnqueueIfHrefNotSet() { + void shouldNotUpdateMenuItemIfCategoryNotFound() { + Supplier menuItemSupplier = () -> createMenuItem("fake-name", spec -> { + spec.setCategoryRef(Ref.of("fake-category")); + }); + + when(client.fetch(MenuItem.class, "fake-name")) + .thenReturn(Optional.of(menuItemSupplier.get())); + when(client.fetch(Category.class, "fake-category")).thenReturn(Optional.empty()); + + var result = reconciler.reconcile(new Request("fake-name")); + + assertTrue(result.reEnqueue()); + assertEquals(Duration.ofMinutes(1), result.retryAfter()); + verify(client).fetch(MenuItem.class, "fake-name"); + verify(client).fetch(Category.class, "fake-category"); + verify(client, never()).update(isA(MenuItem.class)); + } + + @Test + void shouldUpdateMenuItemIfCategoryFound() { + Supplier menuItemSupplier = () -> createMenuItem("fake-name", spec -> { + spec.setCategoryRef(Ref.of("fake-category")); + }); + + when(client.fetch(MenuItem.class, "fake-name")) + .thenReturn(Optional.of(menuItemSupplier.get())) + .thenReturn(Optional.of(menuItemSupplier.get())); + when(client.fetch(Category.class, "fake-category")) + .thenReturn(Optional.of(createCategory())); + + var result = reconciler.reconcile(new Request("fake-name")); + + assertTrue(result.reEnqueue()); + assertEquals(Duration.ofMinutes(1), result.retryAfter()); + verify(client, times(2)).fetch(MenuItem.class, "fake-name"); + verify(client).fetch(Category.class, "fake-category"); + verify(client).update(argThat(menuItem -> { + var status = menuItem.getStatus(); + return status.getHref().equals("fake://permalink") + && status.getDisplayName().equals("Fake Category"); + })); + } + + Category createCategory() { + var metadata = new Metadata(); + metadata.setName("fake-category"); + + var spec = new Category.CategorySpec(); + spec.setDisplayName("Fake Category"); + var status = new Category.CategoryStatus(); + status.setPermalink("fake://permalink"); + + var category = new Category(); + category.setMetadata(metadata); + category.setSpec(spec); + category.setStatus(status); + return category; + } + } + + @Nested + class WhenOtherRefsNotSet { + + @Test + void shouldNotRequeueIfHrefNotSet() { var menuItem = createMenuItem("fake-name", spec -> { spec.setHref(null); spec.setDisplayName("Fake display name"); }); when(client.fetch(MenuItem.class, "fake-name")).thenReturn(Optional.of(menuItem)); - assertThrows(IllegalArgumentException.class, - () -> reconciler.reconcile(new Request("fake-name"))); + + var result = reconciler.reconcile(new Request("fake-name")); + assertFalse(result.reEnqueue()); + verify(client).fetch(MenuItem.class, "fake-name"); verify(client, never()).update(menuItem); } @Test - void shouldReEnqueueIfDisplayNameNotSet() { + void shouldNotRequeueIfDisplayNameNotSet() { var menuItem = createMenuItem("fake-name", spec -> { spec.setHref("/fake"); spec.setDisplayName(null); }); - when(client.fetch(MenuItem.class, "fake-name")).thenReturn( - Optional.of(menuItem)); - - assertThrows(IllegalArgumentException.class, - () -> reconciler.reconcile(new Request("fake-name"))); + when(client.fetch(MenuItem.class, "fake-name")).thenReturn(Optional.of(menuItem)); + var result = reconciler.reconcile(new Request("fake-name")); + assertFalse(result.reEnqueue()); verify(client).fetch(MenuItem.class, "fake-name"); verify(client, never()).update(menuItem); @@ -64,19 +135,26 @@ class MenuItemReconcilerTest { @Test void shouldReconcileIfHrefAndDisplayNameSet() { - var menuItem = createMenuItem("fake-name", spec -> { + Supplier menuItemSupplier = () -> createMenuItem("fake-name", spec -> { spec.setHref("/fake"); spec.setDisplayName("Fake display name"); }); - when(client.fetch(MenuItem.class, "fake-name")).thenReturn( - Optional.of(menuItem)); + when(client.fetch(MenuItem.class, "fake-name")) + .thenReturn(Optional.of(menuItemSupplier.get())) + .thenReturn(Optional.of(menuItemSupplier.get())); var result = reconciler.reconcile(new Request("fake-name")); assertFalse(result.reEnqueue()); - verify(client).fetch(MenuItem.class, "fake-name"); - verify(client).update(menuItem); + verify(client, times(2)).fetch(MenuItem.class, "fake-name"); + verify(client).update(argThat(ext -> { + if (!(ext instanceof MenuItem menuItem)) { + return false; + } + return menuItem.getStatus().getHref().equals("/fake") + && menuItem.getStatus().getDisplayName().equals("Fake display name"); + })); } }