Conversation
jpa 중복 의존성 삭제
도메인, dao(repository), 서비스, 컨트롤러 작성 - dao(repository) : JPA사용하여 구현 - 서비스 : 인터페이스 설계 후, 구현체 작성 - 컨트롤러 : 공통 응답 포맷 만들어서 resposneEntity에 담아서 응답
- 커스텀 예외와 에러코드 추가
- controller 에 responseEntity<ApiResponse>로 응답 - dto 를 inner class로 작성
요약 분석Walkthrough이 PR은 Spring Boot 애플리케이션에 멤버 태그 관리 기능을 추가합니다. REST 컨트롤러, 데이터 전송 객체(DTO), JPA 저장소 인터페이스, 도메인 엔티티, 서비스 계층, 에러 처리 로직이 포함되어 있습니다. build.gradle에서 사용하지 않는 spring-boot-starter-data-jpa 의존성을 제거했습니다. Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as MemberTagController
participant Service as MemberTagServiceImpl
participant Dao as MemberTagJpaDao
participant DB as Database
Note over Client,DB: 태그 생성
Client->>Controller: POST /api/tags
Controller->>Service: create(ownerMemberId, tagName)
Service->>Dao: existsByOwnerMemberIdAndTagName()
Dao->>DB: 조회
DB-->>Dao: 존재 여부
Dao-->>Service: boolean
Service->>Dao: save(new MemberTag)
Dao->>DB: INSERT
DB-->>Dao: MemberTag
Dao-->>Service: MemberTag
Service-->>Controller: Long (tagId)
Controller-->>Client: 201 Created + tagId
Note over Client,DB: 태그 조회
Client->>Controller: GET /api/tags/{tagId}
Controller->>Service: get(tagId)
Service->>Dao: findById(tagId)
Dao->>DB: SELECT
DB-->>Dao: MemberTag
Dao-->>Service: Optional<MemberTag>
Service-->>Controller: MemberTag
Controller->>Controller: DTO 변환
Controller-->>Client: 200 OK + Response DTO
Note over Client,DB: 태그 업데이트
Client->>Controller: PUT /api/tags/{tagId}
Controller->>Service: update(tagId, tagName)
Service->>Dao: findById(tagId)
Dao->>DB: SELECT
DB-->>Dao: MemberTag
Dao-->>Service: Optional<MemberTag>
Service->>Service: changeName() 검증
Service->>Dao: save(updated MemberTag)
Dao->>DB: UPDATE
DB-->>Dao: MemberTag
Dao-->>Service: MemberTag
Service-->>Controller: void
Controller-->>Client: 200 OK
Note over Client,DB: 태그 삭제
Client->>Controller: DELETE /api/tags/{tagId}
Controller->>Service: delete(tagId)
Service->>Dao: deleteById(tagId)
Dao->>DB: DELETE
DB-->>Dao: void
Dao-->>Service: void
Service-->>Controller: void
Controller-->>Client: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@src/main/java/com/web/SearchWeb/tag/controller/dto/MemberTagDto.java`:
- Around line 11-22: Add Bean Validation annotations to the DTOs so invalid
requests are rejected at controller level: annotate CreateRequest.ownerMemberId
with `@NotNull` and CreateRequest.tagName and UpdateRequest.tagName with `@NotBlank`
(use javax.validation.constraints.*), keep the existing
`@Getter/`@NoArgsConstructor, and ensure the controller method validates the
request by using `@Valid` on the CreateRequest/UpdateRequest parameters so Spring
returns 400 on violations.
In `@src/main/java/com/web/SearchWeb/tag/controller/MemberTagController.java`:
- Around line 28-35: The controller currently accepts ownerMemberId from the
request body which enables IDOR; update MemberTagController.create to stop using
req.getOwnerMemberId() and instead obtain the authenticated user's id from the
security context (e.g., `@AuthenticationPrincipal` or
SecurityContextHolder.getContext().getAuthentication()) and pass that id to
memberTagService.create; remove or ignore ownerMemberId from
MemberTagDto.CreateRequest (or validate and override it) so only the
authenticated user's id is used, and update any callers/tests accordingly
(references: MemberTagController.create, MemberTagDto.CreateRequest,
memberTagService.create).
- Around line 64-69: Change the delete handler in MemberTagController to return
204 No Content without a response body: update the method signature and return
type from ResponseEntity<ApiResponse<Void>> to ResponseEntity<Void>, keep the
call to memberTagService.delete(tagId), and return
ResponseEntity.noContent().build() (or
ResponseEntity.status(HttpStatus.NO_CONTENT).build()) instead of
ResponseEntity.ok(ApiResponse.success(null)).
- Line 30: Add javax/ jakarta validation to the controller method parameters so
Spring triggers Bean Validation: annotate the `@RequestBody` parameter in
MemberTagController#create (method signature with create(`@RequestBody`
MemberTagDto.CreateRequest req)) with `@Valid`, and do the same for the update
endpoint method (the update(...) method at line ~58). Also add the appropriate
import for `@Valid` (javax.validation.Valid or jakarta.validation.Valid to match
project Jakarta/Javax usage). Ensure DTO fields already have validation
annotations so this takes effect.
In `@src/main/java/com/web/SearchWeb/tag/domain/MemberTag.java`:
- Line 17: The `@AllArgsConstructor` on MemberTag should be made private to
prevent bypassing the `@Builder`; change the annotation on the MemberTag class to
`@AllArgsConstructor`(access = AccessLevel.PRIVATE) and add the necessary
lombok.AccessLevel import if it's missing so the builder is the only intended
creation path.
- Around line 15-29: MemberTag’s ownerMemberId and tagName fields lack DB-level
non-null constraints and tagName has no length limit; update the MemberTag
entity by adding nullable = false to the `@Column` on ownerMemberId and tagName,
and add a sensible length (e.g., length = 100) to the `@Column` on tagName so DDL
generation enforces non-null and a max length; ensure you edit the fields named
ownerMemberId and tagName in the MemberTag class.
In `@src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java`:
- Line 11: The enum constant in TagErrorCode uses incorrect casing: rename
Tag_NOT_FOUND to TAG_NOT_FOUND in the TagErrorCode enum and update every
reference to it (for example in TagException.java) to use TAG_NOT_FOUND; ensure
imports/usages still compile and adjust any switch/statements or tests that
expect the old name.
In `@src/main/java/com/web/SearchWeb/tag/service/MemberTagServiceImpl.java`:
- Around line 19-32: Replace the IllegalArgumentException in
MemberTagServiceImpl.create with a consistent TagException using a new
TagErrorCode entry TAG_ALREADY_EXISTS(HttpStatus.CONFLICT, "T002", "이미 존재하는
태그입니다."); remove the pre-check race-prone existsByOwnerMemberIdAndTagName or
keep it only for fast-fail but do not rely on it as the sole guard; add a unique
DB constraint on (owner_member_id, tag_name) and wrap memberTagJpaDao.save(tag)
to catch DataIntegrityViolationException and rethrow TagException with
TagErrorCode.TAG_ALREADY_EXISTS so concurrent requests map to the same HTTP 409
behavior (use the existing TagException pattern used by TagException.NotFound
for structure).
- Around line 67-71: The service method validateTagName currently duplicates
null/blank checks already enforced by the domain entity MemberTag.changeName and
throws IllegalArgumentException; remove the duplicated validation or make it a
thin delegator and instead map domain validation failures to a meaningful
service-level exception using your TagErrorCode (e.g., catch the domain
exception from MemberTag.changeName and throw a TagException/Error with the
appropriate TagErrorCode), ensuring the entity keeps its own validation while
the service returns domain-specific errors; update references to validateTagName
and MemberTag.changeName accordingly so the service no longer throws
IllegalArgumentException for tag-name problems but returns TagErrorCode-based
exceptions.
- Around line 58-65: 현재 delete(Long memberTagId)는 존재하지 않는 엔티티에 대해 조용히
성공(return)하는데, get()과 update()가 존재하지 않을 때 TagException.NotFound를 던지는 것과 일관되지
않습니다; 수정 방법은 delete()에서 memberTagJpaDao.existsById(memberTagId) 체크 시 단순 return을
제거하고 엔티티가 없으면 TagException.NotFound를 던지도록 변경(또는 설계상 조용한 삭제가 의도라면 해당 의도를 코드 주석 또는
문서화)하고, 중복 쿼리를 줄이려면 필요에 따라 existsById 대신 findById 사용 후 존재하지 않으면 예외를 던지고 존재하면
memberTagJpaDao.deleteById(memberTagId)를 호출하도록 일관된 동작을 구현하세요 (참고 식별자:
delete(Long), memberTagJpaDao.existsById, memberTagJpaDao.deleteById,
TagException.NotFound, get(), update()).
| @Getter | ||
| @NoArgsConstructor | ||
| public static class CreateRequest { | ||
| private Long ownerMemberId; | ||
| private String tagName; | ||
| } | ||
|
|
||
| @Getter | ||
| @NoArgsConstructor | ||
| public static class UpdateRequest { | ||
| private String tagName; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
요청 DTO에 Bean Validation 어노테이션 추가 권장
CreateRequest와 UpdateRequest에 @NotNull, @NotBlank 등의 Bean Validation 어노테이션이 없어서 잘못된 요청이 서비스 레이어까지 도달한 후에야 IllegalArgumentException으로 처리됩니다. DTO 레벨에서 검증하면 컨트롤러에서 @Valid와 함께 자동으로 400 Bad Request 응답을 생성할 수 있습니다.
♻️ 수정 제안
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
`@Getter`
`@NoArgsConstructor`
public static class CreateRequest {
+ `@NotNull`
private Long ownerMemberId;
+ `@NotBlank`
private String tagName;
}
`@Getter`
`@NoArgsConstructor`
public static class UpdateRequest {
+ `@NotBlank`
private String tagName;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Getter | |
| @NoArgsConstructor | |
| public static class CreateRequest { | |
| private Long ownerMemberId; | |
| private String tagName; | |
| } | |
| @Getter | |
| @NoArgsConstructor | |
| public static class UpdateRequest { | |
| private String tagName; | |
| } | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.NotNull; | |
| `@Getter` | |
| `@NoArgsConstructor` | |
| public static class CreateRequest { | |
| `@NotNull` | |
| private Long ownerMemberId; | |
| `@NotBlank` | |
| private String tagName; | |
| } | |
| `@Getter` | |
| `@NoArgsConstructor` | |
| public static class UpdateRequest { | |
| `@NotBlank` | |
| private String tagName; | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/controller/dto/MemberTagDto.java` around
lines 11 - 22, Add Bean Validation annotations to the DTOs so invalid requests
are rejected at controller level: annotate CreateRequest.ownerMemberId with
`@NotNull` and CreateRequest.tagName and UpdateRequest.tagName with `@NotBlank` (use
javax.validation.constraints.*), keep the existing `@Getter/`@NoArgsConstructor,
and ensure the controller method validates the request by using `@Valid` on the
CreateRequest/UpdateRequest parameters so Spring returns 400 on violations.
| // 생성 | ||
| @PostMapping | ||
| public ResponseEntity<ApiResponse<Long>> create(@RequestBody MemberTagDto.CreateRequest req) { | ||
| Long tagId = memberTagService.create(req.getOwnerMemberId(), req.getTagName()); | ||
| return ResponseEntity | ||
| .status(HttpStatus.CREATED) | ||
| .body(ApiResponse.success(tagId)); | ||
| } |
There was a problem hiding this comment.
보안 위험: ownerMemberId가 클라이언트 요청 본문에서 제공됩니다.
현재 ownerMemberId를 요청 본문에서 직접 받고 있어, 인증된 사용자가 아닌 다른 사용자의 ID를 지정하여 임의 사용자에게 태그를 생성할 수 있습니다. 인증/인가 구현 시 ownerMemberId는 인증된 사용자의 세션/토큰에서 추출해야 합니다.
PR 설명에 "추가 정제가 필요하다"고 되어 있으므로 향후 구현 계획이 있을 수 있지만, 현재 상태로 배포되면 IDOR(Insecure Direct Object Reference) 취약점이 됩니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/controller/MemberTagController.java`
around lines 28 - 35, The controller currently accepts ownerMemberId from the
request body which enables IDOR; update MemberTagController.create to stop using
req.getOwnerMemberId() and instead obtain the authenticated user's id from the
security context (e.g., `@AuthenticationPrincipal` or
SecurityContextHolder.getContext().getAuthentication()) and pass that id to
memberTagService.create; remove or ignore ownerMemberId from
MemberTagDto.CreateRequest (or validate and override it) so only the
authenticated user's id is used, and update any callers/tests accordingly
(references: MemberTagController.create, MemberTagDto.CreateRequest,
memberTagService.create).
|
|
||
| // 생성 | ||
| @PostMapping | ||
| public ResponseEntity<ApiResponse<Long>> create(@RequestBody MemberTagDto.CreateRequest req) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
@RequestBody에 @Valid 어노테이션 누락
DTO에 Bean Validation 어노테이션을 추가한다면, 컨트롤러의 @RequestBody 파라미터에 @Valid를 붙여야 자동 검증이 동작합니다. update 엔드포인트(Line 58)에도 동일하게 적용해야 합니다.
♻️ 수정 제안
+import jakarta.validation.Valid;
+
`@PostMapping`
-public ResponseEntity<ApiResponse<Long>> create(`@RequestBody` MemberTagDto.CreateRequest req) {
+public ResponseEntity<ApiResponse<Long>> create(`@Valid` `@RequestBody` MemberTagDto.CreateRequest req) {🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/controller/MemberTagController.java` at
line 30, Add javax/ jakarta validation to the controller method parameters so
Spring triggers Bean Validation: annotate the `@RequestBody` parameter in
MemberTagController#create (method signature with create(`@RequestBody`
MemberTagDto.CreateRequest req)) with `@Valid`, and do the same for the update
endpoint method (the update(...) method at line ~58). Also add the appropriate
import for `@Valid` (javax.validation.Valid or jakarta.validation.Valid to match
project Jakarta/Javax usage). Ensure DTO fields already have validation
annotations so this takes effect.
| // 삭제 | ||
| @DeleteMapping("/{tagId}") | ||
| public ResponseEntity<ApiResponse<Void>> delete(@PathVariable Long tagId) { | ||
| memberTagService.delete(tagId); | ||
| return ResponseEntity.ok(ApiResponse.success(null)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
삭제 응답으로 204 No Content가 더 적절합니다.
삭제 성공 시 반환할 데이터가 없으므로 200 OK에 null 바디 대신 204 No Content가 RESTful 컨벤션에 부합합니다.
♻️ 수정 제안
`@DeleteMapping`("/{tagId}")
-public ResponseEntity<ApiResponse<Void>> delete(`@PathVariable` Long tagId) {
+public ResponseEntity<Void> delete(`@PathVariable` Long tagId) {
memberTagService.delete(tagId);
- return ResponseEntity.ok(ApiResponse.success(null));
+ return ResponseEntity.noContent().build();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 삭제 | |
| @DeleteMapping("/{tagId}") | |
| public ResponseEntity<ApiResponse<Void>> delete(@PathVariable Long tagId) { | |
| memberTagService.delete(tagId); | |
| return ResponseEntity.ok(ApiResponse.success(null)); | |
| } | |
| // 삭제 | |
| `@DeleteMapping`("/{tagId}") | |
| public ResponseEntity<Void> delete(`@PathVariable` Long tagId) { | |
| memberTagService.delete(tagId); | |
| return ResponseEntity.noContent().build(); | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/controller/MemberTagController.java`
around lines 64 - 69, Change the delete handler in MemberTagController to return
204 No Content without a response body: update the method signature and return
type from ResponseEntity<ApiResponse<Void>> to ResponseEntity<Void>, keep the
call to memberTagService.delete(tagId), and return
ResponseEntity.noContent().build() (or
ResponseEntity.status(HttpStatus.NO_CONTENT).build()) instead of
ResponseEntity.ok(ApiResponse.success(null)).
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @AllArgsConstructor | ||
| @Builder | ||
| @Entity | ||
| @Table(name = "member_tag") | ||
| public class MemberTag { | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "member_tag_id") | ||
| private Long memberTagId; | ||
| @Column(name = "owner_member_id") | ||
| private Long ownerMemberId; | ||
| @Column(name = "tag_name") | ||
| private String tagName; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
ownerMemberId와 tagName 컬럼에 nullable = false 제약 조건 누락
두 필드 모두 비즈니스 로직상 필수값이지만 JPA @Column에 nullable = false가 설정되지 않아 DDL 생성 시 DB 레벨에서 NULL이 허용됩니다. 또한 tagName에 길이 제한(length)을 추가하는 것도 권장합니다.
♻️ 수정 제안
- `@Column`(name = "owner_member_id")
+ `@Column`(name = "owner_member_id", nullable = false)
private Long ownerMemberId;
- `@Column`(name = "tag_name")
+ `@Column`(name = "tag_name", nullable = false, length = 50)
private String tagName;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Getter | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| @AllArgsConstructor | |
| @Builder | |
| @Entity | |
| @Table(name = "member_tag") | |
| public class MemberTag { | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| @Column(name = "member_tag_id") | |
| private Long memberTagId; | |
| @Column(name = "owner_member_id") | |
| private Long ownerMemberId; | |
| @Column(name = "tag_name") | |
| private String tagName; | |
| `@Getter` | |
| `@NoArgsConstructor`(access = AccessLevel.PROTECTED) | |
| `@AllArgsConstructor` | |
| `@Builder` | |
| `@Entity` | |
| `@Table`(name = "member_tag") | |
| public class MemberTag { | |
| `@Id` | |
| `@GeneratedValue`(strategy = GenerationType.IDENTITY) | |
| `@Column`(name = "member_tag_id") | |
| private Long memberTagId; | |
| `@Column`(name = "owner_member_id", nullable = false) | |
| private Long ownerMemberId; | |
| `@Column`(name = "tag_name", nullable = false, length = 50) | |
| private String tagName; |
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/domain/MemberTag.java` around lines 15 -
29, MemberTag’s ownerMemberId and tagName fields lack DB-level non-null
constraints and tagName has no length limit; update the MemberTag entity by
adding nullable = false to the `@Column` on ownerMemberId and tagName, and add a
sensible length (e.g., length = 100) to the `@Column` on tagName so DDL generation
enforces non-null and a max length; ensure you edit the fields named
ownerMemberId and tagName in the MemberTag class.
|
|
||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @AllArgsConstructor |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
@AllArgsConstructor 접근 수준을 PRIVATE으로 제한 권장
@Builder와 함께 사용할 때 @AllArgsConstructor가 public이면 빌더를 우회하여 직접 생성자를 호출할 수 있습니다. @AllArgsConstructor(access = AccessLevel.PRIVATE)로 제한하면 빌더 패턴의 의도를 보호할 수 있습니다.
♻️ 수정 제안
-@AllArgsConstructor
+@AllArgsConstructor(access = AccessLevel.PRIVATE)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @AllArgsConstructor | |
| `@AllArgsConstructor`(access = AccessLevel.PRIVATE) |
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/domain/MemberTag.java` at line 17, The
`@AllArgsConstructor` on MemberTag should be made private to prevent bypassing the
`@Builder`; change the annotation on the MemberTag class to
`@AllArgsConstructor`(access = AccessLevel.PRIVATE) and add the necessary
lombok.AccessLevel import if it's missing so the builder is the only intended
creation path.
| @Getter | ||
| @RequiredArgsConstructor | ||
| public enum TagErrorCode implements ErrorCode { | ||
| Tag_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다."); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Enum 상수 네이밍 컨벤션 위반: Tag_NOT_FOUND → TAG_NOT_FOUND
Java enum 상수는 UPPER_SNAKE_CASE를 따라야 합니다. Tag_NOT_FOUND는 대소문자가 혼합되어 있어 컨벤션에 맞지 않습니다. TagException.java에서의 참조도 함께 수정해야 합니다.
♻️ 수정 제안
- Tag_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다.");
+ TAG_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다.");🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java` at line 11, The
enum constant in TagErrorCode uses incorrect casing: rename Tag_NOT_FOUND to
TAG_NOT_FOUND in the TagErrorCode enum and update every reference to it (for
example in TagException.java) to use TAG_NOT_FOUND; ensure imports/usages still
compile and adjust any switch/statements or tests that expect the old name.
| public Long create(Long ownerMemberId, String tagName) { | ||
| validateTagName(tagName); | ||
|
|
||
| if (memberTagJpaDao.existsByOwnerMemberIdAndTagName(ownerMemberId, tagName)) { | ||
| throw new IllegalArgumentException("Tag already exists."); | ||
| } | ||
|
|
||
| MemberTag tag = MemberTag.builder() | ||
| .ownerMemberId(ownerMemberId) | ||
| .tagName(tagName) | ||
| .build(); | ||
|
|
||
| return memberTagJpaDao.save(tag).getMemberTagId(); | ||
| } |
There was a problem hiding this comment.
create에서 중복 태그 시 IllegalArgumentException 사용은 에러 처리 일관성을 깨뜨립니다.
get/update에서는 TagException.NotFound를 통해 적절한 HTTP 상태 코드가 반환되지만, 중복 태그의 경우 IllegalArgumentException이 글로벌 예외 핸들러에서 어떻게 처리되는지에 따라 500 에러로 노출될 수 있습니다. TagErrorCode에 TAG_ALREADY_EXISTS(HttpStatus.CONFLICT, "T002", "이미 존재하는 태그입니다.")를 추가하고 전용 예외를 사용하는 것을 권장합니다.
또한 existsBy... 확인과 save 사이에 TOCTOU 레이스 컨디션이 존재합니다. 동시 요청 시 중복 태그가 생성될 수 있으므로, DB에 (owner_member_id, tag_name) 유니크 제약 조건을 추가하고 DataIntegrityViolationException을 catch하는 방식이 더 안전합니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/service/MemberTagServiceImpl.java` around
lines 19 - 32, Replace the IllegalArgumentException in
MemberTagServiceImpl.create with a consistent TagException using a new
TagErrorCode entry TAG_ALREADY_EXISTS(HttpStatus.CONFLICT, "T002", "이미 존재하는
태그입니다."); remove the pre-check race-prone existsByOwnerMemberIdAndTagName or
keep it only for fast-fail but do not rely on it as the sole guard; add a unique
DB constraint on (owner_member_id, tag_name) and wrap memberTagJpaDao.save(tag)
to catch DataIntegrityViolationException and rethrow TagException with
TagErrorCode.TAG_ALREADY_EXISTS so concurrent requests map to the same HTTP 409
behavior (use the existing TagException pattern used by TagException.NotFound
for structure).
| @Override | ||
| @Transactional | ||
| public void delete(Long memberTagId) { | ||
| if (!memberTagJpaDao.existsById(memberTagId)) { | ||
| return; | ||
| } | ||
| memberTagJpaDao.deleteById(memberTagId); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file and surrounding context
cd /tmp && find . -type f -name "MemberTagServiceImpl.java" 2>/dev/null | head -5Repository: Searchweb-Dev/Searchweb-Back
Length of output: 54
🌐 Web query:
Spring Data JPA 3.x deleteById behavior non-existent entity
💡 Result:
In Spring Data JPA 3.x, the intended contract for CrudRepository.deleteById(id) is:
- If the entity does not exist, the call is silently ignored (no exception). [1]
However, Spring Data JPA 3.0.0 (and early 3.0.x) did not follow that contract: SimpleJpaRepository.deleteById looked up the entity and, if missing, threw EmptyResultDataAccessException (see the referenced implementation pattern and discussion). [2], [3]
That mismatch was tracked as a Spring Data JPA bug. [3] Community notes (and common guidance) indicate the behavior was fixed in Spring Data JPA 3.0.2+, so later 3.x lines (e.g., 3.1/3.2) should align with the “silently ignored” contract. [4]
Sources: [1] CrudRepository Javadoc (current) • [2] Spring Data Commons issue documenting the exception behavior/implementation • [3] Spring Data JPA bug issue #2719 • [4] Stack Overflow answer referencing the fix version.
🏁 Script executed:
# Check for build configuration to understand the Spring Data JPA version
find . -name "pom.xml" -o -name "build.gradle" -o -name "build.gradle.kts" | head -5 | xargs cat 2>/dev/null | grep -i "spring-data-jpa\|spring.data" -A 2 -B 2 | head -40Repository: Searchweb-Dev/Searchweb-Back
Length of output: 54
🏁 Script executed:
# Search for build configuration files more comprehensively
find . -type f \( -name "pom.xml" -o -name "build.gradle" -o -name "build.gradle.kts" \) 2>/dev/nullRepository: Searchweb-Dev/Searchweb-Back
Length of output: 86
🏁 Script executed:
# Also check for the MemberTagServiceImpl file location
find . -type f -name "MemberTagServiceImpl.java" 2>/dev/nullRepository: Searchweb-Dev/Searchweb-Back
Length of output: 143
🏁 Script executed:
# Look for gradle or maven wrapper to understand project structure
ls -la | grep -E "gradlew|mvnw|pom|gradle"Repository: Searchweb-Dev/Searchweb-Back
Length of output: 372
🏁 Script executed:
# Check the build.gradle file for Spring Data JPA version
cat -n build.gradleRepository: Searchweb-Dev/Searchweb-Back
Length of output: 2007
🏁 Script executed:
# Also read the MemberTagServiceImpl file to see the complete context
cat -n ./src/main/java/com/web/SearchWeb/tag/service/MemberTagServiceImpl.javaRepository: Searchweb-Dev/Searchweb-Back
Length of output: 2687
🏁 Script executed:
# Search for test files related to MemberTagService
find . -type f -name "*MemberTagService*Test*.java" 2>/dev/nullRepository: Searchweb-Dev/Searchweb-Back
Length of output: 54
🏁 Script executed:
# Search for any tests in the test directory
find . -type f -path "*/test/*" -name "*MemberTag*" 2>/dev/nullRepository: Searchweb-Dev/Searchweb-Back
Length of output: 54
🏁 Script executed:
# Check if there's any interface definition that might explain the contract
find . -type f -name "MemberTagService.java" 2>/dev/nullRepository: Searchweb-Dev/Searchweb-Back
Length of output: 139
🏁 Script executed:
# Read the MemberTagService interface
cat -n ./src/main/java/com/web/SearchWeb/tag/service/MemberTagService.javaRepository: Searchweb-Dev/Searchweb-Back
Length of output: 573
🏁 Script executed:
# Search for any test classes in the project
find . -type f -path "*/test/java/*" -name "*.java" 2>/dev/null | head -20Repository: Searchweb-Dev/Searchweb-Back
Length of output: 136
🏁 Script executed:
# Look for any comments or documentation in the codebase about the delete behavior
rg "delete.*idempotent|idempotent.*delete|silent.*delete|delete.*silent" -iRepository: Searchweb-Dev/Searchweb-Back
Length of output: 54
delete 메서드의 설계 의도와 기타 메서드와의 일관성 확인 필요
현재 코드는 Spring Data JPA 3.3.1에서 deleteById()가 존재하지 않는 엔티티에 대해 예외를 던지지 않는다는 점을 활용해 existsById 체크를 통한 멱등성 삭제를 구현하고 있습니다. 다만 몇 가지 검토 사항이 있습니다:
-
불필요한 쿼리:
existsById+deleteById로 인해 두 번의 데이터베이스 쿼리가 발생합니다.deleteById만 호출해도 같은 결과를 얻을 수 있습니다. -
설계 일관성:
get()과update()메서드는 존재하지 않는 엔티티에 대해TagException.NotFound를 던지지만,delete()메서드는 조용히 성공합니다. 이러한 불일치가 의도된 것인지, 아니면 명시적으로 문서화되어야 하는지 확인이 필요합니다.
♻️ 간소화 제안
`@Override`
`@Transactional`
public void delete(Long memberTagId) {
- if (!memberTagJpaDao.existsById(memberTagId)) {
- return;
- }
memberTagJpaDao.deleteById(memberTagId);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| @Transactional | |
| public void delete(Long memberTagId) { | |
| if (!memberTagJpaDao.existsById(memberTagId)) { | |
| return; | |
| } | |
| memberTagJpaDao.deleteById(memberTagId); | |
| } | |
| `@Override` | |
| `@Transactional` | |
| public void delete(Long memberTagId) { | |
| memberTagJpaDao.deleteById(memberTagId); | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/service/MemberTagServiceImpl.java` around
lines 58 - 65, 현재 delete(Long memberTagId)는 존재하지 않는 엔티티에 대해 조용히 성공(return)하는데,
get()과 update()가 존재하지 않을 때 TagException.NotFound를 던지는 것과 일관되지 않습니다; 수정 방법은
delete()에서 memberTagJpaDao.existsById(memberTagId) 체크 시 단순 return을 제거하고 엔티티가 없으면
TagException.NotFound를 던지도록 변경(또는 설계상 조용한 삭제가 의도라면 해당 의도를 코드 주석 또는 문서화)하고, 중복
쿼리를 줄이려면 필요에 따라 existsById 대신 findById 사용 후 존재하지 않으면 예외를 던지고 존재하면
memberTagJpaDao.deleteById(memberTagId)를 호출하도록 일관된 동작을 구현하세요 (참고 식별자:
delete(Long), memberTagJpaDao.existsById, memberTagJpaDao.deleteById,
TagException.NotFound, get(), update()).
| private void validateTagName(String tagName) { | ||
| if (tagName == null || tagName.isBlank()) { | ||
| throw new IllegalArgumentException("tagName must not be blank"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
validateTagName — 도메인 엔티티 MemberTag.changeName과 검증 로직이 중복됩니다.
서비스와 엔티티 양쪽에서 동일한 null/blank 검증을 수행합니다. 도메인 엔티티의 검증은 도메인 무결성 보호를 위해 유지하되, 서비스 레이어에서는 별도 예외 타입(예: TagErrorCode 기반)으로 더 의미 있는 에러 응답을 반환하는 것이 이상적입니다. 현재 둘 다 IllegalArgumentException을 던지므로 에러 응답 측면에서 차별화가 없습니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/tag/service/MemberTagServiceImpl.java` around
lines 67 - 71, The service method validateTagName currently duplicates
null/blank checks already enforced by the domain entity MemberTag.changeName and
throws IllegalArgumentException; remove the duplicated validation or make it a
thin delegator and instead map domain validation failures to a meaningful
service-level exception using your TagErrorCode (e.g., catch the domain
exception from MemberTag.changeName and throw a TagException/Error with the
appropriate TagErrorCode), ensuring the entity keeps its own validation while
the service returns domain-specific errors; update references to validateTagName
and MemberTag.changeName accordingly so the service no longer throws
IllegalArgumentException for tag-name problems but returns TagErrorCode-based
exceptions.
2 participants
💡 이슈
resolve {SW-40}
🤩 개요
PR의 개요를 적어주세요.
Tag의 기본 기능 구현
🧑💻 작업 사항
Tag의 CRUD 구현을 규칙에 맞게 구현
비지니스 로직을 제외한 상태여서 추가로 보완 필요
📖 참고 사항
공유할 내용, 레퍼런스, 추가로 발생할 것으로 예상되는 이슈, 스크린샷 등을 넣어 주세요.
Summary by CodeRabbit
릴리스 노트
새로운 기능
의존성 제거