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

[BE] refactor: 발자국 리팩터링 #92

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9dbc53a
refactor: 위도, 경도로 거리 계산하는 메서드 가독성 개선
takoyakimchi Jul 17, 2024
1c73276
refactor: 반경 meter의 기준을 메서드의 파라미터로 넣을 수 있게 변경
takoyakimchi Jul 17, 2024
f4eb4df
refactor: 변수명 및 상수명 가독성 개선
takoyakimchi Jul 17, 2024
51fb137
refactor: 예외 메시지 구체화
takoyakimchi Jul 17, 2024
feec061
refactor: Query Parameter를 DTO로 받도록 변경
takoyakimchi Jul 17, 2024
117b737
refactor: 주변 발자국 조회 명세에서 축약어 사용
takoyakimchi Jul 17, 2024
cd088ae
test: FootprintController REST Docs용 테스트 추가
takoyakimchi Jul 17, 2024
e1b21fb
chore: api문서 자동화 의존성 추가
jimi567 Jul 17, 2024
9eff84d
test: 문서용 테스트 base클래스 및 예시 테스트 작성
jimi567 Jul 17, 2024
af6464b
feat: CORS 설정 추가
jimi567 Jul 17, 2024
b7ec36e
[BE] feat: 커스텀 예외 도입 및 GlobalExceptionHandler 처리 예외 수정 (#93)
J-I-H-O Jul 17, 2024
231cc29
[BE] chore: CI 구축 (#87)
ehtjsv2 Jul 17, 2024
d4e8569
[AN] feat: 채팅 ui 구현 (#83)
gaeun5744 Jul 17, 2024
a4b9402
refactor: 메서드명을 동사로 시작하도록 변경
takoyakimchi Jul 17, 2024
504b713
refactor: 예외 클래스 변경
takoyakimchi Jul 17, 2024
a12912a
refactor: 상수 이름 변경
takoyakimchi Jul 17, 2024
39baf3c
refactor: 위도, 경도 축약어 사용하지 않도록 변경
takoyakimchi Jul 17, 2024
4e350b1
test: 발자국 API 문서화 클래스를 포맷에 맞게 수정
takoyakimchi Jul 17, 2024
451caa7
[BE] refactor: 회원 도메인 리팩토링 (#95)
J-I-H-O Jul 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@RestController
Expand All @@ -32,10 +31,7 @@ public ResponseEntity<Void> save(@RequestBody SaveFootprintRequest request) {
}

@GetMapping("/near")
public List<FindNearFootprintResponse> findNear(
@RequestParam double lat,
@RequestParam double lng
) {
return footprintQueryService.findNear(new FindNearFootprintRequest(lat, lng));
public List<FindNearFootprintResponse> findNear(FindNearFootprintRequest request) {
return footprintQueryService.findNear(request);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
@NoArgsConstructor
public class Footprint {

private static final int RADIUS_AS_METER = 1000;

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
Expand All @@ -40,6 +42,6 @@ public Footprint(Member member, Location location) {
}

public boolean isNear(Location location) {
return this.location.isNear(location);
return this.location.isWithin(location, RADIUS_AS_METER);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.woowacourse.friendogly.footprint.domain;

import static java.lang.Math.*;
Copy link
Contributor

@ehtjsv2 ehtjsv2 Jul 17, 2024

Choose a reason for hiding this comment

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

인텔리제이에서 자동으로 와일드카드를 사용하게 하는 옵션이 켜져있는 걸까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reformat 단축키 누르니까 잘 되네요 ,...!


import jakarta.persistence.Embeddable;
import lombok.AccessLevel;
import lombok.AllArgsConstructor;
Expand All @@ -12,32 +14,19 @@
@AllArgsConstructor
public class Location {

private static final int BOUNDARY_RADIUS_METER = 1000;

private double latitude;
private double longitude;
Comment on lines 22 to 23
Copy link
Contributor

Choose a reason for hiding this comment

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

약어만 사용하거나, 정확한 표현만 사용하거나 둘 중 하나로 통일하면 좋겠어요.

추가로 약어를 사용하는게 과연 좋을지 잘 모르겠어요.
오랜 시간이 지나고 해당 도메인을 유지보수할 때, 경도 를 나타내는 표현이 lon 이었는지 lng 였는지 헷갈리지 않을까요?

또 여러 개발자들과 협업하는 상황에서는 인지 비용을 줄이기 위해서라도 정확한 표현을 사용하는 것이 좋지 않을까 생각합니다. 쿼리 파라미터에서도 사용될 표현이기 때문에 안드로이드 크루들도 인지해야 한다는 점 또한 고려해야겠네요.

개인적으로, 적어도 협업에 있어서는 개발의 간편함 보다는 이해하기 쉬운 코드를 작성하는 것이 더 높은 우선순위를 가져야하지 않나 싶습니다.

투기장 오픈, 아무나 참전 가능(1/8)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

query parameter를 짧게 가져가고 싶은 욕심이 있었는데, 일관성까지 챙기기는 확실히 어렵네요.


public boolean isNear(Location other) {
return distance(this.latitude, this.longitude, other.latitude, other.longitude) <= BOUNDARY_RADIUS_METER;
public boolean isWithin(Location other, int radius) {
return distanceAsMeter(other) <= radius;
}

private double distance(double lat1, double lon1, double lat2, double lon2) {
double theta = lon1 - lon2;
double dist = Math.sin(degToRad(lat1)) * Math.sin(degToRad(lat2)) +
Math.cos(degToRad(lat1)) * Math.cos(degToRad(lat2)) * Math.cos(degToRad(theta));

dist = Math.acos(dist);
dist = radToDeg(dist);
dist = dist * 60 * 1.1515 * 1609.344;

return Math.abs(dist);
}

private double degToRad(double deg) {
return (deg * Math.PI / 180.0);
}
private double distanceAsMeter(Location other) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private double distanceAsMeter(Location other) {
private double calculateDistanceAsMeter(Location other) {

메서드명은 동사로 시작하는게 좋겠습니당

Copy link
Contributor Author

Choose a reason for hiding this comment

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

동사로 통일!! 👍

double theta = this.longitude - other.longitude;
double dist = sin(toRadians(this.latitude)) * sin(toRadians(other.latitude)) +
cos(toRadians(this.latitude)) * cos(toRadians(other.latitude)) * cos(toRadians(theta));

private double radToDeg(double rad) {
return (rad * 180 / Math.PI);
dist = toDegrees(acos(dist));
return abs(dist * 60 * 1.1515 * 1609.344);
Copy link
Member

Choose a reason for hiding this comment

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

return abs(dist * 60 * 1.1515 * 1609.344);

이 부분 공식에서 각 값에 의미를 부여하기 힘들다면,
해당 반환문을 메서드로 따로 빼서 네이밍 해주면 이해하기 쉬울 것 같은데 어떠신가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 어차피 공식이 워낙 복잡해서 하나의 메서드로 가져가도 될 것 같아요...!

}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package com.woowacourse.friendogly.footprint.dto.request;

public record FindNearFootprintRequest(double latitude, double longitude) {
public record FindNearFootprintRequest(double lat, double lng) {

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class FootprintCommandService {

public Long save(SaveFootprintRequest request) {
Member member = memberRepository.findById(request.memberId())
.orElseThrow(() -> new IllegalArgumentException("멤버 없음"));
.orElseThrow(() -> new IllegalArgumentException("존재하지 않는 사용자 ID입니다."));
Copy link
Contributor

Choose a reason for hiding this comment

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

#93
develop 브랜치 rebase 후에 IllegalArgumnetException 대신 Custom Exception 사용해주세요!

Copy link
Member

Choose a reason for hiding this comment

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

이 부분도 저희 커스텀 예외 만들어둔걸로 대체해주세요.!

예외처리 repo로 내리는건 추후에 service 두꺼워지면 고려해봅시다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영했습니다 👍


Footprint footprint = footprintRepository.save(
Footprint.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@
@RequiredArgsConstructor
public class FootprintQueryService {

private static final int HOURS_AGO = 24;
private static final int FOOTPRINT_REMAIN_HOURS = 24;
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 상수는 실제로 현재 시간으로 부터 n시간 이내 를 나타내는데 사용되는 반면,
REMAIN_HOURS 라는 네이밍은 남은 시간 이라는 의미가 강한 것 같아요. 조금 더 적절한 네이밍을 고민해보면 좋겠습니다!


private final FootprintRepository footprintRepository;

public List<FindNearFootprintResponse> findNear(FindNearFootprintRequest request) {
LocalDateTime since = LocalDateTime.now().minusHours(HOURS_AGO);
List<Footprint> recentFootprints = footprintRepository.findByCreatedAtAfter(since);
LocalDateTime startTime = LocalDateTime.now().minusHours(FOOTPRINT_REMAIN_HOURS);
List<Footprint> recentFootprints = footprintRepository.findByCreatedAtAfter(startTime);

Location currentLocation = new Location(request.latitude(), request.longitude());
Location currentLocation = new Location(request.lat(), request.lng());

return recentFootprints.stream()
.filter(footprint -> footprint.isNear(currentLocation))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package com.woowacourse.friendogly.footprint.controller;

import static org.mockito.BDDMockito.given;
import static org.springframework.http.HttpHeaders.LOCATION;
import static org.springframework.http.MediaType.APPLICATION_JSON;
import static org.springframework.restdocs.headers.HeaderDocumentation.headerWithName;
import static org.springframework.restdocs.headers.HeaderDocumentation.responseHeaders;
import static org.springframework.restdocs.mockmvc.MockMvcRestDocumentation.document;
import static org.springframework.restdocs.operation.preprocess.Preprocessors.preprocessRequest;
import static org.springframework.restdocs.operation.preprocess.Preprocessors.preprocessResponse;
import static org.springframework.restdocs.operation.preprocess.Preprocessors.prettyPrint;
import static org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath;
import static org.springframework.restdocs.payload.PayloadDocumentation.requestFields;
import static org.springframework.restdocs.payload.PayloadDocumentation.responseFields;
import static org.springframework.restdocs.request.RequestDocumentation.parameterWithName;
import static org.springframework.restdocs.request.RequestDocumentation.queryParameters;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.woowacourse.friendogly.footprint.dto.request.FindNearFootprintRequest;
import com.woowacourse.friendogly.footprint.dto.request.SaveFootprintRequest;
import com.woowacourse.friendogly.footprint.dto.response.FindNearFootprintResponse;
import com.woowacourse.friendogly.footprint.service.FootprintCommandService;
import com.woowacourse.friendogly.footprint.service.FootprintQueryService;
import java.time.LocalDateTime;
import java.util.List;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.restdocs.AutoConfigureRestDocs;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.web.servlet.MockMvc;

@AutoConfigureRestDocs
@WebMvcTest(FootprintController.class)
class FootprintControllerTest {

@Autowired
private MockMvc mockMvc;

@Autowired
private ObjectMapper objectMapper;

@MockBean
private FootprintCommandService footprintCommandService;
Copy link
Member

Choose a reason for hiding this comment

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

이거 RestDocs Base 코드 있어요. 추상 클래스라서 예시 Member 문서 테스트 참고해서 다시 작성해주시면 감사하겠습니다.


@MockBean
private FootprintQueryService footprintQueryService;

@DisplayName("발자국 저장")
@Test
void save() throws Exception {
SaveFootprintRequest request = new SaveFootprintRequest(1L, 37.5173316, 127.1011661);

given(footprintCommandService.save(request))
.willReturn(1L);

mockMvc
.perform(post("/footprints")
.content(objectMapper.writeValueAsString(request))
.contentType(APPLICATION_JSON))
.andDo(print())
.andDo(document("footprints/save",
preprocessRequest(prettyPrint()),
preprocessResponse(prettyPrint()),
requestFields(
fieldWithPath("memberId").description("사용자 ID"),
fieldWithPath("latitude").description("현재 위치의 위도"),
fieldWithPath("longitude").description("현재 위치의 경도")
),
responseHeaders(
headerWithName(LOCATION).description("Location")
)
))
.andExpect(status().isCreated())
.andExpect(header().string(LOCATION, "/footprints/1"));
}

@DisplayName("주변 발자국 조회")
@Test
void findNear() throws Exception {
FindNearFootprintRequest request = new FindNearFootprintRequest(37.5173316, 127.1011661);

given(footprintQueryService.findNear(request))
.willReturn(
List.of(
new FindNearFootprintResponse(1L, 37.5136533, 127.0983182, LocalDateTime.now().minusMinutes(10)),
new FindNearFootprintResponse(3L, 37.5131474, 127.1042528, LocalDateTime.now().minusMinutes(20)),
new FindNearFootprintResponse(6L, 37.5171728, 127.1047797, LocalDateTime.now().minusMinutes(30)),
new FindNearFootprintResponse(11L, 37.516183, 127.1068874, LocalDateTime.now().minusMinutes(40))
)
);

mockMvc
.perform(get("/footprints/near")
.param("lat", "37.5173316")
.param("lng", "127.1011661"))
.andDo(print())
.andDo(document("footprints/near",
preprocessRequest(prettyPrint()),
preprocessResponse(prettyPrint()),
queryParameters(
parameterWithName("lat").description("현재 위치의 위도"),
parameterWithName("lng").description("현재 위치의 경도")
),
responseFields(
fieldWithPath("[].memberId").description("발자국을 찍은 사용자 ID"),
fieldWithPath("[].latitude").description("발자국 위치의 위도"),
fieldWithPath("[].longitude").description("발자국 위치의 경도"),
fieldWithPath("[].createdAt").description("발자국 생성 시간")
)
))
.andExpect(status().isOk());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ class LocationTest {
void isNear_True() {
Location location1 = new Location(0.0, 0.0);
Location location2 = new Location(0.0, 0.008993216);
assertThat(location1.isNear(location2)).isTrue();
assertThat(location1.isWithin(location2, 1000)).isTrue();
}

@DisplayName("약 1001m 차이 나는 두 위치는 주변에 있지 않다.")
@Test
void isNear_False() {
Location location1 = new Location(0.0, 0.0);
Location location2 = new Location(0.0, 0.009001209);
assertThat(location1.isNear(location2)).isFalse();
assertThat(location1.isWithin(location2, 1000)).isFalse();
}
}