-
Notifications
You must be signed in to change notification settings - Fork 90
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
[보스독] 4단계 - HTTP 웹 서버 리팩토링 미션 제출합니다. #187
Open
yeonnseok
wants to merge
10
commits into
woowacourse:yeonnseok
Choose a base branch
from
yeonnseok:bossdog
base: yeonnseok
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
867e493
docs: 요구사항 업데이트
yeonnseok 2243e0f
refactor: 멀티모듈로 분리
yeonnseok f8110e1
refactor: gradle 의존성 수정
yeonnseok 6116942
refactor: JSESSIONID 하드코딩 상수화
yeonnseok f7d1322
refactor: 의미없는 쿠키 검증 제거
yeonnseok 94968c5
refactor: 한 요청에 대해서 쿠키와 response 싱크 맞추기
yeonnseok e242adf
refactor: 공통 gradle 설정 root level로 추출
yeonnseok 83c9081
refactor: web-common 이름 web-domain으로 변경
yeonnseok 4054c34
refactor: web-session 모듈 분리
yeonnseok ae4f028
docs: 각 모듈의 README.md 파일 작성
yeonnseok 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
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,25 +1,58 @@ | ||
buildscript { | ||
ext { | ||
springBootVersion = '2.2.2.RELEASE' | ||
} | ||
repositories { | ||
mavenCentral() | ||
} | ||
dependencies { | ||
classpath("org.springframework.boot:spring-boot-gradle-plugin:${springBootVersion}") | ||
} | ||
} | ||
|
||
plugins { | ||
id 'java' | ||
id 'idea' | ||
id 'eclipse' | ||
} | ||
|
||
version = '1.0.0' | ||
sourceCompatibility = 1.8 | ||
allprojects { | ||
version = '1.0.0' | ||
} | ||
|
||
subprojects { | ||
apply plugin: 'java' | ||
apply plugin: 'idea' | ||
apply plugin: 'eclipse' | ||
|
||
sourceCompatibility = 1.8 | ||
|
||
repositories { | ||
mavenCentral() | ||
} | ||
|
||
dependencies { | ||
implementation 'ch.qos.logback:logback-classic:1.2.3' | ||
testImplementation 'org.junit.jupiter:junit-jupiter:5.6.2' | ||
testImplementation 'org.assertj:assertj-core:3.16.1' | ||
} | ||
|
||
repositories { | ||
mavenCentral() | ||
test { | ||
useJUnitPlatform() | ||
} | ||
} | ||
|
||
dependencies { | ||
implementation 'com.google.guava:guava:29.0-jre' | ||
implementation 'ch.qos.logback:logback-classic:1.2.3' | ||
implementation 'com.github.jknack:handlebars:4.2.0' | ||
implementation 'org.springframework:spring-core:5.2.6.RELEASE' | ||
testImplementation 'org.junit.jupiter:junit-jupiter:5.6.2' | ||
testImplementation 'org.assertj:assertj-core:3.16.1' | ||
project(':web-server') { | ||
dependencies { | ||
compile project(':web-domain') | ||
compile project(':web-session') | ||
} | ||
} | ||
|
||
test { | ||
useJUnitPlatform() | ||
project(':service-app') { | ||
dependencies { | ||
compile project(':web-domain') | ||
compile project(':web-server') | ||
compile project(':web-session') | ||
} | ||
} |
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,4 @@ | ||
## service-app module | ||
- 웹 애플리케이션을 수행하기 위한 표현계층과 응용계층 영속계층이 정의된다. (현재 응용계층은 존재하지 않는다.) | ||
- web-domain, web-server 모듈을 참조하고 있다. | ||
- UrlAppender에서 enum으로 정의된 url주소와 Controller를 한 쌍으로 지정하고, web-server에서 제공하는 RequestMapper에 내용이 반영된다. |
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,3 @@ | ||
dependencies { | ||
implementation 'com.google.guava:guava:29.0-jre' | ||
} |
File renamed without changes.
14 changes: 12 additions & 2 deletions
14
.../web/controller/CreateUserController.java → ...java/interfaces/CreateUserController.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
5 changes: 3 additions & 2 deletions
5
.../java/web/controller/LoginController.java → ...main/java/interfaces/LoginController.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
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
28 changes: 28 additions & 0 deletions
28
service-app/src/main/java/interfaces/urlmapper/UrlAppender.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,28 @@ | ||
package interfaces.urlmapper; | ||
|
||
import java.util.Arrays; | ||
|
||
import interfaces.CreateUserController; | ||
import interfaces.LoginController; | ||
import interfaces.UserListController; | ||
import web.controller.Controller; | ||
import webserver.RequestMapping; | ||
|
||
public enum UrlAppender { | ||
CREATE_USER_CONTROLLER("/user/create", new CreateUserController()), | ||
LOGIN_CONTROLLER("/user/login", new LoginController()), | ||
USER_LIST_CONTROLLER("/user/list", new UserListController()); | ||
|
||
private final String url; | ||
private final Controller controller; | ||
|
||
UrlAppender(String url, Controller controller) { | ||
this.url = url; | ||
this.controller = controller; | ||
} | ||
|
||
static { | ||
Arrays.stream(values()) | ||
.forEach(v -> RequestMapping.addUrl(v.url, v.controller)); | ||
} | ||
} |
File renamed without changes.
File renamed without changes.
File renamed without changes.
Empty file.
File renamed without changes
Empty file.
Empty file.
Empty file.
File renamed without changes
File renamed without changes.
File renamed without changes.
File renamed without changes.
Empty file.
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
File renamed without changes.
File renamed without changes.
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
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
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
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
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
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
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
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
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
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.
모듈 잘 분리해주셨네요! 피드백은 이쪽에 작성하겠습니다 :)
전체 프로젝트에 적용되어야 할 의존성을
common
모듈을 사용하셨네요. root level 에build.gradle
을 정의하여 전체 프로젝트에 적용되어야 할 설정을 작성해보는 것은 어떨까요?web-server
모듈을 완전한 라이브러리로 분리해보시는 것은 어떨까요? 그동안 구현하셨던 웹서버를 어떤 어플리케이션 비지니스에도 녹일 수 있도록 작성하셨을 것이라고 생각합니다. 그러나 현재의web-server
모듈은 비지니스를 포함하고 있는 것 처럼 보이네요 :) 어떤 어플리케이션에서도 해당 라이브러리를 사용할 수 있도록 조금 더 분리해보시길 바랍니다.common
이란 네이밍은 다소 위험함이 있습니다.common
이라는 추상적인 네이밍은 네이밍이 가진 추상적인 의미 때문에 어떠한 코드도 들어갈 수 있습니다. 그래서 되도록이면 명시적인 네이밍을 작성할 것을 추천합니다. 실제로 추상적인 네이밍으로 어마무시한 슈퍼 모듈이 만들어지고 이 슈퍼 모듈이 스파게티 의존을 만들어내는 경우들을 많이 보아왔습니다.각 모듈은 본인이 작성한 의도가 있긴 할텐데요. 그것을 다른 사람이 보았을 때도 명확히 이해하기 힘들 수 있습니다. 아무리 명시적으로 하려고 해도 추상적인 의미를 갖는 네이밍을 갖기도 할텐데요.
README.md
를 각 모듈별로 작성하여, 해당 모듈의 책임과 역할이 어느 범위인지를 명시해주면 좋을 것 같습니다 :)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.
킹뽀대님! 이번에도 섬세한 리뷰 감사합니다.!
다른 부분은 어느정도 제가 이해를 한 것 같아 반영해보았습니다.
다만 2번에서 web-server모듈을 완전한 라이브러리로 분리하는 피드백을 반영하려다보니, 비즈니스 로직을 포함하고 있는 부분을 캐치하는게 쉽지 않았습니다. ㅠㅠ 그나마 Session에 관한 내용이 요구사항으로 인해 적용되었던 점을 감안해서 Session부분을 다른 모듈로 분리해보았는데요. 의도하신 부분이 이게 맞는지는 잘 모르겠습니다.
또한 Session을 분리하고 있다고 해도 ReuqestHandler등에서 이미 HttpSession을 사용하고 있던터라, 완전한 라이브러리화는 안되고 있는 것 같은데요, 이 부분에 대해서 조금만 더 힌트를 주시면 감사드리겠습니다!