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

Feat : 학교 홈페이지 리뉴얼로인한 교직원 스크랩 및 스크랩간 직위 추가 #223

Merged
merged 48 commits into from
Dec 3, 2024

Conversation

rlagkswn00
Copy link
Collaborator

📝작업 내용

1. 스크랩할 원본페이지 선택

스크랩은 모든 학과 페이지의 교수진 기준으로 했습니다.
예시) 컴퓨터공학부 교수진 홈페이지

각 학과/교수에 따라 다를 수 있는 점은 다음과 같습니다.
1. 교수 직위에 따른 차이

  • 한 페이지 내 전임교수, 기타 교수(명예교수 등) 존재 vs 전임교수, 기타 교수 페이지 분리
  • 이 경우 전임교수 기준으로 했으며, 종종 기타 교수가 포함된 학과도 있습니다.

2. 수집한 정보 및 제공된 정보에 따른 차이

  • 이름 : 영어이름 포함 가능성
  • 연구분야 : 없는 경우 존재
  • 연구실 : 없는 경우 존재
  • 연락처 : 없는 경우가 있거나 형식이 다름(내선번호 4글자 or 전체 번호)
  • 이메일 : 없는 경우 존재

추가로 기존과 url구조가 변경되었으며 필요 정보가 바뀌었습니다.

  • 기존에는 몇몇 학과(부동산학과, 예디대 일부 학과)와 그 외 학과의 url이 달랐으나, 이번에 찾아봤을 때 전부 아래와 같은 형식임을 확인했습니다.
  • 이에 따라 각 학과별 교수진 홈페이지의 학과 구분 정보(siteName)과 페이지 정보(siteId)를 추가했으며, 학과의 구분없이 사용 가능한 baseurl이라 판단되어 구분해놨던 url을 통합했습니다.(staff.each-dept-url)

url 구조 : https://{department}.konkuk.ac.kr/{department}/{siteId}/subview.do

  • 서브도메인 대신 www. 사용 가능하나, 이 경우 Jsoup으로 접근 시도시 redirect 횟수 초과 발생(정확한 원인은 찾지 못했으나 관리자 모드 확인 해보니 합니다)
image

2.구현

  • 기존 사용하던 DeptInfo, ParserTemplate, ApiClient을 변경하여 사용했습니다.
  • DeptInfo의 학과별 정보를 siteName, siteId로 변경
  • ParsertTemplate은 부동산 학과와 그 외 학과를 분리했습니다.(html구조 차이 존재)
  • ApiClient는 하나로 통합하였습니다. 다만, 사회봉사교육센터의 경우 siteId가 Empty List로 이루어져 있어 에러가 발생하였고, 이를 해결하기 위해 학과 홈페이지에 교수진 정보를 제공하는 지 유무를 체크할 검증 메서드를 하나 추가했습니다.
  • 기존과 다르게 직위 정보를 추가했습니다. 추후 Staff Entity에도 반영예정입니다.
  • 수의학과, 수의예과의 경우 겹치는 교수진이 있어서 Map 자료구조 변환간distinct()처리 하였습니다.

3. Test

  • 파싱 테스트를 위해 부동산학과, 컴퓨터공학부 두개의 페이지로 시도했습니다.
  • 자체적으로 로컬에서 실행했을 때 모든 학과에대해서 에러 없이 스크랩 되는 것도 확인했습니다.

3. 이후 TODO 제안

1. �공지사항 스크랩과 교직원 스크랩 분리

List<Integer> siteIds = List.of(11135, 11136);
this.staffScrapInfo = new StaffScrapInfo(VET_MEDICINE.getHostPrefix(), siteIds);
this.noticeScrapInfo = new NoticeScrapInfo(VET_MEDICINE.getHostPrefix(), 948);

