-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: 인기 게시글 기능 구현 완료 #610
Conversation
...java/page/clab/api/domain/community/board/application/service/HotBoardsRetrievalService.java
Outdated
Show resolved
Hide resolved
...java/page/clab/api/domain/community/board/application/service/HotBoardsRetrievalService.java
Outdated
Show resolved
Hide resolved
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.
메소드추출을 잘 해주셔서 로직을 이해하기 편했습니다.
다만, 저와 @SongJaeHoonn님, @Jeong-Ag님이 핫 게시글 정책에 대해 회의를 했을 때,
일주일마다 핫 게시글을 새로 등재하기로 이야기 나눴던 것 같아요.
지금까지의 작업으로는 매번 API를 호출할 때마다 핫 게시글을 판단하는 쿼리문이 실행되는 것 같습니다. 즉, 일주일에 한 번 핫 게시글이 갱신 되는 것이 아니라 페이지 방문할 때마다 핫 게시글이 계속 갱신 되는 것 같아요. (물론, 게시글이 많이 올라오고, 반응도 그만큼 많이 달려야겠지만요)
이러한 동작은 저희가 처음 설계한 정책과는 조금 다른 모습이기도 하고, api 호출 시 발생하는 쿼리가 많아서 부담이 되어보입니다.
따라서 핫 게시글 선정 로직을 매주 스케쥴링을 걸어 DB에 저장해두는 방식을 제안드립니다.
@mingmingmon 그러나 현재 방식은 API를 호출할 때마다 댓글 수와 이모지 수를 계산하는 복잡한 로직들과 복잡한 조회 방식이 요구되므로 원래의 DB에 |
...va/page/clab/api/domain/community/board/adapter/out/persistence/BoardPersistenceAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/page/clab/api/domain/community/board/application/port/out/RetrieveBoardPort.java
Outdated
Show resolved
Hide resolved
...java/page/clab/api/domain/community/board/application/service/HotBoardsRetrievalService.java
Outdated
Show resolved
Hide resolved
...java/page/clab/api/domain/community/board/application/service/HotBoardsRetrievalService.java
Outdated
Show resolved
Hide resolved
...java/page/clab/api/domain/community/board/application/service/HotBoardsRetrievalService.java
Outdated
Show resolved
Hide resolved
...n/java/page/clab/api/domain/community/board/adapter/in/web/HotBoardsRetrievalController.java
Outdated
Show resolved
Hide resolved
...java/page/clab/api/domain/community/board/application/service/HotBoardsRetrievalService.java
Outdated
Show resolved
Hide resolved
@SongJaeHoonn 님의 제안도 효율적인 접근 방식이지만, 추가적으로 캐싱 전략을 활용하는 방안도 고려해보시면 좋을 것 같아요. 새로운 컬럼을 추가할 경우 테이블 관리와 데이터 정합성 유지가 더 복잡해질 수 있고, 매번 연산이 발생할 때마다 기존 데이터를 동기화해야 한다는 부담이 있어요. 이번 인기 게시글 기능처럼 정합성이 절대적으로 요구되지 않고 주기적으로 갱신되는 데이터라면, TTL(Time to Live) 설정이 가능한 Redis와 같은 캐시 시스템을 통해 성능을 최적화할 수 있어요. 예를 들어, 인기 게시글 정보를 Redis에 저장하고 일정 시간마다 갱신하도록 설정하면 데이터베이스 부하를 줄이면서도 빠른 응답성을 유지할 수 있어요. 또한, 현재는 카테고리와 무관하게 인기 게시글이 관리되는 것으로 보이는데, 카테고리별 인기 게시글을 도입하면 사용자에게 더욱 맞춤화된 정보를 제공할 수 있을 것으로 보여요. |
...java/page/clab/api/domain/community/board/application/service/HotBoardsRetrievalService.java
Outdated
Show resolved
Hide resolved
프론트에서 인기 게시글을 어떻게 표시할지는 모르지만, 일반 게시글과 함께 하나의 목록으로 표시된다면, 게시글 조회 시 인기 게시글 정보를 함께 반환하여 API 호출 횟수를 줄이는 방법도 고려해보시면 좋을 것 같네요. |
Redis를 사용하는 것이 익숙하지 않아 이런 방법은 생각해보지 않았는데, 인메모리로 동작하는 Redis 특성상 조회가 빠르고 DB에 부담을 주지 않아 좋을 것 같아요..! 고려해보도록 하겠습니다. 그리고, 카테고리별 인기 게시글을 생각해보긴 했지만 회의 결과 활발하지 않은 카테고리는 인기 게시글이 계속 고정되거나 아예 올라오지 않을 가능성도 고려하여 모든 카테고리에서 전체 게시글 중의 인기 게시글을 표시하기로 결정했습니다. |
저는 인기 게시글 조회 API를 따로 추출하는 것이 좋을 것 같아요. @limehee 님이 제안 주신 방식으로 구현하게 되면 페이지네이션된 결과(10개의 일반 게시글)와 다른 응답 객체(5개의 인기 게시글)을 다시 묶어서 responseDto로 반환하는 방식일 것 같은데, 제가 이해한 바가 맞을까요? 현재 일반 게시글 조회 시에 페이지네이션이 적용되어있어요. 만약 일반 게시글 조회에서 인기 게시글을 함께 반환하도록 한다면, 10개의 일반게시글 + 5개의 인기 게시글이 함께 반환되는 형태가 될 것 같습니다. 그렇게 되면 responseDto가 더욱 복잡해지고, 사용자가 페이지를 넘길 때 마다 동일한 정보인 5개의 인기 게시글이 중복해서 전달될 것 같아요. |
제가 코드를 꼼꼼히 살펴보지 못하고 이전 코멘트에서 "일반 게시글"이라는 모호한 표현을 사용하면서 혼동을 드린 것 같아요(코멘트를 잘못 남긴 것 같습니다). 이에 따라 남겨주신 의견과 다소 어긋난 부분이 있을 수 있다는 점 양해 부탁드립니다.
|
@limehee @mingmingmon 변경 사항
Strategy 패턴 적용시 전략 클래스들은 일단 테스트id 2, 3 ⇒ 지지지난주 2번 댓글 한개
스케줄링 후 (8번을 지난주로 옮김)
|
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.
피드백 반영 감사합니다! 좋은 작업물로 저도 많이 배워갑니다.
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.
인기 게시글 산정 방식에 대해 전략 패턴을 적용해달라고 요청드렸었는데, 현재 코드를 확인해보니 산정 방식뿐만 아니라 요청에 따라 DB에서 데이터를 조회하고 결과를 생성하는 전체 과정을 하나의 전략으로 묶은 것처럼 보여요.
그러나, 인기 게시글 산정 로직 외의 부분은 모든 전략에서 동일하게 동작할 가능성이 높기 때문에, 산정 로직만을 전략 패턴으로 분리하는 것이 더 적합하다고 판단돼요.
또한, 클래스명이 DefaultHotBoardService
로 되어 있는데, 일반적으로 전략 패턴을 구현하는 클래스명은 ___Strategy
의 형태를 따르는 경우가 많아요. 클래스명을 더 명확히 변경해주세요.
요약
- 전략 패턴은 특정 동작만을 분리하여 캡슐화하는 데 초점이 맞춰져 있어요. 현재 코드는 전략이 과도하게 많은 역할을 맡고 있어요.
- 게시글 조회나 매핑같은 로직은 산정 방식과 무관하게 모든 전략에서 동일하게 작동할 가능성이 높아요. 이러한 로직은 전략이 아닌 상위 서비스에서 관리되어야 해요.
DefaultHotBoardService
라는 이름은 전략 패턴의 일반적인 명명 규칙(___Strategy
)에 부합하지 않아요.DefaultHotBoardStrategy
와 같이 역할이 더 명확히 드러나는 이름으로 변경해주세요.
참고
...in/java/page/clab/api/domain/community/board/application/service/DefaultHotBoardService.java
Outdated
Show resolved
Hide resolved
피드백주신 사항 잘 이해했습니다! 그리고 전략 패턴 책임 분리에 관해서는, 이렇게 전략의 책임을 분리하고 나니, 컨트롤러에서는 인기게시글 선정과 저장에 관심사를 두는 것이 아니라 redis에서 조회하는 비즈니스 로직에만 연결되어 있기 때문에 인기게시글 선정에 관여할 수 없었습니다. 이런 상황에서는 어떻게 코드를 작성해야 할까요? |
전략 패턴 적용 방식에 대한 개선 방향을 제안드려요.
관련 구현 사례 제가 이전에 구현했던 전략 패턴 기반 코드가 현재 논의 중인 설계와 유사한 점이 많아, 참고하실 수 있도록 공유드려요. https://github.com/limehee/ngram-similarity-search/blob/main/NGramSimilarityService.java 해당 코드는 유사도 계산에 다양한 알고리즘을 적용하기 위해 전략 패턴을 활용한 구현 사례에요. 현재 설계에서 참고할만한 부분은 다음과 같아요.
|
상세히 써주신 리뷰와, 좋은 설계 패턴이 담겨있는 프로젝트를 참고용으로 추천해주셔서 정말 감사합니다! 하지만,
이와 같은 이유들로 코드 수정이 어려웠으며, 예시 들어주신 유사도 계산 알고리즘을 적용하는 프로젝트에서는, 우리의 인기 게시글 기능과는 달리 유사도 계산 및 저장에 클라이언트가 관여할 수 있는 것 같아 이 답글을 작성하는 바입니다..! |
@SongJaeHoonn
전략명을 상수로 관리하면, 굳이 열거형(enum)을 사용하여 설명을 추가할 필요가 없다고 생각했습니다. 또한, 전략에 따른 동작을 선언할 필요도 없을 것 같다는 판단 하에 이 방법을 제안드렸습니다. 열거형을 사용할 경우 현재 전략의 빈 이름이 상수로 관리되지 않고 하드코딩되어 있습니다. 추후 전략 이름이 변경될 경우를 대비하여 @Service("default") // 상수로 변경 필요
@RequiredArgsConstructor
public class DefaultHotBoardSelectionStrategy implements HotBoardSelectionStrategy {
현재 public void registerHotBoards() {
HotBoardSelectionStrategy strategy = strategies.get(HotBoardSelectionStrategyType.DEFAULT.getKey()); // DEFAULT 전략으로 고정됨 따라서, 이전에 제안드린 대로 클라이언트로부터 인기 게시글 선정 방식을 받아 요청에 따라 다양한 전략을 유동적으로 사용할 수 있도록 설계를 개선하는 것이 더 나은 방향일 것 같습니다.
Redis 캐싱과 제가 이전에 제안드린 내용이 상충하는 부분이 있어 위와 같은 코멘트를 남기신 것으로 생각됩니다. 말씀하신 대로 모든 산정 방식을 미리 실행하여 결과를 저장해두는 것은 비효율적입니다. 번거롭겠지만 캐싱 방식을 다음과 같이 변경할 것을 제안드립니다.
이렇게 변경하면 필요한 시점에만 전략을 실행하고 캐싱함으로써 효율성을 높일 수 있을 것으로 생각됩니다. 요약
|
@limehee
|
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.
“HotBoardSelectionStrategies”는 실질적인 도메인 로직보다는 전략 식별용 상수를 모아둔 유틸성 클래스에 가까워서 service 계층 쪽 패키지로 옮겨 관리하는 편이 더 자연스러울 것 같아요.
뭔가 서비스 계층에서는 비즈니스 로직들만 처리한다는 인식이 있어서 해당 상수들을 service 패키지에서만 이용하다보니 import문이 하나씩 줄어들어서 이런 점에서도 괜찮네요! |
public class HotBoardSelectionStrategies { | ||
|
||
public static final String DEFAULT = "default"; | ||
|
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.
전략패턴과 OCP 관련해서 많은 고민들을 진행해주신 작업인 것 같습니다! 저도 디자인 패턴 책 읽을 때 가볍게
들어봤던 전략패턴에 대해서 실제로 어떻게 사용할 수 있을 지 알아갈 수 있던 기회였습니다. 제가 @SongJaeHoonn님의 작업을 이해한 게 맞는지 한 번 확인해주시면 감사할 것 같습니다!
-
추후 새로운 인기 게시글 산정 기준이 생긴다면, 이 부분에 전략 이름을 명시하고,
DefaultHotBoardSelectionStrategy
처럼 새로운 전략을 구현하는 방식일까요? -
다양한 전략들은 모두
HotBoardSelectionsStrategy
를 implement하기 때문에 Map으로 관리 될 수 있는 것이고, 전략 이름으로 원하는 것을 사용할 수 있는 것인거죠?
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.
추후 새로운 인기 게시글 산정 기준이 생긴다면, 이 부분에 전략 이름을 명시하고, DefaultHotBoardSelectionStrategy처럼 새로운 전략을 구현하는 방식일까요?
정확합니다!
다양한 전략들은 모두 HotBoardSelectionsStrategy를 implement하기 때문에 Map으로 관리 될 수 있는 것이고, 전략 이름으로 원하는 것을 사용할 수 있는 것인거죠?
맞습니다. 클라이언트가 파라미터로 전략 이름과 함께 요청하면, HotBoardSelectionsStrategy
가 전략 이름으로 매핑된 Map에서 전략을 찾아 인기 게시글을 저장(해당 전략으로 저장된 인기게시글이 없을 시)하거나 조회(해당 전략으로 저장된 인기게시글이 있을 시)하게 됩니다.
Summary
게시판 상단에 핫 게시글을 노출하는 기능을 위해, 지난주의 인기 게시글을 조회하는 api를 구현했습니다.
Tasks
ETC
size
를 추가해 개수 조절이 가능하도록 구현했습니다. (기본값은 5)size
개보다 적다면, 이번 주를 기준으로 두 주 전, 세 주 전... 을 계속 조회하여 핫 게시글이size
개수가 될 때까지 반복합니다.size
개보다 적다면 전체 게시글을 반응 순으로 정렬 후 반환합니다.API Response
2024-11-08
일에 api 호출했을 때 (size = 3)1번 게시글 댓글 1, 이모지 2
2번 게시글 댓글 1, 이모지 5
3번 게시글 댓글 1, 이모지 1
1번 게시글 댓글 1, 이모지 1
2번 게시글 댓글 1, 이모지 1
3번 게시글 댓글 1, 이모지 1
-> 최신 순 정렬
size - 1
개 이하일 때11월 8일 기준 지난주는 10월 28일 ~ 11월 3일 (1, 2번)
1번 → 댓글 1개, 이모지 0개
2번 → 댓글 0개, 이모지 0개
5번, 6번, 7번 → 지지난주 핫게시글