Skip to content
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

장박2바스단구터계니 #12

Open
wants to merge 60 commits into
base: main
Choose a base branch
from

Conversation

drunkenhw
Copy link
Member

No description provided.

drunkenhw and others added 30 commits April 26, 2023 22:02
* docs: 기능 목록 작성

Co-authored-by: drunkenhw <[email protected]>

* feat(Product): 상품 객체 생성

Co-authored-by: drunkenhw <[email protected]>

* feat(DbProductDao): 상품 목록 조회 기능 추가

Co-authored-by: drunkenhw <[email protected]>

* refactor(ProductService): Service 계층 추가

Co-authored-by: drunkenhw <[email protected]>

* feat(DbProductDao): 상품 생성

Co-authored-by: drunkenhw <[email protected]>

* feat(DbProductDao): 상품 수정

Co-authored-by: drunkenhw <[email protected]>

* feat(DbProductDao): 상품 삭제

Co-authored-by: drunkenhw <[email protected]>

* refactor(DbProductDao): ParameterSource 간결화

Co-authored-by: drunkenhw <[email protected]>

* docs: 리팩터링 목록 추가

Co-authored-by: drunkenhw <[email protected]>

* feat(ProductController): `/admin`로 접근할 경우 전체 상품 조회

Co-authored-by: drunkenhw <[email protected]>

* feat(DbProductDao): 상품 id로 조회 기능 추가

Co-authored-by: drunkenhw <[email protected]>

* feat(ProductController): `수정` 클릭 시 상품 수정 API 호출

Co-authored-by: drunkenhw <[email protected]>

* feat(ProductController): `상품 추가` 클릭 시 상품 생성 API 호출

Co-authored-by: drunkenhw <[email protected]>

* feat(ProductController): `삭제` 클릭 시 상품 삭제 API 호출

Co-authored-by: drunkenhw <[email protected]>

* feat(ProductController): 예외 처리 메서드 작성

Co-authored-by: drunkenhw <[email protected]>

* feat(ProductIntegrationTest): 요청과 응답에 대한 테스트 코드 작성

Co-authored-by: drunkenhw <[email protected]>

* fix(data.sql): 테이블 명 오류 수정

Co-authored-by: drunkenhw <[email protected]>

* fix(admin) : url 수정

* test(DbProductDao): 테스트 수정

* refactor(ProductService): transactional 추가

* style(ProductController): import 와일드 카드 제거

---------

Co-authored-by: gitchan <[email protected]>
@drunkenhw drunkenhw self-assigned this May 6, 2023
Copy link

@woo-chang woo-chang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

박스터신! 박스터신! 🏎️

Comment on lines +22 to +46
private void saveDummyData() {
productDao.save(new Product(
"피자",
"https://cdn.dominos.co.kr/admin/upload/goods/20200311_x8StB1t3.jpg",
13000));
productDao.save(new Product(
"샐러드",
"https://m.subway.co.kr/upload/menu/K-%EB%B0%94%EB%B9%84%ED%81%90-%EC%83%90%EB%9F%AC%EB%93%9C-%EB%8B%A8%ED%92%88_20220413025007802.png",
20000));
productDao.save(new Product(
"치킨",
"https://cdn.thescoop.co.kr/news/photo/202010/41306_58347_1055.jpg",
10000));

memberDao.save(new Member(
"[email protected]",
"boxster"
)
);
memberDao.save(new Member(
"[email protected]",
"member"
)
);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PostConstruct 굳 👍

한편으로는 sql 작성에 비해 dao 구현에 의존적이라 dao 변경에 영향을 받을 수 있을 것 같은데 어떻게 생각해?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sql 은 sql에 의존적이라고 생각해서 이렇게 했어요~
만약 nosql 로 바뀐다면?
쿼리 문법이 바뀐다면?
이라는 생각에요~

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다즐 의견에 동의합니다

Comment on lines +28 to +29
String authentication = webRequest.getHeader(AUTHORIZATION);
return authenticationExtractor.extractAuthInfo(authentication);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

깔끔 👍

Comment on lines +12 to +13
private static final int EMAIL = 0;
private static final int PASSWORD = 1;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사소하지만 INDEX를 명시해주는건 어떻게 생각하시나용?

Comment on lines 12 to 13
@Configuration
@Scope

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

붙어 있는 이유가 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 개망했다 뭐 테스트해본다고 붙인건데 이거 붙여놓고 리뷰요청 보냈네 ;;;;;;


@PostMapping
public ResponseEntity<ProductCartResponse> addMyCart(
@RequestBody CartRequest cartRequest,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Valid가 없어도 검증이 가능한가요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검증 안했어요.. ㅈㅅ

Comment on lines +30 to +40
private void validateName(String name) {
if (name.isBlank() || name.length() > 20) {
throw new IllegalArgumentException("상품명은 20자 이하로 입력해주세요");
}
}

private void validatePrice(Integer price) {
if (price < 1000) {
throw new IllegalArgumentException("상품 가격은 1000원 이상이여야 합니다.");
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VO로 검증 로직 전달하는건 어떠십니까?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

귀찮아서 ㅎㅎ..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

하지만 좋은 생각

this.memberDao = memberDao;
}

public List<MemberResponse> findMembers() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Transactional(readOnly=true) 일관성 있게 붙여주는게 좋을 것 같아요 :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

허걱 ㅈㅅ합니다

Comment on lines +1 to +2
DELETE
FROM product_cart;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모든 데이터를 지울 때는 DELETE보다 TRUNCATE를 사용하는건 어때?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은거 같아요 감사합니다

Copy link
Member

@greeng00se greeng00se left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

박스터고수

Comment on lines +10 to +34
@Component
public class DbInit {

private final ProductDao productDao;
private final MemberDao memberDao;

public DbInit(ProductDao productDao, MemberDao memberDao) {
this.productDao = productDao;
this.memberDao = memberDao;
}

@PostConstruct
private void saveDummyData() {
productDao.save(new Product(
"피자",
"https://cdn.dominos.co.kr/admin/upload/goods/20200311_x8StB1t3.jpg",
13000));
productDao.save(new Product(
"샐러드",
"https://m.subway.co.kr/upload/menu/K-%EB%B0%94%EB%B9%84%ED%81%90-%EC%83%90%EB%9F%AC%EB%93%9C-%EB%8B%A8%ED%92%88_20220413025007802.png",
20000));
productDao.save(new Product(
"치킨",
"https://cdn.thescoop.co.kr/news/photo/202010/41306_58347_1055.jpg",
10000));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data.sql 파일을 이용해서 초기값을 설정해보는건 어떨까? 🤔

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

더미데이터 넣을 때 data.sql 안쓰고 @PostConstruct한 이유가 뭐야?


@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
public @interface AuthPrincipal {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom Annotation 👍


public class AuthenticationArgumentResolver implements HandlerMethodArgumentResolver {

private static final String AUTHORIZATION = "Authorization";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떤 리뷰에서 봤는데 Spring이 제공하는 HttpHeaders.AUTHORIZATION 있다고 하네 나도 그걸 쓸까 했는데 머지댐

import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

@Configuration
@Scope
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope는 어떤 이유로 사용했어? 🤔


@ExceptionHandler
public ResponseEntity<ErrorResponse> handleException(AuthenticationException e) {
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(new ErrorResponse(e.getMessage()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인증을 위해 들어오는 값에 대한 로깅을 조금 더 자세히 남겨보는건 어떨까?

.body(productRequest)

.when()
.put("/products/{경id}", gitchan.getId())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

경id?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

경마id입니다

Copy link

@hanueleee hanueleee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

박스터 나 리뷰 열심히 달았어!!! 칭찬해줘 ><

- [x] `/settings`로 접근할 경우 전체 사용자 조회
- [ ] `select` 클릭 시 이후 요청에 사용자 인증 정보 포함

- [ ] 장바구니 기능 구현

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

뭐죠? x해줘요

Comment on lines +10 to +34
@Component
public class DbInit {

private final ProductDao productDao;
private final MemberDao memberDao;

public DbInit(ProductDao productDao, MemberDao memberDao) {
this.productDao = productDao;
this.memberDao = memberDao;
}

@PostConstruct
private void saveDummyData() {
productDao.save(new Product(
"피자",
"https://cdn.dominos.co.kr/admin/upload/goods/20200311_x8StB1t3.jpg",
13000));
productDao.save(new Product(
"샐러드",
"https://m.subway.co.kr/upload/menu/K-%EB%B0%94%EB%B9%84%ED%81%90-%EC%83%90%EB%9F%AC%EB%93%9C-%EB%8B%A8%ED%92%88_20220413025007802.png",
20000));
productDao.save(new Product(
"치킨",
"https://cdn.thescoop.co.kr/news/photo/202010/41306_58347_1055.jpg",
10000));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

더미데이터 넣을 때 data.sql 안쓰고 @PostConstruct한 이유가 뭐야?

Comment on lines +22 to +46
private void saveDummyData() {
productDao.save(new Product(
"피자",
"https://cdn.dominos.co.kr/admin/upload/goods/20200311_x8StB1t3.jpg",
13000));
productDao.save(new Product(
"샐러드",
"https://m.subway.co.kr/upload/menu/K-%EB%B0%94%EB%B9%84%ED%81%90-%EC%83%90%EB%9F%AC%EB%93%9C-%EB%8B%A8%ED%92%88_20220413025007802.png",
20000));
productDao.save(new Product(
"치킨",
"https://cdn.thescoop.co.kr/news/photo/202010/41306_58347_1055.jpg",
10000));

memberDao.save(new Member(
"[email protected]",
"boxster"
)
);
memberDao.save(new Member(
"[email protected]",
"member"
)
);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다즐 의견에 동의합니다

import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;

public class AuthenticationArgumentResolver implements HandlerMethodArgumentResolver {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argumentResolver를 bean으로 등록하는건 어떻게 생각해?

private void validateCredentials(String authorization) {
validateNull(authorization);
validateBasicAuth(authorization);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

깔끔하고 멋지네여 👍

);

ALTER TABLE PRODUCT_CART
ADD FOREIGN KEY (member_id) REFERENCES member (id);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

왜 Product_cart 테이블 만들때 맨 밑에 FK지정 안해주고 마지막에 ALTER TABLE로 해준거야?

import static io.restassured.RestAssured.given;
import static org.assertj.core.api.Assertions.assertThat;

@Sql("delete.sql")
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
public class ProductIntegrationTest {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 ㅁㅊ 나도모르게 이거 그냥 기본반찬테스트(?)인줄 알고 무시했는데 이거 진짜로 Product 통합테스트 하라는거였구나.. 지금 알았어 ㅁㅊㅁㅊ

public void indexTest() {
RestAssured.given()
.accept(MediaType.TEXT_HTML_VALUE)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅋㅋㅋㅋㅋ빈칸 왤케 웃기지

.body("imgUrl", equalTo("https://boxster.com"))
.body("price", equalTo(10000));
assertThat(productDao.findAll())
.map(Product::getName)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우와 assertThat에 map한거 신기해

import org.springframework.test.web.servlet.MockMvc;


@WebMvcTest(PageController.class)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨트롤러 슬라이스 테스트 멋지다. 나도 이거 해보려구 ㅎㅎㅎ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants