[volume-5] 상품 목록 조회 성능, 좋아요 수 정렬 구조 개선 및 인덱스,캐시 적용 #200
Open
plan11plan wants to merge 27 commits intoLoopers-dev-lab:plan11planfrom
Open
[volume-5] 상품 목록 조회 성능, 좋아요 수 정렬 구조 개선 및 인덱스,캐시 적용 #200plan11plan wants to merge 27 commits intoLoopers-dev-lab:plan11planfrom
plan11plan wants to merge 27 commits intoLoopers-dev-lab:plan11planfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- FashionDataPool 신규: 브랜드 100개(실제 패션 브랜드명), 카테고리 7개, 상품명/가격/재고 생성 - BulkDataGeneratorService: 소프트 딜리트 Phase 추가 (브랜드 10%, 상품 ~15% 2-wave), like_count 동기화, 멱등성 수정 - DataGeneratorRepository: 소프트 딜리트 배치, 전체 카운트, 삭제 브랜드 조회 메서드 추가 - README 업데이트: 패션 데이터 설계, 소프트 딜리트 정책, 생성 흐름, 검증 SQL Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough캐싱, 상품 이미지 관리, 좋아요 카운트 저장소화, 관리자 UI, 데이터 생성 도구, 쇼핑 프론트엔드를 추가한다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant ProductV1Controller
participant ProductFacade
participant ProductService
participant ProductImageService
participant ProductRepository
participant ImageRepository
participant DB
Client->>ProductV1Controller: GET /api/v1/products/{id}
ProductV1Controller->>ProductFacade: getProductDetail(productId)
ProductFacade->>ProductService: getById(productId)
ProductService->>ProductRepository: findById(productId)
ProductRepository->>DB: SELECT product
DB-->>ProductRepository: ProductModel
ProductRepository-->>ProductService: ProductModel
ProductService-->>ProductFacade: ProductModel
ProductFacade->>ProductImageService: getImagesByProductId(productId)
ProductImageService->>ImageRepository: findAllByProductId(productId)
ImageRepository->>DB: SELECT images WHERE product_id=?
DB-->>ImageRepository: ProductImageModels (MAIN/DETAIL)
ImageRepository-->>ProductImageService: Filtered by type
ProductImageService-->>ProductFacade: List<ProductImageModel> (MAIN), List<ProductImageModel> (DETAIL)
ProductFacade->>ProductFacade: Build DetailWithImages(product, mainImages, detailImages)
ProductFacade-->>ProductV1Controller: DetailWithImages
ProductV1Controller->>ProductV1Controller: Map to DetailResponse
ProductV1Controller-->>Client: ApiResponse with images
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60분 Possibly related PRs
검토 시 주의 사항운영 관점 고려 사항
보안 관점 고려 사항
성능 관점 고려 사항
테스트 관점 고려 사항
결론대규모 기능 추가로 인해 캐싱, 이미지 관리, 데이터 생성, 사용자 UI 등 여러 도메인에 걸친 변경이 이루어졌다. 운영 안정성, 보안, 성능, 테스트 측면에서 위의 지적 사항들을 검토하고 보완이 필요하다. 특히 데이터 생성 자동화 활성화 제어, 인증 정보 보안, 동시성 제어, 캐시 관리 전략에 우선순위를 두고 개선하길 권장한다. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/commerce-api/src/main/java/com/loopers/interfaces/brand/AdminBrandV1Controller.java (1)
30-33:⚠️ Potential issue | 🟡 Minorpage, size 파라미터에 대한 입력 검증이 필요하다.
음수 값이 들어오면
PageRequest.of()에서IllegalArgumentException이 발생한다. 이 예외는CoreException을 통한 통합 에러 핸들링 경로를 벗어나 일관되지 않은 에러 응답이 반환될 수 있다.🛡️ 개선 제안
`@GetMapping` `@Override` public ApiResponse<AdminBrandV1Dto.ListResponse> list( - `@RequestParam`(defaultValue = "0") int page, - `@RequestParam`(defaultValue = "20") int size + `@RequestParam`(defaultValue = "0") `@Min`(0) int page, + `@RequestParam`(defaultValue = "20") `@Min`(1) `@Max`(100) int size ) {또는 Service 레이어에서
CoreException으로 래핑하여 처리한다.Based on learnings: "enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/brand/AdminBrandV1Controller.java` around lines 30 - 33, Validate the page and size request parameters in AdminBrandV1Controller.list before calling PageRequest.of: ensure page >= 0 and size > 0 (or other project semantics) and if invalid throw a CoreException (or wrap the IllegalArgumentException) so the error flows through ApiControllerAdvice; update the controller method (AdminBrandV1Controller.list) to perform these checks and raise CoreException with a clear message/code rather than allowing PageRequest.of to throw directly.apps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.java (1)
127-153:⚠️ Potential issue | 🟡 Minor좋아요 정렬 테스트에서 실제 정렬 순서 검증이 누락되어 있다.
GetProductsWithActiveBrandSortedByLikes테스트에서 결과 개수와 페이지네이션 정보만 검증하고, 실제로 좋아요 수 내림차순으로 정렬되었는지는 검증하지 않는다. 정렬 로직 버그가 있어도 이 테스트로는 발견할 수 없다.반환된 상품들의
likeCount가 내림차순인지 명시적으로 검증하는 assertion을 추가해야 한다.💚 수정 제안
// assert assertThat(result.getContent()).hasSize(2); assertThat(result.getTotalElements()).isEqualTo(3); assertThat(result.getTotalPages()).isEqualTo(2); +assertThat(result.getContent()) + .extracting(ProductResult::likeCount) + .isSortedAccordingTo(Comparator.reverseOrder());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.java` around lines 127 - 153, The test getProductsSortedByLikes_returnsSortedAndPaginated in the GetProductsWithActiveBrandSortedByLikes nested class only checks pagination metadata but not the ordering; update this test to also verify that the returned Page<ProductResult> from productFacade.getProductsWithActiveBrandSortedByLikes(0, 2) is ordered by like count descending by extracting the likeCount values from result.getContent() (ProductResult.likeCount or from mapped ProductModel) and asserting they are in non‑increasing order so sorting regressions are caught.
🟠 Major comments (25)
.gitignore-48-49 (1)
48-49:⚠️ Potential issue | 🟠 Major
src/main/resources/static전체 무시는 향후 정적 리소스 누락을 초래한다.현재 저장소에는 admin/shop UI를 위한 21개의 HTML/CSS/JS 파일이
src/main/resources/static하위에 소스 관리되고 있다. 이 변경으로 추가되는.gitignore패턴**/src/main/resources/static/은 디렉터리 전체를 무시하므로, 향후 추가되는 새로운 정적 리소스는 Git에 포함되지 않는다. 기존 추적 파일은 유지되지만, 배포 전에 누락을 발견하지 못하면 운영 환경에서 정적 자산이 누락되어 UI 404/깨짐 현상으로 이어진다.생성 산출물만 제외하는 것이 목적이라면 무시 범위를 실제 빌드 출력 디렉터리(예:
generated/,dist/)로 좁혀야 한다. 변경 후에는 "새로 추가한 정적 파일이 커밋되는지" 확인하는 테스트를 CI에 추가하는 것이 안전하다.수정 예시
### Static Resources ### -**/src/main/resources/static/ +**/src/main/resources/static/generated/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 48 - 49, 현재 추가된 .gitignore 패턴 '**/src/main/resources/static/'가 소스관리 중인 정적 리소스 전체를 무시하여 향후 정적 파일이 Git에 추가되지 않게 됩니다; .gitignore에서 해당 패턴을 제거하거나 좁혀서(예: 빌드 출력 디렉터리인 generated/, dist/, 또는 target/ 이하만 무시) 정적 소스(src/main/resources/static/)는 계속 추적되게 변경하고, 필요하다면 기존으로 추적되던 파일들이 누락되지 않도록 .gitignore에서 '**/src/main/resources/static/' 라인을 삭제 또는 대체해 주세요.apps/commerce-api/src/main/java/com/loopers/support/cache/RedisCacheHelper.java-56-65 (1)
56-65:⚠️ Potential issue | 🟠 Major
keys()명령은 운영 환경에서 Redis를 블로킹하여 전체 서비스 장애를 유발할 수 있다.
KEYS명령은 O(N) 시간복잡도로 전체 키스페이스를 스캔하며, 대용량 데이터 환경에서 Redis 서버를 수 초간 블로킹한다. 이 메서드가 호출되는 순간 다른 모든 Redis 요청이 대기하게 되어 서비스 전체 장애로 이어진다.
SCAN명령을 사용하여 커서 기반으로 점진적 삭제를 수행해야 한다. 현재 이 메서드가 호출되지 않고 있으나, 향후 사용될 때 발생할 수 있는 장애를 미리 방지해야 한다.🔧 SCAN 기반 점진적 삭제로 수정
public void deleteByPattern(String pattern) { try { - Set<String> keys = redisTemplate.keys(pattern); - if (keys != null && !keys.isEmpty()) { - redisTemplate.delete(keys); - } + redisTemplate.execute((RedisCallback<Void>) connection -> { + try (Cursor<byte[]> cursor = connection.scan( + ScanOptions.scanOptions().match(pattern).count(100).build())) { + while (cursor.hasNext()) { + connection.del(cursor.next()); + } + } + return null; + }); } catch (Exception e) { log.warn("Redis DELETE 패턴 실패, 무시: pattern={}", pattern, e); } }추가 import 필요:
import org.springframework.data.redis.core.Cursor; import org.springframework.data.redis.core.RedisCallback; import org.springframework.data.redis.core.ScanOptions;추가로 pattern 파라미터에 대한 null 검증을 추가하여 예상 밖 동작을 방지하고, 키 패턴이 민감정보를 포함한다면 로깅 레벨을 조정하도록 검토한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/support/cache/RedisCacheHelper.java` around lines 56 - 65, The deleteByPattern method in RedisCacheHelper currently uses redisTemplate.keys(pattern) which blocks Redis in production; change it to perform a cursor-based SCAN iteration using RedisCallback<Cursor<byte[]>> and ScanOptions.match(pattern) to collect and delete keys in batches (e.g., accumulate a small batch and call redisTemplate.delete(Collection) periodically) so deletion is incremental and non-blocking; add a null/blank check for the pattern at method start and avoid logging sensitive key contents (or lower log level) when pattern may contain sensitive data; ensure you import Cursor, RedisCallback and ScanOptions and update the deleteByPattern implementation to use the SCAN-based loop and batched deletes.apps/commerce-api/src/main/java/com/loopers/interfaces/user/UserV1Controller.java-20-20 (1)
20-20:⚠️ Potential issue | 🟠 MajorController가 Domain Service를 직접 사용하고 있다.
기존 엔드포인트들은
UserFacade(Application 계층)를 통해 도메인 로직에 접근하는데, 새로 추가된 포인트 관련 엔드포인트는UserService(Domain 계층)를 직접 호출한다. 계층 간 의존 방향이 일관되지 않으면 다음 문제가 발생한다:
- 트랜잭션 경계 관리가 분산되어 장애 추적이 어려워진다
- AOP 기반 횡단 관심사(로깅, 캐싱 등)가 Facade에만 적용되면 누락된다
- 코드베이스 일관성이 깨져 유지보수 비용이 증가한다
포인트 관련 로직을
UserFacade로 이동하거나, 별도의PointFacade를 생성하여 일관된 계층 구조를 유지해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/user/UserV1Controller.java` at line 20, UserV1Controller currently depends directly on the domain-level UserService for the newly added point endpoints, breaking the existing application-layer boundary (other endpoints use UserFacade); refactor the controller to call the application layer instead by moving the point-related logic into UserFacade (or create a new PointFacade) and replace the UserService injection/usage in UserV1Controller with the facade method(s) so transactional/AOP concerns and layering remain consistent (update constructor/field to inject UserFacade/PointFacade and delegate point operations to facade methods).apps/commerce-api/src/main/java/com/loopers/interfaces/user/UserV1Controller.java-51-59 (1)
51-59:⚠️ Potential issue | 🟠 Major두 개의 독립적인 트랜잭션으로 인한 데이터 불일치 위험이 있다.
addPoint()와getById()가 별도 트랜잭션으로 실행되어 다음 문제가 발생할 수 있다:
- Race condition: 동시 요청 시
addPoint()커밋 후getById()호출 전에 다른 요청이 포인트를 변경하면, 반환되는 포인트 값이 실제 충전 결과와 다를 수 있다- 불필요한 DB 조회: 이미
addPoint()에서 조회한 사용자를 다시 조회한다- 트랜잭션 일관성: 충전과 조회가 하나의 원자적 연산이어야 정확한 결과를 보장한다
운영 환경에서 포인트 관련 문의 발생 시 원인 파악이 어려워질 수 있다. Facade 계층에서 단일 트랜잭션으로 처리하고 결과를 반환하도록 리팩토링해야 한다.
♻️ 단일 트랜잭션 처리 제안 (Facade 계층)
// UserFacade에 추가 `@Transactional` public UserResult chargePoint(Long userId, long amount) { userService.addPoint(userId, amount); UserModel user = userService.getById(userId); return UserResult.from(user); }// Controller 수정 `@PostMapping`("/me/point") public ApiResponse<UserV1Dto.PointResponse> chargePoint( `@Login` LoginUser loginUser, `@Valid` `@RequestBody` UserV1Dto.ChargePointRequest request ) { - userService.addPoint(loginUser.id(), request.amount()); - long currentPoint = userService.getById(loginUser.id()).getPoint(); - return ApiResponse.success(new UserV1Dto.PointResponse(currentPoint)); + UserResult result = userFacade.chargePoint(loginUser.id(), request.amount()); + return ApiResponse.success(new UserV1Dto.PointResponse(result.point())); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/user/UserV1Controller.java` around lines 51 - 59, The controller currently calls userService.addPoint(...) and then userService.getById(... ) as separate operations, causing race conditions and extra DB reads; move the charge+fetch into a single transactional facade method (e.g., add a UserFacade.chargePoint(Long userId, long amount) annotated with `@Transactional` that calls userService.addPoint(userId, amount) then userService.getById(userId) and returns a result/DTO), then update UserV1Controller.chargePoint to call UserFacade.chargePoint(...) and return the returned PointResponse so the add and read happen in one atomic transaction.apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java-15-15 (1)
15-15:⚠️ Potential issue | 🟠 Major관리자 API의 제한 없는 사용자 전체 조회는 성능 장애 위험이 있다.
getAllUsers()는 삭제되지 않은 모든 사용자를 메모리에 한 번에 로드하며, AdminUserV1Controller:53에서 관리자 기능으로 호출된다. 사용자 테이블 크기가 증가하면 메모리 부족 또는 GC 지연으로 이어질 수 있다.페이징 처리가 이미 구현되어 있으므로(
getUsers(Pageable pageable)), 해당 메서드를 사용하도록 관리자 컨트롤러를 수정해야 한다. 만약 전체 데이터가 필요한 경우라면 스트림 기반 처리로 배치 단위 로드를 검토해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java` at line 15, The repository method UserJpaRepository.findAllByDeletedAtIsNull() (used by getAllUsers()) loads all non-deleted users into memory and risks OOM/GC issues for large tables; update AdminUserV1Controller to stop calling getAllUsers() and instead call the paginated service method getUsers(Pageable pageable) (or pass through pageable from the controller) to fetch users page-by-page; if truly all records are required, replace the eager-load approach with a streaming/batched load (e.g., repository stream or chunked queries) in the service layer rather than using findAllByDeletedAtIsNull().apps/commerce-api/src/main/resources/static/shop/js/auth.js-22-29 (1)
22-29:⚠️ Potential issue | 🟠 Major보안 위험: 매 요청마다 비밀번호를 HTTP 헤더로 전송
X-Loopers-LoginPw헤더로 비밀번호를 매 요청마다 전송하는 것은 다음 문제가 있다:
- 네트워크 로그/프록시에서 비밀번호 노출 가능성
- 브라우저 개발자 도구 Network 탭에서 모든 요청의 비밀번호 확인 가능
- HTTPS 환경이라도 서버 로그에 헤더가 기록될 수 있음
프로덕션 환경에서는 인증 토큰 기반 방식으로 전환해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/shop/js/auth.js` around lines 22 - 29, getHeaders() currently reads the raw password from this.get() and places it into the X-Loopers-LoginPw header each request; remove sending the password and switch to a token-based approach: update the login flow to exchange credentials for a short-lived auth token (JWT or opaque token), store only the token via the existing set()/get() storage, and change getHeaders() to return an Authorization: Bearer <token> (or X-Loopers-Auth-Token) header instead of X-Loopers-LoginPw; also ensure logout clears the token and update any server-side endpoints to accept the token header rather than raw password.apps/commerce-api/src/main/resources/static/shop/js/auth.js-10-12 (1)
10-12:⚠️ Potential issue | 🟠 Major보안 위험: localStorage에 평문 비밀번호 저장
localStorage에 비밀번호를 평문으로 저장하는 것은 XSS(Cross-Site Scripting) 공격 발생 시 인증 정보가 탈취될 수 있는 심각한 보안 취약점이다.localStorage는 JavaScript에서 직접 접근 가능하므로, 악성 스크립트가 삽입되면 모든 사용자의 비밀번호가 노출된다.운영 관점 문제점:
- XSS 취약점과 결합 시 대규모 계정 탈취 가능
- 브라우저 개발자 도구에서 비밀번호 직접 확인 가능
- 공용 PC 사용 시 비밀번호 잔류 위험
권장 수정안:
- 세션 기반 인증 또는 JWT 토큰 방식으로 전환
- HttpOnly 쿠키 사용 검토
- 최소한 개발/데모 환경 전용임을 명시하고, 프로덕션 배포 전 반드시 변경 필요
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/shop/js/auth.js` around lines 10 - 12, The save(loginId, password, name) helper currently persists plaintext passwords to localStorage under STORAGE_KEY; remove storing password entirely and stop writing sensitive creds to localStorage in the save function (and any callers). Instead persist only a non-secret identifier or an auth token issued by the server (e.g., authToken or sessionId), or rely on HttpOnly cookies/session storage handled by the backend; if this code is demo-only, add an explicit DEV/DEMO guard and a prominent comment warning not to use in production. Update any code that reads STORAGE_KEY to expect the token/identifier shape and fail-safe when absent.apps/commerce-api/src/main/resources/static/shop/js/api.js-31-31 (1)
31-31:⚠️ Potential issue | 🟠 Major고정된 문자열 기반의 약한 관리자 인증
ADMIN_HEADER에 관리자 LDAP 값이 하드코딩되어 있으며, 클라이언트 측 JavaScript는 브라우저에서 누구나 확인할 수 있다. 서버 측AdminAuthFilter에서는 이 값을 검증하지만, 실제 LDAP 백엔드 연동 없이 단순 문자열 비교만 수행하고 있다. 결과적으로 현재 인증 메커니즘은 공개된 공유 비밀(shared secret)에 의존하는 약한 설계이며, 실제 사용자 신원을 검증하지 않는다.운영 환경에서는 다음을 반드시 개선해야 한다:
- 실제 LDAP 서버 또는 기업 인증 시스템(SSO, OAuth 등) 연동
- 클라이언트에서 관리자 인증 정보를 직접 구성하지 않도록 설계 변경
- 서버에서 발급한 세션/토큰 기반의 인증으로 전환
- 관리자 API 접근 로그 및 감시 기능 추가
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/shop/js/api.js` at line 31, ADMIN_HEADER is a hardcoded client-side shared secret and AdminAuthFilter currently validates by simple string comparison; remove the client-side hardcoded ADMIN_HEADER usage and replace the flow with server-issued session/token-based auth: stop sending a static X-Loopers-Ldap header from api.js, implement a login endpoint that authenticates against a real identity provider (LDAP/SSO/OAuth) and issues a secure session cookie or JWT, update AdminAuthFilter to validate the server-side session/token with the ID provider (not by comparing a string) and add centralized admin access logging/auditing in the filter to record user identity and actions for all admin API calls.apps/commerce-api/src/main/java/com/loopers/interfaces/user/AdminUserV1Controller.java-49-60 (1)
49-60:⚠️ Potential issue | 🟠 Major전체 사용자 포인트 지급 로직이 성능과 원자성 문제를 가진다.
- 메모리 문제:
getAllUsers()가 전체 사용자를 메모리에 로드한다. 대용량 데이터(10만+ 사용자) 환경에서 OOM 위험이 있다.- N+1 호출: 사용자별로
addPoint를 개별 호출하면 N번의 DB 트랜잭션이 발생한다.- 원자성 부재: 중간에 실패 시 일부 사용자만 포인트가 지급되고 롤백되지 않는다.
- 비즈니스 로직 위치: 컨트롤러에 비즈니스 로직이 포함되어 있다. 서비스 레이어로 이동해야 한다.
🔧 서비스 레이어로 이동 및 배치 처리 제안
서비스에 배치 처리 메서드 추가:
// UserService에 추가 `@Transactional` public int addPointToAllUsers(long amount) { // 배치 UPDATE 또는 페이징 처리로 구현 return userRepository.addPointToAll(amount); }컨트롤러 수정:
`@PostMapping`("/point") public ApiResponse<AdminUserV1Dto.AddPointResponse> addPointToAll( `@Valid` `@RequestBody` AdminUserV1Dto.AddPointAllRequest request ) { - List<UserModel> users = userService.getAllUsers(); - for (UserModel user : users) { - userService.addPoint(user.getId(), request.amount()); - } + int updatedCount = userService.addPointToAllUsers(request.amount()); return ApiResponse.success( - new AdminUserV1Dto.AddPointResponse(users.size(), request.amount(), - "전체 " + users.size() + "명에게 " + request.amount() + " 포인트를 지급했습니다.")); + new AdminUserV1Dto.AddPointResponse(updatedCount, request.amount(), + "전체 " + updatedCount + "명에게 " + request.amount() + " 포인트를 지급했습니다.")); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/interfaces/user/AdminUserV1Controller.java` around lines 49 - 60, The controller method AdminUserV1Controller.addPointToAll currently loads all users via userService.getAllUsers and iterates calling userService.addPoint per user causing memory blowup, N+1 DB calls and no atomicity; move this logic to the service layer by adding a transactional batch method (e.g., UserService.addPointToAllUsers(long amount)) that performs a single bulk update (or paginated updates) via userRepository.addPointToAll(amount) or equivalent, return the affected row count, and have the controller call that service method and return its result; ensure the service method is annotated with `@Transactional` and uses repository-level bulk update or paged processing to avoid OOM and provide atomicity.apps/commerce-api/src/main/resources/static/admin/js/components/products.js-138-138 (1)
138-138:⚠️ Potential issue | 🟠 Major
esc()함수가 XSS 방어에 불충분하다 (다른 파일과 동일한 문제).동일한 보안 취약점이 존재한다. 공통 유틸리티 모듈로 추출하여 일괄 수정을 권장한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/admin/js/components/products.js` at line 138, The esc(s) function is insufficient for XSS mitigation because it only replaces single and double quotes and not &, <, > etc.; extract esc into a shared utility (e.g., sanitizeHtml or escapeHtml) and replace all occurrences (including esc in products.js) with this centralized function that escapes &, <, >, ", ' (and optionally /) or, preferably, use safe DOM APIs (textContent/innerText) where user input is inserted; update callers to import the new utility and remove the ad-hoc local esc implementation so all files use the consistent secure routine.apps/commerce-api/src/main/resources/static/admin/js/components/orders.js-73-73 (1)
73-73:⚠️ Potential issue | 🟠 Major
esc()함수가 XSS 방어에 불충분하다 (users.js와 동일한 문제).
<,>,&문자 이스케이프가 누락되어 있다. 상품명, 브랜드명에 악의적인 스크립트가 포함될 경우 XSS 공격이 가능하다.수정안은 users.js의
esc()함수 수정안과 동일하다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/admin/js/components/orders.js` at line 73, The esc(s) function currently only escapes single and double quotes and misses escaping of <, > and &, enabling XSS via product/brand names; update the esc function (esc) to first coerce null/undefined to empty string and then replace ampersand (&) with &, less-than (<) with <, greater-than (>) with >, and then replace single quote (') and double quote (") (mirror the users.js fix) so all these characters are HTML-escaped in the output.apps/commerce-api/src/main/resources/static/admin/js/components/users.js-120-120 (1)
120-120:⚠️ Potential issue | 🟠 Major
esc()함수가 XSS 방어에 불충분하다.현재 구현은
'와"만 이스케이프하며,<,>,&문자를 처리하지 않는다. 악의적인 사용자 데이터(loginId, name, email 등)에<script>태그가 포함되면 XSS 공격이 가능하다.운영 관점: 관리자 콘솔은 민감한 작업(포인트 지급 등)을 수행하므로, XSS를 통한 권한 탈취 위험이 크다.
수정안: HTML 엔티티 이스케이프 추가
-function esc(s) { return (s || '').replace(/'/g, "\\'").replace(/"/g, '"'); } +function esc(s) { + return (s || '') + .replace(/&/g, '&') + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +}추가 테스트: loginId에
<img src=x onerror=alert(1)>입력 후 렌더링 결과 확인이 필요하다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/admin/js/components/users.js` at line 120, The esc(s) helper only escapes single and double quotes and misses HTML-sensitive characters, enabling XSS; update the esc function (esc) to also replace &, <, and > with their HTML entities (&, <, >) (and optionally slash / if you want extra protection) or replace it entirely by using a safe text node/DOM text encoder approach; ensure the function still handles null/undefined and returns a string, and update any call sites that assume quote-only escaping (e.g., where loginId, name, email are inserted) to use this strengthened esc to prevent payloads like <img src=x onerror=...> from executing.apps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorRunner.java-16-19 (1)
16-19:⚠️ Potential issue | 🟠 Major
generateAll()동기 호출로 인해 애플리케이션 시작이 장시간 블로킹될 수 있다.설정된 데이터량(100 브랜드, 10만 상품, 50만 좋아요 등)을 생성하는 동안 애플리케이션이 준비 상태가 되지 않는다. 또한 예외 발생 시 애플리케이션 시작 자체가 실패한다.
운영 관점:
- Health check 타임아웃으로 컨테이너 재시작 루프 발생 가능
- 데이터 생성 실패 시 원인 파악이 어려움 (로깅 부재)
수정안: 비동기 실행 및 예외 처리 추가
+import lombok.extern.slf4j.Slf4j; +import org.springframework.scheduling.annotation.Async; + `@Component` `@RequiredArgsConstructor` +@Slf4j `@ConditionalOnProperty`(name = "app.data-generator.enabled", havingValue = "true") public class BulkDataGeneratorRunner implements ApplicationRunner { private final BulkDataGeneratorService bulkDataGeneratorService; `@Override` public void run(ApplicationArguments args) { - bulkDataGeneratorService.generateAll(); + log.info("Bulk data generation started"); + try { + bulkDataGeneratorService.generateAll(); + log.info("Bulk data generation completed"); + } catch (Exception e) { + log.error("Bulk data generation failed", e); + // 개발/테스트 환경이므로 시작은 허용하되 로그로 실패 기록 + } } }추가 테스트: 데이터 생성 중 예외 발생 시나리오에서 애플리케이션 동작 확인이 필요하다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorRunner.java` around lines 16 - 19, BulkDataGeneratorRunner currently calls bulkDataGeneratorService.generateAll() synchronously inside run(ApplicationArguments), blocking startup and bubbling exceptions; change it to invoke generateAll asynchronously (e.g., submit to a background thread/ExecutorService or annotate an async method) so run() returns immediately, and wrap the async task with try/catch to log errors and avoid crashing startup (use the same BulkDataGeneratorRunner.run, bulkDataGeneratorService.generateAll() symbol names to locate the call and add logging and exception handling around the async invocation).apps/commerce-api/src/main/resources/static/admin/js/app.js-58-69 (1)
58-69:⚠️ Potential issue | 🟠 Major페이지 변경 콜백이 한 번 클릭에 두 번 실행된다.
현재 버튼에 인라인
onclick과addEventListener를 함께 붙여서 클릭 1회에onChange가 2회 호출된다. 운영 중 목록 API가 중복 호출되어 로딩 깜빡임과 불필요한 트래픽이 발생하므로, 인라인 핸들러를 제거하고 DOM 이벤트만 바인딩해야 한다. 다음/이전 버튼을 한 번 눌렀을 때 페이지 로드가 정확히 1회만 발생하는 UI 테스트를 추가하는 편이 안전하다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/admin/js/app.js` around lines 58 - 69, The page-change callback is being invoked twice because the template string includes inline onclick attributes and you also attach handlers via addEventListener; update the pagination rendering in the block that builds html so the buttons do NOT include onclick="this._prev()" or onclick="this._next()" (remove those inline handlers from the template), keep the assignments to container.querySelector('button:first-child')._prev and ._next and the container.querySelectorAll('button') loop that adds click listeners which call onChange(currentPage ± 1), and optionally use data-attributes or clearer naming if needed to locate the button elements; ensure only the DOM event listeners trigger onChange so a single click yields one call.apps/commerce-api/src/main/resources/static/admin/js/components/data-generator.js-463-504 (1)
463-504:⚠️ Potential issue | 🟠 Major가격과 재고 범위 역전 입력을 선검증해야 한다.
현재
randInt(min, max)호출 전 검증이 없어 최소값이 최대값보다 크면 비정상 가격이나 재고가 생성될 수 있다. 운영 중 잘못된 테스트 데이터가 쌓이면 정렬·필터·재고 검증 결과까지 왜곡되므로, 각 모드에서priceMin <= priceMax,stockMin <= stockMax를 확인하고 실패 시Toast.error후 즉시 중단해야 한다. 역전 범위 입력 시 API 호출이 발생하지 않고 진행률도 시작되지 않는 테스트를 추가해야 한다.Also applies to: 845-845
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/resources/static/admin/js/components/data-generator.js` around lines 463 - 504, The generateProducts function lacks validation for inverted ranges which can produce invalid prices or stocks; add checks to ensure stockMin <= stockMax (immediately after reading stockMin/stockMax) and for each mode verify priceMin <= priceMax (for 'all' use the computed priceMin/priceMax, for 'single' and 'multi' use their respective fields) and if any check fails call Toast.error with a clear message and return before pushing tasks or making API calls so progress/ isRunning are not started; update the same validation at the other similar generator function referenced (the other occurrence around line ~845) to mirror this behavior.apps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorService.java-349-355 (1)
349-355:⚠️ Potential issue | 🟠 Major좋아요 샘플링이 목표 수를 끝까지 채우지 못한다.
attempts < count * 3제한 때문에count가 사용자 수에 가까우면 충돌이 누적되어selectedUserIndices.size()가 목표치에 못 미친다. 운영 중 인기 상품의like_count가 계속 과소 생성되어 정렬과 후속 soft-delete 기준이 왜곡되므로, 사용자 목록을 섞어 앞에서count개를 취하는 방식처럼 중복 없는 샘플링으로 바꿔야 한다. 사용자 1,000명에 좋아요 1,000개를 요청했을 때 정확히 1,000개의 like row가 생성되는 테스트를 추가해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorService.java` around lines 349 - 355, The current dedup sampling loop in BulkDataGeneratorService (variables selectedUserIndices, random, userIds, count) can fail to reach the requested count because attempts is capped; replace this logic with true deduplicated sampling by shuffling the userIds (or their indices) and selecting the first Math.min(count, userIds.size()) entries (or throw/error if count > userIds.size()), removing the attempts cap and any add-if-present behavior. Update the like-generation call site to use the shuffled selection and ensure like_count is incremented once per selected user. Add a test that creates 1,000 users and requests 1,000 likes and asserts exactly 1,000 like rows are produced.apps/commerce-api/src/main/resources/static/admin/js/components/coupons.js-64-76 (1)
64-76:⚠️ Potential issue | 🟠 Major
datetime-local기본값을 UTC로 채워 만료 시각이 어긋난다.
toISOString()은 UTC 기준이라 브라우저 로컬 시간대와 다른 값이 input에 들어간다. 운영 중 쿠폰 만료가 의도보다 몇 시간 빠르거나 늦어질 수 있으므로, 기본값은 로컬 시간 포맷터로 채우고 제출 시에도 같은 로컬 기준으로 직렬화해야 한다. 기본값 표시 시각과 서버에 전달되는 만료 시각이 동일한 로컬 시각을 유지하는 UI 테스트를 추가해야 한다.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java-31-38 (1)
31-38:⚠️ Potential issue | 🟠 Major소프트 삭제 상품도 like_count 갱신 대상에 포함된다
운영 관점에서 삭제된 상품의 집계값이 계속 변하면 비정규화 데이터가 오염되고, 복구·배치·통계 시 실제 좋아요 원본과 어긋난다. 두 UPDATE 쿼리에
p.deletedAt IS NULL조건을 추가해 활성 상품만 갱신되도록 제한해야 한다. 추가 테스트로 soft delete 된 상품에 대해incrementLikeCount와decrementLikeCount를 호출했을 때 0건만 갱신되는지 확인해야 한다.As per coding guidelines, "
**/*Repository*.java: ... 쿼리 조건 누락/과다 조회 ... 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java` around lines 31 - 38, The UPDATE queries in methods incrementLikeCount and decrementLikeCount currently affect soft-deleted rows; modify both `@Query` strings on ProductJpaRepository so they include "AND p.deletedAt IS NULL" (i.e., WHERE p.id = :id AND p.deletedAt IS NULL for increment and WHERE p.id = :id AND p.likeCount > 0 AND p.deletedAt IS NULL for decrement) to restrict updates to active ProductModel rows only, and add unit tests that call incrementLikeCount/decrementLikeCount against a soft-deleted ProductModel to assert zero rows updated.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java-26-29 (1)
26-29:⚠️ Potential issue | 🟠 Major좋아요 정렬 페이지에 고정 타이브레이커가 없다
운영 관점에서
likeCount동률 데이터가 많아지면 offset 페이지 경계가 매 요청마다 달라져 중복/누락이 생기고, 페이지 캐시도 불안정해진다.OrderByLikeCountDescIdDesc처럼 2차 정렬 키를 추가하고, 이에 맞춰 인덱스도(deleted_at, like_count, id)및(brand_id, deleted_at, like_count, id)형태로 맞추는 편이 안전하다. 추가 테스트로 동일한likeCount를 가진 상품 여러 개를 만든 뒤 연속 페이지 조회 시 순서가 고정되고 페이지 간 겹침이 없는지 검증해야 한다.As per coding guidelines, "
**/*Repository*.java: ... 정렬/인덱스 활용 가능성, 대량 데이터에서의 병목을 점검한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.java` around lines 26 - 29, 현재 메서드 findAllByDeletedAtIsNullOrderByLikeCountDesc 및 findAllByBrandIdAndDeletedAtIsNullOrderByLikeCountDesc는 likeCount 동률 시 정렬의 타이브레이커가 없어 페이지 경계가 불안정하므로 2차 정렬 키로 id를 추가해 메서드명을 각각 findAllByDeletedAtIsNullOrderByLikeCountDescIdDesc 및 findAllByBrandIdAndDeletedAtIsNullOrderByLikeCountDescIdDesc로 변경하고, DB 인덱스를 deleted_at, like_count, id 및 brand_id, deleted_at, like_count, id 순으로 생성하도록 마이그레이션을 추가하며(예: ALTER INDEX/CREATE INDEX 스크립트), 동일한 likeCount를 가진 여러 상품으로 연속 페이지 조회 테스트를 추가해 순서 고정성과 중복/누락이 없는지 검증하세요.apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java-63-70 (1)
63-70:⚠️ Potential issue | 🟠 Major0건 갱신을 성공으로 삼키면 like_count 정합성이 깨진다
운영 관점에서 존재하지 않거나 방금 삭제된 상품에 대해 UPDATE가 0건이어도 현재 흐름은 성공으로 끝나므로, 좋아요 원본 데이터와
like_count가 영구히 어긋날 수 있다.productJpaRepository.incrementLikeCount(id)와decrementLikeCount(id)의 반환값을 검사해 0이면ProductErrorCode.NOT_FOUND같은 도메인 예외를 던져 상위 트랜잭션이 롤백되도록 처리해야 한다. 추가 테스트로 없는 상품 ID 또는 삭제된 상품 ID에 대해 호출했을 때 예외가 발생하고 상위 변경도 커밋되지 않는지 검증해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.java` around lines 63 - 70, The repository methods incrementLikeCount and decrementLikeCount currently ignore the row-count returned by productJpaRepository.incrementLikeCount(id) / productJpaRepository.decrementLikeCount(id); change ProductRepositoryImpl.incrementLikeCount(Long id) and decrementLikeCount(Long id) to capture the int return value, and if it is 0 throw a domain exception indicating not-found (use the existing domain error/exception pattern, e.g. throw new ProductException(ProductErrorCode.NOT_FOUND) or the equivalent used elsewhere) so the upper transaction will roll back; add unit tests that call these methods with non-existent or deleted IDs and assert the exception is thrown and no higher-level changes are committed.apps/commerce-api/src/main/java/com/loopers/domain/product/ProductImageModel.java-23-24 (1)
23-24:⚠️ Potential issue | 🟠 MajorProductService.delete() 메서드에서 ProductImageModel도 함께 삭제해야 한다.
productId를Long으로 저장하여 JPA 연관관계 매핑이 없다. 현재ProductService.delete()는ProductModel.delete()만 호출하고 있어서ProductModel삭제 시 관련ProductImageModel레코드가 orphan 상태로 남는다. Soft delete 패턴 사용으로 인해deletedAt만 설정되므로 물리적 고아 레코드가 DB에 계속 쌓인다.
ProductImageService.deleteAllByProductId()메서드가 이미 구현되어 있으므로,ProductService.delete()메서드에서 상품 삭제 시 다음과 같이 이미지도 함께 삭제하도록 수정한다:`@Transactional` public void delete(Long id) { getById(id).delete(); productImageService.deleteAllByProductId(id); }동일하게
deleteAllByBrandId()메서드도 브랜드의 모든 상품 삭제 시 각 상품의 이미지를 함께 삭제하도록 리팩토링이 필요하다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/domain/product/ProductImageModel.java` around lines 23 - 24, ProductService.delete(Long) currently only calls ProductModel.delete() leaving ProductImageModel rows orphaned; update ProductService.delete(Long id) to call productImageService.deleteAllByProductId(id) after getById(id).delete() (ensure method is annotated `@Transactional`) and similarly refactor deleteAllByBrandId() to iterate affected products and invoke productImageService.deleteAllByProductId(productId) (or call a batch variant) so product images have their deletedAt set when products are soft-deleted; reference ProductService.delete(), ProductService.deleteAllByBrandId(), ProductModel.delete(), and ProductImageService.deleteAllByProductId() when making the change.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java-159-170 (1)
159-170:⚠️ Potential issue | 🟠 Major허용되지 않은
sort값을 조용히 DESC로 접으면 캐시 키만 늘어난다.현재 구현은
"price_asc"외의 모든 값을 같은 DESC 정렬로 처리하지만, cache key는 원본sort문자열을 그대로 사용한다. 운영에서는 잘못된 클라이언트 입력 하나마다 같은 데이터를 다른 Redis key로 저장하게 되어 key cardinality만 증가하고, 요청 오류도 조기에 드러나지 않는다. 수정안은sort를 enum이나 whitelist로 제한해 유효하지 않으면CoreException으로 실패시키거나, 최소한 캐시 키와 실제 정렬 값을 canonical value로 정규화하는 것이다. 추가 테스트로null, 오탈자, 대소문자 변형 입력이 들어왔을 때 4xx 일관 오류 또는 하나의 canonical key/응답으로만 수렴하는지 검증해야 한다.
Based on learnings, In the loop-pack-be-l2-vol3-java project, enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format. Do not introduce MethodArgumentNotValidException handlers or Bean Validation handling, as that would create inconsistent error handling patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java` around lines 159 - 170, The cache key and sorting logic in getProductListByPrice silently accept any sort string (only "price_asc" is treated specially) which inflates cache cardinality; update getProductListByPrice to validate/normalize the incoming sort into a canonical enum or whitelist (e.g., PRICE_ASC / PRICE_DESC) and use that canonical value both for determining the Sort (Sort.by(...)) and for the `@Cacheable` key (replace "#sort" with the canonical variable), and if the input is invalid (null or unknown after normalization) throw a CoreException so the error routes through ApiControllerAdvice; add unit tests for null, typos and case variations to assert a 4xx CoreException or that they map to a single canonical cache key/response.apps/commerce-api/src/main/java/com/loopers/application/product/dto/ProductResult.java-40-46 (1)
40-46:⚠️ Potential issue | 🟠 Major페이지 조회 뒤 inactive brand를 걸러내면 페이지 메타데이터가 틀어진다.
이 helper는 현재 페이지에서 inactive brand 상품을 제거한다. 호출부는 원래
totalElements를 그대로 사용하므로 운영에서는 한 페이지가 덜 차거나 다음 페이지가 비어도 총 개수는 더 크게 보이는 현상이 생긴다. 캐시를 적용하면 이 잘못된 페이지가 TTL 동안 재사용된다. 수정안은 활성 브랜드 조건을 DB 조회 단계로 내려서 그 결과로 페이징을 계산하는 것이다. 추가 테스트로 활성/비활성 브랜드 상품을 페이지 경계에 섞어 두고items.size,totalElements,totalPages가 서로 일관적인지 검증해야 한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/product/dto/ProductResult.java` around lines 40 - 46, The helper ProductResult.fromWithActiveBrand currently filters out products with inactive brands after pagination, causing misaligned page metadata; instead, push the "brand is active" predicate into the DB query that produces the paged result (modify the repository/query method that returns the Page<ProductModel> to join/filter by brand.active or equivalent), stop post-filtering in ProductResult.fromWithActiveBrand (or remove its use in paging flows), and ensure the service/controller uses the DB-provided totalElements/totalPages directly; add integration tests that place active/inactive brand products around page boundaries and assert items.size, totalElements and totalPages remain consistent.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java-149-169 (1)
149-169:⚠️ Potential issue | 🟠 Major비고유 정렬 키로 페이징하면 페이지 경계가 흔들린다.
createdAt과price는 동점이 흔해서 지금처럼 단일 컬럼만으로 정렬하면 운영에서 같은 상품이 여러 페이지에 중복되거나 일부가 누락될 수 있다. 캐시를 얹으면 이 불안정한 페이지가 TTL 동안 재사용된다. 수정안은id같은 고유 컬럼을 보조 정렬로 추가하고, 인덱스도 같은 순서로 맞추는 것이다. 추가 테스트로 동일한createdAt또는 동일한price를 가진 상품을 페이지 경계에 배치해도 중복/누락이 없는지 검증해야 한다.정렬 안정화 예시
- Pageable pageable = PageRequest.of(page, size, Sort.by(Sort.Direction.DESC, "createdAt")); + Pageable pageable = PageRequest.of( + page, + size, + Sort.by(Sort.Direction.DESC, "createdAt") + .and(Sort.by(Sort.Direction.DESC, "id"))); - Sort sortOrder = "price_asc".equals(sort) - ? Sort.by(Sort.Direction.ASC, "price") - : Sort.by(Sort.Direction.DESC, "price"); + Sort sortOrder = "price_asc".equals(sort) + ? Sort.by(Sort.Direction.ASC, "price") + .and(Sort.by(Sort.Direction.ASC, "id")) + : Sort.by(Sort.Direction.DESC, "price") + .and(Sort.by(Sort.Direction.DESC, "id"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java` around lines 149 - 169, The current single-column ordering in getProductListLatest (Sort.by(... "createdAt")) and getProductListByPrice (Sort.by(... "price")) is unstable for ties; change the Sort used when creating PageRequest.of(...) to add a deterministic secondary key (e.g., Sort.by(primaryDirection, "createdAt").and(Sort.by(Sort.Direction.DESC, "id")) for latest, and for price use the same primary direction for "price" and then .and(Sort.by(Sort.Direction.ASC, "id")) or matching direction as desired) so pagination is stable; update the PageRequest.of(...) calls inside getProductListLatest and getProductListByPrice to use these composite Sorts, add a DB index matching the sort order (primary column then id) and add tests that place multiple products with identical createdAt or price on page boundaries to assert no duplicates or missing items across pages.apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java-32-35 (1)
32-35:⚠️ Potential issue | 🟠 Major좋아요·이미지 변경 후 목록 캐시가 무효화되지 않아 stale 상태가 지속된다.
캐시 응답에 포함된
likeCount와thumbnailUrl이 ProductFacade의 상품 CRUD에서만 무효화되고 있다. 다음 경로에서는 캐시 무효화가 누락되어 있다:
- ProductLikeFacade의
like(),unlike()메서드: 좋아요 추가/취소 후 likeCount가 변경되지만PRODUCT_LIST_LATEST,PRODUCT_LIST_PRICE,PRODUCT_LIST_LIKES캐시가 무효화되지 않음- ProductImageService의
addImage(),deleteAllByProductId(): 이미지 변경 후 thumbnailUrl이 변경되지만 위 세 캐시가 무효화되지 않음브랜드 변경(BrandFacade.updateBrand, deleteBrand)은 이미 해당 캐시를 무효화하고 있다.
ProductLikeFacade와 ProductImageService의 변경 메서드에
@CacheEvict또는@Caching어노테이션을 추가하여 일관된 무효화 전략을 적용한다. 추가로 좋아요 변경, 이미지 추가/삭제, 브랜드 비활성화 직후 목록 응답의 cache miss/hit 흐름이 즉시 갱신되는지 통합 테스트로 검증한다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.java` around lines 32 - 35, The product list caches (CacheType.Names.PRODUCT_LIST_LATEST, PRODUCT_LIST_PRICE, PRODUCT_LIST_LIKES) are not evicted when likes or images change; add cache eviction annotations to the mutating methods: annotate ProductLikeFacade.like() and ProductLikeFacade.unlike() with `@Caching` or multiple `@CacheEvict` entries that evict allEntries=true for the three caches, and likewise annotate ProductImageService.addImage() and ProductImageService.deleteAllByProductId() to evict the same caches; after applying these annotations, add an integration test that performs like/unlike, addImage/deleteAllByProductId (and brand disable flows are already covered) and asserts that list endpoints reflect updated likeCount/thumbnailUrl immediately by verifying cache miss followed by refreshed hit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d03d85c6-4256-403b-9711-d8c67c242ba8
⛔ Files ignored due to path filters (1)
docs/round5/README.mdis excluded by!**/*.mdand included by**
📒 Files selected for processing (71)
.gitignoreapps/commerce-api/src/main/java/com/loopers/application/brand/BrandFacade.javaapps/commerce-api/src/main/java/com/loopers/application/brand/dto/BrandResult.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/ProductLikeFacade.javaapps/commerce-api/src/main/java/com/loopers/application/product/dto/ProductResult.javaapps/commerce-api/src/main/java/com/loopers/application/user/dto/UserResult.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ImageType.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductImageModel.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductImageRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductImageService.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductModel.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/product/ProductService.javaapps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/user/UserService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorProperties.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorRunner.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/DataGeneratorRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/FashionDataPool.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductImageJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductImageRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/product/ProductRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/auth/AuthFilter.javaapps/commerce-api/src/main/java/com/loopers/interfaces/brand/AdminBrandV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/datagenerator/AdminDataGeneratorV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/datagenerator/dto/AdminDataGeneratorV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/product/ProductV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/product/dto/ProductV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/user/AdminUserV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/user/UserV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/user/dto/AdminUserV1Dto.javaapps/commerce-api/src/main/java/com/loopers/interfaces/user/dto/UserV1Dto.javaapps/commerce-api/src/main/java/com/loopers/support/cache/CacheConfig.javaapps/commerce-api/src/main/java/com/loopers/support/cache/CacheType.javaapps/commerce-api/src/main/java/com/loopers/support/cache/RedisCacheHelper.javaapps/commerce-api/src/main/resources/application.ymlapps/commerce-api/src/main/resources/static/admin/css/styles.cssapps/commerce-api/src/main/resources/static/admin/index.htmlapps/commerce-api/src/main/resources/static/admin/js/api.jsapps/commerce-api/src/main/resources/static/admin/js/app.jsapps/commerce-api/src/main/resources/static/admin/js/components/brands.jsapps/commerce-api/src/main/resources/static/admin/js/components/coupons.jsapps/commerce-api/src/main/resources/static/admin/js/components/dashboard.jsapps/commerce-api/src/main/resources/static/admin/js/components/data-generator.jsapps/commerce-api/src/main/resources/static/admin/js/components/orders.jsapps/commerce-api/src/main/resources/static/admin/js/components/products.jsapps/commerce-api/src/main/resources/static/admin/js/components/users.jsapps/commerce-api/src/main/resources/static/shop/css/styles.cssapps/commerce-api/src/main/resources/static/shop/index.htmlapps/commerce-api/src/main/resources/static/shop/js/api.jsapps/commerce-api/src/main/resources/static/shop/js/app.jsapps/commerce-api/src/main/resources/static/shop/js/auth.jsapps/commerce-api/src/main/resources/static/shop/js/pages/coupons.jsapps/commerce-api/src/main/resources/static/shop/js/pages/home.jsapps/commerce-api/src/main/resources/static/shop/js/pages/login.jsapps/commerce-api/src/main/resources/static/shop/js/pages/mypage.jsapps/commerce-api/src/main/resources/static/shop/js/pages/product.jsapps/commerce-api/src/test/java/com/loopers/application/product/ProductFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/application/product/ProductLikeFacadeTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/FakeProductImageRepository.javaapps/commerce-api/src/test/java/com/loopers/domain/product/FakeProductRepository.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductImageModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductImageServiceTest.javaapps/commerce-api/src/test/java/com/loopers/domain/product/ProductModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/user/FakeUserRepository.javaapps/commerce-api/src/test/java/com/loopers/interfaces/product/ProductV1ApiE2ETest.java
Comment on lines
+58
to
63
| @Cacheable(cacheNames = CacheType.Names.BRAND_LIST, key = "'all'") | ||
| @Transactional(readOnly = true) | ||
| public Page<BrandResult> getBrands(Pageable pageable) { | ||
| return brandService.getAll(pageable).map(BrandResult::from); | ||
| public BrandResult.ListPage getBrands(Pageable pageable) { | ||
| return BrandResult.ListPage.from( | ||
| brandService.getAll(pageable).map(BrandResult::from)); | ||
| } |
There was a problem hiding this comment.
캐시 키에 Pageable 정보가 포함되지 않아 잘못된 데이터가 반환된다.
key = "'all'"로 고정되어 있어, 서로 다른 page/size 요청이 동일한 캐시 데이터를 반환한다. 예를 들어 page=0, size=10 요청 후 캐시된 결과가 page=1, size=20 요청에도 반환되어 운영 환경에서 데이터 불일치가 발생한다.
🐛 수정 제안
-@Cacheable(cacheNames = CacheType.Names.BRAND_LIST, key = "'all'")
+@Cacheable(cacheNames = CacheType.Names.BRAND_LIST, key = "#pageable.pageNumber + '-' + `#pageable.pageSize`")
`@Transactional`(readOnly = true)
public BrandResult.ListPage getBrands(Pageable pageable) {또는 페이지네이션이 필요 없는 전체 목록 조회라면, Pageable 파라미터를 제거하고 전체 데이터를 캐시해야 한다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Cacheable(cacheNames = CacheType.Names.BRAND_LIST, key = "'all'") | |
| @Transactional(readOnly = true) | |
| public Page<BrandResult> getBrands(Pageable pageable) { | |
| return brandService.getAll(pageable).map(BrandResult::from); | |
| public BrandResult.ListPage getBrands(Pageable pageable) { | |
| return BrandResult.ListPage.from( | |
| brandService.getAll(pageable).map(BrandResult::from)); | |
| } | |
| `@Cacheable`(cacheNames = CacheType.Names.BRAND_LIST, key = "#pageable.pageNumber + '-' + `#pageable.pageSize`") | |
| `@Transactional`(readOnly = true) | |
| public BrandResult.ListPage getBrands(Pageable pageable) { | |
| return BrandResult.ListPage.from( | |
| brandService.getAll(pageable).map(BrandResult::from)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/application/brand/BrandFacade.java`
around lines 58 - 63, The cache key in BrandFacade.getBrands is fixed to "'all'"
while the method accepts a Pageable, causing different page/size requests to
return the same cached data; update the `@Cacheable` on getBrands to include
pageable information in the cache key (e.g., incorporate pageNumber, pageSize
and sort from the Pageable via SpEL like referencing
`#pageable.pageNumber/`#pageable.pageSize/#pageable.sort or `#root.methodName` with
those fields) so each page/size/sort combination gets a distinct cache entry, or
if you truly want to cache the entire list remove the Pageable parameter and
cache the full result instead.
Comment on lines
+23
to
+35
| private volatile boolean running = false; | ||
|
|
||
| public boolean isRunning() { | ||
| return running; | ||
| } | ||
|
|
||
| public void generateAll() { | ||
| if (running) { | ||
| log.warn("BulkDataGenerator is already running. Skipping."); | ||
| return; | ||
| } | ||
| running = true; | ||
| try { |
There was a problem hiding this comment.
실행 중 플래그 체크가 원자적이지 않다.
if (running) 뒤에 running = true를 두면 동시 호출 두 건이 모두 진입할 수 있다. 운영 중 애플리케이션 시작 훅과 관리자 호출이 겹치면 대량 데이터가 중복 생성되므로, AtomicBoolean.compareAndSet(false, true) 같은 원자적 진입 제어로 바꿔야 한다. 동시 호출 2개를 넣어도 실제 생성 로직이 1회만 수행되는 동시성 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorService.java`
around lines 23 - 35, The running flag is not updated atomically causing a race
in generateAll(); replace the boolean running field with an AtomicBoolean (keep
isRunning() but have it return running.get()), and change generateAll() to
perform an atomic entry check using running.compareAndSet(false, true) (only
proceed when it returns true, otherwise log and return) and ensure
running.set(false) is done in the finally block; also add a concurrency unit
test that invokes generateAll() from multiple threads and asserts the underlying
creation logic runs exactly once to verify the atomic behavior.
Comment on lines
+444
to
+456
| private int flushOrderBatch(List<Object[]> orderBatch, List<List<Object[]>> itemsPerOrder) { | ||
| long maxId = dataGeneratorRepository.getMaxOrderId(); | ||
| dataGeneratorRepository.batchInsertOrders(orderBatch); | ||
|
|
||
| List<Object[]> allItems = new ArrayList<>(); | ||
| for (int i = 0; i < orderBatch.size(); i++) { | ||
| long orderId = maxId + 1 + i; | ||
| for (Object[] item : itemsPerOrder.get(i)) { | ||
| allItems.add(new Object[]{ | ||
| orderId, item[0], item[1], item[2], item[3], item[4], item[5]}); | ||
| } | ||
| } | ||
| dataGeneratorRepository.batchInsertOrderItems(allItems); |
There was a problem hiding this comment.
주문 ID를 maxId + offset으로 계산하면 주문 아이템이 다른 주문에 연결될 수 있다.
배치 insert 후 실제 생성된 주문 ID를 받지 않고 getMaxOrderId()에 의존하면, 동시 트래픽이나 시퀀스 갭이 있는 환경에서 아이템이 잘못된 주문에 붙는다. 또한 주문 insert 후 아이템 insert가 실패하면 고아 주문이 남으므로, 같은 트랜잭션 안에서 생성된 주문 ID 목록을 DB에서 반환받아 그 ID로 아이템을 저장해야 한다. 동시 insert 상황과 order-item insert 실패 상황에서 주문/아이템 정합성이 유지되는 통합 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/datagenerator/BulkDataGeneratorService.java`
around lines 444 - 456, flushOrderBatch currently computes order IDs via
getMaxOrderId() + offset which can misassociate items under concurrent/sequenced
gaps; change the flow so batchInsertOrders in dataGeneratorRepository returns
the actual generated order IDs (e.g., List<Long>) instead of relying on
getMaxOrderId(), use that returned List from flushOrderBatch to build allItems
and then call batchInsertOrderItems with those IDs, wrap the order + order-items
inserts in a single transaction to avoid orphan orders on failure (or perform
compensating rollback on error), and add integration tests for concurrent
inserts and order-item insert failure to assert consistency.
Comment on lines
+70
to
+79
| long startUserId = dataGeneratorRepository.getMaxUserId() + 1; | ||
| List<long[]> pairs = new ArrayList<>(); | ||
|
|
||
| for (Long productId : request.productIds()) { | ||
| for (int i = 0; i < request.likesPerProduct(); i++) { | ||
| pairs.add(new long[]{startUserId + i, productId}); | ||
| } | ||
| } | ||
|
|
||
| int created = dataGeneratorRepository.batchInsertLikes(pairs); |
There was a problem hiding this comment.
좋아요를 존재하지 않는 사용자 ID로 생성하고 있다.
getMaxUserId() + 1부터 매핑하면 실제 사용자 없이 like row만 쌓이거나 FK 제약으로 전체 배치가 실패한다. 운영 중 like_count와 좋아요 정렬 기준이 유령 사용자 데이터로 오염되므로, 기존 사용자 ID를 조회해 중복 없이 배분하고 부족분은 skipped로 반환해야 한다. 사용자 수보다 큰 likesPerProduct 요청에서도 생성 건수가 실제 사용자 수를 넘지 않는 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/datagenerator/AdminDataGeneratorV1Controller.java`
around lines 70 - 79, The current loop in AdminDataGeneratorV1Controller builds
likes starting at getMaxUserId()+1 which creates likes for non-existent users;
change it to query existing user IDs from the repository (e.g., a new
dataGeneratorRepository.fetchAllUserIds()/fetchUserIdsLimit or similar),
distribute those real user IDs across request.productIds() without reusing a
user ID for multiple likes where the reviewer expects uniqueness, and only add
pairs when a real user ID is available; track and return a skipped count for any
requested likes beyond available users, then call batchInsertLikes(pairs) with
the valid pairs; also add a unit/integration test asserting that when
likesPerProduct > number of users the created count never exceeds actual user
count.
Comment on lines
+130
to
+148
| for (Long userId : request.userIds()) { | ||
| try { | ||
| List<OrderCriteria.Create.CreateItem> orderItems = isSpecificMode | ||
| ? specifiedItems | ||
| : pickRandomItems(dataGeneratorRepository.findRandomProducts(100), | ||
| request.itemsPerOrder() != null ? request.itemsPerOrder() : 3); | ||
|
|
||
| int totalCost = orderItems.stream() | ||
| .mapToInt(item -> item.expectedPrice() * item.quantity()) | ||
| .sum(); | ||
|
|
||
| userService.addPoint(userId, totalCost + 1000L); | ||
|
|
||
| orderFacade.createOrder(userId, new OrderCriteria.Create(orderItems)); | ||
| created++; | ||
| } catch (Exception e) { | ||
| log.warn("주문 생성 실패 (userId={}): {}", userId, e.getMessage()); | ||
| failed++; | ||
| } |
There was a problem hiding this comment.
주문 실패 시 포인트 적립만 남는 흐름이다.
현재 한 사용자 처리에서 addPoint가 먼저 실행되고 이후 createOrder가 실패해도 롤백되지 않는다. 운영 중 재고 부족이나 검증 실패가 발생하면 유저 포인트가 계속 증가하므로, 이 오케스트레이션을 서비스로 이동해 사용자 단위 @Transactional로 묶거나 실패 시 보상 처리해야 한다. 주문 생성 예외를 강제로 발생시켜도 포인트 잔액이 변경되지 않는 테스트를 추가해야 한다. As per coding guidelines "Controller는 요청 검증(Bean Validation)과 응답 조립에 집중하고 비즈니스 로직은 Service로 이동한다." 그리고 "**/*Service*.java: 트랜잭션 경계(@Transactional) 위치와 전파, readOnly, 롤백 조건을 점검한다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/datagenerator/AdminDataGeneratorV1Controller.java`
around lines 130 - 148, The controller currently calls userService.addPoint(...)
before orderFacade.createOrder(...) inside AdminDataGeneratorV1Controller, so if
createOrder fails the points are not rolled back; move this orchestration into a
new or existing Service (e.g., AdminDataGeneratorService) and wrap the per-user
flow (addPoint + createOrder) in a `@Transactional` method (or implement
compensation logic to deduct points on failure) so the point change and order
creation share a transaction boundary; update the controller to call that
service method and add a unit/integration test that forces
orderFacade.createOrder(...) to throw and asserts user points remain unchanged.
| @@ -0,0 +1,76 @@ | |||
| const ADMIN_HEADER = { 'X-Loopers-Ldap': 'loopers.admin' }; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 백엔드에서 X-Loopers-Ldap 헤더 처리 방식 확인
rg -n "X-Loopers-Ldap" --type=java -A 5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6661
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/auth/AdminAuthFilter.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2788
클라이언트에 하드코딩된 관리자 인증 헤더는 추가 검증 메커니즘 없이는 보안상 취약하다.
백엔드에서 헤더 값(X-Loopers-Ldap: loopers.admin)만 검증하고 있다. HTTP 헤더는 클라이언트에서 쉽게 조작할 수 있으므로, 내부 관리자 도구라 하더라도 다음과 같은 추가 검증이 필요하다:
- 세션/토큰 기반 인증(JWT, 세션 검증)
- IP 화이트리스트 제한
- 관리자 계정 인증(아이디/패스워드) 추가
현재 구조는 헤더 값이 노출되면 누구나 관리자 권한으로 요청할 수 있다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-api/src/main/resources/static/admin/js/api.js` at line 1, The
client-side constant ADMIN_HEADER in
apps/commerce-api/src/main/resources/static/admin/js/api.js hardcodes
'X-Loopers-Ldap: loopers.admin', which is insecure; remove this hardcoded header
and implement proper authentication flows instead: stop relying on ADMIN_HEADER
and X-Loopers-Ldap for admin auth, require a server-issued session or JWT for
admin requests (validate token/session server-side), add optional IP whitelist
checks or an admin login flow (username/password) that exchanges credentials for
a token, and update any code paths that reference ADMIN_HEADER to use the
authenticated token or session cookie and server-side permission checks.
Comment on lines
+29
to
+39
| tbody.innerHTML = data.items.length === 0 | ||
| ? '<tr><td colspan="4" style="text-align:center;color:#94a3b8">브랜드가 없습니다</td></tr>' | ||
| : data.items.map(b => `<tr> | ||
| <td>${b.id}</td> | ||
| <td><strong>${b.name}</strong></td> | ||
| <td>${formatDate(b.createdAt)}</td> | ||
| <td> | ||
| <button class="btn btn-secondary btn-sm" onclick="window._brandEdit(${b.id},'${esc(b.name)}')">수정</button> | ||
| <button class="btn btn-danger btn-sm" onclick="window._brandDel(${b.id},'${esc(b.name)}')">삭제</button> | ||
| </td> | ||
| </tr>`).join(''); |
There was a problem hiding this comment.
브랜드명이 관리자 화면에서 스크립트로 해석될 수 있다.
b.name을 목록 HTML에 그대로 넣고 있고 esc도 따옴표만 치환하므로 <, >, &가 그대로 남는다. 운영 중 악성 브랜드명이 저장되면 목록 조회만으로 관리자 세션 XSS가 가능하므로, 셀 렌더링은 textContent 기반으로 바꾸고 버튼 액션은 inline JS 대신 data-*와 이벤트 위임으로 분리해야 한다. <img src=x onerror=alert(1)>, 줄바꿈, 따옴표가 포함된 브랜드명을 등록해도 화면에 문자 그대로 보이고 수정/삭제 버튼이 정상 동작하는 UI 테스트를 추가해야 한다.
Also applies to: 87-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/commerce-api/src/main/resources/static/admin/js/components/brands.js`
around lines 29 - 39, The current row rendering uses tbody.innerHTML with
interpolated b.name and a limited esc function and inline onclick handlers
(window._brandEdit/window._brandDel), which allows HTML/JS injection; change
rendering to create DOM nodes programmatically instead of innerHTML: for each
item use document.createElement to build tr/td, set brand cell via
element.textContent (not innerHTML), attach edit/delete buttons without inline
JS by setting data-brand-id and data-brand-name attributes and register a
delegated click listener on the tbody to handle actions (parse dataset values
and call the existing edit/delete logic), and remove reliance on esc and
window._brandEdit/window._brandDel; apply the same fix to the other occurrence
at lines 87-88 and add UI tests that insert brand names containing <,>,&,
quotes, newline and an image onerror payload to assert text shows literally and
edit/delete buttons still work.
Comment on lines
+302
to
+323
| function populateBrandSelectors() { | ||
| // Single brand select | ||
| const sel = document.getElementById('gen-prod-single-brand'); | ||
| if (sel) { | ||
| sel.innerHTML = cachedBrands.map(b => `<option value="${b.id}">${b.name} (ID:${b.id})</option>`).join(''); | ||
| } | ||
| // Multi brand checklist | ||
| const checklist = document.getElementById('gen-prod-multi-brands'); | ||
| if (checklist) { | ||
| if (cachedBrands.length === 0) { | ||
| checklist.innerHTML = '<p style="color:#94a3b8;font-size:13px">브랜드가 없습니다. 먼저 생성해주세요.</p>'; | ||
| return; | ||
| } | ||
| checklist.innerHTML = ` | ||
| <div style="margin-bottom:8px"> | ||
| <button class="btn btn-secondary btn-sm" id="brand-check-all">전체 선택</button> | ||
| <button class="btn btn-secondary btn-sm" id="brand-uncheck-all">전체 해제</button> | ||
| <span style="font-size:12px;color:#64748b;margin-left:8px" id="brand-check-count">0개 선택</span> | ||
| </div> | ||
| <div class="checkbox-grid">${cachedBrands.map(b => | ||
| `<label class="checkbox-item"><input type="checkbox" value="${b.id}" class="brand-cb"> ${b.name}</label>` | ||
| ).join('')}</div>`; |
There was a problem hiding this comment.
저장된 이름과 오류 메시지를 그대로 HTML로 렌더링하고 있다.
브랜드명·상품명·로그 메시지를 innerHTML로 붙이고 있어 DB 값이나 e.message에 포함된 HTML이 그대로 실행된다. 운영 중 테스트 데이터 한 건이나 서버 오류 문자열 하나만으로 관리자 세션 XSS가 가능하므로, option/label/log 항목은 createElement와 textContent로 렌더링해야 한다. 악성 이름과 오류 메시지를 주입해도 화면에 문자 그대로 출력되고 스크립트가 실행되지 않는 UI 테스트를 추가해야 한다.
Also applies to: 357-385, 837-842
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/commerce-api/src/main/resources/static/admin/js/components/data-generator.js`
around lines 302 - 323, populateBrandSelectors currently injects DB values
directly into innerHTML (using cachedBrands.map) causing XSS; replace the
innerHTML builds for the single select (element id "gen-prod-single-brand") and
the multi-brand checklist (element id "gen-prod-multi-brands") with DOM-safe
construction: createOption elements with document.createElement('option') and
set option.value and option.textContent (use cachedBrands), and for the
checklist create container elements, buttons, span and label/input pairs via
document.createElement (set input.value but set label.textContent for brand
name) instead of template strings; ensure event wiring for
"brand-check-all"/"brand-uncheck-all" and the brand checkbox class "brand-cb"
remains the same and apply the same createElement/textContent approach to the
other similar blocks noted (lines ~357-385, ~837-842).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Summary
리뷰 포인트
이번 주차를 돌아보면, 깊은 고민보다는 학습 자체에 집중한 한 주였습니다. AI를 활용해 빠르게 구현하고 트레이드오프를 접하며 선택하고 적용 효과를 뽑아내는 경험을 했지만, 제가 한 선택의 판단이 사실은 잘못되지 않았을까..하는 의심병이 있습니다.
다음 관점에서 리뷰 해주시면 감사하겠습니다.
시간이 괜찮으시다면 개인적인 고민을 여쭤보고 싶습니다.(그냥 넘어가셔도 괜찮습니다.)
진행 상황
과제 작업 ToDoList
아쉬운점
캐시에 대한 학습이 아직 충분하지 않다는 점이 아쉽다.
현재는 AI에게 전적으로 캐시 도입을 위임하여 성능 개선 효과만 확인한 단계이며, 이렇게 동작이 가는하다는 것만 알고 있는 상태다. 캐시에 대한 학습을 이어서 진행중에 있다.
5주차 진행 후 바뀐 점
인덱스
B+Tree 구조와 인덱스 개념을 학습하며 인덱스가 내부적으로 어떻게 동작하는지 이해하게 됐다.
조회 속도 개선을 위해 비정규화 테이블에 인덱스를 적용하는 경험을 했고, 이 과정에서 동기화 전략과 인덱스 설계 방향에 대해 깊이 고민했다. 특히 "카디널리티가 높으면 무조건 인덱스를 건다"는 기존의 인식이 바뀌었는데, 실제로는 비즈니스 특성에 따라 인덱스 전략이 달라져야 한다는 것을 배웠다.
이를 프로젝트에 직접 적용하면서, 조회 성능이 기대에 미치지 못할 때 가장 먼저 인덱스 누락 여부를 점검하는 나름의 기준이 생긴 것 같다.
캐시
캐시를 처음 사용해봤다. 처음에는 단순히 조회 속도를 높이는 수단 정도로만 생각했는데, 본질적으로는 "변하지 않는 데이터를 반복적으로 DB에 요청하는 상황 자체"를 문제로 보고 해결하는 기술임을 이해하게 됐다. AI를 활용해 빠르게 적용하고 개선 효과를 직접 눈으로 확인할 수 있었지만, 캐시 자체에 대한 이해는 아직 부족하다는 것도 느꼈다. 5주차가 시작되기 전까지는 "캐시 그냥 붙이면 되는 거 아니야?"라고 가볍게 생각했었는데, 멘토링에서 다른 멤버들의 고민을 들으면서 생각이 완전히 바뀌었다. 캐시 활용 전략, 정합성 유지, 장애 복구 전략 등 실제로 고민해야 할 포인트가 훨씬 많다는 걸 알게 됐고, 이 부분들에 흥미가 생겨 질문들에 나온 키워드 가지고 다음 학습 방향도 잡히게 됐다.
🧭 Context & Decision
👍 1. 좋아요순으로 상품 목록 조회
blog : https://plan22plan.tistory.com/entry/%EC%A2%8B%EC%95%84%EC%9A%94%EC%88%9C-%EC%83%81%ED%92%88-%EC%A1%B0%ED%9A%8C-%EC%9D%BD%EA%B8%B0-%EC%84%B1%EB%8A%A5-%EC%B5%9C%EC%A0%81%ED%99%94
문제 정의
GET /api/v1/products?sort=likes_desc&page=0&size=20선택지와 결정
비정규화 후 인덱스를 적용하기로 했고, 동기화 방법과 인덱스 전략 2가지에 대한 고민이 있었다.
고려한 대안:
트레이드오프:
추후 개선 여지(있다면):
문제 해결 결과
2. 🎁 상품 목록 조회 (8가지 정렬 조합)
blog : 작성하지 않음
문제 정의
GET /api/v1/products?sort={sort}&brandId={brandId}&page=0&size=20(deleted_at, like_count)1개만 존재선택지와 결정
복합 인덱스를 유즈케이스별로 설계하기로 했고, 인덱스 개수와 컬럼 순서에 대한 고민이 있었다.
고려한 대안:
(brand_id, deleted_at, sort_column)✅(deleted_at, brand_id, sort_column)트레이드오프:
추후 개선 여지(있다면):
문제 해결 결과
3.🧺 브랜드 목록 Redis 캐시 적용
blog : 작성하지 않음
브랜드 목록 조회
문제 정의
GET /api-admin/v1/brands?page=0&size=200선택지와 결정
캐시 저장소와 캐싱 패턴에 대한 고민이 있었다.
고려한 대안:
트레이드오프:
추후 개선 여지(있다면):
문제 해결 결과
brand:list::all