Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public class GroupJpaEntity {
@Enumerated(EnumType.STRING)
private GroupCategory category;

@ManyToOne(fetch = FetchType.LAZY, optional = false)
@JoinColumn(name = "fk_user_id", nullable = false)
@ManyToOne(fetch = FetchType.LAZY, optional = true)
@JoinColumn(name = "fk_user_id", nullable = true)
private UserJpaEntity owner;
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public Group toDomain(GroupJpaEntity groupJpaEntity) {
groupJpaEntity.getPassword(),
groupJpaEntity.getPasswordRequired(),
groupJpaEntity.getCategory(),
groupJpaEntity.getOwner().getId()
groupJpaEntity.getOwner() != null ? groupJpaEntity.getOwner().getId() : null
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@
public interface GroupJpaRepository extends JpaRepository<GroupJpaEntity, Long> {

@EntityGraph(attributePaths = {"owner"})
Page<GroupJpaEntity> findAll(Pageable pageable);
@Query("select g from GroupJpaEntity g where g.category != :excluded")
Page<GroupJpaEntity> findAllExcluding(@Param("excluded") GroupCategory excluded, Pageable pageable);

@EntityGraph(attributePaths = {"owner"})
Page<GroupJpaEntity> findAllByCategory(GroupCategory category, Pageable pageable);

@EntityGraph(attributePaths = {"owner"})
Optional<GroupJpaEntity> findById(Long id);

@EntityGraph(attributePaths = {"owner"})
Optional<GroupJpaEntity> findByCategory(GroupCategory category);
Comment on lines +26 to +27

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The findByCategory method returns an Optional, which implies that only one group can exist per category. While this is correct for the GLOBAL category, other categories like TEAM or CLUB typically have multiple groups. Calling this method with such categories will result in a NonUniqueResultException at runtime. Consider renaming this to findGlobalGroup() or changing the return type to List<GroupJpaEntity> to avoid unexpected crashes. Additionally, adding a unique constraint on the category column for the GLOBAL value in the database migration would further ensure data integrity.


boolean existsByName(String name);

@EntityGraph(attributePaths = {"owner"})
Expand All @@ -30,8 +34,9 @@ public interface GroupJpaRepository extends JpaRepository<GroupJpaEntity, Long>
from GroupJpaEntity g
join GroupMemberJpaEntity gm on gm.group = g
where gm.user.id = :userId
and g.category != :excluded
""")
Page<GroupJpaEntity> findAllByMemberUserId(@Param("userId") Long userId, Pageable pageable);
Page<GroupJpaEntity> findAllByMemberUserId(@Param("userId") Long userId, @Param("excluded") GroupCategory excluded, Pageable pageable);

@EntityGraph(attributePaths = {"owner"})
@Query("""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,19 @@ public class GroupPersistenceAdapter implements GroupRepositoryPort {

@Override
public Group save(Group group) {
UserJpaEntity owner = userJpaRepository.getReferenceById(group.ownerId());
UserJpaEntity owner = group.ownerId() != null
? userJpaRepository.getReferenceById(group.ownerId())
: null;
GroupJpaEntity entity = groupJpaMapper.toJpaEntity(group, owner);
GroupJpaEntity saved = groupJpaRepository.save(entity);
return groupJpaMapper.toDomain(saved);
}

@Override
public Optional<Group> findByCategory(GroupCategory category) {
return groupJpaRepository.findByCategory(category).map(groupJpaMapper::toDomain);
}

@Override
public Optional<Group> findById(Long groupId) {
return groupJpaRepository.findById(groupId)
Expand All @@ -52,7 +59,7 @@ public void deleteById(Long groupId) {
@Override
public PageResult findAllByPage(Integer page, Integer size) {
Pageable pageable = PageRequest.of(page - 1, size, Sort.by(Sort.Direction.DESC, "createdAt"));
Page<GroupJpaEntity> pageResult = groupJpaRepository.findAll(pageable);
Page<GroupJpaEntity> pageResult = groupJpaRepository.findAllExcluding(GroupCategory.GLOBAL, pageable);
return mapPageResult(pageResult);
}

Expand All @@ -66,12 +73,15 @@ public PageResult findAllByPageAndCategory(Integer page, Integer size, GroupCate
@Override
public PageResult findAllByMemberUserId(Long userId, Integer page, Integer size) {
Pageable pageable = PageRequest.of(page - 1, size, Sort.by(Sort.Direction.DESC, "createdAt"));
Page<GroupJpaEntity> pageResult = groupJpaRepository.findAllByMemberUserId(userId, pageable);
Page<GroupJpaEntity> pageResult = groupJpaRepository.findAllByMemberUserId(userId, GroupCategory.GLOBAL, pageable);
return mapPageResult(pageResult);
}

@Override
public PageResult findAllByMemberUserIdAndCategory(Long userId, Integer page, Integer size, GroupCategory category) {
if (category == GroupCategory.GLOBAL) {
return new PageResult(List.of(), 0);
}
Pageable pageable = PageRequest.of(page - 1, size, Sort.by(Sort.Direction.DESC, "createdAt"));
Page<GroupJpaEntity> pageResult = groupJpaRepository.findAllByMemberUserIdAndCategory(userId, category, pageable);
return mapPageResult(pageResult);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ public static GroupCategory toCategoryFilter(String category) {
return null;
}

if ("GLOBAL".equalsIgnoreCase(category)) {
throw new InvalidCategoryException("유효하지 않은 카테고리입니다: 'GLOBAL'");
}

try {
return GroupCategory.valueOf(category.toUpperCase(Locale.ROOT));
} catch (IllegalArgumentException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.process.clash.application.group.exception.exception.conflict;

import com.process.clash.application.common.exception.exception.ConflictException;
import com.process.clash.application.group.exception.statuscode.GroupStatusCode;

public class CannotQuitGlobalGroupException extends ConflictException {
public CannotQuitGlobalGroupException() {
super(GroupStatusCode.CANNOT_QUIT_GLOBAL_GROUP);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ public enum GroupStatusCode implements StatusCode {
"DUPLICATE_GROUP_NAME",
"이미 사용 중인 그룹 이름입니다.",
HttpStatus.CONFLICT
),

CANNOT_QUIT_GLOBAL_GROUP(
"CANNOT_QUIT_GLOBAL_GROUP",
"전체 유저 그룹에서는 탈퇴할 수 없습니다.",
HttpStatus.CONFLICT
);

private final String code;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

import com.process.clash.application.common.actor.Actor;
import com.process.clash.application.common.exception.exception.ValidationException;
import com.process.clash.application.group.exception.exception.conflict.CannotQuitGlobalGroupException;
import com.process.clash.application.group.exception.exception.conflict.GroupDuplicateNameException;
import com.process.clash.application.group.exception.exception.conflict.GroupMemberLimitReachedException;
import com.process.clash.application.group.exception.exception.conflict.GroupOwnerCannotQuitException;
import com.process.clash.application.group.exception.exception.forbidden.GroupAccessDeniedException;
import com.process.clash.domain.group.entity.Group;
import com.process.clash.domain.group.enums.GroupCategory;
import org.springframework.stereotype.Component;

@Component
Expand Down Expand Up @@ -43,8 +45,23 @@ public void validateMemberLimit(int maxMembers, int memberCount) {
}

public void canQuitGroup(Actor actor, Group group) {
if (group.category() == GroupCategory.GLOBAL) {
throw new CannotQuitGlobalGroupException();
}
if (group.ownerId().equals(actor.id())) {
throw new GroupOwnerCannotQuitException();
}
}

public void validateNotGlobal(Group group) {
if (group.category() == GroupCategory.GLOBAL) {
throw new GroupAccessDeniedException();
}
}
Comment on lines +56 to +60

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Refactoring validateNotGlobal to accept a GroupCategory instead of a Group object makes the policy more versatile. This allows you to validate both the current state of a group and the intended state during an update (e.g., preventing a group from being changed to the GLOBAL category).

Suggested change
public void validateNotGlobal(Group group) {
if (group.category() == GroupCategory.GLOBAL) {
throw new GroupAccessDeniedException();
}
}
public void validateNotGlobal(GroupCategory category) {
if (category == GroupCategory.GLOBAL) {
throw new GroupAccessDeniedException();
}
}


public void validateNotGlobal(GroupCategory category) {
if (category == GroupCategory.GLOBAL) {
throw new GroupAccessDeniedException();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ public interface GroupRepositoryPort {

Optional<Group> findById(Long groupId);

Optional<Group> findByCategory(GroupCategory category);

void deleteById(Long groupId);

PageResult findAllByPage(Integer page, Integer size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public void execute(DeleteGroupData.Command command) {
Group group = groupRepositoryPort.findById(command.groupId())
.orElseThrow(GroupNotFoundException::new);

groupPolicy.validateNotGlobal(group);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the call to validateNotGlobal to pass the group's category, matching the suggested policy refactoring.

Suggested change
groupPolicy.validateNotGlobal(group);
groupPolicy.validateNotGlobal(group.category());

groupPolicy.validateOwnership(command.actor(), group);
List<Long> memberUserIds = groupRepositoryPort.findMemberUserIdsByGroupIds(List.of(group.id()));
groupRepositoryPort.deleteById(group.id());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public void execute(UpdateGroupData.Command command) {
.orElseThrow(GroupNotFoundException::new);

int currentMemberCount = groupRepositoryPort.countMembers(command.groupId());
policy.validateNotGlobal(group);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current validation only prevents modifying an existing GLOBAL group. It is critical to also prevent regular groups from being updated to the GLOBAL category, as this would create unauthorized global groups with owners, potentially breaking system assumptions and security boundaries.

Suggested change
policy.validateNotGlobal(group);
policy.validateNotGlobal(group.category());
policy.validateNotGlobal(command.category());

policy.validateNotGlobal(command.category());
policy.validateOwnership(command.actor(), group);
policy.validateMaxMembers(command.maxMembers());
policy.validateMemberLimit(group.maxMembers(), currentMemberCount);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.process.clash.application.user.user.service;

import com.process.clash.application.group.port.out.GroupRepositoryPort;
import com.process.clash.application.mail.port.out.VerificationCodePort;
import com.process.clash.application.user.user.data.VerifyEmailData;
import com.process.clash.application.user.user.exception.exception.badrequest.VerificationCodeExpiredOrWrongEmailException;
Expand All @@ -10,6 +11,7 @@
import com.process.clash.application.user.user.port.in.VerifyEmailUseCase;
import com.process.clash.application.user.user.port.out.PendingUserCachePort;
import com.process.clash.application.user.user.port.out.UserRepositoryPort;
import com.process.clash.domain.group.enums.GroupCategory;
import com.process.clash.domain.user.user.entity.User;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
Expand All @@ -22,6 +24,7 @@ public class VerifyEmailService implements VerifyEmailUseCase {
private final VerificationCodePort verificationCodePort;
private final UserRepositoryPort userRepositoryPort;
private final PendingUserCachePort pendingUserCachePort;
private final GroupRepositoryPort groupRepositoryPort;

@Override
@Transactional
Expand All @@ -46,7 +49,11 @@ public void execute(VerifyEmailData.Command command) {
}

// 인증 완료 처리 후 DB 저장
userRepositoryPort.save(pendingUser.active());
User savedUser = userRepositoryPort.save(pendingUser.active());

// 전체 유저 그룹 자동 가입
groupRepositoryPort.findByCategory(GroupCategory.GLOBAL)
.ifPresent(g -> groupRepositoryPort.addMember(g.id(), savedUser.id()));

// Redis 정리
pendingUserCachePort.delete(command.token());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ public enum GroupCategory {
CLUB,
TEAM,
NARSHA,
ETC
ETC,
GLOBAL
}
12 changes: 12 additions & 0 deletions src/main/resources/db/migration/V42__add_global_user_group.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- 1. groups 테이블의 fk_user_id 컬럼을 nullable로 변경
ALTER TABLE groups ALTER COLUMN fk_user_id DROP NOT NULL;

-- 2. 전체 유저 그룹 생성 (owner 없음)
INSERT INTO groups (created_at, updated_at, name, description, max_members, password, password_required, category, fk_user_id)
VALUES (NOW(), NOW(), '전체 유저', '모든 클래시 유저가 자동으로 참여하는 그룹입니다.', 2147483647, '', false, 'GLOBAL', NULL);

-- 3. 기존 활성 유저 전원을 GLOBAL 그룹에 추가
INSERT INTO group_members (fk_group_id, fk_user_id)
SELECT (SELECT id FROM groups WHERE category = 'GLOBAL'), u.id
FROM users u
WHERE u.user_status = 'ACTIVE';
Loading