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

Check that every combination of event::Name and event::Version correspond to single Rust type (#1) #3

Merged
merged 4 commits into from
Sep 2, 2021

Conversation

ilslv
Copy link
Member

@ilslv ilslv commented Aug 31, 2021

Part of #1, #2

Synopsis

As discussed we should check that every combination of event::Name and event::Version correspond to single Rust type.

Solution

As TypeId::of() is const fn yet (tracking issue), we use codegen location to uniquely identify Rust type.

Checklist

  • Created PR:
    • In draft mode
    • Name contains Draft: prefix
    • Name contains issue reference
    • Has k:: labels applied
    • Has assignee
  • Documentation is updated (if required)
  • Tests are updated (if required)
  • Changes conform code style
  • CHANGELOG entry is added (if required)
  • FCM (final commit message) is posted
    • and approved
  • Review is completed and changes are approved
  • Before merge:
    • Milestone is set
    • PR's name and description are correct and up-to-date
    • Draft: prefix is removed
    • All temporary labels are removed

@ilslv ilslv added enhancement Improvement of existing features or bugfix k::api Related to API (application interface) k::design Related to overall design and/or architecture labels Aug 31, 2021
@ilslv ilslv added this to the 0.1.0 milestone Aug 31, 2021
@ilslv ilslv self-assigned this Aug 31, 2021
@ilslv
Copy link
Member Author

ilslv commented Aug 31, 2021

FCM

Check that every combination of event::Name and event::Version correspond to single Rust type (#3, #1)

Additionally:
- extend support for event::Initial in derive macros

@ilslv ilslv requested a review from tyranron August 31, 2021 07:39
Copy link
Member

@tyranron tyranron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilslv please, add also some examples with Initial<_> in #[derive(Event)]. It seems that we haven't covered this situation, which may result to be quite tricky one.

@ilslv
Copy link
Member Author

ilslv commented Sep 1, 2021

please, add also some examples with Initial<_> in #[derive(Event)]. It seems that we haven't covered this situation, which may result to be quite tricky one.

Quite tricky one, indeed. As we discussed, we want to express, that some state can be Initialized pretty strictly.

// Compiles just fine
#[derive(Event)]
enum ChatEvent {
    Created(Init<ChatCreated>),
    MessagePosted(ChatMessagePosted),
}

// Failed to compile, as we forgot the `Init<...>` wrapper
#[derive(Event)]
enum ChatEvent {
    Created(ChatCreated),
    MessagePosted(ChatMessagePosted),
}

Proposal

  1. Add Initial trait, which will be the same as Versioned, but with special blanket impl.
pub trait Initial {
    fn name() -> Name;

    fn version() -> Version;
}

impl<Ev: Initial + ?Sized> Event for Init<Ev> {
    fn name(&self) -> Name {
        Ev::name()
    }

    fn version(&self) -> Version {
        Ev::version()
    }
}
  1. Change existing blanket impls
//         ⌄ wrong
impl<Ev: Event + ?Sized, S: Sourced<Ev>> Sourced<Ev> for Option<S> {
    fn apply(&mut self, event: &Ev) {
        if let Some(state) = self {
            state.apply(event);
        }
    }
}

If we want to store Init<Event> inside enum variant, this blanket impl needs small change:

//            ⌄ was `Event`, became `Versioned`
impl<Ev: Versioned + ?Sized, S: Sourced<Ev>> Sourced<Ev> for Option<S> {
    fn apply(&mut self, event: &Ev) {
        if let Some(state) = self {
            state.apply(event);
        }
    }
}

And add new blanket

impl<Ev: Initial + ?Sized, S: Initialized<Ev>> Sourced<Init<Ev>> for Option<S> {
    fn apply(&mut self, event: &Initial<Ev>) {
        *self = Some(S::init(&event.0));
    }
}
  1. Add Initial derive macro, that works similar to Versioned

Main idea of those changes is to separate Initial events from Versioned (maybe rename it to reflect that separation?).

ack @tyranron

@tyranron
Copy link
Member

tyranron commented Sep 1, 2021

@ilslv Events are primary thing in event sourcing, while the state is secondary. It should be OK for the same Event to be initial for one state and not initial for another. Initiality is responsibility of event::Sourced/event::Initialized traits which express relation between an Event and the state. It's not the reposnibility of an Event itself.

I don't see why se should introduce an additional trait/macro to handle the case. It should be possible with current setup by either specializing over Initial wrapper (giving it some additonal type machinery) or introducing #[event(initial)] attribute for the variant which will consider that sitouation in code generation.

@ilslv
Copy link
Member Author

ilslv commented Sep 1, 2021

Discussed: we don't try to derive Event on Initial, but rather write our own copy of Borrow trait (but sealed) with blanket impl for Self and use it in codegen

@ilslv ilslv requested a review from tyranron September 2, 2021 06:48
@tyranron tyranron force-pushed the 1-unique-events-on-different-rust-types branch from eabd582 to 7701281 Compare September 2, 2021 10:09
///
/// [`Borrow`]: std::borrow::Borrow
#[sealed]
pub trait BorrowInitial<Borrowed: ?Sized> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having Borrowed as an associated type here will aid type inference a little bit better.

@tyranron tyranron removed the k::api Related to API (application interface) label Sep 2, 2021
@tyranron tyranron changed the title Draft: Check that every combination of event::Name and event::Version correspond to single Rust type (#1) Check that every combination of event::Name and event::Version correspond to single Rust type (#1) Sep 2, 2021
@tyranron tyranron marked this pull request as ready for review September 2, 2021 11:21
@tyranron tyranron merged commit 8efd428 into master Sep 2, 2021
@tyranron tyranron deleted the 1-unique-events-on-different-rust-types branch September 2, 2021 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement of existing features or bugfix k::design Related to overall design and/or architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants