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

Akka Should Allow Separate Read and Write Event Adapter Bindings #4567

Closed
briansain opened this issue Sep 10, 2020 · 8 comments
Closed

Akka Should Allow Separate Read and Write Event Adapter Bindings #4567

briansain opened this issue Sep 10, 2020 · 8 comments

Comments

@briansain
Copy link
Contributor

  • Akka.Net Version: 1.3

We are currently using Akka.Net and another 3rd party library (Akkatecture) in our application. Akkatecture has their own class that inherits from IReadEventAdapter that is used for event upgrading, and we have built our own IWriteEventAdapter that will tag our events according to our specifications. We made changes to some of our events, so we need to use IReadEventAdapter in order to upgrade those events to the new version, as well as use our own event tagging IWriteEventAdapter. We're not able to bind multiple read event adapters and a single write adapter, to a single event type.

Example/Reproduce (Taken from .\src\core\Akka.Persistence.Tests\Journal\MemoryEventAdaptersSpec.cs):

In the constructor of MemoryEventAdaptersSpec, I add this configuration to the in-line hocon file:

  `""" + typeof(ReadWriteEvent).FullName + @", Akka.Persistence.Tests"" = [reader, writer]`

And then below, I add this class:

    public class ReadWriteEvent
    {
        public override string ToString()
        {
            return "ReadWriteEvent()";
        }
    }

I should expect that the Event Adapters that are bound to ReadWriteEvent would be an instance of the reader event adapter and the write event adapters that are defined in the test class constructor. However, the writer adapter isn't being used, but a NoopWriteEventAdapter is being set instead.

Proposed Solution:
I will have a pull request out shortly that will add another event adapter that will accept multiple read event adapters and a single write event adapter.

@ismaelhamed
Copy link
Member

@briansain @Aaronontheweb

You can only have multiple event adapters if they're of type read. Otherwise, any write event adapter will be wrapped in a CombinedReadEventAdapter for safety, I guess.

Now, as to why it's not allow to have multiple read event adapters with just once write event adapter, I'm not quite sure. But you can easily overcome this by creating a full-fledged EventAdapter:

public class ReaderWriterAdapter : IEventAdapter
{
    public string Manifest(object evt) => string.Empty;

    public object ToJournal(object evt) => "to-" + evt;

    public IEventSequence FromJournal(object evt, string manifest) => EventSequence.Single("from-" + evt);
}

@briansain
Copy link
Contributor Author

briansain commented Sep 11, 2020

@ismaelhamed @Aaronontheweb

Normally I would agree, but our issue is we're using a 3rd party service for our read adapter, and we wrote our own write adapter. I won't be able to create a full-fledged EventAdapter because I don't have access to their read adapter.

In the documentation for Event Adapters it says that we can configure an array of 1 read and 1 write EventAdapters for an event binding. It was confusing to me on why I wasn't able to apply a read adapter and a write adapter.

event adapter config documentation

@Aaronontheweb
Copy link
Member

Hmmmm.... I think the rules are that only a single type of binding can be used for an IEventAdapter. You can have multiple event adapters declared but they're supposed to be used for discrete events. I might be wrong about that though - let me take a look at the code that we have today.

@briansain
Copy link
Contributor Author

I updated my unit test to match our use case.

We have 3 main aggregates, and each aggregate has a handful of events that will need to be upcast when we read from the journal. In our use case, because we're using Akkatecture all of our events are coming from the journal as the same type, so we can only have 1 adapter binding in the hocon config. Each aggregate needs it's own read adapter, and we currently cannot set any write adapter if we also have multiple read adapters.

@briansain
Copy link
Contributor Author

@Aaronontheweb Is there an update to this issue?

@Aaronontheweb
Copy link
Member

Taking a look at it now - very, very sorry for the delay. Had a lot of stuff going on.

@Aaronontheweb Aaronontheweb added this to the 1.4.14 milestone Dec 29, 2020
@Aaronontheweb
Copy link
Member

Looks good to me - just approved your PR

@briansain
Copy link
Contributor Author

Awesome, thank you!

@Aaronontheweb Aaronontheweb modified the milestones: 1.4.14, 1.4.15 Dec 30, 2020
This was referenced Dec 30, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.15, 1.4.14 Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants