vlingo / xoom-symbio-jdbc

The VLINGO XOOM SYMBIO implementation for JDBC for Reactive storage using Event-Sourcing, Key-Value, and Object storage.
https://vlingo.io
Mozilla Public License 2.0
4 stars 12 forks source link

JDBCJournalActor is missing a constructor (..and more) #48

Closed Florian-Schoenherr closed 4 years ago

Florian-Schoenherr commented 4 years ago

There is an assumption that Journals should have a constructor which just takes a dispatcher:

(this is used in schemata:) InMemoryJournalActor takes dispatcher (this is inside the xoom-api:) JDBCJournalActor takes dispatcher Goes here, so dispatcher is in parameters: actorFor(final Class<T> protocol, final Class<? extends Actor> type, final Object...parameters)

The dispatcher ends up inside Definition.parameters and gets send to ActorFactory.actorFor and gets handled like this:

Actor actor = null;
//... some other checks, but with dispatcher it ends up here:
for (final Constructor<?> ctor : definition.type().getConstructors()) {
        if (ctor.getParameterCount() == definition.internalParameters().size()) {
          actor = start(ctor, definition, address, logger);
//...
if (actor == null) {
      throw new IllegalArgumentException("No constructor matches the given number of parameters.");
}

This would only work if every Journal built by DefaultJournalActorBuilder has a ctor with just one parameter like in:

public InMemoryJournalActor(final Dispatcher<Dispatchable<Entry<T>,RS>> dispatcher) {
    this.journal = new InMemoryJournal<>(dispatcher, stage().world());
}

..maybe even every StoreActorBuilder implementation needs this, then. I don't know the specifics, maybe this needs to be build another way (change the Definition, so it's not just "parameters", which then just get used to look if the count of parameters is the same, but do it somehow else, more declarative). Or if the Journals need this dispatcher ctor, maybe put something in the Journal interface.

I found this out because of related issue: https://github.com/vlingo/vlingo-xoom-starter/issues/3 They just use this, which doesn't seem wrong to me, please correct me if I'm wrong: simple startup code

refs: ActorFactory InMemoryJournalActor JDBCJournalActor

VaughnVernon commented 4 years ago

@Florian-Schoenherr I think that the original issue was not accurately written. There is not missing ctor. It's that the Starter's generated code for Event Sourcing used a non-existing ctor because the code generator is wrong. This is probably more due to Event Sourcing code gen being tested less than StateStore gen. If @danilo-ambrosio agrees this issue can be closed as a non-issue.

/cc @danilo-ambrosio

Florian-Schoenherr commented 4 years ago

@VaughnVernon but this still wouldn't make sense. This isn't generated: dispatcher as ...parameters parameters in Definition dispatcher ends up in Definition.parameters, so ActorFactory will never find any ctor. Or should there be an instantiator inside Definition by then?

Maybe I just don't get it right now, so yes, if @danilo-ambrosio finds something or some version of this issue should be in xoom instead, this can be closed.

VaughnVernon commented 4 years ago

@Florian-Schoenherr Please discuss and coordinate with @danilo-ambrosio. He told me that the fix could be done rather easily, but he didn't indicate how. I leave it to you two to work out.

Florian-Schoenherr commented 4 years ago

Fixed by https://github.com/vlingo/vlingo-xoom/commit/479d550cd3473dd015f347fae088765a91ad2adb.