github
/
halo
mirror of https://github.com/halo-dev/halo
1
0
Fork 0
Code Issues Releases Wiki Activity

refactor: add creationTime field for comment extension to resolve migration issue (#3341) 

#### What type of PR is this?
/kind improvement
/area core
/milestone 2.3.x
/kind api-change

#### What this PR does / why we need it:
1. spec 中新增 creationTime
2. 旧数据的 spec.creationTime 默认等于 approvedTime
3. 按照 metadata.creationTimestamp 排序的使用 spec.creationTime 代替

how to test it?
1. 使用迁移插件迁移看评论和回复的排序是否正确
2. 使用评论插件创建评论和回复看顺序是否正确
#### Which issue(s) this PR fixes:
Fixes #3330

#### Special notes for your reviewer:
/cc @halo-dev/sig-halo 
#### Does this PR introduce a user-facing change?

```release-note
评论和回复新增创建时间以兼容迁移数据的排序
```
pull/3379/head
guqing 2023-02-23 18:06:12 +08:00 committed by GitHub
parent 6c2064f1e0
commit 3a1587bab5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 218 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/main/java/run/halo/app/content/comment/CommentServiceImpl.java
View File

@ -93,6 +93,9 @@ public class CommentServiceImpl implements CommentService {
if (comment.getSpec().getOwner() != null) {
return Mono.just(comment);
}
if (comment.getSpec().getCreationTime() == null) {
comment.getSpec().setCreationTime(Instant.now());
}
// populate owner from current user
return fetchCurrentUser()
.map(this::toCommentOwner)

12
src/main/java/run/halo/app/content/comment/CommentSorter.java
View File

@ -30,12 +30,12 @@ public enum CommentSorter {
static Comparator<Comment> from(CommentSorter sorter) {
if (sorter == null) {
return defaultCommentComparator();
return lastReplyTimeComparator();
}
if (CREATE_TIME.equals(sorter)) {
Function<Comment, Instant> comparatorFunc =
comment -> comment.getMetadata().getCreationTimestamp();
return Comparator.comparing(comparatorFunc)
comment -> comment.getSpec().getCreationTime();
return Comparator.comparing(comparatorFunc, Comparators.nullsLow())
.thenComparing(name);
}
@ -65,12 +65,12 @@ public enum CommentSorter {
return null;
}
static Comparator<Comment> defaultCommentComparator() {
static Comparator<Comment> lastReplyTimeComparator() {
Function<Comment, Instant> comparatorFunc =
comment -> {
Instant lastReplyTime = comment.getStatusOrDefault().getLastReplyTime();
return Optional.ofNullable(
lastReplyTime).orElse(comment.getMetadata().getCreationTimestamp());
return Optional.ofNullable(lastReplyTime)
.orElse(comment.getSpec().getCreationTime());
};
return Comparator.comparing(comparatorFunc, Comparators.nullsLow())
.thenComparing(name);

20
src/main/java/run/halo/app/content/comment/ReplyService.java
View File

@ -1,5 +1,9 @@
package run.halo.app.content.comment;
import java.time.Instant;
import java.util.Comparator;
import java.util.function.Function;
import org.springframework.util.comparator.Comparators;
import reactor.core.publisher.Mono;
import run.halo.app.core.extension.content.Reply;
import run.halo.app.extension.ListResult;
@ -15,4 +19,20 @@ public interface ReplyService {
Mono<Reply> create(String commentName, Reply reply);
Mono<ListResult<ListedReply>> list(ReplyQuery query);
/**
* Ascending order by creation time.
*
* @return reply comparator
*/
static Comparator<Reply> creationTimeAscComparator() {
Function<Reply, Instant> creationTime = reply -> reply.getSpec().getCreationTime();
Function<Reply, Instant> metadataCreationTime =
reply -> reply.getMetadata().getCreationTimestamp();
// ascending order by creation time
// asc nulls high will be placed at the end
return Comparator.comparing(creationTime, Comparators.nullsHigh())
.thenComparing(metadataCreationTime)
.thenComparing(reply -> reply.getMetadata().getName());
}
}

13
src/main/java/run/halo/app/content/comment/ReplyServiceImpl.java
View File

@ -3,7 +3,6 @@ package run.halo.app.content.comment;
import static run.halo.app.extension.router.selector.SelectorUtil.labelAndFieldSelectorToPredicate;
import java.time.Instant;
import java.util.Comparator;
import java.util.function.Function;
import java.util.function.Predicate;
import org.apache.commons.lang3.BooleanUtils;
@ -51,6 +50,9 @@ public class ReplyServiceImpl implements ReplyService {
if (reply.getSpec().getPriority() == null) {
reply.getSpec().setPriority(0);
}
if (reply.getSpec().getCreationTime() == null) {
reply.getSpec().setCreationTime(Instant.now());
}
return environmentFetcher.fetchComment()
.map(commentSetting -> {
if (Boolean.FALSE.equals(commentSetting.getEnable())) {
@ -104,7 +106,8 @@ public class ReplyServiceImpl implements ReplyService {
@Override
public Mono<ListResult<ListedReply>> list(ReplyQuery query) {
return client.list(Reply.class, getReplyPredicate(query), defaultComparator(),
return client.list(Reply.class, getReplyPredicate(query),
ReplyService.creationTimeAscComparator(),
query.getPage(), query.getSize())
.flatMap(list -> Flux.fromStream(list.get()
.map(this::toListedReply))
@ -140,12 +143,6 @@ public class ReplyServiceImpl implements ReplyService {
"Unsupported owner kind: " + owner.getKind());
}
Comparator<Reply> defaultComparator() {
Function<Reply, Instant> createTime = reply -> reply.getMetadata().getCreationTimestamp();
return Comparator.comparing(createTime)
.thenComparing(reply -> reply.getMetadata().getName());
}
Predicate<Reply> getReplyPredicate(ReplyQuery query) {
Predicate<Reply> predicate = reply -> true;
if (query.getCommentName() != null) {

21
src/main/java/run/halo/app/core/extension/content/Comment.java
View File

@ -1,5 +1,7 @@
package run.halo.app.core.extension.content;
import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
import com.fasterxml.jackson.annotation.JsonIgnore;
import io.swagger.v3.oas.annotations.media.Schema;
import java.time.Instant;
@ -70,6 +72,11 @@ public class Comment extends AbstractExtension {
private Instant approvedTime;
/**
* The user-defined creation time default is <code>metadata.creationTimestamp</code>.
*/
private Instant creationTime;
@Schema(required = true, defaultValue = "0")
private Integer priority;
@ -84,6 +91,15 @@ public class Comment extends AbstractExtension {
@Schema(required = true, defaultValue = "false")
private Boolean hidden;
/**
* If the creation time is null, use approvedTime.
*
* @return if creationTime is null returned approved time, otherwise creationTime.
*/
public Instant getCreationTime() {
return defaultIfNull(creationTime, approvedTime);
}
}
@Data
@ -130,8 +146,9 @@ public class Comment extends AbstractExtension {
if (lastReadTime == null) {
return true;
}
return existingReply.getMetadata().getCreationTimestamp()
.isAfter(lastReadTime);
Instant creationTime = defaultIfNull(existingReply.getSpec().getCreationTime(),
existingReply.getMetadata().getCreationTimestamp());
return creationTime.isAfter(lastReadTime);
})
.count();
return (int) unreadReplyCount;

12
src/main/java/run/halo/app/core/extension/reconciler/CommentReconciler.java
View File

@ -1,16 +1,15 @@
package run.halo.app.core.extension.reconciler;
import java.time.Instant;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.ObjectUtils;
import org.springframework.lang.Nullable;
import org.springframework.stereotype.Component;
import run.halo.app.content.comment.ReplyService;
import run.halo.app.core.extension.Counter;
import run.halo.app.core.extension.content.Comment;
import run.halo.app.core.extension.content.Constant;
@ -110,7 +109,7 @@ public class CommentReconciler implements Reconciler<Reconciler.Request> {
List<Reply> replies = client.list(Reply.class,
reply -> commentName.equals(reply.getSpec().getCommentName())
&& reply.getMetadata().getDeletionTimestamp() == null,
defaultReplyComparator());
ReplyService.creationTimeAscComparator());
// calculate unread reply count
comment.getStatusOrDefault()
@ -181,11 +180,4 @@ public class CommentReconciler implements Reconciler<Reconciler.Request> {
}
return new GroupVersionKind(ref.getGroup(), ref.getVersion(), ref.getKind());
}
Comparator<Reply> defaultReplyComparator() {
Function<Reply, Instant> createTime = reply -> reply.getMetadata().getCreationTimestamp();
return Comparator.comparing(createTime)
.thenComparing(reply -> reply.getMetadata().getName())
.reversed();
}
}

26
src/main/java/run/halo/app/metrics/ReplyEventReconciler.java
View File

@ -1,14 +1,15 @@
package run.halo.app.metrics;
import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
import java.time.Duration;
import java.time.Instant;
import java.util.Comparator;
import java.util.List;
import java.util.function.Function;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.SmartLifecycle;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Component;
import run.halo.app.content.comment.ReplyService;
import run.halo.app.core.extension.content.Comment;
import run.halo.app.core.extension.content.Reply;
import run.halo.app.event.post.ReplyEvent;
@ -51,20 +52,24 @@ public class ReplyEventReconciler implements Reconciler<ReplyEvent>, SmartLifecy
.filter(comment -> comment.getMetadata().getDeletionTimestamp() == null)
.ifPresent(comment -> {
// order by reply creation time desc to get first as last reply time
List<Reply> replies = client.list(Reply.class,
record -> commentName.equals(record.getSpec().getCommentName())
&& record.getMetadata().getDeletionTimestamp() == null,
defaultReplyComparator());
ReplyService.creationTimeAscComparator().reversed());
Comment.CommentStatus status = comment.getStatusOrDefault();
// total reply count
status.setReplyCount(replies.size());
// calculate last reply time
if (!replies.isEmpty()) {
Instant lastReplyTime = replies.get(0).getMetadata().getCreationTimestamp();
status.setLastReplyTime(lastReplyTime);
}
Instant lastReplyTime = replies.stream()
.findFirst()
.map(reply -> defaultIfNull(reply.getSpec().getCreationTime(),
reply.getMetadata().getCreationTimestamp())
)
.orElse(null);
status.setLastReplyTime(lastReplyTime);
Instant lastReadTime = comment.getSpec().getLastReadTime();
status.setUnreadReplyCount(Comment.getUnreadReplyCount(replies, lastReadTime));
@ -74,13 +79,6 @@ public class ReplyEventReconciler implements Reconciler<ReplyEvent>, SmartLifecy
return new Result(false, null);
}
Comparator<Reply> defaultReplyComparator() {
Function<Reply, Instant> createTime = reply -> reply.getMetadata().getCreationTimestamp();
return Comparator.comparing(createTime)
.thenComparing(reply -> reply.getMetadata().getName())
.reversed();
}
@Override
public Controller setupWith(ControllerBuilder builder) {
return new DefaultController<>(

52
src/main/java/run/halo/app/theme/finders/impl/CommentFinderImpl.java
View File

@ -1,10 +1,8 @@
package run.halo.app.theme.finders.impl;
import java.security.Principal;
import java.time.Instant;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.function.Function;
import java.util.function.Predicate;
import org.apache.commons.lang3.BooleanUtils;
@ -13,9 +11,11 @@ import org.apache.commons.lang3.StringUtils;
import org.springframework.security.core.context.ReactiveSecurityContextHolder;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.util.Assert;
import org.springframework.util.comparator.Comparators;
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;
@ -70,12 +70,11 @@ public class CommentFinderImpl implements CommentFinder {
@Override
public Mono<ListResult<ReplyVo>> listReply(String commentName, Integer page, Integer size) {
Comparator<Reply> comparator =
Comparator.comparing(reply -> reply.getMetadata().getCreationTimestamp());
return fixedReplyPredicate(commentName)
.flatMap(fixedPredicate ->
client.list(Reply.class, fixedPredicate,
comparator.reversed(), pageNullSafe(page), sizeNullSafe(size))
ReplyService.creationTimeAscComparator(), pageNullSafe(page),
sizeNullSafe(size))
.flatMap(list -> Flux.fromStream(list.get().map(this::toReplyVo))
.concatMap(Function.identity())
.collectList()
@ -166,19 +165,36 @@ public class CommentFinderImpl implements CommentFinder {
}
static Comparator<Comment> defaultComparator() {
Function<Comment, Boolean> top =
comment -> Objects.requireNonNullElse(comment.getSpec().getTop(), false);
Function<Comment, Integer> priority =
comment -> Objects.requireNonNullElse(comment.getSpec().getPriority(), 0);
Function<Comment, Instant> creationTimestamp =
comment -> comment.getMetadata().getCreationTimestamp();
Function<Comment, String> name =
comment -> comment.getMetadata().getName();
return Comparator.comparing(top)
.thenComparing(priority)
.thenComparing(creationTimestamp)
.thenComparing(name)
.reversed();
return new CommentComparator();
}
static class CommentComparator implements Comparator<Comment> {
@Override
public int compare(Comment c1, Comment c2) {
boolean c1Top = BooleanUtils.isTrue(c1.getSpec().getTop());
boolean c2Top = BooleanUtils.isTrue(c2.getSpec().getTop());
if (c1Top == c2Top) {
int c1Priority = ObjectUtils.defaultIfNull(c1.getSpec().getPriority(), 0);
int c2Priority = ObjectUtils.defaultIfNull(c2.getSpec().getPriority(), 0);
if (c1Top) {
// 都置顶
return Integer.compare(c1Priority, c2Priority);
}
// 两个评论不置顶根据 creationTime 降序排列
return Comparator.comparing(
(Comment comment) -> comment.getSpec().getCreationTime(),
Comparators.nullsLow())
.thenComparing((Comment comment) -> comment.getMetadata().getName())
.compare(c2, c1);
} else if (c1Top) {
// 只有 c1 置顶，c1 排前面
return -1;
} else {
// 只有c2置顶, c2排在前面
return 1;
}
}
}
int pageNullSafe(Integer page) {

1
src/test/java/run/halo/app/content/comment/CommentServiceImplTest.java
View File

@ -127,6 +127,7 @@ class CommentServiceImplTest {
verify(client, times(1)).create(captor.capture());
Comment comment = captor.getValue();
comment.getSpec().setCreationTime(null);
JSONAssert.assertEquals("""
{
"spec": {

10
src/test/java/run/halo/app/content/comment/CommentSorterTest.java
View File

@ -102,7 +102,7 @@ class CommentSorterTest {
@Test
void sortByDefaultDesc() {
Comparator<Comment> defaultComparator = CommentSorter.defaultCommentComparator().reversed();
Comparator<Comment> defaultComparator = CommentSorter.lastReplyTimeComparator().reversed();
List<String> commentNames = comments().stream()
.sorted(defaultComparator)
.map(comment -> comment.getMetadata().getName())
@ -124,8 +124,9 @@ class CommentSorterTest {
commentA.getMetadata().setName("A");
// create time
commentA.getMetadata().setCreationTimestamp(now.plusSeconds(10));
commentA.setSpec(new Comment.CommentSpec());
commentA.getSpec().setCreationTime(commentA.getMetadata().getCreationTimestamp());
commentA.setStatus(new Comment.CommentStatus());
// last reply time
commentA.getStatus().setLastReplyTime(now.plusSeconds(5));
@ -140,6 +141,7 @@ class CommentSorterTest {
commentB.setStatus(new Comment.CommentStatus());
commentB.getStatus().setLastReplyTime(now.plusSeconds(15));
commentB.getStatus().setReplyCount(8);
commentB.getSpec().setCreationTime(commentB.getMetadata().getCreationTimestamp());
Comment commentC = new Comment();
commentC.setMetadata(new Metadata());
@ -151,6 +153,7 @@ class CommentSorterTest {
commentC.setStatus(new Comment.CommentStatus());
commentC.getStatus().setLastReplyTime(now.plusSeconds(3));
commentC.getStatus().setReplyCount(10);
commentC.getSpec().setCreationTime(commentC.getMetadata().getCreationTimestamp());
return List.of(commentA, commentB, commentC);
}
@ -165,6 +168,7 @@ class CommentSorterTest {
commentD.getMetadata().setCreationTimestamp(now.plusSeconds(50));
commentD.setSpec(new Comment.CommentSpec());
commentD.getSpec().setCreationTime(commentD.getMetadata().getCreationTimestamp());
commentD.setStatus(new Comment.CommentStatus());
Comment commentE = new Comment();
@ -172,8 +176,8 @@ class CommentSorterTest {
commentE.getMetadata().setName("E");
commentE.getMetadata().setCreationTimestamp(now.plusSeconds(20));
commentE.setSpec(new Comment.CommentSpec());
commentE.getSpec().setCreationTime(commentE.getMetadata().getCreationTimestamp());
commentE.setStatus(new Comment.CommentStatus());
List<Comment> comments = new ArrayList<>(comments());

52
src/test/java/run/halo/app/content/comment/ReplyServiceTest.java Normal file
View File

@ -0,0 +1,52 @@
package run.halo.app.content.comment;
import static org.assertj.core.api.Assertions.assertThat;
import java.time.Instant;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import run.halo.app.core.extension.content.Reply;
import run.halo.app.extension.Metadata;
/**
* Tests for {@link ReplyService}.
*
* @author guqing
* @since 2.0.0
*/
class ReplyServiceTest {
private final Instant now = Instant.now();
@Test
void creationTimeAscComparator() {
// creation time:
// 1. now + 5s, name: 1
// 2. now + 3s, name: 2
// 3. now + 3s, name: 3
// 5. now + 1s, name: 4
// 6. now - 1s, name: 5
// 7. null, name: 6
Reply reply1 = createReply("1", now.plusSeconds(5));
Reply reply2 = createReply("2", now.plusSeconds(3));
Reply reply3 = createReply("3", now.plusSeconds(3));
Reply reply4 = createReply("4", now.plusSeconds(1));
Reply reply5 = createReply("5", now.minusSeconds(1));
Reply reply6 = createReply("6", null);
String result = Stream.of(reply1, reply2, reply3, reply4, reply5, reply6)
.sorted(ReplyService.creationTimeAscComparator())
.map(reply -> reply.getMetadata().getName())
.collect(Collectors.joining(", "));
assertThat(result).isEqualTo("5, 4, 2, 3, 1, 6");
}
Reply createReply(String name, Instant creationTime) {
Reply reply = new Reply();
reply.setMetadata(new Metadata());
reply.getMetadata().setName(name);
reply.getMetadata().setCreationTimestamp(now);
reply.setSpec(new Reply.ReplySpec());
reply.getSpec().setCreationTime(creationTime);
return reply;
}
}

57
src/test/java/run/halo/app/theme/finders/impl/CommentFinderImplTest.java
View File

@ -5,8 +5,10 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.when;
import java.time.Instant;
import java.util.List;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
@ -25,6 +27,7 @@ import run.halo.app.core.extension.content.Reply;
import run.halo.app.extension.GroupVersionKind;
import run.halo.app.extension.ListResult;
import run.halo.app.extension.Metadata;
import run.halo.app.extension.MetadataOperator;
import run.halo.app.extension.ReactiveExtensionClient;
import run.halo.app.extension.Ref;
import run.halo.app.infra.AnonymousUserConst;
@ -100,6 +103,60 @@ class CommentFinderImplTest {
.verifyComplete();
}
@Test
void commentComparator() {
// 1, now + 1s, top, 0
// 2, now + 2s, top, 1
// 3, now + 3s, top, 2
// 4, now + 4s, top, 2
// 5, now + 4s, top, 3
// 6, now + 1s, no, 0
// 7, now + 2s, no, 0
// 8, now + 3s, no, 0
// 9, now + 3s, no, 0
// 10, null, no, 0
// 11, null, no, 1
// 12, null, no, 3
// 13, now + 3s, no, 3
Instant now = Instant.now();
var comment1 = commentForCompare("1", now.plusSeconds(1), true, 0);
var comment2 = commentForCompare("2", now.plusSeconds(2), true, 1);
var comment3 = commentForCompare("3", now.plusSeconds(3), true, 2);
var comment4 = commentForCompare("4", now.plusSeconds(4), true, 2);
var comment5 = commentForCompare("5", now.plusSeconds(4), true, 3);
var comment6 = commentForCompare("6", now.plusSeconds(4), true, 3);
var comment7 = commentForCompare("7", now.plusSeconds(1), false, 0);
var comment8 = commentForCompare("8", now.plusSeconds(2), false, 0);
var comment9 = commentForCompare("9", now.plusSeconds(3), false, 0);
var comment10 = commentForCompare("10", now.plusSeconds(3), false, 0);
var comment11 = commentForCompare("11", null, false, 0);
var comment12 = commentForCompare("12", null, false, 1);
var comment13 = commentForCompare("13", null, false, 3);
var comment14 = commentForCompare("14", now.plusSeconds(3), false, 3);
var result = Stream.of(comment1, comment2, comment3, comment4, comment5, comment6,
comment7, comment8, comment9, comment10, comment11, comment12, comment13,
comment14)
.sorted(CommentFinderImpl.defaultComparator())
.map(Comment::getMetadata)
.map(MetadataOperator::getName)
.collect(Collectors.joining(", "));
assertThat(result).isEqualTo("1, 2, 3, 4, 5, 6, 9, 14, 10, 8, 7, 13, 12, 11");
}
Comment commentForCompare(String name, Instant creationTime, boolean top, int priority) {
Comment comment = new Comment();
comment.setMetadata(new Metadata());
comment.getMetadata().setName(name);
comment.getMetadata().setCreationTimestamp(Instant.now());
comment.setSpec(new Comment.CommentSpec());
comment.getSpec().setCreationTime(creationTime);
comment.getSpec().setTop(top);
comment.getSpec().setPriority(priority);
return comment;
}
@SuppressWarnings("unchecked")
private void mockWhenListComment() {
// Mock