[Team-07, K] baseball 기능 구현 및 배포 by kihyuk-sung · Pull Request #29 · codesquad-members-2021/baseball

kihyuk-sung · 2021-05-12T10:36:16Z

안녕하세요 백엔드의 K입니다.

배포주소

게임 시작점

http://3.36.239.71/games

api 형태에 대한 문서

http://3.36.239.71/apidocs/

게임 진행

curl -X POST http://3.36.239.71/games/1/pitch

현재 진행한 것은 다음과 같습니다.

aws 에 VPC public / private subnet 설정 후 , 인터넷 게이트웨이 설정
EC2와 RDS 사용
야구 게임 구현
oauth 후 jwt 토큰 발행
임시로 팀을 추가하거나, 팀을 이용하여 게임 추가

Home하고 Away는 기능이 같은데, db 필드 매핑을 위해서 다른 객체로 추가하고, 중복코드가 발생했습니다.

리뷰 잘 부탁드립니다.

- 깃 커밋 컨벤션 추가

- 브랜치 전략 추가

- 팀원 추가

- 프로젝트 초기화 - gitignore.io에서 설정 추가 issue: #1

- Json Serialize Test code 추가 issue: #2

- 완전히 같은지 확인 issue: #2

- 테스트코드 포함 issue: #2

- GET /games issue: #2

- Spring REST Docs를 이용하여 테스트코드 작성 issue: #2

issue:

- api를 위한 REST Docs issue: #2

- Empty Objects about BaseballGame issue: #2

- Team을 TeamRepository에 저장하고 가져오기 기능 테스트 issue: #2

- MySQL하고 호환되도록 함 issue: #2

- 게임 생성시의 초기 내용을 포함한 객체 추가 issue: #2

- BaseballGame 객체를 디비에 저장하고, 가져옵니다. issue: #2

issue: #2

- 팀과 게임을 이용하여 Mock up api 반환 issue: #2

- wrapper로 game 추가 - Spring Rest Docs 테스트 추가 issue: #2

[BE] Mock API

- 던질 때 마다 Inning 진행 issue: #12

- BaseballGame과 Team을 이용하여 구성하도록 수정 issue: #12

- BaseballGameTeamInformation에 id 추가 이유 주석 추가 - 포매팅 issue: #12

- 상대팀의 Enum을 가져오는 기능 issue: #12

- hit 시에 게임 진행 issue: #12

- 안타 시에 점수 증가 issue: #12

- 볼 - 볼 넷 issue: #12

- Out 기능 구현 - Out 시에 선수 기록 추가 - 3아웃 시 공수 교체 issue: #12

- 스트라이크 처리 - 삼진 아웃 issue: #12

- N + 1 문제 해결을 위해, BaseballGame에서 팀 정보를 갖도록 수정 - BaseballGameTitle 수정 완료 issue: #12

- Team 없이, BaseballGame 만을 이용해서 View 구성 issue: #12

- 서비스에서 게임들을 가져오도록 수정 issue: #12

- 중복되는 Embedded 적용 안되서, 새로 설계 issue: #12

- 게임 하나를 가져올 수 있습니다. issue: #12

- OAuth 테스트 용 issue: #12

- 공 던지기 가능 (post) issue: #12

[BE] 게임 기능 구현

- 타자가 바뀔 때, 한번 멈춥니다. issue: #12

- 게임이 끝나면, 승자를 추가 issue: #12

- Winner가 null이면, 끝나지 않은 게임 - 끝나지 않은 게임만 가져옵니다. issue: #12

- 게임이 9회 이후에 조건에 따라 끝납니다. - 승자를 보여줍니다. issue: #12

- 테스트용으로 Get 사용했던 것을 Post로 수정 issue: #12

- 게임이 끝나면 저장하지 않습니다. issue: #12

[BE] 야구 게임 추가 기능

- mysql 연결을 위한 세팅 추가 - 게임과 팀을 추가하기 위한 엔드포인트 추가 issue: #18

- 테스트용으로 GET으로 했던것 수정 issue: #18

[BE] MySQL

wheejuni

인터페이스를 적극적으로 도입한 시도는 좋았지만 인터페이스를 통해 타입을 설계할 때는 명확한 목적 의식이 있어야 합니다.
다형성을 최대한 살리면서 객체에게 동작을 위임하고, 특히 instanceof 연산자의 사용 없이 로직 수행이 가능한지, 불필요한 캐스팅이나 조건문 로직이 발생하지 않는지를 점검하며 간결한 코드를 구현하는 것을 목적으로 해야 합니다.
그런 면에서 바라본다면 지금의 인터페이스 분리는 다소 아쉬운 감이 있습니다.

그러나 정말 좋은 시도인 것 같구요. 💯 앞으로도 이런 시도를 많이 해 보시면 좋겠습니다.
코드에 대한 제 의견을 남겨 보았습니다. 확인해주세요. 👍

wheejuni · 2021-05-13T00:25:45Z

BE/src/main/java/codesquad/team7/baseball/auth/AuthController.java

+    private Optional<User> getUserFromGitHub(GithubAccessTokenResponse accessToken, RestTemplate gitHubRequest) {
+        RequestEntity<Void> request = RequestEntity
+                .get(GITHUB_USER_URI)
+                .header("Accept", "application/json")
+                .header("Authorization", "token " + accessToken.getAccessToken())
+                .build();
+
+        ResponseEntity<User> response = gitHubRequest
+                .exchange(request, User.class);
+
+        return Optional.ofNullable(response.getBody());
+    }
+
+    private Optional<GithubAccessTokenResponse> getAccessToken(String code, RestTemplate gitHubRequest) {
+        RequestEntity<GithubAccessTokenRequest> request = RequestEntity
+                .post(GITHUB_ACCESS_TOKEN_URI)
+                .header("Accept", "application/json")
+                .body(new GithubAccessTokenRequest(CLIENT_ID, CLIENT_SECRET, code));
+
+        ResponseEntity<GithubAccessTokenResponse> response = gitHubRequest
+                .exchange(request, GithubAccessTokenResponse.class);
+
+        return Optional.ofNullable(response.getBody());
+    }


여기에 있어야 할 로직이 아니겠죠? 어떻게 해결할 수 있을까요?
우선은 별도의 Service 로 구성해야 할 것 같고, 추후 로그인되어 있는 유저의 정보를 컨트롤러 안에서 활용하려면,
컨트롤러에 진입하기 전에 요청에 로그인 정보가 묻어있어야 할 것 같습니다.
이런 방식의 구현을 위해 어떤 고민을 해야 할지 검색해보시는 것도 좋을 것 같습니다.

참고할만한 검색어

Filter

Interceptor

공부한다고 코드를 작성했는데, 서비스를 사용할 생각을 안했네요.

그리고 인증은 세션을 사용한다면, 로그인을 확인하고 유저 정보와 권한을 세션에 저장할 것입니다.

jwt를 이용한다면 토큰을 상대방에게 보내준 후, 이후 요청에서 Authorization: Bearer {token} 으로 토큰을 확인하는 형태로 구현할 수 있을 것 같습니다.

세션이나 jwt 토큰이나 중간에 Interceptor를 이용하여 httpServletRequest를 확인하고 인증과 권한이 있으면 컨트롤러로, 그렇지 않으면 거부하거나 인증을 요구하는 형태로 구현할 수 있을 것 같습니다.

wheejuni · 2021-05-13T00:26:47Z

BE/src/main/java/codesquad/team7/baseball/auth/AuthController.java

+    }
+
+    @GetMapping
+    public ResponseEntity<Jwt> auth(String code) {


인증에 관한 부분은 엄밀히 말해 API의 "응답" 이라고 보긴 어렵기때문에, 컨트롤러가 아닌 외부의 다른 곳에서 처리하는 곳이 원칙입니다.
컨트롤러 앞 단에 세워두고, 어떤 오리진을 통해 요청이 들어와도 응답해줄 수 있도록요.
이를 위해 어떤 구조를 가져가야 할지 다시 한 번 고민해보시면 좋겠습니다. (프로젝트가 끝나고 나서라도요.)

Bearer 타입의 토큰 인증은 토큰을 응답으로 보내주는 것까지도 표준인 것 같습니다.

https://datatracker.ietf.org/doc/html/rfc6750

Example Access Token Response
Typically, a bearer token is returned to the client as part of an
OAuth 2.0 [RFC6749] access token response. An example of such a
response is:

HTTP/1.1 200 OK Content-Type: application/json;charset=UTF-8 Cache-Control: no-store Pragma: no-cache { "access_token":"mF_9.B5f-4.1JqM", "token_type":"Bearer", "expires_in":3600, "refresh_token":"tGzv3JOkF0XG5Qx2TlKWIA" }

그리고 제가 생각한 jwt 인증 방식은 다음과 같습니다.

AuthController 에서 OAuth 인증이 정상적으로 처리되서 User 객체를 구성할 수 있으면, 로그인을 할 수 있다고 보고서 jwt 토큰에 정보를 담아 상대방에게 보내줍니다.

이후 상대방이 헤더에 Authorization: Bearer {token}으로 보내면, 인터셉터에서 토큰과 내부에 담긴 권한을 확인한 다음에 이를 유저 객체로 만들어 컨트롤러로 보내줄 수 있을 것 같습니다.

wheejuni · 2021-05-13T00:33:27Z

BE/src/main/java/codesquad/team7/baseball/controller/BaseballController.java

+    private BaseballGameView buildGameView(BaseballGame game) {
+        return new BaseballGameView.Builder(game).build();
+    }


이정도라면 Service 에 있어도 괜찮을 것 같습니다.

wheejuni · 2021-05-13T00:34:27Z

BE/src/main/java/codesquad/team7/baseball/controller/TeamController.java

+    @PostMapping("/dinos")
+    public Team addDinos() {


@PostMapping 인데 정작 아규먼트로 @RequestBody 를 받지 않는 것도 어색하고요,
안에 담긴 로직을 보니, 데이터 초기화성 로직인 것 같은데 이런 목적이라면 엔드포인트를 꾸리는 것보다는 CommandLineRunner 를 구성하는 것이 더 좋습니다.

여기는 잠시 테스트용으로 추가한 것인데, 나중에 시간이 되면 관리자용 페이지에 팀 추가 게임 추가 등을 해보려고 했습니다.
CommandLineRunner는 추가로 공부해보겠습니다.

wheejuni · 2021-05-13T00:34:49Z

BE/src/main/java/codesquad/team7/baseball/game/Away.java

+    private Integer awayScore;
+    private Integer awayBatterNumber;
+
+    Away(String awayUser,


private 으로 가려야 하겠네요.

codesquad.team7.baseball.game 패키지에 있는 모든 객체는 Spring Data jdbc를 이용하여 db에 저장하는 객체입니다.

https://docs.spring.io/spring-data/commons/docs/2.5.0/reference/html/#mapping.object-creation
https://docs.spring.io/spring-data/jdbc/docs/2.2.0/reference/html/#mapping.object-creation

여기에 객체 생성시에 reflection을 사용하지 않도록 하려면, Spring data에서 사용하는 생성자가 private가 되면 안된다고 되어있어, 기본 생성자로 두었습니다.

wheejuni · 2021-05-13T00:37:41Z

BE/src/main/java/codesquad/team7/baseball/game/HomePlayerStatistics.java

+
+import codesquad.team7.baseball.team.Player;
+
+public class HomePlayerStatistics extends PlayerStatistics {


여기서도 궁금합니다. HomePlayer 와 AwayPlayer 간에 어떤 차이점을 갖나요?

wheejuni · 2021-05-13T00:38:25Z

BE/src/main/java/codesquad/team7/baseball/game/HomePitcher.java

+    @Override
+    public Integer getNumber() {
+        return homePitcherNumber;
+    }
+
+    @Override
+    public Integer getPitches() {
+        return homePitches;
+    }
+
+    @Override
+    public void pitchUp() {
+        homePitches++;
+    }


변수명들이 달라진 것을 제외하곤, 어웨이 피쳐를 나타내는 클래스와 세부 구현에 있어 달라진 점이 전혀 없는 것 같습니다.
인터페이스로 분리한 이점을 제대로 살리지 못하는 것 같은데요, 조금 더 정교한 설계가 필요해 보입니다.

wheejuni · 2021-05-13T00:40:08Z

BE/src/main/java/codesquad/team7/baseball/game/Home.java

+    private final List<HomePlayerStatistics> playersStatistics;
+
+    @Embedded.Empty
+    private final HomePitcher homePitcher;


여기선 HomePitcher 를 받았지만 어차피 TeamInformation 에서 Pitcher 타입의 리턴을 갖기 때문에,
리턴이 나갈 때는 HomePitcher 가 아닌 Pitcher 로 나가야 하고, HomePitcher 와 AwayPitcher 사이에 유의미한 구현의 차이도 없기 때문에, 해당 메소드를 소비하는 측에서는 연산 결과값의 차이를 느끼기 어렵겠네요.
조금 아쉬운 설계입니다.

wheejuni · 2021-05-13T00:41:13Z

BE/src/main/java/codesquad/team7/baseball/game/BaseballGame.java

+        if (pitch == Pitch.HIT) {
+            hit(attack);
+            return;
+        }
+
+        if (pitch == Pitch.BALL) {
+            ball(attack);
+            return;
+        }
+
+        if (pitch == Pitch.OUT) {
+            out(attack);
+            return;
+        }
+
+        if (pitch == Pitch.STRIKE) {
+            strike(attack);
+        }


비슷한 로직이 네 번 반복되었는데 TeamInformation 타입 파라메터를 받는 메소드가 Pitch 상에 구현되어 있다면, 얼마나 코드가 깨끗해질까요?

Pitch 에 타입별로 동작을 기술하는 객체를 추가하고, pitch.do( ... )와 같이 진행하면 if문 없이 코드를 작성할 수 있을 것 같습니다. 다만, TeamInformation 뿐만 아니라 Inning 등도 상태가 변해서, 그에 맞게 사용하면 될것 같습니다. 그리고 그렇게 되면 내부에 private 메소드도 어쩌면 Pitch 내에서 사용하는 객체로 이동할 것 같습니다.

이런 식의 설계가 전략 패턴이나 템플릿 메소드 패턴 같은 것인가요?

wheejuni · 2021-05-13T00:42:19Z

BE/src/main/java/codesquad/team7/baseball/game/BaseballGame.java

+import java.util.ArrayList;
+import java.util.List;
+
+public class BaseballGame {


public 메소드는 private 메소드보다 위에 있는 것이 코드 편집에 좋겠죠. 시선이 분산되지 않으니까요.
그리고 private 메소드가 3개 이상 작성되어 있다면, 이것은 code smell로 판단하고 객체 분리나 설계 변경을 고려해볼 만 합니다.

네 알겠습니다.

kihyuk-sung · 2021-05-13T06:29:49Z

Home하고 Away와 관련 있는 객체는 db에 저장하기 위해서, 그리고 깊게 생각하지 못하고 작성하다 보니 코드 중복이 생기게 되었습니다.

정말 좋은 리뷰 감사드립니다.

kihyuk-sung added 30 commits May 3, 2021 14:40

docs: Add git commit convention

11dfe27

- 깃 커밋 컨벤션 추가

docs: Add branch strategy

af35331

- 브랜치 전략 추가

docs: Add teamates

dc890fd

- 팀원 추가

feat: Initialize Project

bed0cd9

- 프로젝트 초기화 - gitignore.io에서 설정 추가 issue: #1

feat: BaseballGameTitle

098d80c

- Json Serialize Test code 추가 issue: #2

test: Improve BaseballGameTitleTest

1230432

- 완전히 같은지 확인 issue: #2

feat: BaseballGames json

22c7488

- 테스트코드 포함 issue: #2

feat: BaseballGames mock-api

59ba7ed

- GET /games issue: #2

test: GET /games using Spring REST docs

5344c48

- Spring REST Docs를 이용하여 테스트코드 작성 issue: #2

fix: add gradle-wrapper.jar

bcfdfbc

issue:

docs: Spring REST Docs

405b78d

- api를 위한 REST Docs issue: #2

feat: the Prototype of BaseballGame

bf436f5

- Empty Objects about BaseballGame issue: #2

feat: Team and TeamRepository

fe8a340

- Team을 TeamRepository에 저장하고 가져오기 기능 테스트 issue: #2

test: 테스트용 h2 디비 설정

a082954

- MySQL하고 호환되도록 함 issue: #2

feat: BaseballGame Mock

84518eb

- 게임 생성시의 초기 내용을 포함한 객체 추가 issue: #2

test: BaseballGameRepositoryTest

5861e0b

- BaseballGame 객체를 디비에 저장하고, 가져옵니다. issue: #2

feat: Add user at game

78da95c

issue: #2

feat: MockApi View

5af8ec5

- 팀과 게임을 이용하여 Mock up api 반환 issue: #2

test: Add BaseballGameView test

b149954

- wrapper로 game 추가 - Spring Rest Docs 테스트 추가 issue: #2

Merge pull request #10 from PizzaCola-K/mock-api

b253076

[BE] Mock API

feat: BaseballGame Inning

3eca584

- 던질 때 마다 Inning 진행 issue: #12

feat: BaseballGameTitle

a72fcfb

- BaseballGame과 Team을 이용하여 구성하도록 수정 issue: #12

feat: BaseballGameTeamInformation

8ea6e31

- BaseballGameTeamInformation에 id 추가 이유 주석 추가 - 포매팅 issue: #12

feat: TeamEnum opposite Team

d2de841

- 상대팀의 Enum을 가져오는 기능 issue: #12

feat: BaseballGame Pitch Hit

2622e3b

- hit 시에 게임 진행 issue: #12

feat: Pitch Hit

2baa395

- 안타 시에 점수 증가 issue: #12

feat: BaseballGame ball

8c0f032

- 볼 - 볼 넷 issue: #12

feat: BaseballGame Out

84a6841

- Out 기능 구현 - Out 시에 선수 기록 추가 - 3아웃 시 공수 교체 issue: #12

feat: Baseball strike

4ddbc87

- 스트라이크 처리 - 삼진 아웃 issue: #12

feat: Move Team info into BaseballGame

5b9b835

- N + 1 문제 해결을 위해, BaseballGame에서 팀 정보를 갖도록 수정 - BaseballGameTitle 수정 완료 issue: #12

kihyuk-sung added 17 commits May 10, 2021 17:46

feat: BaseballGameView without Team

cc2aca2

- Team 없이, BaseballGame 만을 이용해서 View 구성 issue: #12

feat: BaseballGameService BaseballGames

389d547

- 서비스에서 게임들을 가져오도록 수정 issue: #12

refactor: DB Mapping

dc73636

- 중복되는 Embedded 적용 안되서, 새로 설계 issue: #12

feat: Get BaseballGame

f89076b

- 게임 하나를 가져올 수 있습니다. issue: #12

feat: OAuth 2.0

b0efc24

- OAuth 테스트 용 issue: #12

feat: pitch endpoint

d7090f9

- 공 던지기 가능 (post) issue: #12

Merge pull request #16 from PizzaCola-K/baseball-game

09d948e

[BE] 게임 기능 구현

feat: Stop game when batter is changed

e5e5786

- 타자가 바뀔 때, 한번 멈춥니다. issue: #12

feat: Add winner field in BaseballGame

2b00beb

- 게임이 끝나면, 승자를 추가 issue: #12

feat: Get Games that aren't finished

4ed488e

- Winner가 null이면, 끝나지 않은 게임 - 끝나지 않은 게임만 가져옵니다. issue: #12

feat: End the BaseballGame

f8cc6bd

- 게임이 9회 이후에 조건에 따라 끝납니다. - 승자를 보여줍니다. issue: #12

feat: Post /games/:gameId/pitch

45973b4

- 테스트용으로 Get 사용했던 것을 Post로 수정 issue: #12

feat: Don't save if the game is finished

d85c22c

- 게임이 끝나면 저장하지 않습니다. issue: #12

Merge pull request #17 from PizzaCola-K/baseball-game

327cd2b

[BE] 야구 게임 추가 기능

feat: Add mysql setting

43fb810

- mysql 연결을 위한 세팅 추가 - 게임과 팀을 추가하기 위한 엔드포인트 추가 issue: #18

feat: Change pitch to POST

a33486d

- 테스트용으로 GET으로 했던것 수정 issue: #18

Merge pull request #19 from PizzaCola-K/mysql

5852303

[BE] MySQL

kihyuk-sung added the review-BE label May 12, 2021

wheejuni self-assigned this May 12, 2021

wheejuni requested changes May 13, 2021

View reviewed changes


		import codesquad.team7.baseball.team.Player;

		public class HomePlayerStatistics extends PlayerStatistics {

Comments

Conversation

kihyuk-sung commented May 12, 2021

배포주소

게임 시작점

api 형태에 대한 문서

Uh oh!

wheejuni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kihyuk-sung May 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kihyuk-sung May 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kihyuk-sung commented May 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kihyuk-sung May 13, 2021 •

edited

Loading

kihyuk-sung May 13, 2021 •

edited

Loading