Skip to content

refactor/KD-29 Lab, Club, Professor, About의 도메인과 JPA 엔티티 분리 #267

Merged
dkdltm221 merged 9 commits intodevelopfrom
refactor/KD-29
Sep 28, 2025
Merged

refactor/KD-29 Lab, Club, Professor, About의 도메인과 JPA 엔티티 분리 #267
dkdltm221 merged 9 commits intodevelopfrom
refactor/KD-29

Conversation

@dkdltm221
Copy link
Copy Markdown
Contributor

Summary

Lab, Club, Professor, About 도메인을 JPA 엔티티와 분리하였습니다.
이에 맞추어 테스트 코드를 수정하였고, 엔티티-도메인간 매핑 메서드는 엔티티에 위치시켰습니다.

Tasks

  • Lab, Club, Professor, About 도메인을 JPA 엔티티로 분리하엿습니다.
  • 변경사항에 맞추어 테스트 코스 수정 및 엔티티-도메인간 매핑 메서드는 엔티티에 위치시켰습니다.
  • 기존 도메인 기반 테스트 저장소 로직을 JPA 엔티티 기반으로 변경했습니다.

@dkdltm221 dkdltm221 self-assigned this Sep 16, 2025
@dkdltm221 dkdltm221 added the 🔨refactor refactoring code label Sep 16, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 16, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

도메인 모델(About/Club/Lab/Professor)을 JPA 비의존 POJO로 전환하고, 대응하는 *JpaEntity 클래스를 추가했습니다. Spring Data JPA 리포지토리 제네릭을 *JpaEntity로 교체하고, 구현체에서 도메인↔엔티티 매핑을 도입했습니다. 커맨드 서비스는 업데이트 후 save 호출을 추가했고, 테스트/페이크 리포지토리도 매핑 방식으로 정비되었습니다.

Changes

Cohort / File(s) Change summary
Domain POJO 전환
.../domain/about/domain/About.java, .../domain/club/domain/Club.java, .../domain/lab/domain/Lab.java, .../domain/professor/domain/Professor.java
JPA 애노테이션/상속 제거, 도메인 객체로 단순화; About에 updateContent 추가, Professor에 soft-delete용 deletedAt/markDeleted 추가
JPA 엔티티 신설
.../domain/about/infrastructure/AboutJpaEntity.java, .../domain/club/infrastructure/ClubJpaEntity.java, .../domain/lab/infrastructure/LabJpaEntity.java, .../domain/professor/infrastructure/ProfessorJpaEntity.java
테이블 매핑 추가, ID/컬럼 제약 정의, toDomain()/fromDomain() 브리지 메서드 제공
JpaRepository 제네릭 교체
.../about/infrastructure/JpaAboutRepository.java, .../club/infrastructure/JpaClubRepository.java, .../lab/infrastructure/JpaLabRepository.java, .../professor/infrastructure/JpaProfessorRepository.java
리포지토리 대상 타입을 도메인 → *JpaEntity로 변경; 메서드 반환타입도 *JpaEntity로 조정
리포지토리 구현 매핑 도입
.../about/infrastructure/AboutRepositoryImpl.java, .../club/infrastructure/ClubRepositoryImpl.java, .../lab/infrastructure/LabRepositoryImpl.java, .../professor/infrastructure/ProfessorRepositoryImpl.java, .../professor/infrastructure/QueryProfessorRepository.java
save/find 계열에서 도메인↔엔티티 변환 추가; LabRepositoryImpl에 deleteById 위임 추가; QueryDSL 루트 Q타입을 *JpaEntity로 교체 후 결과를 도메인으로 매핑
커맨드 서비스 저장 추가
.../about/application/command/AboutCommandService.java, .../club/application/command/ClubCommandService.java, .../lab/application/command/LabCommandService.java, .../professor/application/command/ProfessorCommandService.java
업데이트 후 명시적 repository.save 호출 추가; Club의 fileId 업데이트 호출 제거; Professor 삭제를 markDeleted()+save로 변경
테스트용 페이크 리포지토리 정비
.../testFixtures/java/mock/repository/FakeAboutRepository.java, .../mock/repository/FakeClubRepository.java, .../mock/repository/FakeLabRepository.java, .../mock/repository/FakeProfessorRepository.java
내부 저장소를 도메인 → *JpaEntity로 변경, 입출력 시 도메인 매핑; FakeClubRepository에 deleteById 추가
테스트 수정
.../testFixtures/java/club/application/ClubCommandServiceTest.java, .../testFixtures/java/lab/application/LabCommandServiceTest.java
신규 저장 로직에 맞춰 픽스처/설정 변경(리포지토리에서 조회해 갱신 검증 등)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • LeeHanEum
  • JangYeongHu

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed 제목은 PR의 핵심 변경을 정확히 요약합니다: Lab, Club, Professor, About 도메인의 JPA 엔티티 분리를 명시하고 있어 변경 내용과 직접적으로 일치합니다. 브랜치/이슈 식별자(refactor/KD-29)를 포함해 연관 이슈 추적에 도움이 되며, 과도하게 모호하거나 무의미한 용어를 사용하지 않아 스캔 시 의도를 파악하기 쉽습니다.
Description Check ✅ Passed PR 설명은 도메인과 JPA 엔티티 분리, 테스트 수정, 엔티티-도메인 매핑 위치 변경 등 변경셋과 직접 관련된 내용을 구체적으로 기술하고 있어 변경 목적과 범위가 명확합니다.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 16, 2025

Test Coverage Report

Overall Project 95.08% 🍏
Files changed 100% 🍏

Module Coverage
aics-domain 92.6% 🍏
Files
Module File Coverage
aics-domain About.java 100% 🍏
AboutCommandService.java 100% 🍏
ClubCommandService.java 100% 🍏
Lab.java 100% 🍏
LabCommandService.java 100% 🍏
Professor.java 100% 🍏
ProfessorCommandService.java 100% 🍏

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #267      +/-   ##
=============================================
+ Coverage      93.29%   93.38%   +0.09%     
- Complexity       163      164       +1     
=============================================
  Files             49       49              
  Lines            477      484       +7     
  Branches           4        4              
=============================================
+ Hits             445      452       +7     
  Misses            26       26              
  Partials           6        6              
Files with missing lines Coverage Δ Complexity Δ
...about/application/command/AboutCommandService.java 100.00% <100.00%> (ø) 2.00 <0.00> (ø)
...java/kgu/developers/domain/about/domain/About.java 100.00% <ø> (ø) 2.00 <0.00> (ø)
...n/club/application/command/ClubCommandService.java 100.00% <100.00%> (ø) 3.00 <0.00> (ø)
...ain/lab/application/command/LabCommandService.java 100.00% <100.00%> (ø) 3.00 <0.00> (ø)
...ain/java/kgu/developers/domain/lab/domain/Lab.java 100.00% <ø> (ø) 6.00 <0.00> (ø)
...r/application/command/ProfessorCommandService.java 100.00% <100.00%> (ø) 3.00 <1.00> (ø)
.../developers/domain/professor/domain/Professor.java 100.00% <100.00%> (ø) 8.00 <1.00> (+1.00)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d08be0b...905833f. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
aics-domain/src/main/java/kgu/developers/domain/about/application/command/AboutCommandService.java (1)

11-16: 쓰기 트랜잭션(@transactional) 누락

두 메서드 모두 쓰기 작업이며, 다중 DB 작업(조회 후 저장)을 하나의 원자적 단위로 보장해야 합니다. 서비스 레벨에 @transactional이 없어 경쟁 상황/부분 실패 시 일관성이 깨질 수 있습니다.

적용 예시:

+import org.springframework.transaction.annotation.Transactional;
 public class AboutCommandService {
   private final AboutRepository aboutRepository;

+  @Transactional
   public Long createAbout(Category category, String content) {
     About about = About.create(category, content);
     About savedAbout = aboutRepository.save(about);
     return savedAbout.getId();
   }
+  @Transactional
   public void updateAbout(Category category, String content) {
     About about = aboutRepository.findByCategory(category)
       .orElseThrow(AboutNotFoundException::new);
     about.updateContent(content);
     aboutRepository.save(about);
   }
 }
aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorRepositoryImpl.java (1)

36-39: 소프트 삭제 정책과 충돌: 하드 삭제 유지 중

도메인/서비스에서 soft delete를 도입했지만, 구현체의 deleteById가 여전히 물리 삭제를 수행합니다. 데이터 무결성/감사 추적 관점에서 정책 불일치입니다. soft delete로 동작하도록 변경을 권장합니다.

아래처럼 도메인 경로로 soft delete를 수행하는 방향을 제안합니다:

 @Override
 public void deleteById(Long id) {
-  jpaProfessorRepository.deleteById(id);
+  this.findById(id).ifPresent(p -> {
+    p.markDeleted();
+    this.save(p);
+  });
 }
🧹 Nitpick comments (41)
aics-domain/src/main/java/kgu/developers/domain/club/domain/Club.java (2)

10-12: 도메인 분리 이후 불변성/동등성 전략이 필요합니다

엔티티에서 도메인 POJO로 전환되면서 JPA 제약/동등성 규칙이 사라졌습니다. 도메인 객체의 동일성(예: id 기반)과 동등성(예: 비즈니스 키 기반)을 명확히 해 주세요. Lombok의 @EqualsAndHashCode(of = "id") 또는 명시적 구현을 고려해 주세요. id가 null인 신규 객체의 비교 규칙도 정의가 필요합니다.


10-12: JPA @column 제약 제거에 따른 도메인 레벨 검증 보강 제안

이제 null/blank 값이 도메인 레벨에서 그대로 생성/수정될 수 있습니다. create/update 계열에서 필수 필드(name 등)와 형식(site URL 등)을 검증하는 편이 실패를 DB 계층까지 끌고 가지 않아 안전합니다.

다음과 같은 형태를 권장합니다(예시):

public static Club create(String name, String description, String site, Long fileId) {
    requireNonBlank(name, "name");
    // URL 형식 검증 필요시 별도 Validator 사용
    return Club.builder().name(name).description(description).site(site).fileId(fileId).build();
}

public void updateName(String name) {
    requireNonBlank(name, "name");
    this.name = name;
}

private static void requireNonBlank(String v, String field) {
    if (v == null || v.isBlank()) throw new IllegalArgumentException(field + " must not be blank");
}
aics-domain/src/testFixtures/java/club/application/ClubCommandServiceTest.java (2)

51-59: ID 1L에 대한 강결합으로 테스트가 취약합니다

시드 데이터의 시작 ID에 의존하면 페이크 리포지토리 구현 변경에 취약합니다. init()에서 저장한 엔티티의 ID를 필드로 보관하거나, findAll()/stream().findFirst()로 가져오도록 변경해 주세요.


60-64: 변경 내역이 저장소에 반영됐는지 재조회로 검증하세요

현재는 로컬 객체의 필드만 검증합니다. save 호출까지 포함된 시나리오이므로 저장소에서 재조회하여 일관성까지 확인하는 편이 좋습니다.

다음 추가를 권장합니다:

   // then
-  assertEquals(newName, club.getName());
-  assertEquals(newDescription, club.getDescription());
-  assertEquals(newSite, club.getSite());
-  assertNull(club.getFileId());
+  assertEquals(newName, club.getName());
+  assertEquals(newDescription, club.getDescription());
+  assertEquals(newSite, club.getSite());
+  assertNull(club.getFileId());
+  // 저장소 상태 검증
+  Club persisted = fakeClubRepository.findById(club.getId()).orElseThrow();
+  assertEquals(newName, persisted.getName());
+  assertEquals(newDescription, persisted.getDescription());
+  assertEquals(newSite, persisted.getSite());
+  assertNull(persisted.getFileId());
aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubRepositoryImpl.java (2)

28-32: 스트림 수집 간결화 제안(JDK 16+)

JDK 16+라면 Collectors 없이 toList() 사용이 간결합니다. 하위 버전이면 현 구현 유지.

- return entities.stream()
-         .map(ClubJpaEntity::toDomain)
-         .collect(Collectors.toList());
+ return entities.stream()
+         .map(ClubJpaEntity::toDomain)
+         .toList();

5-5: 불필요 import 정리(선택)

JDK 16+로 toList()를 사용한다면 Collectors import(Line 5)는 제거 가능합니다.

aics-domain/src/testFixtures/java/mock/repository/FakeClubRepository.java (6)

14-14: 동시성: synchronizedList + stream/removeIf 조합은 안전하지 않습니다.

synchronizedList는 개별 연산만 동기화합니다. 현재 stream() 반복과 removeIf/add가 분리되어 있어 병렬 테스트 시 가시성/원자성 문제가 날 수 있습니다. 테스트 픽스처라 하더라도 안전하게 가는 편이 좋습니다.

다음 중 하나를 권장합니다(선호순):

- private final List<ClubJpaEntity> data = Collections.synchronizedList(new ArrayList<>());
+ private final List<ClubJpaEntity> data = new java.util.concurrent.CopyOnWriteArrayList<>();

또는 각 읽기/삭제 구간을 synchronized (data) { ... }로 감싸세요(아래 메서드별 제안 참고).


37-39: findAll의 외부 동기화 필요

synchronizedList 위에서 stream()을 사용할 때는 외부 동기화가 필요합니다.

- return data.stream()
-         .map(ClubJpaEntity::toDomain)
-         .toList();
+ synchronized (data) {
+     return data.stream()
+             .map(ClubJpaEntity::toDomain)
+             .toList();
+ }

44-48: findById의 외부 동기화 및 변수명 가독성

동일하게 외부 동기화가 필요하고, 람다 파라미터명을 entity로 바꾸면 가독성이 좋아집니다.

- return data.stream()
-     .filter(club -> club.getId().equals(id))
-     .findFirst()
-         .map(ClubJpaEntity::toDomain);
+ synchronized (data) {
+     return data.stream()
+         .filter(entity -> entity.getId().equals(id))
+         .findFirst()
+         .map(ClubJpaEntity::toDomain);
+ }

51-53: deleteById도 일관되게 동기화 필요

삭제 또한 반복자를 사용하는 내부 구현일 수 있어 외부 동기화가 안전합니다.

