feat: 리포트 전체 목록 조회 #875

verus-j · 2022-05-06T04:50:58Z

Resolve #860

기능

최신순으로 작성된 리포트 목록을 모두 조회

Request

GET /reports HTTP/1.1
accept: */*
host: localhost:4000
connection: Keep-Alive
user-agent: Apache-HttpClient/4.5.13 (Java/12.0.2)
accept-encoding: gzip,deflate

Response

HTTP/1.1 200 
Vary: Origin
Vary: Access-Control-Request-Method
Vary: Access-Control-Request-Headers
Content-Type: application/json
Transfer-Encoding: chunked
Date: Fri, 06 May 2022 04:34:52 GMT
Keep-Alive: timeout=60
Connection: keep-alive

{
    "data": [
        {
            "id": 3,
            "title": "새로운 리포트2",
            "description": "리포트 설명",
            "startDate": "2022-04-03",
            "endDate": "2022-04-10",
            "abilities": [
                
            ],
            "studylogs": [
                
            ]
        },
        {
            "id": 2,
            "title": "새로운 리포트1",
            "description": "리포트 설명",
            "startDate": "2022-04-02",
            "endDate": "2022-04-09",
            "abilities": [
                
            ],
            "studylogs": [
                
            ]
        },
        {
            "id": 1,
            "title": "새로운 리포트0",
            "description": "리포트 설명",
            "startDate": "2022-04-01",
            "endDate": "2022-04-08",
            "abilities": [
                
            ],
            "studylogs": [
                
            ]
        }
    ],
    "totalSize": 3,
    "totalPage": 1,
    "currentPage": 1
}

verus-j · 2022-05-06T04:52:32Z

backend/src/acceptanceTest/resources/wooteco/prolog/report.feature

+  Scenario: 리포트 목록 조회하기
+    And "2022-04-01" 일자 리포트를 등록하고
+    And "2022-04-02" 일자 리포트를 등록하고
+    And "2022-04-03" 일자 리포트를 등록하고
+    When 리포트 목록을 조회하면
+    Then 최신순으로 정렬되어 반환된다
+


cucumber를 잘 모르지만 E2E 테스트가 없으니 먼가 아쉬워서 추가해봤습니다.
이렇게 사용하는게 맞는지 여쭤보고 싶습니다.

👏👏 너무 잘 사용해주셨네요!

verus-j · 2022-05-06T04:53:09Z

backend/src/main/java/wooteco/prolog/common/PageableResponse.java

+    public static <T> PageableResponse<T> of(List<T> data, Page<?> page) {
+        return new PageableResponse<>(data, page.getTotalElements(), page.getTotalPages(), page.getNumber() + ONE_INDEXED_PARAMETER);


PageableResponse에 제네릭이 적용되어 있지 않아서 적용해봤는데,
혹시 적용하지 않은 특별한 이유가 있는지 궁금합니다.

인간미 아닐까요? ^.^
제네릭을 추가해서 컴파일 타임에 좀더 명시적인 타입 체크가 가능하게 되었네요 👍

verus-j · 2022-05-06T04:59:49Z

backend/src/test/java/wooteco/prolog/report/application/ReportServiceTest.java

+    void findAllReports() {
+        Report report1 = new Report("title1", "desc1", member.getId(), LocalDate.of(2022, 3, 5),
+            LocalDate.of(2022, 3, 6));
+        Report report2 = new Report("title2", "desc2", member.getId(), LocalDate.of(2022, 3, 6),
+            LocalDate.of(2022, 3, 7));
+        Report report3 = new Report("title3", "desc3", member.getId(), LocalDate.of(2022, 4, 5),
+            LocalDate.of(2022, 4, 10));
+
+        reportRepository.save(report1);
+        reportRepository.save(report2);
+        reportRepository.save(report3);
+
+        Pageable pageable = PageRequest.of(0, 10, Sort.Direction.DESC, "startDate");
+        PageableResponse<ReportResponse> allReports = reportService.findReports(pageable);
+        List<Long> expectIds = allReports.getData().stream()
+            .map(ReportResponse::getId)
+            .collect(Collectors.toList());
+
+        assertThat(expectIds).containsExactly(report3.getId(), report2.getId(), report1.getId());


AbilityServiceTest를 참고해서 통합 테스트를 작성했습니다. 처음에는 Report에서 Member와 연관관계가 없어서 1L과 같은 값으로 id를 하드 코딩했습니다. 그런데 Report의 프로퍼티 명이 memberId여서 현재는 연관관계가 없지만 Member와 관계를 맺을 계획인 거 같아 보여 Member의 Id를 사용했습니다. 현 상황에서 하드 코딩을 하는게 좋을 지 아니면 Membe의 id를 사용하는 것이 좋을 지 궁금합니다.

저는 1L로 하드코딩 하겠습니다. 두 가지 이유가 있는데요.

왜냐하면 Report의 memberId의 프로퍼티의 타입이 Long이었거든용
베루스는 memberId라는 표식에서 연관관계를 연상하셨지만, 전 Long 타입으로부터 Report와 Member의 도메인 분리 라는 발상을 떠올렸어요. (전 프롤로그의 역사를 알고 있고, 도메인 연관관계 분리 라는 학습 내용이 있기 때문입니다)
그래서 전 처음엔 Member객체를 직접 참조했다가 Long 타입으로 변환했을 거라고 예상했고, 앞으로도 쭉 Long타입을 유지할 것이라 예상하기 때문에 1L로 하드 코딩 할 것같네요
약간의 부연을 하자면,
Report에 Member 연관관계를 넣는 것이 간편하지만, Report 도메인과 Member 도메인에 강한 결합도가 생깁니다. Long 타입(혹은 ID Dto)으로 다루어 ID를 주고받으면 그런 연관관계를 끊어낼 수 있고요.
객체의 연관관계를 끊는 부분은 객체지향과 대치되어 보이지만, 여러 장점이 있습니다. 일례로 해당 테스트에서 MemberReporsitory에 대한 의존성 없이도 1L을 삽입해 순수하게 Report 서비스에 대해서만 검증할 수 있겠죠

두 번째 이유로, OCP를 학습하다보면 나오는 Crystal ball problem🔮이란 것이 있습니다.
모든 것을 확장으로부터 열려있게 구현하면 좋지만, 사실 저희는 미래의 변경사항을 마법의 수정구처럼 예측할 수 없습니다. 경험으로부터 '이건 확실히 변경될꺼야' 확신이 없다면, 불확실한 미래의 변경사항에 대비하는 것보다 현재의 단순함을 지키는게 미덕이 될 때도 있습니다. 고래서 더 간단한 1L을 사용할 것 같네요.
해당 값이 memberId라는 것은 변수명 / 메서드명을 통해 확실하게 해두고요..

AbilityServiceTest를 작성한 사람이라 member.getId()에 대한 고민에 대해 제 의견도 공유하고 싶어요.

웨지 말씀처럼 현재 Report가 MemberId(Long)로만 Member를 의존하는 것은 가능한 의존도를 떨어뜨리고 싶다는 의미로 보여요.
그러나 '의존도가 떨어지니 테스트에서 1L로 하드코딩을 해야한다.' 라기보단, '현재 테스트를 작성하는 의도가 Report를 확인하기 위함이니 Member는 크게 중요하지 않다. 그래서 1L로 하드코딩 해도 된다.' 로 접근해보시면 어떨까 생각이 들어요. "무얼 테스트 하려하는가. 이 테스트가 왜 필요한가."가 제 1순위 고민 포인트라고 생각합니다.

AbilityServiceTest.java를 보셔서 아시겠지만 하드코딩을 해도 될 정도로 연관관계가 깊지 않은(ID만 갖고 있다던지) 엔티티도 굳이 만들어서 저장한 후 getId()를 사용하는 코드가 많은데요, '(그래서는 안되겠지만) 테스트간 영속성 컨텍스트가 공유되어서 테스트간 서로 같은 데이터를 조회하는 상황이 발생해도 실패하지 않는 단단한 테스트를 만들자.' 라는 생각에 그렇게 작성했습니다. (그리고 Long 타입으로 들어가는 값이 뭐하는 값인지 명확하잖아요?! ㅋㅋㅋ)

테스트를 작성하는 시간도 분명히 개발 리소스라서 1L 하드코딩을 하는게 훨씬 이득일 수도 있습니다.
(베루스가 그러신 것처럼, 첫 코드가 작성되기 시작하면 다른 개발자들이 그 패턴을 따라함 - 전체 개발자들에게 영향을 끼침)
"ReportServiceTest가 왜 필요한가" 관점을 고민해보시고, Member.getId()가 유효한지, 불필요한지 저울질해보시면 좋을거 같아요.

sihyung92

넘 잘해주셔서~~ 딱히 피드백할 부분이 없네요~~👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏👏
테스트에 @DisplayName 관련한 부분만 한번 신경써주세요 👍

sihyung92 · 2022-05-06T06:02:19Z

backend/src/acceptanceTest/resources/wooteco/prolog/report.feature

+  Scenario: 리포트 목록 조회하기
+    And "2022-04-01" 일자 리포트를 등록하고
+    And "2022-04-02" 일자 리포트를 등록하고
+    And "2022-04-03" 일자 리포트를 등록하고
+    When 리포트 목록을 조회하면
+    Then 최신순으로 정렬되어 반환된다
+


👏👏 너무 잘 사용해주셨네요!

sihyung92 · 2022-05-06T06:03:44Z

backend/src/main/java/wooteco/prolog/common/PageableResponse.java

+    public static <T> PageableResponse<T> of(List<T> data, Page<?> page) {
+        return new PageableResponse<>(data, page.getTotalElements(), page.getTotalPages(), page.getNumber() + ONE_INDEXED_PARAMETER);


인간미 아닐까요? ^.^
제네릭을 추가해서 컴파일 타임에 좀더 명시적인 타입 체크가 가능하게 되었네요 👍

sihyung92 · 2022-05-06T06:07:41Z

backend/src/test/java/wooteco/prolog/report/application/ReportServiceTest.java

+    @Test
+    void findAllReports() {


메서드 이름은 "리포트 전체 조회"인데, 테스트하는 내용은 "리포트를 전체조회 하고 정렬되어 있는지 확인" 입니다.
보통 코드를 수정한 후 테스트 디렉토리에 있는 모든 테스트를 실행해보게 되는데, 이 테스트가 실패했을 때 다른 개발자로 하여금 잘 못된 표식(beacon)을 줄 수 있어요.
(Sorting 로직을 변경해서 테스트가 실패했는데, 조회 전체가 틀어졌나? 하는 생각)

다른 테스트처럼 @DisplayName 어노테이션을 통해 명시적인 표식을 남겨주면, 다른 개발자의 10초를 아껴줄 수 있지 않을까 생각합니다

sihyung92 · 2022-05-06T06:37:53Z

backend/src/test/java/wooteco/prolog/report/application/ReportServiceTest.java

+    void findAllReports() {
+        Report report1 = new Report("title1", "desc1", member.getId(), LocalDate.of(2022, 3, 5),
+            LocalDate.of(2022, 3, 6));
+        Report report2 = new Report("title2", "desc2", member.getId(), LocalDate.of(2022, 3, 6),
+            LocalDate.of(2022, 3, 7));
+        Report report3 = new Report("title3", "desc3", member.getId(), LocalDate.of(2022, 4, 5),
+            LocalDate.of(2022, 4, 10));
+
+        reportRepository.save(report1);
+        reportRepository.save(report2);
+        reportRepository.save(report3);
+
+        Pageable pageable = PageRequest.of(0, 10, Sort.Direction.DESC, "startDate");
+        PageableResponse<ReportResponse> allReports = reportService.findReports(pageable);
+        List<Long> expectIds = allReports.getData().stream()
+            .map(ReportResponse::getId)
+            .collect(Collectors.toList());
+
+        assertThat(expectIds).containsExactly(report3.getId(), report2.getId(), report1.getId());


저는 1L로 하드코딩 하겠습니다. 두 가지 이유가 있는데요.

왜냐하면 Report의 memberId의 프로퍼티의 타입이 Long이었거든용
베루스는 memberId라는 표식에서 연관관계를 연상하셨지만, 전 Long 타입으로부터 Report와 Member의 도메인 분리 라는 발상을 떠올렸어요. (전 프롤로그의 역사를 알고 있고, 도메인 연관관계 분리 라는 학습 내용이 있기 때문입니다)
그래서 전 처음엔 Member객체를 직접 참조했다가 Long 타입으로 변환했을 거라고 예상했고, 앞으로도 쭉 Long타입을 유지할 것이라 예상하기 때문에 1L로 하드 코딩 할 것같네요
약간의 부연을 하자면,
Report에 Member 연관관계를 넣는 것이 간편하지만, Report 도메인과 Member 도메인에 강한 결합도가 생깁니다. Long 타입(혹은 ID Dto)으로 다루어 ID를 주고받으면 그런 연관관계를 끊어낼 수 있고요.
객체의 연관관계를 끊는 부분은 객체지향과 대치되어 보이지만, 여러 장점이 있습니다. 일례로 해당 테스트에서 MemberReporsitory에 대한 의존성 없이도 1L을 삽입해 순수하게 Report 서비스에 대해서만 검증할 수 있겠죠

두 번째 이유로, OCP를 학습하다보면 나오는 Crystal ball problem🔮이란 것이 있습니다.
모든 것을 확장으로부터 열려있게 구현하면 좋지만, 사실 저희는 미래의 변경사항을 마법의 수정구처럼 예측할 수 없습니다. 경험으로부터 '이건 확실히 변경될꺼야' 확신이 없다면, 불확실한 미래의 변경사항에 대비하는 것보다 현재의 단순함을 지키는게 미덕이 될 때도 있습니다. 고래서 더 간단한 1L을 사용할 것 같네요.
해당 값이 memberId라는 것은 변수명 / 메서드명을 통해 확실하게 해두고요..

sonarqubecloud · 2022-05-11T07:05:45Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

leo0842 and others added 3 commits May 6, 2022 13:40

feat: 리포트 목록 조회 기능 추가

b7a6bcd

refactor: PageableResponse 제네릭 적용

e9fd287

test: 리포트 목록 조회 E2E 테스트 추가

d46d45e

verus-j requested review from gracefulBrown and sihyung92 May 6, 2022 04:50

verus-j self-assigned this May 6, 2022

verus-j assigned leo0842 May 6, 2022

verus-j commented May 6, 2022

View reviewed changes

refactor: 사용하지 않는 import문 정리

331f721

sihyung92 approved these changes May 6, 2022

View reviewed changes

test: @DisplayName 적용 및 Member Id 하드 코딩

d47f2a4

Hyeon9mak added the backend label May 14, 2022

sihyung92 merged commit 3cc27a8 into develop May 18, 2022

wishoon mentioned this pull request Jun 9, 2022

feat: 인기있는 학습로그를 각 카테고리 별 공통, 프론트엔드, 백엔드로 제공하는 기능 구현 #897

Closed

2 tasks

seokhongkim deleted the feature/reports-list branch January 20, 2023 07:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 리포트 전체 목록 조회 #875

feat: 리포트 전체 목록 조회 #875

verus-j commented May 6, 2022

verus-j May 6, 2022

sihyung92 May 6, 2022

verus-j May 6, 2022

sihyung92 May 6, 2022

verus-j May 6, 2022

sihyung92 May 6, 2022

Hyeon9mak May 14, 2022

sihyung92 left a comment

sihyung92 May 6, 2022

sihyung92 May 6, 2022

sihyung92 May 6, 2022

sihyung92 May 6, 2022

sonarqubecloud bot commented May 11, 2022

		public static <T> PageableResponse<T> of(List<T> data, Page<?> page) {
		return new PageableResponse<>(data, page.getTotalElements(), page.getTotalPages(), page.getNumber() + ONE_INDEXED_PARAMETER);

feat: 리포트 전체 목록 조회 #875

feat: 리포트 전체 목록 조회 #875

Conversation

verus-j commented May 6, 2022

기능

Request

Response

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

sihyung92 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

sonarqubecloud bot commented May 11, 2022