Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
Walkthrough반려견 DBTI 제출 흐름에 사용자 식별자(userId)를 도입하고, 반려견 DBTI 조회 GET /pets/dbti/{petId} 엔드포인트를 추가했으며, Pet/Breed/Dbti 엔티티와 관련 리포지토리·서비스·DTO를 관계 기반으로 리팩토링했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as DbtiController
participant Facade as DbtiQueryFacade
participant Service as DbtiQueryService
participant PetRepo as PetRepository
participant ResultRepo as DbtiResultRepository
participant DbtiRepo as DbtiRepository
Client->>Controller: GET /pets/dbti/{petId}
activate Controller
Controller->>Facade: getPetDbtiResult(userId, petId)
activate Facade
Facade->>Service: getPetDbtiResultDetail(userId, petId)
activate Service
Service->>PetRepo: findById(petId)
PetRepo-->>Service: Optional<PetEntity>
alt Pet not found
Service-->>Facade: throw PetBusinessException(PET_NOT_FOUND)
Facade-->>Controller: 404
Controller-->>Client: ApiResponse(404)
else Pet found
Service->>Service: verify pet.userId == userId
alt user mismatch
Service-->>Facade: throw PetBusinessException(PET_ACCESS_DENIED)
Facade-->>Controller: 403
Controller-->>Client: ApiResponse(403)
else user matches
Service->>ResultRepo: findByPet_PetId(petId)
ResultRepo-->>Service: Optional<DbtiResultEntity>
alt result not found
Service-->>Facade: throw UserBusinessException(USER_PET_NOT_REGISTERED)
Facade-->>Controller: 404
Controller-->>Client: ApiResponse(404)
else result found
Service->>DbtiRepo: findByDbtiType(result.dbtiType)
DbtiRepo-->>Service: DbtiEntity
Service-->>Facade: DbtiResultDetailVo
Facade-->>Controller: DbtiResultResponseDto
Controller-->>Client: ApiResponse(200, body)
end
end
end
deactivate Service
deactivate Facade
deactivate Controller
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/dbti/api/controller/DbtiController.java`:
- Around line 75-81: The getPetDbtiResult endpoint is missing the `@UserId`
authentication annotation and owner verification; update the method signature of
getPetDbtiResult to accept the authenticated user id via `@UserId` (matching how
getDbtiQuestions/submitDbti do) and pass/validate that id before calling
dbtiQueryFacade.getPetDbtiResult to ensure the requesting user owns the pet (or
otherwise enforce authorization); if dbtiQueryFacade lacks an overload for owner
checks, add/adjust a service/facade method to perform ownership validation and
return the DbtiResultResponseDto only when authorized.
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/dbti/infra/persistence/entity/DbtiResultEntity.java`:
- Around line 33-35: The pet relationship on DbtiResultEntity is currently a
nullable=false `@JoinColumn` but lacks a uniqueness constraint allowing multiple
DbtiResultEntity rows to reference the same Pet; update the mapping to enforce
1:1 uniqueness by adding a unique constraint for the pet_id join column (e.g.,
set unique=true on the `@JoinColumn` for the pet field or add a
`@Table`(uniqueConstraints = ...) on DbtiResultEntity) so that
DbtiResultEntity.pet cannot be duplicated and the DB schema enforces one
DbtiResult per Pet.
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/pet/api/dto/response/PetProfileResponseDto.java`:
- Around line 28-29: The map/construction in PetProfileResponseDto calls
pet.getBreed().getName() without null-safety, which can NPE if breed is null;
update the code that builds the response (where pet.getBreed() is accessed) to
null-check pet.getBreed() and use a safe fallback (e.g., empty string or "품종
미등록") similar to how profileImage and dbtiResult are handled — replace direct
pet.getBreed().getName() with a conditional or Optional access that returns a
default when getBreed() is null.
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/pet/infra/persistence/entity/PetEntity.java`:
- Around line 65-66: The PetEntity has a lazy `@OneToOne` dbtiResult causing an
extra SELECT when calling the inherited findById(petId); apply the same eager
fetch via an EntityGraph or add a repository method that loads it only when
needed. Concretely, add `@EntityGraph`(attributePaths =
{"profileImage","dbtiResult"}) to the repository method used for single-item
loads (the findById equivalent) or create a new method such as
findByIdWithDbtiResult(Long petId) that carries that `@EntityGraph`, so accesses
to PetEntity.dbtiResult won't trigger the extra lazy SELECT at runtime.
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/user/application/service/UserPetQueryService.java`:
- Line 19: Remove the unused DbtiResultRepository dependency from
UserPetQueryService: delete the DbtiResultRepository field and its
constructor/injection parameter, and remove any unused import; rely on
SpringDataPetRepository's `@EntityGraph` (attributePaths =
{"profileImage","dbtiResult"}) and the existing PetProfileResponseDto.from()
which accesses pet.getDbtiResult() to provide DBTI data instead.
🧹 Nitpick comments (3)
src/main/java/org/sopt/pawkey/backendapi/domain/pet/api/dto/request/PetRequestDto.java (1)
20-20: 양수 검증 추가를 고려해 주세요.
@NotNull은 null 체크만 수행합니다. 유효하지 않은 ID 값(예: 0 또는 음수)이 전달될 경우를 방지하려면@Positive어노테이션을 추가하는 것을 권장합니다.💡 선택적 개선 제안
- `@NotNull`(message = "품종은 필수값입니다.") Long breedId + `@NotNull`(message = "품종은 필수값입니다.") + `@Positive`(message = "품종 ID는 양수여야 합니다.") Long breedId
jakarta.validation.constraints.Positiveimport가 필요합니다.src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiQueryService.java (2)
30-32: 필드 선언 위치가 일반적인 Java 컨벤션과 다릅니다.
resultRepository와petRepository필드가 기존 메서드 아래에 선언되어 있습니다. 일반적으로 Java에서는 모든 필드를 클래스 상단에 모아 선언합니다.♻️ 제안하는 수정
`@Service` `@RequiredArgsConstructor` `@Transactional`(readOnly = true) public class DbtiQueryService { private final DbtiRepository dbtiRepository; + private final DbtiResultRepository resultRepository; + private final PetRepository petRepository; public List<DbtiQuestionEntity> getAllQuestions() { return dbtiRepository.findAllQuestionsWithDetails(); } - - private final DbtiResultRepository resultRepository; - private final PetRepository petRepository; public DbtiResultDetailVo getPetDbtiResultDetail(Long petId) {
34-47: 불필요한 DB 호출 및 중복 코드 개선 가능
petRepository.existsById(petId)체크는resultRepository.findByPetId(petId)가 빈 결과를 반환하면 자연스럽게 처리되므로, 별도의 존재 확인 쿼리가 중복될 수 있습니다. 단, Pet이 존재하지만 DBTI 결과가 없는 경우와 Pet 자체가 없는 경우를 구분해야 한다면 현재 로직이 맞습니다.Line 46에서
DbtiResultDetailVo가 이미 import 되어 있으므로 FQN(fully qualified name) 사용이 불필요합니다.♻️ 제안하는 수정
- return new org.sopt.pawkey.backendapi.domain.dbti.application.dto.DbtiResultDetailVo(result, dbtiInfo); + return new DbtiResultDetailVo(result, dbtiInfo);
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiCommandService.java`:
- Around line 42-44: The ownership check in DbtiCommandService currently risks
NPE and uses the wrong error code; update the check to first null-check
pet.getUser() (avoid calling pet.getUser().getUserId() when pet.getUser() is
null) and, if the owner is null or owner.getUserId() does not equal userId,
throw a permission-related UserBusinessException (e.g., use
UserErrorCode.FORBIDDEN or add a new UserErrorCode.USER_FORBIDDEN constant and
throw that) instead of USER_NOT_FOUND; ensure you update any enum/constants if
you add USER_FORBIDDEN and use that new constant in the throw site
(DbtiCommandService).
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiQueryService.java`:
- Around line 42-44: In DbtiQueryService replace the direct call to
pet.getUser().getUserId() with a null-safe check: if pet.getUser() is null,
throw UserBusinessException(UserErrorCode.USER_NOT_FOUND); otherwise, when the
owner's userId does not equal the provided userId, throw an
authorization-specific error (e.g.,
UserBusinessException(UserErrorCode.FORBIDDEN) or USER_NOT_AUTHORIZED) instead
of USER_NOT_FOUND to reflect lack of permission; update the throw site that
currently uses UserErrorCode.USER_NOT_FOUND to use the authorization error and
ensure you reference the existing pet.getUser() and userId variables.
♻️ Duplicate comments (1)
src/main/java/org/sopt/pawkey/backendapi/domain/pet/infra/persistence/entity/PetEntity.java (1)
65-66:findById()메서드에서dbtiResult지연 로딩 최적화 필요
@OneToOne(mappedBy="pet", fetch=FetchType.LAZY)설정이 되어 있지만, Hibernate에서 inverse side의@OneToOneLAZY 로딩은 실제로 프록시를 생성하지 못하고 추가 SELECT가 발생합니다.findAllByUser_UserId()에는@EntityGraph가 적용되어 있으나,findById()에는 적용되지 않아 개별 Pet 조회 시 추가 쿼리가 발생할 수 있습니다.
🧹 Nitpick comments (5)
src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiCommandService.java (1)
62-75: 양방향 연관관계 동기화 고려가 필요합니다.새로운
DbtiResultEntity생성 시pet.setDbtiResult(result)를 호출하지 않아, 같은 트랜잭션 내에서pet.getDbtiResult()가null을 반환할 수 있습니다.DbtiResultEntity에 이미setPet()메서드가 양방향 동기화를 처리하도록 구현되어 있으므로 활용하는 것이 좋습니다.♻️ 수정 제안
DbtiResultEntity result = resultRepository.findByPetId(petId) .map(existingResult -> { existingResult.updateResult(dbtiType, eiScore, psScore, rfScore); return existingResult; }) .orElseGet(() -> { - return resultRepository.save(DbtiResultEntity.builder() + DbtiResultEntity newResult = DbtiResultEntity.builder() .pet(pet) .dbtiType(dbtiType) .eiScore(eiScore) .psScore(psScore) .rfScore(rfScore) - .build()); + .build(); + pet.setDbtiResult(newResult); + return resultRepository.save(newResult); });src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiQueryService.java (2)
33-35: 필드 선언 위치가 일반적인 컨벤션을 따르지 않습니다.
resultRepository와petRepository필드가 메서드 정의 이후에 선언되어 있습니다. Java에서는 일반적으로 모든 필드를 클래스 상단에 선언하는 것이 가독성과 유지보수에 좋습니다.♻️ 수정 제안
`@Service` `@RequiredArgsConstructor` `@Transactional`(readOnly = true) public class DbtiQueryService { private final DbtiRepository dbtiRepository; + private final DbtiResultRepository resultRepository; + private final PetRepository petRepository; public List<DbtiQuestionEntity> getAllQuestions() { return dbtiRepository.findAllQuestionsWithDetails(); } - - private final DbtiResultRepository resultRepository; - private final PetRepository petRepository; public DbtiResultDetailVo getPetDbtiResultDetail(Long userId, Long petId) {
52-52: 불필요한 FQN(Fully Qualified Name) 사용
DbtiResultDetailVo는 이미 5번 라인에서 import되어 있으므로 FQN 없이 사용할 수 있습니다.♻️ 수정 제안
- return new org.sopt.pawkey.backendapi.domain.dbti.application.dto.DbtiResultDetailVo(result, dbtiInfo); + return new DbtiResultDetailVo(result, dbtiInfo);src/main/java/org/sopt/pawkey/backendapi/domain/pet/infra/persistence/entity/PetEntity.java (1)
93-95: 양방향 연관관계 동기화가 누락되었습니다.
setDbtiResult메서드가DbtiResultEntity.setPet()처럼 양방향 동기화를 수행하지 않습니다. 일관성을 위해 양쪽 setter 모두 동기화 로직을 포함하거나, 한쪽에서만 관리하도록 명확히 하는 것이 좋습니다.♻️ 수정 제안
public void setDbtiResult(DbtiResultEntity dbtiResult) { this.dbtiResult = dbtiResult; + if (dbtiResult != null && dbtiResult.getPet() != this) { + dbtiResult.setPet(this); + } }단,
DbtiResultEntity.setPet()에서 이미 역방향 동기화를 수행하므로 무한 루프 방지 조건이 있는지 확인하세요.src/main/java/org/sopt/pawkey/backendapi/domain/dbti/infra/persistence/entity/DbtiResultEntity.java (1)
15-15: 사용하지 않는 import 문이 있습니다.
@ManyToOne이 import되어 있지만 실제로는@OneToOne만 사용됩니다.♻️ 수정 제안
-import jakarta.persistence.ManyToOne;
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/sopt/pawkey/backendapi/domain/user/api/controller/UserController.java (1)
111-123: Swagger 설명을 단일 반려견 조회에 맞게 수정하세요.
@Operation설명이 "반려견 목록을 조회합니다"로 되어 있지만, 실제 API는 단일PetProfileResponseDto를 반환합니다. 설명을 실제 동작과 일치하도록 수정해야 합니다.📝 제안된 수정사항
- `@Operation`(summary = "유저 반려견 프로필 조회", description = "유저가 등록한 반려견 목록을 조회합니다.", tags = {"Users"}) + `@Operation`(summary = "유저 반려견 프로필 조회", description = "유저가 등록한 반려견 프로필을 조회합니다.", tags = {"Users"})
🤖 Fix all issues with AI agents
In
`@src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiQueryService.java`:
- Around line 46-47: The code currently throws
UserBusinessException(UserErrorCode.USER_PET_NOT_REGISTERED) when
resultRepository.findByPetId(petId) returns empty; change this to throw a
domain-appropriate exception by adding a DBTI_RESULT_NOT_FOUND entry to
DbtiErrorCode (e.g., "해당 반려견의 DBTI 결과가 존재하지 않습니다.") and then replace the throw
with new DbtiBusinessException(DbtiErrorCode.DBTI_RESULT_NOT_FOUND) in the
DbtiQueryService where DbtiResultEntity is looked up
(resultRepository.findByPetId).
🧹 Nitpick comments (5)
src/main/java/org/sopt/pawkey/backendapi/domain/pet/infra/persistence/repository/PetRepositoryImpl.java (1)
3-18: 사용되지 않는 import와 의존성을 제거하세요.리팩토링 후 다음 항목들이 사용되지 않습니다:
- Line 3:
java.util.Listimport- Line 7:
BreedEntityimport- Line 18:
springDataBreedRepository필드♻️ 제안된 수정사항
package org.sopt.pawkey.backendapi.domain.pet.infra.persistence.repository; -import java.util.List; import java.util.Optional; import org.sopt.pawkey.backendapi.domain.pet.domain.repository.PetRepository; -import org.sopt.pawkey.backendapi.domain.pet.infra.persistence.entity.BreedEntity; import org.sopt.pawkey.backendapi.domain.pet.infra.persistence.entity.PetEntity; import org.springframework.stereotype.Repository; import lombok.RequiredArgsConstructor; `@Repository` `@RequiredArgsConstructor` public class PetRepositoryImpl implements PetRepository { private final SpringDataPetRepository springDataPetRepository; - private final SpringDataBreedRepository springDataBreedRepository;src/main/java/org/sopt/pawkey/backendapi/domain/user/application/facade/query/UserPetQueryFacade.java (1)
3-3: 사용되지 않는 import를 제거하세요.
java.util.Listimport가 단일 객체 반환으로 리팩토링된 후 더 이상 사용되지 않습니다.♻️ 제안된 수정사항
package org.sopt.pawkey.backendapi.domain.user.application.facade.query; -import java.util.List; - import org.sopt.pawkey.backendapi.domain.pet.api.dto.response.PetProfileResponseDto;src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiCommandService.java (1)
19-20: 사용되지 않는 import 구문이 있습니다.
UserBusinessException과UserErrorCode가 import되어 있지만, 접근 권한 검사가PetBusinessException(PetErrorCode.PET_ACCESS_DENIED)로 변경되면서 더 이상 사용되지 않습니다.🧹 제거 제안
-import org.sopt.pawkey.backendapi.domain.user.exception.UserBusinessException; -import org.sopt.pawkey.backendapi.domain.user.exception.UserErrorCode;src/main/java/org/sopt/pawkey/backendapi/domain/dbti/application/service/DbtiQueryService.java (2)
33-35: 필드 선언 위치가 일관성 없습니다.
resultRepository와petRepository필드가 메서드(getAllQuestions) 이후에 선언되어 있습니다. 클래스 상단의 다른 필드(dbtiRepository)와 함께 배치하는 것이 좋습니다.♻️ 리팩토링 제안
`@Service` `@RequiredArgsConstructor` `@Transactional`(readOnly = true) public class DbtiQueryService { private final DbtiRepository dbtiRepository; + private final DbtiResultRepository resultRepository; + private final PetRepository petRepository; public List<DbtiQuestionEntity> getAllQuestions() { return dbtiRepository.findAllQuestionsWithDetails(); } - - private final DbtiResultRepository resultRepository; - private final PetRepository petRepository;
52-52: 불필요한 전체 패키지 경로 사용
DbtiResultDetailVo는 이미 line 5에서 import되어 있으므로 전체 패키지 경로 없이 사용할 수 있습니다.🧹 수정 제안
- return new org.sopt.pawkey.backendapi.domain.dbti.application.dto.DbtiResultDetailVo(result, dbtiInfo); + return new DbtiResultDetailVo(result, dbtiInfo);
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class DbtiQueryFacade { |
There was a problem hiding this comment.
[P1]
이전에 리뷰 드렸던 UpdateUserInfoFacade 사례와 비슷한 맥락의 의견이 있습니다!
모든 기능(Query, Update 등)이 Controller에 도달하기 전 기계적으로 Facade를 거쳐야 한다는 접근보다는, 해당 유즈케이스에서 Facade가 없을 때, Controller가 실제로 어떤 문제가 있을지를 먼저 고민해 보면 좋을 것 같습니다
현재 DbtiQueryFacade는 유즈케이스 흐름을 조율하기보다 조회 결과를 변환하는 전달하는 역할에 집중되어 있어, 결과적으로 계층의 복잡도만 높이는 상황인 것 같아요(사실 Facade는 비즈니스 로직을 조율하는 곳이지, 단순 변환 로직을 담당하기에는 역할이 다소 모호하다고 생각합니다)
물론 저희가 도메인별로 Facade와 Service를 기본 패키지 구조로 가져가고는 있지만, 모든 기능을 1:1로 Facade로 쪼개는 것이 최선인지는 논의가 필요해 보입니다! 제가 제안드리는 방향은 이렇습니다
-
여러 도메인을 조합할 필요가 없는 단순 유즈케이스는 Facade 없이 Application Service에서 담백하게 처리 (단순 전달자 역할의 Facade를 지양해서, Facade의 역할을 인지하면서 개발)
-
여러 도메인(User, Image 등)을 엮거나, 정책적인 흐름 제어가 필요한 시점에 도입
-
단위 재정의: 기능을 하나하나 Facade 클래스로 쪼개기보다, 의미 있는 큰 유즈케이스 단위(ex: DbtiFacade)로 응집도를 높여서 관리
앞으로도 Facade가 단순 전달자 역할만 하게 될 때, 이 방식을 관성적으로 지키는 것이 생산성과 유지보수에 도움이 될 지 같이 이야기해봐요!
There was a problem hiding this comment.
리뷰 주신 Facade 레이어의 관성적 사용에 대한 우려에 공감하는 부분입니다. 제 코드에서도 단순 전달 역할만 수행하는 부분이 계층 복잡도를 높였다는 점을 인지했습니다.
그렇지만, 현재 저희 프로젝트는 CQRS 원칙에 따라 Command와 Query의 책임을 패키지 수준에서 분리하고 있습니다. 또한 특정 유즈케이스에서만 Facade를 생략할 경우, 전체적인 아키텍처의 일관성이 깨진다는 우려가 있습니다. 그리고 단순 조회일지라도 향후 권한 체크, 로깅, 혹은 타 도메인과의 조율 로직이 추가될 때 Facade 레이어가 이미 존재한다면 코드 수정이 훨씬 안정적일 것이라는 생각도 드는 바입니다.
따라서 이번 PR에서는 구조적 통일성을 위해 Facade 레이어는 유지하되, 언급해 주신 '단순 변환 로직' 문제는 DTO 내부에 정적 팩토리 메서드(from)를 구현하여 Facade의 코드를 최소화하는 방향으로 개선해 보았습니다. 이렇게 함으로써 Facade가 순수하게 유즈케이스 흐름만 제어하는 역할에 집중하도록 교정했습니다.
마지막으로 현재 모든 도메인에 동일한 아키텍처가 적용되어 있는 상태이기 때문에, 말씀해주신것처럼 추후에 함께 아키텍처 표준을 논의하고 확정된 안을 별도 이슈로 생성하여 전사적으로 적용하는 방향은 어떨까요? 의견 부탁드립니다!
| private static final int EXPECTED_OPTION_COUNT = 9; | ||
|
|
||
| public DbtiResultDetailVo calculateSaveAndGetDetail(Long petId, DbtiSubmitRequestDto request) { | ||
| public DbtiResultDetailVo calculateSaveAndGetDetail(Long userId, Long petId, DbtiSubmitRequestDto request) { |
There was a problem hiding this comment.
[P1]
DbtiCommandService와 DbtiQueryService 모두에서 DbtiResultDetailVo를 반환하고 있는데, 현재 이 객체는 두 엔티티(DbtiResultEntity, DbtiEntity)를 함께 묶어 전달하기 위한 중간 전달 객체로 사용되고 있는 것으로 보입니다. 혹시 의도를 알 수 있을까요?! 개인적으로 이 부분에 대해 우려되는 지점이 있습니다
앱잼 초기에 Entity와 Domain 객체를 분리하는 DDD구조로 방향을 잡은 후, 개발 과정에서 복잡도와 N+1 문제 등을 고려하여, JPA Entity를 도메인 모델로 직접 사용하기로 합의한 상태로 알고 있습니다
현재 VO 사용에 대한 합의가 없는 상황에서, 식별자를 가진 엔티티들을 단순히 묶어주는 객체에 Vo라는 네이밍을 사용하는 것은 역할 해석이나 향후 리팩터링 과정에서 혼란을 줄 수 있을 것 같다는 생각이 들었습니다!!
(다은님은 평소에 VO 계층을 어떤 용도로 정의하고 사용하시는지 궁금합니다! 이번 기회에 이 부분에 대한 의도를 들어보고, 오프라인 회의에서 Facade/Service 반환 컨벤션과 VO 도입 여부를 함께 고민해보면 좋을 것 같아요~!)
There was a problem hiding this comment.
리뷰 감사합니다! DbtiResultDetailVo는 말씀하신 대로 복잡한 의도가 있었다기보다는, 두 엔티티를 한꺼번에 반환해야 하는 상황에서 단순히 데이터를 묶어 전달(Wrapper)하기 위한 용도로 사용했습니다. 평소 저는 VO를 '불변성을 가진 값의 집합' 정도로 생각하고 사용해 왔는데, 우리 프로젝트의 컨벤션(Entity를 도메인 모델로 직접 사용)을 고려했을 때 Vo라는 명칭이 오해를 살 수 있다는 점을 알게 되었습니다. 따라서, 이번 기회에 제안해 주신 대로 단순 전달 객체의 네이밍 컨벤션을 명확히 정하면 좋을 것 같습니다!
우선은 혼란을 방지하기 위해 명칭을 Dto 혹은 목적에 맞는 이름으로 수정해 보려 했으나, 이전 리뷰에서 제안해 주신 'Facade의 역할 재정의'와 연결 지어 고민해 본 결과, 서비스에서 임시 객체(Vo)로 묶어 반환하기보다 Facade에서 각 서비스를 호출하여 결과를 조합(Orchestration)하는 방식이 더 적절하다고 판단하여 해당 방식으로 리팩터링했습니다.
이를 통해 서비스는 각자의 도메인 로직에 더 집중할 수 있게 되었고, Facade는 여러 서비스를 조율하는 본연의 역할을 수행하게 되었습니다. 네이밍 혼란을 야기했던 중간 전달 객체도 제거하여 구조가 한결 담백해지도록 구성했습니다-!
| public class DbtiQueryService { | ||
| private final DbtiRepository dbtiRepository; |
There was a problem hiding this comment.
[P1]
질문 조회(getAllQuestions)는 엔티티 리스트를, 결과 조회(getPetDbtiResultDetail)는 중간 객체를 반환하고 있는데 의도된 건가요?~
제 생각으로는
엔티티 반환보다는 ResponseDto 반환으로 역할을 통일하는 것이 구조적으로 더 명확할 것 같습니다!(queryService의 성격이 api명세이 dto에 맞는 데이터를 서빙하는 역할이니까요)
There was a problem hiding this comment.
리뷰 감사합니다! 말씀해 주신 내용을 바탕으로 서비스에서 DTO를 반환할 때의 실용적인 이점들에 대해 찾아보고 고민해보게 되었습니다.
그럼에도 이번 PR에서 모든 반환 타입을 엔티티로 통일한 이유는 현재 우리 프로젝트가 Facade 계층을 적극적으로 활용하고 있기 때문입니다. 파사드 레이어는 여러 서비스의 도메인 데이터를 조합하여 하나의 유즈케이스를 완성하는 오케스트레이션 역할을 수행한다고 인지했기에, 서비스가 특정 API 스펙인 DTO에 종속되기보다 순수한 엔티티를 제공하는 것이 로직의 재사용성과 유연한 조합 측면에서 더 유리하다고 판단했습니다!
또한 이러한 구조는 프론트엔드의 요구사항 변경이 비즈니스 로직이 담긴 서비스 레이어까지 전파되는 것을 차단하여 레이어 간의 결합도를 낮추는 효과가 있고, 엔티티 반환 시에 우려될 수 있는 지연 로딩 문제는 레포지토리 단에서 Fetch Join 등을 활용해 안정적으로 관리하고 있기 때문에, 이를 통해 성능과 설계의 일관성을 동시에 확보할 수 있다고 생각했습니당.
다만 제안해 주신 방식이 주는 개발 생산성과 구조적 명확성 역시 큰 장점인 만큼, 어떤 방향이 우리 팀의 표준 아키텍처로서 더 적합할지에 대해서는 오프라인 회의를 통해 함께 논의해 보고 싶습니다!
|
고생하셨어요를레이히~ |
|
리뷰해주신 내용을 바탕으로 반려견 검증 로직 공통화 및 레이어 책임 분리 리팩터링을 완료하여 푸시했습니다. 제안해주신 덕분에 중복을 제거하고 도메인 간 결합도를 낮출 수 있었습니다. 서비스 반환 타입과 같은 아키텍처 방향성에 대해서는 이번 리팩터링 구조를 바탕으로 추후 오프라인에서 함께 논의해보고 싶습니다. 시간 되실 때 다시 한번 확인 부탁드립니다. 감사합니다! |
2 participants
📌 PR 제목
[feat] 반려견 dbti 조회 API 및 데이터 구조 최적화 (Breed, DBTI 연관관계 개선)
✨ 요약 설명
반려견 DBTI 검사 결과를 조회하는 API를 구현하고, 이 과정에서 발생할 수 있는 성능 저하(N+1 문제)를 방지하기 위해 연관관계를 최적화했습니다. 또한, 최신 develop 브랜치 머지 과정에서 누락되거나 개선이 필요한 재검사 로직, DBTI 타입 반환 연동, 견종(Breed) 엔티티 연결 작업을 통합하여 수행했습니다.
🧾 변경 사항
📂 PR 타입
🧪 테스트
✅ 체크리스트
💬 리뷰어에게 전달할 말
🔗 관련 이슈
Summary by CodeRabbit
릴리스 노트
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.