[톰캣 구현하기 1단계] 우르(김현우) 미션 제출합니다 #346
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
| public static String detect(final String resourcePath) throws IOException { | ||
| final URL resource = classLoader.getResource(resourcePath); | ||
|
|
||
| if (resource == null) { | ||
| throw new NoSuchElementException("해당 resource는 존재하지 않습니다."); |
There was a problem hiding this comment.
File I/O에 대한 예외처리를 별도로 분리해주셨네요.
관심사 분리도 잘 되고, 안전해서 좋은 아이디어인 것 같습니다.
| public interface DynamicHandler extends Handler{ | ||
|
|
||
| } |
There was a problem hiding this comment.
Handler 인터페이스를 상속하는 DynamicHandler 인터페이스를 별도로 생성해주셨네요.
OCP를 지키기 위함일까요?
이 구조를 만든 의도가 궁금합니다!
There was a problem hiding this comment.
따로 로직을 처리하는 요청은 DynamicHandler,
정적인 페이지만 처리하는 요청은 StaticHandler로 나눠보았습니다
| @Override | ||
| public String handle(final HttpRequest httpRequest) { | ||
| return handlers.stream() | ||
| .filter(it -> it.canHandle(httpRequest)) | ||
| .map(it -> it.safeHandle(httpRequest)) | ||
| .findAny() | ||
| .orElseThrow(() -> new IllegalArgumentException("handler가 존재하지 않습니다.")); | ||
| } |
There was a problem hiding this comment.
Composite 패턴을 사용해 확장이 용이하면서도, 응집도 높은 구조를 만들어주셨네요 👍
대응되는 핸들러를 찾고, 사용하는 방법이 한 곳에서 관리되는 구조라 굉장히 편리할 것 같습니다.
|
|
||
| private final Socket connection; | ||
| private final Socket connection; | ||
| private final HandlerComposite handlerComposite; |
There was a problem hiding this comment.
Handler를 구현하는 HandlerComposite를 만드셔서 유연한 구조를 가져가셨는데,
Http11Processor에서는 '꼭' HandlerComposite만 사용하도록 구체 타입을 명시해주셨네요!
우르가 의도해서 설정해두신 걸까요?
There was a problem hiding this comment.
오,, 그러네요 이 부분은 Handler 인터페이스로 수정했습니다 !
|
|
||
| List<String> result = new ArrayList<>(); | ||
| String line; | ||
| while ((line = bufferedReader.readLine()) != null && !line.isEmpty()) { |
There was a problem hiding this comment.
(트러블슈팅하다가 새로 알게된 점이 있어 공유드립니다)
소켓 환경에서 readLine()을 사용할 때 블로킹 관련해 주의할 점이 조금 있더라구요
노션 에 정리해봤는데 궁금하시면 봐주세요 ㅎㅎ
There was a problem hiding this comment.
서버는 개행문자를 기다리는 일종의 교착 상태가 생긴다.
기다린다는 의미는 애플리케이션 상에서 파싱할 때까지 로직을 처리하는 것이고, 브라우저는 이 로직을 처리할 때까지 기다린다는 의미인건가요?
종료되지 않는 연결이 많아지면
브라우저 입장에서 서버가 응답값을 주지 않는 경우인 건가요?
주어진 InputStream은 네트워크를 통해 전송되는 바이트들을 받으므로, 블락될 여지가 남아있다.
"블락될 여지가 남아있다"라는 부분이 잘 이해 못해서,,(제가 네트워크 지식이 좀 딸립니다요) 혹시 자세하게 설명 부탁드려도 될까여?
소켓에 타임아웃을 설정
그러면 지금 미션에 비유하면 10초동안 제가 입력 값(HTTP Request)가 없으면 자동으로 애플리케이션이 꺼지는건가요?
There was a problem hiding this comment.
서버는 개행문자를 기다리는 일종의 교착 상태가 생긴다.
기다린다는 의미는 애플리케이션 상에서 파싱할 때까지 로직을 처리하는 것이고, 브라우저는 이 로직을 처리할 때까지 기다린다는 의미인건가요?
맞습니다!
서버 어플리케이션은 readLine()에서 멈춰있으므로 응답이 나가지 않을 것이고, 브라우저는 계속해서 그 응답을 기다릴거라는 뜻입니다.
There was a problem hiding this comment.
종료되지 않는 연결이 많아지면
브라우저 입장에서 서버가 응답값을 주지 않는 경우인 건가요?
맞습니다! 브라우저 입장에서 응답을 못받았으므로, TCP 소켓 연결을 종료하지 않고 계속 응답을 기다린다는 의미입니다.
There was a problem hiding this comment.
주어진 InputStream은 네트워크를 통해 전송되는 바이트들을 받으므로, 블락될 여지가 남아있다.
"블락될 여지가 남아있다"라는 부분이 잘 이해 못해서,,(제가 네트워크 지식이 좀 딸립니다요) 혹시 자세하게 설명 부탁드려도 될까여?
우리는 블로킹 방식으로 스트림을 읽고 있습니다.
그래서 스트림을 읽는 게 느려지거나 멈추면 그 후의 파싱 작업이나 응답 전송도 같이 느려지고 멈추게 됩니다.
이런 일은 블로킹 IO를 하는 이상 피할 수 없으므로 최소한의 안전 확보를 위해 소켓에 타임아웃을 걸자는 의도였습니다.
(논블로킹 IO도 있다는데 저는 아직 이해하기가 어렵네요 ㅎㅎ;)
There was a problem hiding this comment.
소켓에 타임아웃을 설정
그러면 지금 미션에 비유하면 10초동안 제가 입력 값(HTTP Request)가 없으면 자동으로 애플리케이션이 꺼지는건가요?
조금 의도가 잘못 전달된 것 같습니다. 일정시간동안 요청이 없으면 어플리케이션을 종료하는 것은 아닙니다.
우리가 요청을 읽기 위해서 소켓의 InputStream을 통해 IO를 하는데, 이 작업이 10초 이상 블로킹되면 연결을 종료한다는 뜻입니다.
setSoTimeout()메서드 설명 보시면 이해가 빠르실거에요!
서버 입장에서 생각했을 때
브라우저 소켓 연결을 받아주고, 읽기 작업을 했는데, 데이터가 10초 넘게 도착을 안하면 뭔가 문제가 있다고 생각해 볼 수 있습니다.
(네트워크 지연을 생각해볼 수 있을 것 같아요, 요청 전달 과정에 문제가 생긴거죠!)
그 때 더 이상 지체하지 않고, 그 브라우저와의 소켓 연결을 종료하겠다는 뜻입니다.
| default String safeHandle(final HttpRequest httpRequest) { | ||
| try { | ||
| return handle(httpRequest); | ||
| } catch (IOException e) { | ||
| throw new IllegalArgumentException("I/O 작업 관련 에러가 발생했습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
어떤 책(자바의 신)에서는 'default 메서드는 하위 호환성을 위한 것이다' 라고 하더라구요.
맞는 말이지만, 다른 용도로도 쓰일 수 있는 것 같아요.
우르가 작성해주신 코드처럼, 인터페이스에 메서드의 기본 동작을 지정하기 위해 사용해도 오용은 아니라고 생각해요.
safeHandle() default 메서드가 없다면,
기본 동작을 지정하기 위해 Handler를 구현하는 추상 클래스가 추가적으로 필요하겠죠.
하지만 default 메서드가 있어 불필요한 추상화 단계가 하나 생략되어 읽기가 더 편한 것 같습니다.
우르가 default 메서드를 작성할 때 마음에 걸리는 부분은 없었나요?
There was a problem hiding this comment.
제 의도를 정확히 파악해주셨네여 ㅋㅋㅋ 역시,,,
처음에 설계할 때는 handle 메서드에서 IOException 을 던지지 않았는데, 코드를 작성하다보니 던져야하겠더라구요.
공통적으로 에러 핸들링이 필요했고, 이 때문에 추상 클래스를 만들고 싶지 않았습니다.
추상 클래스는 부모 클래스와의 강결합으로 변경에 대한 사이드 이펙트가 너무 크다고 생각합니다.
최대한 느슨하게 만들고 싶었기 때문에 인터페이스로 구현했어요.
그리고 제가 생각하는 비즈니스 로직이 없는 경우에 한해서는 default 메서드로 처리해도 되겠다라는 생각을 했어요.
솔직히 default 메서드가 등장하게 된 배경에 맞지 않게 사용했지만 공통 로직도 삭제하면서 느슨한 결합을 유지하는 것은 이 방법이라 생각하고 작성했습니다.
안녕하세요 땡칠 !
이번 미션을 진행하면서 2단계는 조금 더 학습이 필요하다고 생각이 들어서 1단계만 제출해보겠습니다,,
리뷰 받으면서 테스트랑 기능을 천천히 만들어가볼게요!
감사합니다~