Skip to content
Merged
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
1 change: 0 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springframework.boot:spring-boot-starter-oauth2-client'
implementation 'me.paulschwarz:spring-dotenv:4.0.0'
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'

compileOnly 'org.projectlombok:lombok'
runtimeOnly 'org.postgresql:postgresql'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package com.web.SearchWeb.tag.controller;

import com.web.SearchWeb.config.ApiResponse;
import com.web.SearchWeb.tag.controller.dto.MemberTagDto;
import com.web.SearchWeb.tag.domain.MemberTag;
import com.web.SearchWeb.tag.service.MemberTagService;
import java.util.List;
import java.util.stream.Collectors;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequiredArgsConstructor
@RequestMapping("/api/tags")
public class MemberTagController {

private final MemberTagService memberTagService;

// 생성
@PostMapping
public ResponseEntity<ApiResponse<Long>> create(@RequestBody MemberTagDto.CreateRequest req) {
Copy link

Choose a reason for hiding this comment

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

🛠️ 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.

Long tagId = memberTagService.create(req.getOwnerMemberId(), req.getTagName());
return ResponseEntity
.status(HttpStatus.CREATED)
.body(ApiResponse.success(tagId));
}
Comment on lines +28 to +35
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

보안 위험: 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).


// 단건 조회
@GetMapping("/{tagId}")
public ResponseEntity<ApiResponse<MemberTagDto.Response>> get(@PathVariable Long tagId) {
MemberTag tag = memberTagService.get(tagId);
return ResponseEntity.ok(ApiResponse.success(MemberTagDto.Response.from(tag)));
}

// 목록 조회
@GetMapping("/owners/{ownerMemberId}")
public ResponseEntity<ApiResponse<List<MemberTagDto.Response>>> list(@PathVariable Long ownerMemberId) {
List<MemberTagDto.Response> responses = memberTagService.listByOwner(ownerMemberId)
.stream()
.map(MemberTagDto.Response::from)
.collect(Collectors.toList());
return ResponseEntity.ok(ApiResponse.success(responses));
}

// 수정
@PutMapping("/{tagId}")
public ResponseEntity<ApiResponse<Void>> update(
@PathVariable Long tagId,
@RequestBody MemberTagDto.UpdateRequest req
) {
memberTagService.update(tagId, req.getTagName());
return ResponseEntity.ok(ApiResponse.success(null));
}

// 삭제
@DeleteMapping("/{tagId}")
public ResponseEntity<ApiResponse<Void>> delete(@PathVariable Long tagId) {
memberTagService.delete(tagId);
return ResponseEntity.ok(ApiResponse.success(null));
}
Comment on lines +64 to +69
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

삭제 응답으로 204 No Content가 더 적절합니다.

삭제 성공 시 반환할 데이터가 없으므로 200 OKnull 바디 대신 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.

Suggested change
// 삭제
@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)).

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.web.SearchWeb.tag.controller.dto;

import com.web.SearchWeb.tag.domain.MemberTag;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

public class MemberTagDto {

@Getter
@NoArgsConstructor
public static class CreateRequest {
private Long ownerMemberId;
private String tagName;
}

@Getter
@NoArgsConstructor
public static class UpdateRequest {
private String tagName;
}
Comment on lines +11 to +22
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

요청 DTO에 Bean Validation 어노테이션 추가 권장

CreateRequestUpdateRequest@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.

Suggested change
@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.


@Getter
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public static class Response {
private Long tagId;
private Long ownerMemberId;
private String tagName;

@Builder
private Response(Long tagId, Long ownerMemberId, String tagName) {
this.tagId = tagId;
this.ownerMemberId = ownerMemberId;
this.tagName = tagName;
}

public static Response from(MemberTag tag) {
return Response.builder()
.tagId(tag.getMemberTagId())
.ownerMemberId(tag.getOwnerMemberId())
.tagName(tag.getTagName())
.build();
}
}
}
15 changes: 15 additions & 0 deletions src/main/java/com/web/SearchWeb/tag/dao/MemberTagJpaDao.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.web.SearchWeb.tag.dao;

import com.web.SearchWeb.tag.domain.MemberTag;
import java.util.List;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;

public interface MemberTagJpaDao extends JpaRepository<MemberTag, Long> {

List<MemberTag> findAllByOwnerMemberId(Long ownerMemberId);

Optional<MemberTag> findByOwnerMemberIdAndTagName(Long ownerMemberId, String tagName);

boolean existsByOwnerMemberIdAndTagName(Long ownerMemberId, String tagName);
}
37 changes: 37 additions & 0 deletions src/main/java/com/web/SearchWeb/tag/domain/MemberTag.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.web.SearchWeb.tag.domain;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@AllArgsConstructor
Copy link

Choose a reason for hiding this comment

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

🧹 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.

Suggested change
@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.

@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;
Comment on lines +15 to +29
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

ownerMemberIdtagName 컬럼에 nullable = false 제약 조건 누락

두 필드 모두 비즈니스 로직상 필수값이지만 JPA @Columnnullable = 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.

Suggested change
@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.


public void changeName(String tagName) {
if (tagName == null || tagName.isBlank()) {
throw new IllegalArgumentException("tagName must not be blank");
}
this.tagName = tagName;
}
}
15 changes: 15 additions & 0 deletions src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.web.SearchWeb.tag.error;

import com.web.SearchWeb.config.ErrorCode;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;

@Getter
@RequiredArgsConstructor
public enum TagErrorCode implements ErrorCode {
Tag_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다.");
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Enum 상수 네이밍 컨벤션 위반: Tag_NOT_FOUNDTAG_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.

private final HttpStatus status;
private final String code;
private final String message;
}
19 changes: 19 additions & 0 deletions src/main/java/com/web/SearchWeb/tag/error/TagException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.web.SearchWeb.tag.error;

import com.web.SearchWeb.config.BusinessException;
import com.web.SearchWeb.config.ErrorCode;
import lombok.Getter;

@Getter
public class TagException extends BusinessException {

public TagException(ErrorCode errorCode) {
super(errorCode);
}
public static class NotFound extends TagException {
public NotFound() {
super(TagErrorCode.Tag_NOT_FOUND);
}
}

}
17 changes: 17 additions & 0 deletions src/main/java/com/web/SearchWeb/tag/service/MemberTagService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.web.SearchWeb.tag.service;

import com.web.SearchWeb.tag.domain.MemberTag;
import java.util.List;

public interface MemberTagService {

Long create(Long ownerMemberId, String tagName);

MemberTag get(Long memberTagId);

List<MemberTag> listByOwner(Long ownerMemberId);

void update(Long memberTagId, String tagName);

void delete(Long memberTagId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package com.web.SearchWeb.tag.service;

import com.web.SearchWeb.tag.dao.MemberTagJpaDao;
import com.web.SearchWeb.tag.domain.MemberTag;
import com.web.SearchWeb.tag.error.TagException;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class MemberTagServiceImpl implements MemberTagService {

private final MemberTagJpaDao memberTagJpaDao;

@Override
@Transactional
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();
}
Comment on lines +19 to +32
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

create에서 중복 태그 시 IllegalArgumentException 사용은 에러 처리 일관성을 깨뜨립니다.

get/update에서는 TagException.NotFound를 통해 적절한 HTTP 상태 코드가 반환되지만, 중복 태그의 경우 IllegalArgumentException이 글로벌 예외 핸들러에서 어떻게 처리되는지에 따라 500 에러로 노출될 수 있습니다. TagErrorCodeTAG_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(readOnly = true)
public MemberTag get(Long memberTagId) {
return memberTagJpaDao.findById(memberTagId)
.orElseThrow(TagException.NotFound::new);
}

@Override
@Transactional(readOnly = true)
public List<MemberTag> listByOwner(Long ownerMemberId) {
return memberTagJpaDao.findAllByOwnerMemberId(ownerMemberId);
}

@Override
@Transactional
public void update(Long memberTagId, String tagName) {
validateTagName(tagName);

MemberTag tag = memberTagJpaDao.findById(memberTagId)
.orElseThrow(TagException.NotFound::new);

tag.changeName(tagName);
}

@Override
@Transactional
public void delete(Long memberTagId) {
if (!memberTagJpaDao.existsById(memberTagId)) {
return;
}
memberTagJpaDao.deleteById(memberTagId);
}
Comment on lines +58 to +65
Copy link

Choose a reason for hiding this comment

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

🧹 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 -5

Repository: 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 -40

Repository: 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/null

Repository: 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/null

Repository: 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.gradle

Repository: 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.java

Repository: 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/null

Repository: 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/null

Repository: 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/null

Repository: 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.java

Repository: 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 -20

Repository: 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" -i

Repository: Searchweb-Dev/Searchweb-Back

Length of output: 54


delete 메서드의 설계 의도와 기타 메서드와의 일관성 확인 필요

현재 코드는 Spring Data JPA 3.3.1에서 deleteById()가 존재하지 않는 엔티티에 대해 예외를 던지지 않는다는 점을 활용해 existsById 체크를 통한 멱등성 삭제를 구현하고 있습니다. 다만 몇 가지 검토 사항이 있습니다:

  1. 불필요한 쿼리: existsById + deleteById로 인해 두 번의 데이터베이스 쿼리가 발생합니다. deleteById만 호출해도 같은 결과를 얻을 수 있습니다.

  2. 설계 일관성: 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.

Suggested change
@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");
}
}
Comment on lines +67 to +71
Copy link

Choose a reason for hiding this comment

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

🧹 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.

}