[BE] 김예찬 강화하기 #4

yechan-kim · 2024-05-29T11:01:15Z

미션 실행 결과

[테스트 실행 터미널 캡쳐 이미지 파일 첨부]

API 기능 명세서

POST /users/register - 회원가입
POST /users/login - 로그인
GET /users/check-duplicate-id - id 중복 확인
PATCH /items- 아이템 추가
GET /items - 내 아이템 조회
POST /enhance - 아이템 강화하기 및 결과 조회
GET /items/top10 - 상위 10개 아이템 조회

해결하지 못한 문제

인증을 원하는 부분의 API를 hasRole("USER")나 authenticated()을 통해 권한을 지정해 주려고 했으나, 403 오류가 반환되어 모든 API를 permitAll로 권한 없이 접근이 가능하도록 한 문제
요구사항에는 제한된 레벨이 없으나, 레벨이 제한되어 있는 문제

rootTiket

고생하셨습니다! Dto를 record로 구성하고 자바 스터디에서 공부하고 있는 스트림, 람다 문법을 적극적으로 활용해서 코드 가독성을 올리고 계신게 눈에 띕니다
코멘트를 추가하였으니 읽고 의견 부탁드려요!

rootTiket · 2024-05-29T11:06:22Z

src/main/java/leets/enhance/domain/blade/controller/BladeController.java

+    private final BladeService bladeService;
+
+    @PatchMapping("/items")
+    public ResponseEntity<BladeCreateResponse> createBlade(@RequestHeader("Authorization") String authorizationHeader, @RequestBody @Valid BladeCreateRequest bladeCreateRequest) {


Valid 어노테이션을 쓰시는 이유가 있을까요?

@Vaild를 사용해서 RequestDTO들의 유효성을 검사하기 위해서 입니다.

DTO들의 입력 유효성 검사를 백엔드 / 프론트 엔드 중 어느쪽에서 하는게 맞을지 같이 고민해보면 좋을 거 같아요

rootTiket · 2024-05-29T11:09:54Z

src/main/java/leets/enhance/domain/blade/service/BladeService.java

+    @Transactional
+    public BladeCreateResponse createBlade(String authorizationHeader, BladeCreateRequest bladeCreateRequest) {
+        String userEmail = jwtService.extractEmailFromToken(authorizationHeader);
+        User user = userRepository.findByUsername(userEmail).orElseThrow(IllegalStateException::new);


유저가 존재하지 않을 때 예외를 직접 구현하시면 더 좋을 것 같아요!

예외 객체를 만드는 것이 아직 익숙하지 않아서 학습하고 있습니다 추후에 리팩토링 해보겠습니다.

rootTiket · 2024-05-29T11:12:36Z

src/main/java/leets/enhance/domain/blade/service/BladeService.java

+        User user = userRepository.findByUsername(userEmail).orElseThrow(IllegalStateException::new);
+        Blade blade = bladeRepository.findByUser(user).orElseThrow(IllegalStateException::new);


위 코멘트와 비슷한 맥락이지만 같은 예외가 나올 경우 어디서 예외가 발생했는지 찾기 어려울 수 있습니다. 같은 종류의 에러라도 발생한 원인(유저를 찾지 못한다, 칼을 찾지 못한다) 같이 어느 부분에서 예외가 발생하는지 알 수 있게 하는 것이 관리면에서 더 편할 것 같습니다 :)

네! 감사합니다!

rootTiket · 2024-05-29T11:13:42Z

src/main/java/leets/enhance/domain/blade/service/BladeService.java

+        if (Math.random() <= blade.getLevel().getSuccessProbability() + increaseProbability) {
+            blade.updateLevel(BladeLevel.fromLevel(blade.getLevel().getLevel() + 1));
+            bladeRepository.save(blade);
+            return BladeEnhanceResponse.of(blade, "강화에 성공했습니다.");
+        }
+
+        if (Math.random() <= blade.getLevel().getDestroyProbability()) {
+            blade.updateLevel(BladeLevel.fromLevel(0));
+            bladeRepository.save(blade);
+            return BladeEnhanceResponse.of(blade, "검이 파괴되었습니다.");
+        }
+
+        blade.updateLevel(BladeLevel.fromLevel(blade.getLevel().getLevel() - 1));
+        bladeRepository.save(blade);
+        return BladeEnhanceResponse.of(blade, "강화에 실패했습니다.");
+    }


bladeRepository.save(blade) 코드가 중복되고 있는데 변경사항을 가장 나중에 저장하여 중복코드를 줄여보는 것은 어떨까요?

반환되는 객체가 다르기 때문에 저장하고 반환하는 로직만 생각해서 이 부분을 고려하지 못 했던 것 같습니다. 해당 사항은 수정하겠습니다!

rootTiket · 2024-05-29T11:16:23Z

src/main/java/leets/enhance/domain/blade/service/BladeService.java

+        List<Blade> top10Blade = bladeRepository.findTop10ByOrderByLevelDesc();
+        return top10Blade.stream()
+                .map(Top10BladeResponse::of)
+                .collect(Collectors.toList());


변수로 리스트를 가져와서 stream으로 만드는 것 보다

return bladeRepository.findTop10ByOrderByLevelDesc().stream() .map(Top10BladeResponse::of) .collect(Collectors.toList());

로 수정하는 것은 어떨까요? 불 필요한 선언이 줄어들 것 같습니다!

넵! 개선하겠습니다!

rootTiket · 2024-05-29T11:17:54Z

src/main/java/leets/enhance/domain/user/domain/repository/UserRepository.java

+public interface UserRepository extends JpaRepository<User, UUID> {
+    Optional<User> findByUsername(String email);
+
+    boolean existsByUsername(String email);


Raw 타입으로 가져오시는 이유가 있을까요? Boolean으로 바꾸시는 건 어떨까요?

Row Type으로 받아도 문제가 없어서 수정하지 않고, 그냥 지나친것 같습니다. 수정하겠습니다!

jwnnoh

고생하셨습니다!

jwnnoh · 2024-05-30T07:59:03Z

src/main/java/leets/enhance/domain/blade/controller/BladeController.java

+        return ResponseEntity.ok(bladeService.enhance(authorizationHeader, bladeEnhanceRequest));
+    }
+
+    @GetMapping("/items/top10")


컨트롤러단에 RequestMapping을 통해 /item으로 uri를 우선 구분해주고,
이후 /top10 방식으로 uri를 매핑해주면 더욱 좋을 것 같습니다.

@RequestMapping("/item") /* ... */ @GetMapping("/top10")

해당컨트롤러에는 /items와 /enhance가 같이 존재하는데 어떻게 처리하면 좋을까요?

jwnnoh · 2024-05-30T08:00:39Z

src/main/java/leets/enhance/domain/blade/domain/Blade.java

+@RequiredArgsConstructor
+@Builder
+@Entity
+@Table


엔티티하고 테이블 어노테이션은 중복된 기능을 가집니다.
둘 중 하나를 선택해서 제거하는 것을 추천드립니다!

감사합니다! @Entity로 통일하겠습니다!

jwnnoh · 2024-05-30T08:02:30Z

src/main/java/leets/enhance/domain/blade/domain/BladeLevel.java

+        return Arrays.stream(BladeLevel.values())
+                .filter(l -> l.getLevel().equals(level))
+                .findFirst()
+                .orElse(BladeLevel.LV12);


에러를 따로 명시해주지 않고 최대 레벨로 반환하는 이유가 있을까요?

0레벨에서 강화에 성공할 확률은 100%이기 때문에 0미만의 레벨은 나올 수가 없습니다.
하지만 13 이상의 레벨은 나올 수 있기 때문에 13레벨 이상이 나오면 최대 레벨로 반환하는 로직으로 구현했습니다.
원래는 최대 레벨 이후로는 확률이 동일해 확률은 동일하고 레벨만 상승하게 하려는 기능을 구현하려고 했으나, 아직까지 구현하지 못해 임시로 저렇게 조치를 해두었습니다. 추후에 기능이 구현된다면 해당 부분은 수정할 예정입니다!

hyxklee · 2024-05-30T08:24:39Z

src/main/java/leets/enhance/domain/blade/controller/BladeController.java

