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

This is my suggestion on how to solve the event conversion. #76

Merged
merged 14 commits into from Oct 1, 2011

Conversation

mikeeast
Copy link

To use it, simply implement the IConvertEvents interface and hookup the EventConverter when Wiring up the store.

::m

@mikeeast
Copy link
Author

Thanks for noticing!

I will add support for getting explicitly implemented methods as well.

I'll submit a new Pull request in about 7 hours or so. Or something.

@joliver
Copy link
Contributor

joliver commented Sep 28, 2011

You'll probably want have the Convert function use a while loop internally because it's possible that you want to convert eventA to eventB and then eventB to eventC. It would also be really nice to have a constructor overloads so that those who wanted to use explicit registration instead of reflection-based wireup could do so (as per the project goals found in the readme). Lastly, I noticed the IConvertEvents interface was .NET 4.0-specific. I'm still supporting .NET 3.5 for this release (which should be the last one). Any particular reason for the in/out covariant/contravariant parameters on the interface?

@mikeeast
Copy link
Author

The looping thing is a great idea. I will implement it.

I was thinking about the explicit registration thing this morning and it should indeed be allowed. Assembly scanning is not a very nice way to do it. Also, I am thinking about if it should make it into its own thing in the Wireup but I better leave the decision about that to you.

The in/out covariant/contravariant thing is just a alt-enter Resharper reflex consequence. My brain had no part in that at all. It has no function whatsoever and I'll remove it.

@joliver
Copy link
Contributor

joliver commented Sep 28, 2011

The only other thought I had was about the location of the conversion. In the back of my mind, I always pictured conversion as something a serialization wrapper would take care of. In other words, I've been thinking we could accomplish the same thing from the serializer level. I'm still back and forth on this one though and a pipeline hook is definitely a place where it could work, but at that point, the ordering of the pipeline hooks may become important.

@mikeeast
Copy link
Author

You have a point. The sooner the better.

Still, the interface IConvertEvents<,> is pretty much what we want, right?

@mikeeast
Copy link
Author

How about promoting the conversion to its own thing?

The default behavior is no conversion.

Using Wireup,

you can choose something like .UsingEventConversion().AutoScan() for automatic scanning of converters, like I have done now.

You can also choose something like .UsingEventConversion().WithConverters.FromAssemblyContainingType(typeof(MyConverter)). Here I think we can improve the API, but you get the idea.

Then for the implementation, the conversion will not be a IPipelineHook but occur just before the pipeline hooks executes, in OptimisticEventStore.GetFrom(..). Line #65 or so. OptimisticEventStore will get the converters injected.

I think that might be the best place. Regardless, this way the functionality is kept internal so that compability is kept even though the serializer wrapper idea of yours materializes and gets implemented.

@mikeeast
Copy link
Author

Here is another suggestion. Forgive my commit frenzy. I evidently suck at git.

In this commit, the Event conversion has become its own thing and is configurable from the Wireup. I don't know what you think about me poking around in the API, but as this is a suggestion I took the liberty in doing so.

Have a look and tell me what you think.

@joliver
Copy link
Contributor

joliver commented Sep 29, 2011

Okay, here's what I'd like to do. I think making the IConvertCommits is a great interface and it should definitely have it's own interface like you have done. After looking at the issues and different storage engines, I determined that wrapping it up in the serializer is absolutely the wrong place, so it's good we didn't go down that road. However, I believe that wrapping things up in a "pipeline hook" is the right place. I'm trying to keep the primary EventStore objects as clean and uncluttered as possible which is one of the reasons I took the dispatcher and put it into a pipeline hook.

One other big one is that I'd like to move all reflection and type/assembly scanning into the wireup and out of the constructor. Ideally the converter which just be using a Dictionary<Type, Func<TIn, TOut>> and however those were discovered or registered is a wireup issue.

@mikeeast
Copy link
Author

Ok, that sounds great!

I'll turn it back into a IPipelineHook and try to figure out a useful API for the Wireup. I'll also move all scanning things to the wireup. I might commit something tonight.

@mikeeast
Copy link
Author

Here we go! This version should do it!

The default behaviour now is that there are no event conversion. To enable it, you have to explicitly use the Wireup. There are three ways to do it.
The simplest way is to use .UsingEventUpconversion() which makes the Wireup scan all assemblies for converters.
The second alternative is to use .WithConvertersFrom(params Assembly[]) which only scans the supplied assemblies.
The final alternative is to use .WithConvertersFromAssemblyContaining(params Type[]) which scans all assemblies containing the supplied types.

The event converter pipeline hook will use the converters found from scanning and convert events when they are retrieved from the underlying persistence. This will take place before any other pipeline hook registered executes.

The event converter convert specific events recursively which means that an event of type Event1 will be converted to Event2 and then to Event3 and so forth. This recursion will continue until there is no converter found.

Event converters are created by implementing the IConvertEvents<TSource, TTarget> interface. The event converter chooses which converter to use based on TSource.

Feel free to trash the Wireup API as you see fit.

Let me know what you think.

@joliver joliver merged commit 757026a into NEventStore:master Oct 1, 2011
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.

None yet

3 participants