Skip to content

[Telos] week 2 과제 완료 #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 37 commits into
base: telos
Choose a base branch
from

Conversation

rover0811
Copy link

시험기간과 겹쳐 시간이 촉박해 최대한 기능 위주로 먼저 구현하였습니다.

주요 변경 사항

JPA 마이그레이션

  • Member, Time, Theme, Reservation 등 모든 주요 엔티티에 대해 JdbcTemplate에서 JPA로 데이터 액세스 계층 마이그레이션
  • 엔티티 간 관계 설정 및 정규화
  • 스키마 자동 생성 및 업데이트 지원

대기 시스템 구현

  • 테마별 예약 대기열 기능 추가
  • 대기 순위 계산 로직 구현
  • 회원별 대기 현황 조회 기능

세부 개발 내용

  1. 엔티티 클래스 구현:

    • 모든 주요 도메인에 JPA 어노테이션 적용
    • 연관 관계 설정 (ManyToOne, OneToMany 등)
  2. 리포지토리 계층:

    • JpaRepository 인터페이스 구현
    • 복잡한 쿼리를 위한 JPQL 작성
    • 대기 순위 계산을 위한 커스텀 쿼리 메서드
  3. 서비스 로직:

    • 예약 및 대기 관리를 위한 트랜잭션 로직
    • 예약 가능 여부 검증
    • 대기열 처리 및 순위 관리
  4. 테스트:

    • JPA 엔티티 영속성 테스트
    • 예약 시스템 기능 검증

DWL21 and others added 13 commits April 11, 2025 13:03
Replaced MemberDao with MemberRepository to leverage Spring Data JPA, simplifying data access and improving maintainability. Updated MemberService and Member entity to support JPA, including annotations for entity mapping and adjustments to business logic.
Replaced `TimeDao` with `TimeRepository` for better integration with Spring Data JPA. Updated related service methods and `Time` entity to use JPA annotations for ORM functionality. This simplifies database interaction and aligns with best practices.
Replaced ThemeDao with ThemeRepository and updated ThemeController to reflect this change. Annotated the Theme class with JPA annotations to support ORM, including ID, column definitions, and a new "deleted" field. This prepares the codebase for better integration with JPA and database operations.
Replaced ReservationDao with JPA-based ReservationRepository, integrating JPA annotations in the Reservation entity. Added support for member-specific reservations and combined reservation and waiting list retrieval. Simplified service layer for better maintainability and expanded functionality.
Replaced JDBC dependency with Spring Data JPA and streamlined Logback dependency in build configuration. Introduced waiting system with new tests, schema updates, and foreign key relationships in database. Enabled SQL initialization and adjusted JPA properties for better database handling.
Introduced JpaTest class to verify JPA entity persistence and API response handling. The tests ensure correct storage of Time entities and validate reservation retrieval functionality using a mock authentication token.
Implemented `ReservationRepository`, `ThemeRepository`, and `TimeRepository` to interact with corresponding entities. Introduced `MyReservationResponse` to encapsulate reservation data for API responses. These changes lay the groundwork for managing reservations and related data.
This commit introduces the waiting feature, including entity, service, controller, and repository components. Users can join the waiting list for a specific theme and time, check their position, and cancel their waiting. It ensures no duplicate reservations or waitlist entries for the same slot.
Copy link
Collaborator

@kwonyj1022 kwonyj1022 left a comment

Choose a reason for hiding this comment

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

저번 리뷰 답변 및 반영 안해주신 게 있어서 중복 리뷰가 좀 있어요!

@@ -0,0 +1,20 @@
package com.yourssu.roomescape;
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 프로젝트 최상위 패키지에 AppConstants, ExceptionController, JwtTokenProvider, PageController, WebConfig가 존재하고 있어요. 이 각각을 프로젝트 최상위 패키지에 둔 이유가 궁금해요

