arcane-rs / arcane

1 stars 1 forks source link

Storable events #8

Open 50U10FCA7 opened 1 year ago

50U10FCA7 commented 1 year ago

Synopsis

Storing events in repository.

Solution

Implement wrapper around Event containing metadata needed to determine event.

Checklist

50U10FCA7 commented 1 year ago

@tyranron Found a problem with TryFrom<Raw> impl codegen. It requires us to use reflect feature, because the types we hold in es::event::codegen::Reflect can't be compared with es::event::Name and es::event::Revision.

#[automatically_derived]
impl<'__raw, __Data> TryFrom<Raw<'__raw, __Data, ()>> for Event
where
    FileEvent: TryFrom<Raw<'__raw, __Data, ()>>,
    ChatEvent: TryFrom<
        Raw<'__raw, __Data, ()>,
        Error = <FileEvent as TryFrom<Raw<'__raw, __Data, ()>>::Error
    >
{
    type Error = FromRawError<<FileEvent as TryFrom<__Data>>::Error, ()>;

    fn try_from(raw: Raw<'__raw, __Data, ()>) -> Result<Self, Self::Error> {
        for (_, var_name, var_rev) in <FileEvent as codegen::Reflect>::META {
            if var_name == raw.name && var_rev == raw.rev {  // Can't compare because `Reflect` hold values as strings.
                return <FileEvent as TryFrom<__Data>>::try_from(raw.data)
                    .map(Self::File)
                    .map_err(FromRawError::FromDataError);
            }
        }

       /* check ChatEvent */

        Err(FromRawError::UnknownEvent {
            name: raw.name.to_string(),
            revision: raw.revision,
        })
    }
}

Possible solutions:

  1. Use es::event::reflect::* instead of es::event::codegen::Reflect, but this makes reflect functionality non-optional.
  2. Add bound to Revision makes it convertible into string variant to compare with es::event::codegen::Reflect value.
tyranron commented 1 year ago

@50U10FCA7 as the code is generated, I think we may go with string conversion.

50U10FCA7 commented 1 year ago

@tyranron Maybe we should block duplicated variants in enums, because it drives ambiguous behaviour.

#[derive(Event)]
enum MyEvent {
    #[event(init)]
    Init(SomethingHappened),
    NotInit(SomethingHappened),
}

Once enum from example above is deserialized/converted from raw it always be init.

Alternatively we can error in cases when duplicated variants have different Source-behaviour.

50U10FCA7 commented 1 year ago

@tyranron Maybe we should block duplicated variants in enums, because it drives ambiguous behaviour.

#[derive(Event)]
enum MyEvent {
    #[event(init)]
    Init(SomethingHappened),
    NotInit(SomethingHappened),
}

Once enum from example above is deserialized/converted from raw it always be init.

Alternatively we can error in cases when duplicated variants have different Source-behaviour.

Discussed: Type-level doesn't allows to use such Events later, so the block/warn is not required.

Type-check recursion when expanding from/into Raw conversion

Also discussed the problem with recursion without same variants deduplication.

For example the following code:

#[derive(Event)]
enum ParEvent {
    A(SubEvent),
    B(SubEvent),
}

will expand the following (code is simplified):

// from `event::Raw` to `ParEvent`
impl<__Data> TryFrom<Raw<__Data>> for ParEvent
where
    SubEvent: TryFrom<__Data>,
    SubEvent: TryFrom<__Data, Error = <SubEvent as TryFrom<__Data>>::Error>, // recursion
{
    type Error = <SubEvent as TryFrom<__Data>>:Error;

    fn try_from(raw: Raw<__Data>) -> Result<Self, Self::Error> {
        todo!()
    }
}

As discussed there is no way to dedup the variants properly in non type-level (in macro expansion), but I checked the following case:

type SubEvent2 = SubEvent;

#[derive(Event)]
enum ParEvent {
    A(SubEvent),
    B(SubEvent2),
}

which generates:

// from `event::Raw` to `ParEvent`
impl<__Data> TryFrom<Raw<__Data>> for ParEvent
where
    SubEvent: TryFrom<__Data>,
    SubEvent2: TryFrom<__Data, Error = <SubEvent as TryFrom<__Data>>::Error>, // no recursion!
{
    type Error = <SubEvent as TryFrom<__Data>>:Error;

    fn try_from(raw: Raw<__Data>) -> Result<Self, Self::Error> {
        todo!()
    }
}

And I think its OK to dedup exactly the same variants because only them drives recursion.

50U10FCA7 commented 1 year ago

Found an alternative to current discussed design of the event::Raw. Instead of using typed revision in it we can use a string representation of it (like we do in es::event::codegen::Reflect). It may help to abstract the event::Raw from exact Event. When casting from abstracted Raw into concrete Event we only need to check that two strings of revision are equal. This way requires change the ToString bound of es::event::Revision to FromStr.