[Refactor] Downloader, Scraper 인터페이스 추가를 통한 구조 변경, 크롤링 로직 개선 - #107
[Refactor] Downloader, Scraper 인터페이스 추가를 통한 구조 변경, 크롤링 로직 개선#107kymokim wants to merge 2 commits into
Conversation
suhongkim98
left a comment
suhongkim98
left a comment
There was a problem hiding this comment.
고생하셨습니다. 1등으로 PR 올려주셨어요.
코드리뷰 피드백 1차적으로 반영해주세요, 반영 후 2차 피드백은 회의 때 해드릴게요.
| private static final Logger log = LoggerFactory.getLogger(CrawlingServiceImpl.class); | ||
| private final ApiEndpointSecretProperties apiEndpointSecretProperties; | ||
|
|
||
| public Iterator<Element> generalDownloader(int pageNum, UnivNoticeType type){ |
There was a problem hiding this comment.
Downloader는 HTML 문서를 다운로드하는 용도로만 쓰여야할 것 같은데 다운로드 후 태그 기반으로 크롤링을 수행하는 로직이 있네요.
Document doc = Jsoup.connect(url).execute().parse(); 까지만 수행하고 Document를 리턴하도록 하는게 어떨까요?
나머지 로직은 Scraper에 있는게 좋을 것 같아요.
There was a problem hiding this comment.
우선 CrawlingServiceImpl로 변경하였습니다 !
| import java.io.IOException; | ||
| import java.util.Iterator; | ||
|
|
||
| @Service |
There was a problem hiding this comment.
서비스 레이어는 아닌 것 같아요.
@Component로 수정해주세용
| private final KnuNoticeScraper knuNoticeScraper; | ||
|
|
||
| @Override | ||
| public Optional<List<ResponseCrawling.UnivNotice>> getUnivNotice(int pageNum, UnivNoticeType type) { |
There was a problem hiding this comment.
현재 Optional<List<..>> 형태인데요.
리팩토링 하는 김에 단일 dto를 반환하도록 변경해보겠어요?
리팩토링 후에도 API Response는 동일해야 합니다.
|
|
||
| public Iterator<Element> generalDownloader(int pageNum, UnivNoticeType type){ | ||
| //type에 따라 전체, 학사, 장학, 학습/상담, 취창업 공지 크롤링 | ||
| String url = apiEndpointSecretProperties.getCrawling().getUnivNotice() + "?paginationInfo.currentPageNo=" |
There was a problem hiding this comment.
이렇게 Url을 내부에 고정하면 크롤링 해야하는 Url 개수만큼 다운로더가 존재해야할 것 같아요.
GeneralDownloader를 범용적으로 사용할 수 있게끔 url은 외부에서 입력받게끔 수정해주세요.
그러기 위해서 GeneralDownloader를 스프링 빈으로 등록하지 않고 애플리케이션 레이어에서 에서 필요할 때마다 new 키워드를 이용해 다운로더를 생성하는 방법도 있습니다.
| private final ApiEndpointSecretProperties apiEndpointSecretProperties; | ||
| private final ObjectMapper objectMapper; | ||
| @Override | ||
| public List<ResponseCrawling.UnivNotice> scrapKnuNotice(Iterator<Element> rows) { |
There was a problem hiding this comment.
Scrap 후에는 리스트 형태가 아니라 단일 dto를 반환하게끔 리팩토링 해주세용
2 participants
구현내용
GeneralDownloader, KnuNoticeScraper로 공지사항 처리 로직 분리 후, 서비스에서 이들을 호출하도록 구조 변경.
예외와 상관없이 자원 할당 해제하도록 변경 (GeneralDownloader.java)
Document doc = Jsoup.connect(url).execute().parse();
생성과 사용 동시에 하도록 변경 (KnuNoticeScraperImpl.java)
Element linkElement = li.get(1).selectFirst("a.detailLink"); // 링크li
JsonNode jsonNode = objectMapper.readTree(linkElement.attr("data-params"));
String link = apiEndpointSecretProperties.getCrawling().getUnivNoticeItem()
+ "?scrtWrtiYn=false&encMenuSeq=" + jsonNode.get("encMenuSeq").asText()
+ "&encMenuBoardSeq=" + jsonNode.get("encMenuBoardSeq").asText();
학습 내용(optional)
자원을 안전하게 할당 해제하여 리소스 누수를 방지하는 방법을 학습했습니다.
테스트코드의 경우 테스트 실행은 잘 되었으나 테스트코드를 새로 작성하거나 수정하는 것은 감이 안 잡혀 작성하지 못 했습니다.