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

[5기 조인수] Shorten URL 과제 PR입니다. #71

Open
wants to merge 42 commits into
base: ZZAMBAs
Choose a base branch
from

Conversation

ZZAMBAs
Copy link

@ZZAMBAs ZZAMBAs commented Dec 17, 2023

📌 과제 설명

원본 URL을 짧은 URL로 변환하는 프로그램을 만들어 냅니다. 다음을 만족하도록 합니다.

  • URL 입력폼 제공 및 결과 출력
  • URL Shortening Key는 8 Character 이내로 생성
  • 단축된 URL 요청시 원래 URL로 리다이렉트
  • 단축된 URL에 대한 요청 수 정보저장 (optional)
  • Shortening Key를 생성하는 알고리즘 2개 이상 제공하며 애플리케이션 실행중 동적으로 변경 가능 (optional)

👩‍💻 요구 사항과 구현 내용

image
image
구조와 ERD는 위와 같습니다. ERD에서 변환된 URL(transformed_url)을 PK로, 원본 URL(original_url) 앞 255자를 인덱스로 설정하여 검색 속도를 향상시키도록 하였습니다.

기본적인 구현은 BASE62 알고리즘을 이용하도록 하였습니다.

  • 사용자는 메인 화면에서 URL을 변환시킬 수 있습니다. 원본 URL의 앞 http 또는 https 프로토콜을 제거하여 저장하도록 하였고, 짧은 URL로 저장할 때, 처음에 원본 URL이 DB에 존재하는지 확인한 후, 존재하지 않으면 저장하고 저장한 엔티티를 반환합니다. 존재하면 DB에서 존재하는 엔티티를 꺼내어 반환합니다. 이 때 사용자는 해당 짧은 URL 방문 횟수가 얼마나 되고, 언제 마지막으로 방문했는지를 알 수 있습니다.
    image
    image

  • 짧은 URL로 요청을 하게되면 DB에서 바로 짧은 URL을 이용해 엔티티를 검색합니다. 이 때 없으면 UrlException을 반환합니다. 있으면 엔티티로부터 원본 URL을 가져오고 리다이렉트합니다. 이 때, 마지막 접속 일자와 방문 횟수가 업데이트 됩니다.

패키지 구조는 도메인을 위주로 하였습니다. 사실상 도메인이 ShortUrl 하나밖에 없다보니 springbooturlshortener/domain/shorturl 패키지에 모든 부분이 들어가 있습니다. 주요 클래스를 설명하겠습니다.

  • config/WebMvcConfig: favicon.ico를 아예 불러오지 않도록 mvc 설정을 수정했습니다. 계속 /favicon.ico를 불러오다보니 컨트롤러단에서 에러가 났습니다.
  • shorturl/service/transformer/Base62Transformer: BASE62 알고리즘 기반으로 인코딩합니다. 디코딩 기능은 존재하지 않습니다.
  • shorturl/exception/UrlException: shortUrl 처리 과정에서 일어나는 예외를 담당합니다. 지금은 존재하지 않는 원본 URL을 DB에서 가져오려 할 때에 대한 예외만 존재합니다.

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

  1. 현재는 ShortUrl을 생성하기 위해여 랜덤으로 BASE62 6자리를 생성하고 그것이 DB에 존재하면 다시 생성하는 식으로 반복하면서 저장하고 있습니다. 이것은 나중에 문제가 될까요?
  2. 이 과제를 함에따라 여러 블로그들을 참고하였고, 상당수 많은 디자인패턴을 적용하고 있음을 알았습니다. 퍼사드 패턴을 이용하거나 인코딩 방식이 여러개일 경우 팩토리 형식으로 클래스를 만드는 등 다양했습니다. 저는 이를 생각조차 못했었는데, 이런 디자인 패턴 적용에 대해 술술 생각이 나려면 따로 연습? 방법이 존재하는지 아니면 개발을 하다보면 느는건지 궁금합니다.
  3. UrlTransformer에 대해 ShortUrlService에서 @Qualifier를 이용하여 주입을 해주었습니다. 이것보다 팩토리 형식(이를테면 ShortURLGeneratorFactory.getStrategy(GenerateType.BASE62); 같은 형식)으로 생성해 주는 것이 나은 선택인지 궁금합니다!
  4. 아직 실험하진 않았지만, original url 앞 255자를 인덱스로 설정하는 것보다 original url을 해시함수로 해싱한 뒤, 그 값을 DB에 저장하도록 하고, 그 해시 컬럼을 인덱스로 설정하면 더 빠르지 않을까 생각했는데 멘토님의 생각은 어떠신가요? 아니면 더 나은 빠른 성능 향상 방법이 있을까요?