- data.removeIf(club -> club.getId().equals(id));
+ synchronized (data) {
+     data.removeIf(entity -> entity.getId().equals(id));
+ }

19-26: 엔티티 변환 일관성: fromDomain 사용 고려

다른 모듈과 패턴을 맞추려면 직접 빌더 구성 대신 ClubJpaEntity.fromDomain(club)을 사용해 변환 일관성을 유지하는 편이 좋습니다. 이후 id가 없다면 시퀀스로 보강하세요.

예시:

ClubJpaEntity newEntity = ClubJpaEntity.fromDomain(club);
if (newEntity.getId() == null) {
    newEntity = ClubJpaEntity.builder()
        .id(sequence.getAndIncrement())
        .name(newEntity.getName())
        .description(newEntity.getDescription())
        .site(newEntity.getSite())
        .fileId(newEntity.getFileId())
        .build();
}

27-32: DB 제약(Unique name) 미시뮬레이션 누락

엔티티에 name unique 제약이 있으나(FK 제약과 동일 급은 아니더라도) 페이크 저장소에서 이를 검증하지 않으면 테스트와 실제 동작이 어긋날 수 있습니다. 저장 전 중복 검사 추가를 권장합니다.

- if (club.getId() != null) {
+ // simulate unique constraint on name
+ if (data.stream().anyMatch(e -> e.getName().equals(club.getName())
+         && (club.getId() == null || !e.getId().equals(club.getId())))) {
+     throw new IllegalStateException("Duplicate club name: " + club.getName());
+ }
+ if (club.getId() != null) {
     data.removeIf(entity -> entity.getId().equals(club.getId()));
- }
+ }

추가로 import java.util.Objects;가 필요하다면 함께 반영하세요.

aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubJpaEntity.java (1)

31-33: site 길이 50은 URL에 비해 보수적

일반 URL이 50자를 초과하는 경우가 많습니다. 255로 상향 권장합니다.

- @Column(length = 50)
+ @Column(length = 255)
 private String site;
aics-domain/src/main/java/kgu/developers/domain/about/application/command/AboutCommandService.java (1)

22-27: 동시성 하에서 갱신 손실 가능성 점검

현재 읽기(findByCategory) → 쓰기(save)의 비관리 도메인-매핑 구조로, 동시 수정 시 마지막 저장이 덮어쓸 수 있습니다. 엔티티에 @Version을 추가한 낙관적 락을 권장합니다. 관련 변경은 AboutJpaEntity에 제안했습니다.

aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutRepositoryImpl.java (1)

27-29: Optional 중간 변수 제거로 간결화 가능

가독성 차원의 사소한 제안입니다. 중간 변수를 제거해도 의미 손실 없이 간결해집니다.

-  Optional<AboutJpaEntity> optionalEntity = jpaAboutRepository.findByCategory(category);
-  return optionalEntity.map(AboutJpaEntity::toDomain);
+  return jpaAboutRepository.findByCategory(category).map(AboutJpaEntity::toDomain);
-  Optional<AboutJpaEntity> optionalEntity = jpaAboutRepository.findById(id);
-  return optionalEntity.map(AboutJpaEntity::toDomain);
+  return jpaAboutRepository.findById(id).map(AboutJpaEntity::toDomain);

Also applies to: 33-35

aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutJpaEntity.java (2)

31-33: DB 이식성 고려: columnDefinition("text") 대신 @lob 권장

DB 벤더별 DDL 차이를 줄이려면 String + @lob 사용이 안전합니다. MySQL/PostgreSQL 모두에서 CLOB/TEXT로 매핑됩니다.

-    @Column(nullable = false, columnDefinition = "text")
-    private String content;
+    @Lob
+    @Column(nullable = false)
+    private String content;

22-50: 낙관적 락(@Version) 추가로 갱신 손실 방지

