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

🐷 step3 : 즐겨찾기 기능 구현 #241

Open
wants to merge 10 commits into
base: minyul
Choose a base branch
from
13 changes: 6 additions & 7 deletions src/main/java/nextstep/auth/AuthConfig.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package nextstep.auth;

import com.fasterxml.jackson.databind.ObjectMapper;
import nextstep.auth.authentication.Authorizor;
import nextstep.auth.authentication.Authorizer;
import nextstep.auth.authentication.SessionAuthenticationInterceptor;
import nextstep.auth.authentication.TokenAuthenticationInterceptor;
import nextstep.auth.authorization.AuthenticationPrincipalArgumentResolver;
import nextstep.auth.authorization.SessionSecurityContextPersistenceInterceptor;
import nextstep.auth.authorization.TokenSecurityContextPersistenceInterceptor;
import nextstep.auth.token.JwtTokenProvider;
import nextstep.member.application.CustomUserDetailsService;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
Expand All @@ -20,19 +19,19 @@ public class AuthConfig implements WebMvcConfigurer {
private UserDetailsService userDetailsService;
private JwtTokenProvider jwtTokenProvider;
private ObjectMapper objectMapper;
private Authorizor authorizor;
private Authorizer authorizor;

public AuthConfig(UserDetailsService userDetailsService, JwtTokenProvider jwtTokenProvider) {
public AuthConfig(UserDetailsService userDetailsService, JwtTokenProvider jwtTokenProvider, ObjectMapper objectMapper) {
this.userDetailsService = userDetailsService;
this.jwtTokenProvider = jwtTokenProvider;
this.objectMapper = new ObjectMapper();
this.authorizor = new Authorizor();
this.objectMapper = objectMapper;
this.authorizor = new Authorizer(userDetailsService);
}

@Override
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(new SessionAuthenticationInterceptor(userDetailsService, authorizor)).addPathPatterns("/login/session");
registry.addInterceptor(new TokenAuthenticationInterceptor(userDetailsService, jwtTokenProvider, objectMapper, authorizor))
registry.addInterceptor(new TokenAuthenticationInterceptor(jwtTokenProvider, objectMapper, authorizor))
.addPathPatterns("/login/token");
registry.addInterceptor(new SessionSecurityContextPersistenceInterceptor());
registry.addInterceptor(new TokenSecurityContextPersistenceInterceptor(jwtTokenProvider));
Expand Down
33 changes: 33 additions & 0 deletions src/main/java/nextstep/auth/authentication/Authorizer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package nextstep.auth.authentication;

import nextstep.auth.User;
import nextstep.auth.UserDetailsService;
import nextstep.auth.context.Authentication;
import org.springframework.util.ObjectUtils;

public class Authorizer {

private UserDetailsService userDetailsService;

public Authorizer(UserDetailsService userDetailsService) {
this.userDetailsService = userDetailsService;
}

public void checkAuthentication(User userDetails, AuthenticationToken token) {

Choose a reason for hiding this comment

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

private으로 선언해볼 수 있겠네요 :)

Copy link
Author

Choose a reason for hiding this comment

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

리뷰는 천천히 해주셔도 됩니다! 이미 시간이 지나버렸는걸요! 천천히 해주셔도 괜찮습니다. 해주시는것만으로도 정말 감사하구요! 감사합니다!

if (ObjectUtils.isEmpty(userDetails)) {
throw new AuthenticationException();
}

if (!userDetails.checkPassword(token.getCredentials())) {
throw new AuthenticationException();

Choose a reason for hiding this comment

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

AuthenticationException을 던지면 401 UNAUTHORIZED가 응답됩니다 :)
AuthenticationException클래스에 보면 @ResponseStatus(HttpStatus.UNAUTHORIZED) 어노테이션이 붙어있기 때문입니다!

Choose a reason for hiding this comment

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

AuthenticationPrincipalArgumentResolverresolveArgument에서 authentication 이 null인 경우는 로그인이 안된 케이스로 볼 수 있겠네요 :)

}
}

public Authentication authenticate(AuthenticationToken authenticationToken) {
final String principal = authenticationToken.getPrincipal();
final User userDetails = userDetailsService.loadUserByUsername(principal);
checkAuthentication(userDetails, authenticationToken);

return new Authentication(userDetails);
}
}
17 changes: 0 additions & 17 deletions src/main/java/nextstep/auth/authentication/Authorizor.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package nextstep.auth.authentication;

import nextstep.auth.User;
import nextstep.auth.UserDetailsService;
import nextstep.auth.context.Authentication;
import nextstep.auth.context.SecurityContext;
Expand All @@ -20,32 +19,24 @@ public class SessionAuthenticationInterceptor implements HandlerInterceptor, Aut
public static final String PASSWORD_FIELD = "password";

private UserDetailsService userDetailsService;
private Authorizor authorizor;
private Authorizer authorizor;

public SessionAuthenticationInterceptor(UserDetailsService userDetailsService, Authorizor authorizor) {
public SessionAuthenticationInterceptor(UserDetailsService userDetailsService, Authorizer authorizor) {
this.userDetailsService = userDetailsService;
this.authorizor = authorizor;
}

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException {
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
AuthenticationToken token = convert(request);
Authentication authentication = authenticate(token);
Authentication authentication = authorizor.authenticate(token);

HttpSession httpSession = request.getSession();
httpSession.setAttribute(SPRING_SECURITY_CONTEXT_KEY, new SecurityContext(authentication));
response.setStatus(HttpServletResponse.SC_OK);
return false;
}

public Authentication authenticate(AuthenticationToken token) {
String principal = token.getPrincipal();
User userDetails = userDetailsService.loadUserByUsername(principal);
authorizor.checkAuthentication(userDetails, token);

return new Authentication(userDetails);
}

@Override
public AuthenticationToken convert(HttpServletRequest request) {
Map<String, String[]> paramMap = request.getParameterMap();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package nextstep.auth.authentication;

import com.fasterxml.jackson.databind.ObjectMapper;
import nextstep.auth.User;
import nextstep.auth.UserDetailsService;
import nextstep.auth.context.Authentication;
import nextstep.auth.token.JwtTokenProvider;
Expand All @@ -16,16 +15,13 @@

public class TokenAuthenticationInterceptor implements HandlerInterceptor, AuthenticationConverter {

private UserDetailsService userDetailsService;
private JwtTokenProvider jwtTokenProvider;
private ObjectMapper objectMapper;
private Authorizor authorizor;
private Authorizer authorizor;

public TokenAuthenticationInterceptor(UserDetailsService userDetailsService,
JwtTokenProvider jwtTokenProvider,
public TokenAuthenticationInterceptor(JwtTokenProvider jwtTokenProvider,
ObjectMapper objectMapper,
Authorizor authorizor) {
this.userDetailsService = userDetailsService;
Authorizer authorizor) {
this.jwtTokenProvider = jwtTokenProvider;
this.objectMapper = objectMapper;
this.authorizor = authorizor;
Expand All @@ -34,7 +30,7 @@ public TokenAuthenticationInterceptor(UserDetailsService userDetailsService,
@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws IOException {
AuthenticationToken authenticationToken = convert(request);
Authentication authentication = authenticate(authenticationToken);
Authentication authentication = authorizor.authenticate(authenticationToken);

final String payload = objectMapper.writeValueAsString(authentication.getPrincipal());
final String token = jwtTokenProvider.createToken(payload);
Expand All @@ -49,14 +45,6 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
return false;
}

public Authentication authenticate(AuthenticationToken authenticationToken) {
final String principal = authenticationToken.getPrincipal();
final User userDetails = userDetailsService.loadUserByUsername(principal);
authorizor.checkAuthentication(userDetails, authenticationToken);

return new Authentication(userDetails);
}

@Override
public AuthenticationToken convert(HttpServletRequest request) throws IOException {
TokenRequest tokenRequest = objectMapper.readValue(request.getInputStream(), TokenRequest.class);
Expand Down
45 changes: 41 additions & 4 deletions src/main/java/nextstep/member/application/MemberService.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
package nextstep.member.application;

import nextstep.member.application.dto.FavoriteRequest;
import nextstep.member.application.dto.FavoriteResponse;
import nextstep.member.application.dto.MemberRequest;
import nextstep.member.application.dto.MemberResponse;
import nextstep.member.domain.LoginMember;
import nextstep.member.domain.Member;
import nextstep.member.domain.MemberRepository;
import nextstep.member.domain.*;
import nextstep.subway.applicaion.StationService;
import nextstep.subway.domain.Station;
import org.springframework.stereotype.Service;

import java.util.List;
import java.util.stream.Collectors;

@Service
public class MemberService {

private MemberRepository memberRepository;
private FavoriteRepository favoriteRepository;
private StationService stationService;

public MemberService(MemberRepository memberRepository) {
public MemberService(MemberRepository memberRepository,
FavoriteRepository favoriteRepository,
StationService stationService) {
this.memberRepository = memberRepository;
this.favoriteRepository = favoriteRepository;
this.stationService = stationService;
}

public MemberResponse createMember(MemberRequest request) {
Expand Down Expand Up @@ -53,4 +65,29 @@ public void deleteMemberOfMine(LoginMember loginMember) {
Member member = memberRepository.findByEmail(loginMember.getEmail()).orElseThrow(RuntimeException::new);
memberRepository.delete(member);
}

public FavoriteResponse createFavorite(LoginMember loginMember, FavoriteRequest favoriteRequest) {
Member member = memberRepository.findById(loginMember.getId()).orElseThrow(RuntimeException::new);
Station source = stationService.findById(favoriteRequest.getSource());
Station target = stationService.findById(favoriteRequest.getTarget());
Favorite favorite = new Favorite(member, source, target);
favoriteRepository.save(favorite);

return FavoriteResponse.of(favorite);
}

public List<FavoriteResponse> findFavoritesOfMine(LoginMember loginMember) {
Member member = memberRepository.findById(loginMember.getId()).orElseThrow(RuntimeException::new);

return member.getFavorites().stream()
.map(FavoriteResponse::of)
.collect(Collectors.toList());
}

public void deleteFavorite(LoginMember loginMember, Long favoriteId) {
Member member = memberRepository.findById(loginMember.getId()).orElseThrow(RuntimeException::new);
Favorite favorite = favoriteRepository.findById(favoriteId).orElseThrow(RuntimeException::new);

member.removeFavorite(favorite);
}
}
15 changes: 15 additions & 0 deletions src/main/java/nextstep/member/application/dto/FavoriteRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package nextstep.member.application.dto;

public class FavoriteRequest {

private Long source;
private Long target;

public Long getSource() {
return source;
}

public Long getTarget() {
return target;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package nextstep.member.application.dto;

import nextstep.member.domain.Favorite;
import nextstep.subway.applicaion.dto.StationResponse;

public class FavoriteResponse {

private Long id;
private StationResponse source;
private StationResponse target;

public static FavoriteResponse of(Favorite favorite) {

return new FavoriteResponse(
favorite.getId(),
StationResponse.of(favorite.getSource()),
StationResponse.of(favorite.getTarget()));
}

public FavoriteResponse(Long id, StationResponse source, StationResponse target) {
this.id = id;
this.source = source;
this.target = target;
}

public Long getId() {
return id;
}

public StationResponse getSource() {
return source;
}

public StationResponse getTarget() {
return target;
}
}
49 changes: 49 additions & 0 deletions src/main/java/nextstep/member/domain/Favorite.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package nextstep.member.domain;

import nextstep.subway.domain.Station;

import javax.persistence.*;

@Entity
public class Favorite {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@ManyToOne(cascade = CascadeType.PERSIST)
@JoinColumn(name = "member_id")
private Member member;

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.

과거 신입 프로젝트로 로그인쪽을 구현하고 코드리뷰를 받았을 때, 사용자와 관련된 부분은 직접참조, 아닌 경우에는 간접 참조를 하라고 피드백을 받았었는데요! 어떻게 생각하시나요?

그리고 준우님 말씀대로 직접 -> 간접으로 객체참조를 바꿨습니다.
혹시 잘했는지 확인해주실수있을까요?!

감사합니다.


@ManyToOne(cascade = CascadeType.PERSIST)
@JoinColumn(name = "up_station_id")
private Station source;

@ManyToOne(cascade = CascadeType.PERSIST)
@JoinColumn(name = "down_station_id")
private Station target;

Choose a reason for hiding this comment

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

Favorite이 저장될 때, Member, source, target들이 함께 저장되는게 의도하신 바가 맞을까요?
라이프사이클을 모두 하나로 보신 것인지 궁금합니다 👀

Copy link
Author

Choose a reason for hiding this comment

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

음..... 이부분은 기존에 LineSection의 관계의 코드를 보고 구현했습니다.
지금 다시 생각해보면 Member에만 CascadeType.PERSIST 가 있는 것이 맞다고 생각되고
Favorite에는 굳이 CascadeType.PERSIST가 필요없다고 생각합니다.


더 나아가서 이 부분이 진짜 계속 멤돌았는데..

Section 에서 Upstation, DownStation.
Favorite 에서 Source, Target.

이부분에서 @ManyToOne을 쓰는 것이 맞나?! 하는 의문이 있습니다.

image

음.. 구간(Section) 에 역 2개 (Upstation, DownStation) 이 있으며
즐겨찾기(Favorite)에 역 2개 (Source, Target) 이 있다.

결국엔 같지만 왜 @ManyToOne이지? 라는 의문이 살아지지가 않아서요.ㅠㅠㅠㅠㅠ

Comment on lines +17 to +23

Choose a reason for hiding this comment

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

cascade는 없이 하는게 더 좋을 것 같아요.
Station의 라이프 사이클이 Favorite에 종속될 이유가 없을 것 같습니다!
ManyToOne의 경우 N+1을 피하기 위해 fetchType을 LAZY로 설정하시고 fetch join을 사용해보시면 좋을 것 같아요!

Choose a reason for hiding this comment

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

@minyul

음.. 구간(Section) 에 역 2개 (Upstation, DownStation) 이 있으며
즐겨찾기(Favorite)에 역 2개 (Source, Target) 이 있다.
결국엔 같지만 왜 @manytoone이지? 라는 의문이 살아지지가 않아서요.ㅠㅠㅠㅠㅠ

요 질문의 요지를 제가 잘 이해하지 못하겠습니다....... 😭
Section의 upStation, downStation은 한 섹션의 상행역과 바로 다음 하행역을 말하는 것이고,
Favorite은 source(출발지), target(도착지)라서 단순히 역을 두개 가지고 있다는 것만 같을 뿐 전혀 다른 관계라서요....
혹시 요건 따로 디엠이나 추가 댓글로 질의주시면 답변드리겠습니다!


protected Favorite() {}

public Favorite(Member member, Station source, Station target) {
this.member = member;
this.source = source;
this.target = target;
}

public Long getId() {
return id;
}

public Member getMember() {
return member;
}

public Station getSource() {
return source;
}

public Station getTarget() {
return target;
}
}
7 changes: 7 additions & 0 deletions src/main/java/nextstep/member/domain/FavoriteRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package nextstep.member.domain;

import org.springframework.data.jpa.repository.JpaRepository;

public interface FavoriteRepository extends JpaRepository<Favorite, Long> {

}
27 changes: 27 additions & 0 deletions src/main/java/nextstep/member/domain/Favorites.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package nextstep.member.domain;

import javax.persistence.CascadeType;
import javax.persistence.Embeddable;
import javax.persistence.OneToMany;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

@Embeddable
public class Favorites {

@OneToMany(mappedBy = "member", cascade = {CascadeType.PERSIST, CascadeType.MERGE}, orphanRemoval = true)
private List<Favorite> favorites = new ArrayList<>();

public void add(Favorite favorite) {
this.favorites.add(favorite);
}

public void remove(Favorite favorite) {
this.favorites.remove(favorite);
}

public List<Favorite> getFavorites() {
return Collections.unmodifiableList(favorites);
}
Comment on lines +10 to +26

Choose a reason for hiding this comment

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

더이상 쓰이지 않는 객체라 제거해도 좋을 것 같습니다 :)

}
Loading