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

Refactor to use amphp and event-store v8 #51

Merged
merged 8 commits into from
Jun 9, 2020
Merged

Refactor to use amphp and event-store v8 #51

merged 8 commits into from
Jun 9, 2020

Conversation

prolic
Copy link
Member

@prolic prolic commented Apr 21, 2020

[WIP] knitted with a hot needle - some changes / bugfixes may be required before merge

  • This makes prooph/micro even smaller (only 2 files with less than 200 LOC combined).
  • 100% async using amphp
  • compatible with event-store v8 (async)
  • Support async aggregate handlers

How to use this?

  • extend a class from CommandSpecification, see f.e. UserSpecification in the examples dir
  • add a command map and create dispatcher, see f.e. register_and_change_username.php in the examples dir

Todos:

ping @codeliner @enumag

As the kernel returns an immutable list of all emitted events, you can simply publish them yourself however you like, that's event a dedicated event publisher will not be part of this. Also in most cases you would rely on event store projections to be running anyway.

@@ -20,7 +21,7 @@ final class InMemoryEmailGuard implements UniqueEmailGuard

public function isUnique(string $email): bool
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this return Promise<bool> now?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Comment on lines 29 to 36
function registerUser(callable $stateResolver, RegisterUser $command, UniqueEmailGuard $guard): Promise
{
if ($guard->isUnique($command->email())) {
return [new UserWasRegistered($command->payload())];
return new Success(ImmList(new UserRegistered($command->payload())));
}

return [new UserWasRegisteredWithDuplicateEmail($command->payload())];
return new Success(ImmList(new UserRegisteredWithDuplicateEmail($command->payload())));
}
Copy link
Member

@enumag enumag Apr 26, 2020

Choose a reason for hiding this comment

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

I like that these functions are now async and as such can use async guards. But I'm slightly doubtful about it resolving to an ImmList of event. In my project I let them return an asynchronous Iterator instead. What is your thought about that?

Note: Code examples assume that UniqueEmailGuard returns a Promise. Also I highly prefer to use the convention of handling error cases first with early return, leaving the happy scenario as main branch so I negated the condition.

function registerUser(callable $stateResolver, RegisterUser $command, UniqueEmailGuard $guard): Iterator
{
    return new Producer(function (callable $emit) use ($command, $guard): Generator {
        if (! yield $guard->isUnique($command->email())) {
             yield $emit(new UserRegisteredWithDuplicateEmail($command->payload()));
             return;
        }
        
        yield $emit(new UserRegistered($command->payload()));
    });
}

Admittedly while pure code should return Amp\Iterator here, I later decided that I don't like the Producer + $emit syntax so I actually let these functions return Generator<Promise|EventInterface>. Then there is a wrapper function that transforms it into \Amp\Iterator<EventInterface>. I consider such syntactic improvement as acceptable over code purity for my project although not sure if it's suitable for an example like this... But I'll show it to you anyway for comparison:

function registerUser(callable $stateResolver, RegisterUser $command, UniqueEmailGuard $guard): Generator
{
    if (! yield $guard->isUnique($command->email())) {
         yield new UserRegisteredWithDuplicateEmail($command->payload());
         return;
    }

    yield new UserRegistered($command->payload());
}

The wrapper function looks something like this:

function transform(Generator $generator): Iterator
{
    return new Producer(function (callable $emit) use ($generator) {
        foreach ($generator as $eventOrPromise) {
            if ($eventOrPromise instanceof Promise) {
                yield $eventOrPromise;
            } else {
                yield $emit($eventOrPromise);
            }
        }
    });
}

Copy link
Member Author

Choose a reason for hiding this comment

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

awesome, thanks !!

Copy link
Member Author

Choose a reason for hiding this comment

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

@enumag I ran into issues with this. On yield $eventOrPromise; the promises is yielded, but not assigned to the variable in the handler function.
$state = yield $stateResolver(); results in $state = null.

Copy link
Member

@enumag enumag Jun 8, 2020

Choose a reason for hiding this comment

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

Ah yeah... sorry, I copy pasted a wrong function. I'll post the correct one tomorrow.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe can you directly submit a PR to this branch?

@prolic
Copy link
Member Author

prolic commented Apr 26, 2020

@enumag changes done

@codeliner
Copy link
Member

@prolic the implementation looks really great. Hopefully I find some time to try it out. I'd also look at a way to connect Event Engine Cockpit to it.

src/Kernel.php Outdated
Comment on lines 50 to 56
foreach ($specification->handle(stateResolver($eventStore, $specification, $readBatchSize)) as $eventOrPromise) {
if ($eventOrPromise instanceof Promise) {
yield $eventOrPromise;
} else {
yield $emit($eventOrPromise);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
foreach ($specification->handle(stateResolver($eventStore, $specification, $readBatchSize)) as $eventOrPromise) {
if ($eventOrPromise instanceof Promise) {
yield $eventOrPromise;
} else {
yield $emit($eventOrPromise);
}
}
$generator = $specification->handle(stateResolver($eventStore, $specification, $readBatchSize));
while ($generator->valid()) {
$eventOrPromise = $generator->current();
if ($eventOrPromise instanceof Promise) {
$generator->send(yield $eventOrPromise);
} else {
yield $emit($eventOrPromise);
$generator->next();
}
}

Copy link
Member

Choose a reason for hiding this comment

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

@prolic This should work I think. ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome, thank you.

@prolic prolic changed the title [WIP] refactor to use amphp and event-store v8 Refactor to use amphp and event-store v8 Jun 9, 2020
@prolic
Copy link
Member Author

prolic commented Jun 9, 2020

I added new issues for snapshot support and documentation. I'll merge this to master.

@prolic prolic merged commit e99c964 into master Jun 9, 2020
@prolic prolic deleted the amphp branch June 9, 2020 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants