-
Notifications
You must be signed in to change notification settings - Fork 0
[DP-516] 기술블로그 ES → MySQL 마이그레이션 #185
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
Conversation
…roved clarity and structure
ssosee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다.
| .or(techArticle.regDate.eq(findTechArticle.getRegDate()) | ||
| .and(techArticle.regDate.eq(findTechArticle.getRegDate()))); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 기억이 잘 안 나서 그러는데요
reg_date 는 기술블로그 등록일로 알고 있는데, 정확도순이랑 무슨 관계가 있는 건가요?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정확도순과 관련은 없으나, 기술블로그 기본 정렬이 최신순이어서 id 가 아닌 regDate로 2차 정렬하도록 수정했습니다!
| return findTechArticlesByCursorWithKeyword(pageable, techArticleId, techArticleSort, companyId, keyword, score); | ||
| } else { | ||
| return findTechArticlesByCursorWithoutKeyword(pageable, techArticleId, techArticleSort, companyId); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사소한건데, else 를 사용하지 않고 바로 return 하면 가독성이 더 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵!! 반영하겠습니다!
| NumberTemplate<Double> totalScore = Expressions.numberTemplate(Double.class, | ||
| "({0} * 2.0) + {1}", titleScore, contentsScore | ||
| ); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요것도 지난 번 키워드 검색처럼 Boolean mode로 필터링, Natural language mode로 정렬 하면 좋을 것 같은데, 어떻게 생각하시나요?!
// 스코어 계산을 위한 expression
NumberTemplate<Double> titleScore = Expressions.numberTemplate(Double.class,
"function('" + MATCH_AGAINST_NL_FUNCTION + "', {0}, {1})",
techArticle.title.title, keyword
);
NumberTemplate<Double> contentsScore = Expressions.numberTemplate(Double.class,
"function('" + MATCH_AGAINST_NL_FUNCTION + "', {0}, {1})",
techArticle.contents, keyword
);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다 반영하겠습니다!
…endpoint in security constants
…endpoint in security constants
[DP-558] 인스턴스 마이그레이션 from EC2
…rver into DP-516 # Conflicts: # src/main/java/com/dreamypatisiel/devdevdev/domain/service/member/MemberService.java
|
@codex review please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| @Override | ||
| public Slice<TechArticleMainResponse> getTechArticles(Pageable pageable, String elasticId, | ||
| public Slice<TechArticleMainResponse> getTechArticles(Pageable pageable, Long techArticleId, | ||
| TechArticleSort techArticleSort, String keyword, | ||
| Long companyId, Float score, Authentication authentication) { | ||
| // 회원 조회 | ||
| Member member = memberProvider.getMemberByAuthentication(authentication); | ||
|
|
||
| // 엘라스틱서치 기술블로그 조회 | ||
| SearchHits<ElasticTechArticle> elasticTechArticleSearchHits = elasticTechArticleService.getTechArticles( | ||
| pageable, elasticId, techArticleSort, keyword, companyId, score); | ||
| List<ElasticResponse<ElasticTechArticle>> elasticTechArticles = elasticTechArticleService.mapToElasticResponse( | ||
| elasticTechArticleSearchHits); | ||
| // 기술블로그 조회 | ||
| SliceCustom<TechArticle> techArticles = techArticleRepository.findTechArticlesByCursor( | ||
| pageable, techArticleId, techArticleSort, companyId, keyword, score); | ||
|
|
||
| // 데이터 가공 | ||
| List<TechArticleMainResponse> techArticlesResponse = getTechArticlesResponse(elasticTechArticles, member); | ||
| List<TechArticleMainResponse> techArticlesResponse = techArticles.stream() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Return score for keyword pagination
When getTechArticles is called with TechArticleSort.HIGHEST_SCORE, TechArticleRepositoryImpl#getCursorConditionForKeywordSearch requires both the last techArticleId and its score to compute the next page’s cursor. After the migration, MemberTechArticleService (and the guest variant) build responses with TechArticleMainResponse.of(techArticle, member) which never populates the score field. Clients therefore cannot obtain a score to send on subsequent requests, so any follow‑up call with only the article id will pass a null score and the repository returns the first page again. Pagination by accuracy effectively loops on the first page.
Useful? React with 👍 / 👎.
ssosee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
| .where(companyId != null ? techArticle.company.id.eq(companyId) : null) | ||
| .fetchCount(); | ||
|
|
||
| return new SliceCustom<>(contents, pageable, totalElements); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
companyId != null ? techArticle.company.id.eq(companyId) : null 가 중복으로 사용되고 있는데, 함수로 설정하는 것 어떤가요?
e.g)
public BooleanExpression getCompanyIdCondition(Long companyId) {
if(companyId == null) {
return null;
}
return techArticle.company.id.eq(companyId);
}
📝 작업 내용
🔗 참고할만한 자료(선택)
💬 리뷰 요구사항(선택)