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

[inversify-express-utils] allow to inject dependencies into middleware constructor #368

Open
b1ff opened this issue May 27, 2020 · 2 comments · Fixed by namecheap/express-inversify#2

Comments

@b1ff
Copy link

b1ff commented May 27, 2020

Expected Behavior

Middlewares should be created by http request scoped container.
Since by default they are not singleton it does make sense to create them using per-request container, which allows to inject request-scoped dependencies in constructor instead calling this.httpContext.container.get(...). For middlewares that are registered as singleton nothing will be changed, so it should add any performance difference.

Current Behavior

Right now middlewares in express-utils are always created using root container which does not allow to inject request scoped variables in constructor.

Possible Solution

PR: https://github.com/inversify/inversify-express-utils/pull/325/files

Steps to Reproduce (for bugs)

    it("Should allow constructor injections from http-scope in middlewares", async () => {

        const TYPES = {
            Value: Symbol.for("Value"),
            ReadValue: Symbol.for("ReadValue"),
            HttpContextValueSetMiddleware: Symbol.for("HttpContextValueSetMiddleware"),
            HttpContextValueReadMiddleware: Symbol.for("HttpContextValueReadMiddleware"),
        };

        class HttpContextValueSetMiddleware extends BaseMiddleware {
            public handler(
                req: express.Request,
                res: express.Response,
                next: express.NextFunction
            ) {
                this.bind<string>(TYPES.Value).toConstantValue(`MyValue`);
                next();
            }
        }

        class HttpContextValueReadMiddleware extends BaseMiddleware {
            constructor(@inject(TYPES.Value) private value: string) {
                super();
            }

            public handler(
                req: express.Request,
                res: express.Response,
                next: express.NextFunction
            ) {
                this.bind(TYPES.ReadValue).toConstantValue(`${this.value} is read`);
                next();
            }
        }

        @controller("/http-scope-middleware-injection-test")
        class MiddlewareInjectionTestController extends BaseHttpController {

            constructor(@inject(TYPES.ReadValue) @optional() private value: string) {
                super();
            }

            @httpGet(
                "/get-value",
                TYPES.HttpContextValueSetMiddleware,
                TYPES.HttpContextValueReadMiddleware
            )
            public getValue() {
                return this.value;
            }
        }

        const container = new Container();

        container.bind<HttpContextValueReadMiddleware>(TYPES.HttpContextValueReadMiddleware)
            .to(HttpContextValueReadMiddleware);
        container.bind<HttpContextValueSetMiddleware>(TYPES.HttpContextValueSetMiddleware)
            .to(HttpContextValueSetMiddleware);
        container.bind<string>(TYPES.Value).toConstantValue("DefaultValue");
        const app = new InversifyExpressServer(container).build();

        await supertest(app)
            .get("/http-scope-middleware-injection-test/get-value")
            .expect(200, "MyValue is read");
    });

Context

In application I would like to register some values in middlewares at the beginning of the request and then use classes that abstract HTTP specific things for the consumers. Allowing to inject dependencies into middleware constructor simplifies unit testing of those middlewares.

Your Environment

NodeJS 12.x.x
Windows

@b1ff b1ff closed this as completed Nov 18, 2021
@PodaruDragos
Copy link
Member

hey @b1ff .

I saw that you've made a PR for this. but it's already possible to do this in inversify-express-utils. Let me give you an example.

@injectable()
export class AuthorizationMiddleware extends BaseMiddleware {
  @inject(TYPES.USER_REPOSITORY)
  private readonly userRepository!: EntityRepository<User>;

  constructor(
    @inject(TYPES.USER_REPOSITORY)
    private readonly userRepositoryFromConstructor: EntityRepository<User>
  ) {
    super();
  }

  public handler(_req: Request, _res: Response, next: NextFunction): void {
    console.log(this.userRepository);
    console.log(this.userRepositoryFromConstructor);
    next();
  }
}

And then for the container binding

container.bind(AuthorizationMiddleware).toSelf();

Is this what you were looking for ?

@PodaruDragos PodaruDragos reopened this Nov 18, 2021
@b1ff
Copy link
Author

b1ff commented Nov 18, 2021

@PodaruDragos hey there, not really. The issue is when I do

this.bind(..).to...();

in middleware, I cannot inject it using constructor to middleware. I have to use httpContext.containter.get(). While it works as workaround, it is better from testability and cleanness to inject it via middleware constructor, even if it was bound to container on httpContext.
You might see that behavior in test that I have added.

@PodaruDragos PodaruDragos transferred this issue from inversify/InversifyJS Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants