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

🎊 Refactoring the project into smaller distinct crates for developer productivity. #239

Closed
14 of 15 tasks
insertish opened this issue Apr 22, 2023 · 4 comments
Closed
14 of 15 tasks
Labels
enhancement New feature or request meta Issues regarding repository maintenance

Comments

@insertish
Copy link
Member

insertish commented Apr 22, 2023

So I've done a bunch of design work around the architecture / strategy and ended up with a nice solution for developing the backend going forwards which I'll get into below.

First of all, the main driving motivations behind this are to:

  • Decouple the database models from the API models
    This causes a bunch of issues because you have to change both at once, you can't have API only fields without also putting them in the database model, etc.
  • Remove hard dependencies on the database for using the API models on their own and hence allow API library developers to consume these models directly from their libraries
    By splitting API models into their own crate, developers can freely pull this in without pulling in hundreds of other unrelated dependencies which they don't need. As well as this, developers need not manually figure out their models anymore.
  • Allow versioning and improving the API independent of database models.
    This will give us the freedom to improve the API without needing database migrations and as such, we could deploy multiple versions of the API with different models as people transition over in the future.
  • Introduce testing on top of the core database models and then eventually on top of the API as well.

There are a couple conditions with this refactor though:

  • It must be ongoing not a one-shot; this is a lot of work that can't be done in one go.
  • As per the previous point, everything must still work as things are refactored.
  • To reiterate, do not break the API! But instead complement new tests and fix anything wrong.

So, here's what's changed:

  • The project structure is now as follows:
    |- crates
      |- core
        |- database: Database Implementation (In-Memory and MongoDB)
        |- result: Result and Error types
        |- models: API Models
        |- presence: Global user presence system
      |- quark: (now deprecated) the old "core" crate for everything
      |- delta: API, nothing changes here
      |- bonfire: events, nothing changes here
    
  • API routes now have access to rocket::State<revolt_database::Database> for any new database model operations.
  • There is a new in-memory database for development purposes as opposed to the old useless "dummy db".
  • All (new and current that have been ported) database models are and must be unit tested! 🎉
    Continous integration has already been setup to run unit tests against the in-memory database and MongoDB.
  • Quark errors may be mapped to from the new revolt_result::Error type, do it as such:
    // db comes from revolt_database
    // we use Error::from_core with map_err to convert
    db.fetch_bot(&target.id).await.map_err(Error::from_core)?;
    Your errors should now look like:
    {
      "type": "NotFound",
      "location": "crates/core/database/src/models/bots/ops/mongodb.rs:15:62"
    }
  • Since the database and API models are separate, you can convert between them by using .into() or the appropriate function if more than one object is required from the database!

There are also some things that need to be sorted out still:

  • Relicense revolt-result crate to be MIT
  • Relicense revolt-models crate to "be MIT" (without from_database feature) or "MIT / AGPL" (with from_database feature)
    I'd need to read up a bit on this and check if I need to do anything special here with how Cargo dependencies work, in theory I should just need to add a notice that while revolt-models itself is MIT, it has an optional dependency to an AGPL licensed crate.

You can read the new code on the refactor/split-project-into-core-crates branch (which will be merged in the near future).
These changes will also be used to test #210 (webhooks) before it is merged in.

Future plans:

  • Clean up the bad things about the API, for example, getting rid of _id in favour of id or introducing a better system for remvoing fields such as sending empty strings. As mentioned earlier, we could provide a "v1" and "v2" API endpoint to avoid disruption since database models would be separate.
  • Eventually migrate to utoipa, epic: switch to utoipa #215.

Note: If I forgot something, I'll edit the post and also mention it in the comments that it's changed.

Remaining models and implementations:

Other issues to tackle:

@insertish insertish added enhancement New feature or request meta Issues regarding repository maintenance labels Apr 22, 2023
@insertish insertish changed the title Refactoring the project into smaller distinct crates for developer productivity. 🎊 Refactoring the project into smaller distinct crates for developer productivity. Apr 22, 2023
@insertish insertish pinned this issue Apr 22, 2023
@insertish
Copy link
Member Author

Just added a presence crate to core with a unit test covering the whole crate.
Found one issue with how I was handling flags in the mean time and that's been fixed now.

@insertish
Copy link
Member Author

insertish commented Apr 26, 2023

Overall progress for porting models:

  • Port quark database models to core/database.
  • Create corresponding models in core/models/[..]/v0.
  • Create conversions from database to v0 models.

moved to OP

@insertish
Copy link
Member Author

delta now no longer uses quark and only relies on core crates
bonfire TODO (but it'll be a much faster and smaller job)

@insertish
Copy link
Member Author

this took a year :clueless:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request meta Issues regarding repository maintenance
Projects
Archived in project
Status: Pending / Staging
Development

No branches or pull requests

1 participant