VeterinaryMedicineDept의 생성자 내용중 일부입니다. StaffScrapInfositeId정보를 List로 구현한 이유입니다. 수의과대학의 경우 수의학과, 수의예과의 교수진 정보를 다른 페이지에서 관리하고 있습니다. 이로인해 prefix는 동일하지만 siteId`는 두개입니다.

이에, 학과 구분에 따라 수의학과, 수의예과를 각각 구분하여 DepartmentName, DeptInfo를 생성하려 했으나, 공지사항 스크랩에서 같은 객체를 공유하고 있다는 것을 확인했습니다. 객체 추가로 인해 교직원 중복은 해결될 수 있지만, 반대로 공지사항이 중복 스크랩되는 등 부수효과가 발생할 수 있다고 예상됩니다.

결론적으로, 현재 학과 공지사항과 교직원 정보 제공 유무에 따라 각 정보의 스크랩 유무가 달라지며 따로 예외처리해야합니다. 이를 구분할 수 있는 방법이 필요하다고 생각합니다.

간단히 생각한 방법은 스크랩 시 NoticeScrapInfo, StaffScrapInfo을 바탕으로 지원 유무를 체크할 수 있는 로직을 추가하여, 스크랩간 스크랩 대상인지 체크할 수 있을거 같습니다.(isSupportStaffScrap()와 같은 역할을 하는)
혹은, 객체 자체를 분리할 수도 있을거 같으나 작업공수가...🥲

좋은 방법� 생각나시면 공유 부탁드립니닷!

2. 스크랩 정보(NoticeScrapInfo, StaffScrapInfo)를 DepartmentName을 활용하여 한 파일 내에서 관리
이번 작업에서 모든 학과별 DeptInfo 객체에서 siteIdsiteName을 변경하는게 상당히 귀찮으면서 실수할 여지가 컸다고 생각합니다.
스크랩 특성상 학교 홈페이지에 전적으로 의존하고 있는만큼 학교 홈페이지 리뉴얼에 의해 스크랩 방식이 변경될 가능성이 크다고 생각합니다. 이에, 스크랩을 위한 정보들을(hostPrefix, siteId, siteName)은 모두 한 Enum에서 관리하는게 낫지 않을까 생각합니다.

현재 DeptInfoDepartmentName을 필드로 갖고 있으므로 이를 변경해서 적용하면(DepartmentScrapInfo 이런 느낌의 네이밍..?), 스크랩 정보(NoticeScrapInfo, StaffScrapInfo)가 없어도 Enum을 통해 바로 스크랩을 위한 사이트 정보를 얻을 수 있지 않을까 생각하빈다.

3. Parsing 후 반환하는 String[] 형식을 DTO형식으로 변경하면 변경에 용이할거 같습니다.
매번 배열 순서를 체크해야하고, 모든 값이 String이라 헷갈릴 여지가 충분한거 같습니다. 이에 DTO 추가를 하면 어떨지 생각합니다.

테스트랑 스크랩 리다이렉션때문에 살짝 헤맸습니다 ㅜㅜ

@rlagkswn00 rlagkswn00 added 🔥 Enhancement New feature or request ⭐ Feat labels Nov 28, 2024
@rlagkswn00 rlagkswn00 self-assigned this Nov 28, 2024
Copy link

github-actions bot commented Nov 28, 2024

Unit Test Results

  48 files  ±0    48 suites  ±0   51s ⏱️ +7s
252 tests  - 5  251 ✔️ +4  1 💤  - 9  0 ±0 
255 runs   - 5  254 ✔️ +4  1 💤  - 9  0 ±0 

Results for commit 5afe41f. ± Comparison against base commit 9b605f6.

♻️ This comment has been updated with latest results.

@rlagkswn00 rlagkswn00 changed the base branch from main to develop November 28, 2024 07:03
@rlagkswn00 rlagkswn00 requested review from zbqmgldjfh and removed request for zbqmgldjfh November 29, 2024 12:23
@rlagkswn00
Copy link
Collaborator Author

기존 Disabled 되어있는 StaffScraperTest를 기존 코드 수정하다가 일부만 수정한채 커밋해서 생긴 문제였습니닷!!
아얘 테스트 갈아엎고 약간의 리팩토링(?)을 첨가해서 만들어서 올립니당..!

Copy link
Member

@zbqmgldjfh zbqmgldjfh left a comment

Choose a reason for hiding this comment

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

EmailConverter를 조금 수정하면 좋을것 같은데,
말로 설명하기가 어려워 수정을 조금 해봤습니다~
확인후 본인이 의도한 대로 동작 하는지 점검해줴요~

}

// Konkuk 도메인 우선 선택
private static String selectEmail(String[] emails) {
Copy link
Member

Choose a reason for hiding this comment

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

인자로 받는 emails가 0개인 일은 없나요?

Copy link
Collaborator Author

@rlagkswn00 rlagkswn00 Dec 2, 2024

Choose a reason for hiding this comment

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

현재는 빈 공백이라도 입력되도록 구현되어있어서 괜찮다고 넘긴 부분인데, 이 메서드의 입장에서 봤을 때 불확실한 믿을을 가져야 하는거 같아서 검증할 수 있도록 추가하겠습니다!


public static String convertValidEmail(String email) {
if (email == null || email.isBlank()) {
return ""; // 빈 입력 처리
Copy link
Member

@zbqmgldjfh zbqmgldjfh Dec 1, 2024

Choose a reason for hiding this comment

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

함수 이름이 convertValidEmail 인 상황인데 "" 이 나오면 이건 ValidEmail로 간주하는 것 일까요?
방어 로직이 있는거 자체는 좋아요!, 다만 호출하는쪽에서 null과 blank가 아닌 경우에 converting을 시도하는것도 좋을수도??
의견입니다!

Copy link
Collaborator Author

@rlagkswn00 rlagkswn00 Dec 2, 2024

Choose a reason for hiding this comment

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

동의합니다~! 조금 확장해서 EmailSupporter로 만드는게 더 좋을 수 있겠다는 생각이 드네용!
외부에서 null과 blank를 체크하는 메서드를 호출(isNullOrBlank)한 후 결과에 따라 convertEmail 메서드를 통해 변환하도록 구현하는게 조금 더 명확할거 같다고 생각합니다!

이런 방향성이면 PhoneNumberConverter도 수정이 필요할거 같네욤

//여러 이메일인 경우 있으니 분리.
String[] emailGroups = email.split("[/,]");
//정상 구조가 아닌 경우 구조 정상화
for (int i = 0; i < emailGroups.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

코드를 조금 다시 재구성 해봤는데 확인 부탁해요~

public class EmailConverter {

    private static final Pattern AT_PATTERN = Pattern.compile("\\s+at\\s+");
    private static final Pattern DOT_PATTERN = Pattern.compile("\\s+dot\\s+");
    private static final Pattern EMAIL_PATTERN = Pattern.compile("^[a-zA-Z0-9_!#$%&'\\*+/=?{|}~^.-]+@[a-zA-Z0-9.-]+$");

    private static final String KONKUK_DOMAIN = "@konkuk.ac.kr";
    private static final String EMPTY_EMAIL = "";

    public static String convertValidEmail(String email) {
        if (isNullOrEmpty(email)) {
            return EMPTY_EMAIL;
        }

        String[] emailGroups = splitEmails(email);
        String[] normalizedEmails = normalizeEmails(emailGroups);

        return selectPreferredEmail(normalizedEmails);
    }

    private static boolean isNullOrEmpty(String str) {
        return str == null || str.isBlank();
    }

    private static String[] splitEmails(String email) {
        return email.split("[/,]");
    }

    private static String[] normalizeEmails(String[] emailGroups) {
        return Arrays.stream(emailGroups)
                .map(EmailConverter::normalizeEmail)
                .toArray(String[]::new);
    }

    private static String normalizeEmail(String email) {
        if (isNullOrEmpty(email)) {
            return EMPTY_EMAIL;
        }

        if (EMAIL_PATTERN.matcher(email).matches()) {
            return email;
        }

        if (containsSubstitutePatterns(email)) {
            return replaceSubstitutePatterns(email);
        }

        return EMPTY_EMAIL;
    }

    private static boolean containsSubstitutePatterns(String email) {
        return DOT_PATTERN.matcher(email).find() && AT_PATTERN.matcher(email).find();
    }

    private static String replaceSubstitutePatterns(String email) {
        return email.replaceAll(DOT_PATTERN.pattern(), ".")
                .replaceAll(AT_PATTERN.pattern(), "@");
    }

    private static String selectPreferredEmail(String[] emails) {
        return Arrays.stream(emails)
                .filter(email -> !email.isBlank())
                .filter(email -> email.endsWith(KONKUK_DOMAIN))
                .findFirst()
                .orElseGet(() -> emails.length > 0 ? emails[0] : EMPTY_EMAIL);
    }
}

@rlagkswn00
Copy link
Collaborator Author

rlagkswn00 commented Dec 2, 2024

  1. 소나큐브 이슈 13개중 13개 해결, 추가 이슈 4개 중 2개 해결. (2개는 메서드,생성자 파라미터 개수 관련인데 어떻게 할까요,,,?)
  2. 스케쥴링 5분 설정 후 DB반영 확인
  3. API 호출 정상 호출 확인(api/v2/staffs/search)
    API 스펙에 position 추가해도 앱에서 호출할 때 문제 안생기는지 확실하지 않아서 우선 그대로 직위 없이 뒀습니닷!

@zbqmgldjfh 확인 부탁드립니당 👍

@rlagkswn00 rlagkswn00 requested a review from zbqmgldjfh December 2, 2024 10:17
Copy link
Member

@zbqmgldjfh zbqmgldjfh left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~

1.생성자 이슈는 무시하고 넘어가시쥬~
2.굳굳!
3.직위는 스크랩은 하되, API 반영은 클라이언트 팀과 직접 논의해보시쥬~

테스트에서 출력문만 제거해주시구~
develop으로 Squash Merge 해주세요~
Approve남겨둘게요~


//then
for (int i = 0; i < numbers.length; i++) {
System.out.println(numbers[i] + " " +convertedNumbers.get(i) + " " + answer[i]);
Copy link
Member

Choose a reason for hiding this comment

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

아마 테스트 용도였을 것 같은데? 검증에서 출력을 제외해주세요!~

@rlagkswn00 rlagkswn00 changed the title Feat : 리뉴얼 학교 홈페이지 기준 교직원 스크랩, 파싱 로직 변경 Feat : 학교 홈페이지 리뉴얼로인한 교직원 스크랩 및 스크랩간 직위 추가 Dec 2, 2024
@rlagkswn00 rlagkswn00 requested a review from zbqmgldjfh December 2, 2024 13:59
Copy link
Member

@zbqmgldjfh zbqmgldjfh left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~

@zbqmgldjfh zbqmgldjfh self-requested a review December 3, 2024 12:10
Copy link
Member

@zbqmgldjfh zbqmgldjfh left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

@rlagkswn00 rlagkswn00 merged commit a73c120 into develop Dec 3, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔥 Enhancement New feature or request ⭐ Feat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants