WIP: Implementation proposition by mateusz-kolecki · Pull Request #1 · snapshotpl/commander

mateusz-kolecki · 2018-05-15T21:48:20Z

Hi!
I would like to help create Command Bus library for TypeScript. Any feedback will be appreciated.

snapshotpl · 2018-05-18T09:59:28Z

src/commandBus.ts

+}
+
+export interface CommandBus {
+    handle(command: Command): Promise<any>


snapshotpl · 2018-05-18T11:04:43Z

src/middlewares/commandLogger.ts

+    }
+
+    public async run(command: Command, next: NextMiddleware): Promise<any> {
+        const start = Date.now();


Move before call next()

snapshotpl · 2018-05-18T11:05:16Z

src/middlewares/commandLogger.ts

+            throw err;
+        } finally {
+            const time = Date.now() - start;
+            this.logger.log(this.level, 'Command %s time %dms', commandName, time);


Do you want to log command two times if error occured?

snapshotpl · 2018-05-18T11:19:18Z

src/middlewares/index.ts

+
+import { Command } from '../commandBus';
+
+export type NextMiddleware = () => Promise<any>;


Next middleware should receive command as arg

Hmm... this was something I was thinking about for a long time...

My conclusion was this: if next() callback would receive command as argument then for most of the time it would be the same command in all meddlewares across the chain invocation. In other words it would be the same command as this passed to run() method.
Now i pass command to next middleware in the background (see for execution chain reducer);

Ok, but maby middleware can change command? Maby middleware creator would create his own command instance and pass this new one to next()?
In my opinion sneaky...

What do You think about this?

ok, i added command as argument

snapshotpl · 2018-05-18T11:21:30Z

src/resolvers/memory.ts

+        const handler = this.handlers.get(command.constructor as ConstructorOf<Command>);
+
+        if (!handler) {
+            throw new Error(`Could not resolve handler for ${command.constructor.name}`);


imo this exception should come from get()

Can you explain in more details?

snapshotpl · 2018-05-18T11:36:41Z

src/resolvers/memory.ts

+        return this.handlers.size;
+    }
+
+    public entries(): IterableIterator<[ConstructorOf<Command>, Handler<Command>]> {


Are we need this?

snapshotpl · 2018-05-18T11:37:38Z

src/resolvers/memory.ts

+export class HandlerResolverInMemory implements HandlerResolver {
+    private handlers = new Map<ConstructorOf<Command>, Handler<Command>>();
+
+    public register<T>(commandConstructor: ConstructorOf<T>, handler: Handler<T>): HandlerResolverInMemory {


Why not to get this list by constructor?

I wanted to be sure that each entry in Map is a command constructor as key and handler instance which handles exacly this particular command as value.
Now it can be checked at compile time for register() method calls.
Have no idea, how to declare handlers property to get same result.
Now propert is declared as any command constructor as value and any handler instane which handles any command.

In other words each call to register() method is made with some generic type which is inferred from command constructor type and this allow compiler to check that handler instance matches that command type.

This prevent mistakes like registering handler for another command.

snapshotpl · 2018-05-18T12:01:30Z

test/component/commander.test.ts

+    it("should write info logs", async () => {
+        await commander.handle(new HelloCommand('John', 'Doe'));
+
+        const infos = logger.messages.filter(entry => entry.level === 'info');


Are we want to test logger middeware here?

snapshotpl · 2018-05-18T12:02:31Z

test/fakes/fakeLogger.ts

+
+export class FakeLogger implements CommandLogger {
+
+    public messages: { level: string, msg: string, meta: any }[] = [];


if public, can be changed out of scope

This is also dump implementation for testing. I can use read-only getter like this:

class FakeLogger implements CommandLogger { // ... public get Messages() { return this.messages; } }

and then in test code:

const infos = logger.Messages.filter(...);

What do you think about this?

snapshotpl · 2018-05-18T12:03:21Z

test/unit/commander.test.ts

+import { Middleware, NextMiddleware } from '../../src/middlewares';
+
+
+class CallbackMiddleware implements Middleware {


Can exist as provided implementation

TypeScript do not offer final classes :(

And this class is only for testing.

mateusz-kolecki added 2 commits May 15, 2018 23:55

First working implementation with tests

a3a31d5

Add .travis.yml

44ca126

mateusz-kolecki force-pushed the implementation-proposition branch from b6f8d9b to 44ca126 Compare May 15, 2018 21:56

mateusz-kolecki added 2 commits May 16, 2018 00:05

Customize .travis.yml

fd2459f

Add more component tests

0412086

snapshotpl reviewed May 18, 2018

View reviewed changes

mateusz-kolecki added 5 commits May 19, 2018 23:00

Change messages to be private in FakeLogger and add getter

ac5fe7a

Make command handler to return Promise<void>

60d95df

Log command end only once on error

f5830d3

Make next() function to take command as argument

af6642b

Add copyright info

18f0bfd


		import { Command } from '../commandBus';

		export type NextMiddleware = () => Promise<any>;


		export class FakeLogger implements CommandLogger {

		public messages: { level: string, msg: string, meta: any }[] = [];

		import { Middleware, NextMiddleware } from '../../src/middlewares';


		class CallbackMiddleware implements Middleware {

Conversation

mateusz-kolecki commented May 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateusz-kolecki May 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateusz-kolecki May 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mateusz-kolecki commented May 15, 2018 •

edited

Loading

mateusz-kolecki May 18, 2018 •

edited

Loading

mateusz-kolecki May 18, 2018 •

edited

Loading