Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public enum ErrorStatus implements BaseErrorCode {
ITEM_NOT_EQUIPPED(HttpStatus.BAD_REQUEST,"ITEM_5004","아이템을 착용하고 있지 않습니다."),
ITEM_ALREADY_EQUIPPED(HttpStatus.BAD_REQUEST,"ITEM_5005","동일한 아이템을 이미 착용하고 있습니다."),
ITEM_NOT_BUY(HttpStatus.BAD_REQUEST,"ITEM_5006","구매한 아이템이 아닙니다."),
ITEM_BODYPART_QUERY_STRING_INVALID(HttpStatus.BAD_REQUEST,"ITEM_5007","bodyPart 쿼리 스트링이 올바르지 않습니다."),

// 챌린지 관련 응답 6000
CHALLENGE_FULL(HttpStatus.BAD_REQUEST,"CHANLLENGE_6001","챌린지 3개가 이미 생성되어 있어 새로운 챌린지를 생성할 수 없습니다."),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package TtattaBackend.ttatta.domain.enums;

public enum BodyPart {
HEAD, TORSO
HEAD, EYES, TORSO
}
15 changes: 15 additions & 0 deletions src/main/java/TtattaBackend/ttatta/repository/ItemRepository.java
Copy link
Member

Choose a reason for hiding this comment

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

3개의 쿼리 형태가 동일하고 메소드명만 다른데 굳이 이렇게 구분한 이유가 있나요?

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,21 @@

@Repository
public interface ItemRepository extends JpaRepository<Items, Long> {
@Query("SELECT i FROM Items i " +
"WHERE i.bodyPart = :bodyPart " +
"AND NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItemByBodyPartHead(@Param("user") Users user, @Param("bodyPart") Optional<BodyPart> bodyPart);

@Query("SELECT i FROM Items i " +
"WHERE i.bodyPart = :bodyPart " +
"AND NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItemByBodyPartEyes(@Param("user") Users user, @Param("bodyPart") Optional<BodyPart> bodyPart);

@Query("SELECT i FROM Items i " +
"WHERE i.bodyPart = :bodyPart " +
"AND NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItemByBodyPartTorso(@Param("user") Users user, @Param("bodyPart") Optional<BodyPart> bodyPart);
Comment on lines +17 to +30

Choose a reason for hiding this comment

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

high

거의 동일한 쿼리를 사용하는 getShopItemByBodyPartHead, getShopItemByBodyPartEyes, getShopItemByBodyPartTorso 세 개의 메서드가 추가되었습니다. 이는 코드 중복이며 유지보수를 어렵게 만듭니다. bodyPart를 파라미터로 받는 단일 메서드로 통합하는 것이 좋습니다. 또한, 리포지토리 메서드의 파라미터로 Optional을 직접 넘기기보다, 서비스 레이어에서 Optional의 존재 여부를 확인하고 실제 값을 전달하는 것이 더 일반적이고 안전한 방법입니다.

Suggested change
@Query("SELECT i FROM Items i " +
"WHERE i.bodyPart = :bodyPart " +
"AND NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItemByBodyPartHead(@Param("user") Users user, @Param("bodyPart") Optional<BodyPart> bodyPart);
@Query("SELECT i FROM Items i " +
"WHERE i.bodyPart = :bodyPart " +
"AND NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItemByBodyPartEyes(@Param("user") Users user, @Param("bodyPart") Optional<BodyPart> bodyPart);
@Query("SELECT i FROM Items i " +
"WHERE i.bodyPart = :bodyPart " +
"AND NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItemByBodyPartTorso(@Param("user") Users user, @Param("bodyPart") Optional<BodyPart> bodyPart);
@Query("SELECT i FROM Items i " +
"WHERE i.bodyPart = :bodyPart " +
"AND NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItemByBodyPart(@Param("user") Users user, @Param("bodyPart") BodyPart bodyPart);


@Query("SELECT i FROM Items i " +
"WHERE NOT EXISTS (SELECT o FROM OwnedItems o WHERE o.users = :user AND o.items.id = i.id)")
List<Items> getShopItem(@Param("user") Users user);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package TtattaBackend.ttatta.service.ItemService;

import TtattaBackend.ttatta.domain.Items;
import TtattaBackend.ttatta.domain.enums.BodyPart;
import TtattaBackend.ttatta.domain.mapping.OwnedItems;

import java.util.List;
import java.util.Optional;

public interface ItemQueryService {
List<Items> getShopItem();
List<Items> getShopItem(Optional<BodyPart> bodyPart);
List<OwnedItems> getMyItem();
List<OwnedItems> getEquippedItem();

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package TtattaBackend.ttatta.service.ItemService;

import TtattaBackend.ttatta.apiPayload.code.status.ErrorStatus;
import TtattaBackend.ttatta.apiPayload.exception.handler.ExceptionHandler;
import TtattaBackend.ttatta.config.security.SecurityUtil;
import TtattaBackend.ttatta.domain.Items;
import TtattaBackend.ttatta.domain.Users;
import TtattaBackend.ttatta.domain.enums.BodyPart;
import TtattaBackend.ttatta.domain.mapping.OwnedItems;
import TtattaBackend.ttatta.repository.ItemRepository;
import TtattaBackend.ttatta.repository.OwnedItemRepository;
Expand All @@ -13,27 +15,40 @@
import org.springframework.stereotype.Service;

import java.util.List;
import java.util.Optional;

import static TtattaBackend.ttatta.apiPayload.code.status.ErrorStatus.USER_NOT_FOUND;
import static TtattaBackend.ttatta.domain.enums.BodyPart.*;

@Slf4j
@Service
@RequiredArgsConstructor
public class ItemQueryServiceImpl implements ItemQueryService{

private final UserRepository userRepository;

private final ItemRepository itemRepository;

private final OwnedItemRepository ownedItemRepository;

@Override
public List<Items> getShopItem() {
public List<Items> getShopItem(Optional<BodyPart> bodyPart) {
Long userId = SecurityUtil.getCurrentUserId();
Users user = userRepository.findById(userId)
.orElseThrow(() -> new ExceptionHandler(USER_NOT_FOUND));

return itemRepository.getShopItem(user);
// bodyPart에 따른 아이템 조회
if (bodyPart.isPresent()) {
if (bodyPart.get() == HEAD)
return itemRepository.getShopItemByBodyPartHead(user, bodyPart);
else if (bodyPart.get() == EYES)
return itemRepository.getShopItemByBodyPartEyes(user, bodyPart);
else if (bodyPart.get() == TORSO)
return itemRepository.getShopItemByBodyPartTorso(user, bodyPart);
else if (bodyPart.get() == null)
return itemRepository.getShopItem(user);
else
throw new ExceptionHandler(ErrorStatus.ITEM_BODYPART_QUERY_STRING_INVALID);
Copy link
Member

Choose a reason for hiding this comment

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

방어적인 코드인건가요?
어차피 bodypart.get으로 구분이 된다면 그냥 getShopItemByBodyPart로 통일해서 거기다 파라미터로 넣으면 알아서 쿼리에서 구분해서 넣는 것 아닌가 궁금합니다.

} else
return itemRepository.getShopItem(user);
}
Comment on lines +38 to 52

Choose a reason for hiding this comment

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

high

ItemRepository의 메서드를 하나로 통합하는 변경에 맞춰 서비스 로직을 간소화할 수 있습니다. 현재의 if-else if 구조는 복잡하며, 새로운 BodyPart enum 값이 추가될 때마다 코드를 수정해야 하는 단점이 있습니다. 특히 else if (bodyPart.get() == null) 조건은 bodyPart.isPresent()가 참인 블록 내에 있어 절대 실행될 수 없는 코드(dead code)입니다. bodyPart가 존재할 경우, 제안된 단일 리포지토리 메서드를 호출하고, 그렇지 않은 경우 전체 미소유 아이템을 조회하도록 로직을 단순화하는 것을 권장합니다.

        // bodyPart에 따른 아이템 조회
        if (bodyPart.isPresent()) {
            return itemRepository.getShopItemByBodyPart(user, bodyPart.get());
        } else {
            return itemRepository.getShopItem(user);
        }
    }


@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@
import TtattaBackend.ttatta.domain.Users;
import TtattaBackend.ttatta.domain.UsersWithdrawals;
import TtattaBackend.ttatta.domain.enums.*;
import TtattaBackend.ttatta.domain.mapping.OwnedItems;
import TtattaBackend.ttatta.jwt.JwtUtils;
import TtattaBackend.ttatta.oidc.*;
import TtattaBackend.ttatta.repository.DiaryCategoryRepository;
import TtattaBackend.ttatta.repository.DiaryRepository;
import TtattaBackend.ttatta.repository.UserRepository;
import TtattaBackend.ttatta.repository.UserWithdrawalRepository;
import TtattaBackend.ttatta.repository.*;
import TtattaBackend.ttatta.web.dto.DiaryCategoryRequestDTO;
import TtattaBackend.ttatta.web.dto.UserRequestDTO;
import TtattaBackend.ttatta.web.dto.UserResponseDTO;
Expand All @@ -40,9 +38,7 @@
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.temporal.ChronoUnit;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import java.util.*;
import java.util.concurrent.TimeUnit;

import static TtattaBackend.ttatta.apiPayload.code.status.ErrorStatus.*;
Expand All @@ -52,19 +48,14 @@
@Slf4j
public class UserCommandServiceImpl implements UserCommandService {

private final OauthOIDCHelper oauthOIDCHelper;
private final JwtAuthenticationFilter jwtAuthenticationFilter;
@Value("${jwt.ACCESS_EXP_TIME}")
private int accessExpTime;
@Value("${jwt.REFRESH_EXP_TIME}")
private int refreshExpTime;

@Value("${oidc.iss}")
private String iss;

@Value("${oidc.aud}")
private String aud;

private final UserRepository userRepository;
private final DiaryRepository diaryRepository;
private final DiaryCategoryRepository diaryCategoryRepository;
Expand All @@ -79,6 +70,10 @@ public class UserCommandServiceImpl implements UserCommandService {
private final KakaoOauthHelper kakaoOauthHelper;
private final JwtOIDCProvider jwtOIDCProvider;
private final AmazonS3Manager s3Manager;
private final OauthOIDCHelper oauthOIDCHelper;
private final JwtAuthenticationFilter jwtAuthenticationFilter;
private final ItemRepository itemRepository;
private final OwnedItemRepository ownedItemRepository;

@Override
public Users createTestUser() {
Expand All @@ -101,19 +96,40 @@ public Users createTestUser() {
@Override
@Transactional // ???
public Users signUp(UserRequestDTO.SignUpRequestDTO request) {
// 아이디 중복 확인
Users newUser = UserConverter.toUsers(request);
IsAvailable usernameAvailable = verifyUsernameOverlap(request.getUsername());

// 아이디 중복 확인
if (usernameAvailable.equals(IsAvailable.UNAVAILABLE)) {
throw new ExceptionHandler(ErrorStatus.USERNAME_ALREADY_EXIST);
}

Users newUser = UserConverter.toUsers(request);
newUser.encodePassword(passwordEncoder.encode(request.getPassword()));
// 일상 카테고리 생성
createDefaultCategory(newUser);
// 기본 아이템 착용
saveAndEquipDefaultOwnedItems(newUser);

return userRepository.save(newUser);
}

private void saveAndEquipDefaultOwnedItems(Users newUser) {
List<OwnedItems> ownedItemsList = new ArrayList<>();

ownedItemsList.add(createDefaultOwnedItems(newUser, 1L));
ownedItemsList.add(createDefaultOwnedItems(newUser, 4L));
ownedItemsList.add(createDefaultOwnedItems(newUser, 7L));
ownedItemsList.add(createDefaultOwnedItems(newUser, 9L));
Comment on lines +118 to +121

Choose a reason for hiding this comment

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

medium

기본 아이템 ID 1L, 4L, 7L, 9L이 하드코딩되어 있습니다. 이러한 '매직 넘버'는 코드의 의도를 파악하기 어렵게 만들고, 향후 ID가 변경될 경우 수정이 번거로워질 수 있습니다. private static final Long DEFAULT_HEAD_ITEM_ID = 1L;과 같이 의미 있는 이름의 상수로 정의하여 가독성과 유지보수성을 높이는 것을 권장합니다.

ownedItemRepository.saveAll(ownedItemsList);
}

private OwnedItems createDefaultOwnedItems(Users newUser, Long itemId) {
return OwnedItems.builder()
.users(newUser)
.items(itemRepository.findById(itemId).get()) // 머리

Choose a reason for hiding this comment

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

high

Optional.get()을 직접 호출하면 itemRepository.findById(itemId)가 비어있는 Optional을 반환할 때 NoSuchElementException이 발생하여 회원가입 전체가 실패할 수 있습니다. orElseThrow()를 사용하여 아이템이 없을 경우 ITEM_NOT_FOUND와 같은 명확한 예외를 발생시키는 것이 더 안전한 방법입니다. 또한, // 머리라는 주석은 첫 번째 아이템에만 해당되어 다른 아이템 ID에 대한 정보를 주지 못해 오해의 소지가 있습니다.

Suggested change
.items(itemRepository.findById(itemId).get()) // 머리
.items(itemRepository.findById(itemId).orElseThrow(() -> new ExceptionHandler(ITEM_NOT_FOUND)))

.isEquipped(true)
.build();
}

@Override
@Transactional
public UserResponseDTO.IsPendingResultDTO checkIsPending() {
Expand Down Expand Up @@ -182,18 +198,20 @@ public UserResponseDTO.KaKaoFinalSignUpResultDTO kakaoSignUp(UserRequestDTO.Sign
Long userId = SecurityUtil.getCurrentUserId();
Users savedUser = userRepository.findById(userId)
.orElseThrow(() -> new ExceptionHandler(ErrorStatus.USER_NOT_FOUND));
// 액세스 토큰 및 리프레시 토큰 생성
String key = "users:" + savedUser.getId().toString();
String accessToken = generateAccessToken(savedUser.getId(), accessExpTime);
String refreshToken = generateAndSaveRefreshToken(key, refreshExpTime);

// 해당 닉네임을 업데이트
savedUser.updateNickname(request.getNickname());
// 유저의 상태 pending -> activate 업데이트
savedUser.updateStatus(UserStatus.ACTIVE);
// 일상 카테고리 생성
createDefaultCategory(savedUser);
// 기본 아이템 착용
saveAndEquipDefaultOwnedItems(savedUser);

// 액세스 토큰 및 리프레시 토큰 생성
String key = "users:" + savedUser.getId().toString();
String accessToken = generateAccessToken(savedUser.getId(), accessExpTime);
String refreshToken = generateAndSaveRefreshToken(key, refreshExpTime);
return UserConverter.toUserKaKaoFinalSignUpResultDTO(accessToken, refreshToken, savedUser);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import TtattaBackend.ttatta.apiPayload.ApiResponse;
import TtattaBackend.ttatta.converter.ItemConverter;
import TtattaBackend.ttatta.domain.Items;
import TtattaBackend.ttatta.domain.enums.BodyPart;
import TtattaBackend.ttatta.domain.mapping.OwnedItems;
import TtattaBackend.ttatta.service.ItemService.ItemCommandService;
import TtattaBackend.ttatta.service.ItemService.ItemQueryService;
Expand All @@ -13,6 +14,7 @@
import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.*;
import java.util.List;
import java.util.Optional;

@RestController
@RequiredArgsConstructor
Expand Down Expand Up @@ -62,8 +64,10 @@ public ApiResponse<ItemResponseDTO.ItemDisrobeResultDTO> disrobe (@PathVariable
@Operation(summary = "미소유 아이템 (shop) api",
description = "shop 화면에서 사용자가 구매하지 않은 아이템을 반환하는 API 입니다.")
@GetMapping("/shop")
public ApiResponse<ItemResponseDTO.ItemShopListDTO> shop () {
List<Items> itemsList = itemQueryService.getShopItem();
public ApiResponse<ItemResponseDTO.ItemShopListDTO> shop (
@RequestParam(required = false) Optional<BodyPart> bodyPart
) {
List<Items> itemsList = itemQueryService.getShopItem(bodyPart);
Long point = userCommandService.getUserPoint();

return ApiResponse.onSuccess(
Expand Down