Skip to content
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

리소스 생성 및 단일 조회에 대한 HTTP API를 리팩터링한다. #561

Closed
woowahan-pjs opened this issue Jul 12, 2022 · 3 comments
Assignees
Labels
리팩터링 Refactor code

Comments

@woowahan-pjs
Copy link
Contributor

woowahan-pjs commented Jul 12, 2022

  1. create api reponse 수정
    1. ❌ 그대로 둔다. -> dsl 생성시 id를 넣어주어야하는데 id를 모른다. 불가능.
    2. ❌ create시에 Location header에 id를 넣어준다. -> dsl 생성시 create된 id를 넣어줄 수 있다.
    3. ⭕️ 객체를 반환한다. -> id 외의 다른 필드 값들은 갖고 있기 때문에 굳이 객체까지 받을 필요는 없으나, 객체를 반환한다고 문제 되는 부분이 없고 확장성이 더 좋다. 채택.
  2. id로 조회하는 api가 없는 도메인이 있다.
    • 모집 생성 시 기수 id만 받는 경우, id로 조회하는 api가 없어서 기수 이름을 알 수 없다. 따라서 기수를 생성할 수가 없다.
    1. ❌ 이름에 아무 default 값을 넣는다. -> 어차피 service의 save 로직에서 id 외의 이름 등의 필드는 사용하지 않는다. 하지만 그렇다고 아무 값이나 넣어주는 것이 맞을까?
    2. ⭕️ id로 기수를 조회하는 api를 만든다. -> 클라이언트가 사용하지 않는 api를 테스트를 위해 만드는 것이 맞을까? 하지만 클라이언트가 사용하지 않는다고 해서 단건 조회 api가 진짜 불필요하다고 말할 수 없다고 생각할 수도 있음. 채택
  3. TermController에 @LoginUser 추가
    • 현재 Term관련 api에 인가과정이 없다.
@summerlunaa
Copy link
Contributor

summerlunaa commented Jul 24, 2022

✨[UPDATE] 생성 메서드 네이밍 규칙✨

  1. save : 주로 vaddin에서 사용, 코드에는 생성 로직만 존재하더라도 vaddin에서 생성과 수정 두 경우 모두 save를 호출한다.
  2. create : vaddin에서 사용 X, update 메서드가 따로 존재하며 생성하는 경우에만 create를 호출한다.

결론: update 메서드가 있는 경우 create를 사용한다. update 메서드가 없는 경우 save를 사용한다.

[처음 생각] 생성 메서드 네이밍 통일

domain 생성 Controller 이름 호출하는 Service 메서드명 생성 외의 로직 존재 여부
ApplicationForm create create X
Assignment create create X
Cheater save save X
Evaluation createEvaluation save O, 기존 데이터 삭제 후 저장함
MailHistory save save X
Mission save save X
Recruitment save save O, 기존 데이터 삭제 후 저장함
Term create save X

현재 생성 API의 이름이 통일성이 없다. 메서드명을 통일해줄 필요가 있어 보인다.

  • create : 단순 생성
  • save : 생성 + 이외의 로직

@summerlunaa
Copy link
Contributor

summerlunaa commented Jul 24, 2022

Response 타입 : Data 타입 VS Response 타입

이전엔 두 DTO의 차이를 명확히 알지 못한 채 Data 타입을 응답으로 반환해주었다. 하지만 결론적으로는 Response 타입을 반환해주는 것으로 변경되었다.

Response 타입은 우리가 생각하는 일반적인 응답을 위한 DTO이다. 반면 Data 타입은 Vaddin에서 사용할 목적으로 만든 DTO. 앞으로 Vaddin 사용을 지양하고 걷어내는 상황까지 고려한다면 최대한 Data 타입 사용을 자제해야 한다. 따라서 생성시 Response 타입을 반환하는 것으로 결정했다.

@dongho108
Copy link
Contributor

Response 타입 : Data 타입 VS Response 타입

생성시 반환 타입을 우선 Data 타입으로 지정했다.

  • Data 타입 반환 시

    • 요청 타입과 응답 타입이 같아 필드에 id만 추가해 주면 됨
    • 테스트 시 응답을 Data 객체로 받으면 받은 객체를 바로 다른 요청에 넣어 보낼...수 있을줄 알았으나 어차피 변환이 필요함
  • Response 타입 반환 시

    • DTO 목적 자체가 응답 목적이기 때문에 조금 더 적절해보임
    • 테스트 시 Response Dto의 필드와 (요청시 필요한) Data 객체의 필드가 다른 경우 문제가 복잡해진다.

어떤 게 더 적절할까?..

아직 프론트와 약속된 응답이 아니기 때문에 지금은 post 요청한 필드 그대로 반환해주는게 맞는 것 같고, 추후에 프론트와 응답이 약속된 상태에선 해당 응답을 기준으로 response를 만들면 되지 않을까 생각합니다..

@woowahan-pjs woowahan-pjs changed the title 리소스 생성 HTTP API를 리팩터링한다. HTTP API를 리팩터링한다. Aug 1, 2022
@woowahan-pjs woowahan-pjs changed the title HTTP API를 리팩터링한다. 리소스 생성 및 단일 조회에 대한 HTTP API를 리팩터링한다. Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
리팩터링 Refactor code
Projects
None yet
Development

No branches or pull requests

3 participants