[#15] 지역 인증 기능, 주변 지역 찾기 기능 구현 #19

tilsong · 2023-04-05T21:44:36Z

PR 유형

변경사항

변경 전

지역 인증 기능, 주변 지역 찾기 기능 x

변경 후

좌표 데이터를 추가했습니다.
- 블로그에서 지역 좌표 데이터를 받아 가공했습니다. 데이터는 줄 단위로 나누어져 있으며, 각 줄은 FullAddress, UnitAddress, x, y 좌표(UTM-K) 로 이루어져 있습니다.
- 데이터 선정의 기준은 각 지역 좌표가 각 지역의 행정 센터를 기준으로 하고 있는지 여부였습니다. 카카오 지도, 네이버 지도의 경우 일반적으로 지역 중심 좌표를 행정 센터 기준으로 하기 때문입니다.
- 각 지역 별 FullAddress에 포함된 Depth는 아래와 같습니다. FullAddress를 Depth로 나누어 사용하지는 않았습니다. 필요하다면 추후 나누어 사용해도 좋을 것 같습니다. UnitAddress는 가장 마지막의 Depth를 의미합니다.
  - 2~3 Depth : 세종
    ex. 세종특별자치시 소담동, 세종특별자치시 조치원읍 신흥리
  - 3 Depth : 서울
    ex. 서울특별시 송파구 방이동
  - 3~4 Depth : 강원, 경기, 경남, 경북, 광주, 대구, 대전, 부산, 서울, 울산, 인천, 전남, 전북, 제주, 충남, 충북
    ex. 인천광역시 옹진군 덕적면, 경상남도 거창군 신원면 구사리
지역 인증 및 주변 지역 찾기 기능을 구현했습니다.
- 통계청(SGIS)에서 제공하는 open API를 사용했습니다.
Region과 UserRegion을 분리했습니다.
- Region은 위의 좌표 데이터를 저장하고 이를 사용하는 역할을 담당합니다.
- UserRegion은 User와 Region을 매핑해주는 역할을 합니다.

기타

의존성 추가
- org.locationtech.jts:jts-core:1.19.0 지리 데이터 사용 목적입니다.
- org.springframework.retry:spring-retry 외부 api 요청 시 재시도 목적입니다.

- 통계청 api를 사용한 테스트 코드를 작성했습니다. 테스트한 api는 아래와 같습니다. - 인증 토큰 획득 api - 위치 값을 이용한 행정 구역 획득 api - test-application.properties 파일에 api 사용 키가 있어서 해당 파일은 업로드하지 않았습니다.

- 동 단위 지역명 테스트, 동이 아닌 지역명 테스트 - 패키지명 수정 (field -> specialty)

- 블로그(https://skyseven73.tistory.com/23 )에서 지역 좌표 데이터를 받아 가공했습니다. - 데이터는 줄 단위로 나누어져 있으며, 각 줄은 FullAddress, UnitAddress, x, y 좌표(EPSG:4326) 로 이루어져 있습니다.

…리 관련 의존성 추가 - Region 도메인 변경 - fullAddress, unitAddress를 Address 값 객체로 변경 - 지역 정보(주소, 좌표)에 대한 책임을 가짐. 유저와는 별개의 책임을 가지도록 하기. - 애플리케이션 시작 시 csv 파일 데이터를 기반으로 Region 도메인의 영속성 데이터들을 저장하도록 로직 작성 - 지리 관련 의존성 추가 - org.locationtech.jts:jts-core:1.19.0

- User와 Region을 매핑해주는 UserRegion 추가 - 기존 로직 및 패키지, 클래스 변경

- meter 단위로 좌표 간의 거리를 측정하기 위해, 좌표 파일의 좌표 단위를 변환했습니다. - 경위도 기준 좌표(WGS84, EPSG:4326) --> 한국 기준 좌표 UTM-K (GRS80, EPSG:5179) - 이 과정에서 testImplementation에 'org.locationtech.proj4j:proj4j:1.2.3' 의존성을 추가했습니다. 사용할 파일 내용을 애플리케이션 사용 전에 전처리하는 로직이기 때문에, 테스트 코드에서 처리하도록 했습니다.

… Long) 및 코드 수정

…itory

…retry 의존성 추가 - 외부 api 요청 시 재시도 목적으로 org.springframework.retry:spring-retry 의존성이 추가되었습니다.

f-lab-fi · 2023-04-06T17:13:48Z

src/main/java/com/directors/application/region/RegionService.java

+    @PostConstruct
+    private void loadRegionData() {


@PostConstruct가 붙은 메서드는 생성자와 비슷한 취급이 되어야 하지 않을까요?
만약 이 메서드가 생성자였다면, public 메서드와 private 메서드 사이에 위치 시키는게 자연스러울지 궁금합니다. :)

@PostConstruct이 붙은 메서드는 빈 객체가 생성되고, 필드가 초기화된 후, 빈 객체를 초기화하는 기능을 가지므로 생성자와 비슷하게 취급되어야 합니다.
따라서 필드 아래, 메서드 위에 위치시키는 것이 적절할 것 같습니다.

f-lab-fi · 2023-04-06T17:18:08Z

src/main/java/com/directors/application/region/RegionService.java

+        String pathPrefix = "/regionCSV/";
+        String pathSuffix = "_좌표.csv";
+        String[] regions = {
+                "강원", "경기", "경남", "경북", "광주", "대구", "대전", "부산", "서울", "세종", "울산", "인천", "전남", "전북", "제주", "충남", "충북"
+        };
+
+        for (int i = 0; i < regions.length; i++) {
+            List<String> regionDataLines = null;
+
+            InputStream inputStream = getClass().getResourceAsStream(pathPrefix + regions[i] + pathSuffix);
+            BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream));
+            regionDataLines = reader.lines().collect(Collectors.toList());
+
+            List<Region> collect = regionDataLines.stream()
+                    .map(this::regionDataLineToRegion)
+                    .collect(Collectors.toList());
+
+            regionRepository.saveAll(collect);
+        }


지금은 좌표들을 파일에서 읽어서 애플리케이션이 시작된 후(정확히는 RegionService가 생성된 후) DB에 넣어주고 있는데요(지금은 진짜 DB는 아니지만), 이 구조가 실제 데이터베이스를 쓰는 환경에서 서버가 여러개 떠서 로드밸런싱 되는 환경이라면 적절할까요?
어떻게 개선해보려는 방향성을 가지고 있는지 궁금합니다.

loadRegionData를 실행할 때 먼저 db에 data가 있는지 조회하고, 없으면 DB에 데이터를 넣고 있으면 넣지 않도록 로직을 작성할 수 있을 것 같습니다. -> 다시 생각해보니까 애플리케이션이 거의 같은 시점에 시작된다면, 동시성 문제가 발생할 수 있을 것 같습니다.

@PostConstruct를 사용하지 않고, 미리 데이터가 준비된 sql 스크립트 파일을 통해 DB에 따로 데이터를 넣어줄 수 있습니다.

@tilsong

넵 보통 자주 바뀌는 데이터는 아니라 문제 없겠지만, 이런것도 고민해보셔야합니다.

이 csv 파일은 어딘가에서 받아오신거겠죠?
그럼 그 '어딘가'에서 주기적으로, 데이터를 다시 받아와서 갱신할 수 있도록 만들어 보시는 것도 좋습니다.
실무에서도 이런식으로 외부에서 DB를 받아와서 갱신하는 경우가 제법 있어요.

말씀하신 내용 보고 구글 drive api를 이용해서 구글 drive에 저장된 csv 파일을 가져오고, 위의 로직을 통해 저장하는 방법을 생각해봤습니다.

