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

Introduce UrlRepository abstraction layer #165

Merged
merged 1 commit into from
Jun 9, 2020
Merged

Conversation

JasonChong96
Copy link
Contributor

@JasonChong96 JasonChong96 commented Jun 8, 2020

Problem

According to #156, the back-end directory structure beginning to look incoherent and is showing signs of unbridled growth. It is currently a good time for refactoring given the recent expansion of file upload feature.

This process will start with refactoring and introducing an abstraction barrier between the data store and other parts of the code.

Solution

Refactor API endpoints to use UrlRepository when accessing data store.

Improvements:

  • All endpoints that edit URLs now access the same UrlRepository method, which can handle all cases. e.g File vs Not file, Changing state, Changing ownership
  • Removed now obselete old UrlCache and old UrlRepository

Alternatives Considered:

I have considered sharing more methods in the BaseRepository interface but found that we have few repositories and they have unique requirements. e.g UrlRepository needs to handle files/cache/db, Otp needs cache only, User needs db only.
For the same reason, I also decided against having a BaseCache/SequelizeRepository and it is difficult to share code and method signatures.

As transactions in URL involve both S3 and Sequelize, it is difficult to abstract them into different files while keeping code readable, hence the relatively large size of UrlRepository.

Tests

I have run through the following on staging:

  1. Creating URL works
  2. Transferring ownership of URL works
  3. Editing of URL works
  4. Redirect works (The automated tests should cover this but just to be sure)
  5. Toggling state to active/inactive works and is reflected accordingly
  6. Repeat all previous steps for files

@JasonChong96 JasonChong96 marked this pull request as draft June 8, 2020 15:30
@JasonChong96 JasonChong96 marked this pull request as ready for review June 8, 2020 15:39
@JasonChong96 JasonChong96 requested a review from liangyuanruo June 8, 2020 15:39
@JasonChong96 JasonChong96 marked this pull request as draft June 8, 2020 15:40
@JasonChong96 JasonChong96 marked this pull request as ready for review June 8, 2020 15:53
@JasonChong96 JasonChong96 requested a review from liangyuanruo June 8, 2020 16:02
@JasonChong96 JasonChong96 merged commit 123f559 into develop Jun 9, 2020
@JasonChong96 JasonChong96 deleted the url-repository branch June 9, 2020 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants