From 7a0ba922af94aa995d39fdf87335a49c57165e11 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Fri, 24 Feb 2023 14:24:12 +0800 Subject: [PATCH] refactor: APIs returns ghost users when user is deleted (#3373) 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: 用户被删除时关联到的用户返回 ghost 用户信息 #### Which issue(s) this PR fixes: Fixes #3356 #### Special notes for your reviewer: /cc @halo-dev/sig-halo #### Does this PR introduce a user-facing change? ```release-note 用户被删除时关联到的用户返回 ghost 用户信息 ``` --- .../content/comment/CommentServiceImpl.java | 10 ++-- .../halo/app/content/comment/OwnerInfo.java | 17 +----- .../app/content/comment/ReplyServiceImpl.java | 15 ++--- .../app/content/impl/PostServiceImpl.java | 12 ++-- .../content/impl/SinglePageServiceImpl.java | 11 ++-- .../core/extension/service/UserService.java | 2 + .../extension/service/UserServiceImpl.java | 7 +++ .../theme/finders/impl/CommentFinderImpl.java | 14 ++--- .../finders/impl/ContributorFinderImpl.java | 13 ++--- .../comment/CommentServiceImplTest.java | 55 ++++++++++++++----- .../finders/impl/CommentFinderImplTest.java | 26 +++++++++ 11 files changed, 115 insertions(+), 67 deletions(-) diff --git a/src/main/java/run/halo/app/content/comment/CommentServiceImpl.java b/src/main/java/run/halo/app/content/comment/CommentServiceImpl.java index aa40beb66..464714dc1 100644 --- a/src/main/java/run/halo/app/content/comment/CommentServiceImpl.java +++ b/src/main/java/run/halo/app/content/comment/CommentServiceImpl.java @@ -15,6 +15,7 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Comment; +import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.Extension; import run.halo.app.extension.ListResult; import run.halo.app.extension.ReactiveExtensionClient; @@ -33,14 +34,16 @@ import run.halo.app.plugin.ExtensionComponentsFinder; public class CommentServiceImpl implements CommentService { private final ReactiveExtensionClient client; + private final UserService userService; private final ExtensionComponentsFinder extensionComponentsFinder; private final SystemConfigurableEnvironmentFetcher environmentFetcher; public CommentServiceImpl(ReactiveExtensionClient client, - ExtensionComponentsFinder extensionComponentsFinder, + UserService userService, ExtensionComponentsFinder extensionComponentsFinder, SystemConfigurableEnvironmentFetcher environmentFetcher) { this.client = client; + this.userService = userService; this.extensionComponentsFinder = extensionComponentsFinder; this.environmentFetcher = environmentFetcher; } @@ -153,9 +156,8 @@ public class CommentServiceImpl implements CommentService { private Mono getCommentOwnerInfo(Comment.CommentOwner owner) { if (User.KIND.equals(owner.getKind())) { - return client.fetch(User.class, owner.getName()) - .map(OwnerInfo::from) - .switchIfEmpty(Mono.just(OwnerInfo.ghostUser())); + return userService.getUserOrGhost(owner.getName()) + .map(OwnerInfo::from); } if (Comment.CommentOwner.KIND_EMAIL.equals(owner.getKind())) { return Mono.just(OwnerInfo.from(owner)); diff --git a/src/main/java/run/halo/app/content/comment/OwnerInfo.java b/src/main/java/run/halo/app/content/comment/OwnerInfo.java index e63f198e5..69f98e3ac 100644 --- a/src/main/java/run/halo/app/content/comment/OwnerInfo.java +++ b/src/main/java/run/halo/app/content/comment/OwnerInfo.java @@ -2,7 +2,6 @@ package run.halo.app.content.comment; import lombok.Builder; import lombok.Value; -import org.apache.commons.lang3.StringUtils; import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Comment; @@ -60,18 +59,4 @@ public class OwnerInfo { .displayName(user.getSpec().getDisplayName()) .build(); } - - /** - * Obtain a ghost owner info when user not found. - * - * @return a ghost user if user not found. - */ - public static OwnerInfo ghostUser() { - return OwnerInfo.builder() - .kind(User.KIND) - .name("ghost") - .email(StringUtils.EMPTY) - .displayName("Ghost") - .build(); - } -} \ No newline at end of file +} diff --git a/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java b/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java index 271d49b89..9282f12e9 100644 --- a/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java +++ b/src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java @@ -5,6 +5,7 @@ import static run.halo.app.extension.router.selector.SelectorUtil.labelAndFieldS import java.time.Instant; import java.util.function.Function; import java.util.function.Predicate; +import lombok.RequiredArgsConstructor; import org.apache.commons.lang3.BooleanUtils; import org.springframework.security.core.context.ReactiveSecurityContextHolder; import org.springframework.stereotype.Service; @@ -13,6 +14,7 @@ import reactor.core.publisher.Mono; import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Comment; import run.halo.app.core.extension.content.Reply; +import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.Extension; import run.halo.app.extension.ListResult; import run.halo.app.extension.ReactiveExtensionClient; @@ -26,16 +28,12 @@ import run.halo.app.infra.exception.AccessDeniedException; * @since 2.0.0 */ @Service +@RequiredArgsConstructor public class ReplyServiceImpl implements ReplyService { private final ReactiveExtensionClient client; private final SystemConfigurableEnvironmentFetcher environmentFetcher; - - public ReplyServiceImpl(ReactiveExtensionClient client, - SystemConfigurableEnvironmentFetcher environmentFetcher) { - this.client = client; - this.environmentFetcher = environmentFetcher; - } + private final UserService userService; @Override public Mono create(String commentName, Reply reply) { @@ -132,9 +130,8 @@ public class ReplyServiceImpl implements ReplyService { private Mono getOwnerInfo(Reply reply) { Comment.CommentOwner owner = reply.getSpec().getOwner(); if (User.KIND.equals(owner.getKind())) { - return client.fetch(User.class, owner.getName()) - .map(OwnerInfo::from) - .switchIfEmpty(Mono.just(OwnerInfo.ghostUser())); + return userService.getUserOrGhost(owner.getName()) + .map(OwnerInfo::from); } if (Comment.CommentOwner.KIND_EMAIL.equals(owner.getKind())) { return Mono.just(OwnerInfo.from(owner)); 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 b7ede0bb0..e60394015 100644 --- a/src/main/java/run/halo/app/content/impl/PostServiceImpl.java +++ b/src/main/java/run/halo/app/content/impl/PostServiceImpl.java @@ -28,10 +28,10 @@ import run.halo.app.content.PostRequest; import run.halo.app.content.PostService; import run.halo.app.content.PostSorter; import run.halo.app.content.Stats; -import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Category; import run.halo.app.core.extension.content.Post; import run.halo.app.core.extension.content.Tag; +import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.ListResult; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.extension.Ref; @@ -51,11 +51,14 @@ import run.halo.app.metrics.MeterUtils; public class PostServiceImpl extends AbstractContentService implements PostService { private final ReactiveExtensionClient client; private final CounterService counterService; + private final UserService userService; - public PostServiceImpl(ReactiveExtensionClient client, CounterService counterService) { + public PostServiceImpl(ReactiveExtensionClient client, CounterService counterService, + UserService userService) { super(client); this.client = client; this.counterService = counterService; + this.userService = userService; } @Override @@ -191,7 +194,7 @@ public class PostServiceImpl extends AbstractContentService implements PostServi } private Mono setOwner(String ownerName, ListedPost post) { - return client.fetch(User.class, ownerName) + return userService.getUserOrGhost(ownerName) .map(user -> { Contributor contributor = new Contributor(); contributor.setName(user.getMetadata().getName()); @@ -224,8 +227,7 @@ public class PostServiceImpl extends AbstractContentService implements PostServi return Flux.empty(); } return Flux.fromIterable(usernames) - .flatMap(username -> client.fetch(User.class, username) - .switchIfEmpty(Mono.defer(() -> client.fetch(User.class, "ghost")))) + .flatMap(userService::getUserOrGhost) .map(user -> { Contributor contributor = new Contributor(); contributor.setName(user.getMetadata().getName()); 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 7aa762555..5cbbf079c 100644 --- a/src/main/java/run/halo/app/content/impl/SinglePageServiceImpl.java +++ b/src/main/java/run/halo/app/content/impl/SinglePageServiceImpl.java @@ -28,9 +28,9 @@ import run.halo.app.content.SinglePageRequest; import run.halo.app.content.SinglePageService; import run.halo.app.content.SinglePageSorter; import run.halo.app.content.Stats; -import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Post; import run.halo.app.core.extension.content.SinglePage; +import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.ListResult; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.extension.Ref; @@ -51,11 +51,14 @@ public class SinglePageServiceImpl extends AbstractContentService implements Sin private final ReactiveExtensionClient client; private final CounterService counterService; + private final UserService userService; - public SinglePageServiceImpl(ReactiveExtensionClient client, CounterService counterService) { + public SinglePageServiceImpl(ReactiveExtensionClient client, CounterService counterService, + UserService userService) { super(client); this.client = client; this.counterService = counterService; + this.userService = userService; } @Override @@ -242,7 +245,7 @@ public class SinglePageServiceImpl extends AbstractContentService implements Sin } private Mono setOwner(String ownerName, ListedSinglePage page) { - return client.fetch(User.class, ownerName) + return userService.getUserOrGhost(ownerName) .map(user -> { Contributor contributor = new Contributor(); contributor.setName(user.getMetadata().getName()); @@ -273,7 +276,7 @@ public class SinglePageServiceImpl extends AbstractContentService implements Sin return Flux.empty(); } return Flux.fromIterable(usernames) - .flatMap(username -> client.fetch(User.class, username)) + .flatMap(userService::getUserOrGhost) .map(user -> { Contributor contributor = new Contributor(); contributor.setName(user.getMetadata().getName()); diff --git a/src/main/java/run/halo/app/core/extension/service/UserService.java b/src/main/java/run/halo/app/core/extension/service/UserService.java index 2b3ad8fab..e958c1734 100644 --- a/src/main/java/run/halo/app/core/extension/service/UserService.java +++ b/src/main/java/run/halo/app/core/extension/service/UserService.java @@ -10,6 +10,8 @@ public interface UserService { Mono getUser(String username); + Mono getUserOrGhost(String username); + Mono updatePassword(String username, String newPassword); Mono updateWithRawPassword(String username, String rawPassword); diff --git a/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java b/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java index f47b99dfa..6905ecc7c 100644 --- a/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java +++ b/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java @@ -19,6 +19,7 @@ import run.halo.app.extension.ReactiveExtensionClient; @Service public class UserServiceImpl implements UserService { + public static final String GHOST_USER_NAME = "ghost"; private final ReactiveExtensionClient client; @@ -34,6 +35,12 @@ public class UserServiceImpl implements UserService { return client.get(User.class, username); } + @Override + public Mono getUserOrGhost(String username) { + return client.fetch(User.class, username) + .switchIfEmpty(Mono.defer(() -> client.get(User.class, GHOST_USER_NAME))); + } + @Override public Mono updatePassword(String username, String newPassword) { return getUser(username) diff --git a/src/main/java/run/halo/app/theme/finders/impl/CommentFinderImpl.java b/src/main/java/run/halo/app/theme/finders/impl/CommentFinderImpl.java index fd389b98a..4afb78b2a 100644 --- a/src/main/java/run/halo/app/theme/finders/impl/CommentFinderImpl.java +++ b/src/main/java/run/halo/app/theme/finders/impl/CommentFinderImpl.java @@ -5,6 +5,7 @@ import java.util.Comparator; import java.util.List; import java.util.function.Function; import java.util.function.Predicate; +import lombok.RequiredArgsConstructor; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; @@ -16,9 +17,9 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import run.halo.app.content.comment.OwnerInfo; import run.halo.app.content.comment.ReplyService; -import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Comment; import run.halo.app.core.extension.content.Reply; +import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.ListResult; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.extension.Ref; @@ -35,13 +36,11 @@ import run.halo.app.theme.finders.vo.ReplyVo; * @since 2.0.0 */ @Finder("commentFinder") +@RequiredArgsConstructor public class CommentFinderImpl implements CommentFinder { private final ReactiveExtensionClient client; - - public CommentFinderImpl(ReactiveExtensionClient client) { - this.client = client; - } + private final UserService userService; @Override public Mono getByName(String name) { @@ -107,9 +106,8 @@ public class CommentFinderImpl implements CommentFinder { if (Comment.CommentOwner.KIND_EMAIL.equals(owner.getKind())) { return Mono.just(OwnerInfo.from(owner)); } - return client.fetch(User.class, owner.getName()) - .map(OwnerInfo::from) - .defaultIfEmpty(OwnerInfo.ghostUser()); + return userService.getUserOrGhost(owner.getName()) + .map(OwnerInfo::from); } private Mono> fixedCommentPredicate(Ref ref) { diff --git a/src/main/java/run/halo/app/theme/finders/impl/ContributorFinderImpl.java b/src/main/java/run/halo/app/theme/finders/impl/ContributorFinderImpl.java index 42aacf3e5..3d05eef7f 100644 --- a/src/main/java/run/halo/app/theme/finders/impl/ContributorFinderImpl.java +++ b/src/main/java/run/halo/app/theme/finders/impl/ContributorFinderImpl.java @@ -1,10 +1,10 @@ package run.halo.app.theme.finders.impl; import java.util.List; +import lombok.RequiredArgsConstructor; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -import run.halo.app.core.extension.User; -import run.halo.app.extension.ReactiveExtensionClient; +import run.halo.app.core.extension.service.UserService; import run.halo.app.theme.finders.ContributorFinder; import run.halo.app.theme.finders.Finder; import run.halo.app.theme.finders.vo.ContributorVo; @@ -16,17 +16,14 @@ import run.halo.app.theme.finders.vo.ContributorVo; * @since 2.0.0 */ @Finder("contributorFinder") +@RequiredArgsConstructor public class ContributorFinderImpl implements ContributorFinder { - private final ReactiveExtensionClient client; - - public ContributorFinderImpl(ReactiveExtensionClient client) { - this.client = client; - } + private final UserService userService; @Override public Mono getContributor(String name) { - return client.fetch(User.class, name) + return userService.getUserOrGhost(name) .map(ContributorVo::from); } diff --git a/src/test/java/run/halo/app/content/comment/CommentServiceImplTest.java b/src/test/java/run/halo/app/content/comment/CommentServiceImplTest.java index 7640eefab..b645cdb43 100644 --- a/src/test/java/run/halo/app/content/comment/CommentServiceImplTest.java +++ b/src/test/java/run/halo/app/content/comment/CommentServiceImplTest.java @@ -31,6 +31,7 @@ import run.halo.app.content.TestPost; import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Comment; import run.halo.app.core.extension.content.Post; +import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.ListResult; import run.halo.app.extension.Metadata; import run.halo.app.extension.ReactiveExtensionClient; @@ -55,6 +56,9 @@ class CommentServiceImplTest { @Mock private ReactiveExtensionClient client; + @Mock + private UserService userService; + @Mock private ExtensionComponentsFinder extensionComponentsFinder; @@ -70,15 +74,10 @@ class CommentServiceImplTest { when(client.list(eq(Comment.class), any(), any(), anyInt(), anyInt())) .thenReturn(Mono.just(comments)); - User user = new User(); - user.setMetadata(new Metadata()); - user.getMetadata().setName("B-owner"); - user.setSpec(new User.UserSpec()); - user.getSpec().setAvatar("B-avatar"); - user.getSpec().setDisplayName("B-displayName"); - user.getSpec().setEmail("B-email"); - when(client.fetch(eq(User.class), eq("B-owner"))) - .thenReturn(Mono.just(user)); + when(userService.getUserOrGhost(eq("A-owner"))) + .thenReturn(Mono.just(createUser("A-owner"))); + when(userService.getUserOrGhost(eq("B-owner"))) + .thenReturn(Mono.just(createUser("B-owner"))); when(client.fetch(eq(User.class), eq("C-owner"))) .thenReturn(Mono.empty()); @@ -90,8 +89,26 @@ class CommentServiceImplTest { when(postCommentSubject.get(eq("fake-post"))).thenReturn(Mono.just(post())); } + private static User createUser(String name) { + User user = new User(); + user.setMetadata(new Metadata()); + user.getMetadata().setName(name); + user.setSpec(new User.UserSpec()); + user.getSpec().setAvatar(name + "-avatar"); + user.getSpec().setDisplayName(name + "-displayName"); + user.getSpec().setEmail(name + "-email"); + return user; + } + @Test void listComment() { + when(userService.getUserOrGhost(any())) + .thenReturn(Mono.just(ghostUser())); + when(userService.getUserOrGhost("A-owner")) + .thenReturn(Mono.just(createUser("A-owner"))); + when(userService.getUserOrGhost("B-owner")) + .thenReturn(Mono.just(createUser("B-owner"))); + Mono> listResultMono = commentService.listComment(new CommentQuery(new LinkedMultiValueMap<>())); StepVerifier.create(listResultMono) @@ -118,6 +135,8 @@ class CommentServiceImplTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Comment.class); + when(client.fetch(eq(User.class), eq("B-owner"))) + .thenReturn(Mono.just(createUser("B-owner"))); Comment commentToCreate = commentRequest.toComment(); commentToCreate.getMetadata().setName("fake"); Mono commentMono = commentService.create(commentToCreate); @@ -136,7 +155,7 @@ class CommentServiceImplTest { "owner": { "kind": "User", "name": "B-owner", - "displayName": "B-displayName" + "displayName": "B-owner-displayName" }, "priority": 0, "top": false, @@ -239,6 +258,16 @@ class CommentServiceImplTest { return commentSetting; } + User ghostUser() { + User user = new User(); + user.setMetadata(new Metadata()); + user.getMetadata().setName("ghost"); + user.setSpec(new User.UserSpec()); + user.getSpec().setDisplayName("Ghost"); + user.getSpec().setEmail(""); + return user; + } + private String expectListResultJson() { return """ { @@ -318,9 +347,9 @@ class CommentServiceImplTest { "owner": { "kind": "User", "name": "B-owner", - "displayName": "B-displayName", - "avatar": "B-avatar", - "email": "B-email" + "displayName": "B-owner-displayName", + "avatar": "B-owner-avatar", + "email": "B-owner-email" }, "subject": { "spec": { diff --git a/src/test/java/run/halo/app/theme/finders/impl/CommentFinderImplTest.java b/src/test/java/run/halo/app/theme/finders/impl/CommentFinderImplTest.java index a7cf6dceb..e0ee7252c 100644 --- a/src/test/java/run/halo/app/theme/finders/impl/CommentFinderImplTest.java +++ b/src/test/java/run/halo/app/theme/finders/impl/CommentFinderImplTest.java @@ -10,6 +10,7 @@ import java.util.List; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -24,6 +25,7 @@ import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Comment; import run.halo.app.core.extension.content.Post; import run.halo.app.core.extension.content.Reply; +import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.GroupVersionKind; import run.halo.app.extension.ListResult; import run.halo.app.extension.Metadata; @@ -43,10 +45,20 @@ class CommentFinderImplTest { @Mock private ReactiveExtensionClient client; + @Mock + private UserService userService; @InjectMocks private CommentFinderImpl commentFinder; + @BeforeEach + void setUp() { + User ghost = createUser(); + ghost.getMetadata().setName("ghost"); + when(userService.getUserOrGhost(eq("ghost"))).thenReturn(Mono.just(ghost)); + when(userService.getUserOrGhost(eq("fake-user"))).thenReturn(Mono.just(createUser())); + } + @Nested class ListCommentTest { @Test @@ -211,6 +223,7 @@ class CommentFinderImplTest { return Mono.just(new ListResult<>(1, 10, comments.size(), comments)); }); + extractedUser(); when(client.fetch(eq(User.class), any())).thenReturn(Mono.just(createUser())); } @@ -340,6 +353,7 @@ class CommentFinderImplTest { return Mono.just(new ListResult<>(1, 10, replies.size(), replies)); }); + extractedUser(); when(client.fetch(eq(User.class), any())).thenReturn(Mono.just(createUser())); } @@ -362,6 +376,18 @@ class CommentFinderImplTest { } } + private void extractedUser() { + User another = createUser(); + another.getMetadata().setName("another"); + when(userService.getUserOrGhost(eq("another"))).thenReturn(Mono.just(another)); + + User ghost = createUser(); + ghost.getMetadata().setName("ghost"); + when(userService.getUserOrGhost(eq("ghost"))).thenReturn(Mono.just(ghost)); + when(userService.getUserOrGhost(eq("fake-user"))).thenReturn(Mono.just(createUser())); + when(userService.getUserOrGhost(any())).thenReturn(Mono.just(ghost)); + } + User createUser() { User user = new User(); user.setMetadata(new Metadata());