Skip to content

[MVC 구현하기 - 2, 3단계] 포츈(정윤성) 미션 제출합니다. #97

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

Merged
merged 15 commits into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions app/src/main/java/com/techcourse/AppWebApplicationInitializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletRegistration;
import nextstep.mvc.DispatcherServlet;
import nextstep.mvc.HandlerMapping;
import nextstep.mvc.controller.adapter.HandlerAdapter;
import nextstep.mvc.controller.adapter.HandlerExecutionHandlerAdapter;
import nextstep.mvc.controller.tobe.AnnotationHandlerMapping;
import nextstep.web.WebApplicationInitializer;
import org.slf4j.Logger;
Expand All @@ -16,13 +19,18 @@ public class AppWebApplicationInitializer implements WebApplicationInitializer {
@Override
public void onStartup(ServletContext servletContext) {
final DispatcherServlet dispatcherServlet = new DispatcherServlet();
dispatcherServlet.addHandlerMapping(new ManualHandlerMapping());
dispatcherServlet.addHandlerMapping(new AnnotationHandlerMapping(BASE_PACKAGE));

addFrameWork(dispatcherServlet, new AnnotationHandlerMapping(BASE_PACKAGE), new HandlerExecutionHandlerAdapter());

final ServletRegistration.Dynamic dispatcher = servletContext.addServlet("dispatcher", dispatcherServlet);
dispatcher.setLoadOnStartup(1);
dispatcher.addMapping("/");

LOGGER.info("Start AppWebApplication Initializer");
}

private void addFrameWork(final DispatcherServlet dispatcherServlet, final HandlerMapping handlerMapping, final HandlerAdapter handlerAdapter) {
dispatcherServlet.addHandlerMapping(handlerMapping);
dispatcherServlet.addHandlerAdapter(handlerAdapter);
}
}
11 changes: 5 additions & 6 deletions app/src/main/java/com/techcourse/JwpApplication.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package com.techcourse;

import java.io.File;
import java.util.stream.Stream;
import org.apache.catalina.Context;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.startup.Tomcat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;
import java.util.stream.Stream;

public class JwpApplication {

private static final Logger log = LoggerFactory.getLogger(JwpApplication.class);
Expand All @@ -31,9 +30,9 @@ public static void main(String[] args) throws Exception {

private static int defaultPortIfNull(String[] args) {
return Stream.of(args)
.findFirst()
.map(Integer::parseInt)
.orElse(DEFAULT_PORT);
.findFirst()
.map(Integer::parseInt)
.orElse(DEFAULT_PORT);
}

private static Context addWebapp(Tomcat tomcat) {
Expand Down
37 changes: 0 additions & 37 deletions app/src/main/java/com/techcourse/ManualHandlerMapping.java

This file was deleted.

18 changes: 18 additions & 0 deletions app/src/main/java/com/techcourse/controller/ForwardController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.techcourse.controller;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import nextstep.mvc.view.JspView;
import nextstep.mvc.view.ModelAndView;
import nextstep.web.annotation.Controller;
import nextstep.web.annotation.RequestMapping;
import nextstep.web.support.RequestMethod;

@Controller
public class ForwardController {

@RequestMapping(value = "/", method = RequestMethod.GET)
public ModelAndView execute(HttpServletRequest req, HttpServletResponse res) throws Exception {
return new ModelAndView(new JspView("/index.jsp"));
Copy link

@taehee-kim-dev taehee-kim-dev Sep 16, 2021

Choose a reason for hiding this comment

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

"/index.jsp" 등의 View name들이 여러 컨트롤러에 반복적으로 쓰이고 있어요.

mvc 모듈의 nextstep.web.support 패키지의 MediaType.class처럼,
별도의 클래스 상수용 클래스를 만들어 View name을 관리하면 어떨까요?

}
}
49 changes: 35 additions & 14 deletions app/src/main/java/com/techcourse/controller/LoginController.java
Original file line number Diff line number Diff line change
@@ -1,39 +1,60 @@
package com.techcourse.controller;

import com.techcourse.domain.User;
import com.techcourse.domain.UserSession;
import com.techcourse.repository.InMemoryUserRepository;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
import nextstep.mvc.controller.asis.Controller;
import nextstep.mvc.view.JspView;
import nextstep.mvc.view.ModelAndView;
import nextstep.mvc.view.View;
import nextstep.web.annotation.Controller;
import nextstep.web.annotation.RequestMapping;
import nextstep.web.support.RequestMethod;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class LoginController implements Controller {
@Controller
public class LoginController {

private static final Logger log = LoggerFactory.getLogger(LoginController.class);
private static final Logger LOGGER = LoggerFactory.getLogger(LoginController.class);

@Override
public String execute(HttpServletRequest req, HttpServletResponse res) {
@RequestMapping(value = "/login/view", method = RequestMethod.GET)
public ModelAndView view(HttpServletRequest req, HttpServletResponse res) {
return UserSession.getUserFrom(req.getSession())
.map(user -> {
LOGGER.info("logged in {}", user.getAccount());
View view = new JspView("redirect:/index.jsp");
return new ModelAndView(view);
})
.orElse(new ModelAndView(new JspView("/login.jsp")));
}

Choose a reason for hiding this comment

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

아래의 POST /login요청에서 쓰고있는

if (UserSession.isLoggedIn(req.getSession())) {
    View view = new JspView("redirect:/index.jsp");
    return new ModelAndView(view);
}

를 사용한다면, 가독성이 좋아지고 통일성이 생길 것 같아요!


@RequestMapping(value = "/login", method = RequestMethod.POST)
public ModelAndView login(HttpServletRequest req, HttpServletResponse res) {
if (UserSession.isLoggedIn(req.getSession())) {
return "redirect:/index.jsp";
View view = new JspView("redirect:/index.jsp");
return new ModelAndView(view);
}
Copy link
Author

Choose a reason for hiding this comment

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

/login/login/view
prefix url이 같으니, 하나의 컨트롤러로 합쳤어요.


return InMemoryUserRepository.findByAccount(req.getParameter("account"))
.map(user -> {
log.info("User : {}", user);
return login(req, user);
})
.orElse("redirect:/401.jsp");
.map(user -> {
LOGGER.info("User : {}", user);
return new ModelAndView(loginCheck(req, user));
})
.orElse(new ModelAndView(
new JspView("redirect:/401.jsp")
));
}

private String login(HttpServletRequest request, User user) {
private View loginCheck(HttpServletRequest request, User user) {
if (user.checkPassword(request.getParameter("password"))) {
final HttpSession session = request.getSession();
session.setAttribute(UserSession.SESSION_KEY, user);
return "redirect:/index.jsp";
return new JspView("redirect:/index.jsp");
} else {
return "redirect:/401.jsp";
return new JspView("redirect:/401.jsp");
}
}
}

This file was deleted.

18 changes: 13 additions & 5 deletions app/src/main/java/com/techcourse/controller/LogoutController.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
package com.techcourse.controller;

import com.techcourse.domain.UserSession;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
import nextstep.mvc.controller.asis.Controller;
import nextstep.mvc.view.JspView;
import nextstep.mvc.view.ModelAndView;
import nextstep.mvc.view.View;
import nextstep.web.annotation.Controller;
import nextstep.web.annotation.RequestMapping;
import nextstep.web.support.RequestMethod;

public class LogoutController implements Controller {
@Controller
public class LogoutController {

@Override
public String execute(HttpServletRequest req, HttpServletResponse res) throws Exception {
@RequestMapping(value = "/logout", method = RequestMethod.GET)
public ModelAndView logout(HttpServletRequest req, HttpServletResponse res) throws Exception {
final HttpSession session = req.getSession();
session.removeAttribute(UserSession.SESSION_KEY);
return "redirect:/";
View view = new JspView("redirect:/");
return new ModelAndView(view);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.techcourse.controller;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import nextstep.mvc.view.JsonView;
import nextstep.mvc.view.ModelAndView;
import nextstep.web.annotation.Controller;
import nextstep.web.annotation.RequestMapping;
import nextstep.web.support.RequestMethod;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@Controller
public class MultiInputController {

private static final Logger LOGGER = LoggerFactory.getLogger(MultiInputController.class);

@RequestMapping(value = "/api/multi", method = RequestMethod.GET)
public ModelAndView show(HttpServletRequest request, HttpServletResponse response) {
String input1 = request.getParameter("input1");
String input2 = request.getParameter("input2");

LOGGER.debug("input1 : {}", input1);
LOGGER.debug("input2 : {}", input2);

ModelAndView modelAndView = new ModelAndView(new JsonView());

modelAndView.addObject("input1", input1);
modelAndView.addObject("input2", input2);
return modelAndView;
}
}
Comment on lines +13 to +32
Copy link
Author

Choose a reason for hiding this comment

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

2개 입력에 대한 테스트가 용이하도록, 컨트롤러를 추가해주었어요.

Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@

@Controller
public class RegisterController {

Choose a reason for hiding this comment

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

이미 로그인이 된 사용자라면, GET /register/ivew, POST /register 요청을 했을 때 /index.jsp 로 redirect 하도록 할 수도 있을 것 같아요.

LoginControllerTestController는 메서드가 GET, POST 순으로 있는데, 여기에는 POST, GET 순으로 되어있네요.
여기도 GET, POST 순으로 통일시키면 어떨까요?


@RequestMapping(value = "/register", method = RequestMethod.POST)
public ModelAndView save(final HttpServletRequest req, final HttpServletResponse res) {
User user = new User(2,
req.getParameter("account"),
req.getParameter("password"),
req.getParameter("email"));
req.getParameter("email")
);
InMemoryUserRepository.save(user);

return new ModelAndView(new JspView("redirect:/index.jsp"));
Expand Down
32 changes: 32 additions & 0 deletions app/src/main/java/com/techcourse/controller/UserController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.techcourse.controller;

import com.techcourse.domain.User;
import com.techcourse.repository.InMemoryUserRepository;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import nextstep.mvc.view.JsonView;
import nextstep.mvc.view.ModelAndView;
import nextstep.web.annotation.Controller;
import nextstep.web.annotation.RequestMapping;
import nextstep.web.support.RequestMethod;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@Controller
public class UserController {

private static final Logger LOGGER = LoggerFactory.getLogger(UserController.class);

@RequestMapping(value = "/api/user", method = RequestMethod.GET)
public ModelAndView show(HttpServletRequest request, HttpServletResponse response) {
final String account = request.getParameter("account");
LOGGER.debug("user id : {}", account);

final ModelAndView modelAndView = new ModelAndView(new JsonView());
final User user = InMemoryUserRepository.findByAccount(account)
.orElseThrow();

modelAndView.addObject("user", user);
return modelAndView;
}
}
Comment on lines +15 to +32

Choose a reason for hiding this comment

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

final ModelAndView modelAndView = new ModelAndView(new JsonView());
modelAndView.addObject("user", user);
return modelAndView;

이렇게 ModelAndView 사용 부분을 모아두면 가독성이 더 좋아질 것 같아요.

final User user = InMemoryUserRepository.findByAccount(account)
    .orElseThrow();

orElseThrow();에 특정 Exception을 메세지를 포함해 던지도록 하면 에러 핸들링, 디버깅 등에 더 유리할 것 같아요. :)

Copy link
Author

@unluckyjung unluckyjung Sep 17, 2021

Choose a reason for hiding this comment

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

제가 작성한 코드가 아니고, 요구조건에서 주어진 class라서 그냥 곧이 그대로 가져왔었네요. ㅎㅎ;;

수정 하도록할게용

17 changes: 12 additions & 5 deletions app/src/main/java/com/techcourse/domain/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,17 @@ public String getAccount() {
@Override
public String toString() {
return "User{" +
"id=" + id +
", account='" + account + '\'' +
", email='" + email + '\'' +
", password='" + password + '\'' +
'}';
"id=" + id +
", account='" + account + '\'' +
", email='" + email + '\'' +
'}';
}

public long getId() {
return id;
}

public String getEmail() {
return email;
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.techcourse.controller;
package com.techcourse.domain;

import com.techcourse.domain.User;
import jakarta.servlet.http.HttpSession;

import java.util.Optional;

public class UserSession {
Expand All @@ -18,5 +16,6 @@ public static boolean isLoggedIn(HttpSession session) {
return getUserFrom(session).isPresent();
}

private UserSession() {}
private UserSession() {
}

Choose a reason for hiding this comment

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

private 생성자의 접근제어자가 private 이다 보니까
자동포맷팅 시에 public 메서드를 위에, private 메서드를 아래에 두면서 이렇게 밑으로 내려가더라구요.

생성자니까 멤버 변수 아래, 메서드 위에 두는게 어떨까요?

Copy link
Author

@unluckyjung unluckyjung Sep 17, 2021

Choose a reason for hiding this comment

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

이부분은 자동 포매팅 때문에 종종 놓치게 되네요 ㅋㅋ;

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.techcourse.repository;

import com.techcourse.domain.User;

import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
Expand All @@ -23,5 +22,6 @@ public static Optional<User> findByAccount(String account) {
return Optional.ofNullable(database.get(account));
}

private InMemoryUserRepository() {}
private InMemoryUserRepository() {
}
}
Loading