-
Notifications
You must be signed in to change notification settings - Fork 305
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
[MVC 구현하기 - 1단계] 에버(손채영) 미션 제출합니다. #694
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
df6fa40
test: 학습테스트 Reflection
helenason cb172db
feat: 핸들러 매핑 로직 추가
helenason 6e4c0e8
feat: 뷰 로직 JspView로 위임
helenason f0cda4b
refactor: 핸들러 매핑 reflection 사용하도록
helenason 2ecca46
feat: RequestMapping method 선언하지 않은 경우 모든 HTTP method 지원
helenason e312664
test: 유효하지 않은 요청의 경우 예외 발생
helenason 4e45a35
refactor: 핸들 로직 reflection 사용하도록
helenason e76390e
refactor: DispatcherServlet service 메서드 ModelAndView를 다루도록
helenason aa674d8
style: 불필요한 import문 제거
helenason d4d274b
refactor: HandlerKey 캐싱
helenason 0118167
refactor: HandlerExecution 내에서 매번 인스턴스 생성하지 않도록
helenason ba17506
test: EnumSource 활용하여 파라미터 주입
helenason de7370e
test: getMethods vs getDeclaredMethods 학습테스트 작성
helenason 613a0c1
style: 재정의 메서드 순서 및 구조 통일
helenason 5b297ce
style: EOF
helenason 54f3b36
test: 학습테스트 Servlet
helenason fdd3078
test: 학습테스트 Filter
helenason 8ace1d8
test: 포함 여부 확인 로직 수정
helenason 6afede4
style: 불필요한 주석 제거
helenason 279fc92
test: 동등성 대신 동일성 테스트하도록 및 테스트를 위한 비즈니스 코드 제거
helenason 21fcb37
refactor: HandlerKey 캐시 자료구조 변경
helenason File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 10 additions & 1 deletion
11
mvc/src/main/java/com/interface21/web/bind/annotation/RequestMethod.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,14 @@ | ||
package com.interface21.web.bind.annotation; | ||
|
||
import java.util.Arrays; | ||
|
||
public enum RequestMethod { | ||
GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE | ||
GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE; | ||
|
||
public static RequestMethod find(String raw) { | ||
return Arrays.stream(values()) | ||
.filter(value -> value.name().equals(raw)) | ||
.findAny() | ||
.orElseThrow(() -> new IllegalArgumentException(String.format("유효하지 않은 요청 메서드입니다: %s", raw))); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
18 changes: 16 additions & 2 deletions
18
mvc/src/main/java/com/interface21/webmvc/servlet/mvc/tobe/HandlerExecution.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,26 @@ | ||
package com.interface21.webmvc.servlet.mvc.tobe; | ||
|
||
import com.interface21.webmvc.servlet.ModelAndView; | ||
import jakarta.servlet.http.HttpServletRequest; | ||
import jakarta.servlet.http.HttpServletResponse; | ||
import com.interface21.webmvc.servlet.ModelAndView; | ||
import java.lang.reflect.Method; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class HandlerExecution { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(HandlerExecution.class); | ||
|
||
private final Object controller; | ||
private final Method handler; | ||
|
||
public HandlerExecution(Object controller, Method handler) { | ||
this.controller = controller; | ||
this.handler = handler; | ||
} | ||
|
||
public ModelAndView handle(final HttpServletRequest request, final HttpServletResponse response) throws Exception { | ||
return null; | ||
log.info("handler execution handle"); | ||
return (ModelAndView) handler.invoke(controller, request, response); | ||
} | ||
} |
48 changes: 32 additions & 16 deletions
48
mvc/src/main/java/com/interface21/webmvc/servlet/mvc/tobe/HandlerKey.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,53 @@ | ||
package com.interface21.webmvc.servlet.mvc.tobe; | ||
|
||
import com.interface21.web.bind.annotation.RequestMethod; | ||
|
||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
|
||
public class HandlerKey { | ||
|
||
private final String url; | ||
private final RequestMethod requestMethod; | ||
private static final Map<String, HandlerKey> CACHE = new ConcurrentHashMap<>(); | ||
|
||
public HandlerKey(final String url, final RequestMethod requestMethod) { | ||
this.url = url; | ||
this.requestMethod = requestMethod; | ||
public final String url; | ||
public final RequestMethod requestMethod; | ||
|
||
public static HandlerKey from(String url, RequestMethod requestMethod) { | ||
String key = url + requestMethod.name(); | ||
if (CACHE.containsKey(key)) { | ||
return CACHE.get(key); | ||
} | ||
return new HandlerKey(url, requestMethod); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "HandlerKey{" + | ||
"url='" + url + '\'' + | ||
", requestMethod=" + requestMethod + | ||
'}'; | ||
private HandlerKey(final String url, final RequestMethod requestMethod) { | ||
this.url = url; | ||
this.requestMethod = requestMethod; | ||
CACHE.put(url + requestMethod.name(), this); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) return true; | ||
if (!(o instanceof HandlerKey)) return false; | ||
HandlerKey that = (HandlerKey) o; | ||
return Objects.equals(url, that.url) && requestMethod == that.requestMethod; | ||
if (this == o) { | ||
return true; | ||
} | ||
if (!(o instanceof HandlerKey)) { | ||
return false; | ||
} | ||
HandlerKey handlerKey = (HandlerKey) o; | ||
return Objects.equals(url, handlerKey.url) && requestMethod == handlerKey.requestMethod; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(url, requestMethod); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "HandlerKey{" + | ||
"url='" + url + '\'' + | ||
", requestMethod=" + requestMethod + | ||
'}'; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
mvc/src/test/java/com/interface21/web/bind/annotation/RequestMethodTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package com.interface21.web.bind.annotation; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class RequestMethodTest { | ||
|
||
@DisplayName("전달받은 문자열과 형태가 같은 요청 메서드를 반환한다.") | ||
@Test | ||
void should_findRequestMethod_when_givenValidString() { | ||
// given | ||
String raw = "GET"; | ||
|
||
// when | ||
RequestMethod requestMethod = RequestMethod.find(raw); | ||
|
||
// then | ||
assertThat(requestMethod).isEqualTo(RequestMethod.GET); | ||
} | ||
|
||
@DisplayName("전달받은 문자열이 유효하지 않은 경우 예외가 발생한다.") | ||
@Test | ||
void should_throwException_when_givenInvalidString() { | ||
// given | ||
String raw = "INVALID"; | ||
|
||
// when & then | ||
assertThatThrownBy(() -> RequestMethod.find(raw)) | ||
.isInstanceOf(IllegalArgumentException.class) | ||
.hasMessage("유효하지 않은 요청 메서드입니다: " + raw); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
HandlerKey도 Handler인스턴스처럼 요청마다 새로운 인스턴스가 생성될 것 같은데, 개선해보면 좋을 것 같습니다:)
(저도 제 리뷰어한테 받았던 리뷰랍니다 ㅎㅎ)
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.
좋은 생각이에요! 하나의 서비스에서 HandlerKey가 필드로 갖는 url과 requestMethod는 한정적이기 때문에 캐싱을 적용해 중복 객체를 생성하지 않도록 하였습니다.
d4d274b
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.
[질문] 캐싱 로직을 테스트하기 위해 HandlerKeyTest 테스트 코드를 작성했는데요! private으로 생성된 CACHE 데이터를 확인하기 위해서는 테스트만을 위한 메서드를 생성하거나, reflection을 활용하여 접근해야 했어요. 리니는 둘 중 어느 방법이 더 나은 방법이라 생각하시나요? 혹은 캐싱 로직을 테스트하기 위한 좋은 방법을 알고 계시나요? 궁금합니다! 😊
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.
테스트를 위한 메서드가 production에 생기는 것보단, reflection을 활용하는 것이 더 좋아보여요! assertJ의
extract
메서드도 reflection으로 구현되어있다는 점을 생각한다면 reflection으로 검증하는 것도 괜찮다고 생각합니다!혹은 검증하고자 하는 것이
이미 존재한다면 있는 인스턴스를 반환하고, 아니라면 새로운 인스턴스를 생성한다
라면 다음과 같이 테스트를 작성해볼 수도 있을 것 같아요!(저도 이 부분은 테스트 없이 넘어갔었는데, 에버 덕분에 발견했습니다 ㅎㅎ)
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.
오 두 객체의 동일성을 판단하는 메서드가 제공되는 줄 몰랐네요! isSameAs 활용해서 테스트 진행해야겠어요.
279fc92