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

get- path-detail 과 post- path -favorites API를 구현, 리팩토링했습니다. #18

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -4,6 +4,8 @@
import com.walking.api.web.dto.response.detail.PointDetail;
import com.walking.api.web.dto.response.detail.TrafficDetail;
import com.walking.data.entity.traffic.TrafficEntity;
import java.util.List;
import java.util.stream.Collectors;

public final class TrafficDetailConverter {

Expand All @@ -21,15 +23,44 @@ public static TrafficDetail execute(PredictedData predictedData) {

return TrafficDetail.builder()
.id(trafficEntity.getId())
.color(predictedData.getCurrentColor().toString())
.timeLeft(predictedData.getCurrentTimeLeft())
.color(predictedData.getCurrentColorDescription())
.timeLeft(predictedData.getCurrentTimeLeft().orElse(null))
.point(
PointDetail.builder().lng(trafficEntity.getLng()).lat(trafficEntity.getLat()).build())
.redCycle(predictedData.getRedCycle())
.greenCycle(predictedData.getGreenCycle())
.redCycle(predictedData.getRedCycle().orElse(null))
.greenCycle(predictedData.getGreenCycle().orElse(null))
.detail(TrafficDetailInfoConverter.execute(trafficEntity))
.isFavorite(false)
.viewName(trafficEntity.getName())
.build();
}

/**
* PredictedData를 기반으로 TrafficDetail의 List를 생성합니다.
*
* @param predictedData 사이클 정보 와 현재 색상 및 잔여시간을 예측한 데이터 리스트
* @return 예측 값을 바탕으로 만든 TrafficDetail의 List
*/
public static List<TrafficDetail> execute(List<PredictedData> predictedData) {

return predictedData.stream()
.map(
predictedDatum ->
TrafficDetail.builder()
.id(predictedDatum.getTraffic().getId())
.color(predictedDatum.getCurrentColorDescription())
.timeLeft(predictedDatum.getCurrentTimeLeft().orElse(null))
.point(
PointDetail.builder()
.lng(predictedDatum.getTraffic().getLng())
.lat(predictedDatum.getTraffic().getLat())
.build())
.redCycle(predictedDatum.getRedCycle().orElse(null))
.greenCycle(predictedDatum.getGreenCycle().orElse(null))
.detail(TrafficDetailInfoConverter.execute(predictedDatum.getTraffic()))
.isFavorite(false)
.viewName(predictedDatum.getTraffic().getName())
.build())
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,37 @@ public interface TrafficRepository extends JpaRepository<TrafficEntity, Long> {
nativeQuery = true)
List<TrafficEntity> findClosetTrafficByLocation(
@Param("longitude") Double longitude, @Param("latitude") Double latitude);

@Query(
value =
"SELECT * FROM traffic "
Copy link

Choose a reason for hiding this comment

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

주어진 코드 패치에 대한 간략한 코드 리뷰:

  1. 주변 교통 데이터 조회 쿼리 (findClosetTrafficByLocation) 사용시 주의사항:

    • ST_DISTANCE 함수를 통해 거리를 계산할 때 정확성과 성능을 위해 인덱스를 확인할 것.
    • 입력값(:longitude, :latitude) 유효성 검사 필요 (null 체크 등)
    • Polygon 생성 부분의 좌표 계산이 정확한지 확인
    • Subquery를 사용하기 때문에 큰 데이터셋에서는 성능 이슈가 발생할 수 있음
  2. SQL 쿼리와 지역적 기능 관련 테스트를 추가하여 작동 여부 확인이 필요.

  3. @Query 어노테이션 명확하게 사용 및 설명 주석 추가 권장.

  4. 오타나 문법적 오류가 있는지 세심히 확인하고 수정.

  5. 잠재적인 보안 문제 시 주변 코드와 함께 검토.

  6. Repository 메소드 네이밍 명확성을 높일 수 있도록 고민.

  7. 코드의 가독성을 높일 수 있도록 줄 바꾸기 등 포맷팅 확인.

  8. 코드 리뷰 이후 동료나 팀 내 다른 멤버들과 리뷰 결과 공유.

코드 최적화 및 버그 리스크를 줄이는 방향으로 진행하는 것이 중요합니다.

+ "WHERE ST_Equals(point_value, "
+ "ST_PointFromText(CONCAT('POINT(', :lat, ' ', :lng, ')'), 4326))",
nativeQuery = true)
List<TrafficEntity> findByLocation(@Param("lat") Double lat, @Param("lng") Double lng);

@Query(
"SELECT t FROM TrafficEntity t "
+ "WHERE FUNCTION('ST_Distance_Sphere', t.point, "
+ "FUNCTION('ST_PointFromText', CONCAT('POINT(', :lat, ' ', :lng, ')'), 4326)) < :distance")
List<TrafficEntity> findByLocationAndDistance(
@Param("lat") Float lat, @Param("lng") Float lng, @Param("distance") Integer distance);

@Query(
value =
"SELECT * FROM traffic "
+ "WHERE ST_Contains("
+ " ST_SRID("
+ " ST_MakeEnvelope("
+ " POINT(:blLng, :blLat), "
+ " POINT(:trLng, :trLat)"
+ " ), 4326"
+ " ), point_value"
+ ")",
nativeQuery = true)
List<TrafficEntity> findTrafficWithinBounds(
@Param("blLng") float blLng,
@Param("blLat") float blLat,
@Param("trLng") float trLng,
@Param("trLat") float trLat);
}
27 changes: 25 additions & 2 deletions api/src/main/java/com/walking/api/service/dto/PredictedData.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public PredictedData(TrafficEntity traffic) {
* @return redCycle 이 존재하면 true
*/
public boolean isPredictedRedCycle() {
return Objects.nonNull(getRedCycle());
return getRedCycle().isPresent();
}

/**
Expand All @@ -41,7 +41,7 @@ public boolean isPredictedRedCycle() {
* @return greenCycle 이 존재하면 true
*/
public boolean isPredictedGreenCycle() {
return Objects.nonNull(getGreenCycle());
return getGreenCycle().isPresent();
}

/**
Expand Down Expand Up @@ -104,4 +104,27 @@ public void updateCurrentColor(TrafficColor color) {
public void updateCurrentTimeLeft(Float timeLeft) {
this.currentTimeLeft = timeLeft;
}

public Optional<Float> getRedCycle() {
return Optional.ofNullable(redCycle);
}

public Optional<Float> getGreenCycle() {
return Optional.ofNullable(greenCycle);
}

public Optional<TrafficColor> getCurrentColor() {
return Optional.ofNullable(currentColor);
}

public Optional<Float> getCurrentTimeLeft() {
return Optional.ofNullable(currentTimeLeft);
}

public String getCurrentColorDescription() {
if (getCurrentColor().isPresent()) {
return getCurrentColor().get().toString();
}
return "";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,13 @@ public TrafficEntity executeById(Long trafficId) {
public List<TrafficEntity> executeByIds(List<Long> trafficIds) {
return trafficRepository.findByIds(trafficIds);
}

public List<TrafficEntity> executeByLocationAndDistance(Float lat, Float lng, Integer distance) {
return trafficRepository.findByLocationAndDistance(lat, lng, distance);
}

public List<TrafficEntity> executeWithinBounds(
Float blLng, Float blLat, Float trLng, Float trLat) {
return trafficRepository.findTrafficWithinBounds(blLng, blLat, trLng, trLat);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import com.walking.api.service.dto.PredictedData;
import com.walking.api.service.dto.request.IntegrationPredictRequestDto;
import com.walking.api.service.dto.response.IntegrationPredictResponseDto;
import com.walking.api.web.dto.request.point.OptionalTrafficPointParam;
import com.walking.api.service.traffic.ReadTrafficService;
import com.walking.api.web.dto.request.point.OptionalViewPointParam;
import com.walking.api.web.dto.request.point.ViewPointParam;
import com.walking.api.web.dto.request.traffic.FavoriteTrafficBody;
import com.walking.api.web.dto.request.traffic.PatchFavoriteTrafficNameBody;
import com.walking.api.web.dto.response.BrowseFavoriteTrafficsResponse;
Expand All @@ -20,17 +21,18 @@
import com.walking.api.web.support.ApiResponse;
import com.walking.api.web.support.ApiResponseGenerator;
import com.walking.api.web.support.MessageCode;
import com.walking.data.entity.BaseEntity;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
import javax.validation.Valid;
import javax.validation.constraints.Min;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpStatus;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -49,6 +51,7 @@
public class TrafficController {

private final TrafficIntegrationPredictService integrationPredictService;
private final ReadTrafficService readTrafficService;

static double TF_BACK_DOOR_LAT = 35.178501;
static double TF_BACK_DOOR_LNG = 126.912083;
Expand All @@ -69,23 +72,39 @@ public class TrafficController {

@GetMapping()
public ApiResponse<ApiResponse.SuccessBody<SearchTrafficsResponse>> searchTraffics(
@Valid OptionalViewPointParam viewPointParam,
@Valid OptionalTrafficPointParam trafficPointParam) {
if (!Objects.isNull(trafficPointParam) && trafficPointParam.isPresent()) {
// todo implement: trafficPointParam를 이용하여 교차로 정보 조회
log.info("Search traffics trafficPointParam: {}", trafficPointParam.get());
SearchTrafficsResponse response = getSearchViewTrafficsResponse();
return ApiResponseGenerator.success(response, HttpStatus.OK, MessageCode.SUCCESS);
}
@Valid OptionalViewPointParam viewPointParam) {

// todo implement: viewPointParam을 이용하여 교차로 정보 조회
// viewPointParam을 이용하여 교차로 정보 조회
log.info("Search traffics viewPointParam: {}", viewPointParam.get());
SearchTrafficsResponse response = getSearchTrafficsResponse();
ViewPointParam viewPoint = viewPointParam.getViewPointParam();
float vblLng = viewPoint.getVblLng();
float vblLat = viewPoint.getVblLat();
float vtrLng = viewPoint.getVtrLng();
float vtrLat = viewPoint.getVtrLat();

List<Long> trafficIds =
readTrafficService.executeWithinBounds(vblLng, vblLat, vtrLng, vtrLat).stream()
.map(BaseEntity::getId)
.collect(Collectors.toList());

// trafficDetail 생성
List<PredictedData> predictedData =
new ArrayList<>(
integrationPredictService
.execute(IntegrationPredictRequestDto.builder().trafficIds(trafficIds).build())
.getPredictedDataMap()
.values());

List<TrafficDetail> traffics = TrafficDetailConverter.execute(predictedData);
SearchTrafficsResponse response = SearchTrafficsResponse.builder().traffics(traffics).build();
return ApiResponseGenerator.success(response, HttpStatus.OK, MessageCode.SUCCESS);
}

// todo: HttpServletRequest 파라미터로 설정하고 여기서 header에 있는 Authentication 을 가지고 토큰을 가져오도록
// 직접 header에서 token을 가져오는 이유 -> spring security를 사용하게되면 로그인하지 않은 사용자의 경우 토큰이 전달되지 않으므로
// filter에서 걸러져서 요청이 안들어옴
// Security package 안에 토큰 필터 -> request를 어떻게 가져오는지 보고 그냥 가져다 쓰면 된다.
@GetMapping("/{trafficId}")
@Transactional
public ApiResponse<ApiResponse.SuccessBody<BrowseTrafficsResponse>> browseTraffic(
@PathVariable Long trafficId) {
// todo implement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
@Builder
public class ViewPointParam {
/* 화면 좌측 하단 위도 */
@LatParam private double vblLat;
@LatParam private float vblLat;

/* 화면 좌측 하단 경도 */
@LngParam private double vblLng;
@LngParam private float vblLng;

/* 화면 우측 상단 위도 */
@LatParam private double vtrLat;
@LatParam private float vtrLat;

/* 화면 우측 상단 경도 */
@LngParam private double vtrLng;
@LngParam private float vtrLng;
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ public Object resolveArgument(
return OptionalViewPointParam.builder()
.viewPointParam(
ViewPointParam.builder()
.vblLat(Double.parseDouble(vblLat))
.vblLng(Double.parseDouble(vblLng))
.vtrLat(Double.parseDouble(vtrLat))
.vtrLng(Double.parseDouble(vtrLng))
.vblLat(Float.parseFloat(vblLat))
.vblLng(Float.parseFloat(vblLng))
.vtrLat(Float.parseFloat(vtrLat))
.vtrLng(Float.parseFloat(vtrLng))
.build())
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ void searchTrafficsWithViewPointParam() throws Exception {
.perform(
get(BASE_URL)
.contentType(MediaType.APPLICATION_JSON)
.param("vblLat", "35.175840")
.param("vblLng", "126.912490")
.param("vtrLat", "35.178526")
.param("vtrLng", "124.123457"))
.param("vblLat", "37.595000")
.param("vblLng", "127.07870")
.param("vtrLat", "37.605372")
.param("vtrLng", "127.080309"))
.andExpect(status().is2xxSuccessful())
.andDo(
document(
Expand Down
Loading