feat: region 관련 관리 기능 추가 #561
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
지역을 생성하는 기능을 구현했습니다: - AdminRegionCreateRequest: 지역 생성 요청 DTO - AdminRegionResponse: 지역 응답 DTO - AdminRegionService.createRegion(): 중복 검사를 포함한 지역 생성 로직 - AdminRegionController.createRegion(): HTTP POST 엔드포인트 중복 검사: - 지역 코드 중복 확인 - 한글명 중복 확인 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
지역 관리 기능을 완성했습니다: 구현 기능: - AdminRegionUpdateRequest: 지역 수정 요청 DTO - AdminRegionService.updateRegion(): 한글명 중복 검사를 포함한 지역 수정 - AdminRegionService.deleteRegion(): 지역 삭제 - AdminRegionService.getAllRegions(): 전체 지역 조회 - AdminRegionController: 수정/삭제/조회 HTTP 엔드포인트 테스트 코드 (AdminRegionServiceTest): - CREATE: 정상 생성, 코드 중복, 한글명 중복 테스트 - UPDATE: 정상 수정, NOT_FOUND, 중복 한글명, 동일 한글명 테스트 - DELETE: 정상 삭제, NOT_FOUND 테스트 - READ: 빈 목록, 전체 조회 테스트 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
지역 관리 기능에 필요한 에러코드를 추가했습니다: - REGION_NOT_FOUND: 존재하지 않는 지역 - REGION_ALREADY_EXISTS: 이미 존재하는 지역 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Gyuhyeok99
requested review from
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
whqtker
as code owners
Walkthrough지역(Region) 관리 기능을 위한 완전한 관리자 모듈이 추가되었습니다. 다음과 같이 구성됩니다:
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20분 추가 주의 필요 영역:
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/admin/location/region/service/AdminRegionService.java (1)
21-27: 전체 조회 로직이 적절하게 구현되었습니다.읽기 전용 트랜잭션과 스트림 기반 DTO 매핑이 깔끔합니다. 향후 지역 데이터가 많아질 경우 페이지네이션 추가를 고려할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/example/solidconnection/admin/location/region/controller/AdminRegionController.java(1 hunks)src/main/java/com/example/solidconnection/admin/location/region/dto/AdminRegionCreateRequest.java(1 hunks)src/main/java/com/example/solidconnection/admin/location/region/dto/AdminRegionResponse.java(1 hunks)src/main/java/com/example/solidconnection/admin/location/region/dto/AdminRegionUpdateRequest.java(1 hunks)src/main/java/com/example/solidconnection/admin/location/region/service/AdminRegionService.java(1 hunks)src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/location/region/repository/RegionRepository.java(1 hunks)src/test/java/com/example/solidconnection/admin/location/region/service/AdminRegionServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/solidconnection/admin/location/region/service/AdminRegionService.java (1)
src/main/java/com/example/solidconnection/admin/location/region/controller/AdminRegionController.java (1)
RequiredArgsConstructor(21-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/java/com/example/solidconnection/admin/location/region/controller/AdminRegionController.java (2)
28-57: REST 엔드포인트 구현이 모범 사례를 잘 따르고 있습니다.각 엔드포인트의 특징:
- GET /admin/regions - 전체 목록 조회, 200 OK 반환
- POST /admin/regions - 생성 요청에 @Valid 검증 적용, 201 CREATED 반환
- PUT /admin/regions/{code} - 업데이트 요청에 @Valid 검증 적용, 200 OK 반환
- DELETE /admin/regions/{code} - 삭제 완료 시 204 NO CONTENT 반환
HTTP 메서드 선택, 상태 코드, 그리고 요청 검증이 모두 적절합니다.
21-27: 컨트롤러 구조와 보안 설정이 모두 올바르게 구현되어 있습니다.검증 결과를 정리하면 다음과 같습니다:
Spring Security 설정 확인
SecurityConfiguration.java의 66번 라인에서.requestMatchers("/admin/**").hasRole(ADMIN.name())설정이 적용됨경로 매핑 확인
- 컨트롤러의
@RequestMapping("/admin/regions")가/admin/**패턴에 포함되어 ADMIN 역할만 접근 가능결론
- 인증/인가 설정이 제대로 구현되어 있으므로 관리자 전용 접근이 보장됨
의존성 주입, 경로 매핑, 보안 설정 모두 Spring 관례를 따르고 있으며 추가 수정이 필요하지 않습니다.
Comment on lines
+5
to
+13
| public record AdminRegionResponse( | ||
| String code | ||
| ) { | ||
|
|
||
| public static AdminRegionResponse from(Region region) { | ||
| return new AdminRegionResponse( | ||
| region.getCode() | ||
| ); | ||
| } |
There was a problem hiding this comment.
응답 DTO에 한글 지역명이 포함되지 않았습니다.
- 문제 요약
프론트가 지역 이름을 확인할 수 없어서 관리 화면에서 새 기능을 제대로 보여 줄 수 없습니다. - 영향
getAllRegions, createRegion, updateRegion 응답이 모두 code만 반환해 관리자 UI에서 지역명을 즉시 표시할 수 없습니다. - 제안
AdminRegionResponse에 koreanName을 추가하고 from(Region)에서 region.getKoreanName()을 내려 주세요.
public record AdminRegionResponse(
- String code
+ String code,
+ String koreanName
) {
public static AdminRegionResponse from(Region region) {
return new AdminRegionResponse(
- region.getCode()
+ region.getCode(),
+ region.getKoreanName()
);
}
}🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/admin/location/region/dto/AdminRegionResponse.java
around lines 5 to 13, the response DTO currently exposes only the region code;
add a koreanName field to the record signature and populate it in the
from(Region) factory by calling region.getKoreanName(); update the returned
AdminRegionResponse instance to include region.getKoreanName() so getAllRegions,
createRegion and updateRegion responses include the Korean region name.
Contributor
Author
There was a problem hiding this comment.
아 이거 일부로 지웠는데 그러면 안되겠네
There was a problem hiding this comment.
@Gyuhyeok99, 네, 맞습니다! 관리 화면에서 지역 코드만으로는 어떤 지역인지 바로 알기 어려워서 koreanName이 함께 내려가는 게 좋을 것 같습니다. 조치해 주시면 감사하겠습니다! 😊
Comment on lines
+29
to
+45
| @Transactional | ||
| public AdminRegionResponse createRegion(AdminRegionCreateRequest request) { | ||
| regionRepository.findById(request.code()) | ||
| .ifPresent(region -> { | ||
| throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS); | ||
| }); | ||
|
|
||
| regionRepository.findByKoreanName(request.koreanName()) | ||
| .ifPresent(region -> { | ||
| throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS); | ||
| }); | ||
|
|
||
| Region region = new Region(request.code(), request.koreanName()); | ||
| Region savedRegion = regionRepository.save(region); | ||
|
|
||
| return AdminRegionResponse.from(savedRegion); | ||
| } |
There was a problem hiding this comment.
중복 체크 로직에 경쟁 조건(race condition) 위험이 있습니다.
현재 구조의 문제점:
- 31-34줄: code 중복 체크
- 36-39줄: koreanName 중복 체크
- 41-42줄: Region 생성 및 저장
위 체크와 저장 사이에 다른 트랜잭션이 동일한 code나 koreanName을 가진 Region을 생성할 수 있습니다. 이는 동시성 환경에서 중복 데이터가 저장될 수 있는 위험을 초래합니다.
권장 해결 방안:
- Region 엔티티의 code와 koreanName 필드에 데이터베이스 레벨 unique 제약 조건 추가
- DataIntegrityViolationException을 catch하여 REGION_ALREADY_EXISTS 에러로 변환
@Transactional
public AdminRegionResponse createRegion(AdminRegionCreateRequest request) {
- regionRepository.findById(request.code())
- .ifPresent(region -> {
- throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS);
- });
-
- regionRepository.findByKoreanName(request.koreanName())
- .ifPresent(region -> {
- throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS);
- });
-
- Region region = new Region(request.code(), request.koreanName());
- Region savedRegion = regionRepository.save(region);
-
- return AdminRegionResponse.from(savedRegion);
+ try {
+ Region region = new Region(request.code(), request.koreanName());
+ Region savedRegion = regionRepository.save(region);
+ return AdminRegionResponse.from(savedRegion);
+ } catch (DataIntegrityViolationException e) {
+ throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS);
+ }
}이 방식은 데이터베이스의 ACID 속성을 활용하여 원자적으로 중복을 방지합니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/admin/location/region/service/AdminRegionService.java
around lines 29 to 45, the current pre-checks for duplicate code and koreanName
create a race condition before saving; instead add database-level UNIQUE
constraints for Region.code and Region.koreanName (update the JPA entity
annotations or DB migration/DDL), remove or keep but do not rely on the
in-memory findBy checks, and wrap the save call in a try/catch that catches
DataIntegrityViolationException (or the specific Spring/JPA persistence
exception thrown on unique violation) and rethrow new
CustomException(ErrorCode.REGION_ALREADY_EXISTS) so concurrent inserts are
handled atomically by the DB; keep the method @Transactional as-is.
Comment on lines
+52
to
+57
| regionRepository.findByKoreanName(request.koreanName()) | ||
| .ifPresent(existingRegion -> { | ||
| if (!existingRegion.getCode().equals(code)) { | ||
| throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS); | ||
| } | ||
| }); |
There was a problem hiding this comment.
업데이트 시 중복 이름 체크도 경쟁 조건에 노출되어 있습니다.
createRegion과 동일한 문제로, 중복 체크와 저장 사이에 다른 트랜잭션이 동일한 koreanName을 사용할 수 있습니다. 데이터베이스 unique 제약 조건을 추가하고 예외 처리로 전환하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/admin/location/region/service/AdminRegionService.java
around lines 52-57, the current in-memory check for duplicate koreanName during
update is racy; add a unique constraint on the korean_name column at the
DB/schema level and remove reliance on the pre-check as the sole guard, then
modify the service to catch the persistence constraint exception (e.g.,
DataIntegrityViolationException or ConstraintViolationException) thrown on save
and translate it into a CustomException(ErrorCode.REGION_ALREADY_EXISTS); ensure
the update/save is still performed inside the transactional context but rely on
DB uniqueness + exception handling rather than the non-atomic find-then-check
pattern.
Comment on lines
+59
to
+60
| Region updatedRegion = new Region(region.getCode(), request.koreanName()); | ||
| Region savedRegion = regionRepository.save(updatedRegion); |
There was a problem hiding this comment.
엔티티 업데이트 방식이 잘못되었습니다. 기존 엔티티를 수정해야 합니다.
현재 코드는 새로운 Region 인스턴스를 생성하여 저장하고 있습니다. 이는 다음 문제를 일으킬 수 있습니다:
- JPA가 기존 엔티티를 업데이트하는 것이 아니라 새 엔티티로 대체하려 시도
- Region이 다른 필드나 연관관계를 가질 경우 해당 정보 손실
- 예상치 못한 JPA 동작 (detached entity 이슈 등)
기존 엔티티를 수정하도록 변경해주세요:
@Transactional
public AdminRegionResponse updateRegion(String code, AdminRegionUpdateRequest request) {
Region region = regionRepository.findById(code)
.orElseThrow(() -> new CustomException(ErrorCode.REGION_NOT_FOUND));
regionRepository.findByKoreanName(request.koreanName())
.ifPresent(existingRegion -> {
if (!existingRegion.getCode().equals(code)) {
throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS);
}
});
- Region updatedRegion = new Region(region.getCode(), request.koreanName());
- Region savedRegion = regionRepository.save(updatedRegion);
+ region.updateKoreanName(request.koreanName()); // Region에 update 메서드 추가 필요
+ // 또는 setter가 있다면: region.setKoreanName(request.koreanName());
+ // @Transactional이 있으므로 dirty checking으로 자동 저장됨
- return AdminRegionResponse.from(savedRegion);
+ return AdminRegionResponse.from(region);
}Region 엔티티에 update 메서드를 추가하는 것이 더 나은 설계입니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/admin/location/region/service/AdminRegionService.java
around lines 59-60, the code creates a new Region instance and saves it which
can replace or lose existing data; instead load the existing Region from the
repository, call a Region.update(...) method to mutate its fields (add this
update method to the Region entity if absent), then save the managed entity (or
rely on transactional commit). Ensure the service method runs inside a
@Transactional context, handle the not-found case (throw or return) before
updating, and preserve other fields/associations by mutating the retrieved
entity rather than constructing a new one.
Contributor
There was a problem hiding this comment.
@coderabbitai 영속성 컨텍스트의 추적을 받는 엔티티인데도 새로 생성을 해버리는 경우가 존재해?
There was a problem hiding this comment.
@sukangpunch 좋은 질문입니다! 영속성 컨텍스트에서 추적받는 엔티티를 새로 생성하는 경우는 극히 드물고, 일반적으로는 권장되지 않습니다. 하지만 몇 가지 특수한 상황이 있습니다:
1️⃣ 새 인스턴스를 생성하는 특수한 경우들:
-
불변(Immutable) 엔티티 패턴: 엔티티를 불변 객체로 설계한 경우, 변경 시 새 인스턴스를 만들어야 합니다
// 불변 엔티티 예시 @Entity @Immutable public class ImmutableRegion { ... }
-
Detached 엔티티 병합: 웹 계층에서 받은 detached 엔티티를 다시 영속화할 때
Region detachedRegion = ...; // 웹에서 받은 객체 entityManager.merge(detachedRegion);
-
엔티티 완전 교체: 연관관계나 모든 필드를 완전히 새로운 것으로 대체해야 할 때 (매우 드문 케이스)
2️⃣ 현재 코드의 문제:
현재 코드는 위의 어떤 경우에도 해당하지 않습니다:
Region region = regionRepository.findById(code) // ✅ 영속 상태
.orElseThrow(...);
...
Region updatedRegion = new Region(region.getCode(), request.koreanName()); // ❌ 새 인스턴스
Region savedRegion = regionRepository.save(updatedRegion); // ⚠️ 문제 발생 가능이렇게 하면:
region은 영속 상태인데 버려짐updatedRegion은 같은 ID를 가진 새로운 인스턴스- JPA가 이를 새 엔티티로 인식하거나, 병합을 시도하면서 예상치 못한 동작 발생 가능
- Region이 다른 필드나 연관관계를 가지면 그 정보가 손실됨
3️⃣ 올바른 방법:
Region region = regionRepository.findById(code)
.orElseThrow(...);
region.updateKoreanName(request.koreanName()); // 영속 엔티티 직접 수정
// @Transactional이므로 커밋 시점에 dirty checking으로 자동 UPDATE
return AdminRegionResponse.from(region);결론: 영속 상태의 엔티티는 직접 수정하고 dirty checking에 의존하는 것이 JPA의 표준 패턴입니다! 🎯
Comment on lines
+65
to
+71
| @Transactional | ||
| public void deleteRegion(String code) { | ||
| Region region = regionRepository.findById(code) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.REGION_NOT_FOUND)); | ||
|
|
||
| regionRepository.delete(region); | ||
| } |
There was a problem hiding this comment.
삭제 전 참조 무결성 확인이 필요합니다.
현재 deleteRegion은 해당 Region을 참조하는 다른 엔티티(예: University, Post 등)가 있는지 확인하지 않습니다. 참조가 존재하는 상태에서 삭제를 시도하면 외래 키 제약 조건 위반으로 인한 데이터베이스 예외가 발생합니다.
다음 중 하나의 방안을 선택하여 적용해주세요:
방안 1: 참조 존재 여부 확인 후 에러 처리 (권장)
@Transactional
public void deleteRegion(String code) {
Region region = regionRepository.findById(code)
.orElseThrow(() -> new CustomException(ErrorCode.REGION_NOT_FOUND));
+ // 해당 region을 참조하는 대학이나 게시물이 있는지 확인
+ if (universityRepository.existsByRegion(region) || postRepository.existsByRegion(region)) {
+ throw new CustomException(ErrorCode.REGION_IN_USE);
+ }
+
regionRepository.delete(region);
}방안 2: 데이터베이스 제약 조건 예외 처리
@Transactional
public void deleteRegion(String code) {
Region region = regionRepository.findById(code)
.orElseThrow(() -> new CustomException(ErrorCode.REGION_NOT_FOUND));
- regionRepository.delete(region);
+ try {
+ regionRepository.delete(region);
+ } catch (DataIntegrityViolationException e) {
+ throw new CustomException(ErrorCode.REGION_IN_USE);
+ }
}방안 1이 더 명확한 에러 메시지를 제공하므로 권장됩니다.
Committable suggestion skipped: line range outside the PR's diff.
whqtker
approved these changes
Nov 20, 2025
Member
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
컨벤션 관련한 부분만 코멘트 남겼습니다 ! 간단한 거만 손 보시면 머지해도 될 정도로 클로드가 잘 작성했네요 .. 특히 테스트 코드와 같이 단순한 작성은 클로드가 큰 도움이 될 거 같아요
| @Valid @RequestBody AdminRegionCreateRequest request | ||
| ) { | ||
| AdminRegionResponse response = adminRegionService.createRegion(request); | ||
| return ResponseEntity.status(HttpStatus.CREATED).body(response); |
Member
There was a problem hiding this comment.
응답 코드를 지정하는 방향으로 작성해주었네요
여담으로 예전에 RESTful API 설계 원칙에 맞게 응답 코드를 지정하자고 얘기가 나왔던 것으로 기억하는데, 여유 생기면 한 번 진행해봐야겠네요
| regionRepository.findByKoreanName(request.koreanName()) | ||
| .ifPresent(region -> { | ||
| throw new CustomException(ErrorCode.REGION_ALREADY_EXISTS); | ||
| }); |
Member
There was a problem hiding this comment.
저희는 orElseThrow 로 예외 던졌는데 이 부분도 조금 다르네요
예외 처리를 따로 private 메서드로 분리하지 않는 것도 그렇구요 ..!
| } | ||
|
|
||
| @Test | ||
| @DisplayName("저장된 모든 지역을 조회한다") |
Member
There was a problem hiding this comment.
테스트 메서드에 @DisplayName 어노테이션도 저희 암묵적으론 사용하지 않고 있고요
Contributor
Author
There was a problem hiding this comment.
아직 손볼 곳들이 좀 많군요! 다른분들 확인하신 후에 고치려고 좀 납두긴 했습니다 ㅎㅎ.. 수정해놓겠습니다~
sukangpunch
reviewed
Nov 23, 2025
| class 전체_지역_조회 { | ||
|
|
||
| @Test | ||
| void 지역이_없으면_빈_목록을_반환한다() { |
Contributor
There was a problem hiding this comment.
저희 서비스에서 테스트 코드에 '// given' 부분 내용이 존재하지 않다면 주석을 아예 빼버려도 되나요?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
region 관련 curd기능을 추가했습니다.
특이 사항
놀랍게도 이 pr은 앞 pr에서 말씀드렸듯이 기능, 테스트, 커밋메시지까지 claude code가 해줬습니다! 이렇게 간단한 기능이라 한 번 시켜봤습니다 ㅎㅎ..
물론 아직 커밋 관련해서도 좀 부족한 거 같고 손볼 곳이 많을 거 같은데 다른 분들도 어떤 느낌인지 알면 좋을 거 같아 한 번 맡겨봤습니다!
앞으로 테스트 코드 관련해서는 확실히 스트레스를 덜 받으실 거 같습니다.
리뷰 요구사항 (선택)