-
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 : redis 랭킹, 검색 구현 #31
base: dev
Are you sure you want to change the base?
Conversation
좋아요, 조회수 별 랭킹 로직 작성 검색 api 작성
@@ -0,0 +1,52 @@ | |||
package search; |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getter
와 Setter
메소드는 필요없습니다.
Lombok
의 @Getter
, @Setter
, @Data
에 대해서 알아보고 사용해보세요.
|
||
import java.util.List; | ||
|
||
@Repository |
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.
JPA
를 활용하는 경우에는 @Repository
어노테이션이 꼭 필요하지는 않습니다.
@@ -0,0 +1,16 @@ | |||
package search; |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tag또는 Title에서 기능을 다 가져올 수 있을 것 같습니다.
public SearchVo() { | ||
// 기본 생성자 | ||
} |
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.
@NoArgsConstructor
@@ -0,0 +1,40 @@ | |||
package search; |
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.
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; | ||
} |
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.
@AllArgsConstructor
@Override | ||
public String toString() { | ||
return "SearchVo{" + | ||
"title='" + title + '\'' + | ||
", description='" + description + '\'' + | ||
'}'; | ||
} | ||
} |
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.
@ToString
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; |
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.
@Getter
, @Setter
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.
Lambda나 어노테이션의 활용이 높아진거 매우 보기좋네요,
아직 소스코드 수정이 좀더 필요할 것 같아서 먼저 수정이 필요해 보이는 부분에 대해서만 코멘트를 달았습니다.
수정 끝나면 기능 바로바로 같이 풀어보도록 합시다.
진짜 잘하고 있어용 👍
@@ -0,0 +1,20 @@ | |||
package playlist.server.ranking; |
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.
search의 경우 vo
패키지에 클래스를 정리한거 같아 보이는데, 해당 패이지는 그냥 ranking패키지 root경로에 vo를 생성한 이유가 있을까요?
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.
\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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빌더의 활용 정말 잘했습니당.
주의할점을 말해주자면, Annotation이 편하긴 한데 완벽은 없더라구요,
추후에는 @Builder
의 작동원리, 커스텀하는 방법에 대해서도 공부해보면 좋을 것 같아요.
- 저도 최근에 이 어노테이션에 한번 발등을 찍혀봤거든요.ㅋㅋㅋㅋ
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.
확인했습니다!! 감사해요 증말!! 하트 뿅뿅
@Operation(summary = "일간 랭킹을 조회합니다.") | ||
@GetMapping("/daily") | ||
public ResponseEntity<MainPageRankingInfoVo> getDailyRanking( | ||
@RequestParam(name = "rankingType", required = false, defaultValue = "DAILY") RankingType rankingType) { |
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.
메소드를 daily, week, month로 분류를 하였기 때문에, 해당 파라미터가 꼭 필요한가?
라는 생각이 드네요.
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.
오호라!!! 수정했습니다
private final RankingViewService rankingViewService; | ||
|
||
@Operation(summary = "일간 랭킹을 조회합니다.") | ||
@GetMapping("/daily") |
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.
Mapping을 차라리
/daily?type=view
또는/daily?type=like
/daily/{type}
같은 방식으로 받은 이후 PathParameter로 받은 type을 view또는 like로 구분해서 로직을 태우는게 맞지 않을 까 합니다.
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.
type 매개변수 사용해서 type값에 따라 좋아요랑 조회수 랭킹 정보 가져오는 로직 만들어 봤습니다!
@RestController | ||
@RequestMapping("/ranking") | ||
@RequiredArgsConstructor | ||
@Tag(name = "1. [랭킹]") |
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.
태그로 어울리는 정보를 전달하는거 매우 좋아보입니다 ^^
@NoArgsConstructor | ||
@AllArgsConstructor | ||
@ToString | ||
@Getter | ||
@Setter |
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.
@Data
라는 Annotation도 있으니 활용해보면 좋을 것 같습니다 👍
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.
활용 완료!
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각해보면, 일단 Redis로 구현을 하고 있는데, 굳이 Ranking Entity가 필요한가 의문이네요
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.
바로 지워버리겠습니다.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AllArgsConstructors
, @Getter
, @Setter
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.
수정했습니다!
private final String countsKey; | ||
private final String rankingKey; |
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.
변수명 직관적인거 보기 좋네요
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Search의 경우에는 다른 팀이 직접 구현한 엔티티를 활용해야 하다보니, Search Entity가 필요한 이유가 납득이 가지 않네용.
왜 생성한 것인지 의도를 풀어줄 수 있을까요?
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.
다른 팀이 직접 구현한 엔티티를 활용해야 하다보니 사실 이걸 생각한다는 걸 생각하긴 했는데 , 한 번 Search 관련 Api를 쭉 만들어보고 싶어서 해봤습니다!
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.
- 많이 좋아졌어오
- 잊고있던게, 전체적으로 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른메소드 동일하지만 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.
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
으로 변수선언한 이후에 해당 값을 모두 공유해서 사용하면
추후 변경이 발생시 편하게 공유할 수 있지 않을까요?
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.
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.
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을 선언해서 안전한 개발방식을 만든건 좋네요 👍
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.
LikeIncrementException 완료!
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.
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("데이터 가져오는거 실패"); |
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.
Custom하게 제작한 Exception을 활용해주세요 👍
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like와 view도 enum으로 정의해보는게 좋을것 같아요
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.
완료!
throw new IllegalArgumentException("유효하지 않은 파라미터"); | ||
} | ||
|
||
return MainPageRankingInfoVo.from(rankingInfo, RankingType.valueOf(rankingType.toUpperCase())); |
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.
혹시 Enum만 들어있는 정보를 Return하는 이유가 있을까요?
생각해보니 랭킹정보가 들어있는 리스트를 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.
한 번 해봤습니다! 근데
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("조회수 증가 실패"); |
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.
Exception!
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.
ViewIncrementException 완료!
List<SearchVo> searchResults = searchService.searchByTag(tag); | ||
|
||
if (searchResults.isEmpty()) { | ||
throw new IllegalArgumentException("해당 태그가 없다"); |
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.
Exception!
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.
TagNotFoundException 했습니다!
|
||
// 태그 기반으로 검색 | ||
@GetMapping("/tag") | ||
public ResponseEntity<List<SearchVo>> searchByTag(@RequestParam("tag") String tag) { |
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.
ResponseEntity로 제작하는건 Controller에서 하는게 좋아보이네요.
|
||
public List<SearchVo> searchByTag(String tag) { | ||
return searchRepository.findByTag(tag).stream() | ||
.map(search -> new SearchVo(search.getTitle(), search.getDescription())) |
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.
💯
spotless 적용 안할 시 github action ci 통과못합니다... |
좋아요, 조회수 별 랭킹 로직 작성
검색 api 작성
📝 PR Summary
🌲 Working Branch
🌲 TODOs
Related Issues