[BE] feat: 스태프 권한 인증 기능 구현 (#478) #480
Conversation
| public class StaffCode { | ||
|
|
||
| public static final int RANDOM_CODE_LENGTH = 4; | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @NotNull | ||
| @Size(max = 30) | ||
| private String code; | ||
|
|
||
| @OneToOne(fetch = FetchType.LAZY) | ||
| private Festival festival; | ||
|
|
There was a problem hiding this comment.
domain 설계
“Festival은 Staff를 알 필요가 없다.”고 생각했습니다.
-
❌ Festival이 (String)staffCode를 가진 구조
→ StaffCode는 festival이 아니라, 별도의 staff 패키지에서 관리되는 것이 좋아보입니다
-
❌ Festival이 staffCodeId를 가진 구조
→ 위에서도 언급했 듯, Festoival은 Staff를 알 필요가 없습니다.
→ 차후 한 Festival에 여러 StaffCode가 존재할 수 있습니다. 하지만, 이는 확장에 불리한 구조입니다.
-
⭕️ StaffCode가 Festival을 가진 구조
→ 어드민 페이지에서 Festival을 조회할 때, fetch join으로 StaffCode를 가져오지 못한다는 단점이 존재합니다. 하지만, Staff에서 Festival은 FK로 관리되므로 성능상 문제가 될 것이라 판단하지 않았습니다.
→ 스태프 인증시, 코드 문자열로 어떤 Festival에 대한 코드인지 검색하는 로직이 필요합니다. 이 구조는 해당 기능에 적합합니다.
→ ‘한 Festival에 여러 StaffCode가 존재한다’로 요구사항이 변했을 때, 그저
@OneToOne을@ManyToOne으로 바꾸기만 하면 됩니다.
There was a problem hiding this comment.
저도 FK를 가지는건 StaffCode가 좋다고 생각합니다.(애쉬가 말씀하신 맥락과 다른 건 없네요.)
| @NotNull | ||
| @Size(max = 30) | ||
| private String code; |
There was a problem hiding this comment.
StaffCode의 코드 원시값 vs 값객체 포장
StudentCode(학생 인증 코드)와 통일성있게 하기 위해 인증 코드 부분을 값객체로 분리했었습니다.
하지만, 학생 인증 코드와 달리 해당 값객체에는 검증이나 비즈니스 로직이 포함되지 않았습니다.
이번에는 값객체 포장으로 얻는 이점은 없고, 오히려 구조가 복잡해졌다 판단해 다시 원시값으로 바꾸어주었습니다. (bb03de0)
값객체 포장 vs 원시값 중 의견 부탁드립니다.
There was a problem hiding this comment.
저는 지금은 딱히 포장할 필요가 없다고 생각해요.
특정한 객체의 책임이 커지고, 값 자체도 어느정도 중요도가 높다면 명확하게 분리할 것 같지만
전 따로 포장 안하고, 필요해지면(주관적인 기준이겠지만) 포장한다(나눈다)가 더 좋다고 여겨집니다.
(다만 이 객체 자체가 StaffCode이기때문에 그렇게 커질 일이 있을까 싶긴하네요.)
There was a problem hiding this comment.
스태프 코드를
도메인의 . 나오기 전 값 과 랜덤한 4자리 숫자
로 정의했었는데 이 요구사항을 코드가 항상 만족하도록 값 객체로 만드는 것도 좋을 것 같아요!!
| public interface StaffCodeProvider { | ||
|
|
||
| String provide(Festival festival); | ||
| } |
There was a problem hiding this comment.
StaffCodeProvider의 provide함수의 인자
provide 함수의 인자로 어떤 데이터를 넘길 것인지에 대한 고민이 있었습니다.
(String) domain / school / festival 중 고민했는데, festival로 택했습니다.
이유는, 스태프 인증 코드가 Festival 단위로 관리되기 때문에, Festival 기반으로 코드를 만드는 것이 가장 자연스럽다고 판단했기 때문입니다.
There was a problem hiding this comment.
전 이부분은 Festival을 넘겨주기 보다는 필요한 값만 딱 넘겨주는게 좋겠다고 생각해요.(여기서는 String domain이 될 것 같습니다.)
Festival은 필요하지 않는 값까지 섞여 있기 때문에 구현에 있어서 조금 혼란이 있을 거라 생각해요. 다만 인터페이스의 파라미터가 바뀌지 않는 것을 원한다면 domain을 일종의 Dto 클래스로 감싸줄 수 있을 것이구요.
| package com.festago.staff.application; | ||
|
|
||
| import static java.util.stream.Collectors.joining; | ||
|
|
||
| import com.festago.festival.domain.Festival; | ||
| import com.festago.staff.domain.StaffCode; | ||
| import java.util.Random; | ||
| import java.util.concurrent.ThreadLocalRandom; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class RandomStaffCodeProvider implements StaffCodeProvider { |
There was a problem hiding this comment.
RandomStaffCodeProvider의 패키지 위치
RandomStaffCodeProvider 클래스를 infrastructure가 아닌 application 패키지에 위치시켰습니다.
infrastructure 패키지는 “외부 종속성 분리”를 위한 패키지인데, RandomStaffCodePRovider에는 외부 종속성이 전혀 없고 비즈니스 로직을 담았기 때문에 application 계층이 적합하다 판단했습니다.
There was a problem hiding this comment.
저도 애쉬 생각에 동의해요. Random 값을 뿌려주기에 interface를 구현 하는게 좋을 것 같고, 외부 의존성 없기 때문에 같은 패키지로 가도 문제없다고 생각해요.
There was a problem hiding this comment.
비즈니스 로직을 담고 있다면 domain 계층이 더 올바르다고 생각하는데 이 의견은 어떠신가요?
There was a problem hiding this comment.
덤으로 student 패키지의 RandomVerificationCodeProvider의 위치 또한 고민해봤으면 좋겠네요
xxeol2
changed the title
| @NotNull | ||
| @Size(max = 30) | ||
| private String code; |
There was a problem hiding this comment.
저는 지금은 딱히 포장할 필요가 없다고 생각해요.
특정한 객체의 책임이 커지고, 값 자체도 어느정도 중요도가 높다면 명확하게 분리할 것 같지만
전 따로 포장 안하고, 필요해지면(주관적인 기준이겠지만) 포장한다(나눈다)가 더 좋다고 여겨집니다.
(다만 이 객체 자체가 StaffCode이기때문에 그렇게 커질 일이 있을까 싶긴하네요.)
| package com.festago.staff.application; | ||
|
|
||
| import static java.util.stream.Collectors.joining; | ||
|
|
||
| import com.festago.festival.domain.Festival; | ||
| import com.festago.staff.domain.StaffCode; | ||
| import java.util.Random; | ||
| import java.util.concurrent.ThreadLocalRandom; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class RandomStaffCodeProvider implements StaffCodeProvider { |
There was a problem hiding this comment.
저도 애쉬 생각에 동의해요. Random 값을 뿌려주기에 interface를 구현 하는게 좋을 것 같고, 외부 의존성 없기 때문에 같은 패키지로 가도 문제없다고 생각해요.
| public interface StaffCodeProvider { | ||
|
|
||
| String provide(Festival festival); | ||
| } |
There was a problem hiding this comment.
전 이부분은 Festival을 넘겨주기 보다는 필요한 값만 딱 넘겨주는게 좋겠다고 생각해요.(여기서는 String domain이 될 것 같습니다.)
Festival은 필요하지 않는 값까지 섞여 있기 때문에 구현에 있어서 조금 혼란이 있을 거라 생각해요. 다만 인터페이스의 파라미터가 바뀌지 않는 것을 원한다면 domain을 일종의 Dto 클래스로 감싸줄 수 있을 것이구요.
| public class StaffCode { | ||
|
|
||
| public static final int RANDOM_CODE_LENGTH = 4; | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @NotNull | ||
| @Size(max = 30) | ||
| private String code; | ||
|
|
||
| @OneToOne(fetch = FetchType.LAZY) | ||
| private Festival festival; | ||
|
|
There was a problem hiding this comment.
저도 FK를 가지는건 StaffCode가 좋다고 생각합니다.(애쉬가 말씀하신 맥락과 다른 건 없네요.)
| private String createVerificationCode(Festival festival) { | ||
| String code; | ||
| do { | ||
| code = codeProvider.provide(festival); | ||
| } while (staffCodeRepository.existsByCode(code)); | ||
| return code; | ||
| } |
There was a problem hiding this comment.
무한반복이 리스크가 있긴한데.. 가장 단순하게 푸는 방법이긴 하네요.
의존성 받아서 @Retry 하는것도 가능하지만 로직 처음부터 시작하는게 별로 맘에 안들고..
다들 어떻게 생각하시나요. 이대로 그냥 놔두기 vs 어떤 방식으로든 2~3번 까지 최대 재시도하고 실패 하는 방법도 있을 것 같아요.
There was a problem hiding this comment.
랜덤이라 무한루프 돌 가능성이 없어보여서 저대로 놔뒀는데
만일의 만일로 방어로직 작성하는게 좋아보이네요!!
메서드변수로 tryCount 둬서 푸는 방법이 제일 간단해보이는데 다른분들은 어떻게생각하시나요!
There was a problem hiding this comment.
지금은 축제당 코드가 하나 밖에 없는 것으로 생각하고 있으니
festival 에 대한 모든 스태프 코드를 지운 후 검증 없이 하나만 생성하는 것도 좋을 것 같네요!!
추후에 축제에 2개 이상의 코드를 유지한다면 like 'teco%' 이런 조건문으로 뭐가있는지 확인한 후에 was 레벨에서 복잡도를 풀어내는 것이 좋아보여요!! 지금은 확인을 DB를 통해 하고있으니 다소 비싼 연산일 수도 있겠다는 생각입니다 ㅎㅎ
There was a problem hiding this comment.
스프링 부트 버전업 후 진행하겠습니다 . . . . 🫨
backend/src/main/java/com/festago/auth/dto/StaffLoginRequest.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/festago/common/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
| @Operation(description = "스태프가 티켓을 검사한다.", summary = "티켓 검사") | ||
| public ResponseEntity<TicketValidationResponse> validate( | ||
| @RequestBody @Valid TicketValidationRequest request, | ||
| @Staff Long festivalId) { |
There was a problem hiding this comment.
| @Staff Long festivalId) { | |
| @Staff Long staffId) { |
또한 entryService의 validate 메서드 또한, 파라미터 명이 잘못된 것 같네요!
There was a problem hiding this comment.
코드로 인증할 때, jwt 토큰에 StaffCode 정보를 담지 않고 Festival 정보를 담아서 festivalId를 의도한게 맞긴합니다..!!
There was a problem hiding this comment.
Staff정보를 담는게 더 좋을까요 ?!
그렇게 하면 Staff -> Festival 한 단계 타고 가야해서 이렇게 설계했었어요!
There was a problem hiding this comment.
지금보니 Staff 도메인의 Staff 엔티티가 StaffCode로 되어 있군요 😂
Staff로 이름을 바꾸는게 더 명확해보이네요.
지금 같은 방법을 사용하면 티켓을 검증할 때 바로 festivalId로 MemberTicket 엔티티에서 belongsFestival 메서드를 호출하여 추가적인 쿼리문 없이 티켓을 검증할 수 있겠네요!
하지만 코드 상에서 흐름을 읽기가 더 어려워졌지 않나 싶습니다.. (컨트롤러에서 @Staff로 받아온 Long 타입은 staff의 Id라고 당연하게 생각할 수 있으므로)
There was a problem hiding this comment.
7e8a268 staffId를 넘기는 방식으로 반영완료했습니다!
backend/src/main/java/com/festago/presentation/StaffController.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/festago/entry/application/EntryService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/festago/festival/application/FestivalService.java
Show resolved
Hide resolved
This reverts commit bb03de0.
- StaffCode -> Staff - StaffVerificationCode -> StaffCode
# Conflicts: # backend/src/main/java/com/festago/entry/application/EntryService.java # backend/src/main/java/com/festago/festival/application/FestivalService.java # backend/src/main/java/com/festago/presentation/StaffMemberTicketController.java # backend/src/main/java/com/festago/school/domain/School.java # backend/src/main/java/com/festago/stage/domain/Stage.java # backend/src/test/java/com/festago/application/EntryServiceTest.java # backend/src/test/java/com/festago/domain/StageTest.java
# Conflicts: # backend/src/test/java/com/festago/support/SetUpMockito.java
|
안드로이드분들과 논의 진행 후 머지해야합니다! |
|
불필요 브랜치를 삭제하다가 삭제해버렸네요. 죄송합니다! |
📌 관련 이슈
✨ PR 세부 내용
스태프 인증 권한 기능을 구현했습니다!
상세 요구사항은 관련 이슈 확인해주시면 감사하겠습니다.
구현하며 고민한 부분들에 대해서는 코멘트로 남기겠습니다!