Skip to content

feat: add a random article method to the post finder #7471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chengzhongxue
Copy link
Contributor

What type of PR is this?

/area core
/kind feature

What this PR does / why we need it:

为 postFinder 添加 随机文章方法

Which issue(s) this PR fixes:

Fixes #7327

Does this PR introduce a user-facing change?

为 postFinder 添加 随机文章方法

@f2c-ci-robot f2c-ci-robot bot added area/core Issues or PRs related to the Halo Core release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels May 24, 2025
@f2c-ci-robot f2c-ci-robot bot requested review from JohnNiang and wan92hen May 24, 2025 08:31
Copy link

f2c-ci-robot bot commented May 24, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ruibaby for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@JohnNiang
Copy link
Member

/milestone 2.21.x

@f2c-ci-robot f2c-ci-robot bot added this to the 2.21.x milestone May 26, 2025
public Flux<ListedPostVo> random(Integer limit) {

return postPredicateResolver.getListOptions()
.flatMapMany(listOptions -> client.listAll(Post.class, listOptions, Sort.unsorted()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这看起来会查询所有文章,可能会导致查询性能问题。

@halo-dev/sig-halo 请帮忙确认是否有其他效率更高的方式,如果没有,我认为不值得添加这个方法。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里确实会有性能问题。但是目前我也想不出任何有效的办法。

@JohnNiang JohnNiang requested a review from Copilot May 26, 2025 15:38
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a random article method to the post finder to return a randomly shuffled subset of posts.

  • Implemented a new random(Integer limit) method in PostFinderImpl using a custom shuffle algorithm.
  • Updated the PostFinder interface to declare the new random method.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
application/src/main/java/run/halo/app/theme/finders/impl/PostFinderImpl.java Added random method and a helper to shuffle posts using a custom Fisher-Yates algorithm.
application/src/main/java/run/halo/app/theme/finders/PostFinder.java Added random method declaration in the interface.

Comment on lines +313 to +320
Random random = new Random();
for(int i= shuffledList.size() - 1; i > 0; i--) {
int index = random.nextInt(i + 1);
// 交换元素
Post temp = shuffledList.get(index);
shuffledList.set(index, shuffledList.get(i));
shuffledList.set(i, temp);
}
Copy link
Preview

Copilot AI May 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider using Collections.shuffle(shuffledList, random) instead of a custom shuffle loop to simplify the code and reduce potential maintenance overhead.

Suggested change
Random random = new Random();
for(int i= shuffledList.size() - 1; i > 0; i--) {
int index = random.nextInt(i + 1);
// 交换元素
Post temp = shuffledList.get(index);
shuffledList.set(index, shuffledList.get(i));
shuffledList.set(i, temp);
}
Collections.shuffle(shuffledList, new Random());

Copilot uses AI. Check for mistakes.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@JohnNiang
Copy link
Member

/hold

Hold until we find a better solution.

@f2c-ci-robot f2c-ci-robot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core Issues or PRs related to the Halo Core do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

提供随机文章 API
3 participants