Copy link

@SangHyunGil SangHyunGil left a comment

Choose a reason for hiding this comment

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

작성해주시느라 고생많으셨습니다.
간단하게 코멘트 남겨두었습니다.
테스트 코드를 꼼꼼하게 작성해주셔서 좋네요.
나중에 단위테스트를 통해 작성하는 것도 고민해봐주세요.


현재는 ShortUrl을 생성하기 위해여 랜덤으로 BASE62 6자리를 생성하고 그것이 DB에 존재하면 다시 생성하는 식으로 반복하면서 저장하고 있습니다. 이것은 나중에 문제가 될까요?

  • 초기에는 대부분 충돌이 나지 않을것이라고 기대하고 있기에 큰 문제는 없을 것 같습니다. 다만, 6자리로 이루어져있기에 62^6만큼 점점 차게되는 순간 충돌 횟수는 점점 늘어날 것이기에 서비스와 연관지어 자리수를 고려해보는게 좋을 것 같습니다.

이 과제를 함에따라 여러 블로그들을 참고하였고, 상당수 많은 디자인패턴을 적용하고 있음을 알았습니다. 퍼사드 패턴을 이용하거나 인코딩 방식이 여러개일 경우 팩토리 형식으로 클래스를 만드는 등 다양했습니다. 저는 이를 생각조차 못했었는데, 이런 디자인 패턴 적용에 대해 술술 생각이 나려면 따로 연습? 방법이 존재하는지 아니면 개발을 하다보면 느는건지 궁금합니다.

  • 처음에 당연히 말씀해주신것처럼 어떤 디자인 패턴을 적용하는게 적합한지 난해할 수 밖에 없다고 생각해요.
    이러한 접근이 되기까지는 많은 시간이 필요할 것 같은데요.
    저는 디자인 패턴을 외워서 이 상황에선 이러한 디자인 패턴을 써야해! 라는 방향으로 작성하진 않아요. 디자인 패턴도 문제 상황에 맞는 정형화된 해결 방법을 패턴화시킨 것 중 하나이기에 디자인 패턴을 왜 썼고, 왜 이렇게 풀어나가려고 했는지를 기반으로 학습하려고 했어요. 그래서 문제 상황에서 이 패턴을 써야해! 로 접근한다기보다 이러한 문제를 해결하기 위해선 이렇게 풀면 조금 더 유연하고 확장성있게 작성할 수 있겠다를 고민하다보면 비슷한 해결책으로 접근하게 되더라구요.
    그래서 학습하실때 위와 같은 방법으로 학습해보시는 것도 추천드려요.

UrlTransformer에 대해 ShortUrlService에서 @qualifier를 이용하여 주입을 해주었습니다. 이것보다 팩토리 형식(이를테면 ShortURLGeneratorFactory.getStrategy(GenerateType.BASE62); 같은 형식)으로 생성해 주는 것이 나은 선택인지 궁금합니다!

  • 팩토리를 통해 생성해주는 것도 방법이고, UrlTransformer을 맵으로 주입받아 직접 꺼내는 방법도 고려해볼 수 있고 다양한 방법이 있을뿐 어떤게 더 좋아라는건 없는 것 같아요.
    저는 개인적으로 인터페이스를 맵이나 리스트의 자료구조로 받아 타입으로 적합한 걸 꺼내서 사용하는걸 선호해요.

아직 실험하진 않았지만, original url 앞 255자를 인덱스로 설정하는 것보다 original url을 해시함수로 해싱한 뒤, 그 값을 DB에 저장하도록 하고, 그 해시 컬럼을 인덱스로 설정하면 더 빠르지 않을까 생각했는데 멘토님의 생각은 어떠신가요? 아니면 더 나은 빠른 성능 향상 방법이 있을까요?

  • 별도의 컬럼으로 저장하면서 해당 컬럼을 인덱스로 두는 것보다, id 값을 transformedUrl로 쓰니 original url을 변조시킨 값으로 pk 검색을 하면 pk 인덱스를 타서 더욱 빠르지 않을까라는 생각도 듭니다.
    내부적으로 변환하는 시간은 정말 짧을테니까요.

implementation 'org.springframework.boot:spring-boot-starter'
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
implementation 'com.mysql:mysql-connector-j:8.2.0'

Choose a reason for hiding this comment

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

런타임에만 필요해보이는데 관련해서 확인해주세요.

Copy link
Author

Choose a reason for hiding this comment

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

아무 생각 없이 implementation으로 받아오면 사실 scope 생각을 안해도 되는데 scope를 고려해야 하는 이유는 따로 있을까요?

Comment on lines +32 to +33
testCompileOnly 'org.projectlombok:lombok'
testAnnotationProcessor 'org.projectlombok:lombok'

Choose a reason for hiding this comment

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

테스트 시에도 롬복이 필요한가요?

Copy link
Author

Choose a reason for hiding this comment

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

ShortUrlServiceTest에서 일부 테스트 값을 출력해보기 위해 사용 했었습니다!

public class WebMvcConfig implements WebMvcConfigurer {
@Override
public void addResourceHandlers(ResourceHandlerRegistry registry) {
registry.addResourceHandler("favicon.ico").resourceChain(false);

Choose a reason for hiding this comment

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

왜 제거하려고 하신건가요?

Copy link
Author

Choose a reason for hiding this comment

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

계속 자동으로 /favicon.ico를 불러오다보니 자꾸 "localhost:8080/favicon.ico"로 요청이 가서 서버 로그에 자꾸 에러 로그가 찍혔습니다... 그래서 제거해 주었습니다.

Comment on lines +18 to +19
@DynamicInsert
@DynamicUpdate

Choose a reason for hiding this comment

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

사용하신 이유가 뭔가요?

Copy link
Author

Choose a reason for hiding this comment

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

ShortUrl 내부 lastVisitTime 필드는 마지막 방문 일시를 저장하는데 이 부분이 null일 경우도 있어서 그 경우에는 JPA에서 아예 그 컬럼에 대한 INSERT 문을 안날리도록 하기 위해 추가했습니다!

Comment on lines 27 to 28
@Column(name = "original_url", nullable = false)
private String originalUrl;

Choose a reason for hiding this comment

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

originUrl은 유니크 하지 않아도 되는걸까요?
관련해서 고민해봐주세요.

Copy link
Author

Choose a reason for hiding this comment

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

확실히 고유해야 할 것 같습니다. 추가하겠습니다!

int randomInt;

do {
randomInt = random.nextInt(0, Integer.MAX_VALUE);

Choose a reason for hiding this comment

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

randomInt 또한 url 생성에 포함된다면 urlTransformer안에 포함되어도 될 것 같아보이네요.

Copy link
Author

Choose a reason for hiding this comment

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

오 맞습니다. 반영해 보겠습니다!

class SpringBootUrlShortenerApplicationTest {
@Test
void contextLoad() {

Choose a reason for hiding this comment

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

불필요 공백이 있어보이네요.


import static org.assertj.core.api.Assertions.assertThat;

@SpringBootTest

Choose a reason for hiding this comment

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

간단의견) 나중에 필요한 빈만 쓸 수 있도록 @ConfigurationContext을 활용해봐주세요.

driver-class-name: com.mysql.cj.jdbc.Driver
username: root
password: 1234
url: jdbc:mysql://localhost:3306/test

Choose a reason for hiding this comment

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

간단의견) in-memory를 활용하는걸 고려해봐주세요.

shortUrlRepository.deleteAll();
}

@Test

Choose a reason for hiding this comment

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

이미 잘 작성해주신 것 같은데 given-when-then을 명시적으로 표현해서 가독성을 높여보는 것도 좋은 것 같아요.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants