Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
@@ -0,0 +1,58 @@
package com.example.solidconnection.admin.location.region.controller;

import com.example.solidconnection.admin.location.region.dto.AdminRegionCreateRequest;
import com.example.solidconnection.admin.location.region.dto.AdminRegionResponse;
import com.example.solidconnection.admin.location.region.dto.AdminRegionUpdateRequest;
import com.example.solidconnection.admin.location.region.service.AdminRegionService;
import jakarta.validation.Valid;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RequiredArgsConstructor
@RequestMapping("/admin/regions")
@RestController
public class AdminRegionController {

private final AdminRegionService adminRegionService;

@GetMapping
public ResponseEntity<List<AdminRegionResponse>> getAllRegions() {
List<AdminRegionResponse> responses = adminRegionService.getAllRegions();
return ResponseEntity.ok(responses);
}

@PostMapping
public ResponseEntity<AdminRegionResponse> createRegion(
@Valid @RequestBody AdminRegionCreateRequest request
) {
AdminRegionResponse response = adminRegionService.createRegion(request);
return ResponseEntity.status(HttpStatus.CREATED).body(response);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

응답 코드를 지정하는 방향으로 작성해주었네요

여담으로 예전에 RESTful API 설계 원칙에 맞게 응답 코드를 지정하자고 얘기가 나왔던 것으로 기억하는데, 여유 생기면 한 번 진행해봐야겠네요

}

@PutMapping("/{code}")
public ResponseEntity<AdminRegionResponse> updateRegion(
@PathVariable String code,
@Valid @RequestBody AdminRegionUpdateRequest request
) {
AdminRegionResponse response = adminRegionService.updateRegion(code, request);
return ResponseEntity.ok(response);
}

@DeleteMapping("/{code}")
public ResponseEntity<Void> deleteRegion(
@PathVariable String code
) {
adminRegionService.deleteRegion(code);
return ResponseEntity.noContent().build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.example.solidconnection.admin.location.region.dto;

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Size;

public record AdminRegionCreateRequest(
@NotBlank(message = "지역 코드는 필수입니다")
@Size(min = 1, max = 10, message = "지역 코드는 1자 이상 10자 이하여야 합니다")
String code,

@NotBlank(message = "한글 지역명은 필수입니다")
@Size(min = 1, max = 100, message = "한글 지역명은 1자 이상 100자 이하여야 합니다")
String koreanName
) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.example.solidconnection.admin.location.region.dto;

import com.example.solidconnection.location.region.domain.Region;

public record AdminRegionResponse(
String code
) {

public static AdminRegionResponse from(Region region) {
return new AdminRegionResponse(
region.getCode()
);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package com.example.solidconnection.admin.location.region.service;

import com.example.solidconnection.admin.location.region.dto.AdminRegionCreateRequest;
import com.example.solidconnection.admin.location.region.dto.AdminRegionResponse;
import com.example.solidconnection.admin.location.region.dto.AdminRegionUpdateRequest;
import com.example.solidconnection.common.exception.CustomException;
import com.example.solidconnection.common.exception.ErrorCode;
import com.example.solidconnection.location.region.domain.Region;
import com.example.solidconnection.location.region.repository.RegionRepository;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class AdminRegionService {

private final RegionRepository regionRepository;

@Transactional(readOnly = true)
public List<AdminRegionResponse> getAllRegions() {
return regionRepository.findAll()
.stream()
.map(AdminRegionResponse::from)
.toList();
}

@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);
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

저희는 orElseThrow 로 예외 던졌는데 이 부분도 조금 다르네요
예외 처리를 따로 private 메서드로 분리하지 않는 것도 그렇구요 ..!


Region region = new Region(request.code(), request.koreanName());
Region savedRegion = regionRepository.save(region);

return AdminRegionResponse.from(savedRegion);
}
Comment on lines +29 to +52

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

중복 체크 로직에 경쟁 조건(race condition) 위험이 있습니다.

현재 구조의 문제점:

  1. 31-34줄: code 중복 체크
  2. 36-39줄: koreanName 중복 체크
  3. 41-42줄: Region 생성 및 저장

위 체크와 저장 사이에 다른 트랜잭션이 동일한 code나 koreanName을 가진 Region을 생성할 수 있습니다. 이는 동시성 환경에서 중복 데이터가 저장될 수 있는 위험을 초래합니다.

권장 해결 방안:

  1. Region 엔티티의 code와 koreanName 필드에 데이터베이스 레벨 unique 제약 조건 추가
  2. 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.


@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);
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

업데이트 시 중복 이름 체크도 경쟁 조건에 노출되어 있습니다.

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.


Region updatedRegion = new Region(region.getCode(), request.koreanName());
Region savedRegion = regionRepository.save(updatedRegion);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

return AdminRegionResponse.from(savedRegion);
}

@Transactional
public void deleteRegion(String code) {
Region region = regionRepository.findById(code)
.orElseThrow(() -> new CustomException(ErrorCode.REGION_NOT_FOUND));

regionRepository.delete(region);
}
Comment thread
Gyuhyeok99 marked this conversation as resolved.
}