Conversation
WalkthroughAdds “user tag update” capability: a new PATCH /v1/user-tags endpoint with TagRenameRequest, service update logic, repository finder, new error codes/types, and comprehensive tests. Updates user-facing and error-code documentation to include the update API. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant UC as UserTagController
participant US as UserTagService
participant UR as UserRepository
participant TR as TagRepository
participant UTR as UserTagRepository
participant TReg as TagRegister
C->>UC: PATCH /v1/user-tags {currentTagId, newTagName}
UC->>US: update(loginUser, currentTagId, newTagName)
US->>UR: findById(loginUser.id)
alt user not found
US-->>UC: throw USER_NOT_FOUND
UC-->>C: 404/400 with error
end
US->>TR: findById(currentTagId)
alt tag not found
US-->>UC: throw TAG_NOT_FOUND
UC-->>C: 404 with error
end
US->>UTR: findByUserAndTag(user, currentTag)
alt user-tag not found
US-->>UC: throw USER_TAG_NOT_FOUND
UC-->>C: 404 with error
end
US->>TReg: resolveOrCreate(newTagName)
US->>US: validateDuplicateUserTag(user, newTag)
alt duplicate exists
US-->>UC: throw USER_TAG_ALREADY_EXISTS
UC-->>C: 400 with error
else proceed
US->>UTR: delete(userTag)
US->>UTR: save(new UserTag(user, newTag))
US-->>UC: TagResponse(newTag)
UC-->>C: 200 OK {data: {id, name}}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java (1)
43-44: Avoid hard-coded business limits in messages.EXCEED_MAX_USER_TAG_COUNT bakes in “30”. Consider a generic message or sourcing the limit from a single constant to prevent drift between validation and messaging; also confirm ErrorType mapping to NOT_FOUND_USER_TAG is in place.
capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTagRepository.java (1)
3-4: Finder addition LGTM; verify uniqueness and race-safety.Ensure a DB-level unique constraint on (user_id, tag_id) exists to backstop validateDuplicateUserTag and prevent TOCTOU duplicates under concurrency. If absent, add it and map DataIntegrityViolationException to the appropriate error code.
Also applies to: 11-12
capturecat-core/src/docs/asciidoc/user.adoc (1)
52-60: Update API docs read well; reflect field constraints.Please document request-fields constraints (non-blank, length, etc.) to match the DTO validations and keep create/update sections aligned on naming and examples.
capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java (1)
26-31: Optional: align request style between create and update.
Create uses query param, update uses JSON body. Consider using a body for both for consistency and cleaner validation/docs.Also applies to: 33-39
capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (1)
70-93: Make stubbed response match the request to produce coherent docs.
Response currently returns name "java" while sending "spring". Use "spring" to avoid confusing examples in REST Docs.- BDDMockito.given(userTagService.update(any(), anyLong(), anyString())).willReturn(new TagResponse(1L, "java")); + BDDMockito.given(userTagService.update(any(), anyLong(), anyString())).willReturn(new TagResponse(1L, "spring"));capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java (1)
63-64: Prefer in-place update over delete+save.If the aggregate allows, mutate the existing UserTag and rely on JPA dirty checking to avoid a transient gap and extra INSERT.
For example:
- userTagRepository.delete(userTag); - userTagRepository.save(UserTag.create(user, newTag)); + userTag.changeTag(newTag); // or a setter; repository save not needed if in same Txcapturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (1)
152-197: Add negative tests for duplicate and idempotent updates.Cover:
- When existsByUserAndTag(user, newTag) is true → USER_TAG_ALREADY_EXISTS.
- When newTagName resolves to currentTag → no delete/save (idempotent).
Example snippets:
@Test void 유저_태그_수정_시_새_태그가_이미_등록되어_있으면_실패한다() { var user = DummyObject.newUser("test"); var currentTag = TagFixture.createTag(1L, "java"); var newTag = TagFixture.createTag(2L, "spring"); given(userRepository.findByUsername(anyString())).willReturn(Optional.of(user)); given(tagRepository.findById(anyLong())).willReturn(Optional.of(currentTag)); given(userTagRepository.findByUserAndTag(eq(user), eq(currentTag))) .willReturn(Optional.of(UserTagFixture.createUserTag(1L, user, currentTag))); given(tagRegister.registerTagsFor(anyString())).willReturn(newTag); given(userTagRepository.existsByUserAndTag(eq(user), eq(newTag))).willReturn(true); assertThatThrownBy(() -> userTagService.update(new LoginUser(user), 1L, "spring")) .isInstanceOf(CoreException.class) .hasMessage(ErrorType.USER_TAG_ALREADY_EXISTS.getCode().getMessage()); verify(userTagRepository, never()).delete(any()); verify(userTagRepository, never()).save(any()); } @Test void 유저_태그_수정_시_같은_태그로_요청하면_idempotent() { var user = DummyObject.newUser("test"); var currentTag = TagFixture.createTag(1L, "java"); given(userRepository.findByUsername(anyString())).willReturn(Optional.of(user)); given(tagRepository.findById(anyLong())).willReturn(Optional.of(currentTag)); given(userTagRepository.findByUserAndTag(eq(user), eq(currentTag))) .willReturn(Optional.of(UserTagFixture.createUserTag(1L, user, currentTag))); // new name resolves to same tag given(tagRegister.registerTagsFor(anyString())).willReturn(currentTag); var response = userTagService.update(new LoginUser(user), 1L, "java"); assertThat(response.id()).isEqualTo(currentTag.getId()); verify(userTagRepository, never()).delete(any()); verify(userTagRepository, never()).save(any()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
capturecat-core/src/docs/asciidoc/error-codes.adoc(1 hunks)capturecat-core/src/docs/asciidoc/user.adoc(1 hunks)capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java(2 hunks)capturecat-core/src/main/java/com/capturecat/core/api/user/dto/TagRenameRequest.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTagRepository.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java(3 hunks)capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java(2 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java(2 hunks)capturecat-core/src/test/java/com/capturecat/core/domain/user/UserTagRepositoryTest.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
capturecat-core/src/test/java/com/capturecat/core/domain/user/UserTagRepositoryTest.java (2)
capturecat-core/src/test/java/com/capturecat/core/DummyObject.java (1)
DummyObject(18-100)capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagFixture.java (1)
TagFixture(5-16)
capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (3)
capturecat-core/src/test/java/com/capturecat/core/DummyObject.java (1)
DummyObject(18-100)capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagFixture.java (1)
TagFixture(5-16)capturecat-core/src/test/java/com/capturecat/core/domain/user/UserTagFixture.java (1)
UserTagFixture(7-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy-dev
🔇 Additional comments (13)
capturecat-core/src/docs/asciidoc/error-codes.adoc (1)
97-99: Docs anchor/include for update user tag look correct.Anchor name matches user.adoc links and the snippet path format is consistent with existing sections.
capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java (2)
25-30: LGTM: update error-code docs cover key failure modes.
Covers duplication, user/tag not found, and missing user-tag mapping for PATCH. Looks consistent with service behavior.
3-8: Imports are correct and scoped to the new test.
Static imports match the error types referenced below.capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java (1)
4-6: New imports are appropriate.
Required for PATCH + body + DTO usage.Also applies to: 13-13
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java (1)
48-50: LGTM: added USER_TAG_NOT_FOUND mapping.
Status/Code/LogLevel are appropriate; enum punctuation updated correctly.capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (1)
6-6: Supporting imports look good.
anyLong,requestFields, andMapare needed for the new test.Also applies to: 13-14, 18-19
capturecat-core/src/test/java/com/capturecat/core/domain/user/UserTagRepositoryTest.java (1)
35-53: LGTM: repository happy-path is verified with flush/clear.
Good coverage of the finder behavior and associations.capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java (1)
31-33: DI wiring looks good.Adding TagRepository and TagRegister as constructor-injected deps is appropriate.
capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (5)
6-6: OK to add anyLong().
10-10: OK to add willDoNothing().Not strictly required for void mocks, but harmless.
26-26: Importing TagRepository for update-path tests is correct.
43-45: Mock wiring looks right.
124-150: The verification script above will reveal whetherupdate(...)maps toTagResponse.from(tag)or tofrom(userTag). Once we confirm which ID is used in the response, we can decide:
- If it returns the Tag’s ID, change the test to assert against
newTag.getId()(and adjust the fixture to avoid coincidental matching).- If it returns the UserTag’s ID, update the service to return
TagResponse.from(newUserTag)(or adjust the contract accordingly).Please review the output to determine the correct fix.
| public record TagRenameRequest(Long currentTagId, String newTagName) { | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden request contract: add Bean Validation and trim input.
Guard against null/blank and whitespace-only names; also prefer enforcing numeric ID validity at the DTO boundary.
package com.capturecat.core.api.user.dto;
-public record TagRenameRequest(Long currentTagId, String newTagName) {
-}
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Positive;
+
+public record TagRenameRequest(
+ @NotNull @Positive Long currentTagId,
+ @NotBlank String newTagName
+) {
+ // Normalize input to avoid " duplicate by trailing spaces " issues
+ public TagRenameRequest {
+ if (newTagName != null) {
+ newTagName = newTagName.trim();
+ }
+ }
+}Follow-up: mirror any tag-name size or charset constraints used in Tag creation (e.g., @SiZe, @pattern) for consistency, and ensure the controller uses @Valid on the request body.
📝 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.
| public record TagRenameRequest(Long currentTagId, String newTagName) { | |
| } | |
| package com.capturecat.core.api.user.dto; | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.NotNull; | |
| import jakarta.validation.constraints.Positive; | |
| public record TagRenameRequest( | |
| @NotNull @Positive Long currentTagId, | |
| @NotBlank String newTagName | |
| ) { | |
| // Normalize input to avoid " duplicate by trailing spaces " issues | |
| public TagRenameRequest { | |
| if (newTagName != null) { | |
| newTagName = newTagName.trim(); | |
| } | |
| } | |
| } |
| @PatchMapping | ||
| public ApiResponse<TagResponse> update(@AuthenticationPrincipal LoginUser loginUser, | ||
| @RequestBody TagRenameRequest request) { | ||
| TagResponse response = userTagService.update(loginUser, request.currentTagId(), request.newTagName()); | ||
|
|
||
| return ApiResponse.success(response); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add bean validation to prevent null/blank updates.
Without validation, null/blank newTagName or null currentTagId can slip through to the service. Add @Valid here and constraints on the DTO.
Apply to controller:
@@
-import org.springframework.web.bind.annotation.PatchMapping;
+import org.springframework.web.bind.annotation.PatchMapping;
+import jakarta.validation.Valid;
@@
- public ApiResponse<TagResponse> update(@AuthenticationPrincipal LoginUser loginUser,
- @RequestBody TagRenameRequest request) {
+ public ApiResponse<TagResponse> update(@AuthenticationPrincipal LoginUser loginUser,
+ @Valid @RequestBody TagRenameRequest request) {And in TagRenameRequest (DTO file):
package com.capturecat.core.api.user.dto;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
public record TagRenameRequest(
@NotNull Long currentTagId,
@NotBlank @Size(max = 50) String newTagName
) {}🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java
around lines 33-39, the controller method accepts TagRenameRequest without
validation; add a javax/jakarta validation trigger by annotating the
@RequestBody parameter with @Valid so the framework enforces constraints. Update
the TagRenameRequest DTO
(capturecat-core/src/main/java/com/capturecat/core/api/user/dto/TagRenameRequest.java)
to annotate the record components with @NotNull for currentTagId and @NotBlank
plus @Size(max = 50) for newTagName (using jakarta.validation constraints as
your project uses jakarta.*), and ensure the DTO imports the validation
annotations; this will prevent null/blank values from reaching the service.
| @Transactional | ||
| public TagResponse update(LoginUser loginUser, Long currentTagId, String newTagName) { | ||
| User user = userRepository.findByUsername(loginUser.getUsername()) | ||
| .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND)); | ||
| Tag currentTag = tagRepository.findById(currentTagId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.TAG_NOT_FOUND)); | ||
| UserTag userTag = userTagRepository.findByUserAndTag(user, currentTag) | ||
| .orElseThrow(() -> new CoreException(ErrorType.USER_TAG_NOT_FOUND)); | ||
| Tag newTag = tagRegister.registerTagsFor(newTagName); | ||
|
|
||
| validateDuplicateUserTag(user, newTag); | ||
|
|
||
| userTagRepository.delete(userTag); | ||
| userTagRepository.save(UserTag.create(user, newTag)); | ||
|
|
||
| return TagResponse.from(newTag); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden update: handle idempotent rename and race-to-duplicate.
- Treat rename-to-same-tag as a no-op to avoid unnecessary delete/save.
- Mirror create(): catch DataIntegrityViolationException to map late-appearing duplicates to USER_TAG_ALREADY_EXISTS.
Apply:
@Transactional
public TagResponse update(LoginUser loginUser, Long currentTagId, String newTagName) {
- User user = userRepository.findByUsername(loginUser.getUsername())
- .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND));
- Tag currentTag = tagRepository.findById(currentTagId)
- .orElseThrow(() -> new CoreException(ErrorType.TAG_NOT_FOUND));
- UserTag userTag = userTagRepository.findByUserAndTag(user, currentTag)
- .orElseThrow(() -> new CoreException(ErrorType.USER_TAG_NOT_FOUND));
- Tag newTag = tagRegister.registerTagsFor(newTagName);
-
- validateDuplicateUserTag(user, newTag);
-
- userTagRepository.delete(userTag);
- userTagRepository.save(UserTag.create(user, newTag));
-
- return TagResponse.from(newTag);
+ try {
+ User user = userRepository.findByUsername(loginUser.getUsername())
+ .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND));
+ Tag currentTag = tagRepository.findById(currentTagId)
+ .orElseThrow(() -> new CoreException(ErrorType.TAG_NOT_FOUND));
+ UserTag userTag = userTagRepository.findByUserAndTag(user, currentTag)
+ .orElseThrow(() -> new CoreException(ErrorType.USER_TAG_NOT_FOUND));
+ Tag newTag = tagRegister.registerTagsFor(newTagName);
+
+ // idempotent no-op
+ if (currentTag.getId().equals(newTag.getId())) {
+ return TagResponse.from(currentTag);
+ }
+
+ validateDuplicateUserTag(user, newTag);
+
+ userTagRepository.delete(userTag);
+ userTagRepository.save(UserTag.create(user, newTag));
+
+ return TagResponse.from(newTag);
+ } catch (DataIntegrityViolationException ex) {
+ throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS);
+ }
}📝 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.
| @Transactional | |
| public TagResponse update(LoginUser loginUser, Long currentTagId, String newTagName) { | |
| User user = userRepository.findByUsername(loginUser.getUsername()) | |
| .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND)); | |
| Tag currentTag = tagRepository.findById(currentTagId) | |
| .orElseThrow(() -> new CoreException(ErrorType.TAG_NOT_FOUND)); | |
| UserTag userTag = userTagRepository.findByUserAndTag(user, currentTag) | |
| .orElseThrow(() -> new CoreException(ErrorType.USER_TAG_NOT_FOUND)); | |
| Tag newTag = tagRegister.registerTagsFor(newTagName); | |
| validateDuplicateUserTag(user, newTag); | |
| userTagRepository.delete(userTag); | |
| userTagRepository.save(UserTag.create(user, newTag)); | |
| return TagResponse.from(newTag); | |
| } | |
| @Transactional | |
| public TagResponse update(LoginUser loginUser, Long currentTagId, String newTagName) { | |
| try { | |
| User user = userRepository.findByUsername(loginUser.getUsername()) | |
| .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND)); | |
| Tag currentTag = tagRepository.findById(currentTagId) | |
| .orElseThrow(() -> new CoreException(ErrorType.TAG_NOT_FOUND)); | |
| UserTag userTag = userTagRepository.findByUserAndTag(user, currentTag) | |
| .orElseThrow(() -> new CoreException(ErrorType.USER_TAG_NOT_FOUND)); | |
| Tag newTag = tagRegister.registerTagsFor(newTagName); | |
| // idempotent no-op: if renaming to the same tag, just return it | |
| if (currentTag.getId().equals(newTag.getId())) { | |
| return TagResponse.from(currentTag); | |
| } | |
| validateDuplicateUserTag(user, newTag); | |
| userTagRepository.delete(userTag); | |
| userTagRepository.save(UserTag.create(user, newTag)); | |
| return TagResponse.from(newTag); | |
| } catch (DataIntegrityViolationException ex) { | |
| throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java
around lines 51 to 67, the update method should be hardened to treat renames to
the same tag as a no-op and to handle race-to-duplicate situations: after
resolving user, currentTag and newTag, if newTag equals currentTag simply return
TagResponse.from(currentTag) (no delete/save); otherwise perform the delete and
then save inside a try/catch that catches DataIntegrityViolationException and
throws new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS) to mirror create();
ensure the transaction boundary remains and only map the integrity exception to
the proper error.
2 participants
📌 관련 이슈 (필수)
📝 작업 내용 (필수)
유저 태그 수정 API를 구현했습니다.
💬 리뷰 참고 사항 (선택)
Summary by CodeRabbit
New Features
Documentation