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.

Sign up for GitHub

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

Jump to bottom

[hotfix] Spot 저장방식 수정 및 추가 #155

Merged
merged 8 commits into from
Oct 12, 2023
Merged

[hotfix] Spot 저장방식 수정 및 추가 #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fa94f1a
feat : db에 spot 저장 시 about도 함께 추가
chlqls Oct 2, 2023
bc2cfea
refactor : else 사용 지양하기 위해 수정
chlqls Oct 3, 2023
f99adeb
refactor : TourAPI 사용 시 중복되는 URL 부분 정리
chlqls Oct 11, 2023
c4e31ad
feat : db에 spot 저장 시 modifiedTime도 함께 추가
chlqls Oct 11, 2023
dcc4cfb
feat : 관광정보 로컬서버 저장을 위한 동기화 구현
chlqls Oct 11, 2023
a2b63ef
feat : spot 썸네일 없는 경우 임의로 상세 이미지들 중 첫번째 이미지를 썸네일로 설정
chlqls Oct 11, 2023
65c16f8
feat : spot의 상세 이미지가 없을 때는 썸네일 이미지 보여주도록 구현
chlqls Oct 11, 2023
8dd2a23
Merge branch 'develop' into feature/133-spot
chlqls Oct 12, 2023
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
21 changes: 10 additions & 11 deletions api-server/src/main/java/com/kuddy/apiserver/heart/service/HeartService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,17 @@ public ResponseEntity<StatusResponse> likeSpot(Long contentId, Member member){

if(heartRepository.findByMemberAndSpot(member, spot).isPresent()) {
throw new AlreadyLikedException();
} else {
heartRepository.save(Heart.builder()
.member(member)
.spot(spot)
.build());
spot.likeSpot();
return ResponseEntity.ok(StatusResponse.builder()
.status(StatusEnum.OK.getStatusCode())
.message(StatusEnum.OK.getCode())
.data(contentId+"번 관광지 찜")
.build());
}
heartRepository.save(Heart.builder()
.member(member)
.spot(spot)
.build());
spot.likeSpot();
return ResponseEntity.ok(StatusResponse.builder()
.status(StatusEnum.OK.getStatusCode())
.message(StatusEnum.OK.getCode())
.data(contentId+"번 관광지 찜")
.build());
}

public ResponseEntity<StatusResponse> cancelSpotLike(Long contentId, Member member) {
Copy link

Choose a reason for hiding this comment

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

코드 리뷰를 해드리겠습니다.

  1. patch 코드는 없기 때문에 현재 코드 스니펫만을 기준으로 리뷰를 진행하겠습니다.
  2. 리뷰할 코드 부분은 "likeSpot" 메서드의 수정 부분입니다.

해당 코드의 문제점이나 개선 제안은 다음과 같습니다.

  1. 이미 좋아요한 내역을 확인하는 로직(if(heartRepository.findByMemberAndSpot(member, spot).isPresent()))에서 else 분기가 제거되었습니다. 이로 인해 이미 좋아요한 경우와 좋아요하지 않은 경우 구분이 없어지게 됩니다. 예외 처리 부분을 잘 검토해보세요.
  2. 좋아요 관련 정보를 저장하는 부분(heartRepository.save(), spot.likeSpot())이 항상 실행됩니다. 이미 좋아요한 경우라도 무조건 처리되므로, 중복 저장 및 카운트 증가 등의 부작용이 있을 수 있습니다. 불필요한 DB 작업을 줄일 수 있는 방법을 고려해보세요.
  3. 응답 결과로 반환되는 StatusResponse 객체 생성 부분은 동일한 로직이 throw 전과 throw 후에 모두 존재합니다. 중복된 코드를 제거하고 한 곳에서 생성하여 사용하도록 개선해보세요.

이상이 코드 리뷰에 대한 제안사항입니다. 해당 부분들을 고려하여 코드를 수정하면 더욱 개선된 결과를 얻을 수 있을 것입니다.

Expand Down
12 changes: 5 additions & 7 deletions api-server/src/main/java/com/kuddy/apiserver/spot/controller/SpotController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
import com.kuddy.common.response.StatusResponse;
import com.kuddy.common.spot.domain.Spot;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.json.simple.JSONArray;
import org.springframework.data.domain.Page;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;


@RestController
@RequestMapping("/api/v1/spots")
@RequiredArgsConstructor
Expand All @@ -21,11 +21,9 @@ public class SpotController {
private final SpotService spotService;
private final TourApiService tourApiService;

//테스트용으로 관광정보 저장하는 api
@PostMapping("/test")
public ResponseEntity<StatusResponse> saveTestData(@RequestParam(value = "page") int page, @RequestParam(value = "category") int category) {
spotService.changeAndSave(tourApiService.getApiDataList(page, category));

@PostMapping("/sync")
public ResponseEntity<StatusResponse> syncTourData(@RequestParam(value = "page") int page, @RequestParam(value = "size") int size, @RequestParam(value = "date") String date) {
spotService.synchronizeTourData(tourApiService.getSyncList(page, size, date));
return ResponseEntity.ok(StatusResponse.builder()
.status(StatusEnum.OK.getStatusCode())
.message(StatusEnum.OK.getCode())
Expand Down Expand Up @@ -54,7 +52,7 @@ public ResponseEntity<StatusResponse> getSpotsList(@RequestParam(value = "page")
@GetMapping("/{contentId}")
public ResponseEntity<StatusResponse> getSpotDetail(@PathVariable Long contentId) {
Spot spot = spotService.findSpotByContentId(contentId);
Object commonDetail = tourApiService.getCommonDetail(spot);
Object commonDetail = tourApiService.getCommonDetail(spot.getCategory().getCode(), spot.getContentId());
Object detailInfo = tourApiService.getDetailInfo(spot);
JSONArray imageArr = tourApiService.getDetailImages(contentId);
return spotService.responseDetailInfo(commonDetail, detailInfo, imageArr, spot);
Copy link

Choose a reason for hiding this comment

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

이 코드 패치를 간단히 검토해 드리겠습니다.

  1. Lombok에 @slf4j 어노테이션을 추가하여 로깅 기능을 사용하고 있습니다.
  2. '/api/v1/spots' 경로에 대한 POST 요청을 처리하는 /sync 메서드가 추가되었습니다. 이 메서드는 페이지, 크기 및 날짜에 따른 관광 데이터 동기화를 수행합니다.
  3. '/api/v1/spots/{contentId}' 경로에 대한 GET 요청을 처리하는 getSpotDetail 메서드가 수정되었습니다. 이제 tourApiService.getCommonDetail 메서드를 호출할 때 카테고리 코드와 함께 호출합니다.

개선 제안:

  • 이 코드 패치에서 보여지는 만큼 추가적인 버그 또는 문제는 보이지 않습니다. 하지만 실제 구현 상황과 요구 사항에 따라 개선할 수 있는 사항이 있을 수 있습니다.
  • 코드를 개선하거나 추가하기 전에 주석을 제공하면 좋을 것 같습니다. 코드의 인텐션과 목적을 이해하는 데 도움이 됩니다.
  • 단위 테스트를 작성하여 코드의 동작을 검증하는 것이 좋습니다.

참고: 이 답변은 지식 절단(latest 2021)을 기준으로 작성되었습니다. 코드 패치 내의 사용되는 외부 라이브러리나 프레임워크에 대한 자세한 내용은 최신 문서 또는 해당 라이브러리/프레임워크 공식 문서를 참조해야 합니다.

Copy link

Choose a reason for hiding this comment

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

해당 코드 패치에 대한 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안 사항이 있다면 언제든지 환영합니다:

  • lombok.extern.slf4j.Slf4j를 추가하여 로깅을 적용할 수 있습니다.
  • /api/v1/spots/test 대신 /api/v1/spots/sync로 엔드포인트 이름을 변경할 수 있습니다.
  • saveTestData() 메서드를 syncTourData()로 변경하고, 메서드 파라미터로 size와 date를 추가하였습니다.
  • tourApiService.getApiDataList(page, category)의 대신 tourApiService.getSyncList(page, size, date)를 호출하여 데이터를 동기화할 수 있습니다.
  • /api/v1/spots/{contentId}로 GET 요청이 올 경우 category 정보를 함께 전달하도록 tourApiService.getCommonDetail(spot.getCategory().getCode(), spot.getContentId())로 수정되었습니다.

주의할 점:
해당 코드에서 누락된 부분이 발견되지 않았으며, 기타 잠재적인 버그도 없어 보입니다. 그러나 완벽한 코드 리뷰를 위해서는 클래스 선언부가 보이지 않기 때문에 해당 부분은 확인할 수 없습니다.

Expand Down
127 changes: 104 additions & 23 deletions api-server/src/main/java/com/kuddy/apiserver/spot/service/SpotService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,21 @@
import com.kuddy.common.spot.exception.SpotNotFoundException;
import com.kuddy.common.spot.repository.SpotRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.EnumUtils;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
import org.springframework.data.domain.*;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.http.ResponseEntity;
import org.springframework.scheduling.annotation.EnableScheduling;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.time.LocalDate;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
Expand All @@ -36,40 +43,113 @@
@Service
@Transactional
@RequiredArgsConstructor
@EnableScheduling
@Slf4j
public class SpotService {

private final SpotRepository spotRepository;
private final HeartRepository heartRepository;
private final SpotQueryService spotQueryService;
private final TourApiService tourApiService;

//JSON 응답값 중 필요한 정보(이름, 지역, 카테고리, 사진, 고유id)만 db에 저장
public void changeAndSave(JSONArray spotArr) {
@Scheduled(cron = "0 0 6 * * *")
public void syncTourDataEveryDay() {
LocalDate seoulNow = LocalDate.now(ZoneId.of("Asia/Seoul"));
LocalDate oneDayAgo = seoulNow.minusDays(1);
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyyMMdd");
String modifiedTime = oneDayAgo.format(formatter);
log.info(modifiedTime);
synchronizeTourData(tourApiService.getSyncList(1, 50, modifiedTime));
}

public void synchronizeTourData(JSONArray spotArr) {
for (Object o : spotArr) {
JSONObject item = (JSONObject) o;
String contentType = "";
String areaCode = "";
Long contentId = Long.valueOf((String) item.get("contentid"));
log.info("sync contentId = " + contentId);

if (!spotRepository.existsByContentId(Long.valueOf((String) item.get("contentid")))) {
for (Category category : Category.values()) {
if (item.get("contenttypeid").equals(category.getCode()))
contentType = category.name();
if(Category.hasValue((String) item.get("contenttypeid"))) {
//이미 저장되어 있을 경우 내용 업데이트
if (spotRepository.existsByContentId(contentId)) {
updateSpot(item, contentId);
}
for (District district : District.values()) {
if (item.get("sigungucode").equals(district.getCode()))
areaCode = district.name();
//표출되고 있는 데이터이고 db에 저장되어 있지 않은 경우 새로 저장
if ((item.get("showflag").equals("1")) && (!spotRepository.existsByContentId(contentId))) {
saveSpot(item, contentId);
addImage(contentId);
}
}
}
}

public void updateSpot(JSONObject item, Long contentId) {
Spot spot = findSpotByContentId(contentId);
//더이상 표출되지 않는 데이터이면 내용 비우기
if(item.get("showflag").equals("0")) {
spot.update((String) item.get("title"),
District.valueOfCode((String) item.get("sigungucode")),
"",
Category.valueOfCode((String) item.get("contenttypeid")),
"",
"",
"No more information found for this spot.",
(String) item.get("modifiedtime"));
log.info("update showflag = 0, contentId = " + contentId);
}
//표출되고 있고 저장되어 있는 데이터와 modifiedTime이 다르면 정보 업데이트
if((item.get("showflag").equals("1")) && (!spot.getModifiedTime().equals(item.get("modifiedtime")))) {
spot.update((String) item.get("title"),
District.valueOfCode((String) item.get("sigungucode")),
(String) item.get("firstimage"),
Category.valueOfCode((String) item.get("contenttypeid")),
(String) item.get("mapx"),
(String) item.get("mapy"),
getSpotAbout(item, contentId),
(String) item.get("modifiedtime"));
log.info("update showflag = 1, contentId = " + contentId);
addImage(contentId);
}

}

spotRepository.save(Spot.builder()
.name((String) item.get("title"))
.contentId(Long.valueOf((String) item.get("contentid")))
.district(District.valueOf(areaCode))
.category(Category.valueOf(contentType))
.imageUrl((String) item.get("firstimage"))
.numOfHearts(0L)
.mapX((String) item.get("mapx"))
.mapY((String) item.get("mapy"))
.build());
public void saveSpot(JSONObject item, Long contentId) {
log.info("save contentId = " + contentId);
spotRepository.save(Spot.builder()
.name((String) item.get("title"))
.contentId(contentId)
.district(District.valueOfCode((String) item.get("sigungucode")))
.category(Category.valueOfCode((String) item.get("contenttypeid")))
.imageUrl((String) item.get("firstimage"))
.numOfHearts(0L)
.mapX((String) item.get("mapx"))
.mapY((String) item.get("mapy"))
.about(getSpotAbout(item, contentId))
.modifiedTime((String) item.get("modifiedtime"))
.build());
}

//TourAPI에서 spot about만 가져오기
public String getSpotAbout(JSONObject item, Long contentId) {
String about;
Object commonDetail = tourApiService.getCommonDetail((String) item.get("contenttypeid"), contentId);
JSONObject detail = (JSONObject) commonDetail;
about = (String) detail.get("overview");
return about;

}

//썸네일 없을때 상세 이미지들 중 첫번째 사진을 썸네일로 지정
public void addImage(Long contentId) {
Spot spot = findSpotByContentId(contentId);
if(spot.getImageUrl().isEmpty()){
JSONArray imageArr = tourApiService.getDetailImages(contentId);
if(imageArr == null)
spot.setImageUrl("");
if(!(imageArr == null)) {
JSONObject detailImage = (JSONObject) imageArr.get(0);
spot.setImageUrl((String) detailImage.get("originimgurl"));
}
log.info("addImage URL = " + spot.getImageUrl());
}
}

Expand Down Expand Up @@ -164,15 +244,16 @@ public ResponseEntity<StatusResponse> responseDetailInfo(Object commonDetail, Ob

//이미지
List<String> imageList = new ArrayList<>();
if(!spot.getImageUrl().isEmpty())
imageList.add(spot.getImageUrl());
if(!(imageArr == null)) {
for (Object object : imageArr) {
JSONObject item = (JSONObject) object;
String imageUrl = (String) item.get("originimgurl");
imageList.add(imageUrl);
}
}
if(imageList.isEmpty() && !spot.getImageUrl().isEmpty()) {
imageList.add(spot.getImageUrl());
}

//상세 정보
JSONObject additionalInfo = (JSONObject) detailInfo;
Copy link

Choose a reason for hiding this comment

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

이 코드는 SpotService 클래스의 일부분입니다. 아래는 몇 가지 개선 제안과 버그 확인 사항입니다:

  1. 스케줄링: @EnableScheduling과 @Scheduled 어노테이션을 사용하여 매일 특정 시간에 syncTourDataEveryDay() 메서드가 실행되도록 예약하였습니다. 하지만 해당 메서드의 동작이 코드에 포함되어 있지 않아 작동 방식을 확인할 수 없습니다.

  2. 로깅: log 변수를 사용하여 로깅을 위한 Slf4j 로거를 선언하였습니다. 로그 문장들은 log.info()로 시작하며, 메서드의 수행 상태나 로그인한 데이터 등을 기록합니다.

  3. 여러 개의 라이브러리 추가: lombok.extern.slf4j.Slf4j, org.apache.commons.lang3.EnumUtils 및 java.time 패키지에서 일부 유틸리티 클래스 및 API를 임포트하였습니다.

  4. changeAndSave() 메서드 리네이밍: changeAndSave() 메서드가 synchronizeTourData() 메서드로 변경되었습니다. 이 메서드는 JSON 응답값에서 필요한 정보를 추출하여 DB에 저장하는 역할을 합니다.

  5. synchronizeTourData() 메서드 수정: synchronizeTourData() 메서드에서 한 번의 루프를 사용하여 JSON 배열을 처리하고, 포함된 각 항목에 대한 조건부 로직을 통해 저장 또는 업데이트를 수행합니다. 이 메서드의 구조와 동작은 적합해 보입니다.

  6. updateSpot() 메서드 수정: updateSpot() 메서드는 기존 Spot을 업데이트하는 데 사용됩니다. Spot 객체에서 정보를 가져와서 변경되지 않은 경우 데이터베이스에 대한 실제 업데이트 작업을 수행하지 않을 수 있습니다.

  7. saveSpot() 메서드 추가: saveSpot() 메서드는 새로운 Spot을 저장하기 위해 사용됩니다. synchronizeTourData() 메서드 내에서 호출됩니다.

  8. getSpotAbout() 메서드 추가: XML API response에서 spot에 대한 정보를 추출하기 위한 메서드입니다.

  9. addImage() 메서드 추가: Spot에 이미지가 없을 경우 detail 이미지 중 첫 번째 사진을 썸네일로 추가하는 메서드입니다.

  10. responseDetailInfo() 메서드 수정: 이미지 리스트를 확인하고 비어 있으면 spot.getImageUrl()을 추가하는 부분이 추가되었습니다.

위의 내용을 고려하여 코드 리뷰를 진행할 수 있습니다.

Copy link

Choose a reason for hiding this comment

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

코드 리뷰 요청을 받았습니다. 다음은 해당 코드 패치의 리뷰입니다:

  1. 스프링 스케줄링 설정: @EnableScheduling 어노테이션을 사용하여 스프링 스케줄링을 활성화 했습니다. @Scheduled 어노테이션을 사용하여 매일 아침 6시에 syncTourDataEveryDay() 메서드가 실행됩니다.

  2. 불필요한 import 문: org.apache.commons.lang3.EnumUtils와 org.json.simple.JSONArray import 문이 추가되어 있지만 해당 클래스들이 사용되지 않고 있습니다. 이러한 불필요한 import 문은 제거할 수 있습니다.

  3. 로깅 추가: lombok.extern.slf4j.Slf4j 어노테이션을 사용하여 log 변수를 추가하고, log.info() 메서드를 호출하여 로그를 남기고 있습니다. 로깅은 코드 디버깅 및 유지보수에 도움이 되므로 유용합니다.

  4. 올바른 데이터 타입 사용: contentId는 Long 타입으로 변환해야 합니다. Long.valueOf((String) item.get("contentid"))를 사용하여 contentId를 Long 타입으로 변환하세요.

  5. 열거형 비교 방법: contenttypeid와 sigungucode와 같은 값을 비교하는 부분에서 for 반복문 대신 EnumUtils를 활용하여 비교하고 있습니다. 이런 경우에는 EnumUtils 대신 열거형 클래스의 정적 메서드를 사용하는 것이 더 간결하고 명확합니다.

  6. 업데이트 및 저장 로직 분리: synchronizeTourData() 메서드를 새로 추가하고, saveSpot() 및 updateSpot() 메서드를 해당 메서드로 분리할 수 있습니다. 이렇게 하면 코드를 더 읽기 쉽고 유지 관리하기 쉽게 만들 수 있습니다.

  7. changeAndSave() 메서드명 변경: 현재 changeAndSave() 메서드는 JSON 응답값에서 필요한 정보를 db에 저장하는 작업을 수행합니다. 그러나 메서드 명에서는 그 의도가 명확하지 않습니다. 역할과 의도를 잘 표현하는 이름으로 변경하는 것이 좋습니다.

  8. 이미지 처리 로직 개선: addImage() 메서드에서 이미지 URL을 추가할 때, 이미 URL이 있는 경우에도 다시 추가하고 있습니다. 이미 URL이 없는 경우에만 추가해야 합니다. 이 부분을 개선하여 중복 추가되는 오류를 수정하세요.

  9. 리턴 문 제한: 코드 상세 설명을 위해 너무 많은 주석이 사용되고 있습니다. 보통 간결한 코드와 명확한 네이밍은 자체 설명력을 가지고 있으므로, 필요한 경우에만 주석을 달도록 하는 것이 좋습니다.

이상이 코드 리뷰입니다. 해당 패치에서 발견된 각 위험한 버그 및 개선 사항에 대해 설명하였습니다.

Expand Down
53 changes: 16 additions & 37 deletions api-server/src/main/java/com/kuddy/apiserver/spot/service/TourApiService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,18 @@ public class TourApiService {

@Value("${tourapi.secret-key}")
private String SECRET_KEY;
private final String DEFAULT_QUERY_PARAM = "&MobileOS=ETC&MobileApp=Kuddy&_type=json";

private static final String BASE_URL = "https://apis.data.go.kr/B551011/EngService1/";

//카테고리별로 20개씩 조회
public JSONArray getApiDataList(int page, int category) {

public JSONArray getSyncList(int page, int size, String modifiedTime) {
try {
URL url = new URL(BASE_URL + "areaBasedList1?numOfRows=20&pageNo=" +
page +
"&MobileOS=ETC&MobileApp=Kuddy&_type=json&listYN=Y&arrange=A&contentTypeId=" +
category +
"&areaCode=1&serviceKey="
URL url = new URL(BASE_URL + "areaBasedSyncList1?numOfRows=" +
size + "&pageNo=" + page +
DEFAULT_QUERY_PARAM + "&listYN=Y&arrange=C&areaCode=1&modifiedtime=" +
modifiedTime +
"&serviceKey="
+ SECRET_KEY);

JSONObject items = (JSONObject) extractBody(url).get("items");
JSONArray spotArr = (JSONArray) items.get("item");

Expand All @@ -46,34 +44,15 @@ public JSONArray getApiDataList(int page, int category) {
}
}

//현재 위치 기반으로 2km 반경 관광지 20개씩 조회
public JSONObject getLocationBasedApi(int page, int size, String mapX, String mapY) {

try {
URL url = new URL(BASE_URL + "locationBasedList1?numOfRows=" +
size + "&pageNo=" + page +
"&MobileOS=ETC&MobileApp=Kuddy&_type=json&listYN=Y&mapX=" +
mapX + "&mapY=" + mapY +
"&radius=2000&serviceKey="
+ SECRET_KEY);

return extractBody(url);

} catch(Exception e) {
e.printStackTrace();
throw new TourApiExeption();
}
}

//각 관광지 공통 정보 조회
public Object getCommonDetail(Spot spot) {
public Object getCommonDetail(String category, Long contentId) {

try {
URL url = new URL(BASE_URL + "detailCommon1?contentTypeId=" +
spot.getCategory().getCode() +
category +
"&contentId=" +
spot.getContentId() +
"&MobileOS=ETC&MobileApp=Kuddy&defaultYN=Y&addrinfoYN=Y&overviewYN=Y&_type=json&ServiceKey="
contentId +
DEFAULT_QUERY_PARAM + "&defaultYN=Y&addrinfoYN=Y&overviewYN=Y&ServiceKey="
+ SECRET_KEY);

JSONObject items = (JSONObject) extractBody(url).get("items");
Expand All @@ -92,11 +71,11 @@ public Object getCommonDetail(Spot spot) {
public Object getDetailInfo(Spot spot) {

try {
URL url = new URL(BASE_URL + "detailIntro1?MobileOS=ETC&MobileApp=Kuddy&contentId=" +
spot.getContentId() +
URL url = new URL(BASE_URL + "detailIntro1?contentId=" +
spot.getContentId() + DEFAULT_QUERY_PARAM +
"&contentTypeId=" +
spot.getCategory().getCode() +
"&_type=json&serviceKey="
"&serviceKey="
+ SECRET_KEY);

JSONObject items = (JSONObject) extractBody(url).get("items");
Expand All @@ -114,8 +93,8 @@ public Object getDetailInfo(Spot spot) {
//이미지 정보 조회 API
public JSONArray getDetailImages(Long contentId) {
try {
URL url = new URL(BASE_URL + "detailImage1?MobileOS=ETC&MobileApp=Kuddy&_type=json&contentId=" +
contentId +
URL url = new URL(BASE_URL + "detailImage1?contentId=" +
contentId + DEFAULT_QUERY_PARAM +
"&serviceKey="
+ SECRET_KEY);

Copy link

Choose a reason for hiding this comment

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

아래는 코드 패치입니다. 코드 리뷰를 간략하게 도와드리겠습니다. 버그의 위험과/또는 개선 제안을 환영합니다.

  1. DEFAULT_QUERY_PARAM 변수를 추가하여 기본 쿼리 매개변수를 사용합니다.
  2. getApiDataList 메서드가 getSyncList로 변경되었습니다.
  3. areaBasedList1 API 호출을 areaBasedSyncList1로 변경하여 수정된 시간을 기준으로 동기화된 목록을 받아옵니다.
  4. getLocationBasedApi 메서드가 삭제되었습니다.
  5. getCommonDetail 메서드의 매개변수로 Spot 객체 대신 category와 contentId를 사용합니다.
  6. getDetailInfo 메서드의 URL 생성 부분에서 기본 매개변수 값이 추가되었습니다.
  7. getDetailImages 메서드의 URL 생성 부분에서 기본 매개변수 값이 추가되었습니다.

주의할 사항:

  • 수정된 기능에 대한 테스트가 필요합니다.
  • URL 조립 시 쿼리 매개변수 값을 인코딩하는 것이 좋습니다.

특정 버그를 식별하지 못하였으며, 리뷰를 통해 기능 및 성능 개선을 고려할 수 있을 것입니다.

Expand Down
17 changes: 17 additions & 0 deletions common/src/main/java/com/kuddy/common/spot/domain/Category.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
import lombok.Getter;
import lombok.RequiredArgsConstructor;

import java.util.Arrays;
import java.util.Collections;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

@Getter
@RequiredArgsConstructor
public enum Category {
Expand All @@ -15,4 +21,15 @@ public enum Category {

private final String code;
private final String type;

private static final Map<String, String> CATEGORY_CODE_MAP = Collections.unmodifiableMap(
Stream.of(values()).collect(Collectors.toMap(Category::getCode, Category::name)));

public static Category valueOfCode(final String code) {
return Category.valueOf(CATEGORY_CODE_MAP.get(code));
}

public static boolean hasValue(String code) {
return Arrays.stream(Category.values()).anyMatch(v -> v.code.equals(code));
}
}
Copy link

Choose a reason for hiding this comment

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

주어진 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안을 환영합니다.

  1. 패치 전체적으로 보면, lombok 어노테이션을 사용하여 Getter와 RequiredArgsConstructor가 추가되었습니다. 이들은 자동으로 필요한 메서드를 생성해주는데, 현재 코드에서 필요한지 여부를 파악하기 어렵기 때문에 그대로 두겠습니다.

  2. Category 열거형 클래스에 새로운 기능이 추가되었습니다. HashMap을 사용하는 것보다 CATEGORY_CODE_MAP이라는 불변의 맵을 사용하도록 변경되었습니다. 이를 통해 카테고리 코드와 이름을 매핑하는 방법이 개선되었으며, 코드 검색과 변환 작업의 성능을 향상시켰습니다.

  3. valueOfCode 메서드는 주어진 코드에 해당하는 Category 값을 반환합니다. 이전엔 values() 메서드로 모든 열거형 객체를 반복하면서 일치하는 값을 찾았지만, 이제는 CATEGORY_CODE_MAP을 사용하여 빠르게 값을 가져올 수 있습니다.

  4. hasValue 메서드는 주어진 코드에 해당하는 Category 값이 있는지 여부를 확인합니다. 이를 위해 스트림을 사용하여 모든 Category 값을 반복하면서 일치하는지 검사합니다.

  5. 파일의 끝에 개행 문자(Newline)가 없습니다. 코드 스타일 관련 이슈이므로, 코드 끝에 개행 문자를 추가해주면 좋을 것입니다.

위의 내용은 주어진 코드 패치에 대한 간단한 개선 사항입니다. 다른 문제가 있는지 자세히 파악하려면 전체 소스 코드 및 프로그램 요구 사항을 확인해야 합니다.

7 changes: 7 additions & 0 deletions common/src/main/java/com/kuddy/common/spot/domain/District.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,11 @@ public enum District {
public static District of(final String area) {
return District.valueOf(DISTRICT_MAP.get(area));
}

private static final Map<String, String> DISTRICT_CODE_MAP = Collections.unmodifiableMap(
Stream.of(values()).collect(Collectors.toMap(District::getCode, District::name)));

public static District valueOfCode(final String code) {
return District.valueOf(DISTRICT_CODE_MAP.get(code));
}
}
Copy link

Choose a reason for hiding this comment

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

해당 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다.
이 코드의 목적은 지역 코드에 해당하는 District 열거형 값을 가져오는 것으로 보입니다. 코드에는 큰 문제점은 없어 보이나, 몇 가지 개선점과 버그 가능성을 제시해드리겠습니다.

  1. 버그 가능성:

    • DISTRICT_MAP에서 area와 관련된 값이 없는 경우, NullPointerException이 발생할 수 있습니다. 이 경우 예외 처리를 추가하는 것이 좋습니다.

  2. 개선 제안:

    • DISTRICT_CODE_MAP을 생성하는 방법은 올바르게 보입니다.
    • DISTRICT_CODE_MAP 필드는 불변이므로, Collections.unmodifiableMap()에 의한 래핑이 적절합니다.
    • 그러나 DISTRICT_CODE_MAP은 valueOfCode() 메서드 하나에서만 사용되므로, 클래스 필드로 정의하는 대신 메서드 내부에서 지역 변수로 선언하여 사용하는 것이 좋습니다. 이렇게 하면 필드의 범위를 제한할 수 있습니다.

다른 사소한 문제나 개선 포인트가 있다면 알려주세요.

Loading