한편 주기적으로 데이터를 가져와서 Region 테이블을 업데이트해주기 위해서는 스케줄러나 배치를 통한 작업이 필요하겠다는 생각이 들었습니다. 그리고 그렇다면 처음 말씀하신 대로 기존 프로젝트 서버가 여러 대일 경우 이런 작업을 위해서는 별도의 모듈이 필요하다는 생각을 해봤습니다..

…드 리팩토링

f-lab-fi · 2023-04-07T15:35:20Z

src/main/java/com/directors/domain/region/RegionRepository.java

+    Optional<Region> findByFullAddress(String fullAddress);
+
+    Region findByRegionId(Long regionId);


findByFullAddress는 Optional, findByRegionId는 그냥 객체를 리턴 타입으로 지정한 의도는 어떤걸까요~?

findByRegionId도 Optional 처리를 해주어야 할 것 같습니다!

f-lab-fi · 2023-04-07T15:38:04Z

src/main/java/com/directors/infrastructure/api/ApiSender.java

+    @Retryable(retryFor = {HttpClientErrorException.class, HttpServerErrorException.class}, maxAttempts = 3, backoff = @Backoff(delay = 1000))
+    public ResponseEntity<Map<String, Object>> send(HttpMethod method, UriComponents uri) throws HttpClientErrorException, HttpServerErrorException {
+        RestTemplate restTemplate = new RestTemplate();
+        HttpHeaders header = new HttpHeaders();
+        HttpEntity<String> entity = new HttpEntity<>(header);
+
+        return restTemplate.exchange(uri.toString(), method, entity, new ParameterizedTypeReference<>() {
+        });
+    }


멘토링 시간에 질문 하셨던게 이 부분이군요.
확실히 성공이라고 이야기 할 수 있는 것과 아닌거로 구분해보시면 되겠습니다~

f-lab-fi · 2023-04-07T15:44:13Z

src/main/java/com/directors/infrastructure/api/RegionApiClientManager.java

+@Component
+@RequiredArgsConstructor
+public class RegionApiClientManager implements RegionApiClient {


DIP 대로 잘 추상화시키셨네요.
좋습니다.

f-lab-fi · 2023-04-07T16:22:27Z

src/main/java/com/directors/application/user/AuthenticateRegionService.java

+        UserRegion savedUserRegion;
+
+        if (userRegionRepository.existsByUserId(userId)) {
+            UserRegion userRegion = userRegionRepository.findByUserId(userId).get();
+            userRegion.setAddress(regionByApi.getAddress());
+
+            savedUserRegion = userRegionRepository.save(userRegion);
+        } else {
+            UserRegion newUserRegion = UserRegion.of(regionByApi.getAddress(), userId, regionByApi.getId());
+
+            savedUserRegion = userRegionRepository.save(newUserRegion);
+        }


이런 if, else 문은 개선해볼 여지가 있습니다.
일단 현재 코드의 문제는

UserRegion savedUserRegion; 에서 savedUserRegion이 초기화되지 않은 상태로 남아있어야함.

아래 두 코드가 각각 무엇을 의도하는지 알기 어려움.

if 블록

UserRegion userRegion = userRegionRepository.findByUserId(userId).get(); userRegion.setAddress(regionByApi.getAddress()); savedUserRegion = userRegionRepository.save(userRegion);

else 블록

UserRegion newUserRegion = UserRegion.of(regionByApi.getAddress(), userId, regionByApi.getId()); savedUserRegion = userRegionRepository.save(newUserRegion);

입니다.
어떻게 개선해볼 수 있을까요?

UserRegion savedUserRegion 를 null로 초기화 해줄 수 있습니다.

if 블록과 else블록 각각의 내용을 private method로 추가하고, 의미 있는 메소드 명을 지어 행위의 의도를 드러낼 수 있습니다.
둘 다 해서 아래 코드 처럼 바꾸어 보았는데, 혹시 더 나은 방향이 있을까요? 어렵네요ㅎㅎ

` private Address updateUserRegionAddress(String userId, Region region) {
UserRegion savedUserRegion = null;

if (userRegionRepository.existsByUserId(userId)) { savedUserRegion = updateExistUserRegion(userId, region); } else { savedUserRegion = saveUserRegion(userId, region); } return savedUserRegion.getAddress(); } private UserRegion saveUserRegion(String userId, Region region) { UserRegion savedUserRegion; UserRegion newUserRegion = UserRegion.of(region.getAddress(), userId, region.getId()); savedUserRegion = userRegionRepository.save(newUserRegion); return savedUserRegion; } private UserRegion updateExistUserRegion(String userId, Region region) { UserRegion userRegion = userRegionRepository.findByUserId(userId).get(); userRegion.setAddress(region.getAddress()); userRegion.setRegionId(region.getId()); return userRegionRepository.save(userRegion); }

`

@tilsong

2번은 좋은 방향인데, 1번은 글쎄요 ㅋ...
null로 초기화하나 초기화 안하나 결국 마찬가지 아닐까요~?

그리고 결과적으로 Address가 반환되면 되는걸텐데... UserRegion에 집착할 필요가 있나요?

그렇네요ㅠ

private Address updateUserRegionAddress(String userId, Region region) { if (userRegionRepository.existsByUserId(userId)) { return updateExistUserRegion(userId, region).getAddress(); } return saveUserRegion(userId, region).getAddress(); }

요렇게 바꿔봤습니다. Address를 반환하는 것이 목적이므로, UserRegion을 두지 않고 바로 return을 했습니다.

f-lab-fi · 2023-04-07T16:23:33Z

src/test/java/com/directors/learningTest/geoApiTest/TextTranslateTest.java

+public class TextTranslateTest {
+
+    @Test
+    public void translateTextList() {
+        String defaultPath = "C:\\Users\\admin\\Downloads\\위경도_좌표\\";
+        String fileType = ".csv";
+        List<String> fileList = Arrays.asList(
+                "서울", "경기", "인천", "강원", "충북", "충남", "대전", "전북", "전남", "광주", "경북", "경남", "부산", "울산", "대구", "제주"
+        );
+
+        for (String file : fileList) {
+            translateText(defaultPath + file + fileType, defaultPath + file + "_좌표" + fileType, false);
+        }
+
+        translateText(defaultPath + "세종" + fileType, defaultPath + "세종_좌표" + fileType, true);
+    }


이 코드는 뭔가 학습 테스트에 해당하는 걸까요?

엄밀히 말하면 원본 좌표 데이터를 라이브러리를 사용해서 원하는 형태의 데이터(현재 "서울_좌표.csv" 형태의 파일들)로 변환한 로직이 담긴 파일입니다.
이 내용을 테스트에 올려도 되는지는 조금 애매한 것 같습니다ㅠ

@tilsong

제 생각에는 없어도 될 것 같은데요 ㅋ... 그리고 지금 이 테스트의 중요한 문제점은 '은석님 지금 컴퓨터에서만 실행된다'는 겁니다.
이 리뷰 부분과 더불어 고민해보시면 좋을 것 같네요. :)

넵 이 코드 파일은 제거하고, 위의 리뷰에 근거해서 다시 생각해보겠습니다!

@transactional

- @PostConstruc 메소드 위치 수정 - SpecialtyService 클래스의 메소드에 @transactional 추가

…에 반영되지 않는 test 코드 제거

puallab

확인했습니다.

tilsong added 16 commits March 29, 2023 21:57

Merge branch 'main' into feature/15

fa643dc

[f-lab-edu#15] 지역 인증 기능 구현 - 외부 api 테스트

4748030

- 동 단위 지역명 테스트, 동이 아닌 지역명 테스트 - 패키지명 수정 (field -> specialty)

[f-lab-edu#15] 지역 인증 기능 구현 - 도메인 및 레포지토리 생성

6df6b53

[f-lab-edu#15] 지역 인증 기능 구현 - 도메인 및 레포지토리 수정

7b36e27

[f-lab-edu#15] 지역 인증 기능 구현 - 외부 api를 이용을 위한 api 전송 클래스 및 토큰 획득 클래스 생성

3ec6edc

[f-lab-edu#15] 지역 인증 기능 구현 - 외부 api를 이용한 지역명(주소) 획득 인터페이스 및 구현체 생성

69c609d

[f-lab-edu#15] 지역 인증 기능 구현 - Controller, Service 및 요청, 응답 객체 생성

fea044c

[f-lab-edu#15] 지역 인증 기능 구현 - 외부 api 테스트 추가

0a055e5

[f-lab-edu#15] 지역 인증 기능 구현 - UserRegion 도메인 추가 및 그에 따른 로직 변경

3c14496

- User와 Region을 매핑해주는 UserRegion 추가 - 기존 로직 및 패키지, 클래스 변경

[f-lab-edu#15] 인증된 지역의 주변 지역 목록 찾기 기능 구현 - Region의 id 타입 변경(String ->…

a298196

… Long) 및 코드 수정

[f-lab-edu#15] 인증된 지역의 주변 지역 목록 찾기 기능 구현 - Controller, Service, Repos…

af0ce61

…itory

[f-lab-edu#15] 인증된 지역의 주변 지역 목록 찾기 기능 구현 - 외부 api로 인한 예외 추가 및 spring-…

88d77c4

…retry 의존성 추가 - 외부 api 요청 시 재시도 목적으로 org.springframework.retry:spring-retry 의존성이 추가되었습니다.

tilsong requested review from f-lab-fi and puallab April 5, 2023 21:44

tilsong self-assigned this Apr 5, 2023

tilsong linked an issue Apr 5, 2023 that may be closed by this pull request

지역 인증 기능, 주변 지역 찾기 기능 구현 #15

Closed

2 tasks

f-lab-fi reviewed Apr 6, 2023

View reviewed changes

[f-lab-edu#15] 인증된 지역의 주변 지역 목록 찾기 기능 구현 - 외부 api 및 spring-retry 관련 코…

5cba290

…드 리팩토링

f-lab-fi reviewed Apr 7, 2023

View reviewed changes

tilsong added 2 commits April 9, 2023 02:28

[f-lab-edu#15] 인증된 지역의 주변 지역 목록 찾기 기능 구현 - 피드백 반영 및 코드 수정

715d2a0

- @PostConstruc 메소드 위치 수정 - SpecialtyService 클래스의 메소드에 @transactional 추가

[f-lab-edu#15] 인증된 지역의 주변 지역 목록 찾기 기능 구현 - 피드백 반영 및 코드 수정2, 실제 애플리케이션…

e21ee7a

…에 반영되지 않는 test 코드 제거

puallab approved these changes Apr 9, 2023

View reviewed changes

tilsong merged commit 95c4b23 into f-lab-edu:main Apr 9, 2023

tilsong deleted the feature/15 branch April 9, 2023 15:27

		Optional<Region> findByFullAddress(String fullAddress);

		Region findByRegionId(Long regionId);

[#15] 지역 인증 기능, 주변 지역 찾기 기능 구현 #19

[#15] 지역 인증 기능, 주변 지역 찾기 기능 구현 #19

Uh oh!

Conversation

tilsong commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR 유형

변경사항

변경 전

변경 후

기타

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!

tilsong Apr 8, 2023 • 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!

tilsong Apr 8, 2023 • 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!

tilsong Apr 8, 2023 • 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!

tilsong Apr 8, 2023 • 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!

puallab left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tilsong commented Apr 5, 2023 •

edited

Loading

tilsong Apr 8, 2023 •

edited

Loading

tilsong Apr 8, 2023 •

edited

Loading

tilsong Apr 8, 2023 •

edited

Loading

tilsong Apr 8, 2023 •

edited

Loading