Skip to content

[8주차/주니] 워크북 제출합니다#46

Open
LeeJaeJun1 wants to merge 5 commits into
UMC-Inha:junny/mainfrom
LeeJaeJun1:main
Open

[8주차/주니] 워크북 제출합니다#46
LeeJaeJun1 wants to merge 5 commits into
UMC-Inha:junny/mainfrom
LeeJaeJun1:main

Conversation

@LeeJaeJun1
Copy link
Copy Markdown

✅ 실습 체크리스트

  • 이론 학습을 완료하셨나요?
  • 미션 요구사항을 모두 이해하셨나요?
  • 실습을 수행하기 위한 공부를 완료하셨나요?
  • 실습 요구사항을 모두 완료하셨나요?

✅ 컨벤션 체크리스트

  • 디렉토리 구조 컨벤션을 잘 지켰나요?
  • pr 제목을 컨벤션에 맞게 작성하였나요?
  • pr에 해당되는 이슈를 연결하였나요?(중요)
  • 적절한 라벨을 설정하였나요?
  • 파트장에게 code review를 요청하기 위해 reviewer를 등록하였나요?
  • 닉네임/main 브랜치의 최신 상태를 반영하고 있는지 확인했나요?(매우 중요!)

📌 주안점

Copy link
Copy Markdown
Collaborator

@YoungJJun YoungJJun left a comment

Choose a reason for hiding this comment

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

8주차 피드백

  1. 미션 요구사항에 대해 빠짐없이 잘 구현해주신 것 같습니다!

  2. MemberCommandService - joinMember()

    if(request.getTerms() != null && request.getTerms().size() > 0) {
    			List<MemberTerm> memberTermList = request.getTerms().stream()
    				.map(termDTO -> {
    					Term term = termRepository.findById(termDTO.getTermId())
    						.orElseThrow(()-> new MemberException(MemberErrorCode.TERM_NOT_FOUND));
    
    					return MemberTerm.builder()
    						.member(savedMember)
    						.term(term)
    						.isAgreed(termDTO.getIsAgree())
    						.build();
    				}).collect(Collectors.toList());
    
    			memberTermRepository.saveAll(memberTermList);
    		}

    (1) 우선 Term 존재하지 않는 경우 MemberException으로 설정해주셨는데.

    이 상황에서는 문제가 없어 보이지만 만약에 사용자가 회원가입 상황이 아니라 이 서비스의 약관이 뭐가있지? 해서 조회하는 상황이 있다면 MemberException이 적절할까? 라는 생각이 들었습니다.

    수정하게 된다면 Term을 별도의 도메인으로 보고 TermException, TermErrorCode 이런식으로 분리하면 될 것 같아요.

    (2) 필수 약관에 대해 동의여부 검증 로직도 추가되어야 할 것 같아요.

    (3) Member Entity의 email 컬럼에 unique 제약이 없어요. 추가로 회원가입 과정에서 이메일 중복 체크 로직도 없습니다. 이 경우 동일 이메일로 여러 계정 생성 가능할 것 같아요.

    DB에도 UNIQUE 설정 및 비즈니스 로직으로도 추가해서 중복 이메일 가입 막아줘야 할 것 같아요. DB 제약만 설정해도 방지가 가능하지만 비즈니스 로직에서 저희가 원하는 Exeption을 던져야 공통 에러로 응답할 수 있어요.

    DB 제약만 설정하고 로직으로 검증없이 진행했는데 이메일이 중복이면 DataIntegrityViolation 에러가 발생하고 이는 저희가 설정한 공통에러로 처리가 안됩니당.

    (4) JoinDTO 에서 List<TermDTO> terms 이게 null이나 empty가 가능한 구조입니다.

    사용자가 null이나 emtpy로 가입하려고 하면 위 검증하는 코드 블럭 자체가 스킵되어서 그냥 가입이 됩니다.

    terms를 필수로 설정, size()와 관계없이 term안에 최소한 필수 약관이 모두 포함되었는지 확인하는 구조로 변경하면 해결될 것 같아요.

  3. CustomAccessDenied, CustomEntrypoint

    ObjectMapper 를 요청마다 new로 생성하고 있어요.

    @Bean 으로 주입받거나 private static final 사용하면 좋을 것 같아요.

  4. MemberReqDTO - joinDTO

    생일에 @Past 붙이면 미래 날짜가 입력되는 것을 Validation 으로 방지할 수 있습니다


수고하셨습니다 주니~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chapter08_Spring Security - Security 구조, 폼 로그인

2 participants