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: changes to aggregate::Repository, event::Store, documentation and more #289

Merged
merged 12 commits into from
Feb 23, 2024

Conversation

ar3s3ru
Copy link
Collaborator

@ar3s3ru ar3s3ru commented Feb 23, 2024

In the scope of a closer v0.5.0 release, this PR introduces quite some refactorings to different APIs, which are still experimental:

refactor: generalize event::StreamVersionExpected into version::Check

version::Check is a more general phrasing and naming for optimistic locking, yet expressing quite more precisely its role.

feat: add new aggregate::test::Scenario

It is now possible to write test assertions on aggregate::Root methods, rather than having that only available through a command::test::Scenario.

refactor(aggregate::repository): split Repository traits once again, use concrete error type

After trying the approach of having an unified aggregate::Repository trait, and use an AnyRepository with Error = anyhow::Error (so, opaque) for better usage in trait objects, we've reverted back to Interface Segregation:

  1. Have two separate interfaces -- Saver and Getter -- and a Repository one, which extends both,
  2. The errors returned by those are concrete types, using thiserror, rather than using an associated type to abstract over it.

While this has required the introduction of opaque errors with anyhow::Error in the concrete error types, this should provide a better ergonomics for consumers of this API.

refactor(event::store): move symbols to this module, add AppendError concrete type

The event::Store follows the same approach as aggregate::Repository now: split interfaces, concrete error types. The only exception being Streamer::stream, where it seems to be fine to abstract over the error type returned (for now).

refactor(eventually-macros): remove derive(Message), as it requires better design

#[derive(Message)] requires better design; we've implemented that with a specific idea in mind, but that's very limiting and wouldn't be used most of the time.

We remove it for now while we collect some ideas on how it should work.

chore: add vscode setting for rust-analyzer to use all features

Finally rust-analyzer under VSCode will compile and check the workspace using all feature flags.

docs(eventually): add missing documentation

There was A LOT of missing documentation. This PR addresses some of the missing stuff.
It's not great by any means, we'd likely need some dedicated effort before releasing.

refactor: split Serde trait into separate ones, implement serde::Convert and serde::ProtoJson

After some production usage, we've collected some different ideas on how this trait should work, and have implemented that following the previous concepts: split methods/traits, concrete error types.

chore: update all versions

Too many open dependabot PRs, this will take care of them all at once (but syn, which is a major change).

Copy link

codecov bot commented Feb 23, 2024

Codecov Report

Attention: Patch coverage is 62.53687% with 127 lines in your changes are missing coverage. Please review.

Project coverage is 84.89%. Comparing base (be239c7) to head (4a4977e).

Files Patch % Lines
eventually/src/aggregate/test.rs 0.00% 61 Missing ⚠️
eventually/src/serde.rs 23.40% 36 Missing ⚠️
eventually/src/aggregate/mod.rs 75.51% 12 Missing ⚠️
eventually-postgres/src/aggregate.rs 86.66% 6 Missing ⚠️
eventually/src/tracing.rs 0.00% 5 Missing ⚠️
eventually-postgres/src/event.rs 93.61% 3 Missing ⚠️
eventually/src/event/store.rs 87.50% 3 Missing ⚠️
eventually/src/aggregate/repository.rs 97.82% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #289      +/-   ##
==========================================
- Coverage   91.53%   84.89%   -6.64%     
==========================================
  Files          17       15       -2     
  Lines        1158     1225      +67     
==========================================
- Hits         1060     1040      -20     
- Misses         98      185      +87     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ar3s3ru ar3s3ru marked this pull request as ready for review February 23, 2024 23:35
@ar3s3ru ar3s3ru enabled auto-merge (squash) February 23, 2024 23:36
@ar3s3ru ar3s3ru merged commit 2c91b45 into main Feb 23, 2024
6 of 8 checks passed
@ar3s3ru ar3s3ru deleted the feat/add-aggregate-scenario branch February 23, 2024 23:41
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.

1 participant