Conversation
- 태그 처리 및 생성 로직 추가: 태그 생성과 조회를 한 번의 쿼리로 처리하는 기능 신규 구현 (CTE 활용) - 북마크-태그 연결 로직 추가: 북마크와 태그를 연결하여 DB에 한 번에 저장하는 Bulk Insert 기능 구현
- 태그 처리 및 생성 로직 추가: 태그 생성과 조회를 한 번의 쿼리로 처리하는 기능 신규 구현 (CTE 활용) - 북마크-태그 연결 로직 추가: 북마크와 태그를 연결하여 DB에 한 번에 저장하는 Bulk Insert 기능 구현
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
워크스루북마크 시스템에 태그 기능을 추가하는 변경사항입니다. 데이터 접근 계층에서 URL 기반 링크 조회를 추가하고, 새로운 태그 관리 API(insertAndSelectTags, insertBookmarkTags)를 구현했습니다. 서비스 계층에서 북마크 저장 시 태그 파싱, 중복 제거, 등록 로직을 추가했으며, MyBatis 매퍼에서 태그 조회 조인과 태그 삽입 쿼리를 구현했습니다. 도메인 모델과 DTO에 태그 필드를 추가하고 프론트엔드에서 태그 필드명을 업데이트했습니다. Sequence Diagram(s)sequenceDiagram
actor User as 사용자
participant UI as 프론트엔드<br/>(myPage.html)
participant Service as BookmarkService
participant DAO as BookmarkDao
participant DB as 데이터베이스
User->>UI: 북마크 & 태그 입력
UI->>Service: insertBookmark(BookmarkDto)
Note over Service: memberFolderId null 체크<br/>기본값 1로 설정
Service->>DAO: selectLinkByUrl(url)
DAO->>DB: 링크 조회
DB-->>DAO: Link 반환
DAO-->>Service: Link 결과
alt 링크 미존재
Service->>DAO: insertLink(Link)
DAO->>DB: 링크 삽입
end
Service->>DAO: insertBookmark(bookmarkDto)
DAO->>DB: 북마크 삽입
DB-->>DAO: bookmarkId 반환
DAO-->>Service: 삽입 결과
Note over Service: DataIntegrityViolation<br/>처리
alt 태그 존재
Service->>Service: processAndCreateTags()
Note over Service: 태그 파싱 및<br/>중복 제거
Service->>DAO: insertAndSelectTags(memberId, tagNames)
DAO->>DB: 태그 삽입/조회<br/>(CTE 쿼리)
DB-->>DAO: MemberTagResultDto 반환
DAO-->>Service: 태그 ID 리스트
Service->>DAO: insertBookmarkTags(bookmarkId, tagIds)
DAO->>DB: 북마크-태그 매핑 삽입
DB-->>DAO: 삽입 완료
DAO-->>Service: 결과 반환
end
Service-->>UI: 결과 반환
UI-->>User: 북마크 저장 완료
예상 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~45분 제안 리뷰어
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/main/resources/templates/mypage/myPage.html (3)
639-660:⚠️ Potential issue | 🔴 Critical버그:
response.tag는response.tags로 변경 필요백엔드
Bookmark도메인이tags(List<String>)로 변경되었으므로,response.tag는undefined가 됩니다. 수정 폼에서 기존 태그가 표시되지 않습니다.또한
tags는List<String>이므로 수정 폼에 표시할 때 join 처리가 필요합니다.🐛 수정 제안
- document.getElementById('updateTag').value = response.tag || ''; + document.getElementById('updateTag').value = (response.tags || []).join(' ');
546-577:⚠️ Potential issue | 🔴 CriticalXSS 취약점: 사용자 입력값이 HTML에 직접 삽입됩니다.
displayTitle,note,url등 사용자 입력값이 템플릿 리터럴로 HTML에 이스케이프 없이 삽입되어 Stored XSS 공격에 취약합니다. 예:displayTitle에<img src=x onerror=alert(1)>입력 시 스크립트가 실행됩니다.
textContent또는 HTML 이스케이프 함수를 사용하여 사용자 입력값을 안전하게 렌더링해야 합니다.
676-684:⚠️ Potential issue | 🟠 Major북마크 수정 시 태그 처리가 백엔드에서 누락되어 있습니다.
프론트엔드에서
tags필드를 전송하지만,BookmarkServiceImpl.updateBookmark()는 태그 처리 로직 없이 단순 DAO 위임만 합니다. 수정 시 태그 변경사항이 저장되지 않습니다.src/main/java/com/web/SearchWeb/bookmark/service/BookmarkServiceImpl.java (3)
67-75:⚠️ Potential issue | 🔴 Critical버그:
selectLinkByUrl에 원본 URL을 전달하지만, XML에서는canonical_url로 조회합니다.Line 72에서 원본
url을selectLinkByUrl에 전달하지만,bookmark-mapper.xml의 SQL은WHERE canonical_url = #{url}로 조회합니다. 원본 URL과 정규화된 URL이 다를 경우 (예: 쿼리스트링, 프래그먼트 포함) 기존 링크를 찾지 못해 중복 링크가 생성됩니다.PR 목적이 "non-normalized link로 조회 변경"이라면 XML에서
original_url로 조회해야 하고, 그렇지 않다면 여기서canonicalUrl을 전달해야 합니다.🐛 옵션 1: XML을 original_url로 변경 (PR 의도에 맞춤)
bookmark-mapper.xml수정:<select id="selectLinkByUrl" parameterType="String" resultMap="LinkResultMap"> SELECT * FROM link - WHERE canonical_url = #{url} + WHERE original_url = #{url} AND deleted_at IS NULL </select>🐛 옵션 2: 서비스에서 canonicalUrl 전달
// 기존 링크 조회 - Link existingLink = bookmarkDao.selectLinkByUrl(url); + Link existingLink = bookmarkDao.selectLinkByUrl(canonicalUrl);
96-105:⚠️ Potential issue | 🔴 Critical같은 URL 불일치 버그가
checkBookmarkExistsByUrl에도 존재합니다.Line 97에서
canonicalUrl을 생성하지만 사용하지 않고, Line 99에서 원본url을selectLinkByUrl에 전달합니다. 위의getOrCreateLink수정과 동일하게 수정해야 합니다.
146-149:⚠️ Potential issue | 🟠 Major
updateBookmark에서 태그 처리 로직이 누락되었습니다.
insertBookmark에는processAndCreateTags호출이 추가되었지만,updateBookmark에는 태그 처리가 없습니다. 프론트엔드에서tags필드를 전송하지만 백엔드에서 무시됩니다. 북마크 수정 시 태그 변경이 반영되지 않습니다.기존 태그 삭제 → 새 태그 등록/연결 로직이 필요합니다.
이 부분의 구현을 도와드릴까요? 이슈를 생성하여 추적할 수도 있습니다.
🤖 Fix all issues with AI agents
In `@src/main/java/com/web/SearchWeb/bookmark/dao/BookmarkDao.java`:
- Around line 42-46: The new DAO methods insertAndSelectTags(Long memberId,
List<String> tagNames) and insertBookmarkTags(Long bookmarkId, List<Long>
tagIds) need explicit MyBatis `@Param` annotations on their parameters to ensure
proper parameter binding; update the method signatures in BookmarkDao (add
`@Param`("memberId") and `@Param`("tagNames") to insertAndSelectTags, and
`@Param`("bookmarkId") and `@Param`("tagIds") to insertBookmarkTags) so MyBatis can
resolve the parameters without relying on -parameters compilation or
use-actual-param-name settings.
In `@src/main/java/com/web/SearchWeb/bookmark/domain/Bookmark.java`:
- Around line 33-35: The tags field in class Bookmark can become a list
containing a single null when MyBatis LEFT JOIN yields tag_name = NULL; to
prevent NPEs/empty UI items, initialize and/or defensively filter tags: set the
Bookmark.tags field to an empty list by default (e.g., initialize List<String>
tags in the Bookmark class) and ensure any code that maps or assembles Bookmark
instances (mapper result handling or service methods that populate tags) filters
out null entries before assigning to the Bookmark.tags property.
In `@src/main/java/com/web/SearchWeb/bookmark/service/BookmarkServiceImpl.java`:
- Around line 198-229: The log statement in processAndCreateTags currently logs
the entire tag list at INFO level; change the logging call to use DEBUG (e.g.,
replace log.info("allTags: {}", allTags) with log.debug(...)) so this detailed,
debug-only data (allTags from bookmarkDao.insertAndSelectTags) does not appear
in production INFO logs; keep the same message and variable name (allTags) so
the context remains clear.
- Around line 124-141: The catch in BookmarkServiceImpl around the
insertBookmark call is too broad: it swallows all
DataIntegrityViolationException instances and returns 0, hiding FK/NOT NULL
issues; change it to detect and handle only the specific UNIQUE constraint
(uq_member_saved_link_folder_link) by inspecting the exception cause (e.g.,
SQLIntegrityConstraintViolationException or the constraint name in the message)
and log the full exception when matched, returning 0 only for that case; for any
other DataIntegrityViolationException, log the error with details and rethrow or
wrap in an appropriate runtime exception so unexpected integrity violations are
not silently ignored (retain references to bookmarkDao.insertBookmark,
processAndCreateTags, bookmarkDto.getBookmarkId, and the unique constraint name
in your check).
In `@src/main/resources/mapper/bookmark-mapper.xml`:
- Around line 112-117: The mapper's select id "selectLinkByUrl" currently
queries canonical_url but the service passes the original URL; update the SQL in
the selectLinkByUrl statement so it filters on original_url (i.e., WHERE
original_url = #{url} AND deleted_at IS NULL) or, alternatively, change the
parameter/ID to reflect canonical lookup if you truly intend canonical
lookup—make the column and parameter consistent with the behavior expected by
BookmarkServiceImpl (selectLinkByUrl should use original_url unless the service
is changed to pass canonical_url).
- Around line 171-200: In the select mapped by id "insertAndSelectTags" the ON
CONFLICT DO UPDATE currently always sets updated_at, causing unnecessary
updates; change the conflict clause on the INSERT into member_tag (CTE
inserted_tags) to only perform the UPDATE when the existing row is soft-deleted
(add a WHERE condition like "WHERE member_tag.deleted_at IS NOT NULL" on the DO
UPDATE) so active rows aren't touched, and optionally simplify the final result
by removing the UNION and returning only from inserted_tags (since RETURNING
from the INSERT/ON CONFLICT will yield both inserted and updated rows) to avoid
duplicate work.
- Around line 159-168: Remove the unused mapper definition insertMemberTag from
the mapper XML: delete the <insert id="insertMemberTag" ...> block that upserts
into member_tag (owner_member_id, tag_name) with ON CONFLICT and RETURNING
member_tag_id, since BookmarkDao does not declare it and BookmarkServiceImpl
uses the existing insertAndSelectTags CTE for the same behavior; ensure no other
code references insertMemberTag before deleting and keep the insertAndSelectTags
implementation intact.
| // 태그 등록 및 조회 (Insert & Select) | ||
| List<MemberTagResultDto> insertAndSelectTags(Long memberId, List<String> tagNames); | ||
|
|
||
| // 북마크-태그 연결 일괄 추가 (Bulk Insert) | ||
| int insertBookmarkTags(Long bookmarkId, List<Long> tagIds); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if `@Param` is used elsewhere in DAO interfaces and if mybatis config enables useActualParamName
echo "=== Checking `@Param` usage in DAO interfaces ==="
rg -n '@Param' --type=java -g '*Dao.java' -A1
echo ""
echo "=== Checking if any *Dao.java files exist and their method signatures ==="
fd '*Dao.java' -x cat -n {}
echo ""
echo "=== Checking MyBatis configuration in application properties ==="
fd -e yml -e yaml -e properties | xargs grep -i 'mybatis\|actual-param' 2>/dev/null | head -20
echo ""
echo "=== Checking pom.xml for compiler configuration ==="
fd 'pom.xml' -x cat {} | grep -A10 'maven-compiler-plugin\|mybatis-spring-boot' | head -30
echo ""
echo "=== Checking build.gradle if exists ==="
fd 'build.gradle' -x cat {} 2>/dev/null | head -50Repository: Searchweb-Dev/Searchweb-Back
Length of output: 3094
🏁 Script executed:
#!/bin/bash
echo "=== BookmarkDao.java current state ==="
fd 'BookmarkDao.java' -x cat -n {}
echo ""
echo "=== BookmarkDao mapper XML file ==="
fd 'BookmarkDao.xml' -o 'BookmarkMapper.xml' -x cat -n {}Repository: Searchweb-Dev/Searchweb-Back
Length of output: 6044
🏁 Script executed:
#!/bin/bash
echo "=== Searching for BookmarkDao mapper XML ==="
fd -e xml | grep -i bookmark
echo ""
echo "=== Checking mapper directory contents ==="
fd -e xml -type f | head -20
echo ""
echo "=== Looking for parameter usage in bookmark mapper XML ==="
fd -e xml | xargs grep -l 'insertAndSelectTags\|insertBookmarkTags' 2>/dev/nullRepository: Searchweb-Dev/Searchweb-Back
Length of output: 500
🏁 Script executed:
#!/bin/bash
echo "=== Reading bookmark-mapper.xml ==="
cat -n src/main/resources/mapper/bookmark-mapper.xml | grep -A15 -B2 'insertAndSelectTags\|insertBookmarkTags'Repository: Searchweb-Dev/Searchweb-Back
Length of output: 1415
🏁 Script executed:
#!/bin/bash
echo "=== Full application.properties content ==="
cat -n src/main/resources/application.properties
echo ""
echo "=== Checking other DAO interfaces for `@Param` patterns ==="
fd 'Dao.java' --type f -x grep -l 'interface' {} \; | xargs -I {} sh -c 'echo "=== {} ===" && head -50 {}'
echo ""
echo "=== Checking if build.gradle has -parameters flag ==="
grep -i 'parameters\|compilerArgs' build.gradleRepository: Searchweb-Dev/Searchweb-Back
Length of output: 9316
새 메서드에 @Param 어노테이션 추가가 필요합니다.
프로젝트의 다른 DAO인 BoardDao.java에서 다중 파라미터 메서드에 명시적으로 @Param 어노테이션을 사용하고 있으며, 현재 application.properties에 mybatis.configuration.use-actual-param-name 설정이 없고 build.gradle에 -parameters 컴파일 옵션이 없습니다. 따라서 insertAndSelectTags와 insertBookmarkTags 메서드에서 MyBatis 파라미터 바인딩이 실패할 수 있습니다.
🛡️ 권장 수정
// 태그 등록 및 조회 (Insert & Select)
- List<MemberTagResultDto> insertAndSelectTags(Long memberId, List<String> tagNames);
+ List<MemberTagResultDto> insertAndSelectTags(`@Param`("memberId") Long memberId, `@Param`("tagNames") List<String> tagNames);
// 북마크-태그 연결 일괄 추가 (Bulk Insert)
- int insertBookmarkTags(Long bookmarkId, List<Long> tagIds);
+ int insertBookmarkTags(`@Param`("bookmarkId") Long bookmarkId, `@Param`("tagIds") List<Long> tagIds);🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/bookmark/dao/BookmarkDao.java` around lines
42 - 46, The new DAO methods insertAndSelectTags(Long memberId, List<String>
tagNames) and insertBookmarkTags(Long bookmarkId, List<Long> tagIds) need
explicit MyBatis `@Param` annotations on their parameters to ensure proper
parameter binding; update the method signatures in BookmarkDao (add
`@Param`("memberId") and `@Param`("tagNames") to insertAndSelectTags, and
`@Param`("bookmarkId") and `@Param`("tagIds") to insertBookmarkTags) so MyBatis can
resolve the parameters without relying on -parameters compilation or
use-actual-param-name settings.
|
|
||
| // Tag 목록 (Collection) | ||
| private List<String> tags; // 태그 목록 |
There was a problem hiding this comment.
LEFT JOIN과 MyBatis <collection> 매핑 시 null 요소 주의
bookmark-mapper.xml에서 member_tag와 LEFT JOIN 후 <collection property="tags"> 로 매핑하는데, 태그가 없는 북마크의 경우 tag_name이 NULL인 행이 생성됩니다. MyBatis가 이를 [null] 리스트로 매핑할 수 있어, 하류 코드에서 NullPointerException이나 UI에 빈 태그가 표시될 수 있습니다.
서비스 또는 도메인 레이어에서 null 필터링을 추가하거나, tags 필드 초기화를 고려해 주세요.
🛡️ 방어적 초기화 제안
// Tag 목록 (Collection)
- private List<String> tags; // 태그 목록
+ private List<String> tags = new ArrayList<>(); // 태그 목록🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/bookmark/domain/Bookmark.java` around lines
33 - 35, The tags field in class Bookmark can become a list containing a single
null when MyBatis LEFT JOIN yields tag_name = NULL; to prevent NPEs/empty UI
items, initialize and/or defensively filter tags: set the Bookmark.tags field to
an empty list by default (e.g., initialize List<String> tags in the Bookmark
class) and ensure any code that maps or assembles Bookmark instances (mapper
result handling or service methods that populate tags) filters out null entries
before assigning to the Bookmark.tags property.
| try { | ||
| int result = bookmarkDao.insertBookmark(bookmarkDto, link.getLinkId()); | ||
|
|
||
| // 태그 처리 및 저장 | ||
| if (result > 0 && bookmarkDto.getTags() != null && !bookmarkDto.getTags().isEmpty()) { | ||
| // MyBatis의 useGeneratedKeys="true" 설정에 의해 insert 성공 시, bookmarkDto.bookmarkId에 생성된 PK(member_saved_link_id)가 자동으로 채워짐 | ||
| processAndCreateTags(bookmarkDto.getBookmarkId(), bookmarkDto.getCreatedByMemberId(), bookmarkDto.getTags()); | ||
| } | ||
|
|
||
| return result; | ||
| } catch (DataIntegrityViolationException e) { | ||
| // TODO: 글로벌 예외 처리 구현 후, 커스텀에러 던지도록 변경 | ||
| // 현재는 임시로 0 반환 (DB UNIQUE 제약 uq_member_saved_link_folder_link 위반 시) | ||
| log.warn("북마크 중복 저장 시도: memberId={}, folderId={}, linkId={}", | ||
| bookmarkDto.getCreatedByMemberId(), bookmarkDto.getMemberFolderId(), link.getLinkId()); | ||
| return 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
DataIntegrityViolationException을 광범위하게 catch하면 다른 제약 조건 위반이 숨겨집니다.
UNIQUE 제약 조건 위반만 처리하려는 의도이지만, 모든 DataIntegrityViolationException(FK 위반, NOT NULL 위반 등)을 catch하고 0을 반환합니다. 예상치 못한 데이터 무결성 오류가 조용히 무시될 수 있습니다.
예외 메시지에서 특정 제약 조건명(uq_member_saved_link_folder_link)을 확인하거나, 최소한 예외를 로깅해야 합니다.
🛡️ 개선 제안
} catch (DataIntegrityViolationException e) {
- // TODO: 글로벌 예외 처리 구현 후, 커스텀에러 던지도록 변경
- // 현재는 임시로 0 반환 (DB UNIQUE 제약 uq_member_saved_link_folder_link 위반 시)
- log.warn("북마크 중복 저장 시도: memberId={}, folderId={}, linkId={}",
- bookmarkDto.getCreatedByMemberId(), bookmarkDto.getMemberFolderId(), link.getLinkId());
- return 0;
+ log.warn("북마크 저장 실패 (DataIntegrityViolation): memberId={}, folderId={}, linkId={}, cause={}",
+ bookmarkDto.getCreatedByMemberId(), bookmarkDto.getMemberFolderId(), link.getLinkId(), e.getMessage());
+ return 0;
}🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/bookmark/service/BookmarkServiceImpl.java`
around lines 124 - 141, The catch in BookmarkServiceImpl around the
insertBookmark call is too broad: it swallows all
DataIntegrityViolationException instances and returns 0, hiding FK/NOT NULL
issues; change it to detect and handle only the specific UNIQUE constraint
(uq_member_saved_link_folder_link) by inspecting the exception cause (e.g.,
SQLIntegrityConstraintViolationException or the constraint name in the message)
and log the full exception when matched, returning 0 only for that case; for any
other DataIntegrityViolationException, log the error with details and rethrow or
wrap in an appropriate runtime exception so unexpected integrity violations are
not silently ignored (retain references to bookmarkDao.insertBookmark,
processAndCreateTags, bookmarkDto.getBookmarkId, and the unique constraint name
in your check).
| private void processAndCreateTags(Long bookmarkId, Long memberId, String tags) { | ||
| if (tags == null || tags.isBlank()) return; | ||
|
|
||
| // 1. 태그 파싱 및 중복 제거 | ||
| Set<String> uniqueTags = new HashSet<>(); | ||
| String[] splitTags = tags.split("[,\\s]+"); | ||
| for (String tag : splitTags) { | ||
| if (!tag.isBlank()) { | ||
| uniqueTags.add(tag.trim()); | ||
| } | ||
| } | ||
|
|
||
| if (uniqueTags.isEmpty()) return; | ||
|
|
||
| List<String> tagNames = new ArrayList<>(uniqueTags); | ||
|
|
||
| // 2. 태그 등록 및 조회 (Insert & Select) - CTE를 사용하여 한 번의 쿼리로 처리 | ||
| // 새로운 태그는 생성하고, 기존 태그는 조회하여 모든 태그의 ID를 반환함 | ||
| List<MemberTagResultDto> allTags = bookmarkDao.insertAndSelectTags(memberId, tagNames); | ||
|
|
||
| log.info("allTags: {}", allTags); | ||
|
|
||
| // 최종 태그 ID 목록 추출 | ||
| List<Long> finalTagIds = allTags.stream() | ||
| .map(MemberTagResultDto::getMemberTagId) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| // 3. 북마크-태그 연결 일괄 추가 (Bulk Insert) | ||
| if (!finalTagIds.isEmpty()) { | ||
| bookmarkDao.insertBookmarkTags(bookmarkId, finalTagIds); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
log.info로 전체 태그 목록을 출력하는 것은 DEBUG 레벨이 적합합니다.
Line 218의 log.info("allTags: {}", allTags)는 디버깅용 로그이므로 운영 환경에서 불필요한 노이즈가 됩니다.
♻️ 제안
- log.info("allTags: {}", allTags);
+ log.debug("allTags: {}", allTags);🤖 Prompt for AI Agents
In `@src/main/java/com/web/SearchWeb/bookmark/service/BookmarkServiceImpl.java`
around lines 198 - 229, The log statement in processAndCreateTags currently logs
the entire tag list at INFO level; change the logging call to use DEBUG (e.g.,
replace log.info("allTags: {}", allTags) with log.debug(...)) so this detailed,
debug-only data (allTags from bookmarkDao.insertAndSelectTags) does not appear
in production INFO logs; keep the same message and variable name (allTags) so
the context remains clear.
| <!-- 링크 조회 (url로) --> | ||
| <select id="selectLinkByUrl" parameterType="String" resultMap="LinkResultMap"> | ||
| SELECT * FROM link | ||
| WHERE canonical_url = #{canonicalUrl} | ||
| WHERE canonical_url = #{url} | ||
| AND deleted_at IS NULL | ||
| </select> |
There was a problem hiding this comment.
버그: 메서드 이름은 selectLinkByUrl이지만 SQL은 canonical_url로 조회합니다.
서비스 레이어에서 원본 URL을 전달하지만, SQL은 WHERE canonical_url = #{url}로 조회합니다. 원본 URL과 canonical URL이 다를 경우 조회에 실패합니다. BookmarkServiceImpl 리뷰에서 상세히 설명했습니다.
🐛 PR 의도가 original_url 기반 조회라면
<select id="selectLinkByUrl" parameterType="String" resultMap="LinkResultMap">
SELECT * FROM link
- WHERE canonical_url = #{url}
+ WHERE original_url = #{url}
AND deleted_at IS NULL
</select>📝 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.
| <!-- 링크 조회 (url로) --> | |
| <select id="selectLinkByUrl" parameterType="String" resultMap="LinkResultMap"> | |
| SELECT * FROM link | |
| WHERE canonical_url = #{canonicalUrl} | |
| WHERE canonical_url = #{url} | |
| AND deleted_at IS NULL | |
| </select> | |
| <!-- 링크 조회 (url로) --> | |
| <select id="selectLinkByUrl" parameterType="String" resultMap="LinkResultMap"> | |
| SELECT * FROM link | |
| WHERE original_url = #{url} | |
| AND deleted_at IS NULL | |
| </select> |
🤖 Prompt for AI Agents
In `@src/main/resources/mapper/bookmark-mapper.xml` around lines 112 - 117, The
mapper's select id "selectLinkByUrl" currently queries canonical_url but the
service passes the original URL; update the SQL in the selectLinkByUrl statement
so it filters on original_url (i.e., WHERE original_url = #{url} AND deleted_at
IS NULL) or, alternatively, change the parameter/ID to reflect canonical lookup
if you truly intend canonical lookup—make the column and parameter consistent
with the behavior expected by BookmarkServiceImpl (selectLinkByUrl should use
original_url unless the service is changed to pass canonical_url).
|
|
||
| <!-- 태그 추가 (중복 시 무시 - 애플리케이션 레벨에서 체크하거나 ON CONFLICT 사용) --> | ||
| <!-- PostgreSQL: ON CONFLICT DO NOTHING --> | ||
| <insert id="insertMemberTag" parameterType="map" useGeneratedKeys="true" keyProperty="memberTagId" keyColumn="member_tag_id"> | ||
| INSERT INTO member_tag (owner_member_id, tag_name) | ||
| VALUES (#{memberId}, #{tagName}) | ||
| ON CONFLICT (owner_member_id, tag_name) | ||
| DO UPDATE SET deleted_at = NULL, updated_at = now() | ||
| RETURNING member_tag_id | ||
| </insert> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for insertMemberTag references in the codebase
echo "=== Searching for insertMemberTag references ==="
rg -n 'insertMemberTag' --type=java --type=xml --type=kotlinRepository: Searchweb-Dev/Searchweb-Back
Length of output: 300
🏁 Script executed:
#!/bin/bash
# Locate and examine the BookmarkDao interface
echo "=== Searching for BookmarkDao interface ==="
fd -t f 'BookmarkDao' --exec head -100 {} \;Repository: Searchweb-Dev/Searchweb-Back
Length of output: 3817
🏁 Script executed:
#!/bin/bash
# Search for the insertMemberTag method definition in mapper files
echo "=== Looking for insertMemberTag in mapper XML files ==="
rg -n -B2 -A10 'insertMemberTag' --type=xmlRepository: Searchweb-Dev/Searchweb-Back
Length of output: 1328
🏁 Script executed:
#!/bin/bash
# Search for insertAndSelectTags in mapper and code
echo "=== Searching for insertAndSelectTags definition and usage ==="
rg -n -B2 -A20 'insertAndSelectTags' --type=xml | head -60Repository: Searchweb-Dev/Searchweb-Back
Length of output: 2255
🏁 Script executed:
#!/bin/bash
# Search for insertAndSelectTags usage in Java/Kotlin code
echo "=== Searching for insertAndSelectTags usage in code ==="
rg -n 'insertAndSelectTags' --type=java --type=kotlin -B2 -A5Repository: Searchweb-Dev/Searchweb-Back
Length of output: 3422
insertMemberTag 매퍼 정의는 사용되지 않고 있으므로 제거 권장
insertMemberTag는 BookmarkDao 인터페이스에 정의되지 않았으며, 코드 어디에서도 참조되지 않습니다. 동일한 기능(ON CONFLICT로 삽입/갱신)을 수행하는 insertAndSelectTags CTE 쿼리가 이미 구현되어 있고 BookmarkServiceImpl에서 실제로 사용 중입니다. 불필요한 매퍼 정의이므로 제거하시기 바랍니다.
🤖 Prompt for AI Agents
In `@src/main/resources/mapper/bookmark-mapper.xml` around lines 159 - 168, Remove
the unused mapper definition insertMemberTag from the mapper XML: delete the
<insert id="insertMemberTag" ...> block that upserts into member_tag
(owner_member_id, tag_name) with ON CONFLICT and RETURNING member_tag_id, since
BookmarkDao does not declare it and BookmarkServiceImpl uses the existing
insertAndSelectTags CTE for the same behavior; ensure no other code references
insertMemberTag before deleting and keep the insertAndSelectTags implementation
intact.
| <!-- 태그 등록 및 조회 (insert & select) --> | ||
| <!-- | ||
| 1. 새로운 태그는 INSERT 후 ID 반환 (RETURNING) | ||
| 2. 이미 존재하는 태그는 SELECT로 ID 조회 | ||
| 3. 두 결과를 UNION으로 합쳐서 모든 태그 ID 반환 | ||
| --> | ||
| <select id="insertAndSelectTags" resultType="com.web.SearchWeb.bookmark.dto.MemberTagResultDto"> | ||
| WITH input_tags AS ( | ||
| SELECT unnest(ARRAY[ | ||
| <foreach collection="tagNames" item="tagName" separator=","> | ||
| #{tagName} | ||
| </foreach> | ||
| ]) AS tag_name | ||
| ), | ||
| inserted_tags AS ( | ||
| INSERT INTO member_tag (owner_member_id, tag_name) | ||
| SELECT #{memberId}, tag_name | ||
| FROM input_tags | ||
| ON CONFLICT (owner_member_id, tag_name) | ||
| DO UPDATE SET deleted_at = NULL, updated_at = now() | ||
| RETURNING member_tag_id, tag_name | ||
| ) | ||
| SELECT member_tag_id as "memberTagId", tag_name as "tagName" FROM inserted_tags | ||
| UNION | ||
| SELECT t.member_tag_id as "memberTagId", t.tag_name as "tagName" | ||
| FROM member_tag t | ||
| JOIN input_tags i ON t.tag_name = i.tag_name | ||
| WHERE t.owner_member_id = #{memberId} | ||
| AND t.deleted_at IS NULL | ||
| </select> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
CTE 기반 태그 등록/조회 로직 검토
전반적으로 CTE를 활용한 접근은 좋습니다. 몇 가지 참고사항:
-
ON CONFLICT DO UPDATE SET deleted_at = NULL, updated_at = now()는 이미 활성 상태인(deleted_at IS NULL) 태그도 불필요하게updated_at을 갱신합니다. 대량 태그 처리 시 성능에 미미한 영향이 있을 수 있습니다. -
UNION(중복 제거)을 사용하여inserted_tags와 기존 태그 조회 결과를 합치는데, PostgreSQL의ON CONFLICT DO UPDATE+RETURNING은 충돌 시에도 행을 반환하므로 하단 SELECT와 중복될 수 있습니다. 기능적으로는UNION이 중복을 제거하므로 정확하지만,RETURNING만으로 충분할 수 있습니다.
♻️ RETURNING만 사용하는 간소화 버전 (선택사항)
ON CONFLICT DO UPDATE + RETURNING은 INSERT와 UPDATE 모두에서 행을 반환하므로, 하단 UNION SELECT가 불필요할 수 있습니다:
<select id="insertAndSelectTags" resultType="com.web.SearchWeb.bookmark.dto.MemberTagResultDto">
WITH input_tags AS (
SELECT unnest(ARRAY[
<foreach collection="tagNames" item="tagName" separator=",">
#{tagName}
</foreach>
]) AS tag_name
),
inserted_tags AS (
INSERT INTO member_tag (owner_member_id, tag_name)
SELECT #{memberId}, tag_name
FROM input_tags
ON CONFLICT (owner_member_id, tag_name)
DO UPDATE SET deleted_at = NULL, updated_at = now()
RETURNING member_tag_id, tag_name
)
SELECT member_tag_id as "memberTagId", tag_name as "tagName" FROM inserted_tags
- UNION
- SELECT t.member_tag_id as "memberTagId", t.tag_name as "tagName"
- FROM member_tag t
- JOIN input_tags i ON t.tag_name = i.tag_name
- WHERE t.owner_member_id = #{memberId}
- AND t.deleted_at IS NULL
</select>📝 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.
| <!-- 태그 등록 및 조회 (insert & select) --> | |
| <!-- | |
| 1. 새로운 태그는 INSERT 후 ID 반환 (RETURNING) | |
| 2. 이미 존재하는 태그는 SELECT로 ID 조회 | |
| 3. 두 결과를 UNION으로 합쳐서 모든 태그 ID 반환 | |
| --> | |
| <select id="insertAndSelectTags" resultType="com.web.SearchWeb.bookmark.dto.MemberTagResultDto"> | |
| WITH input_tags AS ( | |
| SELECT unnest(ARRAY[ | |
| <foreach collection="tagNames" item="tagName" separator=","> | |
| #{tagName} | |
| </foreach> | |
| ]) AS tag_name | |
| ), | |
| inserted_tags AS ( | |
| INSERT INTO member_tag (owner_member_id, tag_name) | |
| SELECT #{memberId}, tag_name | |
| FROM input_tags | |
| ON CONFLICT (owner_member_id, tag_name) | |
| DO UPDATE SET deleted_at = NULL, updated_at = now() | |
| RETURNING member_tag_id, tag_name | |
| ) | |
| SELECT member_tag_id as "memberTagId", tag_name as "tagName" FROM inserted_tags | |
| UNION | |
| SELECT t.member_tag_id as "memberTagId", t.tag_name as "tagName" | |
| FROM member_tag t | |
| JOIN input_tags i ON t.tag_name = i.tag_name | |
| WHERE t.owner_member_id = #{memberId} | |
| AND t.deleted_at IS NULL | |
| </select> | |
| <!-- 태그 등록 및 조회 (insert & select) --> | |
| <!-- | |
| 1. 새로운 태그는 INSERT 후 ID 반환 (RETURNING) | |
| 2. 이미 존재하는 태그는 SELECT로 ID 조회 | |
| 3. 두 결과를 UNION으로 합쳐서 모든 태그 ID 반환 | |
| --> | |
| <select id="insertAndSelectTags" resultType="com.web.SearchWeb.bookmark.dto.MemberTagResultDto"> | |
| WITH input_tags AS ( | |
| SELECT unnest(ARRAY[ | |
| <foreach collection="tagNames" item="tagName" separator=","> | |
| #{tagName} | |
| </foreach> | |
| ]) AS tag_name | |
| ), | |
| inserted_tags AS ( | |
| INSERT INTO member_tag (owner_member_id, tag_name) | |
| SELECT #{memberId}, tag_name | |
| FROM input_tags | |
| ON CONFLICT (owner_member_id, tag_name) | |
| DO UPDATE SET deleted_at = NULL, updated_at = now() | |
| RETURNING member_tag_id, tag_name | |
| ) | |
| SELECT member_tag_id as "memberTagId", tag_name as "tagName" FROM inserted_tags | |
| </select> |
🤖 Prompt for AI Agents
In `@src/main/resources/mapper/bookmark-mapper.xml` around lines 171 - 200, In the
select mapped by id "insertAndSelectTags" the ON CONFLICT DO UPDATE currently
always sets updated_at, causing unnecessary updates; change the conflict clause
on the INSERT into member_tag (CTE inserted_tags) to only perform the UPDATE
when the existing row is soft-deleted (add a WHERE condition like "WHERE
member_tag.deleted_at IS NOT NULL" on the DO UPDATE) so active rows aren't
touched, and optionally simplify the final result by removing the UNION and
returning only from inserted_tags (since RETURNING from the INSERT/ON CONFLICT
will yield both inserted and updated rows) to avoid duplicate work.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ba9db4e64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // 기존 링크 조회 | ||
| Link existingLink = bookmarkDao.selectLinkByCanonicalUrl(canonicalUrl); | ||
| Link existingLink = bookmarkDao.selectLinkByUrl(url); |
There was a problem hiding this comment.
현재 getOrCreateLink에서 selectLinkByUrl 호출 시 raw URL(url)을 넘기고 있는데, 매퍼 쿼리는 canonical_url 컬럼을 기준으로 조회합니다.
그래서 쿼리스트링/프래그먼트/마지막 슬래시만 다른 URL은 같은 링크여도 기존 레코드를 찾지 못합니다.
그 다음 로직에서 insert는 정규화된 canonicalUrl로 수행되기 때문에, 이미 존재하는 canonical URL과 충돌하여 unique 제약 오류가 발생할 수 있고, 결과적으로 이미 저장된 링크인데도 북마크 생성이 실패합니다.
수정 방향
getOrCreateLink에서 selectLinkByUrl(url) 대신 selectLinkByUrl(canonicalUrl)을 사용하세요.
같은 패턴으로 checkBookmarkExistsByUrl도 canonical URL 기준으로 조회하도록 맞춰주세요.
Useful? React with 👍 / 👎.
| if (bookmarkDto.getMemberFolderId() == null) { | ||
| bookmarkDto.setMemberFolderId(1L); // 임시 하드코딩 값 | ||
| log.warn("memberFolderId가 null이어서 임시 기본값(1)을 사용합니다. 링크 분석 및 폴더 서비스 연동 후 제거 필요."); |
There was a problem hiding this comment.
현재 memberFolderId가 null일 때 서비스가 1L을 하드코딩해서 사용하고 있습니다.
이 경우(특히 mypage submit 경로처럼 폴더 필드를 보내지 않는 요청) 모든 북마크가 동일한 폴더 ID로 들어가거나, 해당 row가 없으면 실패할 수 있습니다.
또한 folder_id는 identity key이므로 전역 상수처럼 쓰면 안 됩니다.
잘못하면 다른 멤버의 폴더에 북마크가 연결되어 폴더 단위 기능(조회/필터/권한)이 깨질 수 있습니다.
수정 방향
memberFolderId == null이면 1L 같은 전역 상수가 아니라,
요청한 사용자(memberId)의 실제 기본 폴더 ID를 조회해서 사용해야 합니다.
기본 폴더가 없으면 저장을 진행하지 말고 명확한 예외(또는 비즈니스 에러)를 반환하세요.
Useful? React with 👍 / 👎.
| } catch (DataIntegrityViolationException e) { | ||
| // TODO: 글로벌 예외 처리 구현 후, 커스텀에러 던지도록 변경 | ||
| // 현재는 임시로 0 반환 (DB UNIQUE 제약 uq_member_saved_link_folder_link 위반 시) | ||
| log.warn("북마크 중복 저장 시도: memberId={}, folderId={}, linkId={}", | ||
| bookmarkDto.getCreatedByMemberId(), bookmarkDto.getMemberFolderId(), link.getLinkId()); |
There was a problem hiding this comment.
**
현재의 넓은 catch(DataIntegrityViolationException)가 북마크 insert와 태그 저장을 모두 감싸면서 0을 반환하고,
그 결과 중복이 아닌 실패에 대해서도 롤백 동작이 억제됩니다.
만약 북마크 insert는 성공한 뒤 태그 저장에서 실패하면,
트랜잭션은 북마크를 커밋할 수 있는데 API는 실패/중복으로 보고하게 되어 부분 상태(partial state)가 남을 수 있습니다.
따라서 이 핸들러는 북마크 중복 제약에 해당하는 경우로만 제한하거나,
그 외 무결성 예외는 다시 던져(re-throw) 트랜잭션이 롤백되도록 해야 합니다.
Useful? React with 👍 / 👎.
- Controller 전용 DTO(BookmarkRequests) 도입 - BookmarkRequests 에서 생성, 조회, 수정, 삭제 등 DTO 그룹화 - Service 내부에서 Builder 패턴을 통한 Bookmark 엔티티 생성 로직 적용 - 커스텀에러 적용
- Controller 전용 DTO(BookmarkRequests) 도입 - Service 입력 Command 도입 - 커스텀에러 적용 - 조회 SQL 가독성 및 조건 구조 개선 - 프론트 수정
- Controller 전용 DTO(BookmarkRequests) 도입 - 커스텀에러 적용
- Controller 전용 DTO(BookmarkRequests) 도입 - 커스텀에러 적용
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bef39e4dd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| bookmarkDao.deleteBookmarkTags(bookmarkId, memberId); | ||
|
|
||
| // 4. 새 태그 등록 및 관계 생성/재활성화 | ||
| if (tags != null && !tags.isBlank()) { |
There was a problem hiding this comment.
This update path deletes all existing bookmark-tag relations before it checks whether any replacement tags were provided, so requests that omit tags (for example, older clients or edit flows that submit only title/note changes) will silently erase all tags on that bookmark. Because the deletion happens unconditionally and reinsertion is conditional, unrelated edits can cause permanent tag loss unless tags is always populated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7877f9bafb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| url: url, // DTO has url field | ||
| note: description, | ||
| tag: tag, | ||
| tags: tag, |
There was a problem hiding this comment.
This request now sends tags to the backend, but the edit form still populates the input from response.tag (not response.tags), so opening an existing tagged bookmark and saving a title/note-only change can submit an empty tag string and clear all tag links. Because updateBookmark treats an empty tags payload as “delete all tags,” this causes silent tag loss during normal edits.
Useful? React with 👍 / 👎.
| return ResponseEntity.ok(response); | ||
| Long bookmarkId = bookmarkService.insertBookmark( | ||
| memberId, | ||
| request.url, |
There was a problem hiding this comment.
Moving URL from required query param to request.url means create calls can reach the service with null/blank URL, which is no longer rejected at request binding time. In that case insertBookmark proceeds to link creation and can fail with a server-side data error (the link table requires non-null URL columns), returning a 5xx instead of a client 4xx validation error.
Useful? React with 👍 / 👎.
1 participant
💡 이슈
resolve {#이슈번호}
🤩 개요
PR의 개요를 적어주세요.
🧑💻 작업 사항
작업한 내용을 적어주세요.
📖 참고 사항
공유할 내용, 레퍼런스, 추가로 발생할 것으로 예상되는 이슈, 스크린샷 등을 넣어 주세요.
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Refactor