-
Notifications
You must be signed in to change notification settings - Fork 311
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
[1단계] 애쉬 미션 제출합니다! #556
[1단계] 애쉬 미션 제출합니다! #556
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 애쉬~ 우선 학습 테스트에서 생각해볼만 한 내용을 던져두고 가겠습니다 ㅎㅎ 리뷰 마무리되는대로 다시 RC 드릴게요! 팀프로젝트랑 같이 달리느라 너무 고생했어요~~
try (outputStream) { | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try-with-resources 👍🏻
final String actual = ""; | ||
|
||
inputStream.read(bytes); | ||
final String actual = new String(bytes, "UTF-8"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StandardCharsets.UTF_8
라는 상수가 있으니, 컴파일 단계에서 잡을 수 있도록 해도 좋겠네요 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존 코드는 런타임 시점에서 에러가 발생하는 반면
StandardCharsets.UTF_8
를 사용하면
컴파일 시점에서 해당 상수가 존재하는지 자바가 체킹하기 때문에 더 빠르게 에러를 잡을 수 있군요~! 👍
@@ -149,13 +159,14 @@ class InputStream_학습_테스트 { | |||
* java 9 이상에서는 변수를 try-with-resources로 처리할 수 있다. | |||
*/ | |||
|
|||
inputStream.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오마이갓 읽을 생각만 하고 닫진 않았는데 꼼꼼하게 잘 닫았네요 ㅋㅋ
@@ -169,12 +180,13 @@ class FilterStream_학습_테스트 { | |||
* 버퍼 크기를 지정하지 않으면 버퍼의 기본 사이즈는 얼마일까? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
몇 가지 질문을 남겨두고 가겠습니다 ㅎㅎ
- 왜 한 바이트씩 읽어들이는 것보다 덩어리로 가져오는 것이 더 빠를까요?
- (주석에 적힌 질문) 버퍼의 기본 사이즈는 얼마이고, 이를 관리하는 것은 누구의 책임인가요?
- 버퍼 크기와 속도는 항상 비례할까요? 왜 그럴까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
관련하여 알고 있는 / 새로 공부한 내용을 간단히 정리해보았어요~
혹시 사실과 다른 부분이 있다면 이야기해주세요!
왜 한 바이트씩 읽어들이는 것보다 덩어리로 가져오는 것이 더 빠를까요?
'읽어 들이는' 과정은 I/O 호출을 필요로 합니다.
그런데 이 I/O 호출은 코스트가 큽니다. (시간이 오래 걸림)
따라서 이 작업을 덜 호출하면 덜 호출할 수록 코스트가 덜 들겠죠? ( = 더 빠르다)
이 경우 바이트 단위로 매번 이 작업을 호출하기보다는 보다 커다란 단위를 한 번에 처리하는 게 더 효율적인 방식이 됩니다.
버퍼의 기본 사이즈는 얼마이고, 이를 관리하는 것은 누구의 책임인가요?
이건 이번에 공부했던 내용인데, BufferedInputStream
의 기본 버퍼 사이즈는 8kb(=8192 bytes) 라고 하네요!
그러한 기본 사이즈는 자바 라이브러리에 설정되어 있지만
BufferedInputStream bufferedInputStream = new BufferedInputStream(inputStream, 12345);
이처럼 사용자가 임의로 설정해줄 수도 있다고 합니다.
버퍼 크기와 속도는 항상 비례할까요? 왜 그럴까요?
항상
이라는 말은 항상 위험하죠~ 😗
앞서 말한 것처럼 바이트 단위보다 큰 chunk로 읽어오게 되면 발생하는 이점이 분명한 만큼,
어느 시점까지는 버퍼 크기가 커지면 커질 수록 이득이라고 생각합니다.
하지만 커다란 버퍼가 무조건 좋다면 권장 버퍼 크기가 8kb~64kb로 제한되지 않겠죵.
버퍼의 크기가 일정 크기를 넘어가면
- 메모리에 부담이 가고 (메모리 부족 초래, 메모리의 캐시 사이즈를 오버해 캐시 미스 발생)
- 디스크 I/O 속도나 네트워크 대역폭과 같은 하드웨어적 한계가 있음
위와 같은 이유로 영원히 성능이 향상되기만 하는 건 아닌 것 같습니다.
뭐든 적절한 게 중요한 것 같아요 🔥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
디스크는 한 바이트씩 읽어오는 것이 불가능합니다. 디스크의 단위는 바이트가 아니기 때문이예요. 어차피 몇십-몇백 바이트씩 불러올 것이라면 버퍼를 사용하지 않을 이유가 없기도 하죠 👍🏻 항상 어느 정도가 적정선인지는 많은 경험을 통해 다져질 것이라고 생각해요.
import org.springframework.stereotype.Component; | ||
import org.springframework.web.servlet.HandlerInterceptor; | ||
|
||
@Component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인터셉터는 최초 한 번만 생성하면 되므로, 다음과 같은 두 가지 방법으로 등록할 수 있어요.
- 애쉬가 진행해준 대로,
@Component
를 붙이고 설정 파일에서 의존성 주입 - 어노테이션을 붙이지 않고,
new
연산자를 통해 객체를 직접 생성
애쉬는 왜 전자를 택했나요? (그냥 궁금증이예요~)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
애쉬는 왜 전자를 택했나요?
진짜진짜 솔직하게 말하면 이유는 별로 생각 안 해봤읍니다.
그냥 그 동안 계속 그렇게 해왔고, 우테코에서 그렇게 배웠고, 그런 방식이 제게 가장 익숙하기 때문이 아니었을까요?
그래서 이번 기회에 두 개가 어떻게 다른지 생각을 조금 해봤어요!
@Component
사용
이 방식은 스프링의 IoC 컨테이너가 해당 객체를 관리해요.
장점:
- 의존성 주입: 인터셉터에서 서비스나 레포지토리 같은 다른 Bean을 주입받아 사용 가능 (객체 간 결합도가 낮고 재사용성이 높음)
- 유지 보수 용이: 스프링이 객체 생명 주기를 관리
- 테스트 용이성: 의존성 주입을 통해 Mock 객체나 테스트 전용 Bean을 쉽게 주입 가능
단점:
- 스프링 컨텍스트 필요: 스프링 애플리케이션 컨텍스트가 실행되어야만 객체가 생성되고 관리됨
new
연산자로 직접 생성
이 방식은 개발자가 해당 객체를 직접 관리해요.
장점과 단점은 앞서 말한 @Component
를 사용했을 때 생기는 장점과 단점을 각각 뒤집어 놓은 결과겠고요.
이 중에서 당장 와닿는 장점은 역시 스프링이 해당 객체를 자동으로 관리해준다는 점인 것 같습니다.
저희가 @Controller
, @Service
를 자연스럽게 사용하는 것처럼요. 🤔
우선은 이처럼 정리해볼 수 있을 것 같은데 아루는 어떻게 생각하시나요?
아루는 어떤 방식을 선호하고 또 그 이유가 뭐지요? ㅇ.ㅇ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저라면 @Bean
등록할 때 new
로 생성할 것 같습니다. 그 이유로는:
- IoC 컨테이너가 지속적으로 의존성을 추적하면서 관리할 필요가 없습니다.
- 한 곳에서밖에 사용하지 않을 것이 명확합니다.
테스트가 조금 어려워질 수도 있겠지만, 다른 방식으로도 충분히 테스트할 수 있으리라 생각되네요 ㅎㅎ
chain: | ||
strategy: | ||
content: | ||
enabled: true | ||
paths: /js/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이쪽 설정은 어떻게 추가하게 되었나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void testCacheBustingOfStaticResources()
학습 테스트를 진행하다가 추가된 설정입니다.
"%s/%s/js/index.js"
에 해당하는 URI 처리를 어떻게 해야할 지 고민하는 과정에서 이것저것 건드리며 넣어본 설정인데요.
결론적으로는 필요하지 않게 된... 그런 내력이에요 ^__^... 지우는 걸 잊었네요.
삭제 처리했습니다! (테스트 무사 통과하는 것 확인했습니다.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 애쉬~ 레벨 4 들어오면서 난이도있는 미션 진행하느라 고생하셨습니다 😎
1단계만 진행해서 코드에 대한 리뷰도 남겼지만, 추가적으로 생각하면 좋을 이야기들을 담아 보았어요. 특히 IOStream, 큰 파일 읽기, 리팩터링할 때 고민해볼 점 등을 적어봤습니다.
이해되지 않는 부분이 있다면 편하게 언제든 DM주세요~ 함께 배워보자고요 🤛🏻 변경 요구사항 반영하시면 다시 리뷰 요청 주세요. 단계가 많이 남아 빠르게 어프루브 진행하겠습니다 😁 고생하셨어요!
final String[] httpRequestLine = line.split(" "); | ||
final String method = httpRequestLine[0]; | ||
final String requestURL = httpRequestLine[1]; | ||
|
||
String contentType; | ||
byte[] responseBody; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 친구들 하나의 클래스로 모아도 되겠는데요~~~ 😎 Request가 될 조짐이 보이긴 하죠?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 값들을 필드로 가지고 있는 클래스가 HttpRequest
가 되면 좋겠네요! 😗
이 부분은 3단계 리팩토링하면서 반영해보도록 하겠습니다! 💪
URL resource = getClass().getResource(resourcePath); | ||
|
||
if (resource == null) { | ||
responseBody = "404 Not Found".getBytes(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Responsebody가 404를 내려주지만, 실제 코드로는 200이 내려가는 것으로 보여요. 어떻게 하면 좋을까요? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Responsebody
가 404 NOT FOUND인 경우
실제 페이지도 404 NOT FOUND 응답 + 404.html 페이지를 반환하도록 수정하였습니다.
log.error(e.getMessage(), e); | ||
} | ||
} | ||
|
||
private String determineResourcePath(String requestURL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개인적으로 함수가 private이 된다면, 다른 객체로의 분리를 고려해볼 필요가 있다고 생각해요. 정말 딱 '다른 객체의 책임이지 않을까?' 라는 생각을 해 봅시다!
아래에 수많은 책임을 가지는 private 메서드들이 있어요. 어떤 메서드는 ContentType을 찾아주기도 하고, 어떤 메서드는 우리가 짜던 컨트롤러의 역할을 하기도 합니다. 쿼리 파라미터를 뽑아주는 역할도 하고요.
Tomcat은 서블릿 컨테이너이면서 웹 서버의 역할을 가집니다. 서블릿 중 하나로 많이 들어본 DispatcherServlet이 있어요. Springboot web 모듈에서는 Tomcat의 컨테이너에 DispatcherServlet을 넣어두도록 하고, 외부의 요청을 톰캣이 받아낼 때 적절한 서블릿을 골라 위임하게끔 동작해요.
그렇다면 각각을 역할에 따라 분리할 수 있습니다.
- Request를 읽는 역할
- 404, 200 등을 전달하는 역할
- 사용자의 아이디, 비밀번호를 받아 일치 여부를 확인
- 쿼리 파라미터를 추출
- ...
리팩터링을 하면서 충분히 시간을 가지고 고민해보아도 좋겠습니다. 갈피 잡는 것이 어렵다면 DM주세요~!~!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아직 서블릿 컨테이너나 웹 서버 관련 지식이 부족해서 완전히 세부적인 그림까지 그려지지는 않지만,
그래도 아루의 코멘트를 읽고 나니 3단계 요구사항에 적혀 있는 HttpRequest/Response/Controller 같은 기능 분리가 어떤 방향으로 이루어져야 할 지는 대충 감이 잡히는 느낌이에요! 🥹 이 부분은 조금 더 공부해보고 3단계 진행하면서 천천히 이해해나가보도록 하겠습니다.
도중 이해가 안 가는 부분이 있다면 부담 없이 (?) DM 드릴게요~~
감사합니다!
String account = userInfo.get("account"); | ||
String password = userInfo.get("password"); | ||
|
||
InMemoryUserRepository.findByAccount(account).ifPresent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional
에도 stream
과 마찬가지로 filter
가 있습니다. filter
를 거친 이후에 isPresent
로 확인해보는 것도 하나의 방법이겠네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 로직이 filter
를 사용하도록 리팩토링하였습니다.
+) 계정 정보가 존재하지 않는 경우 분기를 추가 분리하였습니다.
(전자: 그래프 노출되지 않음) | ||
(후자: 그래프 노출됨) | ||
|
||
정확한 이유가 뭘까요? 🤔 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 문제 해결되었나요? 현재 애쉬가 적어준 코드가 Git에 어느 부분인지를 확실히 모르겠어서 다시 재현하기 어렵네요. 제 생각에는 두 개가 같을 것이라고 생각되는데 🤔 getResource
의 인자로 같은 값이 들어갈 것이라고 생각되어요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1-1단계를 구현하면서 생겼던 이슈라 코드의 구조가 많이 변화한 지금에서는 쉽게 재현되지가 않네요.
저도 두 개가 같을 거라고 생각하고 진행했는데 실제 디스플레이 되는 화면이 다르게 출력되어 궁금했던 부분이에요. 🤔
지금도 완벽하게 방법을 알고 에러를 핸들링했다기보다는 코드의 구조가 변화되며 재현이 안되는 얼렁뚱땅 해결(?)에 가까워서
조금 더 명확한 해결법이 뭐였을지, 더 정확히는 문제점이 뭐였을지 궁금한 상태인데요.
일단 이 부분은 일단 차차 구현하면서 같은 문제가 재현되는 걸 발견하게 되면 다시 한 번 질문 드리거나,
저만의 대답을 내어놓도록 하겠습니다.
살펴봐주셔서 감사해요~
responseBody = "404 Not Found".getBytes(); | ||
contentType = "text/plain"; | ||
} else { | ||
responseBody = Files.readAllBytes(new File(resource.getFile()).toPath()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고민해볼 점: 1G 램을 가지고 있는 컴퓨터에서 이 프로그램을 실행했다고 합시다.
3G 크기의 영화를 다운로드받고자 해요. 이 경우에, readAllBytes
는 파일을 모두 메모리(램)에 올리게 돼 OutOfMemoryError
가 발생할 것이라고 생각됩니다. 어떻게 하면 이를 해결할 수 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 경우는 Files.readAllBytes
로 모든 내용을 한 번에 읽어오고 메모리에 올리는 것보다는
스트림을 활용해서 바이트를 chunk(e.g. 8kb~) 단위로 스트리밍해주는 게 좋을 것 같아용. 🤔
그리고 나서 한 번에 outputStream.flush()
같은 메서드로 네트워크에 한 번에 전송하게 되면 파일은 통째로 전송되지만,
메모리에 올라갈 때는 보다 작은 단위로 올라가기 때문에 OutOfMemoryError
를 피할 수 있지 않을까 싶네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 최고예요 👍🏻
안녕하세요 아루~ 당장 반영이 필요한 부분은 반영하고, 3단계 리팩토링 때까지 공부와 함께 고민해볼 내용은 보류해두었어요. 추가적으로 주실 피드백이 있다면 부담 없이 재 rc 주셔도 괜찮으니 편하게 진행해주셔요 ^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
애쉬 고생했어요~! 이번 미션에서 배울 점들은 대부분 챙겨갔다고 생각되네요 😄 어려웠을 수도 있을 질문들 공부하느라 고생 많으셨습니다! 남은 미션도 잘 달려서 제출해주세요! 🔥
안녕하세요 아루~~~ 🙇🏻♀️
이번 미션 페어로 만나뵙게 된 애쉬입니다!
리뷰 잘 부탁 드려요~!!
제 이번 미션 목표는 끔찍한 코드 제출해서 아루를 깜짝 놀라게하기!고요 ㅎㅎ...
디엠에서 말씀주셨던 대로 1단계씩 진행해서 리뷰 부탁 드려 보려고 합니다.
주말에는 리뷰 억지로 안 해주셔도 되고 영업일 기준 24시간 이내로만 주시면 감사하겠습니다! 헤헤
현재 status
진짜 아무것도 모름. 정말 아무것도 모름. 상태고요.
서버 구현 경험은 고사하고 관련 CS 지식도 거의 전멸하다시피 한 상태라 너무 어렵네용.
기본 지식이 거의 없고, 기초부터 공부를 시작해야 하는 단계라고 생각해주시면 될 것 같아요.
그래도 새로운 내용 공부하니까 재미있네용. 컨디션 점수 나쁘지 않습니다.
이번 미션 진행
하나의 클래스 내부에서 할 수 있는 만큼 최대한 역할이 분리된 메소드를 작성해보고자 노력했는데 잘 되었는지는 모르겠네요.
우선은 최대한 요구사항만이라도 만족하는 코드를 최종 목표로 미션 진행했습니다.
이번 코드에서도 리팩토링 쪽은 아직 건드리지 않아서, 거의 모든 코드가 Http11Processor에 들어가있어요.
(깜짝 놀라셨죠? ㅎㅎ...)
클래스 분리와 같은 본격적인 리팩토링은 3단계에 들어간 이후에 진행해보려고 하는데 어떻게 생각하시나요?
추가로 미션하면서 혼자서는 답을 찾지 못했던 부분에 대한 의문을 README에 간단히 정리해두었는데요.
너무 간단하게 적어둬서 이해가 안 가실 수도 있을 것 같아... 관련해서 추가 질문이 있으시다면 코멘트나 디엠으로 부탁드립니다!
받고 싶은 피드백이 있다면
같은 부분이 있다면 들어보고 싶어요! 👍
기타
부족한 부분이 많을 것 같은데 어떤 지적이든 주시면 감사히 받고 성장의 기회로 삼아볼 테니
이건 진짜 엄청 사소한 부분 같은데... 싶은 것도 편하게 말씀해주세요!
급하게 PR 작성 중이라 내용이 중구난방이네요...☺️
어쨌든 이번 미션 잘 부탁드립니다!!! 우선 저는 배워갈 게 너무 많을 거 같아서 너무 기대돼용. 아루가 리뷰어라서 기분 짱입니다.
저라는 폭탄과 함께하는 아루도... 어쨌거나 미션 화이팅입니다!