+    private final BladeService bladeService;
+
+    @PatchMapping("/items")
+    public ResponseEntity<BladeCreateResponse> createBlade(@RequestHeader("Authorization") String authorizationHeader, @RequestBody @Valid BladeCreateRequest bladeCreateRequest) {


DTO들의 입력 유효성 검사를 백엔드 / 프론트 엔드 중 어느쪽에서 하는게 맞을지 같이 고민해보면 좋을 거 같아요

hyxklee · 2024-05-30T08:27:07Z

src/main/java/leets/enhance/domain/blade/exception/TooLongBladeNameException.java

+import leets.enhance.gloal.error.ErrorCode;
+import leets.enhance.gloal.error.exception.ServiceException;
+
+public class TooLongBladeNameException extends ServiceException {


커스텀 예외를 추가로 정의하신 부분은 좋아요!

hyxklee · 2024-05-30T10:40:54Z

src/main/java/leets/enhance/domain/user/service/AuthService.java

+
+        // 2. 실제로 검증 (사용자 비밀번호 체크) 이 이루어지는 부분
+        Authentication authentication = authenticationManagerBuilder.getObject().authenticate(authenticationToken);
+


authenticationManager.authenticate(authenticationToken)로 조금 더 간단하게 구현할 수 있을 것 같은데 한 번 고려해보시는 건 어떨까요?

yechan-kim added 30 commits May 24, 2024 17:35

chore: 의존성 추가 및 db 설정 추가

e6937ac

feat: 사용자 도메인 생성

da12770

feat: 사용자 DTO 생성

795fb3f

feat: 사용자 repository 생성

93af31c

feat: DB에서 사용자를 검색하는 서비스 로직 구현

cf05f68

feat: 토큰 생성 기능 구현

bb5e06b

feat: JWT 인증 필터 추가

3b26a01

feat: 보안 설정 추가

d6ae0d3

feat: 로그인 및 회원 가입 기능 구현

12ebda9

feat: 검 도메인 생성

4377eb4

feat: 검 DTO 생성

2bfbb37

feat: 검 repository 생성

010ed0a

feat: 검 추가, 생성, 조회 기능 구현

3a7fd75

test: 테스트 코드 작성

f5365f9

fix: Authentication에서 동일한 사용자를 얻어오지 못하는 오류 수정

a01768c

feat: 토큰에서 사용자의 E-Mail을 추출하는 기능 구현

ec501b8

refactor: 기능 추가와 예외 처리 로직 변경에 따른 코드 정리

40b84f9

refactor: confirmPassword 로직 삭제 및 업그레이드 강화권 개수 추가

af09f60

refactor: 사용자와 검 매핑 방식 변경 및 사용자 검색 방식 변경

2ed4ecb

refactor: 사용자 탐색 로직 변경

d14a779

fix: 사용자와 검의 매핑 오류 수정

2bfc523

feat: 확률 증가권 로직 구현

aa4ee82

refactor: 권한 수정

db9cc88

test: 테스트 코드 수정

d72db38

fix: 확률 강화권의 증가 확률 수정

b34db93

feat: 확률 강화권 사용 로직 구현

7d0e6ef

refactor: http 메서드 방식 변경

18f2418

feat: 강회 결과 조회 DTO 추가

38e293d

refactor: 강화 결과 조회 로직 변경

b014335

fix: 강화권이 사용되지 않는 오류 수정

da8f249

test: 테스트 코드 수정

b1a0ca6

yechan-kim self-assigned this May 29, 2024

rootTiket reviewed May 29, 2024

View reviewed changes

yechan-kim added 5 commits May 29, 2024 20:51

feat: 에러 처리 기능 구현

dbccac8

refactor: 예외 처리 로직 변경

2045b2b

refactor: 검 강화 결과 저장 로직 변경

4d6b286

refactor: 코드 정리

ad53529

refactor: Row Type 코드 수정

9e8d539

yechan-kim changed the base branch from main to yechan-kim May 29, 2024 13:42

jwnnoh reviewed May 30, 2024

View reviewed changes

hyxklee reviewed May 30, 2024

View reviewed changes

yechan-kim added 2 commits May 30, 2024 17:51

refactor: 불필요한 어노테이션 제거

609e417

refactor: uri 매핑 방식 변경 및 코드 정리

bab077d

hyxklee reviewed May 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] 김예찬 강화하기 #4

[BE] 김예찬 강화하기 #4

yechan-kim commented May 29, 2024 •

edited

Loading

rootTiket left a comment

rootTiket May 29, 2024

yechan-kim May 29, 2024

hyxklee May 30, 2024

rootTiket May 29, 2024

yechan-kim May 29, 2024

rootTiket May 29, 2024

yechan-kim May 29, 2024

rootTiket May 29, 2024 •

edited by yechan-kim

Loading

yechan-kim May 29, 2024

rootTiket May 29, 2024

yechan-kim May 29, 2024

rootTiket May 29, 2024 •

edited by yechan-kim

Loading

yechan-kim May 29, 2024

jwnnoh left a comment

jwnnoh May 30, 2024

yechan-kim May 30, 2024 •

edited

Loading

jwnnoh May 30, 2024

yechan-kim May 30, 2024

jwnnoh May 30, 2024

yechan-kim May 30, 2024

hyxklee May 30, 2024

hyxklee May 30, 2024

hyxklee May 30, 2024

		User user = userRepository.findByUsername(userEmail).orElseThrow(IllegalStateException::new);
		Blade blade = bladeRepository.findByUser(user).orElseThrow(IllegalStateException::new);


		// 2. 실제로 검증 (사용자 비밀번호 체크) 이 이루어지는 부분
		Authentication authentication = authenticationManagerBuilder.getObject().authenticate(authenticationToken);

[BE] 김예찬 강화하기 #4

Are you sure you want to change the base?

[BE] 김예찬 강화하기 #4

Conversation

yechan-kim commented May 29, 2024 • edited Loading

미션 실행 결과

API 기능 명세서

해결하지 못한 문제

rootTiket left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rootTiket May 29, 2024 • edited by yechan-kim Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rootTiket May 29, 2024 • edited by yechan-kim Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwnnoh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yechan-kim May 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yechan-kim commented May 29, 2024 •

edited

Loading

rootTiket May 29, 2024 •

edited by yechan-kim

Loading

rootTiket May 29, 2024 •

edited by yechan-kim

Loading

yechan-kim May 30, 2024 •

edited

Loading