[BE][Team09][Bat & Pyro] API 를 JDBC 와 연동 #59

ghojeong · 2021-04-29T15:00:50Z

#20 의 피드백들을 반영하려다 보니, 구현할 내용이 자꾸만 커져버려서 너무 뒤늦게 PR 을 올리게 되었습니다.
JDBC 와 데이터베이스가 익숙치 않아서 시간이 너무 오래 걸려 죄송합니다.

피드백 주신대로 Spring profile 을 이용해서, H2 와 MySQL 의 분기처리가 쉽게 일어나도록 하였습니다.
기존의 지나치게 거대했던 Dish 도메인 객체도 Image, Sale, Delivery 등으로 분리될 수 있도록 하였습니다.

아직 미구현된 부분은 다음과 같습니다.
refreshable 여부를 결정하는 컴포넌트의 설계 및 구현이 아직 완성되지 않았으며,
익숙치 않은 Spring JDBC 를 사용하다보니, Derived Queries 를 제대로 활용하지 못하고 쿼리를 직접 작성하게 되었습니다.
(Derived Queries 로 발생하는 버그를 며칠간 제대로 해결하지 못해서 시간을 날리다 보니, 뒤늦게 그냥 쿼리를 작성하게 되었습니다.)
지금은 프로젝트 규모가 크지 않다보니 직접 쿼리를 짜는게, Derived Queries 보다 개발 속도가 빠른 것 같습니다.

익숙치 않은 삽질을 계속하다보니, 미흡한 부분이 많습니다.
N+1 문제도 아직 제대로 해결을 하지 못했는데, 쿼리 수정을 통해 해결할 수 있도록 하겠습니다.

#20 에서 주신 피드백 덕분에, 정말로 여러가지를 더 시도할 수 있었습니다.
언제나 좋은 피드백 감사합니다!
그리고 욕심을 부리다가 피드백에 빠르게 반응을 하지 못해서 죄송합니다.

[BE] JDBC연동 - category class

[BE] category 테스트 코드 작성

[BE] JDBC 연동 도메인, 데이터베이스 데이터 삽입

[BE] DTO 생성 (get요청 불가능)

[BE] JDBC 와 서버를 연동

[BE] 주문하기 기능 구현

[BE] JDBC Template 으로 N + 1 문제 해결

[BE] N+1 문제 해결 & 주문하기 기능 구현

wheejuni

빡빡한 일정중에 코드 리뷰까지 요청하시느라 대단히 고생 많으셨습니다. 💯
우선 전반적으로 일정에 쫓기다보니 통합 테스트는 물론이고 유닛 테스트도 많이 부족하네요. 이 부분은 개선되기를 바랍니다.
아울러 enum 을 조금 더 적극적으로 사용해 보시면 전반적인 코드 퀄리티를 높일 수 있을 것 같습니다.
그리고 조금 더 쉽게 쉽게 생각하며 코딩하기를 권해 드리고 싶네요. 한번 꼬아서 생각하는 바람에, 코드도 꼬인 것 같은 부분이 여럿 있네요.

수고 많으셨습니다. 🥇

wheejuni · 2021-05-02T01:20:40Z

backend/src/main/java/com/codesquad/sidedish/SideDish/controller/DishController.java

+        if (dishService.order(detailHash, count)) {
+            return ResponseEntity.ok().body("주문완료");
+        }
+        return ResponseEntity.ok().body("주문불가");


dishService.order() 의 리턴 타입이 스트링이었으면 코드가 조금 더 간결해졌겠네요.
같은 메소드를 소비하는 다른 소비처가 있나요? 그리고 그곳이 boolean 타입 리턴을 필요로 하나요?
그렇지 않다면, 서비스에서 바로 스트링을 내리는 구현도 좋을 것 같습니다.

다만 지금은 주문의 상태에 관한, 그러니까 경우의 수 를 다루는 메소드이므로 enum 을 사용하기에 아주 적절한 상황으로 보입니다.

public enum OrderResult { ORDER_SUCCESSFUL("주문완료"), ORDER_NOT_MADE("주문불가") ...

다만 지금은 주문의 상태에 관한, 그러니까 경우의 수 를 다루는 메소드이므로 enum 을 사용하기에 아주 적절한 상황으로 보입니다.

enum 을 더 적극적으로 사용해보도록 하겠습니다!

enum 생각을 하지 못하고 개발하였습니다....
image 구분할 때도 enum으로 했으면 더 좋았을 것 같습니다.
감사합니다.

wheejuni · 2021-05-02T01:21:01Z

backend/src/main/java/com/codesquad/sidedish/SideDish/controller/DishController.java

    @GetMapping("/main")
    @ApiOperation(value = "메인 요리", notes = "메인 요리의 목록을 반환합니다.")
    public ResponseEntity<List<DishDto>> getMainList() {
-        List<DishDto> dishes = dishService.getList(1);
+        List<DishDto> dishes = dishService.getList(1L);


1L, 2L 은 상수로 관리돼야 할 것 같군요.

이것도 enum 으로 main 과 1L 상수를 하나로 묶는게 좋겠군요.
생각없이 막코딩한 밑천이 다 드러나네요...

wheejuni · 2021-05-02T01:25:52Z

backend/src/main/java/com/codesquad/sidedish/SideDish/domain/Delivery.java

+import java.util.Objects;
+
+public class Delivery {
+    private String deliveryType;


type 이라면... 문자열 필드보단 더 좋은 방법이 있지 않을까요?

type 이라면, enum 으로 만들어서 관리하는 편이 훨씬 좋겠군요.
지적해주셔서 감사합니다.

'deliveryType',' imageType' 그리고 sale에서 'Badge' 모두 enum으로 관리하면 더 좋았을 것 같습니다.
감사합니다.^^

wheejuni · 2021-05-02T01:26:50Z

backend/src/main/java/com/codesquad/sidedish/SideDish/domain/DishDao.java

+
+    @Autowired
+    public DishDao(DataSource dataSource) {
+        jdbcTemplate = new JdbcTemplate(dataSource);


JdbcTemplate 도 의존성 주입을 받을 수 있습니다.
어떻게 하면 될지 검색해 보시고, 그렇게 하시는 편이 좋겠습니다.

JdbcTemplate 도 의존성 주입을 받을 수 있습니다.

와! 이건 상상도 못했는데, 찾아보겠습니다.
알려주셔서 감사합니다.

wheejuni · 2021-05-02T01:28:34Z

backend/src/main/java/com/codesquad/sidedish/SideDish/domain/ImageRepository.java

+    @Query("SELECT image.* from sidedish.image INNER JOIN sidedish.dish_image ON image.id = dish_image.image WHERE image.type = 'thumb' AND dish_image.dish = :detailHash;")
+    List<Image> findThumbImagesByDish(String detailHash);
+
+    @Query("SELECT image.* from sidedish.image INNER JOIN sidedish.dish_image ON image.id = dish_image.image WHERE image.type = 'detail' AND dish_image.dish = :detailHash;")
+    List<Image> findDetailImagesByDish(String detailHash);


두 쿼리 메소드를 하나로 합칠 수 있을 것 같은데요. 어떻게 좋은 방법 없을까요?

WHERE image.type = ? 부분의 조건을 삭제하고, Service 에서 image.type 에 따라 dto 에 다르게 값을 넣어주면 되겠네요.
불필요하게 쿼리가 2번 날아가는 문제 지적해주셔서 감사합니다.

파라미터값에 detail, thumb 을 받아도 될 것같습니다.
불필요한 쿼리메소드 줄이는 것에 신경써서 코딩하겠습니다.
감사합니다.

wheejuni · 2021-05-02T01:29:07Z

backend/src/main/java/com/codesquad/sidedish/SideDish/dto/DishDetailDto.java


 public class DishDetailDto {
    private final List<String> thumbImages;
-    private final int point;
+    private final Integer point;


int 에서 Integer 가 된 특별한 이유가 있나요?

insert 를 하는 초기화 sql 중에서 point 의 값이 비어있는 Record 들이 있어서,
터지는 것을 방지하기 위해서 임시로 Integer 로 바꾸었습니다.
코드를 바꾸는 좋지 못한 해결책 같네요.

wheejuni · 2021-05-02T01:29:29Z

backend/src/main/java/com/codesquad/sidedish/SideDish/service/DishService.java

+//        return dishRepository.findAllByCategoryId(categoryId)
+//                .stream().map(this::convert)
+//                .collect(Collectors.toList());
    }

+//    private DishDto convert(Dish dish) {
+//        String detailHash = dish.getDetailHash();
+//        List<Delivery> deliveries = deliveryRepository.findAllByDish(detailHash);
+//        List<Sale> sales = saleRepository.findAllByDish(detailHash);
+//        return DishDto.from(dish, deliveries, sales);
+//    }


사용하지 않는 코드라면 빠른 삭제를 부탁드립니다.

삭제하도록 하겠습니다.

wheejuni · 2021-05-02T01:32:41Z

backend/src/main/java/com/codesquad/sidedish/SideDish/service/DishService.java

+        List<Image> thumbImages = imageRepository.findThumbImagesByDish(detailHash);
+        List<Image> detailImages = imageRepository.findDetailImagesByDish(detailHash);


결국 두 종류의 이미지를 모두 불러와야 하는군요.
그렇다면 Image 안에 thumb, detail 구분이 가능한 필드만 있으면, 한번에 불러와서 DTO로 넘겨도 되는 거 아닐까요?
DTO가 만들어질 때 두 개를 구분해서 넣으면 될 것 같은데요. 어떻게 생각하세요?

아, 그게 확실히 더 효율적일것 같습니다.
동의합니다.

wheejuni · 2021-05-02T01:33:01Z

backend/src/main/java/com/codesquad/sidedish/SideDish/service/DishService.java

+        Dish dish = getDish(detailHash);
+        QuantityDto quantityDto = getDetailQuantity(detailHash);
+        if (quantityDto.getQuantity() > count) {
+            int upqua = quantityDto.getQuantity() - count;


upqua 라는 변수명을 보고 그 뜻을 알아볼 수 있는 사람은 단언컨대 한 명도 없을 것 같습니다...

급하게 코딩하느라 변수명 신경 못썼습니다.
updatedQuantity 라고 의도했습니다...

ghojeong · 2021-05-02T04:21:37Z

우선 전반적으로 일정에 쫓기다보니 통합 테스트는 물론이고 유닛 테스트도 많이 부족하네요. 이 부분은 개선되기를 바랍니다.
아울러 enum 을 조금 더 적극적으로 사용해 보시면 전반적인 코드 퀄리티를 높일 수 있을 것 같습니다.
그리고 조금 더 쉽게 쉽게 생각하며 코딩하기를 권해 드리고 싶네요. 한번 꼬아서 생각하는 바람에, 코드도 꼬인 것 같은 부분이 여럿 있네요.

부족한 부분을 콕 찝어서 말씀해주셔서 감사합니다.
특히 테스트 코드의 부재는 매우 동감하는 문제 같습니다.
다음에는 테스트 코드와 코드 퀄리티에 더 신경써서 개발할 수 있도록 도전해보겠습니다.

ghojeong · 2021-05-02T04:29:49Z

금요일날 시연이 끝나서, 리뷰를 받을 수 있을까 걱정했는데,
리뷰를 해주셔서 정말 감사합니다!

다음주 미션을 할 때는 특히 테스트 코드를 신경써서 개발을 해 볼 수 있도록 하겠습니다!

kjk402 · 2021-05-02T04:51:24Z

저 역시 프로젝트 기간이 끝나서 리뷰를 못 받지 않을까 했었는데, 리뷰주셔서 감사합니다.~
브라이언 덕분에 프로젝트 코드 다시한번 검토할 수 있는 기회되어 정말 좋습니다.

프로젝트는 기간이 정해져 있어 구현에만 집중하여 코드가 많이 부족합니다.
다음 프로젝트에서는 말씀해주신 바와 같이 조금 쉽게 쉽게 코딩하여 코드가 깔끔하게 되도록 노력하겠습니다.

감사합니다.

kjk402 and others added 20 commits April 26, 2021 15:15

feat: JDBC연동 - category class

f1ae674

Merge pull request #44 from ghojeong/feature/be/jdbc

cb11c76

[BE] JDBC연동 - category class

feat: Add category test code

f307e56

[test] Softl Assertion 적용

01cdef4

Merge pull request #47 from ghojeong/feature/be/jdbc

6650342

[BE] category 테스트 코드 작성

feat: JDBC 연동 도메인, 데이터베이스 데이터 삽입

f8654db

feat: dish table currentDateTime 컬럼추가

5de75e1

feat: image, sale, delivery에 set으로 매핑 추가

42f75af

Merge branch 'backend-develop' into feature/be/jdbc

9061ab7

Merge pull request #49 from ghojeong/feature/be/jdbc

71719dd

[BE] JDBC 연동 도메인, 데이터베이스 데이터 삽입

feat: create DTO

f41a420

[fix] 빌드가 실패하는 버그를 수정함

9d33a4e

Merge pull request #56 from ghojeong/feature/be/jdbc

59f8f50

[BE] DTO 생성 (get요청 불가능)

feat: h2 에 맞추어 properties 와 schema 작성

2371a00

feat: Spring profile 설정; prod & dev

2d4c4c0

feat: findAll 이 동작하지 않던 문제 해결

a544477

refactor: Query 를 통해 dish 목록을 가져오도록 수정

a779680

feat: dishDetail 에서 이미지 목록이 나오도록 함

4172f10

feat: dish 목록에서 delivery 타입과, badges 가 나오도록 함

f3d5fb3

Merge pull request #58 from ghojeong/feature/be/profile

de90a33

[BE] JDBC 와 서버를 연동

ghojeong added the review-BE BE 리뷰 label Apr 29, 2021

ghojeong requested a review from wheejuni April 29, 2021 15:00

ghojeong self-assigned this Apr 29, 2021

kjk402 and others added 6 commits April 30, 2021 11:03

feat: 주문하기 기능 구현

07e69c4

feat: post -> patch 매핑으로 변경

997048b

Merge pull request #59 from ghojeong/feature/be/order

95c6ff6

[BE] 주문하기 기능 구현

feat: JDBC Template 으로 N + 1 문제 해결

8452983

Merge pull request #60 from ghojeong/feature/be/nplus1

96d1d2e

[BE] JDBC Template 으로 N + 1 문제 해결

Merge pull request #61 from ghojeong/backend-develop

7719658

[BE] N+1 문제 해결 & 주문하기 기능 구현

wheejuni self-assigned this May 2, 2021

wheejuni requested changes May 2, 2021

View reviewed changes

고정완 (Can) added 2 commits May 2, 2021 13:00

feat: pstm 으로 SQL Injection 해킹 방지

4db8370

Merge branch 'backend-develop' into backend

c2f0380

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE][Team09][Bat & Pyro] API 를 JDBC 와 연동 #59

[BE][Team09][Bat & Pyro] API 를 JDBC 와 연동 #59

ghojeong commented Apr 29, 2021

wheejuni left a comment

wheejuni May 2, 2021

ghojeong May 2, 2021

kjk402 May 2, 2021

wheejuni May 2, 2021

ghojeong May 2, 2021 •

edited

Loading

wheejuni May 2, 2021

ghojeong May 2, 2021

kjk402 May 2, 2021

wheejuni May 2, 2021

ghojeong May 2, 2021

wheejuni May 2, 2021

ghojeong May 2, 2021

kjk402 May 2, 2021

wheejuni May 2, 2021

ghojeong May 2, 2021 •

edited

Loading

wheejuni May 2, 2021

ghojeong May 2, 2021

wheejuni May 2, 2021

ghojeong May 2, 2021

wheejuni May 2, 2021

kjk402 May 2, 2021

ghojeong commented May 2, 2021

ghojeong commented May 2, 2021

kjk402 commented May 2, 2021

		List<Image> thumbImages = imageRepository.findThumbImagesByDish(detailHash);
		List<Image> detailImages = imageRepository.findDetailImagesByDish(detailHash);

[BE][Team09][Bat & Pyro] API 를 JDBC 와 연동 #59

Are you sure you want to change the base?

[BE][Team09][Bat & Pyro] API 를 JDBC 와 연동 #59

Conversation

ghojeong commented Apr 29, 2021

wheejuni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghojeong May 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghojeong May 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghojeong commented May 2, 2021

ghojeong commented May 2, 2021

kjk402 commented May 2, 2021

ghojeong May 2, 2021 •

edited

Loading

ghojeong May 2, 2021 •

edited

Loading