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

Refactor: Switch to use username for document key instead of email #29

Open
mgroves opened this issue Jun 30, 2023 · 2 comments
Open

Refactor: Switch to use username for document key instead of email #29

mgroves opened this issue Jun 30, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@mgroves
Copy link
Owner

mgroves commented Jun 30, 2023

Seems like username is a better choice for document key, especially since endpoints Follow, Unfollow, and Get profile all have username in their URL. Username is going to be used more for lookups than Email. Using a K-V operation in Couchbase is faster than a SQL query, if the result is only ever supposed to be ONE user/profile. So therefore, refactor to use K-V more often.

@mgroves mgroves added the enhancement New feature or request label Jun 30, 2023
@mgroves
Copy link
Owner Author

mgroves commented Jul 10, 2023

Summary: The Conduit spec requires email for login, but otherwise username is used by most other endpoints. I'm making the assumption that login will happen less frequently than other operations, so therefore the overall app will be less affected by the overhead of SQL when using it for login and k/v elsewhere.

Endpoint analysis:

  • Authentication - uses email, JWT token expires in 4 hours
  • Registration - can use username for inserting new user
  • Get Current User - as long as username is added to JWT token claims, can use username for lookup
  • Update User - can use username
  • Get profile - username is required
  • Follow user - username is required
  • Unfollow user - username is required
  • List Articles - k/v was not really an option here in the first place
  • Feed Articles - ditto
  • Get Article - username is returned as part of author, not emai
  • Create Article - username pulled from JWT token
  • Update Article - username pulled from JWT token
  • Delete Article - doesn't involve user
  • Add Comments to an Article - username pulled from JWT token
  • Get Comments from an Article - username is returned under 'author'
  • Delete Comment - doesn't involve user
  • Favorite Article - username part of author
  • Unfavorite Article - username part of author
  • Get Tags - user not involved

Handlers that needs changed:

  • GetCurrentUserHandler - get username from claim instead of email, use GetUserByUsername instead of GetUserByEmail
  • UpdateUserHandler - use GetUserByUsername instead of GetUserByEmail

Data access that needs changed:

  • UserDataService::GetUserByEmail - should use SQL++ instead of key/value
  • UserDataService::GetUserByUsername - should use key/value instead of SQL++
  • UserDataService::RegisterNewUser - use username as key, not email
  • User Model - Username should get [JsonIgnore], not Email
  • UserDataService::UpdateUserFields - this can now update email, can no longer change username, use username as key instead of email
  • UserDataService::GetProfileByUsername - use k/v instead of SQL++
  • UserDataService::DoesExistUserByEmailAndUsername - switch around username/email address usage

Auth changes:

  • AuthService::GenerateJwtToken Put username in the JWT claims (maybe in addition to email?)
  • AuthService - will probably need a GetUsernameClaim

Migration changes:

  • Rollback migration 2
  • CreateIndexOnUsernameFieldInUsersCollection - since this is the latest migration, just rename and change it to index email instead of username
  • Run all migrations up

Validator changes:

  • No changes needed.

Controllers/Endpoints changes:

  • No changes needed.

@mgroves
Copy link
Owner Author

mgroves commented Jul 12, 2023

I think this went pretty smoothly. All tests are passing, and a manual run-through of the endpoints is working okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant