feat: 모든 스타카토 조회 시 위경도 범위/카테고리를 기준으로 조회할 수 있도록 구현 #759 #763 #779

linirini · 2025-05-02T03:17:43Z

⭐️ Issue Number

🚩 Summary

특정 사용자의 위경도 범위 내에 있는 모든 스타카토 목록을 조회할 수 있도록 구현했습니다. -> 커밋 범위 확인하기
특정 사용자의 위경도 범위 내에 있으면서, 특정 카테고리에 속한 스타카토 목록을 조회할 수 있도록 구현했습니다. -> 커밋 범위 확인하기
query string nullable 함을 활용하여
- 주어진 위경도 범위가 없다면, 모든 스타카토를
- 주어진 카테고리가 없다면, 카테고리 구분 없이 모든 스타카토를
  조회할 수 있도록, jpql로 구현했습니다.

🛠️ Technical Concerns

🙂 To Reviewer

📋 To Do

MySQL 공간 타입 활용하기, 인덱스 적용

github-actions · 2025-05-02T03:19:48Z

Test Results

41 files 41 suites 6s ⏱️
254 tests 254 ✅ 0 💤 0 ❌
270 runs 270 ✅ 0 💤 0 ❌

Results for commit 9b23842.

♻️ This comment has been updated with latest results.

github-actions · 2025-05-02T03:19:49Z

🌻Test Coverage Report

Overall Project	80.93%	🍏
Files changed	100%	🍏

File	Coverage
StaccatoLocationResponseV2.java	100%	🍏
StaccatoLocationResponsesV2.java	100%	🍏
CategoryStaccatoLocationResponses.java	100%	🍏
CategoryStaccatoLocationResponse.java	100%	🍏
CategoryStaccatoLocationRangeRequest.java	100%	🍏
StaccatoLocationRangeRequest.java	100%	🍏
StaccatoService.java	100%	🍏
StaccatoControllerV2.java	100%	🍏
CategoryService.java	98.62%	🍏
StaccatoController.java	90.43%	🍏
CategoryController.java	89.89%	🍏

BurningFalls

까다로운 부분인데 구현 잘 해주셨네요, 리니! 수고하셨습니다.

남긴 코멘트 확인 부탁드려요~

BurningFalls · 2025-05-02T05:47:16Z

backend/src/test/java/com/staccato/staccato/controller/StaccatoControllerV2Test.java

@@ -73,4 +86,44 @@ void readAllStaccato() throws Exception {
                .andExpect(status().isOk())
                .andExpect(content().json(expectedResponse));
    }
+
+    @DisplayName("유효하지 않은 위도 또는 경도 쿼리로 스타카토 목록 조회 시 예외가 발생한다.")


StaccatoLocationRangeRequestFixtures를 만들어서 사용하는 아래와 같은 방식은 어떻게 생각하시나요?

재사용성이 높지 않아 과하다는 생각도 들지만, 가독성 측면에서는 나쁘지 않은 것 같습니다.

static Stream<Arguments> invalidLatLngProvider() { return Stream.of( Arguments.of(StaccatoLocationRangeRequestFixtures.defaultStaccatoLocationRangeRequest() .withNeLat(BigDecimal.valueOf(-90.0001)).build(), "위도는 -90.0 이상이어야 합니다."), Arguments.of(StaccatoLocationRangeRequestFixtures.defaultStaccatoLocationRangeRequest() .withNeLat(BigDecimal.valueOf(90.0001)).build(), "위도는 90.0 이하여야 합니다."), Arguments.of(StaccatoLocationRangeRequestFixtures.defaultStaccatoLocationRangeRequest() .withNeLng(BigDecimal.valueOf(-180.0001)).build(), "경도는 -180.0 이상이어야 합니다."), Arguments.of(StaccatoLocationRangeRequestFixtures.defaultStaccatoLocationRangeRequest() .withNeLng(BigDecimal.valueOf(180.0001)).build(), "경도는 180.0 이하여야 합니다.") ); } @DisplayName("유효하지 않은 위도 또는 경도 쿼리로 스타카토 목록 조회 시 예외가 발생한다.") @ParameterizedTest @MethodSource("invalidLatLngProvider") void failReadAllStaccatoWithInvalidSingleLatLng(StaccatoLocationRangeRequest invalidRequest, String expectedMessage) throws Exception { // given ExceptionResponse exceptionResponse = new ExceptionResponse( HttpStatus.BAD_REQUEST.toString(), expectedMessage); // when & then mockMvc.perform(get("/v2/staccatos") .param("neLat", String.valueOf(invalidRequest.neLat())) .param("neLng", String.valueOf(invalidRequest.neLng())) .param("swLat", String.valueOf(invalidRequest.swLat())) .param("swLng", String.valueOf(invalidRequest.swLng())) .header(HttpHeaders.AUTHORIZATION, "token")) .andExpect(status().isBadRequest()) .andExpect(content().json(objectMapper.writeValueAsString(exceptionResponse))); }

public class StaccatoLocationRangeRequestFixtures { public static StaccatoLocationRangeRequestBuilder defaultStaccatoLocationRangeRequest() { return new StaccatoLocationRangeRequestBuilder() .withCategoryId(null) .withNeLat(BigDecimal.valueOf(0)) .withNeLng(BigDecimal.valueOf(0)) .withSwLat(BigDecimal.valueOf(0)) .withSwLng(BigDecimal.valueOf(0)); } public static class StaccatoLocationRangeRequestBuilder { Long categoryId; BigDecimal neLat; BigDecimal neLng; BigDecimal swLat; BigDecimal swLng; public StaccatoLocationRangeRequestBuilder withCategoryId(Long categoryId) { this.categoryId = categoryId; return this; } public StaccatoLocationRangeRequestBuilder withNeLat(BigDecimal neLat) { this.neLat = neLat; return this; } public StaccatoLocationRangeRequestBuilder withNeLng(BigDecimal neLng) { this.neLng = neLng; return this; } public StaccatoLocationRangeRequestBuilder withSwLat(BigDecimal swLat) { this.swLat = swLat; return this; } public StaccatoLocationRangeRequestBuilder withSwLng(BigDecimal swLng) { this.swLng = swLng; return this; } public StaccatoLocationRangeRequest build() { return new StaccatoLocationRangeRequest(categoryId, neLat, neLng, swLat, swLng); } } }

RequestFixture를 분리하려다가 하지 않았던 이유를 공유드려요!
StaccatoLocationRangeRequest는 내부의 모든 값들이 기준이 되어 핵심 로직 테스트가 이루어지게 됩니다.
그렇다보니, 매 테스트마다 모든 값들에 대해 개발자가 설정을 해주어야 해요.
그렇다보니 픽스처를 분리하더라도 그 이점을 누리기 어렵다고 판단하여 따로 만들지 않았습니다.
폭포는 어떻게 생각하시나요?

매 테스트마다 모든 값들에 대해 개발자가 설정을 해주어야 해요.
그렇다보니 픽스처를 분리하더라도 그 이점을 누리기 어렵다고 판단하여 따로 만들지 않았습니다.

충분히 납득가는 이유네요! 😄 이러면 저도 굳이 만들 필요 없다고 생각합니다.

BurningFalls · 2025-05-02T05:47:30Z

backend/src/main/java/com/staccato/staccato/service/StaccatoService.java

-                .map(CategoryMember::getCategory)
-                .map(Category::getId)
-                .toList();
+        return new StaccatoLocationResponsesV2(responses);


이번 변경사항은 아니지만, 전체적인 코드를 보다가 발견했습니다.

이 코드 상단에 있는 Entity -> Dto 변환 로직을 Dto 내부로 이동시키는 것에 대해서 어떻게 생각하시나요?

public record StaccatoLocationResponsesV2(List<StaccatoLocationResponseV2> staccatoLocationResponses) { public static StaccatoLocationResponsesV2 from(List<Staccato> staccatos) { return new StaccatoLocationResponsesV2( staccatos.stream() .map(staccato -> new StaccatoLocationResponseV2(staccato, staccato.getCategory().getColor())) .toList() ); } // ... }

public StaccatoLocationResponsesV2 readAllStaccato(Member member, StaccatoLocationRangeRequest staccatoLocationRangeRequest) { List<Staccato> staccatos = staccatoRepository.findByMemberAndLocationRangeAndCategory( member, staccatoLocationRangeRequest.swLat(), staccatoLocationRangeRequest.neLat(), staccatoLocationRangeRequest.swLng(), staccatoLocationRangeRequest.neLng(), staccatoLocationRangeRequest.categoryId() ); return StaccatoLocationResponsesV2.from(staccatos); }

마찬가지로 이번 변경사항은 아니지만, StaccatoLocationResponseV2를 보다가 발견한건데,

여기서 왜 color를 staccato를 통해서 얻지 않고 직접 전달하는 방식으로 구현되어있는지 궁금하네요.

// 현재 코드 public StaccatoLocationResponseV2(Staccato staccato, Color color) { this(staccato.getId(), color.getName(), staccato.getSpot().getLatitude(), staccato.getSpot().getLongitude()); }

// 수정 버전 public StaccatoLocationResponseV2(Staccato staccato) { this(staccato.getId(), staccato.getColor().getName(), staccato.getSpot().getLatitude(), staccato.getSpot().getLongitude()); }

이 코드 상단에 있는 Entity -> Dto 변환 로직을 Dto 내부로 이동시키는 것에 대해서 어떻게 생각하시나요?

staccato.getCategory().getColor() 를 지연로딩으로 처리하지 않고 fetch Join으로 바로 가져와서 처리하게 된다면 Controller 에서는 트랜잭션이 유지되지 않는 현재 전략을 그대로 가져가면서 변환로직을 DTO 내부로 이동시킬 수 있겠네용

여기서 왜 color를 staccato를 통해서 얻지 않고 직접 전달하는 방식으로 구현되어있는지 궁금하네요.

스타카토는 카테고리의 색상을 따른다는 것은 비즈니스 정책 중 하나라고 생각했어요. 따라서, 서비스 계층에서 알고 처리해야하는 부분이라고 생각했어요.
폭포는 어떻게 생각하시나요?

스타카토는 카테고리의 색상을 따른다는 것은 비즈니스 정책 중 하나라고 생각했어요. 따라서, 서비스 계층에서 알고 처리해야하는 부분이라고 생각했어요.

"스타카토는 카테고리의 색상을 따른다는 것은 비즈니스 정책 중 하나이며, 이는 서비스 계층에서 알고 처리해야 하는 부분이다"라는 부분은 매우 동의합니다. 다만, 저는 코드 그 자체의 가독성과 사용성 관점을 좀더 많이 바라봤던 것 같습니다.

staccatoId를 갖고 있는 staccato는 반드시 정해진 color를 갖고 있습니다. 그런데 public StaccatoLocationResponseV2(Staccato staccato, Color color) 이런 생성자가 있으면, 마치 스타카토와 색깔을 주면 해당 색깔을 갖는 스타카토를 만들어버리는 느낌을 받았습니다.
사용부에서도 Staccato class 내부의 getColor() 메서드를 사용하게 되면 new StaccatoLocationResponseV2(staccato, staccato.getColor()) 이렇게 될텐데, staccato를 줬는데 왜 color를 나눠주는거지 라는 생각이 들게 합니다.

양쪽을 다 챙길 수 있는 방안이 있는지는 좀더 고민해봐야 할것 같네요. 🤔

음... 어떻게 보면 staccato.getColor() 내부에 staccato의 색깔을 결정하는 책임은 staccato에게 있고, 그 색깔이 카테고리의 색깔을 따른다는 것도 그 일부라고 볼 수도 있을 것 같네요!
반영했습니다👍👍

BurningFalls · 2025-05-02T06:55:52Z

backend/src/test/java/com/staccato/staccato/repository/StaccatoRepositoryTest.java

-                    staccato2),
-                () -> assertThat(anotherMemberResult.size()).isEqualTo(1),
-                () -> assertThat(anotherMemberResult).containsExactlyInAnyOrder(anotherStaccato)
+        assertThat(result).hasSize(2).containsExactlyInAnyOrder(staccato1, staccato2);


의미적으로 봤을 때, 생성된 anotherStaccato를 활용해서 아래 코드처럼 만들어도 좋을 것 같습니다.

assertThat(result) .hasSize(2) .containsExactlyInAnyOrder(staccato1, staccato2) .doesNotContain(anotherStaccato);

궁금한 점이 있습니다!
staccato1과 staccato2만 포함이 되어야 한다 == 나머지(anotherStaccato)는 포함되어야하지 않는다. 이기 때문에 의미가 중첩된다고 생각했는데요.
조금 더 구체적인 폭포 의견을 들어보고 싶어요!

anotherStaccato를 만들지 않아도 테스트는 통과합니다. 하지만 그렇게 하면, 단순히 만든 모든 객체를 조회하는 의미밖에 가지지 않습니다. 조건을 만족하지 않는 객체를 만든 의미를 살려서, 해당 객체들은 걸러져서 포함되지 않는다는걸 명시해주고 싶었습니다.

검증 측면에서는 containsExcatlyInAnyOrder() 만으로 이미 충분하다고 생각해요. 사실 anotherStaccato를 만들지 않는다면, 모든 테스트 데이터가 조건에 만족하는 객체이기 때문에 모든 객체가 조회되는 것이 당연하지 않을까 생각이 들었어요. (어떻게 보면, doesNotContain()에 들어갈 데이터를 만들지 않는다는 말이기도 하니까요)
테스트의 가독성이나 의도를 더 드러낸다는 측면에서는 폭포가 말한 방식이 매력적으로 다가오네요! 반영하겠습니다. 좋은 의견 감사해요😆

BurningFalls · 2025-05-02T06:57:31Z

backend/src/test/java/com/staccato/staccato/repository/StaccatoRepositoryTest.java

        );
+
+        // then
+        assertThat(result).hasSize(3).containsExactlyInAnyOrder(inside1, inside2, inside3);


위 코멘트와 동일합니다. doesNotContainAnyElementOf라는 메서드도 있긴 하지만, 굳이라는 생각이 들어서 사용하지 않았습니다. 기능은 똑같고 가독성 측면에서 약간 차이가 있습니다.

assertThat(result) .hasSize(3) .containsExactlyInAnyOrder(inside1, inside2, inside3) .doesNotContain(outsideLat, outsideLng);

BurningFalls · 2025-05-02T07:05:11Z

backend/src/test/java/com/staccato/staccato/service/StaccatoServiceTest.java

+        StaccatoLocationRangeRequest request = new StaccatoLocationRangeRequest(
+                null,
+                new BigDecimal("37.5"),
+                new BigDecimal("127.5"),
+                new BigDecimal("36.5"),
+                new BigDecimal("126.5")
+        );


StaccatoLocationRangeRequestFixtures를 만든다면, 이 코드에서 해당 Fixture를 활용할 수 있을 것 같습니다.

Fixtures를 만들만한 충분한 이유가 사라져서, 이 부분도 그대로 유지가 좋을 것 같아요!

BurningFalls · 2025-05-02T07:26:39Z

backend/src/test/java/com/staccato/staccato/service/StaccatoServiceTest.java

@@ -160,6 +160,61 @@ void readAllStaccato() {
        );
    }

+    @DisplayName("특정 사용자의 위경도 범위 내에 있는 모든 스타카토 목록 조회에 성공한다.")


처음에 보고 색깔을 이용해서 테스트가 복잡하게 구성된 것에 의아함을 느꼈는데, 카테고리에 상관없이 좌표에 포함되는 스타카토들을 불러오는 걸 검증하려다 이렇게 구현하게 된 것을 이해할 수 있었습니다.
좀더 깔끔하게 구성하고 싶은 생각이 들긴 하지만, 저도 더 좋은 방법을 찾지는 못했습니다.

위경도/카테고리 정보 등등 조건에 따라 스타카토 목록이 조회되는 것은 repository의 책임이라고 생각했어요. 따라서, 해당 테스트에서는 조회가 제대로 되고, 색상 매핑이 잘 되는지를 판단하는 정도로 구성하고, repositoryTest를 조금 더 보충했습니다.

BurningFalls · 2025-05-02T07:39:58Z

backend/src/test/java/com/staccato/staccato/repository/StaccatoRepositoryTest.java

+        );
+
+        // then
+        assertThat(result).hasSize(2).containsExactlyInAnyOrder(category1Inside1, category1Inside2);


이 부분도 동일합니다!

assertThat(result) .hasSize(2) .containsExactlyInAnyOrder(category1Inside1, category1Inside2) .doesNotContain(category1Outside, category2Inside);

BurningFalls · 2025-05-02T08:07:58Z

backend/src/main/java/com/staccato/staccato/repository/StaccatoRepository.java

+            )
+            AND (:categoryId IS NULL OR c.id = :categoryId)
+            """)
+    List<Staccato> findByMemberAndLocationRangeAndCategory(


제 생각이 맞을지 모르겠는데, 색깔을 가져올때 staccato.getCategory()를 사용하기 때문에, 이 쿼리에서 fetch join을 사용하지 않으면 N+1 문제가 발생할 것 같습니다. 어떻게 생각하시나요?

N+1이 충분히 발생할 것 같네용

맞아요~ 반영하겠습니다!

Ho-Tea

고생하셨습니다 리니!!!
간단한 코멘트 몇개 남겨놓았으니, 확인 부탁드려요

Ho-Tea · 2025-05-02T08:34:39Z

backend/src/main/java/com/staccato/staccato/repository/StaccatoRepository.java

+            )
+            AND (:categoryId IS NULL OR c.id = :categoryId)
+            """)
+    List<Staccato> findByMemberAndLocationRangeAndCategory(


N+1이 충분히 발생할 것 같네용

Ho-Tea · 2025-05-02T08:37:19Z

...nd/src/main/java/com/staccato/staccato/service/dto/request/StaccatoLocationRangeRequest.java

+public record StaccatoLocationRangeRequest(
+        @Schema(example = SwaggerExamples.CATEGORY_ID)
+        @Min(value = 1L, message = "카테고리 식별자는 양수로 이루어져야 합니다.")
+        Long categoryId,


Swagger를 통해 프로덕션 코드가 더럽혀진다는 것을 확실히 느낄 수 있었습니닷... 문서화에 대해서도 조만간 한번 얘기해 보시죠!!

Ho-Tea · 2025-05-02T08:43:01Z

backend/src/main/java/com/staccato/staccato/service/StaccatoService.java

-                .map(CategoryMember::getCategory)
-                .map(Category::getId)
-                .toList();
+        return new StaccatoLocationResponsesV2(responses);


이 코드 상단에 있는 Entity -> Dto 변환 로직을 Dto 내부로 이동시키는 것에 대해서 어떻게 생각하시나요?

staccato.getCategory().getColor() 를 지연로딩으로 처리하지 않고 fetch Join으로 바로 가져와서 처리하게 된다면 Controller 에서는 트랜잭션이 유지되지 않는 현재 전략을 그대로 가져가면서 변환로직을 DTO 내부로 이동시킬 수 있겠네용

Ho-Tea · 2025-05-02T08:47:13Z

backend/src/test/java/com/staccato/staccato/controller/StaccatoControllerV2Test.java

+                        .param("neLat", invalidParamKey.equals("neLat") ? invalidParamValue : "37.5")
+                        .param("neLng", invalidParamKey.equals("neLng") ? invalidParamValue : "127.0")


삼항 연산자로 표현하신 이유가 궁금해요!

경계값 테스트를 위해 ParameterizedTest를 수행하는데, 어떤 query String key를 대상으로 하는지에 따라 유효값을 사용할지 말지를 결정해야하다보니 조건문 분기가 필요해졌어요.
이 테스트에서 같은 경우에는 조건문으로 분기하는 것이 오히려 가독성을 헤칠 수 있겠다는 생각이 들어 삼항 연산자를 활용했는데,, 테스트가 깔끔하지 않아 마음에 안들긴 하네요! 조금 더 리팩터링 해보겠습니다~

BurningFalls

리니가 남겨주신 답글에 코멘트 전부 달았습니다! 확인해보시고 피드백 남겨주세요~ 😄

linirini · 2025-05-07T11:14:59Z

이전에 슬랙에서 지도 기준 스타카토 목록 조회와 카테고리별 스타카토 목록 조회에 대한 API를 분리하겠다고 말씀드렸었는데, 최종적으로는 분리하지 않기로 결정했습니다.

그 이유는 고민하는 과정에서 제가 API의 책임을 화면 단위로만 바라보고 있었다는 점을 깨달았기 때문입니다.
처음에는 “사용되는 화면이나 맥락이 다르니까, API도 따로 만들어야 하지 않을까?”라는 생각을 했습니다.
특히 하나의 API가 여러 화면에서 재사용되다 보니, 여러 책임을 뒤섞고 있다는 인상을 받았습니다.

그런데 REST 관점에서 다시 고민해보니, staccato라는 동일한 자원을 다양한 조건으로 필터링해 조회하는 것이라면 API가 하나로 묶여 있는 것이 어색하지 않다는 결론에 도달했습니다.
즉, 필터링 조건(지도 범위, 카테고리 등)이 다를 뿐, 결국 같은 리소스에 대한 조회 요청이므로 API를 분리할 필요까지는 없다는 판단입니다.

물론 이 결정은 제 개인적인 판단이고, 혹시 다른 분들은 어떻게 생각하시는지도 궁금합니다!
다른 시각에서 봤을 때 더 나은 API 설계 방향이 있을지 의견 부탁드립니다.

…taccato into feature/#759-get-staccato-by-lng-lat

Ho-Tea

지도 기준 스타카토 목록 조회와 카테고리별 스타카토 목록 조회에 대한 API를 분리하겠다고 말씀드렸었는데, 최종적으로는 분리하지 않기로 결정했습니다.

리니가 담당하신 태스크이고 논리도 납득이 가기에 그대로 진행하셔도 좋을 것 같아요. 그렇지만 의견이 궁금하다고 하셨으니 제 의견을 간다안 하게 남겨보겠습니다.

단순하게 생각해보았을 때.. Staccato는 Category의 하위 리소스로 표현되어 있고, 카테고리 내부 자원에 대한 조회가 이루어지기에 category를 통한 uri 접근이 이루어져야 된다고 생각했습니다.

다른 예로 폴더라는 상위 리소스 안에 파일이라는 하위 리소스가 있었다고 가정하겠습니다. 여기서 화면에 보여지는 모든 파일을 조회하는 것과 특정 a 폴더 내부에 위치하는 파일들을 조회하는 경우 서로 같은 uri를 통해 접근되는 것이 어색하게 느껴집니다.

물론 제시한 예가 완벽히 저희와 동일한 상황이 아니기에 어느정도 이런 느낌이다를 전달하고 싶었습니다 🙃
의견에 대한 토의가 이루어질 것 같으니 RC 우선 드리겠습니다.

Ho-Tea · 2025-05-07T17:08:08Z

backend/src/test/java/com/staccato/staccato/service/StaccatoServiceTest.java

@@ -122,41 +119,38 @@ void failCreateStaccato() {
                .hasMessageContaining("요청하신 카테고리를 찾을 수 없어요.");
    }

-    @DisplayName("스타카토 목록 조회에 성공한다.")
+    @DisplayName("주어진 조건에 만족하는 스타카토 목록을 조회한다.")


사소할 수 있지만... 테스트 메서드명과 @DisplayName이 조금은 삐그덕거리는 느낌이 있습니다.
@DisplayName 에 적혀있는 주어진 조건을 조금더 구체적으로 작성하는건 어떤가요!

linirini · 2025-05-07T18:32:52Z

단순하게 생각해보았을 때.. Staccato는 Category의 하위 리소스로 표현되어 있고, 카테고리 내부 자원에 대한 조회가 이루어지기에 category를 통한 uri 접근이 이루어져야 된다고 생각했습니다.
다른 예로 폴더라는 상위 리소스 안에 파일이라는 하위 리소스가 있었다고 가정하겠습니다. 여기서 화면에 보여지는 모든 파일을 조회하는 것과 특정 a 폴더 내부에 위치하는 파일들을 조회하는 경우 서로 같은 uri를 통해 접근되는 것이 어색하게 느껴집니다.

윽,,설득 되어 버렸습니다............. 팔랑귀는 API 분리하러 갑니다...ㅋㅋㅋㅋ

BurningFalls

정말 수고많으셨습니다, 리니!!!

제가 남긴 코멘트는 답변 + 이후에 고민해보면 좋을 점들 이라서, 당장 변경사항이 필요한 부분은 없기 때문에 approve로 했습니다.

심해 탐험 ㄱㅂㅈㄱ~

BurningFalls · 2025-05-08T13:23:45Z

backend/src/main/java/com/staccato/staccato/service/StaccatoService.java

-                .map(CategoryMember::getCategory)
-                .map(Category::getId)
-                .toList();
+        return StaccatoLocationResponsesV2.of(staccatos);


역시 도메인->Dto 변환 로직은 Dto가 갖고 있어야 서비스 코드가 간결해지네요! 👍 👍 👍

BurningFalls · 2025-05-08T13:27:32Z

...end/src/main/java/com/staccato/staccato/service/dto/response/StaccatoLocationResponseV2.java

-    public StaccatoLocationResponseV2(Staccato staccato, Color color) {
-        this(staccato.getId(), color.getName(), staccato.getSpot().getLatitude(), staccato.getSpot().getLongitude());
+    public StaccatoLocationResponseV2(Staccato staccato) {
+        this(staccato.getId(), staccato.getColor().getName(), staccato.getSpot().getLatitude(), staccato.getSpot()


음... 어떻게 보면 staccato.getColor() 내부에 staccato의 색깔을 결정하는 책임은 staccato에게 있고, 그 색깔이 카테고리의 색깔을 따른다는 것도 그 일부라고 볼 수도 있을 것 같네요!

오히려 저보다 깔끔하게 정리해주셨네요 ㅋㅋㅋ 👍 👍 👍

BurningFalls · 2025-05-08T13:31:21Z

backend/src/test/java/com/staccato/staccato/repository/StaccatoRepositoryTest.java

+
+                // then
+                assertThat(result).hasSize(2).containsExactlyInAnyOrder(staccatoInCategory1, staccatoInCategory2)
+                        .doesNotContain(anotherStaccato);


검증 측면에서는 containsExcatlyInAnyOrder() 만으로 이미 충분하다고 생각해요.

동의합니다! 검증 측면에서는 저도 이것만으로 충분하다고 생각해요.

테스트의 가독성이나 의도를 더 드러낸다는 측면에서는 폭포가 말한 방식이 매력적으로 다가오네요! 반영하겠습니다. 좋은 의견 감사해요😆

다만, 의도를 더 드러내기 위해서였습니다. annotherStaccato 만들고 안쓰면 섭섭해할 것 같아서요 😄

BurningFalls · 2025-05-08T13:32:26Z

backend/src/main/java/com/staccato/staccato/repository/StaccatoRepository.java

+            JOIN FETCH s.category c
+            JOIN FETCH c.categoryMembers cm


알아봤는데, 여기 엄청 좋은 먹잇감(?)이 있네요!

1:N 관계에서 JOIN FETCH를 사용하는 경우, 중복 row로 인해 불필요한 네트워크 트래픽과 메모리 소비를 하게 됩니다. (DISTINCT의 중복 제거는 SQL 레벨이 아닌 JPA에서 동작)

이를 해결할 수 있는 방법을 검색해보니까, 정말 많은 방법이 나오네요. (여기에 다 정리 못할 정도로)

@EntityGraph 사용

DISTINCT + DTO로 매핑

Hibernate @BatchSize

쿼리 분리

저는 각각의 조회 쿼리를 최적화할 수 있고 그만큼 유지보수가 쉬워진다는 장점에서 쿼리 분리가 좋을 것 같습니다. (Category 먼저 조회하는 쿼리 + CategoryMember 별도로 조회하는 쿼리)

일단 JOIN FETCH 쓰도록 만들고, 여러 방법 시도해보면서 시간 개선하며 시나리오 만들어봐요~ ㅎㅎ 😄

BurningFalls · 2025-05-08T14:11:55Z

backend/src/test/java/com/staccato/staccato/domain/StaccatoTest.java

@@ -231,4 +232,22 @@ void updateCategoryUpdatedDateWhenStaccatoDeleted() {
            assertThat(afterDelete).isAfter(beforeDelete);
        }
    }
+
+    @DisplayName("스타카토의 색상은 카테고리의 색상을 따른다.")


이 테스트가 생김으로써 리니가 바라는 도메인 규칙 표현을 할 수 있어서 너무 좋은데요? 👍 👍 👍

BurningFalls · 2025-05-08T14:16:17Z

backend/src/test/java/com/staccato/staccato/controller/StaccatoControllerV2Test.java

+        mockMvc.perform(get("/v2/staccatos")
+                        .param("neLat", neLat)
+                        .param("neLng", neLng)
+                        .param("swLat", swLat)
+                        .param("swLng", swLng)


Fixture 사용보다는 이 방식이 훨씬 낫네요! 👍

BurningFalls · 2025-05-08T14:46:02Z

backend/src/main/java/com/staccato/staccato/repository/StaccatoRepository.java

@@ -12,11 +13,47 @@ public interface StaccatoRepository extends JpaRepository<Staccato, Long> {
    @Query("SELECT s FROM Staccato s WHERE s.category.id = :categoryId order by s.visitedAt desc, s.createdAt desc")
    List<Staccato> findAllByCategoryIdOrdered(long categoryId);

-    List<Staccato> findAllByCategory_CategoryMembers_Member(Member member);
+    @Query("""
+            SELECT DISTINCT s


생각해보니 저희 항상 SELECT ALL을 사용하고 있었네요. 스타카토는 워낙 속성값들이 많다보니, 가져오는 속성 값을 명확하게 지정하면 유의미한 차이가 있을지도 모르겠다는 생각이 들었습니다.

ResponseDto에서 필요한 값이 아닌 다른 값을 비즈니스 로직에서 사용하지 않는다.

엔티티를 수정하여 변경 감지가 일어나야 하는 로직이 없다.

엔티티 연관관계를 사용하지 않는다.

와 같은 이유로, 엔티티 전체 조회말고 DTO projection을 시도해봐도 좋을 것 같습니다! 이것도 일단 지금 방법으로 머지한 후에, 심해탐험에서 해보시죠!

BurningFalls · 2025-05-08T15:07:05Z

backend/src/main/java/com/staccato/category/controller/CategoryController.java

@@ -76,6 +78,17 @@ public ResponseEntity<CategoryDetailResponse> readCategory(
        return ResponseEntity.ok(categoryDetailResponse.toCategoryDetailResponse());
    }

+    @GetMapping("/{categoryId}/staccatos")


지금은 카테고리 하나를 불러올때, 이와 관련된 스타카토까지도 전부 불러오는 방식으로 구현되어있습니다.
만약 카테고리 정보 전체만을(하위 도메인 제외) 불러오는 API 따로, 하나의 카테고리에 담긴 스타카토 전체를 불러오는 API 따로로 만들었다면,
후자의 API를 /categories/{categoryId}/staccatos로 만들었을테고, 그러면 이번엔 이 API랑 겹쳐버리게 되겠네요.
당장 저 API endpoint만 봤을 때는 하나의 카테고리에 속한 스타카토들의 모든 정보를 불러온다는 느낌이 강해서 생각해봤습니다.

그런 의미에서 이 API는 staccatos?categoryId={categoryId}가 되어야 할 것 같은데, 그러면 다시 쿼리스트링의 고민으로 돌아오네요. RESTful한 API 만드는 게 이렇게 어려울 줄이야...
이 부분은 좀더 의논이 필요할 것 같습니다. 일단 이대로 가시죠!

음 저는 호티 의견에 동의했는데요.
category에 속한 staccato들을 조회하는 것이기 때문에 위의 표기가 적합하게 다가왓어요!
그리고

당장 저 API endpoint만 봤을 때는 하나의 카테고리에 속한 스타카토들의 모든 정보를 불러온다는 느낌이 강해서 생각해봤습니다.

이 부분에 대해서는 뒤에 Query String이 따라오지 않는다면 모든 스타카토 목록을 조회하는 것이 맞고, Query String이 따라온다면, 그 범위에 해당하는 스타카토 목록을 조회하는 것이다보니 REST하다고 생각했습니다!

BurningFalls · 2025-05-08T15:22:07Z

backend/src/test/java/com/staccato/staccato/repository/StaccatoRepositoryTest.java

-                .withCategory(anotherMemberCategory).buildAndSave(categoryMemberRepository);
+    @Nested
+    @DisplayName("사용자(member)의 스타카토 목록 조회")
+    class FindAllStaccatosBy {


테스트 엄청 치밀하고 꼼꼼하게 만드셨네요! 수고하셨습니다 👍
가독성도 좋아서 한눈에 이해하기 쉬웠어요 😄

BurningFalls · 2025-05-08T15:23:51Z

backend/src/test/java/com/staccato/staccato/repository/StaccatoRepositoryTest.java

+            @Test
+            void findAllStaccatoByMemberWithLocationRange() {
+                // given
+                BigDecimal threshold = new BigDecimal("0.01");


저도 저번 코드보고 소수점까지 있는 값인데 -1/+1로는 부족하지 않을까 threshold 사용하면 더 좋지 않을까라는 생각했었는데, 리니가 구현해주셨네요 ㅎㅎ
이 방식 좋은 것 같습니다!

Ho-Tea

컨트롤러 분리하는 작업도 꽤 번거로운 작업이셨을텐데, 정말 고생많으셨습니다 리니!!!
오랜만에 Restful한 api가 무엇인가에 대해 깊게 고민해볼 수 있었던것같아요 👍👍👍

Ho-Tea · 2025-05-08T17:14:19Z

backend/src/test/java/com/staccato/staccato/repository/StaccatoRepositoryTest.java

-        CategoryMemberFixtures.defaultCategoryMember()
-                .withMember(anotherMember)
-                .withCategory(anotherMemberCategory).buildAndSave(categoryMemberRepository);
+    @Nested


테스트 코드의 신, 가독성의 신

해도해도 끝이 없는 테스트 세계.....

linirini added 7 commits May 1, 2025 04:18

feat: 위경도 범위 내 스타카토를 조회하는 컨트롤러 구현

ec52b7e

feat: 위경도 범위 내 스타카토를 조회하는 컨트롤러에 위경도 범위 검증 구현

9307c09

feat: 위경도 범위를 서비스 계층에 전달

ac6c6ee

feat: 특정 사용자의 위경도 범위 내 모든 스타카토를 조회하는 service, repository 구현

b9bb9a7

refactor: ReadStaccatoRequest -> StaccatoLocationRangeRequest

50d0c70

feat: 모든 스타카토 조회 시 조건으로 카테고리 식별자 query String 추가

8346b99

feat: 모든 스타카토 조회 시 카테고리 식별자가 있다면 특정 카테고리 내 스타카토만 조회하도록 구현

32ca6fa

linirini added backend We are backend>< feat 기능 (새로운 기능) labels May 2, 2025

linirini added this to the sprint-12 milestone May 2, 2025

linirini self-assigned this May 2, 2025

linirini added this to 2024-staccato May 2, 2025

linirini requested review from BurningFalls and Ho-Tea and removed request for BurningFalls May 2, 2025 03:17

This was linked to issues May 2, 2025

feat: 지도 기준 스타카토 조회 #759

Open

feat: 스타카토 공유 페이지 화면 디자인 개선 (세부 내용 미정) #760

Open

BurningFalls requested changes May 2, 2025

View reviewed changes

Ho-Tea requested changes May 2, 2025

View reviewed changes

linirini requested a review from BurningFalls May 2, 2025 14:47

BurningFalls requested changes May 2, 2025

View reviewed changes

linirini changed the title ~~feat: 모든 스타카토 조회 시 위경도 범위/카테고리를 기준으로 조회할 수 있도록 구현 #759 #760~~ feat: 모든 스타카토 조회 시 위경도 범위/카테고리를 기준으로 조회할 수 있도록 구현 #759 #763 May 3, 2025

linirini removed a link to an issue May 3, 2025

feat: 스타카토 공유 페이지 화면 디자인 개선 (세부 내용 미정) #760

Open

5 tasks

linirini linked an issue May 3, 2025 that may be closed by this pull request

feat: 조회한 카테고리 기준으로 지도에 마커 표시 #763

Open

6 tasks

linirini added 3 commits May 6, 2025 01:42

refactor: refactor staccatoLocationResponsesV2 parameter

7fbd1c7

test: doesNotContain으로 테스트 의도와 가독성 향상

c99b46c

refactor: fetch join 적용

7223c1b

linirini added 7 commits May 7, 2025 17:01

test: 검증 목적에 맞게 테스트 리팩터링 및 제거

747428d

test: 스타카토 색상 도메인 테스트 추가

fb65ebc

test: 조건에 따른 모든 스타카토 조회 서비스 테스트 수정

67a32b8

test: 위경도 범위 내 스타카토 목록 조회 테스트 오류 수정

a8ade4f

test: 스타카토 목록 조회 시 조건에 따른 repository 테스트 분리

53022c5

test: 스타카토 목록 조회 컨트롤러 테스트 리팩터링

bf875f0

test: 테스트 오류 수정

7d94320

linirini requested review from BurningFalls and Ho-Tea May 7, 2025 11:15

Merge branch 'develop' of https://github.com/woowacourse-teams/2024-s…

8c8866a

…taccato into feature/#759-get-staccato-by-lng-lat

Ho-Tea requested changes May 7, 2025

View reviewed changes

linirini added 4 commits May 8, 2025 11:06

refactor: API 분리로 인한 불필요해진 필드(categoryId) 제거 및 repository 메서드 분리

1e00609

refactor: 카테고리 기준 위경도 범위 내 스타카토 목록 조회 API 분리 (컨트롤러, 서비스 구현)

c8155bc

style: reformat code

2d0d830

test: 위경도/카테고리에 따른 스타카토 목록 조회 repository 테스트 보충

9b23842

linirini requested a review from Ho-Tea May 8, 2025 02:23

BurningFalls approved these changes May 8, 2025

View reviewed changes

Ho-Tea approved these changes May 8, 2025

View reviewed changes

linirini merged commit a1867fa into develop May 9, 2025
2 checks passed

linirini deleted the feature/#759-get-staccato-by-lng-lat branch May 9, 2025 07:10

github-project-automation bot moved this to Done in 2024-staccato May 9, 2025

linirini mentioned this pull request May 9, 2025

refactor: 위경도 범위 조회에 공간 데이터 타입(Point) 도입 #818

Open

		.param("neLat", invalidParamKey.equals("neLat") ? invalidParamValue : "37.5")
		.param("neLng", invalidParamKey.equals("neLng") ? invalidParamValue : "127.0")

feat: 모든 스타카토 조회 시 위경도 범위/카테고리를 기준으로 조회할 수 있도록 구현 #759 #763 #779

feat: 모든 스타카토 조회 시 위경도 범위/카테고리를 기준으로 조회할 수 있도록 구현 #759 #763 #779

Uh oh!

Conversation

linirini commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⭐️ Issue Number

🚩 Summary

🛠️ Technical Concerns

🙂 To Reviewer

📋 To Do

Uh oh!

github-actions bot commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

github-actions bot commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌻Test Coverage Report

Uh oh!

BurningFalls 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!

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!

BurningFalls May 2, 2025 • 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!

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!

Ho-Tea 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!

Choose a reason for hiding this comment

Uh oh!

linirini commented May 2, 2025 •

edited

Loading

github-actions bot commented May 2, 2025 •

edited

Loading

github-actions bot commented May 2, 2025 •

edited

Loading

BurningFalls May 2, 2025 •

edited

Loading

Ho-Tea left a comment •

edited

Loading