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

[톰캣 구현하기 - 3, 4단계] 케로(김지현) 미션 제출합니다. #480

Merged
merged 13 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
4 changes: 2 additions & 2 deletions study/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ handlebars:
server:
tomcat:
accept-count: 1
max-connections: 1
max-connections: 2
threads:
max: 2
max: 5
compression:
enabled: true
min-response-size: 10
2 changes: 1 addition & 1 deletion study/src/test/java/thread/stage0/SynchronizationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private static final class SynchronizedMethods {

private int sum = 0;

public void calculate() {
public synchronized void calculate() {
setSum(getSum() + 1);
}

Expand Down
6 changes: 3 additions & 3 deletions study/src/test/java/thread/stage0/ThreadPoolsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ void testNewFixedThreadPool() {
executor.submit(logWithSleep("hello fixed thread pools"));

// 올바른 값으로 바꿔서 테스트를 통과시키자.
final int expectedPoolSize = 0;
final int expectedQueueSize = 0;
final int expectedPoolSize = 2;
final int expectedQueueSize = 1;

assertThat(expectedPoolSize).isEqualTo(executor.getPoolSize());
assertThat(expectedQueueSize).isEqualTo(executor.getQueue().size());
Expand All @@ -46,7 +46,7 @@ void testNewCachedThreadPool() {
executor.submit(logWithSleep("hello cached thread pools"));

// 올바른 값으로 바꿔서 테스트를 통과시키자.
final int expectedPoolSize = 0;
final int expectedPoolSize = 3;
final int expectedQueueSize = 0;

assertThat(expectedPoolSize).isEqualTo(executor.getPoolSize());
Expand Down
9 changes: 0 additions & 9 deletions tomcat/src/main/java/nextstep/jwp/controller/Controller.java

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package nextstep.jwp.controller;

import org.apache.coyote.request.Request;
import org.apache.coyote.response.ResponseEntity;
import org.apache.coyote.response.ResponseStatus;
import org.apache.coyote.request.HttpRequest;
import org.apache.coyote.response.HttpResponse;
import org.apache.front.AbstractController;

public class HelloWorldController implements Controller{
public class HelloWorldController extends AbstractController {

@Override
public ResponseEntity handle(final Request request) {
return ResponseEntity.fromString(request, "Hello world!", ResponseStatus.OK);
protected void doGet(final HttpRequest request, final HttpResponse response) {
response.setStringAsBody("Hello world!");
}

Choose a reason for hiding this comment

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

HelloWorldController에 doPost() 요청이 들어오면 어떻게 되나요?.?

Copy link
Author

Choose a reason for hiding this comment

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

피알 날리고 궁금해서 해봤는데 NPE 가 발생하더라고여...
이부분도 처리 처리되도록 수정하였습니다 !

}
72 changes: 36 additions & 36 deletions tomcat/src/main/java/nextstep/jwp/controller/LoginController.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,74 +4,74 @@
import nextstep.jwp.exception.MemberNotFoundException;
import nextstep.jwp.model.User;
import org.apache.coyote.request.Cookie;
import org.apache.coyote.request.Request;
import org.apache.coyote.response.ResponseEntity;
import org.apache.coyote.request.HttpRequest;
import org.apache.coyote.response.HttpResponse;
import org.apache.coyote.response.ResponseStatus;
import org.apache.exception.MethodMappingFailException;
import org.apache.exception.PageRedirectException;
import org.apache.front.AbstractController;
import org.apache.session.Session;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Optional;

public class LoginController implements Controller {
public class LoginController extends AbstractController {

private static final Logger log = LoggerFactory.getLogger(LoginController.class);
public static final String ACCOUNT_KEY = "account";
public static final String PASSWORD_KEY = "password";
private static final Logger log = LoggerFactory.getLogger(LoginController.class);

@Override
public ResponseEntity handle(final Request request) {
if (request.isPost()) {
return login(request);
}
if (request.isGet() && request.hasQueryString()) {
return loginInConsole(request);
}
if (request.isGet()) {
return loginPage(request);
protected void doPost(final HttpRequest request, final HttpResponse response) {
login(request, response);
}

@Override
protected void doGet(final HttpRequest request, final HttpResponse response) {
if (request.hasQueryString()) {
loginInConsole(request, response);
return;
}
throw new MethodMappingFailException();
loginPage(request, response);
}

private ResponseEntity loginInConsole(final Request request) {
final String account = request.getQueryValueBy(ACCOUNT_KEY);
final String password = request.getQueryValueBy(PASSWORD_KEY);
private void loginInConsole(final HttpRequest httpRequest, final HttpResponse httpResponse) {
final String account = httpRequest.getQueryValueBy(ACCOUNT_KEY);
final String password = httpRequest.getQueryValueBy(PASSWORD_KEY);

final User user = InMemoryUserRepository.findByAccount(account)
.orElseThrow(MemberNotFoundException::new);
if (user.checkPassword(password)) {
log.info("user : {}", user);
}
return ResponseEntity.fromViewPath(request.httpVersion(), request.getPath(), ResponseStatus.OK);
httpResponse.setViewPathAsBody(httpRequest.getPath());
}

private ResponseEntity login(final Request request) {
final String account = request.getBodyValue(ACCOUNT_KEY);
final String password = request.getBodyValue(PASSWORD_KEY);
private void login(final HttpRequest httpRequest, final HttpResponse httpResponse) {
final String account = httpRequest.getBodyValue(ACCOUNT_KEY);
final String password = httpRequest.getBodyValue(PASSWORD_KEY);
final User user = InMemoryUserRepository.findByAccount(account)
.orElseThrow(() -> new PageRedirectException.Unauthorized(request.httpVersion()));
.orElseThrow(() -> new PageRedirectException.Unauthorized(httpResponse));

if (user.checkPassword(password)) {
final Session session = request.getSession(false);
final Session session = httpRequest.getSession(false);
session.setAttribute("user", user);
final ResponseEntity responseEntity = ResponseEntity.fromViewPath(request.httpVersion(), request.getPath(), ResponseStatus.MOVED_TEMP);
responseEntity.setRedirect("/index.html");
responseEntity.addCookie(Cookie.ofJSessionId(session.getId()));
return responseEntity;
httpResponse.setViewPathAsBodyAndSetStatus(httpRequest.getPath(), ResponseStatus.MOVED_TEMP);
httpResponse.setRedirect("/index.html");
httpResponse.addCookie(Cookie.ofJSessionId(session.getId()));
return;

Choose a reason for hiding this comment

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

로그인에 성공하면 302 헤더와 함께 index.html 로 redirect 하도록 응답을 보내는 것 같아요!
요 상황에서 serViewPathAsBody 를 따로 설정하신 이유가 있나용?.?

Copy link
Author

Choose a reason for hiding this comment

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

헉 그러게요...
필요없이 response에 login.html을 담고 있었네여...
수정했습니다

}
throw new PageRedirectException.Unauthorized(request.httpVersion());
throw new PageRedirectException.Unauthorized(httpResponse);
}

private ResponseEntity loginPage(final Request request) {
final Session session = request.getSession(false);
private void loginPage(final HttpRequest httpRequest, final HttpResponse httpResponse) {
final Session session = httpRequest.getSession(false);

Choose a reason for hiding this comment

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

getSession(boolean) 에 들어가는 파라미터가 어떤 의미를 가지는 값인지 알 수 있을까요?.?

Copy link
Author

Choose a reason for hiding this comment

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

getSession의 메서드 파라미터 명을 수정하여 다시 작성했습니다.
도 실제 request session에서 처럼 true 일 때 기존 세션이 존재하지 않으면 새로 생성하고, false에서 존재하지 않으면 null을 반환하도록 수정했습니다

final Optional<Object> user = session.getAttribute("user");
if(user.isPresent()){
final ResponseEntity responseEntity = ResponseEntity.fromViewPath(request.httpVersion(), request.getPath(), ResponseStatus.MOVED_TEMP);
responseEntity.setRedirect("/index.html");
return responseEntity;
if (user.isPresent()) {
httpResponse.setViewPathAsBodyAndSetStatus(httpRequest.getPath(), ResponseStatus.MOVED_TEMP);
httpResponse.setRedirect("/index.html");
return;

Choose a reason for hiding this comment

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

59번 라인의 코멘트와 동일합니다!

}
return ResponseEntity.fromViewPath(request.httpVersion(), request.getPath(), ResponseStatus.OK);
httpResponse.setViewPathAsBody(httpRequest.getPath());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,47 +3,46 @@
import nextstep.jwp.db.InMemoryUserRepository;
import nextstep.jwp.exception.DuplicationMemberException;
import nextstep.jwp.model.User;
import org.apache.coyote.request.Request;
import org.apache.coyote.response.ResponseEntity;
import org.apache.coyote.request.HttpRequest;
import org.apache.coyote.response.HttpResponse;
import org.apache.coyote.response.ResponseStatus;
import org.apache.exception.MethodMappingFailException;
import org.apache.front.AbstractController;

import java.util.Optional;

public class RegisterController implements Controller {
public class RegisterController extends AbstractController {

public static final String ACCOUNT_KEY = "account";
public static final String PASSWORD_KEY = "password";
public static final String EMAIL_KEY = "email";

@Override
public ResponseEntity handle(final Request request) {
if (request.isPost()) {
return join(request);
}
if (request.isGet()) {
return joinPage(request);
}
throw new MethodMappingFailException();
protected void doPost(final HttpRequest request, final HttpResponse response) {
join(request, response);
}

@Override
protected void doGet(final HttpRequest request, final HttpResponse response) {
joinPage(request, response);
}

private ResponseEntity join(final Request request) {
final String account = request.getBodyValue(ACCOUNT_KEY);
final String password = request.getBodyValue(PASSWORD_KEY);
final String email = request.getBodyValue(EMAIL_KEY);
private void join(final HttpRequest httpRequest, final HttpResponse httpResponse) {
final String account = httpRequest.getBodyValue(ACCOUNT_KEY);
final String password = httpRequest.getBodyValue(PASSWORD_KEY);
final String email = httpRequest.getBodyValue(EMAIL_KEY);

final Optional<User> user = InMemoryUserRepository.findByAccount(account);
if (user.isPresent()) {
throw new DuplicationMemberException();
}
final User newUser = new User(account, password, email);
InMemoryUserRepository.save(newUser);
final ResponseEntity responseEntity = ResponseEntity.fromViewPath(request.httpVersion(), request.getPath(), ResponseStatus.MOVED_TEMP);
responseEntity.setRedirect("/index.html");
return responseEntity;

httpResponse.setViewPathAsBodyAndSetStatus(httpRequest.getPath(), ResponseStatus.MOVED_TEMP);
httpResponse.setRedirect("/index.html");

Choose a reason for hiding this comment

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

위의 코멘트와 동일합니다 😄

}

private ResponseEntity joinPage(final Request request) {
return ResponseEntity.fromViewPath(request.httpVersion(), request.getPath(), ResponseStatus.OK);
private void joinPage(final HttpRequest httpRequest, final HttpResponse httpResponse) {
httpResponse.setViewPathAsBody(httpRequest.getPath());
}
}
15 changes: 11 additions & 4 deletions tomcat/src/main/java/org/apache/catalina/connector/Connector.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,30 @@
import java.io.UncheckedIOException;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class Connector implements Runnable {

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

private static final int DEFAULT_PORT = 8080;
private static final int DEFAULT_ACCEPT_COUNT = 100;
private static final int DEFAULT_ACCEPT_COUNT = 30;
private static final int DEFAULT_MAX_THREAD = 30;
Comment on lines +19 to +20

Choose a reason for hiding this comment

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

이렇게 제한을 두신 이유가 궁금하네용 ㅎㅎ 케로가 코멘트에 말씀해주신 것 처럼 스레드에 대해서 더 공부하시게 된다면... 저도 알려주세용 (@^0^@)/

Copy link
Author

Choose a reason for hiding this comment

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

일단 로컬에서 돌리고 있기 때문에 많은 스레드를 둘 필요가 없다고 판단해서 처음에 10개를 사용하였는데, ci에서 테스트가 깨져서 스레드 풀 개수 때문일가?? 하고 30까지 수정했던것 입니다....ㅎㅅㅎ

아직도 잘 모르겠어요 ...ㅠㅠ

Choose a reason for hiding this comment

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

사실 저도 잘 모르겠어용 ㅋㅋㅋㅋ 근데 지금 미션 상황에서는 케로 말씀대로 많은 accept count와 thread가 필요하지 않을 것 같아요!
(공부한 내용 공유해달라고 꼬드기는 코멘트 였습니당 ㅎㅋㅎㅎ)


private final ServerSocket serverSocket;

private final ExecutorService executorService;
private boolean stopped;

public Connector() {
this(DEFAULT_PORT, DEFAULT_ACCEPT_COUNT);
this(DEFAULT_PORT, DEFAULT_ACCEPT_COUNT, DEFAULT_MAX_THREAD);
}

public Connector(final int port, final int acceptCount) {
public Connector(final int port, final int acceptCount, final int maxThreads) {
this.serverSocket = createServerSocket(port, acceptCount);
this.stopped = false;
this.executorService = Executors.newFixedThreadPool(maxThreads);
}

private ServerSocket createServerSocket(final int port, final int acceptCount) {
Expand Down Expand Up @@ -67,13 +73,14 @@ private void process(final Socket connection) {
return;
}
var processor = new Http11Processor(connection);
new Thread(processor).start();
executorService.execute(processor);
}

public void stop() {
stopped = true;
try {
serverSocket.close();
executorService.shutdown();
} catch (IOException e) {
log.error(e.getMessage(), e);
}
Expand Down
18 changes: 9 additions & 9 deletions tomcat/src/main/java/org/apache/coyote/http11/FileUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import nextstep.jwp.exception.UncheckedServletException;
import org.apache.coyote.common.FileType;
import org.apache.coyote.common.HttpVersion;
import org.apache.coyote.common.PathUrl;
import org.apache.coyote.response.HttpResponse;
import org.apache.exception.PageRedirectException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -25,18 +25,18 @@ private FileUtil() {
throw new IllegalStateException("Utility class");
}

public static String getResource(final HttpVersion httpVersion, final PathUrl resourceUrl){
public static String getResource(final HttpResponse httpResponse, final PathUrl resourceUrl) {
final URL resource = FileUtil.class.getClassLoader().getResource("static" + resourceUrl);
if(Objects.isNull(resource)){
throw new PageRedirectException.PageNotFound(httpVersion);
if (Objects.isNull(resource)) {
throw new PageRedirectException.PageNotFound(httpResponse);
}
return readResource(resource);
}

public static String getResourceFromViewPath(final HttpVersion httpVersion, final String viewPath){
public static String getResourceFromViewPath(final HttpResponse httpResponse, final String viewPath) {
final URL resource = FileUtil.class.getClassLoader().getResource("static" + viewPath + FileType.HTML.getExtension());
if(Objects.isNull(resource)){
throw new PageRedirectException.PageNotFound(httpVersion);
if (Objects.isNull(resource)) {
throw new PageRedirectException.PageNotFound(httpResponse);
}
return readResource(resource);
}
Expand All @@ -45,8 +45,8 @@ private static String readResource(final URL resource) {
final Path path = Paths.get(resource.getPath());
try (final BufferedReader fileReader = new BufferedReader(new FileReader(path.toFile()))) {
return fileReader.lines()
.collect(Collectors.joining(System.lineSeparator()))
+ System.lineSeparator();
.collect(Collectors.joining("\r\n"))
+ "\r\n";
} catch (IOException | UncheckedServletException e) {
log.error(e.getMessage(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

import nextstep.jwp.exception.UncheckedServletException;
import org.apache.coyote.Processor;
import org.apache.coyote.request.Request;
import org.apache.coyote.request.HttpRequest;
import org.apache.coyote.request.RequestReader;
import org.apache.coyote.response.ResponseEntity;
import org.apache.coyote.response.HttpResponse;
import org.apache.front.Proxy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -40,16 +40,17 @@ public void process(final Socket connection) {
final var outputStream = connection.getOutputStream();
final BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(inputStream))
) {
final Request request = Request.from(RequestReader.from(bufferedReader));
final ResponseEntity responseEntity = proxy.process(request);
writeResponse(outputStream, responseEntity);
final HttpRequest httpRequest = HttpRequest.from(RequestReader.from(bufferedReader));
final HttpResponse httpResponse = HttpResponse.create(httpRequest.httpVersion());
proxy.process(httpRequest, httpResponse);
writeResponse(outputStream, httpResponse);
} catch (IOException | UncheckedServletException e) {
log.error(e.getMessage(), e);
}
}

private void writeResponse(final OutputStream outputStream, final ResponseEntity responseEntity) throws IOException {
outputStream.write(responseEntity.toString().getBytes());
private void writeResponse(final OutputStream outputStream, final HttpResponse httpResponse) throws IOException {
outputStream.write(httpResponse.toString().getBytes());
outputStream.flush();
}
}
Expand Down
Loading