[Team-20 / BE - Dong] 숙소 조회, 좋아요를 누른 숙소 기능#53
[Team-20 / BE - Dong] 숙소 조회, 좋아요를 누른 숙소 기능#53moto6 wants to merge 85 commits intocodesquad-members-2021:team-20from
Conversation
[BE] Dto 멤버 내용 추가
imges upload
필드변수로 private String accessToken;
private String tokenType;
private String scope;
가 있음
- 클래스 및 생성자와 구현을 위한 메서드 껍데기만 만듬 - 데이터가 포함된 로직은 아직 구현 안됨
feat: Dao 샘플을 구현
Update README.md
Be/feature/15/oauth
Be/feature/16/suggestions
- 생성자 수정 - 컨트롤러에 getSample() 메서드 추가 - AccommodationDTO 필드변수 도메인과 동일하게 수정
wheejuni
left a comment
wheejuni
left a comment
There was a problem hiding this comment.
데이터베이스에 질의하는 로직을 if로 처리하면, 나중에 데이터가 방대해져서 페이징 등이 발생할 때 어떻게 대처할지도 잘 고민해 보셔야 할 겁니다. 개인적으로 추천하는 방식은 아닙니다.
"행정구역" 에 관한 개념이 추상화되어 있으면 되겠죠? 서로간의 포함관계가 존재하면 될 겁니다.
그런데 지금 그런 부분보다도, "관성적으로 코딩하지 않는 방법" 에 대한 고민이 필요한 시점이라고 판단됩니다.
그런 고민이 부족하신 듯 보여서 조금 아쉽다는 느낌은 들었습니다. 여튼 수고 많으셨고 수정해야 할 부분에 대해 다시 살펴보셨으면 좋겠습니다. 💯
| private final String GITHUB_ACCESS_TOKEN_URI = "https://github.com/login/oauth/access_token"; | ||
| private final String GITHUB_USER_URI = "https://api.github.com/user"; | ||
| private final String ISSUER = ""; | ||
| private final String CLIENT_ID = "your client id"; | ||
| private final String CLIENT_SECRET = "your client secret"; | ||
| private final String CODE = ""; |
There was a problem hiding this comment.
컨트롤러에 위치해야 할 정보들이 아닌 것 같습니다. 어떻게 분리할 수 있을까요?
There was a problem hiding this comment.
제가 너무 안일한 생각을 했습니다.
아주아주 간단한 모듈(auth)이 라서, 서비스계층과 컨트롤러 계층을 나눌 생각을 하지 않았는데, 말씀을 듣고 나니 컨트롤러 계층에서 가지고 있기 부적절한 정보가 맞습니다. 수정 완료했습니다!
| System.out.println(accessToken); | ||
| System.out.println(gitHubRequest); |
| private Optional<User> getUserFromGitHub(GithubAccessTokenResponse accessToken, RestTemplate gitHubRequest) { | ||
| RequestEntity<Void> request = RequestEntity | ||
| .get(GITHUB_USER_URI) | ||
| .header("Accept", "application/json") | ||
| .header("Authorization", "token " + accessToken.getAccessToken()) | ||
| .build(); | ||
|
|
||
| ResponseEntity<User> response = gitHubRequest | ||
| .exchange(request, User.class); | ||
|
|
||
| return Optional.ofNullable(response.getBody()); | ||
| } | ||
|
|
||
| private Optional<GithubAccessTokenResponse> getAccessToken(String code, RestTemplate gitHubRequest) { | ||
| RequestEntity<GithubAccessTokenRequest> request = RequestEntity | ||
| .post(GITHUB_ACCESS_TOKEN_URI) | ||
| .header("Accept", "application/json") | ||
| .body(new GithubAccessTokenRequest(CLIENT_ID, CLIENT_SECRET, code)); | ||
|
|
||
| ResponseEntity<GithubAccessTokenResponse> response = gitHubRequest | ||
| .exchange(request, GithubAccessTokenResponse.class); | ||
|
|
||
| return Optional.ofNullable(response.getBody()); | ||
| } |
There was a problem hiding this comment.
별도의 서비스나 클라이언트 클래스(컴포넌트 클래스) 가 구성돼야 하겠지요?
컨트롤러에 있으면 안될 녀석들인 것 같은데 어떻게 생각하시는지 궁금합니다.
There was a problem hiding this comment.
네 위에서 말씀해주셔서 해당 부분은 코드에 반영했습니다
| .sign(algorithm); | ||
| } catch (JWTCreationException exception) { | ||
| //Invalid Signing configuration / Couldn't convert Claims. | ||
| throw new RuntimeException(exception); |
There was a problem hiding this comment.
JWTCreationException 이라는 라이브러리에서 제공하는 checked exception을 그냥 RuntimeException 으로 던지네요?
좋은 예외처리일까요? 어떻게 생각하시나요? RuntimeException 으로 충분한가요?
| return JWT.create() | ||
| .withClaim("login", user.getLogin()) | ||
| .withClaim("name", user.getName()) | ||
| .withIssuer(ISSUER) | ||
| .sign(algorithm); |
| package com.example.airbnb.exception; | ||
|
|
||
| public class JWTCreationException extends RuntimeException { | ||
| final static String MESSAGE = "error"; |
There was a problem hiding this comment.
우선 이 상수를 이 클래스에서 관리해야겠다고 생각한 이유가 궁금합니다.
그리고 MESSAGE 라는 변수명이 어떠한 의미를 갖는지가 궁금하군요.
There was a problem hiding this comment.
앗 해당부분은 제가 놓쳤네요. 조금 더 명확한 문자열의 이름과 메시지를 작성하였습니다!
BE/airbnb/src/main/java/com/example/airbnb/exception/UnExpectedTimeFormat.java
Outdated
Show resolved
Hide resolved
| private JdbcTemplate jdbcTemplate; | ||
| private NamedParameterJdbcTemplate namedParameterJdbcTemplate; |
There was a problem hiding this comment.
이 두 컴포넌트 다 주입받을 수 있을 겁니다.
Spring Context에서 어떤 컴포넌트를 기본으로 빈 등록해서 주입해줄 수 있는지 알아보시는 것도 큰 도움이 되겠네요.
There was a problem hiding this comment.
넵 리뷰해주신 부분 동의합니다!
- NamedParameterJdbcTemplate 은 사용하지 않아서 해당부분은 걷어냈습니다.
- Spring Context에서 어떤 컴포넌트를 기본으로 빈 등록해서 주입해줄 수 있는지 학습한 내용은 아래 링크에 정리해뒀습니다.
- 좀 많이 부족한데 차근차근 채워나가겠습니다!
| private boolean isPeopleCountValidated(SearchConditions conditions) { | ||
| if (0 >= conditions.getAdults()) { | ||
| return false; | ||
| } | ||
| if (0 >= conditions.getChildren()) { | ||
| return false; | ||
| } | ||
| return 0 < conditions.getInfants(); | ||
| } |
| private static final List<RecommendedDTO> recommendedDTOList = new ArrayList<>(); | ||
| private static final List<ThemeDTO> themeDTOList = new ArrayList<>(); | ||
|
|
||
| static { |
There was a problem hiding this comment.
음 지금은 해당 Recommended, Theme 정보가 추가되거나 삭제되지 않는 형태라서 아직 DB 는 아니지만, 장기적인 관점에서는 확장성을 위해서 DB 에 넣어주는 방식이 좋을꺼같습니다!
- reviewCount가 정상적으로 실행되지 않고 null 값이 나오던 문제를 수정 - Options 필드 추가
- fe 측 요청으로 location 관련한 컬럼 데이터 추가
- 리뷰사항 codesquad-members-2021#53 (comment) - 코드 클리닝을 진행하고 서비스계층과 컨트롤러 계층의 역할을 구분함
- 단순한 String class의 래핑클래스이므로 삭제
4 participants
intro
안녕하세요 Team-20 BE Dong 입니다
지금 많은 부분이 부족한 코드인데 조금은 마음이 싱숭생숭해서 부족한 코드지만PR 날립니다 😥😥😥😥😥
Done
Todo(앞으로 진행할것들)
궁금한점