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

Control over view_id when creating a view #94

Closed
carlos-verdes opened this issue Apr 24, 2024 · 4 comments
Closed

Control over view_id when creating a view #94

carlos-verdes opened this issue Apr 24, 2024 · 4 comments

Comments

@carlos-verdes
Copy link

Related with #90

When we use CQRS the main idea is to segregate commands and queries right?
On this crate we have control over the aggregate_id when we are managing commands, however when we build a view the view_id is populated always with the aggregate_id of an specific aggregate.

This second part for me breaks the main advantage of CQRS, first of all I should be able to use events from 2 different aggregates and secondly (as the problem on issue #90) in many cases the query result (let's call it view record) will not match 1:1 with an aggregate nor have the same id.

image
You can see in the picture how the Query and Command model are not 1:1 (and they should not)

Example where aggregate id is different from view id:

  • Aggregate Portfolio
  • View UserPortfoliosOverview --> aggregated view of all portfolios for a specific user

The Portfolio aggregate commands will use a generated id for identity and will add the user_id as part of the metadata.
The view will use user_id as a key, and in every PortfolioCreated event will decide if it updates the view or not.

Example with different aggregates:

  • Aggregate Portfolio
  • Aggregate Markets
  • View PortfolioUsdValueView --> it will update the portfolio value when a NewPriceEvent is emitted by Markets or a NewAssetAdded is emitted by Portfolio

This case can be managed if we consider PortfolioUsdValue as an aggregate, but the reality is just a view (we will never execute a command from users).
We need to be able on this scenario to receive events from Portfolio and Market aggregates.

What I found

When we submit a command we are able to provide 3 parameters:

  • aggregate_id
  • payload
  • metadata
  cqrs.execute_with_metadata(aggregate_id, payload, metadata)

However when we write a query we can only update the payload of the view record.

When I see the implementation of execute_with_metadata I see why the aggregate_id becomes the view_id (this is only a specific case but not the general expected behavior of a query entity)

pub async fn execute_with_metadata(
        &self,
        aggregate_id: &str,
        command: A::Command,
        metadata: HashMap<String, String>,
    ) -> Result<(), AggregateError<A::Error>> {
       ...
        for processor in &self.queries {
            let dispatch_events = committed_events.as_slice();
            processor.dispatch(aggregate_id, dispatch_events).await;
        }
        Ok(())
    }

Then I see the trait Query has couple of things I'm not fully aligned:

  • it expects a Aggregate type (why not a ViewRecord or a QueryEntity type)
  • there is no way to specify nor change the view_id
  • the aggregate_id is already part of EventEnvelope, so there is no need for it as parameter of dispatch function

Current implementation:

#[async_trait]
pub trait Query<A: Aggregate>: Send + Sync {
    /// Events will be dispatched here immediately after being committed.
    async fn 
    dispatch(&self, aggregate_id: &str, events: &[EventEnvelope<A>]);
}

Proposal

So maybe we could change this into something like:

#[async_trait]
pub trait Query<A: ViewRecord>: Send + Sync {
    /// no need for aggregate_id parameter
    async fn dispatch(&self, events: &[EventEnvelope<A>]);
}

#[async_trait]
pub trait ViewRecord: Default + Serialize + DeserializeOwned + Sync + Send {

    fn view_record_id() -> String;
    fn apply(&mut self, event: Self::Event);
}

And to not disrupt current implementation we can provide From implementations like:

impl From<Aggregate> for ViewRecord ...

Share your thoughts I may be missing something.

@davegarred
Copy link
Collaborator

when we build a view the view_id is populated always with the aggregate_id of an specific aggregate

The GenericQuery reuses the aggregate_id as the view_id, but this is certainly not the only way to do it. It is a common way to handle it and handles many use cases, which is why it is included here.

Note that the dispatch method of Query receives the entire event envelope. For cases where you would you do not want the aggregate_id linked with the view_id, you should build a custom query that looks deeper into the event to find the relevant details for that query.

first of all I should be able to use events from 2 different aggregates

Absolutely, but this is a case where you would want a custom query to handle this logic.

The GenericQuery is useful for simple cases but for anything more complex it should be used as a reference in building a custom query. This may be a documentation failure on my part in making this point clear.

@carlos-verdes
Copy link
Author

Thanks for your answer, but I think the main issue is that both the Query and the CQRS framework only accept one Aggregate type, so I still can't implement a Query which accepts events from two or more aggregates.

For the Query implementation I would lax the definition and instead of asking for the Aggregate type I would just ask for an Event type, then you can do things like:

use aggregate1::Aggregate1Event;
use aggregate2::Aggregate2Event;

enum QueryEvents {
    Aggregate1Event(Aggregate1Event),
    Aggregate2Event(Aggregate2Event),
}

impl DomainEvent for QueryEvent {
    fn event_type(self) -> String {
        match self {
            QueryEvent::Aggregate1Event(event) => event.event_type(),
            QueryEvent::Aggregate2Event(event) => event.event_type(),
        }
    }
}

impl From<Aggregate1Event> for QueryEvents {
    fn from(event: Aggregate1Event) -> Self {
        QueryEvent::Aggregate1Event(event)
    }
}

impl From<Aggregate2Event> for QueryEvents {
    fn from(event: Aggregate2Event) -> Self {
        QueryEvent::Aggregate2Event(event)
    }
}

For the CQRS framework I would add a new abstraction called BoundedContext where you can define all Aggregate types that are part of that bounded context, this way you can manage all commands and events related with the bounded context using a single framework.

@davegarred
Copy link
Collaborator

I think the main issue is that both the Query and the CQRS framework only accept one Aggregate type, so I still can't implement a Query which accepts events from two or more aggregates.

I see your point now. With two aggregates you certainly need two different Query implementations even though it they have the same backing storage and view projection. It would certainly make sense to reduce some of this overhead.

I also like the idea of a bounded context tying multiple aggregates together, that has been a point of confusion.

@carlos-verdes would you mind opening a new issue with your thoughts on these changes?

@carlos-verdes
Copy link
Author

Created issues #96 and #97

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

No branches or pull requests

2 participants