Skip to content
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 : redis 랭킹, 검색 구현 #31

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

shin2012649
Copy link

좋아요, 조회수 별 랭킹 로직 작성
검색 api 작성

📝 PR Summary

🌲 Working Branch

🌲 TODOs

Related Issues

좋아요, 조회수 별 랭킹 로직 작성
검색 api  작성
@shin2012649 shin2012649 linked an issue Sep 15, 2023 that may be closed by this pull request
3 tasks
@donsonioc2010 donsonioc2010 self-requested a review September 15, 2023 11:34
@@ -0,0 +1,52 @@
package search;
Copy link
Member

Choose a reason for hiding this comment

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

  • Entity의 경우에는 domain모듈에 넣어주세요.
  • Search기능의 경우 Article을 가져오는게 주된 기능이기 떄문에 Article에서 Entity에 대한 추가가 완료되면 작업이 가능할 것 같습니다.

Comment on lines 20 to 50
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;
}
Copy link
Member

Choose a reason for hiding this comment

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

GetterSetter 메소드는 필요없습니다.
Lombok@Getter, @Setter, @Data에 대해서 알아보고 사용해보세요.


import java.util.List;

@Repository
Copy link
Member

Choose a reason for hiding this comment

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

JPA를 활용하는 경우에는 @Repository어노테이션이 꼭 필요하지는 않습니다.

@@ -0,0 +1,16 @@
package search;
Copy link
Member

Choose a reason for hiding this comment

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

Repository의 경우에는 Domain에서 생성을 해야 합니다.

Comment on lines 9 to 15
public interface SearchRepository extends JpaRepository<SearchEntity, Long> {

// 태그를 기반으로 검색하기 위한 사용자 정의 메서드
List<SearchEntity> findByTag(String tag);

// 자동완성을 위한 사용자 정의 메서드
List<SearchEntity> findByTitleStartingWith(String query);
Copy link
Member

Choose a reason for hiding this comment

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

Tag또는 Title에서 기능을 다 가져올 수 있을 것 같습니다.

Comment on lines 8 to 10
public SearchVo() {
// 기본 생성자
}
Copy link
Member

Choose a reason for hiding this comment

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

@NoArgsConstructor

@@ -0,0 +1,40 @@
package search;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
package search;
package playlist.server.search.vo;
package playlist.server.search.dto.vo;

Comment on lines 12 to 15
public SearchVo(String title, String description) {
this.title = title;
this.description = description;
}
Copy link
Member

Choose a reason for hiding this comment

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

@AllArgsConstructor

Comment on lines 33 to 40
@Override
public String toString() {
return "SearchVo{" +
"title='" + title + '\'' +
", description='" + description + '\'' +
'}';
}
}
Copy link
Member

Choose a reason for hiding this comment

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

@ToString

Comment on lines 17 to 30
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;
Copy link
Member

Choose a reason for hiding this comment

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

@Getter, @Setter

Copy link
Member

@donsonioc2010 donsonioc2010 left a comment

Choose a reason for hiding this comment

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

Lambda나 어노테이션의 활용이 높아진거 매우 보기좋네요,
아직 소스코드 수정이 좀더 필요할 것 같아서 먼저 수정이 필요해 보이는 부분에 대해서만 코멘트를 달았습니다.

수정 끝나면 기능 바로바로 같이 풀어보도록 합시다.

진짜 잘하고 있어용 👍

@@ -0,0 +1,20 @@
package playlist.server.ranking;
Copy link
Member

Choose a reason for hiding this comment

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

search의 경우 vo패키지에 클래스를 정리한거 같아 보이는데, 해당 패이지는 그냥 ranking패키지 root경로에 vo를 생성한 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

\MainPageRankingInfo 이게 너무 네이밍이 구린거 같아 고민하다 놓친거 같습니다! 패키지 만들어서 vo 에 넣어뒀습니다!

Comment on lines 8 to 19
@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();
}
Copy link
Member

Choose a reason for hiding this comment

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

빌더의 활용 정말 잘했습니당.

주의할점을 말해주자면, Annotation이 편하긴 한데 완벽은 없더라구요,
추후에는 @Builder의 작동원리, 커스텀하는 방법에 대해서도 공부해보면 좋을 것 같아요.

  • 저도 최근에 이 어노테이션에 한번 발등을 찍혀봤거든요.ㅋㅋㅋㅋ

Copy link
Author

Choose a reason for hiding this comment

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

확인했습니다!! 감사해요 증말!! 하트 뿅뿅

@Operation(summary = "일간 랭킹을 조회합니다.")
@GetMapping("/daily")
public ResponseEntity<MainPageRankingInfoVo> getDailyRanking(
@RequestParam(name = "rankingType", required = false, defaultValue = "DAILY") RankingType rankingType) {
Copy link
Member

Choose a reason for hiding this comment

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

메소드를 daily, week, month로 분류를 하였기 때문에, 해당 파라미터가 꼭 필요한가?라는 생각이 드네요.

Copy link
Author

Choose a reason for hiding this comment

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

오호라!!! 수정했습니다

private final RankingViewService rankingViewService;

@Operation(summary = "일간 랭킹을 조회합니다.")
@GetMapping("/daily")
Copy link
Member

Choose a reason for hiding this comment

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

Mapping을 차라리

  • /daily?type=view 또는 /daily?type=like
  • /daily/{type} 같은 방식으로 받은 이후 PathParameter로 받은 type을 view또는 like로 구분해서 로직을 태우는게 맞지 않을 까 합니다.

Copy link
Author

Choose a reason for hiding this comment

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

type 매개변수 사용해서 type값에 따라 좋아요랑 조회수 랭킹 정보 가져오는 로직 만들어 봤습니다!

@RestController
@RequestMapping("/ranking")
@RequiredArgsConstructor
@Tag(name = "1. [랭킹]")
Copy link
Member

Choose a reason for hiding this comment

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

태그로 어울리는 정보를 전달하는거 매우 좋아보입니다 ^^

Comment on lines 9 to 13
@NoArgsConstructor
@AllArgsConstructor
@ToString
@Getter
@Setter
Copy link
Member

Choose a reason for hiding this comment

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

@Data라는 Annotation도 있으니 활용해보면 좋을 것 같습니다 👍

Copy link
Author

Choose a reason for hiding this comment

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

활용 완료!

Comment on lines 13 to 22
@Entity
@Getter
@Table(name = "tbl_ranking")
@NoArgsConstructor
public class Ranking {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Enumerated(EnumType.STRING)
private RankingType rankingType;
Copy link
Member

Choose a reason for hiding this comment

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

생각해보면, 일단 Redis로 구현을 하고 있는데, 굳이 Ranking Entity가 필요한가 의문이네요

Copy link
Author

Choose a reason for hiding this comment

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

바로 지워버리겠습니다.
Redis와 같은 NoSQL 데이터베이스를 사용할 때에는 보통 관계형 데이터베이스와 달리 엔티티를 정의하고 관계를 매핑하는 작업이 필요하지 않다는 것을 알았군요!

Comment on lines 11 to 22
RankingInfo(String countsKey, String rankingKey) {
this.countsKey = countsKey;
this.rankingKey = rankingKey;
}

public String getCountsKey() {
return countsKey;
}

public String getRankingKey() {
return rankingKey;
}
Copy link
Member

Choose a reason for hiding this comment

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

@AllArgsConstructors, @Getter, @Setter

Copy link
Author

Choose a reason for hiding this comment

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

수정했습니다!

Comment on lines 8 to 9
private final String countsKey;
private final String rankingKey;
Copy link
Member

Choose a reason for hiding this comment

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

변수명 직관적인거 보기 좋네요

Comment on lines +11 to +23
@Entity
@Getter
@Setter
@NoArgsConstructor
public class Search {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

private String title;
private String description;
private String tag;
Copy link
Member

Choose a reason for hiding this comment

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

Search의 경우에는 다른 팀이 직접 구현한 엔티티를 활용해야 하다보니, Search Entity가 필요한 이유가 납득이 가지 않네용.

왜 생성한 것인지 의도를 풀어줄 수 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

다른 팀이 직접 구현한 엔티티를 활용해야 하다보니 사실 이걸 생각한다는 걸 생각하긴 했는데 , 한 번 Search 관련 Api를 쭉 만들어보고 싶어서 해봤습니다!

Copy link
Member

@donsonioc2010 donsonioc2010 left a comment

Choose a reason for hiding this comment

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

  • 많이 좋아졌어오
  • 잊고있던게, 전체적으로 log를 활용해서 로직 실행에 따른 기록을 추가하면 좋을 것 같습니다.
  • Redis도 Entity같은게 있을것 같아 알아보니 RedisHash라는게 있었네요. 읽어보고 구현해보면 좋을 것 같아요.

Comment on lines 43 to 51
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();
}
Copy link
Member

Choose a reason for hiding this comment

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

다른메소드 동일하지만 service레벨에서 Exception을 내주고 있는것 같아서 badRequest Return의 경우에는 발생시키는 이유를 모르겠네요.

코드를 이런식으로 만들어 볼 수 있지않을까요?

Suggested change
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);

Copy link
Member

Choose a reason for hiding this comment

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

두번째 인자값인 type의 경우에도 enum으로 정의가 가능할 것 같아요.
세번째 인자값인 boardId는 차라리 어딘가에 public static String으로 변수선언한 이후에 해당 값을 모두 공유해서 사용하면
추후 변경이 발생시 편하게 공유할 수 있지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

boardId 이거는 못했습니다..두번째 인자값인 type의 경우에도 enum으로 정의가 가능할 것 같아요. 이건 했습니다!

String countsKey = rankingType + "_LIKE_COUNTS";
redisTemplate.opsForHash().increment(countsKey, boardId, 1L);
} catch (Exception e) {
throw new RuntimeException("좋아요 증가 실패");
Copy link
Member

Choose a reason for hiding this comment

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

RuntimeException의 사용은 우리 프로젝트에서 사용을 하면 안됩니다.
Handling은 모두 BaseException을 활용하고 있기 때문에, BaseException을 상속받아 Exception을 Custom하게 제작하여 사용해 주시기 바랍니다 :)

