Conversation
좋아요, 조회수 별 랭킹 로직 작성 검색 api 작성
| @@ -0,0 +1,52 @@ | |||
| package search; | |||
There was a problem hiding this comment.
Entity의 경우에는 domain모듈에 넣어주세요.Search기능의 경우Article을 가져오는게 주된 기능이기 떄문에Article에서 Entity에 대한 추가가 완료되면 작업이 가능할 것 같습니다.
| public Long getId() { | ||
| return id; | ||
| } | ||
|
|
||
| public void setId(Long id) { | ||
| this.id = id; | ||
| } | ||
|
|
||
| public String getTitle() { | ||
| return title; | ||
| } | ||
|
|
||
| public void setTitle(String title) { | ||
| this.title = title; | ||
| } | ||
|
|
||
| public String getDescription() { | ||
| return description; | ||
| } | ||
|
|
||
| public void setDescription(String description) { | ||
| this.description = description; | ||
| } | ||
|
|
||
| public String getTag() { | ||
| return tag; | ||
| } | ||
|
|
||
| public void setTag(String tag) { | ||
| this.tag = tag; | ||
| } |
There was a problem hiding this comment.
Getter와 Setter 메소드는 필요없습니다.
Lombok의 @Getter, @Setter, @Data에 대해서 알아보고 사용해보세요.
|
|
||
| import java.util.List; | ||
|
|
||
| @Repository |
There was a problem hiding this comment.
JPA를 활용하는 경우에는 @Repository어노테이션이 꼭 필요하지는 않습니다.
| @@ -0,0 +1,16 @@ | |||
| package search; | |||
There was a problem hiding this comment.
Repository의 경우에는 Domain에서 생성을 해야 합니다.
| public interface SearchRepository extends JpaRepository<SearchEntity, Long> { | ||
|
|
||
| // 태그를 기반으로 검색하기 위한 사용자 정의 메서드 | ||
| List<SearchEntity> findByTag(String tag); | ||
|
|
||
| // 자동완성을 위한 사용자 정의 메서드 | ||
| List<SearchEntity> findByTitleStartingWith(String query); |
There was a problem hiding this comment.
Tag또는 Title에서 기능을 다 가져올 수 있을 것 같습니다.
| public SearchVo() { | ||
| // 기본 생성자 | ||
| } |
| @@ -0,0 +1,40 @@ | |||
| package search; | |||
There was a problem hiding this comment.
| package search; | |
| package playlist.server.search.vo; | |
| package playlist.server.search.dto.vo; |
| public SearchVo(String title, String description) { | ||
| this.title = title; | ||
| this.description = description; | ||
| } |
| @Override | ||
| public String toString() { | ||
| return "SearchVo{" + | ||
| "title='" + title + '\'' + | ||
| ", description='" + description + '\'' + | ||
| '}'; | ||
| } | ||
| } |
| public String getTitle() { | ||
| return title; | ||
| } | ||
|
|
||
| public void setTitle(String title) { | ||
| this.title = title; | ||
| } | ||
|
|
||
| public String getDescription() { | ||
| return description; | ||
| } | ||
|
|
||
| public void setDescription(String description) { | ||
| this.description = description; |
donsonioc2010
left a comment
donsonioc2010
left a comment
There was a problem hiding this comment.
Lambda나 어노테이션의 활용이 높아진거 매우 보기좋네요,
아직 소스코드 수정이 좀더 필요할 것 같아서 먼저 수정이 필요해 보이는 부분에 대해서만 코멘트를 달았습니다.
수정 끝나면 기능 바로바로 같이 풀어보도록 합시다.
진짜 잘하고 있어용 👍
| @@ -0,0 +1,20 @@ | |||
| package playlist.server.ranking; | |||
There was a problem hiding this comment.
search의 경우 vo패키지에 클래스를 정리한거 같아 보이는데, 해당 패이지는 그냥 ranking패키지 root경로에 vo를 생성한 이유가 있을까요?
There was a problem hiding this comment.
\MainPageRankingInfo 이게 너무 네이밍이 구린거 같아 고민하다 놓친거 같습니다! 패키지 만들어서 vo 에 넣어뒀습니다!
| @Getter | ||
| @Builder | ||
| public class MainPageRankingInfoVo { | ||
| private final RankingInfo rankingInfo; | ||
| private final RankingType rankingType; | ||
|
|
||
| public static MainPageRankingInfoVo from(RankingInfo rankingInfo, RankingType rankingType) { | ||
| return MainPageRankingInfoVo.builder() | ||
| .rankingInfo(rankingInfo) | ||
| .rankingType(rankingType) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
빌더의 활용 정말 잘했습니당.
주의할점을 말해주자면, Annotation이 편하긴 한데 완벽은 없더라구요,
추후에는 @Builder의 작동원리, 커스텀하는 방법에 대해서도 공부해보면 좋을 것 같아요.
- 저도 최근에 이 어노테이션에 한번 발등을 찍혀봤거든요.ㅋㅋㅋㅋ
| @Operation(summary = "일간 랭킹을 조회합니다.") | ||
| @GetMapping("/daily") | ||
| public ResponseEntity<MainPageRankingInfoVo> getDailyRanking( | ||
| @RequestParam(name = "rankingType", required = false, defaultValue = "DAILY") RankingType rankingType) { |
There was a problem hiding this comment.
메소드를 daily, week, month로 분류를 하였기 때문에, 해당 파라미터가 꼭 필요한가?라는 생각이 드네요.
| private final RankingViewService rankingViewService; | ||
|
|
||
| @Operation(summary = "일간 랭킹을 조회합니다.") | ||
| @GetMapping("/daily") |
There was a problem hiding this comment.
Mapping을 차라리
/daily?type=view또는/daily?type=like/daily/{type}같은 방식으로 받은 이후 PathParameter로 받은 type을 view또는 like로 구분해서 로직을 태우는게 맞지 않을 까 합니다.
There was a problem hiding this comment.
type 매개변수 사용해서 type값에 따라 좋아요랑 조회수 랭킹 정보 가져오는 로직 만들어 봤습니다!
| @RestController | ||
| @RequestMapping("/ranking") | ||
| @RequiredArgsConstructor | ||
| @Tag(name = "1. [랭킹]") |
There was a problem hiding this comment.
태그로 어울리는 정보를 전달하는거 매우 좋아보입니다 ^^
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| @ToString | ||
| @Getter | ||
| @Setter |
There was a problem hiding this comment.
@Data라는 Annotation도 있으니 활용해보면 좋을 것 같습니다 👍
| @Entity | ||
| @Getter | ||
| @Table(name = "tbl_ranking") | ||
| @NoArgsConstructor | ||
| public class Ranking { | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| private RankingType rankingType; |
There was a problem hiding this comment.
생각해보면, 일단 Redis로 구현을 하고 있는데, 굳이 Ranking Entity가 필요한가 의문이네요
There was a problem hiding this comment.
바로 지워버리겠습니다.
Redis와 같은 NoSQL 데이터베이스를 사용할 때에는 보통 관계형 데이터베이스와 달리 엔티티를 정의하고 관계를 매핑하는 작업이 필요하지 않다는 것을 알았군요!
| RankingInfo(String countsKey, String rankingKey) { | ||
| this.countsKey = countsKey; | ||
| this.rankingKey = rankingKey; | ||
| } | ||
|
|
||
| public String getCountsKey() { | ||
| return countsKey; | ||
| } | ||
|
|
||
| public String getRankingKey() { | ||
| return rankingKey; | ||
| } |
There was a problem hiding this comment.
@AllArgsConstructors, @Getter, @Setter
| private final String countsKey; | ||
| private final String rankingKey; |
| @Entity | ||
| @Getter | ||
| @Setter | ||
| @NoArgsConstructor | ||
| public class Search { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| private String title; | ||
| private String description; | ||
| private String tag; |
There was a problem hiding this comment.
Search의 경우에는 다른 팀이 직접 구현한 엔티티를 활용해야 하다보니, Search Entity가 필요한 이유가 납득이 가지 않네용.
왜 생성한 것인지 의도를 풀어줄 수 있을까요?
There was a problem hiding this comment.
다른 팀이 직접 구현한 엔티티를 활용해야 하다보니 사실 이걸 생각한다는 걸 생각하긴 했는데 , 한 번 Search 관련 Api를 쭉 만들어보고 싶어서 해봤습니다!
donsonioc2010
left a comment
donsonioc2010
left a comment
There was a problem hiding this comment.
- 많이 좋아졌어오
- 잊고있던게, 전체적으로 log를 활용해서 로직 실행에 따른 기록을 추가하면 좋을 것 같습니다.
- Redis도 Entity같은게 있을것 같아 알아보니 RedisHash라는게 있었네요. 읽어보고 구현해보면 좋을 것 같아요.
| try { | ||
| MainPageRankingInfoVo rankingInfoVo = rankingService.getRanking("weekly", type, "boardId"); | ||
| if (rankingInfoVo == null) { | ||
| return ResponseEntity.notFound().build(); | ||
| } | ||
| return ResponseEntity.ok(rankingInfoVo); | ||
| } catch (Exception e) { | ||
| return ResponseEntity.badRequest().build(); | ||
| } |
There was a problem hiding this comment.
다른메소드 동일하지만 service레벨에서 Exception을 내주고 있는것 같아서 badRequest Return의 경우에는 발생시키는 이유를 모르겠네요.
코드를 이런식으로 만들어 볼 수 있지않을까요?
| try { | |
| MainPageRankingInfoVo rankingInfoVo = rankingService.getRanking("weekly", type, "boardId"); | |
| if (rankingInfoVo == null) { | |
| return ResponseEntity.notFound().build(); | |
| } | |
| return ResponseEntity.ok(rankingInfoVo); | |
| } catch (Exception e) { | |
| return ResponseEntity.badRequest().build(); | |
| } | |
| MainPageRankingInfoVo rankingInfoVo = rankingService.getRanking(RankingInfo.WEEKLY, type, "boardId"); | |
| if (rankingInfoVo == null) { | |
| return ResponseEntity.notFound().build(); | |
| } | |
| return ResponseEntity.ok(rankingInfoVo); |
There was a problem hiding this comment.
두번째 인자값인 type의 경우에도 enum으로 정의가 가능할 것 같아요.
세번째 인자값인 boardId는 차라리 어딘가에 public static String으로 변수선언한 이후에 해당 값을 모두 공유해서 사용하면
추후 변경이 발생시 편하게 공유할 수 있지 않을까요?
There was a problem hiding this comment.
boardId 이거는 못했습니다..두번째 인자값인 type의 경우에도 enum으로 정의가 가능할 것 같아요. 이건 했습니다!
| String countsKey = rankingType + "_LIKE_COUNTS"; | ||
| redisTemplate.opsForHash().increment(countsKey, boardId, 1L); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("좋아요 증가 실패"); |
There was a problem hiding this comment.
RuntimeException의 사용은 우리 프로젝트에서 사용을 하면 안됩니다.
Handling은 모두 BaseException을 활용하고 있기 때문에, BaseException을 상속받아 Exception을 Custom하게 제작하여 사용해 주시기 바랍니다 :)
그래도 Exception을 선언해서 안전한 개발방식을 만든건 좋네요 👍
| RankingInfo rankingInfo; | ||
| if ("daily".equalsIgnoreCase(rankingType)) { | ||
| rankingInfo = RankingInfo.DAILY; | ||
| } else if ("weekly".equalsIgnoreCase(rankingType)) { | ||
| rankingInfo = RankingInfo.WEEKLY; | ||
| } else if ("monthly".equalsIgnoreCase(rankingType)) { | ||
| rankingInfo = RankingInfo.MONTHLY; | ||
| } else { | ||
| throw new IllegalArgumentException("유효하지 않은 랭킹 타입"); | ||
| } |
There was a problem hiding this comment.
Controller레벨에서 rankingType을 문자열로 주지않고, Enum타입을 주면 해당 코드도 필요 없지 않을까요?
그래도 서비스레이어에서 모든 로직처리하는건 괜찮아 보이네요.
|
|
||
| return MainPageRankingInfoVo.from(rankingInfo, RankingType.valueOf(rankingType.toUpperCase())); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("데이터 가져오는거 실패"); |
There was a problem hiding this comment.
Custom하게 제작한 Exception을 활용해주세요 👍
There was a problem hiding this comment.
DataFetchException, InvalidParameterException 완료!
| if ("like".equals(type)) { | ||
| rankingLikeService.incrementLikes(rankingType, boardId); | ||
| } else if ("view".equals(type)) { | ||
| rankingViewService.incrementViews(rankingType, boardId); |
There was a problem hiding this comment.
like와 view도 enum으로 정의해보는게 좋을것 같아요
| throw new IllegalArgumentException("유효하지 않은 파라미터"); | ||
| } | ||
|
|
||
| return MainPageRankingInfoVo.from(rankingInfo, RankingType.valueOf(rankingType.toUpperCase())); |
There was a problem hiding this comment.
혹시 Enum만 들어있는 정보를 Return하는 이유가 있을까요?
생각해보니 랭킹정보가 들어있는 리스트를 Return하는게 존재하지 않네요
There was a problem hiding this comment.
한 번 해봤습니다! 근데
getLikeRankingInfoList
getViewRankingInfoList
이거에 대한 레디스에서 가져오는 로직을 잘 모르겠어서
public List getLikeRankingInfoList() {
return Collections.emptyList();
}
이런식으로만 일단 해봤습니다!
| String countsKey = rankingType + "_VIEW_COUNTS"; | ||
| redisTemplate.opsForHash().increment(countsKey, boardId, 1L); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("조회수 증가 실패"); |
| List<SearchVo> searchResults = searchService.searchByTag(tag); | ||
|
|
||
| if (searchResults.isEmpty()) { | ||
| throw new IllegalArgumentException("해당 태그가 없다"); |
|
|
||
| // 태그 기반으로 검색 | ||
| @GetMapping("/tag") | ||
| public ResponseEntity<List<SearchVo>> searchByTag(@RequestParam("tag") String tag) { |
There was a problem hiding this comment.
ResponseEntity로 제작하는건 Controller에서 하는게 좋아보이네요.
|
|
||
| public List<SearchVo> searchByTag(String tag) { | ||
| return searchRepository.findByTag(tag).stream() | ||
| .map(search -> new SearchVo(search.getTitle(), search.getDescription())) |
|
spotless 적용 안할 시 github action ci 통과못합니다... |
3 participants
좋아요, 조회수 별 랭킹 로직 작성
검색 api 작성
📝 PR Summary
🌲 Working Branch
🌲 TODOs
Related Issues