Saving context in application is an anti-pattern

[geekdada]

I found on some occasions, the Model being instantiated with the last request context, then I found this:

https://github.com/Azard/egg-oauth2-server/blob/2d10b76142080b60e8fc3e0742cea25e5788111a/lib/server.js#L86

You should never save ctx in application ever. This is my fix:

{
  getServer(ctx) {
    if (!ctx[SERVER]) {
      const { config, model: Model } = this;
      const model = new Model(ctx);
      ctx[SERVER] = new AuthServer(Object.assign(config, { model }));
      return ctx[SERVER];
    }
    return ctx[SERVER];
  }
}

[geekdada]

Or probably, use app/extend/context.js to get AuthServer attached on ctx.

[Azard]

ctx[SERVER] = new AuthServer(Object.assign(config, { model })); could create AuthServer every time, it's not performance.

[geekdada]

I'm a little confused, then what's this for?

https://github.com/Azard/egg-oauth2-server/blob/2d10b76142080b60e8fc3e0742cea25e5788111a/lib/server.js#L77

If creating a new instance every time when a request comes is a bad approach, maybe you shouldn't have made Model has ctx as parameters in the first place.

[geekdada]

I found this PR https://github.com/oauthjs/node-oauth2-server/pull/462

[rise0chen]

Has the problem been solved?