서비스 레이어가 비관리 도메인 모델을 다시 저장하는 구조라 동시 업데이트 시 마지막 저장이 덮어쓸 수 있습니다. @Version 추가를 권장합니다.

 public class AboutJpaEntity extends BaseTimeEntity {
   @Id
   @GeneratedValue(strategy = IDENTITY)
   private Long id;
@@
   private String content;
 
+  @Version
+  private Long version;
aics-domain/src/testFixtures/java/mock/repository/FakeAboutRepository.java (1)

35-39: 동시 접근 시 List.stream 사용 주의 (테스트 한정이지만 언급)

Collections.synchronizedList라도 stream 연산은 개별 동기화가 필요합니다. 병렬 테스트 가능성이 없다면 무시 가능하나, 필요 시 synchronized 블록으로 감싸는 것을 고려하세요.

Also applies to: 43-47

aics-domain/src/main/java/kgu/developers/domain/about/domain/About.java (4)

12-14: id/category는 불변으로 두는 편이 안전합니다

도메인 관점에서 식별자와 분류는 생성 후 변경되지 않는 값이므로 final 권장합니다. 실수로 변경되는 리스크를 줄이고 동시성/동등성 판단도 안정적입니다.

-    private Long id;
-    private Category category;
+    private final Long id;
+    private final Category category;
     private String content;

16-25: 도메인 불변식(Null/Blank) 검증 추가 제안

현재 create/update 모두 유효성 검증이 없어 null/blank가 쉽게 스며듭니다. 최소한의 방어 코드를 추가해 주세요.

   public static About create(Category category, String content) {
-    return About.builder()
-      .category(category)
-      .content(content)
-      .build();
+    if (category == null) {
+      throw new IllegalArgumentException("category must not be null");
+    }
+    validateContent(content);
+    return About.builder()
+      .category(category)
+      .content(content)
+      .build();
   }
 
   public void updateContent(String content) {
-    this.content = content;
+    validateContent(content);
+    if (this.content != null && this.content.equals(content)) {
+      return;
+    }
+    this.content = content;
   }
+
+  private static void validateContent(String content) {
+    if (content == null || content.isBlank()) {
+      throw new IllegalArgumentException("content must not be blank");
+    }
+  }

10-26: 동등성(equals/hashCode) 정책 확인 요청

현재 equals/hashCode가 없어 콜렉션(Set/Map) 사용 시 참조 동등성에 의존합니다. id 기반 동등성이 필요하다면 명시 구현을 고려해 주세요(특히 id=null 상태 객체 처리 정책 결정 필요).

필요 시 구현 방향: id가 존재하면 id로 비교, 양쪽 id가 없으면 참조 동등성 유지.


7-9: Builder/AllArgsConstructor 공개 범위 재검토

public 빌더/전필드 생성자는 id를 임의로 세팅한 도메인 인스턴스 생성을 허용합니다(저장 시 업데이트로 오인될 수 있음). 팀 규칙에 따라 사용 범위를 제한하거나(예: Javadoc 경고) 테스트/매핑 외 사용 금지를 권장합니다.

만약 제한이 가능하다면, 매핑 전용 팩토리(예: About.of(id, category, content))를 두고 일반 생성 경로는 create(...)만 노출하는 방식을 제안드립니다.

aics-domain/src/main/java/kgu/developers/domain/lab/domain/Lab.java (2)

10-17: 도메인 분리 OK, 식별자 기반 동등성 고려 제안

도메인 객체로 분리된 현재 설계는 적절합니다. 다만 Set/Map 키 사용 및 중복 제거 시 예측 가능성을 위해 id 기반 equals/hashCode 구현을 고려해 주세요.

해당 도메인이 컬렉션 키로 쓰이는지 테스트/사용처를 한 번 점검 부탁드립니다.


19-27: create()에 최소 유효성 검사 추가 제안

name이 null/blank인 Lab 생성을 방지하면 후단에서 NPE/검증 분기를 줄일 수 있습니다.

다음 패치를 적용해 주세요:

 public static Lab create(String name, String loc, String site, String advisor, Long fileId) {
-    return Lab.builder()
+    if (name == null || name.isBlank()) {
+        throw new IllegalArgumentException("name must not be blank");
+    }
+    return Lab.builder()
         .name(name)
         .loc(loc)
         .site(site)
         .advisor(advisor)
         .fileId(fileId)
         .build();
 }
aics-domain/src/main/java/kgu/developers/domain/lab/application/command/LabCommandService.java (2)

1-17: 쓰기 트랜잭션 경계 추가 권장

서비스 계층에 @transactional을 부여해 원자성/롤백 보장을 명시적으로 가져가세요(특히 다중 리포지토리 호출 시 확장 용이).

다음 패치를 제안합니다:

 import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;

 @Service
 @RequiredArgsConstructor
+@Transactional(readOnly = true)
 public class LabCommandService {
   private final LabRepository labRepository;

-  public Long createLab(Long fileId, String name, String location, String site, String advisor) {
+  @Transactional
+  public Long createLab(Long fileId, String name, String location, String site, String advisor) {
     Lab lab = Lab.create(name, location, site, advisor, fileId);
     return labRepository.save(lab).getId();
   }

-  public void updateLab(Lab lab, String name, String loc, String site, String advisor, Long fileId) {
+  @Transactional
+  public void updateLab(Lab lab, String name, String loc, String site, String advisor, Long fileId) {
     lab.updateName(name);
     lab.updateLoc(loc);
     lab.updateSite(site);
     lab.updateAdvisor(advisor);
     lab.updateFileId(fileId);
     labRepository.save(lab);
   }

-  public void deleteLabById(Long id) {
+  @Transactional
+  public void deleteLabById(Long id) {
     labRepository.deleteById(id);
   }
 }

Also applies to: 19-31


19-26: 파라미터 명칭 일관화(naming) 권장

create는 location, update는 loc를 사용합니다. 통일하면 가독성이 좋아집니다.

다음과 같이 수정 가능:

- public void updateLab(Lab lab, String name, String loc, String site, String advisor, Long fileId) {
+ public void updateLab(Lab lab, String name, String location, String site, String advisor, Long fileId) {
-   lab.updateLoc(loc);
+   lab.updateLoc(location);
aics-domain/src/testFixtures/java/lab/application/LabCommandServiceTest.java (1)

22-24: 하드코딩된 ID에 의존하는 테스트는 취약합니다

사전 저장 데이터로 인해 ID=1,2라는 가정이 깨질 수 있습니다. 저장 결과의 ID를 캡처해 사용하도록 변경해 주세요.

다음 패치를 제안합니다:

 public class LabCommandServiceTest {
   private LabCommandService labCommandService;
   private FakeLabRepository fakeLabRepository;

-  private static final Long TARGET_LAB_ID = 2L;
   private static final Long TEST_FILE_ID = 1L;
+  private Long savedLabId;

@@
   private void initializeLabCommandService() {
     FakeFileRepository fakeFileRepository = new FakeFileRepository();
     fakeLabRepository = new FakeLabRepository();
     labCommandService = new LabCommandService(fakeLabRepository);
     fakeFileRepository.save(FileEntity.builder().id(1L).build());
-    fakeLabRepository.save(saveTestLab());
+    Lab saved = fakeLabRepository.save(saveTestLab());
+    savedLabId = saved.getId();
   }
@@
   @DisplayName("createLab은 Lab을 생성할 수 있다")
   public void createLab_Success() {
@@
-    // then
-    assertEquals(TARGET_LAB_ID, createdLabId);
+    // then: ID 자체가 발급되었는지만 검증
+    assertNotNull(createdLabId);
+    assertTrue(createdLabId > 0);
   }
@@
-    Lab lab = fakeLabRepository.findById(1L).orElseThrow();
+    Lab lab = fakeLabRepository.findById(savedLabId).orElseThrow();

또한 다음 정적 임포트를 추가하세요:

 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;

Also applies to: 30-36, 52-57, 63-63

aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabRepositoryImpl.java (1)

33-36: Stream 수집 간결화 제안 — Java 17로 toList() 사용 가능

build.gradle의 sourceCompatibility/targetCompatibility가 JavaVersion.VERSION_17로 설정되어 있어 JDK 16+의 Stream.toList() 사용이 안전합니다. 대상: aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabRepositoryImpl.java (Lines 33–36)

-    return entities.stream()
-        .map(LabJpaEntity::toDomain)
-        .collect(Collectors.toList());
+    return entities.stream()
+        .map(LabJpaEntity::toDomain)
+        .toList();
aics-domain/src/testFixtures/java/mock/repository/FakeLabRepository.java (4)

15-15: 테스트 더블에 불필요한 동기화 사용(일관성 깨짐).

Fake 저장소는 동시성 고려 대상이 아니며, 기존 합의대로 동기화를 넣지 않습니다. 또한 synchronizedList는 스트림 순회에 대한 외부 동기화가 없어 오히려 오해를 유발합니다. 학습 메모 참고.

-    private final List<LabJpaEntity> data = Collections.synchronizedList(new ArrayList<>());
+    private final List<LabJpaEntity> data = new ArrayList<>();

29-30: 명시적 ID 저장 이후 자동 증가기(autoGeneratedId) 갱신 누락.

외부에서 ID가 지정된 저장 이후에도 카운터가 갱신되지 않아, 이후 null ID 저장 시 기존 ID와 충돌 가능성이 있습니다.

-        Long id = lab.getId() == null ? autoGeneratedId.incrementAndGet() : lab.getId();
+        Long id = lab.getId() == null ? autoGeneratedId.incrementAndGet() : lab.getId();
+        if (lab.getId() != null) {
+            autoGeneratedId.accumulateAndGet(lab.getId(), Math::max);
+        }

39-41: ID 비교 시 NPE 방지.

안전하게 비교하도록 Objects.equals 사용을 권장합니다. deleteById에도 동일 적용을 고려해 주세요.

-        data.removeIf(e -> e.getId().equals(id));
+        data.removeIf(e -> java.util.Objects.equals(e.getId(), id));

57-60: 불필요한 도메인 객체 생성 최소화(정렬 → 매핑 순서).

정렬을 엔티티 단계에서 먼저 수행하면 도메인 객체 생성이 한 번만 일어납니다.

-        return data.stream()
-                .map(LabJpaEntity::toDomain)
-            .sorted(Comparator.comparing(Lab::getName))
-            .toList();
+        return data.stream()
+            .sorted(Comparator.comparing(LabJpaEntity::getName))
+            .map(LabJpaEntity::toDomain)
+            .toList();
aics-domain/src/main/java/kgu/developers/domain/professor/domain/Professor.java (1)

59-59: 시간 처리의 테스트 가능성과 일관성 개선 제안

LocalDateTime.now() 대신 Clock 주입(또는 Instant)을 사용하면 테스트 용이성과 시간대 일관성이 좋아집니다.

예시:

public void markDeleted(Clock clock) { this.deletedAt = LocalDateTime.now(clock); }
aics-domain/src/main/java/kgu/developers/domain/professor/application/command/ProfessorCommandService.java (2)

28-29: 쓰기 메서드에 트랜잭션 명시 권장

업데이트 후 저장까지 하나의 원자적 작업으로 보장하려면 @Transactional을 메서드(또는 클래스)에 부여하세요.

예시:

import org.springframework.transaction.annotation.Transactional;

@Transactional
public void updateProfessor(Professor professor, String name, Role role, String contact,
                            String email, String img, String officeLoc) {
  // ...
}

32-34: 소프트 삭제 경로 사용은 적절함 — 하드 삭제 진입점 차단 필요

서비스 레이어에서 soft delete(save 기반)로 일관화된 점은 좋습니다. 다만 구현체의 deleteById 하드 삭제가 남아 있어 우회 경로가 됩니다. 컨트롤러/애플리케이션 레벨에서 해당 메서드 사용을 금지하거나 구현체를 soft delete로 정렬해 주세요.

aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/QueryProfessorRepository.java (1)

33-35: JDK 요구사항 확인(toList())

.toList()는 JDK 16+. 프로젝트 타깃이 더 낮다면 Collectors.toList()로 교체하세요.

대안:

import java.util.stream.Collectors;
// ...
.fetch()
.stream()
.map(ProfessorJpaEntity::toDomain)
.collect(Collectors.toList());
aics-domain/src/testFixtures/java/mock/repository/FakeProfessorRepository.java (3)

16-16: Fake 리포지토리에서 동기화 리스트 사용은 불필요하며, 스트림과 혼용 시 위험

테스트 픽스처에서는 동시성이 요구되지 않고(기존 FakeLabRepository 학습도 동일 결론), Collections.synchronizedList + stream() 조합은 외부 동기화 없으면 CME 위험이 있습니다. 일반 ArrayList로 유지하세요.

적용 diff:

- private final List<ProfessorJpaEntity> data = Collections.synchronizedList(new ArrayList<>());
+ private final List<ProfessorJpaEntity> data = new ArrayList<>();

관련 학습: Fake 저장소에는 동기화를 추가하지 않음.


21-35: DB의 유니크 제약(연락처, 이메일)을 Fake에서도 가볍게 에뮬레이트 제안

실DB는 contact, email에 유니크 제약이 있으나 Fake는 무제한 삽입이 가능합니다. 테스트 일관성 향상을 위해 저장 시 중복을 사전 차단하세요.

적용 예시(중복 검사 추가):

 		Long id = (professor.getId() == null) ? sequence.getAndIncrement() : professor.getId();

 		ProfessorJpaEntity newEntity = ProfessorJpaEntity.fromDomain(
 				Professor.builder()
 						.id(id)
 						.name(professor.getName())
 						.role(professor.getRole())
 						.contact(professor.getContact())
 						.email(professor.getEmail())
 						.img(professor.getImg())
 						.officeLoc(professor.getOfficeLoc())
 						.deletedAt(professor.getDeletedAt())
 						.build()
 		);
+
+		// 유니크 제약 에뮬레이션
+		boolean dupContact = data.stream().anyMatch(e ->
+			java.util.Objects.equals(e.getContact(), newEntity.getContact()) &&
+			!java.util.Objects.equals(e.getId(), id));
+		if (dupContact) {
+			throw new IllegalStateException("Duplicate contact: " + newEntity.getContact());
+		}
+		boolean dupEmail = data.stream().anyMatch(e ->
+			java.util.Objects.equals(e.getEmail(), newEntity.getEmail()) &&
+			!java.util.Objects.equals(e.getId(), id));
+		if (dupEmail) {
+			throw new IllegalStateException("Duplicate email: " + newEntity.getEmail());
+		}

추가로 파일 상단에 import java.util.Objects;를 넣어 정리할 수 있습니다:

import java.util.Objects;

36-36: equals NPE 가능성 제거

id가 null일 가능성은 낮지만, 방어적으로 Objects.equals 사용이 안전합니다(가독성도 향상).

적용 diff:

-		data.removeIf(entity -> entity.getId().equals(id)); // ✅ 중복 방지
+		data.removeIf(entity -> java.util.Objects.equals(entity.getId(), id)); // ✅ 중복 방지
-			.filter(entity -> entity.getId().equals(id))
+			.filter(entity -> java.util.Objects.equals(entity.getId(), id))
-		data.removeIf(professor -> professor.getId().equals(id));
+		data.removeIf(professor -> java.util.Objects.equals(professor.getId(), id));

필요 시 상단에 import java.util.Objects; 추가 권장.

Also applies to: 45-47, 61-61

aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorJpaEntity.java (2)

27-44: 컬럼 길이 제약 재확인 요청(특히 name=10, officeLoc=20)

실데이터 기준으로 충분한지 확인 필요합니다. URL일 가능성이 있는 img는 255 이상이 흔하며, 이름/사무실 위치도 더 길 수 있습니다. 스키마가 이미 존재한다면 일치 여부도 점검 부탁드립니다.

길이 상향을 고려한다면 예:

-    @Column(nullable = false, length = 10)
+    @Column(nullable = false, length = 20)
     private String name;
...
-    @Column(nullable = false)
+    @Column(nullable = false, length = 255)
     private String img;
...
-    @Column(nullable = false, length = 20)
+    @Column(nullable = false, length = 50)
     private String officeLoc;

47-59: deletedAt 직접 대입 접근성 확인

entity.deletedAt = ...는 BaseTimeEntity의 가시성이 protected여야만 컴파일됩니다. private라면 setter가 필요합니다. 현 상태가 컴파일/런타임 모두 OK인지 확인 바랍니다.

필요 시 수정 예:

-        entity.deletedAt = professor.getDeletedAt();
+        entity.setDeletedAt(professor.getDeletedAt());
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f39385 and b6e6430.

📒 Files selected for processing (27)
  • aics-domain/src/main/java/kgu/developers/domain/about/application/command/AboutCommandService.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/about/domain/About.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutJpaEntity.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutRepositoryImpl.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/JpaAboutRepository.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/club/application/command/ClubCommandService.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/club/domain/Club.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubJpaEntity.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubRepositoryImpl.java (2 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/JpaClubRepository.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/lab/application/command/LabCommandService.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/lab/domain/Lab.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/JpaLabRepository.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabJpaEntity.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabRepositoryImpl.java (2 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/professor/application/command/ProfessorCommandService.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/professor/domain/Professor.java (2 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/JpaProfessorRepository.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorJpaEntity.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorRepositoryImpl.java (1 hunks)
  • aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/QueryProfessorRepository.java (2 hunks)
  • aics-domain/src/testFixtures/java/club/application/ClubCommandServiceTest.java (1 hunks)
  • aics-domain/src/testFixtures/java/lab/application/LabCommandServiceTest.java (1 hunks)
  • aics-domain/src/testFixtures/java/mock/repository/FakeAboutRepository.java (1 hunks)
  • aics-domain/src/testFixtures/java/mock/repository/FakeClubRepository.java (1 hunks)
  • aics-domain/src/testFixtures/java/mock/repository/FakeLabRepository.java (2 hunks)
  • aics-domain/src/testFixtures/java/mock/repository/FakeProfessorRepository.java (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-11-13T08:30:13.201Z
Learnt from: minjo-on
PR: kgu-developers/aics-server#94
File: aics-domain/src/main/java/kgu/developers/domain/lab/domain/LabRepository.java:11-11
Timestamp: 2024-11-13T08:30:13.201Z
Learning: In `LabRepository.java`, Labs should be sorted by name, and priority-based sorting is not required.

Applied to files:

  • aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/JpaLabRepository.java
  • aics-domain/src/testFixtures/java/mock/repository/FakeLabRepository.java
  • aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabRepositoryImpl.java
  • aics-domain/src/main/java/kgu/developers/domain/lab/domain/Lab.java
📚 Learning: 2024-12-03T07:09:18.535Z
Learnt from: minjo-on
PR: kgu-developers/aics-server#125
File: aics-domain/src/testFixtures/java/mock/FakeCommentRepository.java:41-45
Timestamp: 2024-12-03T07:09:18.535Z
Learning: `aics-domain/src/testFixtures/java/mock/FakeCommentRepository.java`의 `findById` 메서드에서는 데이터가 확장될 가능성이 없고 성능 차이가 크지 않으므로 Map을 사용하지 않고 List를 사용하는 것이 바람직합니다.

Applied to files:

  • aics-domain/src/testFixtures/java/mock/repository/FakeProfessorRepository.java
  • aics-domain/src/testFixtures/java/mock/repository/FakeLabRepository.java
  • aics-domain/src/testFixtures/java/mock/repository/FakeClubRepository.java
  • aics-domain/src/testFixtures/java/mock/repository/FakeAboutRepository.java
📚 Learning: 2024-11-13T08:30:40.214Z
Learnt from: minjo-on
PR: kgu-developers/aics-server#94
File: aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorRepositoryImpl.java:0-0
Timestamp: 2024-11-13T08:30:40.214Z
Learning: ProfessorRepositoryImpl 클래스에서 'priority' 필드는 사용되지 않으며, 교수 목록은 'role'과 'name'으로 정렬됩니다. 이를 위해 'findAllOrderByRoleAndName' 메서드를 사용합니다.

Applied to files:

  • aics-domain/src/testFixtures/java/mock/repository/FakeProfessorRepository.java
  • aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/QueryProfessorRepository.java
📚 Learning: 2024-12-03T05:04:36.193Z
Learnt from: LeeShinHaeng
PR: kgu-developers/aics-server#124
File: aics-domain/src/testFixtures/java/mock/FakeLabRepository.java:28-37
Timestamp: 2024-12-03T05:04:36.193Z
Learning: `aics-domain/src/testFixtures/java/mock/FakeLabRepository.java`의 `FakeLabRepository` 클래스에서는 현재 동시성 처리가 필요하지 않으며, 다른 `FakeRepository`들과의 통일성을 위해 동기화를 추가하지 않습니다.

Applied to files:

  • aics-domain/src/testFixtures/java/mock/repository/FakeProfessorRepository.java
  • aics-domain/src/testFixtures/java/mock/repository/FakeLabRepository.java
📚 Learning: 2024-12-03T05:01:55.233Z
Learnt from: LeeShinHaeng
PR: kgu-developers/aics-server#124
File: aics-domain/src/testFixtures/java/mock/FakeLabRepository.java:13-57
Timestamp: 2024-12-03T05:01:55.233Z
Learning: `LabServiceTest.java`에서 Java의 `BeforeEach` 애너테이션을 사용하여 각 테스트 전에 `FakeLabRepository`를 초기화하므로, `FakeLabRepository.java`에 `clear()` 메서드를 추가할 필요가 없다.

Applied to files:

  • aics-domain/src/testFixtures/java/mock/repository/FakeProfessorRepository.java
  • aics-domain/src/testFixtures/java/lab/application/LabCommandServiceTest.java
  • aics-domain/src/testFixtures/java/mock/repository/FakeLabRepository.java
📚 Learning: 2024-12-03T04:40:00.172Z
Learnt from: LeeShinHaeng
PR: kgu-developers/aics-server#124
File: aics-api/src/testFixtures/java/lab/application/LabServiceTest.java:76-87
Timestamp: 2024-12-03T04:40:00.172Z
Learning: LabServiceTest.java에서 `getById()`와 같은 private 메소드를 테스트할 때, 이를 내부적으로 사용하는 `deleteLab()`과 같은 public 메소드를 통해 간접적으로 접근하고, 이 접근 방법을 설명하는 주석을 포함하는 것이 허용됩니다.

Applied to files:

  • aics-domain/src/testFixtures/java/lab/application/LabCommandServiceTest.java
🧬 Code graph analysis (6)
aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutJpaEntity.java (3)
aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubJpaEntity.java (1)
  • Entity (14-56)
aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabJpaEntity.java (1)
  • Entity (15-60)
aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorJpaEntity.java (1)
  • Entity (16-74)
aics-domain/src/main/java/kgu/developers/domain/professor/domain/Professor.java (2)
aics-domain/src/main/java/kgu/developers/domain/lab/domain/Lab.java (1)
  • Getter (7-48)
aics-domain/src/main/java/kgu/developers/domain/about/domain/About.java (1)
  • Getter (7-26)
aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorJpaEntity.java (1)
aics-domain/src/main/java/kgu/developers/domain/professor/domain/Professor.java (1)
  • Getter (9-61)
aics-domain/src/main/java/kgu/developers/domain/about/application/command/AboutCommandService.java (1)
aics-domain/src/main/java/kgu/developers/domain/about/exception/AboutNotFoundException.java (1)
  • AboutNotFoundException (7-11)
aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubJpaEntity.java (3)
aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutJpaEntity.java (1)
  • Entity (16-50)
aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabJpaEntity.java (1)
  • Entity (15-60)
aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorJpaEntity.java (1)
  • Entity (16-74)
aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabJpaEntity.java (3)
aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutJpaEntity.java (1)
  • Entity (16-50)
aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubJpaEntity.java (1)
  • Entity (14-56)
aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorJpaEntity.java (1)
  • Entity (16-74)
🔇 Additional comments (27)
aics-domain/src/main/java/kgu/developers/domain/club/application/command/ClubCommandService.java (1)

28-29: updateClub 후 save 추가는 타당합니다

도메인이 POJO가 되면서 변경 감지가 사라졌으므로 save 호출이 필요합니다. 다만, update 경로에서 club.getId()가 null이면 의도치 않은 insert가 될 수 있으니, Line 23 인자에 대해 id 존재 보장(또는 방어적 체크)을 고려해 주세요.

aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/JpaClubRepository.java (1)

5-5: Spring Data 제네릭 전환 LGTM

JpaRepository<ClubJpaEntity, Long>로의 전환 적절합니다. 컴포넌트 스캔 경로에 이 패키지가 포함되는지만 확인해 주세요.

aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubRepositoryImpl.java (2)

36-38: Optional 매핑 처리 깔끔합니다

optionalEntity.map(ClubJpaEntity::toDomain) 사용 적절합니다.


20-24: 맵핑 누락 없음 — fromDomain/toDomain가 fileId 포함 모든 필드 양방향 매핑 확인됨.
ClubJpaEntity.fromDomain/toDomain가 id, name, description, site, fileId를 모두 매핑합니다 (aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubJpaEntity.java).

aics-domain/src/main/java/kgu/developers/domain/club/infrastructure/ClubJpaEntity.java (3)

14-16: 예약어 테이블명 이스케이프 처리 적절

"club" 인용으로 이식성과 충돌 회피에 도움이 됩니다.


28-30: description 길이 정책 확인 필요

length = 100이 실제 요구사항(소개 문구 길이)과 일치하는지 확인해 주세요. 길이가 자주 초과된다면 text 또는 더 큰 길이(예: 500/1000)가 안전합니다.

가능한 대안:

- @Column(nullable = false, length = 100)
+ @Column(nullable = false, columnDefinition = "text")
 private String description;

37-45: 소프트 삭제 필드 매핑 여부 확인

BaseTimeEntitydeletedAt을 제공하고, 도메인 Club에도 동필드가 있다면 ProfessorJpaEntity처럼 매핑을 맞추는 것이 일관됩니다. 없다면 현 상태 유지.

예시(도메인에 deletedAt이 있는 경우):

public Club toDomain() {
    return Club.builder()
            .id(id)
            .name(name)
            .description(description)
            .site(site)
            .fileId(fileId)
            .deletedAt(getDeletedAt())
            .build();
}
public static ClubJpaEntity fromDomain(Club club) {
    ClubJpaEntity entity = ClubJpaEntity.builder()
            .id(club.getId())
            .name(club.getName())
            .description(club.getDescription())
            .site(club.getSite())
            .fileId(club.getFileId())
            .build();
    entity.deletedAt = club.getDeletedAt();
    return entity;
}

Also applies to: 46-54

aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/AboutRepositoryImpl.java (1)

19-22: 도메인↔엔티티 매핑 흐름 적절함

fromDomain → save → toDomain의 브리지 패턴이 일관되고 명확합니다. 현재 변경 범위에서 LGTM.

aics-domain/src/main/java/kgu/developers/domain/about/infrastructure/JpaAboutRepository.java (1)

9-10: JPA 엔티티 대상으로의 전환 적절

리포지토리 제네릭과 시그니처가 엔티티 기준으로 일관화되었습니다. 현재 변경 사항은 타 모듈과 정합적입니다.

aics-domain/src/main/java/kgu/developers/domain/about/domain/About.java (1)

10-10: JPA 비의존 도메인 전환 자체는 좋습니다

엔티티 의존을 제거하고 순수 도메인으로 분리한 방향 동의합니다. 리포지토리/매핑 계층에서만 영속성 모델을 다루도록 일관성을 유지해 주세요.

해당 클래스의 사용처에서 더 이상 JPA 애너테이션/기능에 기대지 않는지, 그리고 AboutJpaEntity.toDomain/fromDomain가 전 경로에서 사용되는지 한번 점검 부탁드립니다.

aics-domain/src/main/java/kgu/developers/domain/lab/application/command/LabCommandService.java (1)

25-25: update 후 save 추가는 적절합니다

도메인 변경 후 영속화까지 보장되어 일관성이 좋아졌습니다.

aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/JpaLabRepository.java (1)

7-9: 이름 기준 정렬 쿼리로의 전환 LGTM

도메인 학습 내용(이름 정렬 우선)에 부합합니다. 인프라 레이어가 도메인 타입에 의존하지 않는 점도 좋습니다.

aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabRepositoryImpl.java (2)

39-42: deleteById 위임 OK

도메인 리포지토리에 삭제 API가 추가된 흐름과 일관됩니다.


18-23: id 매핑 포함 확인 — 수정 불필요
LabJpaEntity.fromDomain에서 .id(lab.getId())와 toDomain에서 .id(id)로 id가 정상 매핑되어 있습니다 (aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabJpaEntity.java, 대체로 라인 22-24, 40-53).

aics-domain/src/main/java/kgu/developers/domain/lab/infrastructure/LabJpaEntity.java (3)

40-59: toDomain/fromDomain 매핑 대칭성 양호.

도메인-엔티티 간 필드 매핑이 대칭적으로 잘 구성되어 있습니다. 이 상태 유지 부탁드립니다. 추가로, Lab 도메인에 소프트 삭제 값이 있다면(예: deletedAt), Professor와 동일하게 동기화가 필요한지 확인해 주세요.


26-37: 컬럼 길이 정책 재확인 요청.

  • name(16), advisor(16), loc(10), site(50)은 비교적 짧습니다. URL(site)은 50자를 쉽게 초과할 수 있습니다. 스키마/요건에 맞는 길이인지 확인해 주세요. 필요 시 text 또는 더 긴 길이로 조정 검토 바랍니다.

15-21: 테이블 네이밍/인용 규칙 일관성 OK.

"lab" 인용 사용이 다른 엔티티들과 일관됩니다. 별도 이슈 없습니다.

aics-domain/src/main/java/kgu/developers/domain/professor/domain/Professor.java (1)

21-21: 소프트 삭제 필드 추가에 따른 조회 정책 확인 필요

deletedAt 도입에 맞춰 조회 계층(Repository/Querydsl)에서 기본적으로 deletedAt IS NULL이 적용되는지 확인해주세요. 적용되지 않으면 삭제된 교수가 목록/단건 조회에 노출됩니다.

aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorRepositoryImpl.java (2)

20-23: 도메인↔엔티티 매핑 저장 흐름 적절

fromDomain → save → toDomain 플로우 적절합니다. 저장 시 생성된 ID가 도메인으로 매핑되는지(엔티티→도메인 변환)만 한번 확인 부탁드립니다.


27-29: Optional 매핑 OK

findById에서 엔티티 Optional을 도메인 Optional로 매핑하는 방식 적절합니다. (엔티티 레벨에서 @Where 등으로 soft delete 필터링이 보장되는지도 함께 점검 부탁드립니다.)

aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/JpaProfessorRepository.java (1)

5-5: 리포지토리 제네릭 전환 LGTM

도메인 분리 방향에 맞는 제네릭 변경입니다.

aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/QueryProfessorRepository.java (1)

23-35: 소프트 삭제 미필터링 가능성: where 절 추가 권장

soft delete(deletedAt) 사용 시 삭제된 엔티티가 기본 조회로 노출될 수 있으니, QueryDSL 기본 조회에 deletedAt IS NULL 조건을 명시적으로 추가하세요. 엔티티에 @Where(clause = "deleted_at IS NULL")가 이미 있다면 적용 불필요합니다.

- return queryFactory.selectFrom(professorJpaEntity)
+ return queryFactory.selectFrom(professorJpaEntity)
+   .where(professorJpaEntity.deletedAt.isNull())
    .orderBy(
      new CaseBuilder()
        .when(professorJpaEntity.role.eq(Role.PROFESSOR)).then(1)
        .when(professorJpaEntity.role.eq(Role.ASSISTANT)).then(2)
        .otherwise(3)
        .asc(),
      professorJpaEntity.name.asc()
    )
    .fetch()
    .stream()
    .map(ProfessorJpaEntity::toDomain)
    .toList();
aics-domain/src/testFixtures/java/mock/repository/FakeProfessorRepository.java (2)

12-12: 도메인↔엔티티 매핑 도입 방향 LGTM

테스트 더블에서도 JPA 엔티티를 저장 표현으로 쓰고 입출구에서 매핑하는 전략이 이번 PR 방향과 일치합니다.


53-56: 정렬 기준 일치 확인

도메인 요구(역할→이름 순)와 일치하며, 기존 학습 노트(Professor 목록은 role, name 기준 정렬)와도 합치됩니다.

테스트에서 enum Role의 정렬(ordinal/선언순)이 의도한 우선순위와 일치하는지 한 번만 확인 부탁드립니다.

aics-domain/src/main/java/kgu/developers/domain/professor/infrastructure/ProfessorJpaEntity.java (3)

16-22: 엔티티 정의 전반 LGTM

테이블 명 이스케이프, 빌더/게터/보호 생성자, BaseTimeEntity 상속 등 표준 패턴을 잘 따르고 있습니다.


30-33: Enum 매핑 STRING 사용 적절

역할 변경 시에도 안전하며 DB 가독성 측면에서 바람직합니다.


60-72: 도메인 변환(toDomain) 매핑 정확

필드 전부 누락 없이 매핑되고 deletedAt도 반영됩니다.

Comment on lines 16 to 20
public Long createAbout(Category category, String content) {
About about = About.create(category, content);
return aboutRepository.save(about).getId();
About savedAbout = aboutRepository.save(about);
return savedAbout.getId();
}
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.

⚠️ Potential issue

카테고리 유니크 제약 사전 검증/예외 변환 필요

DB 레벨에서 category가 unique인데, 사전 검증 없이 생성 시 DataIntegrityViolationException으로 터질 수 있습니다. 도메인 친화적 예외로 변환하거나, 생성 전에 존재 여부를 체크하세요.

간단한 선행 체크 예시:

   public Long createAbout(Category category, String content) {
+    if (aboutRepository.findByCategory(category).isPresent()) {
+      // TODO: 도메인 전용 예외(예: AboutAlreadyExistsException)로 치환
+      throw new IllegalStateException("이미 존재하는 카테고리: " + category);
+    }
     About about = About.create(category, content);
     About savedAbout = aboutRepository.save(about);
     return savedAbout.getId();
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public Long createAbout(Category category, String content) {
About about = About.create(category, content);
return aboutRepository.save(about).getId();
About savedAbout = aboutRepository.save(about);
return savedAbout.getId();
}
public Long createAbout(Category category, String content) {
if (aboutRepository.findByCategory(category).isPresent()) {
// TODO: 도메인 전용 예외(예: AboutAlreadyExistsException)로 치환
throw new IllegalStateException("이미 존재하는 카테고리: " + category);
}
About about = About.create(category, content);
About savedAbout = aboutRepository.save(about);
return savedAbout.getId();
}
🤖 Prompt for AI Agents
aics-domain/src/main/java/kgu/developers/domain/about/application/command/AboutCommandService.java
lines 16-20: the createAbout method can trigger a
DataIntegrityViolationException when the category column is unique; perform a
pre-check (e.g., aboutRepository.existsByCategory(category)) and throw a
domain-friendly DuplicateCategoryException if true, or alternatively wrap the
save call in a try/catch catching DataIntegrityViolationException and rethrowing
a domain-specific exception; ensure the repository has an existsByCategory
method and the new DuplicateCategoryException is defined and used consistently.

Comment on lines +21 to 31
newEntity = AboutJpaEntity.builder()
.id(about.getId() == null ? sequence.getAndIncrement() : about.getId())
.content(about.getContent())
.category(about.getCategory())
.build();
if(about.getId() != null) {
data.removeIf(entity -> entity.getId().equals(about.getId()));
}
data.add(newEntity);
return newEntity.toDomain();
}
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.

⚠️ Potential issue

프로덕션과 불일치: 카테고리 유니크 제약 미반영

실DB는 category unique인데, Fake 저장소는 중복을 허용합니다. 테스트가 잘못된 성공을 낼 수 있어 위험합니다. 유니크 검사를 추가해 주세요. (과거 러닝에 따라 Map 대신 List 유지)

   @Override
   public About save(About about) {
-    AboutJpaEntity newEntity;
-    newEntity = AboutJpaEntity.builder()
-        .id(about.getId() == null ? sequence.getAndIncrement() : about.getId())
-        .content(about.getContent())
-        .category(about.getCategory())
-        .build();
+    AboutJpaEntity newEntity;
+    // 카테고리 유니크 제약 반영: 신규 생성(id == null)인데 동일 카테고리가 있으면 예외
+    if (about.getId() == null && data.stream().anyMatch(e -> e.getCategory().equals(about.getCategory()))) {
+      throw new IllegalStateException("Duplicate category: " + about.getCategory());
+    }
+    Long id = (about.getId() == null) ? sequence.getAndIncrement() : about.getId();
+    newEntity = AboutJpaEntity.builder()
+        .id(id)
+        .content(about.getContent())
+        .category(about.getCategory())
+        .build();
     if(about.getId() != null) {
       data.removeIf(entity -> entity.getId().equals(about.getId()));
     }
     data.add(newEntity);
     return newEntity.toDomain();
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
newEntity = AboutJpaEntity.builder()
.id(about.getId() == null ? sequence.getAndIncrement() : about.getId())
.content(about.getContent())
.category(about.getCategory())
.build();
if(about.getId() != null) {
data.removeIf(entity -> entity.getId().equals(about.getId()));
}
data.add(newEntity);
return newEntity.toDomain();
}
AboutJpaEntity newEntity;
// 카테고리 유니크 제약 반영: 신규 생성(id == null)인데 동일 카테고리가 있으면 예외
if (about.getId() == null && data.stream().anyMatch(e -> e.getCategory().equals(about.getCategory()))) {
throw new IllegalStateException("Duplicate category: " + about.getCategory());
}
Long id = (about.getId() == null) ? sequence.getAndIncrement() : about.getId();
newEntity = AboutJpaEntity.builder()
.id(id)
.content(about.getContent())
.category(about.getCategory())
.build();
if(about.getId() != null) {
data.removeIf(entity -> entity.getId().equals(about.getId()));
}
data.add(newEntity);
return newEntity.toDomain();
}

Copy link
Copy Markdown
Contributor

@JangYeongHu JangYeongHu left a comment

Choose a reason for hiding this comment

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

LGTM 👍👍
테스트 커버리지만 통과하게 코드 수정하면 깔끔할 것 같아요. 아마 fakeFileRepository에 넣을때 physicalPath만 넣으면 해결 되실 것 같네요

@dkdltm221 dkdltm221 merged commit c3f8bcc into develop Sep 28, 2025
3 checks passed
@dkdltm221 dkdltm221 deleted the refactor/KD-29 branch September 28, 2025 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨refactor refactoring code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants