Conversation
Walkthrough이 PR은 그룹 가입/신청 워크플로우를 구현하며, 새로운 조회 엔드포인트들과 함께 도메인 모델을 확장합니다. JoinEntity, 관련 서비스, 마퍼, 그리고 커서 기반 페이지네이션을 위한 QueryDSL 구현이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JoinController
participant ApplicationFormService
participant GroupRepository
participant JoinRepository
participant GroupMemberRepository
participant GroupRoleRepository
Client->>JoinController: POST /v1/groups/{groupId}/joins<br/>(userId, ApplicationFormRequestDto)
JoinController->>ApplicationFormService: joinRequest(ApplicationFormCommand)
ApplicationFormService->>GroupRepository: findGroupById(groupId)
GroupRepository-->>ApplicationFormService: Group
ApplicationFormService->>ApplicationFormService: checkJoinable(group)
ApplicationFormService->>JoinRepository: existsJoin(groupId, userId)
JoinRepository-->>ApplicationFormService: boolean
ApplicationFormService->>GroupRoleRepository: checkPermission(userId, groupId)
GroupRoleRepository-->>ApplicationFormService: boolean (JOIN_MANAGE)
ApplicationFormService->>JoinRepository: save(JoinDomain)
JoinRepository-->>ApplicationFormService: JoinDomain
alt Group Join Policy is OPEN
ApplicationFormService->>GroupMemberRepository: save(groupId, userId, MEMBER)
end
ApplicationFormService-->>JoinController: ApplicationFormResult
JoinController-->>Client: 201 Created<br/>(ApplicationFormResponseDto)
sequenceDiagram
participant Client
participant GroupQueryController
participant FindGroupService
participant GroupRepository
participant GroupRepositoryImpl
Client->>GroupQueryController: GET /v1/groups<br/>(userId, GroupListRequestDto)
GroupQueryController->>FindGroupService: findAllGroup(userId, req)
FindGroupService->>GroupRepository: findAllByCursor(cursorId, category, size)
GroupRepository->>GroupRepositoryImpl: findAllByCursor(...)
GroupRepositoryImpl->>GroupRepositoryImpl: QueryDSL 쿼리 실행<br/>(커서 기반 페이지네이션)
GroupRepositoryImpl-->>GroupRepository: List<GroupInfo>
GroupRepository-->>FindGroupService: List<GroupInfo>
FindGroupService->>FindGroupService: createGroupInfoCursorPagingResponse(...)
FindGroupService-->>GroupQueryController: CursorPagingResponseDto<GroupInfo>
GroupQueryController-->>Client: 200 OK<br/>(CursorPagingResponseDto<GroupInfo>)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (13)
src/main/java/flipnote/group/application/port/out/GroupRoleRepositoryPort.java-3-8 (1)
3-8:⚠️ Potential issue | 🟠 Major
GroupRoleRepositoryPort의 반환 타입을 도메인 모델로 변경하세요.애플리케이션 포트가 어댑터 레이어의
RoleEntity를 직접 반환하고 있습니다. 헥사고날 아키텍처에서 포트는 어댑터에 의존하면 안 되며, 의존 방향은 항상 어댑터 → 애플리케이션 → 도메인이어야 합니다.동일 패키지의 다른 포트들(
GroupRepositoryPort,JoinRepositoryPort)은 도메인 모델(Group,JoinDomain)을 반환하고 있는 반면,GroupRoleRepositoryPort만RoleEntity(어댑터 엔티티)를 반환하고 있어 일관성이 없습니다.
create메서드를 도메인 모델 또는 역할 ID(Long)를 반환하도록 수정하십시오. 현재 구조는 애플리케이션 레이어를 영속성 구현에 직접 결합시킵니다.GroupMemberRepositoryPort도 동일한 문제(RoleEntity임포트)를 가지고 있는지 확인하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/out/GroupRoleRepositoryPort.java` around lines 3 - 8, The port GroupRoleRepositoryPort currently returns the adapter type RoleEntity which couples the application layer to the persistence layer; change the signature of create(Long groupId) to return the domain model (GroupMemberRole) or a Long roleId instead, update imports to remove flipnote.group.adapter.out.entity.RoleEntity and use flipnote.group.domain.model.member.GroupMemberRole, and update all adapter implementations to map their RoleEntity results into the chosen domain type (or return the id) before returning; also inspect GroupMemberRepositoryPort for any RoleEntity imports and apply the same domain-returning fix so ports remain adapter-agnostic.src/main/java/flipnote/group/api/dto/request/ApplicationFormRequestDto.java-3-5 (1)
3-5:⚠️ Potential issue | 🟠 Major
joinIntro필드에 Bean Validation 어노테이션 추가 필요
joinIntro필드에 검증 어노테이션이 없어null, 빈 문자열, 또는 과도하게 긴 문자열이 데이터베이스에 저장될 수 있습니다. 컨트롤러에서@Valid가 적용되어 있으므로, DTO 필드에@NotBlank와@Size어노테이션을 추가하면 유효성 검증이 정상 작동합니다.제안 수정
+import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; + public record ApplicationFormRequestDto( - String joinIntro + `@NotBlank`(message = "가입 소개는 필수입니다.") + `@Size`(max = 500, message = "가입 소개는 500자 이내로 작성해 주세요.") + String joinIntro ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/request/ApplicationFormRequestDto.java` around lines 3 - 5, ApplicationFormRequestDto의 record 구성요소 joinIntro에 Bean Validation 어노테이션이 없으므로 null/빈/과도한 길이 값이 들어갈 수 있습니다; fix는 ApplicationFormRequestDto의 joinIntro 선언에 `@NotBlank` 및 적절한 길이 제한을 위한 `@Size`(max = 1000) 같은 어노테이션을 추가하고 (필요시 실제 DB/요구사항에 맞는 max값으로 조정), jakarta.validation.constraints 패키지에서 NotBlank/Size를 임포트하며 record 요소에 어노테이션을 붙여 컨트롤러의 `@Valid와` 함께 유효성 검사가 동작하도록 하세요.src/main/java/flipnote/group/api/dto/response/FindJoinFormListResponseDto.java-9-14 (1)
9-14:⚠️ Potential issue | 🟠 MajorAPI 응답 DTO에 도메인 모델
JoinDomain이 직접 노출됩니다.
List<JoinDomain>을 API 응답의 필드로 사용하면 도메인 모델이 외부 API 계약에 직접 묶이게 됩니다.JoinDomain의 내부 구조 변경(필드 추가·삭제, 직렬화 어노테이션 부재 등)이 API 응답에 즉시 영향을 주고, 필요 이상의 내부 정보가 노출될 수 있습니다.ApplicationFormResponseDto처럼JoinDomain을 명시적으로 매핑하는 전용 응답 DTO를 사용하는 것을 권장합니다.♻️ 리팩터링 제안
-public record FindJoinFormListResponseDto( - List<JoinDomain> joinList -) { - public static FindJoinFormListResponseDto from(FindJoinFormListResult result) { - return new FindJoinFormListResponseDto(result.joinDomainList()); - } -} +public record FindJoinFormListResponseDto( + List<ApplicationFormResponseDto> joinList +) { + public static FindJoinFormListResponseDto from(FindJoinFormListResult result) { + List<ApplicationFormResponseDto> dtoList = result.joinDomainList().stream() + .map(ApplicationFormResponseDto::from) + .toList(); + return new FindJoinFormListResponseDto(dtoList); + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/response/FindJoinFormListResponseDto.java` around lines 9 - 14, The response DTO exposes the domain model JoinDomain directly; change FindJoinFormListResponseDto to hold a list of a dedicated response DTO (e.g., ApplicationFormResponseDto) instead of List<JoinDomain>, and update the factory method from(FindJoinFormListResult) to map each JoinDomain in result.joinDomainList() to the new DTO (use or add a static ApplicationFormResponseDto.from(JoinDomain) mapper if missing) so the API contract is decoupled from internal domain shapes.src/main/java/flipnote/group/application/port/in/FindGroupUseCase.java-3-4 (1)
3-4: 🛠️ Refactor suggestion | 🟠 MajorUseCase 포트에서 API 계층 DTO 직접 참조 — 레이어 위반
GroupListRequestDto와CursorPagingResponseDto는api.dto패키지 소속임에도 불구하고, application 계층 포트인FindGroupUseCase에서 직접 사용되고 있습니다. 헥사고날 아키텍처에서 포트는 domain 타입이나command/result전용 객체에만 의존해야 하며, API 계층 DTO가 안쪽(application) 레이어로 침투하면 의존성 방향이 역전됩니다.
GroupListRequestDto대신 전용 query 객체를,CursorPagingResponseDto<GroupInfo>대신 전용 result 타입을application.port.in패키지 내에 정의하여 레이어 경계를 명확히 유지하는 것을 권장합니다.Also applies to: 12-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/in/FindGroupUseCase.java` around lines 3 - 4, FindGroupUseCase currently depends on API DTOs (GroupListRequestDto and CursorPagingResponseDto<GroupInfo>), violating layer boundaries; replace those DTO types with new application-layer input/output types defined in application.port.in (e.g., GroupListQuery or FindGroupCommand for the request and GroupListResult or CursorPagingResult<GroupInfo> for the response), update the FindGroupUseCase method signature to accept the new query/command type and return the new result type, and adapt implementations/adapters in the API layer to map between API DTOs and the new application-level types so the application port no longer imports api.dto classes.src/main/java/flipnote/group/application/port/out/GroupMemberRepositoryPort.java-12-12 (1)
12-12: 🛠️ Refactor suggestion | 🟠 Major
existsUserInGroup의void반환 — 메서드명과 의미 불일치
existsUserInGroup이라는 이름은 존재 여부를boolean으로 반환하는 것을 암시하지만 반환 타입이void입니다. 포트 인터페이스 수준에서 호출자는 이 메서드가 실패 시 예외를 던지는지, 단순 조회인지 알 수 없습니다. 의도가 "멤버십 검증 후 실패 시 예외"라면assertUserInGroup또는validateUserInGroup으로 명명하여 계약을 명확히 하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/out/GroupMemberRepositoryPort.java` at line 12, The port method existsUserInGroup currently returns void but its name implies a boolean result; either change its signature to return boolean (change existsUserInGroup(Long groupId, Long userId) to boolean existsUserInGroup(...)) and update all implementing classes and callers to handle a true/false membership check, or rename the method to assertUserInGroup or validateUserInGroup to indicate it throws on failure and update implementations and callers accordingly; make sure to update JavaDoc/comments and any unit tests that call GroupMemberRepositoryPort.existsUserInGroup to reflect the new contract.src/main/java/flipnote/group/application/port/out/GroupMemberRepositoryPort.java-5-5 (1)
5-5:⚠️ Potential issue | 🟠 Major어댑터 계층 타입(
RoleEntity)이 application port에 임포트됨 — 레이어 위반
flipnote.group.adapter.out.entity.RoleEntity는 어댑터(persistence) 계층의 JPA 엔티티입니다. 현재 실제 메서드 시그니처에서RoleEntity는 전혀 사용되지 않으므로(모든 메서드는 domain 타입인GroupMemberRole을 사용), 이 임포트는 unused이면서 동시에 안쪽 레이어(application port)가 바깥쪽 레이어(adapter)에 의존하는 구조적 위반입니다. 해당 임포트를 제거해야 합니다.🔧 수정 제안
-import flipnote.group.adapter.out.entity.RoleEntity; import flipnote.group.domain.model.member.GroupMemberRole; import flipnote.group.domain.model.member.MemberInfo;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/out/GroupMemberRepositoryPort.java` at line 5, The interface GroupMemberRepositoryPort imports adapter entity RoleEntity which is an unused, layer-violating dependency; remove the import of flipnote.group.adapter.out.entity.RoleEntity from GroupMemberRepositoryPort and ensure all method signatures and return types continue to use the domain type GroupMemberRole (verify no references to RoleEntity remain in methods like those declaring GroupMemberRole), then run a compile to confirm no remaining references.src/main/java/flipnote/group/adapter/out/persistence/mapper/GroupMemberMapper.java-42-53 (1)
42-53:⚠️ Potential issue | 🟠 Major
toMemberInfo에서entity.getRole()null 역참조 위험
entity.getRole()이null을 반환할 경우entity.getRole().getRole()호출 시 NPE가 발생합니다.GroupMemberEntity의 role 필드가 not-null 컬럼이라도 데이터 정합성 문제나 미래 변경에 대비하여 null 가드가 필요합니다.🛡️ 제안 수정
return entities.stream() .map(entity -> MemberInfo.builder() .userId(entity.getUserId()) - .role(entity.getRole().getRole()) + .role(entity.getRole() != null ? entity.getRole().getRole() : null) .build() ) .toList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/mapper/GroupMemberMapper.java` around lines 42 - 53, toMemberInfo currently calls entity.getRole().getRole() which can NPE if GroupMemberEntity.getRole() is null; add a null guard in toMemberInfo to handle that case (for example, by mapping when entity.getRole() != null and using a default/optional value or skipping/setting a null-safe role string). Update the stream mapping in toMemberInfo to read the role into a local (e.g., var role = entity.getRole() == null ? "<DEFAULT_ROLE>" : entity.getRole().getRole()) before calling MemberInfo.builder().role(role). Ensure the method still returns Collections.emptyList() for null/empty input and preserves existing behavior for non-null roles.src/main/java/flipnote/group/adapter/out/entity/JoinEntity.java-22-25 (1)
22-25:⚠️ Potential issue | 🟠 Major
(user_id, group_id)복합 유니크 제약 조건 누락서비스 레이어에서
existsJoin()체크를 수행하지만, 동시 요청 시 TOCTOU(Time-Of-Check-To-Time-Of-Use) 경쟁 조건으로 인해 동일 사용자가 동일 그룹에 중복 가입 신청을 저장할 수 있습니다. DB 레벨의 유니크 제약 조건이 반드시 필요합니다.🛡️ 제안 수정
+import jakarta.persistence.UniqueConstraint; `@Getter` `@Entity` -@Table(name = "joins") +@Table(name = "joins", + uniqueConstraints = `@UniqueConstraint`( + name = "uk_joins_user_group", + columnNames = {"user_id", "group_id"} + ) +) `@NoArgsConstructor`(access = AccessLevel.PROTECTED) public class JoinEntity extends BaseEntity {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/entity/JoinEntity.java` around lines 22 - 25, The JoinEntity lacks a DB-level unique constraint for the (user_id, group_id) pair which allows TOCTOU duplicate inserts; update the entity and schema by adding a unique constraint on those columns (e.g., via `@Table`(uniqueConstraints = ...) on class JoinEntity referencing the user_id and group_id column names used in the entity) and also add a database migration that creates a unique index/constraint on joins(user_id, group_id) so duplicates are rejected at the DB level.src/main/java/flipnote/group/adapter/in/web/JoinController.java-59-71 (1)
59-71:⚠️ Potential issue | 🟠 MajorGET 엔드포인트에서
HttpStatus.CREATED(201) 반환 — HTTP 의미론 위반
GET /joins는 자원을 조회하는 엔드포인트이므로HttpStatus.OK(200)을 반환해야 합니다.HttpStatus.CREATED(201)은 자원이 새로 생성되었을 때만 사용됩니다(RFC 9110). 이 오류로 인해 API 클라이언트가 잘못된 상태 코드 처리를 하게 됩니다.🐛 제안 수정
`@GetMapping`("/joins") public ResponseEntity<FindJoinFormListResponseDto> findGroupJoinList( `@RequestHeader`("X-USER-ID") Long userId, `@PathVariable`("groupId") Long groupId) { FindJoinFormCommand cmd = new FindJoinFormCommand(userId, groupId); FindJoinFormListResult result = joinUseCase.findJoinFormList(cmd); FindJoinFormListResponseDto res = FindJoinFormListResponseDto.from(result); - return ResponseEntity.status(HttpStatus.CREATED).body(res); + return ResponseEntity.ok(res); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/in/web/JoinController.java` around lines 59 - 71, The GET handler findGroupJoinList in JoinController currently returns ResponseEntity.status(HttpStatus.CREATED).body(res) which is incorrect for a retrieval endpoint; change the response to use HttpStatus.OK (200) instead (e.g., return ResponseEntity.ok(res) or ResponseEntity.status(HttpStatus.OK).body(res)) so FindJoinFormListResponseDto responses from joinUseCase.findJoinFormList(...) are returned with the correct semantics.src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java-89-93 (1)
89-93:⚠️ Potential issue | 🟠 Major
checkRole에서findByGroupIdAndRolenull 반환 시 NPE
groupRoleRepository.findByGroupIdAndRole(groupId, groupMemberRole)이null을 반환하는 경우(예: 해당 그룹에 역할이 존재하지 않을 때)roleEntity.getId()에서 NPE가 발생하여 처리되지 않은 500 오류로 이어집니다.🛡️ 제안 수정
`@Override` public boolean checkRole(Long userId, Long groupId, GroupMemberRole groupMemberRole) { RoleEntity roleEntity = groupRoleRepository.findByGroupIdAndRole(groupId, groupMemberRole); + if (roleEntity == null) { + return false; + } return groupMemberRepository.existsByUserIdAndRole_Id(userId, roleEntity.getId()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java` around lines 89 - 93, checkRole에서 groupRoleRepository.findByGroupIdAndRole(groupId, groupMemberRole)가 null을 반환할 경우 roleEntity.getId()에서 NPE가 발생하므로 해당 결과를 검사하고 처리하세요: checkRole 메서드에서 RoleEntity roleEntity 반환값이 null이면 적절히 false를 반환하거나(권한 없음으로 처리) 예외를 던지도록 명시적으로 처리하고, 기존의 groupMemberRepository.existsByUserIdAndRole_Id(userId, roleEntity.getId()) 호출은 roleEntity가 non-null일 때만 호출되게 변경하세요.src/main/java/flipnote/group/application/service/ApplicationFormService.java-31-34 (1)
31-34: 🛠️ Refactor suggestion | 🟠 Major포트 대신 구체 어댑터를 직접 주입 → 헥사고날 아키텍처 위반
CreateGroupService가GroupRepositoryPort,GroupMemberRepositoryPort,GroupRoleRepositoryPort를 주입하는 반면,ApplicationFormService는 구체 어댑터 클래스(GroupRepositoryAdapter,JoinRepositoryAdapter등)를 직접 참조합니다. 이는 애플리케이션 레이어가 인프라 레이어에 의존하게 되어 헥사고날 아키텍처의 의존성 방향 원칙을 위반합니다.♻️ 수정 제안
-import flipnote.group.adapter.out.persistence.GroupMemberRepositoryAdapter; -import flipnote.group.adapter.out.persistence.GroupRepositoryAdapter; -import flipnote.group.adapter.out.persistence.GroupRoleRepositoryAdapter; -import flipnote.group.adapter.out.persistence.JoinRepositoryAdapter; +import flipnote.group.application.port.out.GroupMemberRepositoryPort; +import flipnote.group.application.port.out.GroupRepositoryPort; +import flipnote.group.application.port.out.GroupRoleRepositoryPort; +import flipnote.group.application.port.out.JoinRepositoryPort; // ... - private final GroupRepositoryAdapter groupRepository; - private final JoinRepositoryAdapter joinRepository; - private final GroupMemberRepositoryAdapter groupMemberRepository; - private final GroupRoleRepositoryAdapter groupRoleRepository; + private final GroupRepositoryPort groupRepository; + private final JoinRepositoryPort joinRepository; + private final GroupMemberRepositoryPort groupMemberRepository; + private final GroupRoleRepositoryPort groupRoleRepository;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/service/ApplicationFormService.java` around lines 31 - 34, ApplicationFormService currently depends on concrete adapters (GroupRepositoryAdapter, JoinRepositoryAdapter, GroupMemberRepositoryAdapter, GroupRoleRepositoryAdapter) which violates hexagonal dependency rules; change the injected fields and constructor to depend on the port interfaces used elsewhere (e.g., GroupRepositoryPort, JoinRepositoryPort/JoinRepositoryPort if named, GroupMemberRepositoryPort, GroupRoleRepositoryPort), update all uses inside ApplicationFormService to the port types, and remove direct references/imports to the adapter classes so the application layer depends only on the abstraction and not on infrastructure.src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java-30-35 (1)
30-35:⚠️ Potential issue | 🟠 Major
findByGroupIdAndRole반환값 null 처리 누락 → NPE 위험
groupRoleRepository.findByGroupIdAndRole(groupId, role)은 일치하는RoleEntity를 찾지 못할 경우 null을 반환합니다. 현재 코드는 이를 그대로GroupMemberMapper.create에 전달하므로 NPE가 발생합니다.더 심각한 문제는
GroupRoleRepositoryAdapter.checkRole()의 line 90-92에서도 동일한 메서드를 사용하며, 여기서는 반환값에 대해 직접.getId()를 호출하므로 null이 전달될 경우 즉시 NPE가 발생합니다:RoleEntity roleEntity = groupRoleRepository.findByGroupIdAndRole(groupId, groupMemberRole); return groupMemberRepository.existsByUserIdAndRole_Id(userId, roleEntity.getId()); // NPE 위험두 위치 모두에서 null 안전성 처리가 필요합니다.
🛡️ 수정 제안
- RoleEntity roleEntity = groupRoleRepository.findByGroupIdAndRole(groupId, role); + RoleEntity roleEntity = groupRoleRepository.findByGroupIdAndRole(groupId, role); + if (roleEntity == null) { + throw new IllegalStateException("Role not found for group: " + groupId + ", role: " + role); + } groupMemberRepository.save(GroupMemberMapper.create(groupId, userId, roleEntity));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java` around lines 30 - 35, Both usages of groupRoleRepository.findByGroupIdAndRole need null-safety: when it returns null you must stop and surface a clear error instead of passing null to GroupMemberMapper.create or calling roleEntity.getId(); update GroupMemberRepositoryAdapter.save(Long groupId, Long userId, GroupMemberRole role) to check the returned RoleEntity and throw or return a meaningful error (e.g., throw new IllegalArgumentException/RoleNotFoundException) if null before calling GroupMemberMapper.create, and similarly change GroupRoleRepositoryAdapter.checkRole(...) to handle a null RoleEntity (return false or throw a RoleNotFoundException) instead of dereferencing roleEntity.getId(); ensure both places reference groupRoleRepository.findByGroupIdAndRole and RoleEntity for the null-checks.src/main/java/flipnote/group/application/service/ApplicationFormService.java-53-55 (1)
53-55:⚠️ Potential issue | 🟠 Majorjoin 테이블에 (groupId, userId) unique constraint 추가 및 정식 멤버 중복 가입 체크 필요
두 가지 확인된 문제가 있습니다.
정식 멤버의 중복 가입 신청:
existsJoin은 join 신청 이력만 확인하므로, 이미group_members에 등록된 정식 멤버가 재신청할 경우 이를 차단하지 못합니다.groupMemberRepository.existsByGroupIdAndUserId()를 통해 정식 멤버 존재 여부도 함께 확인해야 합니다.Race condition:
existsJoin체크(line 53)와joinRepository.save(line 64) 사이에 두 요청이 동시에 진입하면 중복 join 레코드가 생성될 수 있습니다. JoinEntity의 joins 테이블에(groupId, userId)unique constraint를 추가하여 데이터베이스 레벨에서 중복을 방지하거나, 트랜잭션 격리 수준을 높이거나 명시적 잠금 메커니즘을 적용해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/service/ApplicationFormService.java` around lines 53 - 55, The current join flow only checks joinRepository.existsJoin(cmd.groupId(), cmd.userId()) and can miss users already in group_members and is vulnerable to a race creating duplicate join rows; update the validation in ApplicationFormService to also call groupMemberRepository.existsByGroupIdAndUserId(cmd.groupId(), cmd.userId()) and reject if true, and ensure duplicates are prevented at DB level by adding a (group_id, user_id) unique constraint on the joins table / JoinEntity (or enforce via stricter transaction isolation or explicit locking around the existsJoin + joinRepository.save sequence) so concurrent requests cannot insert duplicate join records.
🟡 Minor comments (6)
src/main/java/flipnote/group/api/dto/response/CursorPagingResponseDto.java-17-19 (1)
17-19:⚠️ Potential issue | 🟡 Minor
hasNext=true이지만nextCursorId가null인 경우 불일치 가능.
hasNext가true인데nextCursorId가null이면 응답에hasNext: true, nextCursor: null이 포함되어 클라이언트가 다음 페이지를 요청할 수 없는 상태가 됩니다. 이 조합에 대한 방어 로직 추가를 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/response/CursorPagingResponseDto.java` around lines 17 - 19, CursorPagingResponseDto.of(List<T>, boolean, Long)에서 hasNext가 true이지만 nextCursorId가 null인 경우 응답 불일치가 발생하므로 방어 로직을 추가하세요: of(List<T> content, boolean hasNext, Long nextCursorId) 내부에서 if (hasNext && nextCursorId == null) 처리해 hasNext를 false로 강제하거나(권장) IllegalArgumentException을 던져 호출자에게 잘못된 상태를 알리도록 변경하고, 선택한 동작을 반영해 이후 호출하는 of(List<T>, boolean, String)로 전달될 nextCursor 값을 null로 유지하거나 적절히 설정하세요; 변경 대상 식별자: CursorPagingResponseDto.of(List<T>, boolean, Long) 및 내부 호출하는 of(content, hasNext, nextCursor).src/main/java/flipnote/group/api/dto/response/CursorPagingResponseDto.java-13-14 (1)
13-14:⚠️ Potential issue | 🟡 Minor
content가null일 경우NullPointerException발생 가능.
content.size()호출 시content가null이면 NPE가 발생합니다. 방어적 null 체크를 추가하거나@NonNull제약을 명시하는 것을 권장합니다.🛡️ null 방어 코드 제안
public static <T> CursorPagingResponseDto<T> of(List<T> content, boolean hasNext, String nextCursor) { + Objects.requireNonNull(content, "content must not be null"); return new CursorPagingResponseDto<>(content, hasNext, hasNext ? nextCursor : null, content.size()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/response/CursorPagingResponseDto.java` around lines 13 - 14, The static factory CursorPagingResponseDto.of should defensively handle a null content to avoid NullPointerException from calling content.size(); update the method (CursorPagingResponseDto.of) to either validate and throw a clear NPE via Objects.requireNonNull(content, "...") or substitute Collections.emptyList() when content is null and compute size from that fallback, ensuring nextCursor logic (hasNext ? nextCursor : null) remains unchanged and the DTO constructor receives a non-null list and correct size.src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java-96-96 (1)
96-96:⚠️ Potential issue | 🟡 Minor주석 오타: "체킁" → "체킹"
🔧 제안 수정
- * 권한 체킁 + * 권한 체킹🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/GroupRoleRepositoryAdapter.java` at line 96, 주석 오타를 수정하세요: 클래스/파일 GroupRoleRepositoryAdapter 내에 있는 주석 텍스트 "권한 체킁"을 "권한 체킹"으로 바로 고치고 관련 주석들이 동일한 오타로 반복되어 있으면 모두 동일하게 수정하세요.src/main/java/flipnote/group/adapter/in/web/MemberController.java-6-6 (1)
6-6:⚠️ Potential issue | 🟡 Minor미사용
PutMappingimport현재
MemberController에는@PutMapping을 사용하는 메서드가 없습니다. TODO 구현 전에는 미사용 import를 제거하는 것이 좋습니다.🔧 제안 수정
-import org.springframework.web.bind.annotation.PutMapping;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/in/web/MemberController.java` at line 6, MemberController에 선언된 불필요한 import 문인 org.springframework.web.bind.annotation.PutMapping을 제거하세요: 파일 내에서 `@PutMapping을` 사용하는 메서드가 없으니 해당 import를 삭제하여 미사용 import 경고를 제거하고 코드 정리를 유지합니다.src/main/java/flipnote/group/adapter/out/persistence/mapper/GroupMemberMapper.java-3-3 (1)
3-3:⚠️ Potential issue | 🟡 Minor미사용
Collectionimport
java.util.Collection은 이 클래스에서 사용되지 않습니다.🔧 제안 수정
-import java.util.Collection; import java.util.Collections; import java.util.List;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/mapper/GroupMemberMapper.java` at line 3, Remove the unused import java.util.Collection from the GroupMemberMapper class: open the GroupMemberMapper file and delete the import line for java.util.Collection since no methods or fields in GroupMemberMapper reference Collection, leaving only the imports that are actually used.src/main/java/flipnote/group/adapter/out/entity/JoinEntity.java-9-14 (1)
9-14:⚠️ Potential issue | 🟡 Minor주석 처리된 코드에서만 사용되는 미사용 import
FetchType,JoinColumn,ManyToOne은 현재 주석 처리된 코드(Line 33–35)에서만 사용됩니다. 관계 매핑이 복원될 때까지는 이 import들을 제거하는 것이 좋습니다.🔧 제안 수정
-import jakarta.persistence.FetchType; import jakarta.persistence.GeneratedValue; import jakarta.persistence.GenerationType; import jakarta.persistence.Id; -import jakarta.persistence.JoinColumn; -import jakarta.persistence.ManyToOne; import jakarta.persistence.Table;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/entity/JoinEntity.java` around lines 9 - 14, Remove the unused JPA imports FetchType, JoinColumn, and ManyToOne from the top of the JoinEntity class since they are only referenced in currently commented-out relationship code; update the import list to keep only the necessary imports (e.g., Id, GeneratedValue, GenerationType) and avoid unused-import warnings until the relationship mapping is restored in the commented block.
🧹 Nitpick comments (23)
src/main/java/flipnote/group/adapter/out/entity/GroupMemberEntity.java (1)
57-69: Javadoc의@param,@return설명이 누락되어 있습니다.
role파라미터가 추가되었지만 Javadoc에 반영되지 않았고,@return설명도 비어 있습니다.📝 Javadoc 수정 제안
/** * 멤버 생성 - * `@param` groupId - * `@param` userId - * `@return` + * `@param` groupId 그룹 ID + * `@param` userId 사용자 ID + * `@param` role 역할 엔티티 + * `@return` 생성된 GroupMemberEntity */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/entity/GroupMemberEntity.java` around lines 57 - 69, Update the Javadoc for the static factory method GroupMemberEntity.create to include a description for the newly added parameter `role` and a meaningful `@return` description; specifically add an `@param role` line describing the RoleEntity being assigned to the new member and an `@return` line explaining that the method returns a constructed GroupMemberEntity instance populated with the provided groupId, userId, and role. Ensure the Javadoc sits immediately above the create(Long groupId, Long userId, RoleEntity role) method and follows the existing style of the file.src/main/java/flipnote/group/api/dto/request/FindJoinFormListRequestDto.java (1)
3-4: 빈 레코드는 불필요할 수 있습니다.필드가 없는 요청 DTO는 현재 역할이 없습니다. 향후 파라미터 추가가 예상되는 경우가 아니라면 제거를 고려해 주세요. 만약 페이지네이션 파라미터 등이 필요하다면 지금 추가하는 것이 좋겠습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/request/FindJoinFormListRequestDto.java` around lines 3 - 4, The record FindJoinFormListRequestDto is empty and serves no purpose; either remove this DTO entirely and update any references to it, or add expected request parameters (e.g., page, size, sort) to the record so it carries pagination/filter data. Locate the record named FindJoinFormListRequestDto and either delete it and adjust callers to not require it, or replace the empty declaration with fields like page/size (and corresponding types) to reflect intended API parameters.src/main/java/flipnote/group/api/dto/request/CursorPagingRequest.java (2)
26-41:getCursorId()방어 로직이 적절합니다.null/빈 문자열/숫자 형식 오류에 대한 처리가 잘 되어 있습니다. 다만 Line 31-33의
trim().isEmpty()체크는StringUtils.hasText()가 이미 공백 문자열을 처리하므로 중복입니다.♻️ 중복 체크 제거 제안
public Long getCursorId() { if (!StringUtils.hasText(cursor)) { return null; } - final String normalized = cursor.trim(); - if (normalized.isEmpty()) { - return null; - } - try { - return Long.valueOf(normalized); + return Long.valueOf(cursor.trim()); } catch (NumberFormatException ex) { return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/request/CursorPagingRequest.java` around lines 26 - 41, In getCursorId(), remove the redundant blank-check: StringUtils.hasText(cursor) already excludes null/blank, so delete the intermediate normalized/isEmpty block and instead, after the hasText() guard, attempt to parse Long.valueOf(cursor.trim()) inside the existing try/catch; this keeps trimming before parsing while eliminating the duplicate empty-string check (refer to getCursorId and cursor).
22-24:sortBy가 검증 없이Sort.by()에 직접 전달됩니다.사용자가 임의의 필드명을
sortBy로 전달할 수 있어, 존재하지 않는 컬럼으로 정렬 시 런타임 예외가 발생하거나, 내부 엔티티 필드명이 노출될 수 있습니다. 허용된 정렬 필드 목록(allowlist)에 대한 검증을 추가하는 것을 권장합니다.Also applies to: 43-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/request/CursorPagingRequest.java` around lines 22 - 24, CursorPagingRequest currently forwards the user-provided sortBy into Sort.by() (see fields sortBy and order and the code around the Sort.by(...) call at lines ~43-55), which allows invalid or sensitive field names; add an allowlist check: define a Set of permitted sort keys (and optional mapping from external names to entity field names) and validate sortBy against that set before building the Sort, falling back to a safe default (e.g., a configured defaultSort) or ignoring the sort param if invalid; if invalid input should be rejected, throw a clear validation exception; ensure the final Sort construction uses the mapped/validated field name and the existing order handling.src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRolePermissionRepositoryImpl.java (2)
5-5: 사용되지 않는BooleanBuilderimport가 있습니다.현재
existsUserInGroupPermission메서드에서BooleanBuilder를 사용하지 않고 있습니다. 불필요한 import를 제거해 주세요.🧹 제안하는 수정 사항
-import com.querydsl.core.BooleanBuilder;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRolePermissionRepositoryImpl.java` at line 5, Remove the unused import of BooleanBuilder from GroupRolePermissionRepositoryImpl: locate the import statement for com.querydsl.core.BooleanBuilder and delete it since existsUserInGroupPermission does not use BooleanBuilder (verify no other methods in GroupRolePermissionRepositoryImpl reference BooleanBuilder before removing).
20-22: Q-entity 경로를private static final로 선언하는 것을 권장합니다.Q-entity 참조는 불변 싱글톤이므로, 인스턴스 필드 대신
private static final로 선언하면 의도가 더 명확해지고 불필요한 인스턴스별 할당을 방지할 수 있습니다.♻️ 제안하는 수정 사항
- QRoleEntity role = QRoleEntity.roleEntity; - QPermissionEntity permission = QPermissionEntity.permissionEntity; - QGroupMemberEntity groupMember = QGroupMemberEntity.groupMemberEntity; + private static final QRoleEntity role = QRoleEntity.roleEntity; + private static final QPermissionEntity permission = QPermissionEntity.permissionEntity; + private static final QGroupMemberEntity groupMember = QGroupMemberEntity.groupMemberEntity;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRolePermissionRepositoryImpl.java` around lines 20 - 22, Declare the Q-entity references as class-level immutable singletons instead of instance fields: change the declarations of QRoleEntity role, QPermissionEntity permission, and QGroupMemberEntity groupMember inside GroupRolePermissionRepositoryImpl to private static final fields so they are shared, immutable, and not reallocated per instance; update any usages to reference these static final fields.src/main/java/flipnote/group/domain/model/join/JoinStatus.java (1)
3-4: 상태 enum 값의 네이밍 관례 검토를 권장합니다.상태(status) enum은 일반적으로 완료된 상태를 나타내므로 과거형(
ACCEPTED,REJECTED,CANCELLED)으로 명명하는 것이 관례입니다. 현재ACCEPT,REJECT는 동사 원형으로, 상태보다 "행위"의 의미에 가깝습니다.PENDING만 현재 진행형을 사용하고 있어 일관성이 부족합니다.이미 다른 코드에서 해당 enum 값을 참조하고 있다면, 이후 리팩토링 시 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/domain/model/join/JoinStatus.java` around lines 3 - 4, Rename the enum constants in JoinStatus from action/infinitive forms to past/completed-state forms for consistency (change ACCEPT→ACCEPTED, REJECT→REJECTED, CANCEL→CANCELLED while keeping PENDING as-is), update all references to JoinStatus.ACCEPT/REJECT/CANCEL across the codebase to the new names, and run/adjust any switch statements, serializers, or persistence mappings that use JoinStatus to ensure compilation and runtime behavior remain correct (handle enum name changes in DB/json mappings if present).src/main/java/flipnote/group/application/port/out/GroupRepositoryPort.java (1)
8-9: 사용되지 않는 Jakarta Validation import가 있습니다.
@Min과@Max가 import되어 있지만 이 인터페이스의 메서드 시그니처에서 사용되지 않고 있습니다. 불필요한 import를 제거해 주세요.🧹 제안하는 수정 사항
-import jakarta.validation.constraints.Max; -import jakarta.validation.constraints.Min;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/out/GroupRepositoryPort.java` around lines 8 - 9, Remove the unused Jakarta Validation imports in GroupRepositoryPort by deleting the import statements for jakarta.validation.constraints.Min and jakarta.validation.constraints.Max since neither annotation is referenced in this interface; ensure only required imports remain and run a quick compile/IDE organize imports to verify no other unused imports persist.src/main/java/flipnote/group/application/port/in/result/FindAllGroupResult.java (1)
7-7: 가변List를 레코드 컴포넌트로 직접 노출레코드는 컴포넌트 자체를 불변으로 만들지 않으므로
groups()접근자로 얻은 리스트를 외부에서 수정할 수 있습니다.♻️ 제안 수정 (방어적 복사)
+import java.util.Collections; + public record FindAllGroupResult(List<GroupInfo> groups) { + public FindAllGroupResult { + groups = Collections.unmodifiableList(groups); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/in/result/FindAllGroupResult.java` at line 7, FindAllGroupResult exposes a mutable List via its record component, so make a defensive, immutable copy: in the record's canonical/compact constructor assign this.groups = List.copyOf(groups) (or new ArrayList<>(groups) wrapped with Collections.unmodifiableList) and/or override the groups() accessor to return List.copyOf(this.groups) to prevent external mutation; reference the record FindAllGroupResult and its groups component/accessor when applying the change.src/main/java/flipnote/group/application/port/in/result/FindJoinFormListResult.java (1)
7-9: 가변List를 레코드 컴포넌트로 직접 노출 (FindAllGroupResult와 동일)
joinDomainList()접근자로 얻은 리스트를 외부에서 수정할 수 있습니다.♻️ 제안 수정 (방어적 복사)
+import java.util.Collections; + public record FindJoinFormListResult( - List<JoinDomain> joinDomainList + List<JoinDomain> joinDomainList ) { + public FindJoinFormListResult { + joinDomainList = Collections.unmodifiableList(joinDomainList); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/port/in/result/FindJoinFormListResult.java` around lines 7 - 9, FindJoinFormListResult currently exposes a mutable List via its record component joinDomainList; change it to return an unmodifiable/defensive copy to prevent external mutation. Modify the record so that either the canonical/compact constructor stores List.copyOf(joinDomainList) or override the accessor joinDomainList() to return List.copyOf(this.joinDomainList) (or Collections.unmodifiableList(...)) so callers cannot mutate the internal list; reference the record name FindJoinFormListResult and component/method joinDomainList.src/main/java/flipnote/group/api/dto/response/FindGroupMemberResponseDto.java (1)
9-9: 도메인 모델MemberInfo가 API 응답 레이어에 직접 노출됩니다.헥사고날 아키텍처에서 API 응답 DTO는 도메인 모델에 의존하지 않는 것이 이상적입니다.
MemberInfo내부 필드가 변경될 경우 API 계약이 묵시적으로 변경될 수 있으므로, 별도의 API 레이어 전용 DTO(예:MemberInfoResponseDto)로 매핑하는 것을 고려하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/response/FindGroupMemberResponseDto.java` at line 9, FindGroupMemberResponseDto currently exposes the domain model MemberInfo via the field memberInfoList; replace this with an API-specific DTO by creating MemberInfoResponseDto, change the field in FindGroupMemberResponseDto to List<MemberInfoResponseDto>, and update any constructors/factory/mapping code (e.g., where FindGroupMemberResponseDto is instantiated or populated) to convert each MemberInfo -> MemberInfoResponseDto (map all needed fields) so the API layer no longer depends on the domain model.src/main/java/flipnote/group/infrastructure/persistence/jpa/JoinRepository.java (1)
8-8:JoinDomain임포트가 사용되지 않습니다.이 인터페이스에서
JoinDomain은 어디에도 참조되지 않습니다. 불필요한 임포트를 제거하세요.♻️ 수정 제안
import org.springframework.data.jpa.repository.JpaRepository; import flipnote.group.adapter.out.entity.JoinEntity; -import flipnote.group.domain.model.join.JoinDomain;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/jpa/JoinRepository.java` at line 8, 이 인터페이스에서 사용되지 않는 import문을 제거하세요: 삭제 대상은 JoinRepository.java 상단의 "import flipnote.group.domain.model.join.JoinDomain;"입니다; JoinDomain은 인터페이스 내에 참조되지 않으므로 해당 import를 제거하고 필요한 경우 IDE의 organize/imports(또는 static import 정리) 기능으로 나머지 import를 정리하세요.src/main/java/flipnote/group/api/dto/response/FindJoinFormListResponseDto.java (1)
7-7:JoinStatus임포트가 사용되지 않습니다.이 파일 어디에도
JoinStatus가 참조되지 않으므로 임포트를 제거하세요.♻️ 수정 제안
import flipnote.group.application.port.in.result.FindJoinFormListResult; import flipnote.group.domain.model.join.JoinDomain; -import flipnote.group.domain.model.join.JoinStatus;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/api/dto/response/FindJoinFormListResponseDto.java` at line 7, Remove the unused import of JoinStatus from FindJoinFormListResponseDto by deleting the line importing flipnote.group.domain.model.join.JoinStatus; (ensure no references to JoinStatus remain in the class); recompile or run linter to confirm there are no other unused imports.src/main/java/flipnote/group/domain/model/group/GroupInfo.java (1)
3-11:record의from()팩토리 메서드 중복Java
record는 canonical constructor가 자동으로public으로 생성되기 때문에,from()은new GroupInfo(...)와 완전히 동일하여 별도의 가치를 제공하지 않습니다. 코드베이스 전체에서from()패턴을 일관되게 사용한다면 유지할 수 있지만, 그렇지 않다면 제거하는 것이 더 간결합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/domain/model/group/GroupInfo.java` around lines 3 - 11, The GroupInfo.record defines a redundant static factory from() that simply calls the record's canonical constructor; remove the public static GroupInfo from(...) method from the GroupInfo record to avoid duplication (or if your codebase relies on the from() callsite convention, replace usages to call new GroupInfo(...) or the canonical constructor and then remove the from() method). Update any call sites that reference GroupInfo.from(...) to use the canonical constructor (new GroupInfo(...)) before deleting the from() method.src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryCustom.java (1)
9-13: 인터페이스 메서드 파라미터 명명 불일치
findAllByCursorAndCreatedUserId의 파라미터명(cursorId,size,id)이 다른 두 메서드(lastId,pageSize,userId)와 일치하지 않아 가독성이 떨어집니다.♻️ 수정 제안
- List<GroupInfo> findAllByCursorAndCreatedUserId(Long cursorId, Category category, int size, Long id); + List<GroupInfo> findAllByCursorAndCreatedUserId(Long lastId, Category category, int pageSize, Long userId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryCustom.java` around lines 9 - 13, Rename the parameters of the interface method findAllByCursorAndCreatedUserId to match the other cursor methods for consistency: change cursorId -> lastId, size -> pageSize, and id -> userId in the method signature, and update any implementing class (e.g., the class that implements GroupRepositoryCustom::findAllByCursorAndCreatedUserId) to use the same parameter names and order so callers and implementations remain consistent and readable.src/main/java/flipnote/group/adapter/out/persistence/mapper/JoinMapper.java (1)
36-44:toDomain에 null 가드 누락 —GroupMemberMapper.toDomain과 불일치같은 패키지의
GroupMemberMapper.toDomain은entity == null체크를 수행하지만,JoinMapper.toDomain은 이 가드가 없어 NPE가 발생할 수 있습니다.toDomains()스트림에서 직접 호출되기 때문에, 리스트 내 null 요소가 있을 경우에도 영향을 받습니다.♻️ 제안 수정
public static JoinDomain toDomain(JoinEntity entity) { + if (entity == null) { + return null; + } return JoinDomain.builder() .id(entity.getId()) .groupId(entity.getGroupId()) .userId(entity.getUserId()) .form(entity.getForm()) .status(entity.getStatus()) .build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/mapper/JoinMapper.java` around lines 36 - 44, JoinMapper.toDomain lacks a null check and can NPE when called from batch converters (e.g., toDomains()); add the same null-guard pattern used in GroupMemberMapper.toDomain by returning null immediately if the input JoinEntity is null before accessing fields. Update JoinMapper.toDomain so it safely handles null entities and keep behavior consistent with GroupMemberMapper.toDomain used in stream/map pipelines.src/main/java/flipnote/group/adapter/out/persistence/JoinRepositoryAdapter.java (1)
26-33:joinRepository.save(entity)반환 값 미사용
save()메서드는 엔티티의 식별자와 상태에 따라EntityManager.persist()또는merge()를 호출하며, 이후 업데이트에 반드시 사용해야 할 관리 인스턴스를 반환합니다.save()가 전달된 인스턴스와 다른 인스턴스를 반환할 수 있기 때문에, 반환 값을 무시하면 변경 사항이 예상대로 영속화되지 않을 수 있습니다.♻️ 제안 수정
`@Override` public JoinDomain save(JoinDomain domain) { JoinEntity entity = JoinMapper.createNewEntity(domain); - joinRepository.save(entity); - return JoinMapper.toDomain(entity); + JoinEntity saved = joinRepository.save(entity); + return JoinMapper.toDomain(saved); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/JoinRepositoryAdapter.java` around lines 26 - 33, The call to joinRepository.save(entity) in JoinRepositoryAdapter.save ignores the saved/managed instance returned by save(), which can differ from the passed entity; update the method to capture the returned entity (e.g. savedEntity = joinRepository.save(entity)) and pass that saved/managed instance into JoinMapper.toDomain instead of the original JoinEntity so persistence state and identifiers are correct (references: JoinRepositoryAdapter.save, joinRepository.save, JoinMapper.createNewEntity, JoinMapper.toDomain, JoinEntity, JoinDomain).src/main/java/flipnote/group/adapter/in/web/GroupQueryController.java (1)
24-33:findGroup메서드 명칭 모호성
GroupController에도findGroup메서드(단건 조회)가 존재하여 혼동의 여지가 있습니다. 목록 조회임을 명확히 하기 위해findAllGroups로 변경하는 것을 고려해 주세요.♻️ 제안 수정
//그룹 전체 조회 `@GetMapping` -public ResponseEntity<CursorPagingResponseDto<GroupInfo>> findGroup( +public ResponseEntity<CursorPagingResponseDto<GroupInfo>> findAllGroups( `@RequestHeader`("X-USER-ID") Long userId, `@Valid` `@ModelAttribute` GroupListRequestDto req ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/in/web/GroupQueryController.java` around lines 24 - 33, Rename the ambiguous controller method GroupQueryController.findGroup to findAllGroups to clarify it performs a list lookup; update the method signature name in GroupQueryController and any callers (e.g., tests or route registries) that reference GroupQueryController.findGroup, and keep the `@GetMapping` and parameter list unchanged; also verify and, if necessary, align the use-case call site naming (findGroupUseCase.findAllGroup) so callers remain consistent after renaming.src/main/java/flipnote/group/adapter/in/web/MemberController.java (1)
30-42:GET /members엔드포인트에 페이지네이션 없음현재 그룹의 모든 멤버를 단일 응답으로 반환합니다. 대규모 그룹에서는 메모리 사용 및 응답 크기 측면에서 문제가 될 수 있습니다. 커서 기반 페이지네이션(이 PR의
GroupQueryController에서 이미 사용 중인 패턴)을 도입하는 것을 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/in/web/MemberController.java` around lines 30 - 42, The findGroupMembers GET handler currently returns all members at once; change MemberController.findGroupMembers to accept cursor and limit query parameters (e.g., String cursor, Integer limit) and pass them into FindGroupMemberCommand so the use case can perform cursor-based pagination (follow the pattern used in GroupQueryController). Update FindGroupMemberCommand and FindGroupMemberResult to carry cursor/limit inputs and a nextCursor plus a paged list of members, and adapt FindGroupMemberUseCase implementation to return only the page and nextCursor. Finally update FindGroupMemberResponseDto.from(...) to include the paged members and nextCursor in the HTTP response while keeping `@RequestHeader`("X-USER-ID") and `@PathVariable`("groupId") unchanged.src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryImpl.java (2)
24-25: Q타입 필드를private static final로 선언 권장
QGroupEntity group과QGroupMemberEntity groupMember가 인스턴스 필드로 선언되어 있습니다. Q타입은 불변(immutable) 객체이므로 기능상 문제는 없으나, Spring 싱글턴 빈 내에서 인스턴스 필드로 두면 가독성이 낮아지고 Q타입의 관용적 사용 방식과 다릅니다.private static final로 선언하는 것이 일반적입니다.♻️ 수정 제안
- QGroupEntity group = QGroupEntity.groupEntity; - QGroupMemberEntity groupMember = QGroupMemberEntity.groupMemberEntity; + private static final QGroupEntity group = QGroupEntity.groupEntity; + private static final QGroupMemberEntity groupMember = QGroupMemberEntity.groupMemberEntity;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryImpl.java` around lines 24 - 25, The Q-type fields QGroupEntity group and QGroupMemberEntity groupMember in GroupRepositoryImpl should be declared as immutable class-level constants; change their declarations to private static final QGroupEntity group = QGroupEntity.groupEntity and private static final QGroupMemberEntity groupMember = QGroupMemberEntity.groupMemberEntity so they are static constants consistent with Q-type usage in a Spring singleton bean (no other behavioral changes required; update any imports or references if necessary).
106-111:findAllByCursorAndCreatedUserIdJavadoc 내용 오류"그룹 테이블에 생성한 유저 추가"는 삽입(INSERT) 연산을 설명하는 문구이나, 실제로는 내가 생성한 그룹을 조회하는 메서드입니다. 올바른 설명으로 수정이 필요합니다.
♻️ 수정 제안
- /** - * 그룹 테이블에 생성한 유저 추가 - * `@param` lastId - * `@param` category - * `@param` pageSize - * `@param` userId - * `@return` - */ + /** + * 내가 생성한 그룹 전체 조회 (OWNER 역할 기준, 커서 기반 페이지네이션) + * `@param` lastId + * `@param` category + * `@param` pageSize + * `@param` userId + * `@return` + */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryImpl.java` around lines 106 - 111, Update the Javadoc for findAllByCursorAndCreatedUserId to accurately describe its behavior: it retrieves (selects) groups created by the specified user using cursor-based pagination and optional category filtering; document the parameters lastId, category, pageSize, and userId and the return value accordingly, replacing the current misleading "insert" wording with a clear description that this method queries the groups created by the given user.src/main/java/flipnote/group/application/service/ApplicationFormService.java (1)
79-91:findJoinFormList에@Transactional(readOnly = true)누락조회 전용 메서드에
@Transactional(readOnly = true)를 명시하면 JPA의 dirty-checking 스냅샷 생성을 건너뛰어 성능이 개선되고 의도가 명확해집니다.joinRepository.findFormList가 지연 로딩 연관 관계를 포함할 경우 트랜잭션 없이 접근하면LazyInitializationException이 발생할 수도 있습니다.♻️ 수정 제안
+ `@Transactional`(readOnly = true) `@Override` public FindJoinFormListResult findJoinFormList(FindJoinFormCommand cmd) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/application/service/ApplicationFormService.java` around lines 79 - 91, Annotate the findJoinFormList method in ApplicationFormService with `@Transactional`(readOnly = true) to ensure the repository call joinRepository.findFormList executes within a read-only transaction; add the import for org.springframework.transaction.annotation.Transactional if missing and keep existing logic (including groupRoleRepository.checkPermission and JOIN_MANAGE) unchanged so lazy-loaded relations are safe and JPA skips dirty-checking.src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java (1)
37-51:existsUserInGroupJavadoc@return태그 및 메서드명 불일치메서드가
void로 변경되었으나 Javadoc에@return태그가 남아 있습니다. 또한exists~라는 이름은 통상boolean을 반환하는 확인 메서드를 의미하므로, 예외를 던지는 현재 semantics와 맞지 않습니다.validateUserInGroup또는assertUserInGroup등으로 이름을 변경하는 것이 더 명확합니다.♻️ 수정 제안
- /** - * 유저가 그룹 내에 있는지 체크 - * `@param` groupId - * `@param` userId - * `@return` - */ - `@Override` - public void existsUserInGroup(Long groupId, Long userId) { + /** + * 유저가 그룹 내에 있는지 검증 (없으면 예외 발생) + * `@param` groupId + * `@param` userId + */ + `@Override` + public void validateUserInGroup(Long groupId, Long userId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/flipnote/group/adapter/out/persistence/GroupMemberRepositoryAdapter.java` around lines 37 - 51, The method existsUserInGroup has mismatched Javadoc and misleading name for a void method that throws on absence; rename the method to validateUserInGroup (or assertUserInGroup) across the class and its interface to reflect its behavior, update the Javadoc to describe validation (remove the `@return` tag and keep `@param` tags), and adjust any callers to the new method name; keep the implementation using groupMemberRepository.existsByGroupIdAndUserId(groupId, userId) and the IllegalArgumentException (optionally improve the message) so behavior remains unchanged.
| @Override | ||
| public List<GroupInfo> findAllByCursor(Long cursorId, Category category, int size) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| @Override | ||
| public List<GroupInfo> findAllByCursorAndUserId(Long cursorId, Category category, int size, Long userId) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| @Override | ||
| public List<GroupInfo> findAllByCursorAndCreatedUserId(Long cursorId, Category category, int size, Long userId) { | ||
| return List.of(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and check GroupRepository interface
fd "GroupRepository\.java" --type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 151
🏁 Script executed:
# Find and check GroupRepositoryAdapter
fd "GroupRepositoryAdapter\.java" --type f -exec cat -n {} +Repository: FlipNoteTeam/FlipNote-Group
Length of output: 301
🏁 Script executed:
# Find GroupRepositoryImpl to check actual implementation
fd "GroupRepositoryImpl\.java" --type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 160
🏁 Script executed:
# Check for GroupRepositoryCustom
fd "GroupRepositoryCustom\.java" --type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 162
🏁 Script executed:
cat src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupRepository.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 322
🏁 Script executed:
cat src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryCustom.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 589
🏁 Script executed:
cat src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryImpl.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 3313
🏁 Script executed:
cat src/main/java/flipnote/group/adapter/out/persistence/GroupRepositoryAdapter.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 1871
스텁 구현 — 실제 QueryDSL 구현이 전혀 호출되지 않아 기능이 동작하지 않습니다
세 메서드 모두 List.of()를 반환하는 스텁 상태입니다. GroupRepositoryImpl에 findAllByCursor, findAllByCursorAndUserId, findAllByCursorAndCreatedUserId의 실제 QueryDSL 구현이 있음에도 GroupRepositoryAdapter에서 이를 호출하지 않아, findAllGroup/findMyGroup/findCreatedGroup API는 항상 빈 결과를 반환합니다.
GroupRepository가 GroupRepositoryCustom을 상속하지 않기 때문에, GroupRepositoryAdapter에서는 실제 구현에 접근할 수 없습니다. GroupRepositoryCustom을 별도로 주입받아 위임하거나, GroupRepository가 GroupRepositoryCustom을 상속하도록 수정해야 합니다.
🔧 수정 예시 (GroupRepositoryCustom 주입 방식)
`@Repository`
`@RequiredArgsConstructor`
public class GroupRepositoryAdapter implements GroupRepositoryPort {
private final GroupRepository groupRepository;
+ private final GroupRepositoryCustom groupRepositoryCustom;
...
`@Override`
public List<GroupInfo> findAllByCursor(Long cursorId, Category category, int size) {
- return List.of();
+ return groupRepositoryCustom.findAllByCursor(cursorId, category, size);
}
`@Override`
public List<GroupInfo> findAllByCursorAndUserId(Long cursorId, Category category, int size, Long userId) {
- return List.of();
+ return groupRepositoryCustom.findAllByCursorAndUserId(cursorId, category, size, userId);
}
`@Override`
public List<GroupInfo> findAllByCursorAndCreatedUserId(Long cursorId, Category category, int size, Long userId) {
- return List.of();
+ return groupRepositoryCustom.findAllByCursorAndCreatedUserId(cursorId, category, size, userId);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/flipnote/group/adapter/out/persistence/GroupRepositoryAdapter.java`
around lines 50 - 63, The three methods in GroupRepositoryAdapter
(findAllByCursor, findAllByCursorAndUserId, findAllByCursorAndCreatedUserId) are
stubs returning List.of() and never delegate to the real QueryDSL implementation
in GroupRepositoryImpl; fix by wiring the custom repository into the adapter and
delegating the calls: either have GroupRepository extend GroupRepositoryCustom
so the existing GroupRepository bean exposes the implementations, or inject
GroupRepositoryCustom directly (add a constructor field for
GroupRepositoryCustom) and forward each adapter method to the corresponding
method on the custom repo (call findAllByCursor(...),
findAllByCursorAndUserId(...), findAllByCursorAndCreatedUserId(...)
respectively), ensuring method signatures and parameters are passed through and
removing the List.of() stubs.
| public record ApplicationFormCommand( | ||
| Long userId, | ||
| Long groupId, | ||
| String joinIntro | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=java -C 8 "ApplicationFormCommand"Repository: FlipNoteTeam/FlipNote-Group
Length of output: 10002
userId 반드시 인증된 주체(authenticated principal)에서 추출하기
현재 JoinController.java 라인 40에서 userId를 @RequestHeader("X-USER-ID")로 받고 있습니다. HTTP 헤더는 클라이언트가 임의로 조작할 수 있으므로, 클라이언트가 다른 사용자의 userId를 전송하여 그 사용자 대신 가입 신청을 할 수 있는 심각한 보안 취약점입니다.
@AuthenticationPrincipal이나 SecurityContext에서 인증된 주체의 userId를 추출하도록 변경해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/flipnote/group/application/port/in/command/ApplicationFormCommand.java`
around lines 3 - 7, The ApplicationFormCommand currently trusts a
client-provided userId; remove userId from the client surface and instead obtain
the authenticated principal server-side: change the ApplicationFormCommand
record to omit Long userId, update JoinController to stop reading
`@RequestHeader`("X-USER-ID") and instead extract the userId from the
authentication (use `@AuthenticationPrincipal` or
SecurityContextHolder.getContext().getAuthentication().getPrincipal()), then
construct ApplicationFormCommand with the server-derived userId when calling the
application service; ensure any service/handler signatures expecting userId are
adjusted to accept it from the controller/principal rather than the request
body/header.
| JoinStatus status = JoinStatus.ACCEPT; | ||
| if(group.getJoinPolicy().equals(JoinPolicy.OPEN)) { | ||
| status = JoinStatus.PENDING; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine JoinPolicy enum definition
rg -n "enum JoinPolicy" --type java -A 20Repository: FlipNoteTeam/FlipNote-Group
Length of output: 419
🏁 Script executed:
#!/bin/bash
# Also search for JoinPolicy class/interface definition in case it's not an enum
rg -n "JoinPolicy" --type java | head -30Repository: FlipNoteTeam/FlipNote-Group
Length of output: 2855
🏁 Script executed:
#!/bin/bash
# Look at the complete ApplicationFormService.java file to understand context
find . -name "ApplicationFormService.java" -type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Find JoinStatus enum definition
rg -n "enum JoinStatus" --type java -A 5Repository: FlipNoteTeam/FlipNote-Group
Length of output: 325
🏁 Script executed:
#!/bin/bash
# View ApplicationFormService.java around lines 50-70
sed -n '50,70p' src/main/java/flipnote/group/application/service/ApplicationFormService.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 703
JoinStatus 결정 로직이 반전됨 — 가입 정책 적용이 역방향
현재 코드는 JoinPolicy.OPEN일 때 PENDING을, 그 외 정책(APPROVAL)일 때 ACCEPT를 부여합니다. 하지만 JoinPolicy 열거형의 정의에 따르면 OPEN은 "바로 가입 가능"이고 APPROVAL은 "가입 승인 필요"입니다.
현재 로직대로면:
- 공개 그룹(OPEN): 신청해도
PENDING상태로 승인을 기다림 - 승인 필요 그룹(APPROVAL): 신청하면
ACCEPT상태로 즉시 멤버로 등록됨
이는 정책의 의미와 완전히 반대입니다. 로직을 수정하세요.
수정 예시
- JoinStatus status = JoinStatus.ACCEPT;
- if(group.getJoinPolicy().equals(JoinPolicy.OPEN)) {
- status = JoinStatus.PENDING;
- }
+ JoinStatus status = JoinStatus.PENDING;
+ if(group.getJoinPolicy().equals(JoinPolicy.OPEN)) {
+ status = JoinStatus.ACCEPT;
+ }📝 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.
| JoinStatus status = JoinStatus.ACCEPT; | |
| if(group.getJoinPolicy().equals(JoinPolicy.OPEN)) { | |
| status = JoinStatus.PENDING; | |
| } | |
| JoinStatus status = JoinStatus.PENDING; | |
| if(group.getJoinPolicy().equals(JoinPolicy.OPEN)) { | |
| status = JoinStatus.ACCEPT; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/group/application/service/ApplicationFormService.java`
around lines 57 - 60, In ApplicationFormService change the inverted join-status
logic: currently the block using group.getJoinPolicy() sets status =
JoinStatus.PENDING when JoinPolicy.OPEN; instead ensure OPEN yields
JoinStatus.ACCEPT and APPROVAL (or non-OPEN) yields JoinStatus.PENDING; update
the conditional around JoinPolicy (the variables JoinStatus, JoinPolicy and the
group.getJoinPolicy() check) so the semantics match the enum definitions (OPEN
-> ACCEPT, APPROVAL -> PENDING).
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import flipnote.group.adapter.out.entity.RoleEntity; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the CreateGroupService.java file and read its contents
find . -name "CreateGroupService.java" -type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 145
🏁 Script executed:
# Check for RoleEntity usage in the entire file
rg -n "RoleEntity" src/main/java/flipnote/group/application/service/CreateGroupService.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 125
🏁 Script executed:
# Check all imports in CreateGroupService.java and look for adapter.out references
rg -n "^import" src/main/java/flipnote/group/application/service/CreateGroupService.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 882
🏁 Script executed:
# Get the full content of CreateGroupService.java to verify context
cat -n src/main/java/flipnote/group/application/service/CreateGroupService.java | head -100Repository: FlipNoteTeam/FlipNote-Group
Length of output: 1992
RoleEntity 임포트는 헥사고날 아키텍처 위반이며 실제로 사용되지 않습니다
RoleEntity는 인프라 레이어(adapter.out.entity)에 속하는 클래스입니다. 헥사고날 아키텍처에서 애플리케이션 서비스 레이어는 포트 인터페이스와 도메인 모델에만 의존해야 하며, 어댑터 레이어의 엔티티를 직접 참조하면 안 됩니다. 서비스 코드 내에서 RoleEntity가 실제로 사용되지 않으므로 즉시 제거하세요.
제안 수정
-import flipnote.group.adapter.out.entity.RoleEntity;
import flipnote.group.adapter.out.persistence.mapper.GroupMapper;📝 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.
| import flipnote.group.adapter.out.entity.RoleEntity; | |
| import flipnote.group.adapter.out.persistence.mapper.GroupMapper; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/group/application/service/CreateGroupService.java` at
line 6, Remove the unused infrastructure-layer import of RoleEntity from
CreateGroupService: delete the line importing
flipnote.group.adapter.out.entity.RoleEntity so the application service no
longer depends on an adapter package; ensure no other references to RoleEntity
exist in the CreateGroupService class (if there are, replace them with the
appropriate domain model or port interface) and run tests/compile to confirm the
service only references domain and port types.
| @Override | ||
| public FindGroupMemberResult findGroupMember(FindGroupMemberCommand cmd) { | ||
|
|
||
| groupMemberRepository.existsUserInGroup(cmd.groupId(), cmd.userId()); |
There was a problem hiding this comment.
existsUserInGroup()의 반환값이 무시되어 인가 검사가 무효화됩니다.
existsUserInGroup이 false를 반환해도 아무런 예외 처리나 분기가 없으므로, 그룹에 속하지 않은 사용자도 모든 멤버 목록을 조회할 수 있습니다. 이 메서드의 결과를 확인하여 그룹 미가입 시 예외를 던져야 합니다.
🔒 수정 제안
- groupMemberRepository.existsUserInGroup(cmd.groupId(), cmd.userId());
+ if (!groupMemberRepository.existsUserInGroup(cmd.groupId(), cmd.userId())) {
+ throw new IllegalArgumentException("해당 그룹의 멤버가 아닙니다.");
+ }📝 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.
| groupMemberRepository.existsUserInGroup(cmd.groupId(), cmd.userId()); | |
| if (!groupMemberRepository.existsUserInGroup(cmd.groupId(), cmd.userId())) { | |
| throw new IllegalArgumentException("해당 그룹의 멤버가 아닙니다."); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/group/application/service/FindGroupMemberService.java`
at line 28, The call to groupMemberRepository.existsUserInGroup(cmd.groupId(),
cmd.userId()) in FindGroupMemberService currently ignores its boolean result;
change the logic to check the returned value and throw an
authorization/validation exception (e.g., throw new UnauthorizedException or
GroupNotMemberException) when it returns false so non-members cannot list group
members; update the method surrounding this call (in FindGroupMemberService) to
perform this conditional check before proceeding to fetch members.
| // 유저가 그룹 내에 존재하는지 확인 | ||
| boolean isMember = groupMemberRepository.existsUserInGroup(cmd.groupId(), cmd.userId()); | ||
|
|
||
| if(!isMember) { | ||
| throw new IllegalArgumentException("user not in Group"); | ||
| } | ||
| groupMemberRepository.existsUserInGroup(cmd.groupId(), cmd.userId()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for existsUserInGroup method definition and implementation
rg -n "existsUserInGroup" --type java -B 2 -A 10Repository: FlipNoteTeam/FlipNote-Group
Length of output: 7619
🏁 Script executed:
#!/bin/bash
# Search for createGroupInfoCursorPagingResponse implementation
rg -n "createGroupInfoCursorPagingResponse" --type java -A 15Repository: FlipNoteTeam/FlipNote-Group
Length of output: 5125
createGroupInfoCursorPagingResponse의 엣지 케이스에서 IndexOutOfBoundsException 발생
req.getSize() == 0일 때 다음과 같은 순서로 예외가 발생합니다:
- 라인 100:
groups.size() > 0이면hasNext = true - 라인 103:
groups = groups.subList(0, 0)→ 빈 리스트 반환 - 라인 106:
hasNext ? groups.get(groups.size() - 1) ...→groups.get(-1)호출 →IndexOutOfBoundsException
hasNext 값 결정 후 groups를 재할당하기 때문에, hasNext가 true일 때 재할당된 빈 groups에서 마지막 요소에 접근하려고 시도합니다. req.getSize()이 0이 아닌 경우에만 안전합니다.
부가사항: existsUserInGroup 메서드는 검증 실패 시 IllegalArgumentException을 던지는 방식으로 구현되어 있으므로, guard 로직은 예외 메커니즘을 통해 보장됩니다. 다만 메서드명이 boolean 반환을 암시하므로 validateUserInGroup 또는 assertUserInGroup으로 명칭 변경을 검토할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/group/application/service/FindGroupService.java`
around lines 34 - 35, The has-next calculation in
createGroupInfoCursorPagingResponse is done before groups is truncated, so when
req.getSize() == 0 you can end up with hasNext==true then groups becomes empty
and groups.get(groups.size()-1) throws IndexOutOfBoundsException; fix by
computing/truthy-checking hasNext only after handling the size-zero truncation
(i.e., if req.getSize() > 0 then truncate groups = groups.subList(0,
req.getSize()) and set hasNext = originalSize > req.getSize(), otherwise ensure
hasNext = false and skip accessing groups.get(groups.size() - 1)); also consider
renaming existsUserInGroup to validateUserInGroup or assertUserInGroup if you
keep its exception-based contract.
|
|
||
| import flipnote.group.domain.model.group.GroupId; | ||
| import flipnote.group.domain.model.user.UserId; | ||
| import flipnote.group.adapter.out.entity.RoleEntity; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the GroupMember.java file
find . -name "GroupMember.java" -type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 138
🏁 Script executed:
# Let's also check the directory structure of the group module
find . -path "*/flipnote/group/*" -type d | head -20Repository: FlipNoteTeam/FlipNote-Group
Length of output: 1027
🏁 Script executed:
# Let's examine the GroupMember.java file, focusing on imports and structure
cat -n ./src/main/java/flipnote/group/domain/model/member/GroupMember.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 793
🏁 Script executed:
# Now run the verification script from the review comment to find other adapter imports in domain layer
rg -n "import flipnote.group.adapter" --type=java -g '*/domain/*'Repository: FlipNoteTeam/FlipNote-Group
Length of output: 219
🏁 Script executed:
# Broader search for adapter imports in domain layer files
find ./src/main/java/flipnote/group/domain -name "*.java" -type f -exec grep -l "import flipnote.group.adapter" {} \;Repository: FlipNoteTeam/FlipNote-Group
Length of output: 138
🏁 Script executed:
# Let's also check what RoleEntity looks like
find . -path "*/adapter/out/entity/RoleEntity.java" -type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 136
🏁 Script executed:
# Check if there are other domain models with similar issues
find ./src/main/java/flipnote/group/domain -name "*.java" -type f -exec grep -l "import flipnote.group.adapter.out.entity" {} \;Repository: FlipNoteTeam/FlipNote-Group
Length of output: 138
🏁 Script executed:
# Look for any domain-level role enum or value object
find ./src/main/java/flipnote/group/domain -name "*Role*" -type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 142
🏁 Script executed:
# Let's examine the domain-level GroupMemberRole
cat -n ./src/main/java/flipnote/group/domain/model/member/GroupMemberRole.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 220
🏁 Script executed:
# And the RoleEntity
cat -n ./src/main/java/flipnote/group/adapter/out/entity/RoleEntity.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 1801
🏁 Script executed:
# Check for mapper files that might handle RoleEntity mapping
find ./src/main/java/flipnote/group -name "*Mapper*" -o -name "*mapper*" | grep -i roleRepository: FlipNoteTeam/FlipNote-Group
Length of output: 53
🏁 Script executed:
# Look at the mapper directory to see what conversions exist
find ./src/main/java/flipnote/group/adapter/out/persistence/mapper -name "*.java" -type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 312
🏁 Script executed:
# Check if any other domain models use domain-level types instead of entities
find ./src/main/java/flipnote/group/domain/model -name "*.java" -type f -exec grep -l "private.*Entity" {} \;Repository: FlipNoteTeam/FlipNote-Group
Length of output: 138
🏁 Script executed:
# Let's examine the GroupMemberMapper to see how it's supposed to handle the conversion
cat -n ./src/main/java/flipnote/group/adapter/out/persistence/mapper/GroupMemberMapper.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 1879
도메인 모델이 어댑터 계층의 JPA 엔티티에 직접 의존하고 있습니다 — 헥사고날 아키텍처 위반입니다.
GroupMember 도메인 모델이 라인 3에서 flipnote.group.adapter.out.entity.RoleEntity를 임포트하고, 라인 15에서 필드 타입으로 사용하고 있습니다. 그러나 도메인 레이어에는 이미 GroupMemberRole enum이 정의되어 있습니다. 이는 명확한 의존성 방향 위반입니다.
GroupMemberMapper.toDomain() (라인 33)은 RoleEntity를 도메인 모델에 직접 할당하고 있는데, 이는 도메인을 JPA 엔티티와 결합시킵니다. 대신 entity.getRole().getRole()처럼 RoleEntity에서 GroupMemberRole을 추출하여 도메인 모델에 전달해야 합니다.
수정 사항:
GroupMember.role필드 타입을RoleEntity에서GroupMemberRole로 변경import flipnote.group.adapter.out.entity.RoleEntity;제거- 생성자 파라미터 타입 변경
GroupMemberMapper.toDomain()에서 라인 33을entity.getRole().getRole()로 수정
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/flipnote/group/domain/model/member/GroupMember.java` at line 3,
The GroupMember domain class currently depends on the adapter-layer JPA entity
RoleEntity; change the domain to use the domain enum GroupMemberRole instead:
remove the import of flipnote.group.adapter.out.entity.RoleEntity, update the
GroupMember.role field and its constructor parameter type from RoleEntity to
GroupMemberRole, and update all usages accordingly. In the mapper
(GroupMemberMapper.toDomain) stop assigning the RoleEntity directly and pass
entity.getRole().getRole() (extracting the GroupMemberRole from the RoleEntity)
into the GroupMember constructor. Ensure any getters/setters or equals/hashCode
in GroupMember reference GroupMemberRole after the change.
| package flipnote.group.infrastructure.persistence.querydsl; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import com.querydsl.core.BooleanBuilder; | ||
| import com.querydsl.core.types.Projections; | ||
| import com.querydsl.jpa.impl.JPAQueryFactory; | ||
|
|
||
| import flipnote.group.adapter.out.entity.QGroupEntity; | ||
| import flipnote.group.adapter.out.entity.QGroupMemberEntity; | ||
| import flipnote.group.domain.model.group.Category; | ||
| import flipnote.group.domain.model.group.GroupInfo; | ||
| import flipnote.group.domain.model.member.GroupMemberRole; | ||
| import lombok.RequiredArgsConstructor; | ||
|
|
||
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class GroupRepositoryImpl implements GroupRepositoryCustom { | ||
|
|
||
| private final JPAQueryFactory queryFactory; | ||
|
|
||
| QGroupEntity group = QGroupEntity.groupEntity; | ||
| QGroupMemberEntity groupMember = QGroupMemberEntity.groupMemberEntity; | ||
|
|
||
| /** | ||
| * 모든 그룹 전체 조회 | ||
| * @param lastId | ||
| * @param category | ||
| * @param pageSize | ||
| * @return | ||
| */ | ||
| @Override | ||
| public List<GroupInfo> findAllByCursor(Long lastId, Category category, int pageSize) { | ||
| //삭제되지 않은 | ||
| BooleanBuilder where = new BooleanBuilder() | ||
| .and(group.deletedAt.isNull()); | ||
|
|
||
| //커서 기반 | ||
| if (lastId != null) { | ||
| where.and(group.id.lt(lastId)); | ||
| } | ||
|
|
||
| //카테고리 제한 | ||
| if (category != null) { | ||
| where.and(group.category.eq(category)); | ||
| } | ||
|
|
||
| return queryFactory.select(Projections.constructor( | ||
| GroupInfo.class, | ||
| group.id, | ||
| group.name, | ||
| group.description, | ||
| group.category, | ||
| group.imageRefId | ||
| )) | ||
| .from(group) | ||
| .where(where) | ||
| .orderBy(group.id.desc()) | ||
| .limit(pageSize + 1) | ||
| .fetch(); | ||
| } | ||
|
|
||
| /** | ||
| * 내가 가입한 그룹 전체 조회 | ||
| * @param lastId | ||
| * @param category | ||
| * @param pageSize | ||
| * @param userId | ||
| * @return | ||
| */ | ||
| @Override | ||
| public List<GroupInfo> findAllByCursorAndUserId(Long lastId, Category category, int pageSize, Long userId) { | ||
|
|
||
| BooleanBuilder where = new BooleanBuilder() | ||
| .and(group.deletedAt.isNull()) | ||
| .and(groupMember.userId.eq(userId)); | ||
|
|
||
| if (lastId != null) { | ||
| where.and(group.id.lt(lastId)); | ||
| } | ||
|
|
||
| if (category != null) { | ||
| where.and(group.category.eq(category)); | ||
| } | ||
|
|
||
| return queryFactory | ||
| .select(Projections.constructor( | ||
| GroupInfo.class, | ||
| group.id, | ||
| group.name, | ||
| group.description, | ||
| group.category, | ||
| group.imageRefId | ||
| )) | ||
| .from(groupMember) | ||
| .join(group).on(group.id.eq(groupMember.groupId)) | ||
| .where(where) | ||
| .orderBy(group.id.desc()) | ||
| .limit(pageSize + 1) | ||
| .fetch(); | ||
| } | ||
|
|
||
| /** | ||
| * 그룹 테이블에 생성한 유저 추가 | ||
| * @param lastId | ||
| * @param category | ||
| * @param pageSize | ||
| * @param userId | ||
| * @return | ||
| */ | ||
| @Override | ||
| public List<GroupInfo> findAllByCursorAndCreatedUserId(Long lastId, Category category, int pageSize, Long userId) { | ||
|
|
||
| return queryFactory | ||
| .select(Projections.constructor( | ||
| GroupInfo.class, | ||
| group.id, | ||
| group.name, | ||
| group.description, | ||
| group.category, | ||
| group.imageRefId | ||
| )) | ||
| .from(group) | ||
| .join(groupMember).on(groupMember.groupId.eq(group.id)) | ||
| .where( | ||
| group.deletedAt.isNull(), | ||
| groupMember.userId.eq(userId), | ||
| groupMember.role.role.eq(GroupMemberRole.OWNER), | ||
| lastId != null ? group.id.lt(lastId) : null, | ||
| category != null ? group.category.eq(category) : null | ||
| ) | ||
| .orderBy(group.id.desc()) | ||
| .limit(pageSize + 1) | ||
| .fetch(); | ||
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# GroupRepository 인터페이스가 GroupRepositoryCustom을 extends 하는지 확인
echo "=== Checking GroupRepository interface ==="
find . -name "GroupRepository.java" -type f | head -5Repository: FlipNoteTeam/FlipNote-Group
Length of output: 196
🏁 Script executed:
#!/bin/bash
# GroupRepository 인터페이스 내용 조회
rg -n "interface GroupRepository" --type java -A 10Repository: FlipNoteTeam/FlipNote-Group
Length of output: 2484
🏁 Script executed:
#!/bin/bash
# GroupRepositoryAdapter의 커서 메서드 구현 확인
rg -n "class GroupRepositoryAdapter" --type java -A 100 | head -120Repository: FlipNoteTeam/FlipNote-Group
Length of output: 5102
GroupRepository가 GroupRepositoryCustom을 extends하지 않아 QueryDSL 구현이 완전히 고장난 상태
검증 결과, GroupRepository 인터페이스가 JpaRepository만 상속하고 GroupRepositoryCustom을 상속하지 않습니다. Spring Data JPA의 커스텀 리포지토리 패턴이 정상 작동하려면:
GroupRepository는 반드시GroupRepositoryCustom을extends해야 합니다 (src/main/java/flipnote/group/infrastructure/persistence/jpa/GroupRepository.java수정 필요)GroupRepositoryAdapter의 세 커서 메서드는List.of()스텁으로 남아있습니다:findAllByCursor()(라인 51-52)findAllByCursorAndUserId()(라인 56-57)findAllByCursorAndCreatedUserId()(라인 61-63)
이 두 조건을 모두 충족해야만 GroupRepositoryImpl의 QueryDSL 구현체가 실제로 동작합니다. 현재 상태에서는 새로 추가된 QueryDSL 쿼리가 어떤 경로로도 실행될 수 없습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryImpl.java`
around lines 1 - 140, The repository wiring is broken: update the
GroupRepository interface to extend GroupRepositoryCustom so Spring Data picks
up GroupRepositoryImpl (ensure interface declaration includes "extends
GroupRepositoryCustom"), and replace the three stubbed methods in
GroupRepositoryAdapter—findAllByCursor, findAllByCursorAndUserId,
findAllByCursorAndCreatedUserId—so they delegate to the Spring Data
GroupRepository (remove List.of() stubs and call the corresponding
GroupRepository methods with the same signatures), ensuring method names and
parameter lists match the QueryDSL implementations in GroupRepositoryImpl.
| public List<GroupInfo> findAllByCursor(Long lastId, Category category, int pageSize) { | ||
| //삭제되지 않은 | ||
| BooleanBuilder where = new BooleanBuilder() | ||
| .and(group.deletedAt.isNull()); | ||
|
|
||
| //커서 기반 | ||
| if (lastId != null) { | ||
| where.and(group.id.lt(lastId)); | ||
| } | ||
|
|
||
| //카테고리 제한 | ||
| if (category != null) { | ||
| where.and(group.category.eq(category)); | ||
| } | ||
|
|
||
| return queryFactory.select(Projections.constructor( | ||
| GroupInfo.class, | ||
| group.id, | ||
| group.name, | ||
| group.description, | ||
| group.category, | ||
| group.imageRefId | ||
| )) | ||
| .from(group) | ||
| .where(where) | ||
| .orderBy(group.id.desc()) | ||
| .limit(pageSize + 1) | ||
| .fetch(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Visibility enum definition
rg -n "enum Visibility" --type java -A 15Repository: FlipNoteTeam/FlipNote-Group
Length of output: 312
🏁 Script executed:
#!/bin/bash
# Check for visibility field in Group/GroupEntity
rg -n "visibility" --type java -B 2 -A 2 | head -100Repository: FlipNoteTeam/FlipNote-Group
Length of output: 8265
🏁 Script executed:
#!/bin/bash
# Find GroupRepositoryImpl file to understand the context
find . -name "GroupRepositoryImpl.java" -type fRepository: FlipNoteTeam/FlipNote-Group
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Check for API controller that uses findAllByCursor
rg -n "findAllByCursor" --type java -B 3 -A 3Repository: FlipNoteTeam/FlipNote-Group
Length of output: 9059
🏁 Script executed:
#!/bin/bash
# Read the full GroupRepositoryImpl.java file
cat src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryImpl.javaRepository: FlipNoteTeam/FlipNote-Group
Length of output: 3313
🏁 Script executed:
#!/bin/bash
# Check GroupController for visibility filtering
rg -n "findAllGroup" --type java -B 5 -A 10Repository: FlipNoteTeam/FlipNote-Group
Length of output: 4583
findAllByCursor에 Visibility 필터 누락 — 비공개 그룹이 공개 목록에 노출
전체 그룹 조회 API(GroupQueryController.findGroup)는 findAllByCursor를 사용하는데, 현재 group.deletedAt.isNull() 외에 visibility 조건이 없어 Visibility.PRIVATE 그룹도 응답에 포함됩니다. 동일한 문제가 findAllByCursorAndUserId와 findAllByCursorAndCreatedUserId에도 있습니다.
🐛 수정 제안
+import flipnote.group.domain.model.group.Visibility;
// ...
BooleanBuilder where = new BooleanBuilder()
- .and(group.deletedAt.isNull());
+ .and(group.deletedAt.isNull())
+ .and(group.visibility.eq(Visibility.PUBLIC));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/flipnote/group/infrastructure/persistence/querydsl/GroupRepositoryImpl.java`
around lines 35 - 63, findAllByCursor (and similarly findAllByCursorAndUserId
and findAllByCursorAndCreatedUserId) currently only filters deleted groups but
omits visibility, allowing PRIVATE groups to be returned; update each method to
add a visibility predicate (e.g., require group.visibility == Visibility.PUBLIC
or exclude Visibility.PRIVATE) to the BooleanBuilder (the variable where) so
only public groups are returned, referencing the group.visibility property and
the Visibility enum in the condition; ensure this new predicate is combined with
existing conditions before executing the query.
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링