그래도 Exception을 선언해서 안전한 개발방식을 만든건 좋네요 👍

Copy link
Author

Choose a reason for hiding this comment

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

LikeIncrementException 완료!

Comment on lines 18 to 27
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("유효하지 않은 랭킹 타입");
}
Copy link
Member

Choose a reason for hiding this comment

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

Controller레벨에서 rankingType을 문자열로 주지않고, Enum타입을 주면 해당 코드도 필요 없지 않을까요?

그래도 서비스레이어에서 모든 로직처리하는건 괜찮아 보이네요.


return MainPageRankingInfoVo.from(rankingInfo, RankingType.valueOf(rankingType.toUpperCase()));
} catch (Exception e) {
throw new RuntimeException("데이터 가져오는거 실패");
Copy link
Member

Choose a reason for hiding this comment

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

Custom하게 제작한 Exception을 활용해주세요 👍

Copy link
Author

Choose a reason for hiding this comment

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

DataFetchException, InvalidParameterException 완료!

Comment on lines 29 to 32
if ("like".equals(type)) {
rankingLikeService.incrementLikes(rankingType, boardId);
} else if ("view".equals(type)) {
rankingViewService.incrementViews(rankingType, boardId);
Copy link
Member

Choose a reason for hiding this comment

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

like와 view도 enum으로 정의해보는게 좋을것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

완료!

throw new IllegalArgumentException("유효하지 않은 파라미터");
}

return MainPageRankingInfoVo.from(rankingInfo, RankingType.valueOf(rankingType.toUpperCase()));
Copy link
Member

Choose a reason for hiding this comment

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

혹시 Enum만 들어있는 정보를 Return하는 이유가 있을까요?
생각해보니 랭킹정보가 들어있는 리스트를 Return하는게 존재하지 않네요

Copy link
Author

Choose a reason for hiding this comment

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

한 번 해봤습니다! 근데

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("조회수 증가 실패");
Copy link
Member

Choose a reason for hiding this comment

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

Exception!

Copy link
Author

Choose a reason for hiding this comment

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

ViewIncrementException 완료!

List<SearchVo> searchResults = searchService.searchByTag(tag);

if (searchResults.isEmpty()) {
throw new IllegalArgumentException("해당 태그가 없다");
Copy link
Member

Choose a reason for hiding this comment

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

Exception!

Copy link
Author

Choose a reason for hiding this comment

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

TagNotFoundException 했습니다!


// 태그 기반으로 검색
@GetMapping("/tag")
public ResponseEntity<List<SearchVo>> searchByTag(@RequestParam("tag") String tag) {
Copy link
Member

Choose a reason for hiding this comment

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

ResponseEntity로 제작하는건 Controller에서 하는게 좋아보이네요.


public List<SearchVo> searchByTag(String tag) {
return searchRepository.findByTag(tag).stream()
.map(search -> new SearchVo(search.getTitle(), search.getDescription()))
Copy link
Member

Choose a reason for hiding this comment

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

💯

@bongsh0112
Copy link
Contributor

spotless 적용 안할 시 github action ci 통과못합니다...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] 조회수 랭킹 데이터 기능 구현
3 participants