Comment on lines 3 to 20
/**
* Application-wide constants
*/
public class AppConstants {
// Cookie names
public static final String TOKEN_COOKIE_NAME = "token";

// Role names
public static final String ROLE_ADMIN = "ADMIN";


// Other constants can be added here as needed

// Private constructor to prevent instantiation
private AppConstants() {
throw new AssertionError("Constants class should not be instantiated");
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기에 있는 주석들은 굳이 없어도 각각 class 이름, 상수 이름, 생성자 접근지정자 등으로 그 의미를 잘 알 수 있기 때문에 필요 없을 것 같습니다~

private AppConstants() {
throw new AssertionError("Constants class should not be instantiated");
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

image

파일 끝에 개행이 없어서 경고문 뜨네요. 인텔리제이 설정으로 자동화할 수 있어요.
블로그: No newline at a end of file, 파일의 끝에는 개행 추가❗️


import java.util.Optional;

public interface MemberRepository extends JpaRepository<Member, Long> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

DAO와 Repository의 차이는 무엇일까요?

Copy link
Author

Choose a reason for hiding this comment

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

주요한 차이점은 추상화 수준이라고 생각합니다. DAO는 데이터베이스의 쿼리를 직접 작성하는 반면, JPA는 무엇을 가져올지만 정하고 세부 구현은 의존합니다.

Comment on lines 1 to 3
package com.yourssu.roomescape.member;

public class TokenDto {
Copy link
Collaborator

Choose a reason for hiding this comment

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

TokenDto를 member 패키지에 둔 이유가 궁금해요. 이거 말고도 인증과 관련된 파일이 모두 member에 있는데 이렇게 설계한 이유가 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

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

기존에 그냥 member에 있길래 두었는데, auth 패키지로 옮기는게 더 좋을 것 같네요

Comment on lines +12 to +14
public void setDate(String date) {
this.date = date;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

다른 request dto에는 setter가 없는데 얘는 setter를 만들어주신 이유가 무엇인가요?
다른 request dto에는 setter가 없는데 어떻게 정상적으로 동작하는 걸까요?

Copy link
Author

Choose a reason for hiding this comment

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

테스트 코드 쪽을 대응하다가, 해당 부분을 수정했었는데 생각해보니 테스트 코드 방식을 수정하는게 더 좋았을 것 같다는 생각을 합니다.
리플렉션이라는 것을 통해 역직렬화를 한다고 합니다. 해당 내용 더 공부해보고 수정하겠습니다.

@@ -0,0 +1,31 @@
package com.yourssu.roomescape.waiting;

public class WaitingRequest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

dto에 record를 사용한 경우도 있고 안 한 경우도 있는데 그 기준이 무엇인가요? 통일하는게 좋아 보입니다~


@Transactional
public WaitingResponse addWaiting(String date, Long themeId, Long timeId, Member member) {
// 중복 예약 확인
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 주석으로 흐름을 구분하기 보다는 메서드를 분리해서 메서드 명으로 동작을 설명하면 좋을 것 같습니다!
ex. validateDuplicateReservation() 등등

#spring.jpa.defer-datasource-initialization=true
spring.jpa.show-sql=true
spring.jpa.properties.hibernate.format_sql=true
spring.jpa.hibernate.ddl-auto=none
Copy link
Collaborator

Choose a reason for hiding this comment

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

ddl-auto 설정으로는 무엇이 있나요? 왜 none으로 설정했는지 이유가 궁금해요!

Copy link
Author

Choose a reason for hiding this comment

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

옵션으로는 아래와 같이 있습니다.

  1. create: 애플리케이션 시작 시 테이블을 삭제하고 다시 생성합니다.
  2. create-drop: 애플리케이션 시작 시 테이블을 생성하고, 종료 시 삭제합니다.
  3. update: 엔티티와 테이블을 비교해 변경점만 수정합니다.
  4. validate: 엔티티와 테이블이 매핑되는지만 확인합니다.
  5. none: 아무 작업도 하지 않습니다.

spring.sql.init.mode=always와 함께 사용해서, 미리 정의된 SQL으로 테스트 데이터베이스를 초기화하고 이렇게 하면 테스트 통과가 되길래 이렇게 두었습니다.

spring.jpa.show-sql=true
spring.jpa.properties.hibernate.format_sql=true
spring.jpa.hibernate.ddl-auto=none
spring.sql.init.mode=always
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 설정은 어떤 설정일까요?

Copy link
Author

Choose a reason for hiding this comment

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

spring.sql.init.mode=always 설정은

  1. 애플리케이션 시작 시마다 schema.sql과 data.sql 스크립트를 실행합니다.
  2. 모든 데이터베이스 유형에 적용됩니다.
  3. Hibernate의 자동 스키마 생성 대신 SQL 스크립트로 데이터베이스를 초기화하는 방식입니다.

코드 구조를 개선하고 유지보수를 용이하게 하기 위해 공통 모듈 및 보안 관련 클래스를 적절한 패키지로 이동했습니다. 이에 따라 import 경로가 수정되었습니다.
AppConstants 클래스의 패키지를 common으로 이동하여 프로젝트 구조를 정리했습니다. 마지막 줄 개행을 주었습니다.
예약 생성 시 필수 필드(date, theme, time)에 대한 유효성 검증을 추가했습니다.
이를 위해 Spring Validation 라이브러리를 적용하고 관련 의존성을 build.gradle에 포함했습니다.
ThemeRepository, TimeRepository, ReservationRepository 필드를 제거하여 ReservationResponse 클래스의 불필요한 요소를 정리했습니다. 응답 모델로서의 역할에 맞지 않는 필드를 삭제하여 코드의 가독성과 유지 보수성을 개선했습니다.
기존 save 메서드를 삭제하고, LoginMember를 추가로 받는 새로운 save 메서드로 통합했습니다. 중복 코드 제거를 통해 유지보수성을 개선했습니다.
로그인된 사용자의 ID를 활용하도록 메서드를 변경하고 관련 컨트롤러에서 MemberRepository 의존성을 제거했습니다. 이를 통해 코드의 책임 분리를 강화하고 불필요한 종속성을 줄였습니다.
사용자 정의 예외 클래스(BadRequestException, ResourceNotFoundException 등)를 생성하고,
Global Exception Handler(ExceptionController)를 통해 통합 예외 처리 로직을 추가했습니다.
관련 서비스에 기존 RuntimeException 대신 사용자 정의 예외를 적용하여 가독성과 유지보수성을 향상시켰습니다.
Reservation 엔티티에서 name 필드를 삭제하고 기존 로직을 이에 맞게 수정했습니다. Member 엔티티의 name 필드를 활용하도록 변경하여 코드 중복을 줄였습니다.
컨트롤러에서 직접 Repository를 이용하던 코드를 Service 계층으로 추상화하여 비즈니스 로직과 데이터 액세스 로직을 분리했습니다. 이를 통해 코드의 응집도를 높이고 유지보수를 용이하게 만들었습니다.
Copy link
Collaborator

@kwonyj1022 kwonyj1022 left a comment

Choose a reason for hiding this comment

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

리뷰하다보니까 뒤쪽 리뷰는 반영 안 되거나 답변 안 달린게 있기도 하고, 지금 스프링부트 실행이 안 돼요. 제가 해결해서 실행해도 예약 대기 생성, 예약 목록 조회 등애서 오류가 발생하고 있어요. 스스로 오류를 찾아서 해결해보셨으면 합니다.

간단한 리뷰 2개, 현수님 답변에 대한 답변 1개 리뷰해놨으니 참고해주시고,
반영 안 된 리뷰 반영 + 답변 안 한 리뷰에 대한 답변 (답변이 필요한 리뷰에 한해서) + 실행 시 발생하는 오류 해결까지 해서 5월 1일 목요일 오후 2시까지 다시 리뷰요청해주세요~

it.getTheme().getName(),
it.getDate(),
it.getTime().getValue(),
"예약"
Copy link
Collaborator

Choose a reason for hiding this comment

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

"예약"이라는 문자열을 상수로 관리할 수도 있겠지만, enum을 사용해서 상태를 관리하면 더 명확할 것 같아요~

@@ -0,0 +1,51 @@
package com.yourssu.roomescape.common.security;

import io.jsonwebtoken.*;
Copy link
Collaborator

Choose a reason for hiding this comment

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

import에 와일드카드를 사용하는 것은 지양해야 한다고 생각합니다~

Comment on lines 4 to 5
import com.yourssu.roomescape.member.Member;
import com.yourssu.roomescape.member.MemberRepository;
Copy link
Collaborator

Choose a reason for hiding this comment

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

사용하지 않는 import문은 삭제해주세요~ 여기 말고도 전체적으로 사용하지 않는 import문이 많네요

rover0811 added 6 commits May 1, 2025 00:32
기존 LoginMember 클래스를 어노테이션으로 변경하고 관련 로직을 수정하여 코드의 일관성과 가독성을 향상시켰습니다. 또한 예약 및 대기 관련 로직에서 Member 객체를 직접 사용하도록 변경하여 중복 코드를 제거하고 효율성을 높였습니다.
불변 데이터 객체 클래스들을 Java Record로 변환하여 코드 가독성 및 간결성을 향상시켰습니다. 추가적인 getter 메서드 제거와 함께 코드 유지보수가 용이해졌습니다.
불필요한 주석을 제거하고 메소드 체인에서 getter 대신 record의 메소드 호출로 수정하여 코드 가독성을 개선했습니다. 코드 전반의 클린업을 통해 유지보수성과 읽기 용이성을 높였습니다.
예약 상태를 표현하기 위한 ReservationStatus enum을 추가했습니다. 기다리는 상태와 순위를 조합한 메시지를 반환하는 메서드도 포함되어 있습니다.
인증 관련 사용자 정보를 제공하는 UserInfo 인터페이스를 추가했습니다.
이 인터페이스는 사용자 ID, 이름, 이메일, 역할 정보를 반환하는 메서드를 포함합니다.
API 명세 제공을 위해 SwaggerConfig 클래스를 추가하고 springdoc-openapi 의존성을 build.gradle에 추가했습니다. 이를 통해 API 문서를 자동으로 생성할 수 있습니다.
@rover0811
Copy link
Author

ee75cba, ec08edc 에서 예약 오류 해결했는데, 원인은 LoginMemberArgumentResolver에서 형을 Member만을 받아서 생기는 문제였습니모든 컨트롤러에서 LoginMember loginMember 타입으로 사용하던 것을 @LoginMember Member member로 변경해서 해결했는데 보통 이렇게 하는걸까요?

@rover0811 rover0811 requested a review from kwonyj1022 May 1, 2025 01:58
Copy link
Collaborator

@kwonyj1022 kwonyj1022 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~
리뷰한 내용 반영 + 테스트코드 추가 해서 5월 4일 일요일 오후 2시까지 재리뷰요청 해주세요.

추가로, 지금 브랜치 병합 방향이 main 브랜치로 되어 있는데 telos 브랜치로 변경해주세요

String email = jwtTokenProvider.getPayload(token);
return memberService.findByEmail(email);
} catch (Exception e) {
return null; // TODO: 예외처리를 어떻게 해야하나
Copy link
Collaborator

Choose a reason for hiding this comment

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

저라면 여기서 커스텀 예외를 던지고 ExceptionController가 예외를 처리하도록 할 것 같아요


@Entity
@Table(name = "member") // 테이블 이름 명시적 지정
public class Member implements UserInfo {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 구조를 사용한 이유가 궁금해요

|| reservationRequest.getDate() == null
|| reservationRequest.getTheme() == null
|| reservationRequest.getTime() == null) {
public ResponseEntity create(@Validated @RequestBody ReservationRequest reservationRequest, @LoginMember Member loginMember) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

컨트롤러 레이어에서 Member라는 도메인 레이어의 객체를 알고 있네요. 도메인을 가지고 있어야 할 이유가 있을까요? 간단히 멤버 식별자 정도만 갖는 dto를 사용해도 괜찮을 것 같아요

private String name;

@Column(nullable = false, unique = true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

email에 unique 제약 조건을 걸었는데, 회원 가입 시점에 중복되는 이메일로 가입을 시도하면 어떻게 될까요? 아마 오류가 발생할 것 같은데 사용자가 오류의 원인을 알아볼 수 있을까요?


public interface MemberRepository extends JpaRepository<Member, Long> {
Optional<Member> findByEmail(String email);
Optional<Member> findByEmailAndPassword(String email, String password);
Copy link
Collaborator

Choose a reason for hiding this comment

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

사용하지 않는 메서드는 삭제해주세요

Comment on lines 41 to 43
if (alreadyReserved) {
throw new IllegalStateException("이미 예약된 시간입니다.");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 예외가 발생하는 상황은 사용자가 이미 예약한 시간에 또 예약을 시도하는 경우인데, "이미 예약된 시간입니다."라는 예외 미시지는 이미 누가 예약한 시간이라 예약을 못한다는 의미와 더 가까운 것 같아요. 이미 예약된 시간이라 대기를 건 건데 왜 이미 예약된 시간이라고하지? 하는 의문이 생길 것 같아요. "이미 예약한 시간입니다."가 좀 더 적절한 예외메시지일 것 같아요.

그리고, localhost:8080으로 접속해서 실제로 예약을 만들고 대기를 걸면 {"message":"서버 오류가 발생했습니다","details":"시스템 관리자에게 문의하세요"} 라고 응답합니다. 적절한 예외메시지를 뿌려주지 못하고 있어요. 밑에 "이미 대기 중인 시간입니다" 예외도 마찬가지입니다.

Comment on lines 36 to 37
@Transactional
public WaitingResponse addWaiting(String date, Long themeId, Long timeId, Member member) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 메서드는 밑에 있는 addWaitingByMemberId()가 호출하는 메서드이기 때문에 public일 이유가 없을 것 같아요. @TransactionaladdWaitingByMemberId()에 붙어 있으니 여기는 안 붙여도 됩니다.

@rover0811 rover0811 changed the base branch from main to telos May 3, 2025 06:05
rover0811 added 9 commits May 3, 2025 23:17
IllegalStateException에 대한 전용 예외 처리 핸들러를 추가하여 예외 상황을 더 명확하게 처리하도록 개선했습니다.
또한 대기열 중복 검사 로직을 수정하고 예약 관련 메시지를 사용자 친화적으로 변경하였습니다.
Member 클래스 대신 UserInfo 클래스를 사용하도록 Controller와 Service 코드를 리팩토링했습니다. 이를 통해 인증 정보 처리가 개선되고 관련 로직이 더 간결해졌습니다.
사용되지 않는 메서드(findByEmailAndPassword, findByMemberId)를 삭제하여 코드의 간결성과 유지보수성을 향상시켰습니다. 관련 메서드 호출은 다른 로직으로 대체되었거나 더 이상 필요하지 않습니다.
@query 어노테이션을 제거하고 Spring Data JPA의 메서드 네이밍 규칙을 활용하여 existsBy 메서드를 간소화하였습니다. 이를 통해 유지보수성을 높이고 코드 가독성을 개선하였습니다.
member를 직접 참조하던 getName 메서드를 name 필드 참조로 수정하였습니다.
member 필드와 name 필드 간의 충돌 및 일관성 문제를 방지하기 위함입니다.
예외 발생 시 null 반환 대신 UnauthorizedException을 던지도록 수정하였습니다. 이를 통해 잘못된 인증 정보에 대한 명확한 예외 처리가 이루어지도록 개선했습니다.
회원 가입 시 이미 사용 중인 이메일에 대해 BadRequestException을 발생시키도록 수정했습니다. 이를 통해 중복 이메일 등록의 문제를 방지합니다.
각 서비스(JwtTokenProvider, Member, Reservation, Theme, Time, Waiting)에 대한 테스트 케이스를 추가하여 주요 로직의 동작을 검증했습니다. 테스트를 통해 유효성 검증, 예외 처리, 핵심 기능이 의도한 대로 동작하는지 확인했습니다.
TimeRepository 인터페이스를 더 이상 사용하지 않아 코드베이스에서 제거했습니다. 불필요한 코드를 정리하여 유지보수성을 높였습니다.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants