feat: 유저 태그 도메인 설계 및 유저 태그 생성 API #121
Conversation
WalkthroughAdds Flyway PostgreSQL dependency; introduces initial and user_tag DB migrations; implements UserTag domain (entity, repository), TagRegister single-tag registration, UserTagService, UserTagController, error codes/types, tests/fixtures, and API/docs; disables Flyway during tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Controller as UserTagController
participant Service as UserTagService
participant UserRepo as UserRepository
participant TagRegister as TagRegister
participant UserTagRepo as UserTagRepository
participant DB as Database
Client->>Controller: POST /v1/user-tags?tagName=java (Bearer)
Controller->>Service: create(loginUser, "java")
Service->>UserRepo: findByUsername(loginUser.username)
UserRepo-->>Service: Optional<User>
Service->>TagRegister: registerTagsFor("java")
TagRegister->>DB: select/insert tag by name
DB-->>TagRegister: Tag
Service->>UserTagRepo: existsByUserAndTag(user, tag)
UserTagRepo-->>Service: boolean
alt not exists and count < 30
Service->>UserTagRepo: save(new UserTag(user, tag))
UserTagRepo->>DB: insert user_tag
DB-->>UserTagRepo: persisted UserTag
Service-->>Controller: TagResponse
else duplicate or limit exceeded
Service-->>Controller: throw CoreException (mapped ErrorType)
end
Controller-->>Client: ApiResponse<TagResponse>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
✨ 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 (5)
capturecat-core/src/main/resources/db/migration/V1__init.sql (5)
1-14: users.id should be identity for consistency and easier inserts.Other tables mostly use identity. Recommend aligning users.id to identity to avoid manual key management.
Apply:
- id bigint not null primary key, + id bigint generated by default as identity primary key,
37-45: Add FK for withdraw_log.user_id.Currently it’s an unbounded bigint. Add FK for referential integrity.
- user_id bigint not null + user_id bigint not null constraint fk_withdraw_log_user references users
65-71: Enforce tag uniqueness.Tags typically require unique, non-null names.
- name varchar(255), + name varchar(255) not null, + constraint uk_tag_name unique (name),
73-80: Prevent duplicate image-tag pairs and enforce NOT NULL.Join table should be constrained to unique pairs.
- image_id bigint constraint fk6q9wuvp5j846qtqod6xu3gma1 references images, - tag_id bigint constraint fk28yowgjl7oksr7dc0wj7f5il references tag, - created_date timestamp(6) not null, - last_modified_date timestamp(6) not null + image_id bigint not null constraint fk6q9wuvp5j846qtqod6xu3gma1 references images, + tag_id bigint not null constraint fk28yowgjl7oksr7dc0wj7f5il references tag, + created_date timestamp(6) not null, + last_modified_date timestamp(6) not null, + constraint uk_image_tag unique (image_id, tag_id)Optionally add indexes:
create index if not exists idx_image_tag_image_id on image_tag (image_id); create index if not exists idx_image_tag_tag_id on image_tag (tag_id);
82-87: Prevent duplicate bookmarks and enforce NOT NULL.Same rationale as image_tag.
- image_id bigint constraint fkpowbsxsu0qwcon1yoxbsqkw4w references images, - user_id bigint constraint fko4vbqvq5trl11d85bqu5kl870 references users + image_id bigint not null constraint fkpowbsxsu0qwcon1yoxbsqkw4w references images, + user_id bigint not null constraint fko4vbqvq5trl11d85bqu5kl870 references users, + constraint uk_bookmark unique (image_id, user_id)
📜 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 (4)
capturecat-core/build.gradle(1 hunks)capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.java(1 hunks)capturecat-core/src/main/resources/db/V2__create_user_tag.sql(1 hunks)capturecat-core/src/main/resources/db/migration/V1__init.sql(1 hunks)
🔇 Additional comments (2)
capturecat-core/build.gradle (1)
32-32: Flyway dependency addition looks good.Core dependency is sufficient for Spring Boot auto-migration at runtime.
capturecat-core/src/main/resources/db/V2__create_user_tag.sql (1)
1-8: Idempotency and environment safety.Since Flyway runs scripts once, embedding the unique constraint in CREATE TABLE is fine. If you keep separate ALTERs in future, avoid IF NOT EXISTS on constraints (PostgreSQL lacks it); use CREATE TABLE-time constraints or a DO-block guard.
Do you want me to generate a guarded ALTER template for future migrations?
| @Entity | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class UserTag extends BaseTimeEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| private User user; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| private Tag tag; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Map table/columns explicitly and enforce uniqueness at the JPA level.
Avoid reliance on naming strategies; add unique pair constraint and non-null FKs.
package com.capturecat.core.domain.user;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.ManyToOne;
+import jakarta.persistence.JoinColumn;
+import jakarta.persistence.Table;
+import jakarta.persistence.UniqueConstraint;
import lombok.AccessLevel;
import lombok.NoArgsConstructor;
+import lombok.Getter;
import com.capturecat.core.domain.BaseTimeEntity;
import com.capturecat.core.domain.tag.Tag;
@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
-public class UserTag extends BaseTimeEntity {
+@Table(name = "user_tag",
+ uniqueConstraints = @UniqueConstraint(name = "uk_user_tag_user_tag", columnNames = {"user_id", "tag_id"}))
+@Getter
+public class UserTag extends BaseTimeEntity {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
- @ManyToOne(fetch = FetchType.LAZY)
- private User user;
+ @ManyToOne(fetch = FetchType.LAZY, optional = false)
+ @JoinColumn(name = "user_id", nullable = false)
+ private User user;
- @ManyToOne(fetch = FetchType.LAZY)
- private Tag tag;
+ @ManyToOne(fetch = FetchType.LAZY, optional = false)
+ @JoinColumn(name = "tag_id", nullable = false)
+ private Tag tag;
}Optional: add ON DELETE CASCADE in SQL (see V2 comment) to auto-clean rows on user/tag deletion.
📝 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.
| @Entity | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| public class UserTag extends BaseTimeEntity { | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| private Long id; | |
| @ManyToOne(fetch = FetchType.LAZY) | |
| private User user; | |
| @ManyToOne(fetch = FetchType.LAZY) | |
| private Tag tag; | |
| } | |
| package com.capturecat.core.domain.user; | |
| import jakarta.persistence.Entity; | |
| import jakarta.persistence.FetchType; | |
| import jakarta.persistence.GeneratedValue; | |
| import jakarta.persistence.GenerationType; | |
| import jakarta.persistence.Id; | |
| import jakarta.persistence.ManyToOne; | |
| import jakarta.persistence.JoinColumn; | |
| import jakarta.persistence.Table; | |
| import jakarta.persistence.UniqueConstraint; | |
| import lombok.AccessLevel; | |
| import lombok.NoArgsConstructor; | |
| import lombok.Getter; | |
| import com.capturecat.core.domain.BaseTimeEntity; | |
| import com.capturecat.core.domain.tag.Tag; | |
| @Entity | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| @Table( | |
| name = "user_tag", | |
| uniqueConstraints = @UniqueConstraint( | |
| name = "uk_user_tag_user_tag", | |
| columnNames = {"user_id", "tag_id"} | |
| ) | |
| ) | |
| @Getter | |
| public class UserTag extends BaseTimeEntity { | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| private Long id; | |
| @ManyToOne(fetch = FetchType.LAZY, optional = false) | |
| @JoinColumn(name = "user_id", nullable = false) | |
| private User user; | |
| @ManyToOne(fetch = FetchType.LAZY, optional = false) | |
| @JoinColumn(name = "tag_id", nullable = false) | |
| private Tag tag; | |
| } |
🤖 Prompt for AI Agents
In capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.java
around lines 16-29, the entity relies on implicit naming and lacks a JPA-level
uniqueness and non-null constraint for the user/tag relationship; update the
entity mapping to explicitly set the table and column names and enforce
uniqueness and non-null foreign keys by adding a @Table(name="user_tag",
uniqueConstraints=@UniqueConstraint(columnNames={"user_id","tag_id"})), annotate
both relations with @ManyToOne(fetch = FetchType.LAZY) and explicit
@JoinColumn(name="user_id", nullable=false,
foreignKey=@ForeignKey(name="fk_user_tag_user")), and @JoinColumn(name="tag_id",
nullable=false, foreignKey=@ForeignKey(name="fk_user_tag_tag")); if using
Hibernate and you want DB-level ON DELETE CASCADE behavior, add @OnDelete(action
= OnDeleteAction.CASCADE) on the associations or ensure your migration/DDL adds
ON DELETE CASCADE for these FKs.
| create table if not exists user_tag | ||
| ( | ||
| id bigint generated by default as identity primary key, | ||
| user_id bigint constraint fk_user_tag_user references users | ||
| tag_id bigint constraint fk_user_tag_tag references tag, | ||
| created_date timestamp(6) not null, | ||
| last_modified_date timestamp(6) not null, | ||
| ); |
There was a problem hiding this comment.
Place the migration under Flyway’s default location.
V1 is under db/migration, but V2 is under db/. Flyway (Spring Boot default) scans classpath:db/migration; V2 will be skipped.
Move file to:
- capturecat-core/src/main/resources/db/migration/V2__create_user_tag.sql
🤖 Prompt for AI Agents
capturecat-core/src/main/resources/db/V2__create_user_tag.sql lines 1-8: the
migration is placed in db/ instead of Flyway’s default classpath location
db/migration so Flyway will skip V2; move the file to
capturecat-core/src/main/resources/db/migration/V2__create_user_tag.sql (keeping
filename intact) so Spring Boot/Flyway will detect and run it.
Fix SQL syntax and add constraints/indexes.
There’s a missing comma after user_id and a trailing comma before the closing parenthesis. Also add NOT NULL, unique pair, and helpful indexes.
-create table if not exists user_tag
-(
- id bigint generated by default as identity primary key,
- user_id bigint constraint fk_user_tag_user references users
- tag_id bigint constraint fk_user_tag_tag references tag,
- created_date timestamp(6) not null,
- last_modified_date timestamp(6) not null,
-);
+create table if not exists user_tag
+(
+ id bigint generated by default as identity primary key,
+ user_id bigint not null constraint fk_user_tag_user references users on delete cascade,
+ tag_id bigint not null constraint fk_user_tag_tag references tag on delete cascade,
+ created_date timestamp(6) not null,
+ last_modified_date timestamp(6) not null,
+ constraint uk_user_tag_user_tag unique (user_id, tag_id)
+);
+
+create index if not exists idx_user_tag_user_id on user_tag (user_id);
+create index if not exists idx_user_tag_tag_id on user_tag (tag_id);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In capturecat-core/src/main/resources/db/V2__create_user_tag.sql around lines 1
to 8, fix the SQL syntax (add the missing comma after user_id and remove the
trailing comma before the closing parenthesis), mark user_id and tag_id NOT
NULL, add a UNIQUE constraint on (user_id, tag_id), explicitly declare foreign
key constraints referencing the correct parent tables, and add indexes for
user_id and tag_id to improve lookup performance.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
capturecat-core/src/test/resources/application.yml (1)
38-40: Tests won’t catch DDL drift; consider running Flyway in tests.Disabling Flyway means tests use Hibernate DDL, not your migrations. Prefer running migrations so constraints/indexes match prod.
Option A (recommended): enable Flyway and stop Hibernate DDL.
spring: - flyway: - enabled: false + flyway: + enabled: true jpa: hibernate: - ddl-auto: create-drop + ddl-auto: noneOption B: keep disabled, but add schema.sql aligned to migrations.
capturecat-core/src/main/resources/db/migration/V1__init.sql (5)
1-14: users: consider identity PK, unique email, longer password.
- Identity PK eases inserts.
- Email likely needs uniqueness.
- Password 70 chars may be tight for Argon2/other encodings.
-create table if not exists users +create table if not exists users ( - id bigint not null primary key, + id bigint generated by default as identity primary key, nickname varchar(50) not null, - email varchar(50) not null, + email varchar(254) not null unique, username varchar(50) not null unique, - password varchar(70), + password varchar(255), role varchar(255) not null constraint users_role_check check (role in ('ADMIN', 'PREMIUM_USER', 'USER')), tutorial_completed boolean not null, provider varchar(255), social_id varchar(255), created_date timestamp(6) not null, last_modified_date timestamp(6) not null );
53-64: images: index/NOT NULL on user_id; longer file_url.
- FK columns aren’t auto-indexed in Postgres; add index.
- If every image belongs to a user, set NOT NULL.
- 255 may be short for URLs; suggest 1024.
create table if not exists images ( id bigint generated by default as identity primary key, size bigint, file_name varchar(255), - file_url varchar(255), + file_url varchar(1024), capture_date date, created_date timestamp(6) not null, last_modified_date timestamp(6) not null, - user_id bigint constraint fk13ljqfrfwbyvnsdhihwta8cpr references users + user_id bigint not null constraint fk13ljqfrfwbyvnsdhihwta8cpr references users ); + +create index if not exists idx_images_user_id on images (user_id);
65-71: tag: enforce unique (prefer case-insensitive) name.Avoid duplicate tags differing by case.
create unique index if not exists uk_tag_name_lower on tag (lower(name));
1-88: Migrations: avoid IF NOT EXISTS in versioned scripts; use Flyway baseline.Using IF NOT EXISTS can hide drift; Flyway already prevents re-execution. For existing prod schemas, prefer baselining (spring.flyway.baseline-on-migrate=true) over IF NOT EXISTS.
1-88: Timestamps: consider timestamptz.If you need absolute instants across time zones, use
timestamp with time zone.
📜 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 (2)
capturecat-core/src/main/resources/db/migration/V1__init.sql(1 hunks)capturecat-core/src/test/resources/application.yml(1 hunks)
⏰ 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: build
| create table if not exists refresh_token | ||
| ( | ||
| id bigint not null primary key, | ||
| refresh_token_expiration bigint not null, | ||
| expiration varchar(255), | ||
| refresh_token varchar(255), | ||
| username varchar(255) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
refresh_token: ambiguous expirations and missing FK to user.
- Two expiration columns conflict; keep one.
- Add FK to users(username) to prevent orphans.
- Consider identity PK.
-create table if not exists refresh_token
+create table if not exists refresh_token
(
- id bigint not null primary key,
- refresh_token_expiration bigint not null,
- expiration varchar(255),
+ id bigint generated by default as identity primary key,
+ refresh_token_expiration bigint not null,
refresh_token varchar(255),
- username varchar(255)
+ username varchar(50) not null,
+ constraint fk_refresh_token_user_username foreign key (username) references users(username) on delete cascade
);If you truly need two expirations, rename with clear semantics.
🤖 Prompt for AI Agents
In capturecat-core/src/main/resources/db/migration/V1__init.sql around lines
28–35, the refresh_token table currently has two ambiguous expiration columns
and lacks a foreign key to users; remove the redundant expiration varchar column
and keep a single clearly typed expiration (use refresh_token_expiration
bigint), change id to an identity/auto-increment column (e.g., BIGINT GENERATED
ALWAYS AS IDENTITY) instead of a plain bigint PK, ensure username is NOT NULL if
appropriate, and add an explicit foreign key constraint referencing
users(username) (optionally with ON DELETE CASCADE or RESTRICT based on desired
behavior); if you truly need two expirations, rename them to explicit semantic
names (e.g., issued_at_ts and expires_at_ts) instead of keeping generic
"expiration".
| create table if not exists withdraw_log | ||
| ( | ||
| id bigint generated by default as identity primary key, | ||
| created_date timestamp(6) not null, | ||
| last_modified_date timestamp(6) not null, | ||
| image_cleanup_status varchar(255) not null constraint withdraw_log_image_cleanup_status_check check (image_cleanup_status in ('PENDING', 'DONE', 'FAILED')), | ||
| reason text, | ||
| user_id bigint not null | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
withdraw_log: add FK for user_id.
Currently no FK; risks orphaned rows.
create table if not exists withdraw_log
(
id bigint generated by default as identity primary key,
created_date timestamp(6) not null,
last_modified_date timestamp(6) not null,
image_cleanup_status varchar(255) not null constraint withdraw_log_image_cleanup_status_check check (image_cleanup_status in ('PENDING', 'DONE', 'FAILED')),
reason text,
- user_id bigint not null
+ user_id bigint not null constraint fk_withdraw_log_user references users
);Optionally add on delete cascade depending on business rules.
🤖 Prompt for AI Agents
In capturecat-core/src/main/resources/db/migration/V1__init.sql around lines 37
to 45, the withdraw_log table lacks a foreign key on user_id which can lead to
orphaned rows; add a foreign key constraint like "constraint
withdraw_log_user_id_fkey foreign key (user_id) references users(id)" and decide
whether to append "on delete cascade" per business rules (or use "on delete
restrict" if cascading deletes are undesired), ensuring the constraint name and
referenced table/column match your schema.
| create table if not exists image_tag | ||
| ( | ||
| id bigint generated by default as identity primary key, | ||
| image_id bigint constraint fk6q9wuvp5j846qtqod6xu3gma1 references images, | ||
| tag_id bigint constraint fk28yowgjl7oksr7dc0wj7f5il references tag, | ||
| created_date timestamp(6) not null, | ||
| last_modified_date timestamp(6) not null | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
image_tag: prevent duplicates and add indexes.
- Add unique (image_id, tag_id).
- Index both FKs for joins.
- Make FKs NOT NULL.
create table if not exists image_tag
(
id bigint generated by default as identity primary key,
- image_id bigint constraint fk6q9wuvp5j846qtqod6xu3gma1 references images,
- tag_id bigint constraint fk28yowgjl7oksr7dc0wj7f5il references tag,
+ image_id bigint not null constraint fk6q9wuvp5j846qtqod6xu3gma1 references images,
+ tag_id bigint not null constraint fk28yowgjl7oksr7dc0wj7f5il references tag,
created_date timestamp(6) not null,
last_modified_date timestamp(6) not null
);
+
+create unique index if not exists uk_image_tag_image_tag on image_tag (image_id, tag_id);
+create index if not exists idx_image_tag_image_id on image_tag (image_id);
+create index if not exists idx_image_tag_tag_id on image_tag (tag_id);📝 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.
| create table if not exists image_tag | |
| ( | |
| id bigint generated by default as identity primary key, | |
| image_id bigint constraint fk6q9wuvp5j846qtqod6xu3gma1 references images, | |
| tag_id bigint constraint fk28yowgjl7oksr7dc0wj7f5il references tag, | |
| created_date timestamp(6) not null, | |
| last_modified_date timestamp(6) not null | |
| ); | |
| create table if not exists image_tag | |
| ( | |
| id bigint generated by default as identity primary key, | |
| image_id bigint not null constraint fk6q9wuvp5j846qtqod6xu3gma1 references images, | |
| tag_id bigint not null constraint fk28yowgjl7oksr7dc0wj7f5il references tag, | |
| created_date timestamp(6) not null, | |
| last_modified_date timestamp(6) not null | |
| ); | |
| create unique index if not exists uk_image_tag_image_tag on image_tag (image_id, tag_id); | |
| create index if not exists idx_image_tag_image_id on image_tag (image_id); | |
| create index if not exists idx_image_tag_tag_id on image_tag (tag_id); |
🤖 Prompt for AI Agents
In capturecat-core/src/main/resources/db/migration/V1__init.sql around lines 73
to 80, modify the image_tag table definition to make image_id and tag_id NOT
NULL, add a unique constraint on (image_id, tag_id) to prevent duplicate
mappings, and add separate indexes on image_id and tag_id to speed joins;
implement this by altering the CREATE TABLE to declare image_id and tag_id as
NOT NULL, adding UNIQUE(image_id, tag_id) (or a named constraint), and adding
CREATE INDEX statements for image_id and tag_id after the table creation.
| create table if not exists bookmark | ||
| ( | ||
| id bigint generated by default as identity primary key, | ||
| image_id bigint constraint fkpowbsxsu0qwcon1yoxbsqkw4w references images, | ||
| user_id bigint constraint fko4vbqvq5trl11d85bqu5kl870 references users | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
bookmark: prevent duplicates and add indexes.
Enforce one bookmark per (user, image) and speed lookups.
create table if not exists bookmark
(
id bigint generated by default as identity primary key,
image_id bigint constraint fkpowbsxsu0qwcon1yoxbsqkw4w references images,
user_id bigint constraint fko4vbqvq5trl11d85bqu5kl870 references users
);
+
+create unique index if not exists uk_bookmark_user_image on bookmark (user_id, image_id);
+create index if not exists idx_bookmark_image_id on bookmark (image_id);
+create index if not exists idx_bookmark_user_id on bookmark (user_id);🤖 Prompt for AI Agents
In capturecat-core/src/main/resources/db/migration/V1__init.sql around lines 82
to 87, the bookmark table allows duplicate (user_id, image_id) pairs and lacks
indexes for fast lookups; add a UNIQUE constraint on (user_id, image_id) to
enforce one bookmark per user-image and create B-tree indexes on user_id and
image_id (or a composite index on (user_id, image_id) if preferred) to speed
queries; update the migration by altering the CREATE TABLE to include the
UNIQUE(user_id, image_id) clause and add CREATE INDEX statements for user_id and
image_id (or a single composite index) after the table creation, ensuring
constraint and index names follow project naming conventions.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.java (2)
13-24: Expose getters for persistence/domain use.Without accessors, consumers must use reflection or package access. Add Lombok getters.
import lombok.AccessLevel; import lombok.NoArgsConstructor; +import lombok.Getter; @@ -@NoArgsConstructor(access = AccessLevel.PROTECTED) -public class UserTag extends BaseTimeEntity { +@NoArgsConstructor(access = AccessLevel.PROTECTED) +@Getter +public class UserTag extends BaseTimeEntity {
30-36: Make FKs non-null at JPA level and align with DB constraints.Both relationships are optional by default and columns are nullable. Given this is a pure join entity, allow neither to be null to prevent bad rows and to match the migration you intend.
Apply:
import jakarta.persistence.JoinColumn; +import jakarta.persistence.ForeignKey; @@ - @ManyToOne(fetch = FetchType.LAZY) - @JoinColumn(name = "user_id") + @ManyToOne(fetch = FetchType.LAZY, optional = false) + @JoinColumn(name = "user_id", nullable = false, + foreignKey = @ForeignKey(name = "fk_user_tag_user")) private User user; @@ - @ManyToOne(fetch = FetchType.LAZY) - @JoinColumn(name = "tag_id") + @ManyToOne(fetch = FetchType.LAZY, optional = false) + @JoinColumn(name = "tag_id", nullable = false, + foreignKey = @ForeignKey(name = "fk_user_tag_tag")) private Tag tag;
🧹 Nitpick comments (2)
capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.java (1)
26-36: Consider equals/hashCode for collections/Set semantics.If you’ll store UserTag in Sets or compare instances, implement equals/hashCode (prefer id fallback, or business key (user, tag) before persist). If not needed now, skip.
capturecat-core/src/main/resources/db/migration/V2__create_user_tag.sql (1)
8-9: Add indexes for common lookups.Typical queries filter by user_id or tag_id. Add supporting non-unique indexes.
constraint uk_user_tag_user_tag unique (user_id, tag_id) ); +create index if not exists idx_user_tag_user_id on user_tag(user_id); +create index if not exists idx_user_tag_tag_id on user_tag(tag_id);If you remove IF NOT EXISTS globally, drop it here too and gate with separate migrations.
📜 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 (2)
capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.java(1 hunks)capturecat-core/src/main/resources/db/migration/V2__create_user_tag.sql(1 hunks)
🔇 Additional comments (2)
capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.java (1)
19-23: JPA uniqueness is good; ensure DB matches and add guard at service level.The unique pair constraint is correct. Ensure service layer handles DuplicateKey/DataIntegrityViolationException to return 409 or idempotent behavior on duplicates.
capturecat-core/src/main/resources/db/migration/V2__create_user_tag.sql (1)
3-8: Referenced tables/PKs match base schema; timestamp(6) usage is consistent.
| package com.capturecat.core.domain.user; | ||
|
|
||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.FetchType; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.JoinColumn; | ||
| import jakarta.persistence.ManyToOne; | ||
| import jakarta.persistence.Table; | ||
| import jakarta.persistence.UniqueConstraint; | ||
|
|
||
| import lombok.AccessLevel; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| import com.capturecat.core.domain.BaseTimeEntity; | ||
| import com.capturecat.core.domain.tag.Tag; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm BaseTimeEntity column names match DDL (created_date/last_modified_date).
If BaseTimeEntity uses camelCase without @column overrides, naming strategy must map to snake_case. Otherwise inserts will fail.
Run:
🏁 Script executed:
#!/bin/bash
# Verify BaseTimeEntity column mappings
fd -t f -a 'BaseTimeEntity.java' | xargs -I{} sh -c '
echo "== {} ==";
rg -n "createdDate|lastModifiedDate|@Column" -n {} -C2
'
# Check application naming strategy
rg -n "spring.jpa.hibernate.naming" -C2 **/application*.yml **/application*.properties || trueLength of output: 678
Explicitly define snake_case column names for audit fields
In BaseTimeEntity.java, annotate the fields with name overrides to match the DDL:
- Line 21:
@Column(name = "created_date", nullable = false, updatable = false) - Line 25:
@Column(name = "last_modified_date", nullable = false)
🤖 Prompt for AI Agents
In capturecat-core/src/main/java/com/capturecat/core/domain/BaseTimeEntity.java
around lines 21 to 25, the audit timestamp fields must map to snake_case column
names; add @Column(name = "created_date", nullable = false, updatable = false)
to the createdDate field and @Column(name = "last_modified_date", nullable =
false) to the lastModifiedDate field, and ensure jakarta.persistence.Column is
imported so the JPA column name overrides compile.
| @@ -0,0 +1,9 @@ | |||
| create table if not exists user_tag | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid IF NOT EXISTS in versioned migrations.
Flyway migrations should be deterministic; IF NOT EXISTS can hide drift and let incompatible schemas slip through.
-create table if not exists user_tag
+create table user_tagIf the table might already exist in some envs, prefer a separate repeatable migration or conditional precheck script, not IF NOT EXISTS in V2.
📝 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.
| create table if not exists user_tag | |
| create table user_tag |
🤖 Prompt for AI Agents
In capturecat-core/src/main/resources/db/migration/V2__create_user_tag.sql
around line 1, the migration uses "IF NOT EXISTS" which hides schema drift and
breaks Flyway determinism; remove the "IF NOT EXISTS" clause so the CREATE TABLE
fails if the table already exists, and instead handle environments where the
table may already exist by adding either (a) a separate repeatable migration
that safely creates/maintains the table, or (b) a one-time precheck script that
conditionally creates the table before running versioned migrations; ensure the
V2 migration contains a plain CREATE TABLE statement and update migration
tracking accordingly.
| user_id bigint constraint fk_user_tag_user references users, | ||
| tag_id bigint constraint fk_user_tag_tag references tag, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
FKs should be NOT NULL and consider ON DELETE CASCADE.
A user_tag without user/tag is invalid. Also consider cascading deletes to avoid orphans when a user/tag is removed.
Option A (edit V2 before merge):
- user_id bigint constraint fk_user_tag_user references users,
- tag_id bigint constraint fk_user_tag_tag references tag,
+ user_id bigint not null
+ constraint fk_user_tag_user references users(id) on delete cascade,
+ tag_id bigint not null
+ constraint fk_user_tag_tag references tag(id) on delete cascade,If V2 may already be applied locally, create V3 with:
alter table user_tag
alter column user_id set not null,
alter column tag_id set not null;
alter table user_tag
drop constraint if exists fk_user_tag_user,
add constraint fk_user_tag_user foreign key (user_id) references users(id) on delete cascade;
alter table user_tag
drop constraint if exists fk_user_tag_tag,
add constraint fk_user_tag_tag foreign key (tag_id) references tag(id) on delete cascade;🤖 Prompt for AI Agents
In capturecat-core/src/main/resources/db/migration/V2__create_user_tag.sql
around lines 4-5, the user_id and tag_id foreign keys are nullable and lack
cascade behavior; make both columns NOT NULL and update the fk_user_tag_user and
fk_user_tag_tag constraints to reference the correct parent id columns and
include ON DELETE CASCADE. If you can edit V2 before merging, change the column
definitions to NOT NULL and alter the FK definitions to add ON DELETE CASCADE;
if V2 may already be applied, add a new migration (V3) that alters the user_tag
table to set both columns NOT NULL and replaces the existing fk constraints with
versions that reference users(id) and tag(id) and include ON DELETE CASCADE.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
capturecat-core/src/main/java/com/capturecat/core/domain/tag/TagRegister.java (2)
27-39: Guard empty input, de-duplicate names, and avoidsaveAllon empty.
Prevents unnecessary queries and duplicate inserts when input contains repeated names.@Transactional public List<Tag> registerTagsFor(List<String> tagNames) { - Map<String, Tag> existingTagsByName = tagRepository.findByNameIn(tagNames).stream() + if (tagNames == null || tagNames.isEmpty()) { + return List.of(); + } + List<String> distinctTagNames = tagNames.stream().distinct().toList(); + + Map<String, Tag> existingTagsByName = tagRepository.findByNameIn(distinctTagNames).stream() .collect(Collectors.toMap(Tag::getName, Function.identity())); - List<Tag> newTags = tagNames.stream() + List<Tag> newTags = distinctTagNames.stream() .filter(tagName -> !existingTagsByName.containsKey(tagName)) .map(Tag::new) .toList(); - List<Tag> savedNewTags = tagRepository.saveAll(newTags); + List<Tag> savedNewTags = newTags.isEmpty() ? List.of() : tagRepository.saveAll(newTags); List<Tag> result = new ArrayList<>(existingTagsByName.values()); result.addAll(savedNewTags); return result; }
9-14: Prepare to handle unique-constraint races when creating a single tag.
CatchingDataIntegrityViolationExceptionmakes the “check-then-insert” path safe under concurrency.+ import org.springframework.dao.DataIntegrityViolationException; import org.springframework.transaction.annotation.Transactional;
🧹 Nitpick comments (15)
capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTagRepository.java (2)
9-11: Consider ID-based queries to avoid loading entities.If callers often have IDs, adding these can cut one query per call:
public interface UserTagRepository extends JpaRepository<UserTag, Long> { boolean existsByUserAndTag(User user, Tag tag); long countByUser(User user); + boolean existsByUserIdAndTagId(Long userId, Long tagId); + long countByUserId(Long userId); }
7-11: DB constraints and indexes: double-check the migration.To backstop service-level checks and keep queries fast:
- Unique index on (user_id, tag_id) in this order (supports countByUser).
- Foreign keys: user_id, tag_id with ON DELETE CASCADE/RESTRICT per domain rule.
- Secondary index on tag_id if you’ll query by tag.
- created_date/last_modified_date nullability consistent with JPA auditing.
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java (1)
42-44: Minor wording consistency (optional).Project mixes “유저/사용자/회원” and punctuation across messages; consider standardizing later for UX polish.
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java (1)
47-48: Status choice check (optional).400 is fine; consider 409 (duplicate) and 422 (limit exceeded) if you prefer more specific semantics.
capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (1)
56-63: Clarify Authorization header format in REST Docs.
Explicitly indicate the Bearer prefix to reduce ambiguity for API consumers.- requestHeaders(headerWithName(HttpHeaders.AUTHORIZATION).description("유효한 Access 토큰")), + requestHeaders(headerWithName(HttpHeaders.AUTHORIZATION).description("Bearer {accessToken} 형식의 유효한 Access 토큰")),capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagRegisterTest.java (2)
86-87: Adjust expectation ifsaveAllis skipped when no new tags.
If you adopt the early-return/skip-empty change, this test should assert no call tosaveAll.- verify(tagRepository, times(1)).saveAll(Collections.emptyList()); + verify(tagRepository, never()).saveAll(anyList());
30-48: Add a test covering duplicate input names.
Ensures only one tag is saved when the request contains duplicates.Example to add:
@Test void 요청에_중복된_태그명이_포함된_경우_한번만_저장한다() { List<String> tagNames = List.of("dup", "dup", "new"); given(tagRepository.findByNameIn(anyList())).willReturn(Collections.emptyList()); given(tagRepository.saveAll(anyList())) .willAnswer(inv -> inv.getArgument(0)); // echo saved list List<Tag> result = tagRegister.registerTagsFor(tagNames); verify(tagRepository, times(1)).saveAll(argThat(list -> list.stream().map(Tag::getName).collect(Collectors.toSet()).size() == list.size() )); assertThat(result).extracting(Tag::getName).containsExactlyInAnyOrder("dup", "new"); }capturecat-core/src/test/java/com/capturecat/core/domain/user/UserTagFixture.java (1)
7-14: Make the fixture class non-instantiable.
Minor test hygiene for utility classes.-public class UserTagFixture { +public final class UserTagFixture { + private UserTagFixture() {} public static UserTag createUserTag(Long id, User user, Tag tag) { UserTag userTag = UserTag.create(user, tag); ReflectionTestUtils.setField(userTag, "id", id); return userTag; } }capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java (3)
9-14: Enforce input validation for tagName (reject blanks).Add validation to prevent empty/blank tag names at the boundary.
@@ -import lombok.RequiredArgsConstructor; +import lombok.RequiredArgsConstructor; +import jakarta.validation.constraints.NotBlank; +import org.springframework.validation.annotation.Validated; @@ @RestController @RequiredArgsConstructor +@Validated @RequestMapping("/v1/user-tags") public class UserTagController { @@ - public ApiResponse<TagResponse> create(@AuthenticationPrincipal LoginUser loginUser, @RequestParam String tagName) { + public ApiResponse<TagResponse> create(@AuthenticationPrincipal LoginUser loginUser, + @RequestParam @NotBlank String tagName) {Also applies to: 16-19, 24-28
3-7: Guard the endpoint explicitly.Ensure auth is enforced at the method. If already covered by WebSecurity config, ignore.
@@ -import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.security.access.prepost.PreAuthorize; @@ - @PostMapping + @PostMapping + @PreAuthorize("isAuthenticated()") public ApiResponse<TagResponse> create(…Also applies to: 23-26
11-14: Decouple TagResponse from image package.Importing TagResponse from service.image is leaky. Consider relocating TagResponse to a tag/user-tag API package and update this import.
capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (3)
68-79: Also assert no tag lookup when user missing.Avoid unnecessary downstream calls when user isn’t found.
@@ // when & then assertThatThrownBy(() -> userTagService.create(new LoginUser(DummyObject.newUser("test")), "java")) .isInstanceOf(CoreException.class) .hasMessage(ErrorType.USER_NOT_FOUND.getCode().getMessage()); verify(userTagRepository, never()).save(any()); + verify(tagRegister, never()).registerTagsFor(anyString());
81-97: Ensure count check isn’t hit on duplicate.Short-circuit after duplicate detection.
@@ assertThatThrownBy(() -> userTagService.create(new LoginUser(DummyObject.newUser("test")), "java")) .isInstanceOf(CoreException.class) .hasMessage(ErrorType.USER_TAG_ALREADY_EXISTS.getCode().getMessage()); verify(userTagRepository, never()).save(any()); + verify(userTagRepository, never()).countByUser(any());
1-20: Add a test for DB unique-constraint conflicts → mapped error.Covers the DataIntegrityViolationException path.
@@ package com.capturecat.core.service.user; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.dao.DataIntegrityViolationException; @@ }@@ } + @Test + void 유저_태그_생성_시_DB_유니크_제약조건_충돌을_에러로_매핑한다() { + // given + var user = DummyObject.newUser("test"); + var tag = TagFixture.createTag(1L, "java"); + + given(userRepository.findByUsername(anyString())).willReturn(Optional.of(user)); + given(tagRegister.registerTagsFor(anyString())).willReturn(tag); + given(userTagRepository.existsByUserAndTag(eq(user), eq(tag))).willReturn(false); + given(userTagRepository.countByUser(eq(user))).willReturn(0L); + given(userTagRepository.save(any())).willThrow(new DataIntegrityViolationException("duplicate key")); + + // when & then + assertThatThrownBy(() -> userTagService.create(new LoginUser(user), "java")) + .isInstanceOf(CoreException.class) + .hasMessage(ErrorType.USER_TAG_ALREADY_EXISTS.getCode().getMessage()); + }Also applies to: 116-116
capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java (1)
3-9: Make the max tag count configurable (no magic number).Keeps limits flexible per environment.
@@ -import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.beans.factory.annotation.Value; @@ - private static final int MAX_USER_TAG_COUNT = 30; + @Value("${capturecat.user-tag.max-count:30}") + private int maxUserTagCount; @@ - if (userTagCount >= MAX_USER_TAG_COUNT) { + if (userTagCount >= maxUserTagCount) { throw new CoreException(ErrorType.TOO_MANY_USER_TAGS); }Also applies to: 26-27, 60-66
📜 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 (15)
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(1 hunks)capturecat-core/src/main/java/com/capturecat/core/domain/tag/TagRegister.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/domain/tag/TagRepository.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.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(1 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(1 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagRegisterTest.java(2 hunks)capturecat-core/src/test/java/com/capturecat/core/domain/user/UserTagFixture.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- capturecat-core/src/docs/asciidoc/error-codes.adoc
🚧 Files skipped from review as they are similar to previous changes (1)
- capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTag.java
🧰 Additional context used
🧬 Code graph analysis (4)
capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (1)
capturecat-tests/api-docs/src/main/java/com/capturecat/test/api/RestDocsUtil.java (1)
RestDocsUtil(7-22)
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)
capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java (1)
capturecat-core/src/test/java/com/capturecat/core/api/error/ErrorCodeDocumentTest.java (1)
ErrorCodeDocumentTest(18-40)
capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagRegisterTest.java (1)
capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagFixture.java (1)
TagFixture(5-16)
⏰ 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: build
🔇 Additional comments (11)
capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTagRepository.java (1)
7-11: Repository API looks correct and aligns with JPA conventions.The derived queries match the intended uniqueness (user, tag) and limit-check use cases.
capturecat-core/src/docs/asciidoc/user.adoc (2)
45-45: Verify snippet type matches the controller.Docs use query-parameters; if the endpoint consumes JSON, switch to request-fields to avoid empty snippets.
50-50: Confirm the error-codes anchor/path.Ensure the included anchor id and snippet location for 유저-태그-생성 actually exist to prevent a broken cross-ref.
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java (1)
42-44: New error codes look good.Messages are clear and user-facing.
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java (1)
41-43: Enum formatting change is fine.Trailing comma/line break keeps style consistent.
capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java (2)
18-20: LGTM: coverage of expected error types.The three documented errors are appropriate for create flow.
16-21: Docs identifier/path verified; no action required.capturecat-core/src/main/java/com/capturecat/core/domain/tag/TagRepository.java (1)
12-12: LGTM: single-tag lookup addition is appropriate.
Optional<Tag> findByName(String name)is a useful complement to the bulk lookup.capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (2)
26-27: Verify TagResponse import package.
It’s unusual for a DTO to live underservice.image. IfTagResponseis undercom.capturecat.core.dto, update the import.Apply if applicable:
- import com.capturecat.core.service.image.TagResponse; + import com.capturecat.core.dto.TagResponse;
50-56: Double-check RestAssured status assertion.
Depending on the module,status(HttpStatus.OK)may not compile; fallback isstatusCode(HttpStatus.OK.value()).If needed:
- .then().status(HttpStatus.OK) + .then().statusCode(HttpStatus.OK.value())capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java (1)
35-37: Verify username immutability or include user ID for lookups
LoginUser doesn’t carry a user ID, so you’re relying on username stability; if usernames can change, extend LoginUser to include the user’s ID and switch tofindById(loginUser.getId())here.
| public Tag registerTagsFor(String tagName) { | ||
| return tagRepository.findByName(tagName) | ||
| .orElseGet(() -> tagRepository.save(new Tag(tagName))); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make single-tag register resilient to concurrent inserts.
If another transaction inserts the same tag between find and save, re-read after the unique-constraint violation.
@Transactional
public Tag registerTagsFor(String tagName) {
- return tagRepository.findByName(tagName)
- .orElseGet(() -> tagRepository.save(new Tag(tagName)));
+ return tagRepository.findByName(tagName)
+ .orElseGet(() -> {
+ try {
+ return tagRepository.save(new Tag(tagName));
+ } catch (DataIntegrityViolationException e) {
+ // Another transaction likely created it; fetch and return.
+ return tagRepository.findByName(tagName).orElseThrow(() -> e);
+ }
+ });
}If not already present, ensure a unique index on tag.name (preferably case-insensitive) in Flyway migrations to make this effective.
To verify constraints exist in migrations, you can run:
🏁 Script executed:
#!/bin/bash
fd -t f -a 'V*__*.sql' capturecat-core/src/main/resources/db/migration
rg -nP 'create\s+table\s+tag|unique.*\(name\)|create\s+unique\s+index.*lower\(name\)' capturecat-core/src/main/resources/db/migration
rg -nP 'create\s+table\s+user_tag|unique.*\(user_id\s*,\s*tag_id\)|foreign\s+key' capturecat-core/src/main/resources/db/migrationLength of output: 475
Enforce unique tag.name constraint and make registerTagsFor concurrency-safe
- In
TagRegister.registerTagsFor, wraptagRepository.save(new Tag(tagName))in a try/catch forDataIntegrityViolationExceptionand re-fetch the existing tag when a unique-constraint violation occurs (as in the suggested diff). - Add a Flyway migration to create a unique index on
tag.name(preferably case-insensitive) to prevent duplicate inserts.
🤖 Prompt for AI Agents
In capturecat-core/src/main/java/com/capturecat/core/domain/tag/TagRegister.java
around lines 48 to 51, make registerTagsFor concurrency-safe by wrapping the
save(new Tag(tagName)) call in a try/catch for DataIntegrityViolationException:
if save throws that exception, re-query tagRepository.findByName(tagName) and
return the found Tag (or rethrow if still absent); this ensures concurrent
inserts return the single persisted entity. Also add a Flyway migration (SQL
file) that creates a unique index on the tag.name column (preferably
case-insensitive, e.g. using LOWER(name) or the DB-specific citext) so the
database enforces uniqueness and prevents duplicate inserts under race
conditions.
| List<Tag> findByNameIn(List<String> names); | ||
|
|
||
| Optional<Tag> findByName(String name); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce uniqueness on Tag.name (ideally case-insensitive) to make findByName reliable and avoid races.
Without a unique index, concurrent creates can produce duplicates and make findByName ambiguous.
Consider adding DB constraints (PostgreSQL examples):
-- strict case-sensitive uniqueness
ALTER TABLE tag ADD CONSTRAINT uk_tag_name UNIQUE (name);
-- or case-insensitive uniqueness
CREATE UNIQUE INDEX ux_tag_name_ci ON tag (lower(name));🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/domain/tag/TagRepository.java
around lines 10 to 12, the repository relies on findByName but the Tag.name
column lacks a uniqueness constraint which allows concurrent inserts to create
duplicates; add a database-level unique constraint (preferably case-insensitive,
e.g. unique index on lower(name)) via a new migration, update the Tag JPA entity
to reflect uniqueness if you use schema generation (or add a unique constraint
annotation), and handle potential race conditions by catching unique-constraint
violations in the create flow so duplicate insert attempts return the existing
Tag or retry/fail gracefully.
| @Transactional | ||
| public TagResponse create(LoginUser loginUser, String tagName) { | ||
| try { | ||
| User user = userRepository.findByUsername(loginUser.getUsername()) | ||
| .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND)); | ||
| Tag tag = tagRegister.registerTagsFor(tagName); | ||
|
|
||
| validate(user, tag); | ||
|
|
||
| userTagRepository.save(UserTag.create(user, tag)); | ||
|
|
||
| return TagResponse.from(tag); | ||
| } catch (DataIntegrityViolationException ex) { | ||
| throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Catch DataIntegrityViolationException only around save().
Avoid mis-mapping integrity errors thrown by unrelated operations.
@@
- @Transactional
- public TagResponse create(LoginUser loginUser, String tagName) {
- try {
- User user = userRepository.findByUsername(loginUser.getUsername())
- .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND));
- Tag tag = tagRegister.registerTagsFor(tagName);
-
- validate(user, tag);
-
- userTagRepository.save(UserTag.create(user, tag));
-
- return TagResponse.from(tag);
- } catch (DataIntegrityViolationException ex) {
- throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS);
- }
- }
+ @Transactional
+ public TagResponse create(LoginUser loginUser, String tagName) {
+ User user = userRepository.findByUsername(loginUser.getUsername())
+ .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND));
+ Tag tag = tagRegister.registerTagsFor(tagName);
+
+ validate(user, tag);
+
+ try {
+ userTagRepository.save(UserTag.create(user, tag));
+ } catch (DataIntegrityViolationException ex) {
+ throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS);
+ }
+
+ return TagResponse.from(tag);
+ }📝 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 create(LoginUser loginUser, String tagName) { | |
| try { | |
| User user = userRepository.findByUsername(loginUser.getUsername()) | |
| .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND)); | |
| Tag tag = tagRegister.registerTagsFor(tagName); | |
| validate(user, tag); | |
| userTagRepository.save(UserTag.create(user, tag)); | |
| return TagResponse.from(tag); | |
| } catch (DataIntegrityViolationException ex) { | |
| throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS); | |
| } | |
| } | |
| @Transactional | |
| public TagResponse create(LoginUser loginUser, String tagName) { | |
| User user = userRepository.findByUsername(loginUser.getUsername()) | |
| .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND)); | |
| Tag tag = tagRegister.registerTagsFor(tagName); | |
| validate(user, tag); | |
| try { | |
| userTagRepository.save(UserTag.create(user, tag)); | |
| } catch (DataIntegrityViolationException ex) { | |
| throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS); | |
| } | |
| return TagResponse.from(tag); | |
| } |
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java
around lines 32 to 47, the current try/catch wraps the entire create method
causing any DataIntegrityViolationException from unrelated calls to be mapped to
USER_TAG_ALREADY_EXISTS; narrow the try block so only the
userTagRepository.save(...) call is inside the try (or catch
DataIntegrityViolationException only around that single statement), leaving
repository lookups, tag registration and validation outside, and rethrow or let
other exceptions propagate unchanged.
| userTagRepository.save(UserTag.create(user, tag)); | ||
|
|
||
| return TagResponse.from(tag); | ||
| } catch (DataIntegrityViolationException ex) { | ||
| throw new CoreException(ErrorType.USER_TAG_ALREADY_EXISTS); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure DB constraints back these checks.
Validate that user_tag has (user_id, tag_id) unique + FKs and relevant indexes; also that tag.name is unique (TagRegister relies on it).
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "user_tag DDL:"
fd -a 'V*__*user_tag*.sql' capturecat-core/src/main/resources/db/migration | xargs -I{} sh -c 'echo "== {} =="; cat "{}"'
echo
echo "[Checks]"
rg -nP 'unique\s*\(\s*user_id\s*,\s*tag_id\s*\)' capturecat-core/src/main/resources/db/migration || echo "Missing composite UNIQUE (user_id, tag_id)"
rg -nP 'foreign key.*user_id' capturecat-core/src/main/resources/db/migration || echo "Missing FK for user_id"
rg -nP 'foreign key.*tag_id' capturecat-core/src/main/resources/db/migration || echo "Missing FK for tag_id"
rg -nP '(created_|last_modified_)' capturecat-core/src/main/resources/db/migration/*user_tag*.sql || echo "Missing auditing timestamps on user_tag"
rg -nP 'index.*user_id' capturecat-core/src/main/resources/db/migration/*user_tag*.sql || echo "Consider index on user_id"
rg -nP 'index.*tag_id' capturecat-core/src/main/resources/db/migration/*user_tag*.sql || echo "Consider index on tag_id"
echo
echo "tag DDL (checking unique name):"
fd -a 'V*__*.sql' capturecat-core/src/main/resources/db/migration | xargs rg -nP 'create\s+table.*tag|unique.*\(name\)' -n -C1Length of output: 2614
Add UNIQUE constraint on tag.name
The tag table in V1__init.sql lacks a UNIQUE(name) constraint—TagRegister depends on name uniqueness. user_tag already has composite UNIQUE(user_id, tag_id), inline FKs, and auditing timestamps.
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java
around lines 41 to 46, the review flags that the tag table lacks a UNIQUE(name)
constraint required by TagRegister; update the DB migration so the tag.name
column is unique by either adding UNIQUE(name) in the tag table DDL in
V1__init.sql or creating a new migration that runs ALTER TABLE tag ADD
CONSTRAINT uq_tag_name UNIQUE(name); ensure the migration handles existing
duplicate tag rows (deduplicate or merge IDs and update referencing user_tag
rows) before adding the constraint and add/adjust tests to reflect the
uniqueness guarantee.
|
궁금한게 있는데, flyway를 도입한 이유는 무엇인가요? |
|
써보는 것도 좋을 것 같아요~ |
leeedohyun
force-pushed
the
feature/#120-user-tag
branch
from
e64fe37 to
9eb0a3e
Compare
📌 관련 이슈 (필수)
📝 작업 내용 (필수)
💬 리뷰 참고 사항 (선택)
일단 개발계로 배포하는 깃허브 액션 워크플로우를 리뷰 끝나도 돌리려고 종료를 했습니다.
Summary by CodeRabbit
New Features
Chores
Documentation
Tests