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

Docs: How to receive context in controller #1881

Open
5 tasks
diegolacarta opened this issue Oct 19, 2018 · 14 comments
Open
5 tasks

Docs: How to receive context in controller #1881

diegolacarta opened this issue Oct 19, 2018 · 14 comments
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Docs good first issue help wanted

Comments

@diegolacarta
Copy link

diegolacarta commented Oct 19, 2018

Is there any way to receive the context inside a controller?

Goal is to access request data (headers, ip, etc.)

Thanks

Acceptance criteria

  • a section in Controllers.html explaining how to access request and response headers or body by using decorators like @inject(RestBindings.Http.REQUEST) public request: Request for different cases:
    • A request header shared by most (or all) endpoints and processed by an express middleware/LB4 sequence action. Example: Authorization or X-API-Key.
    • A request header shared by many endpoints but used internally in the endpoint implementation (an express route handler/LB4 controller method). Maybe come up with a somewhat realistic example of such header.
    • A response header set by an express middleware/LB4 sequence action. For example X-Rate-Limit information provided by a rate-limiting extension in all responses.
    • A response header set by the endpoint (an express route handler/LB4 controller method). An example: Location header included ind 201 Created responses.
@hacksparrow
Copy link
Contributor

@diegolacarta in a controller method you can access this.req.

@bajtos
Copy link
Member

bajtos commented Oct 19, 2018

Goal is to access request data (headers, ip, etc.)

Ideally, you should decouple the code building request data from your controllers to enable you to test the controllers in isolation (unit-test style).

Typically, request headers are mapped to regular method parameters, OpenAPI spec supports in: header source.

I think it makes sense to expose the client IP via a custom binding that will allow you to receive the IP address in your controller methods via dependency injection (e.g. @inject(RestBindings.CLIENT_IP)). This is an addition to be implemented in @loopback/rest, would you like to contribute it yourself? We can help you along the way.

Is there any way to receive the context inside a controller?

Since you are interested in the Request object only, you can configure your controller to receive the request as follows:

class MyController {
  constructor(
    @inject(RestBindings.Http.REQUEST) public request: Request,
    // ...
  ) {}

  @get('/foo')
  foo() {
    console.log(this.request.headers);
  }
}

Accessing the context directly should be the last resort solution. You can receive the current context using @inject.context().

class MyController {
  constructor(
    @inject.context() public context: RequestContext,
    // ...
  ) {}

  @get('/foo')
  foo() {
    console.log(this.context.request.headers);
  }
}

@bajtos
Copy link
Member

bajtos commented Oct 22, 2018

@diegolacarta since you closed the issue, I assume my answer provided a solution for the problem you are facing. It makes me wonder how can we improve our documentation so that future LB4 users can more easily discover the information I have provided in my comment. Do you have any suggestions? I am assuming you have read our docs at https://loopback.io/doc/en/lb4/index.html but didn't find the answer. Where were you looking (which pages did you read)? Where would it makes most sense to document how to access the request from a controller method?

@diegolacarta
Copy link
Author

diegolacarta commented Oct 22, 2018 via email

@bajtos bajtos added Docs and removed question labels Oct 26, 2018
@bajtos
Copy link
Member

bajtos commented Oct 26, 2018

I am reopening this issue. Let's improve our docs!

@diegolacarta

I'm struggling a bit with the documentation for v4, particularly since I'm
migrating a relatively complex express app into it. Things like accessing
headers, setting/clearing cookies, uploads, authentication, is not easy to
find (and I believe some of it doesn't exist yet).

I would expect to find your answer in here:
https://loopback.io/doc/en/lb4/Controllers.html but there's no mention to
how to access request/response from there.

Makes sense. Would a migration guide from Express to LB4 be helpful to you?

Could you please help me to define specific use cases to describe in our docs (e.g. Controllers.html), based on what you need in your app?

accessing headers

  • A request header shared by most (or all) endpoints and processed by an express middleware/LB4 sequence action. Example: Authorization or X-API-Key.

  • A request header shared by many endpoints but used internally in the endpoint implementation (an express route handler/LB4 controller method). Can we come up with a somewhat realistic example of such header?

  • A response header set by an express middleware/LB4 sequence action. For example X-Rate-Limit information provided by a rate-limiting extension in all responses.

  • A response header set by the endpoint (an express route handler/LB4 controller method). An example: Location header included ind 201 Created responses.

setting/clearing cookies

We haven't considered cookies yet. We have #1863 requesting support for sessions. What are looking for - HTTP sessions or generic support for cookies?

uploads

We have discussing file uploads here: #1863

authentication

Authentication and authorization is in our backlog now. Could you please be more specific about your auth/auth requirements? Do you store user credentials locally (e.g. email + password) or rely on OAuth2, OpenID, SAML or another technology? What authorization style do you use - role-based, access-control lists, scopes, something else?

If you look back to the time you were reading our docs, would you find it helpful if the Controller docs was clearly spelling out which HTTP features are not implemented yet (cookies, file uploads), with pointers to the relevant GitHub issues?

@bajtos bajtos added developer-experience Issues affecting ease of use and overall experience of LB users TOB labels Oct 26, 2018
@diegolacarta diegolacarta reopened this Oct 29, 2018
@shadyanwar
Copy link

shadyanwar commented Jan 17, 2019

I got here searching for a way to get the user IP. I hope @inject(RestBindings.CLIENT_IP) gets implemented soon. I can give a try to implement it in the coming few days. Any pointers?

I find "RestBindings" helpful in many things I could need in the future.

By the way, the following:

@inject(RestBindings.Http.REQUEST) public request: Request,

Returned the error:
Cannot start the application. ReferenceError: Request is not defined

Note: this error was not caught by TypeScript. It was returned only after npm start referencing the .js file not .ts.

Had to go with:

@inject(RestBindings.Http.REQUEST) public request: any,

I am using PassportJS Basic Strategy based on the lb4 authentication example.

Another note is that I tried to get the request or the context inside an authentication provider file, but the request body was always not present in the request. Everything else was there (headers, connection, socket, ServerResponse,..) but not the body. For that reason, I moved my logic to a controller rather than the provider.

I don't want the method to be called first. I just want to include the body the same way the header is included in the req. Like the req is sent with the header, so why not the body too?

I have a question:

Ideally, you should decouple the code building request data from your controllers to enable you to test the controllers in isolation (unit-test style).

@bajtos What do you mean? If I am checking for the existence of something in the request body for example, I should not do this in the controller?

@bajtos
Copy link
Member

bajtos commented Feb 8, 2019

@diegolacarta sorry for a late response, I have too many GitHub notifications.

I got here searching for a way to get the user IP. I hope @inject(RestBindings.CLIENT_IP) gets implemented soon. I can give a try to implement it in the coming few days. Any pointers?

Here is the place where we are creating request-specific bindings:

https://github.com/strongloop/loopback-next/blob/0e1b06f546aea97855556f985b39e50cd3e7956e/packages/rest/src/request-context.ts#L25-L37

By the way, the following:

@inject(RestBindings.Http.REQUEST) public request: Request,

Returned the error:
Cannot start the application. ReferenceError: Request is not defined

I suspect this is a bug in TypeScript typings. One of the libraries we use in LoopBack is incorrectly assuming browser environment. To make TypeScript happy, we need to include dom library in tsconfig. As a result, Request and Response become globally defined types. Unfortunately they are browser types that don't work in Node.js! You need to import Request from @loopback/rest.

Another note is that I tried to get the request or the context inside an authentication provider file, but the request body was always not present in the request. Everything else was there (headers, connection, socket, ServerResponse,..) but not the body. For that reason, I moved my logic to a controller rather than the provider.

I don't want the method to be called first. I just want to include the body the same way the header is included in the req. Like the req is sent with the header, so why not the body too?

I am not entirely sure what is the cause of the behavior you are seeing. I think this may be caused by the fact that body must be parsed asynchronously and this happens only when parseParams sequence action is called. Providers are invoked before the sequence started, at the time when the request body was not parsed yet.

Can you try to use @inject.getter for the request body please?

Ideally, you should decouple the code building request data from your controllers to enable you to test the controllers in isolation (unit-test style).

What do you mean? If I am checking for the existence of something in the request body for example, I should not do this in the controller?

Your controller should describe the request body via OpenAPI spec and then receive the parsed (and validated!) body object as a parameter.

See our Todo example:

https://github.com/sanadHaj/loopback-next/blob/2088fc1b3c64f9d8c3e10798d71a660c426efb7c/examples/todo-list/src/controllers/todo.controller.ts#L23-L33

That place does not provide any schema in @requestBody decorator because LB4 is able to infer the schema from model definition.

  @post('/todos', {
    responses: {
      '200': {
        description: 'Todo model instance',
        content: {'application/json': {schema: {'x-ts-type': Todo}}},
      },
    },
  })
  async createTodo(@requestBody() todo: Todo) {
    return await this.todoRepo.create(todo);
  }

The decorator @requestBody allows you to provide your custom parameter spec if you don't want to use one inferred from a model (e.g. because there is no model for your parameter):

const UserSchema = {
  // OpeAPI/JSON schema describing the payload
};

class MyController {
  @put('/Users/{id}')
  async replaceUser(
    @param.path.string('id') id: string,
    @requestBody({
      content: {
        'application/json': UserSchema,
      },
    })
    user: object,
  ) {
    // the implementation
  }
}

Further reading:

@raymondfeng
Copy link
Contributor

@diegolacarta Please make sure you have the following statement:

import {Request} from '@loopback/rest'

In VSCode, Request is also recognized as a global. As a result, it won't prompt you to import it from the correct module.

@dhmlau dhmlau added 2019Q2 and removed TOB labels Feb 8, 2019
@bajtos
Copy link
Member

bajtos commented Mar 21, 2019

In VSCode, Request is also recognized as a global. As a result, it won't prompt you to import it from the correct module.

This problem is caused by a problem in type definitions of supertest, where browser DOM objects are required even when running in Node.js :(

If you are not using @loopback/testlab, then you can try to remove dom from the list of libraries used by your tsconfig.

See https://github.com/strongloop/loopback-next/blob/78a2e79231d3bfd354298a36c501ae297f4674e2/packages/build/config/tsconfig.common.json#L11

@bajtos bajtos changed the title Receive context in controller Docs: How to receive context in controller Mar 21, 2019
@bajtos
Copy link
Member

bajtos commented Mar 21, 2019

In VSCode, Request is also recognized as a global.
This problem is caused by a problem in type definitions of supertest, where browser DOM objects are required even when running in Node.js :(

I found a neat solution how to fix this problem, see #2621

@auxcoder
Copy link

auxcoder commented Sep 20, 2019

You can receive the current context using @inject.context().

How to receive context in unit testing controllers?

import {AuthController} from '../../controllers/auth.controller';
import {UserRepository} from '../../repositories';
import {testdb} from '../../datasources/test.datasource';

export async function authController(): Promise<AuthController> {
  const userRepository = new UserRepository(testdb);
  return new AuthController(
    userRepository,
    // here should be context to access headers, etc
  );
}

@raymondfeng
Copy link
Contributor

You can pass in context via the constructor for unit testing. The other option is to bind the controller to context and retrieve an instance via ctx.get(). For example:

const ctx = new Context();
ctx.bind('controllers.MyController').toClass(MyController);
const controller = await ctx.get('controllers.MyController');

@bajtos
Copy link
Member

bajtos commented Sep 24, 2019

@auxcoder As I explained in #1881 (comment), you shouldn't be injecting the entire context, that's an anti-pattern making unit tests difficult to write.

If your controller needs to access request headers, then you can inject the current request object only. Or even better, define bindings for the individual headers you are interested in.

@raymondfeng
Copy link
Contributor

Reopening the issue to make sure docs are improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Docs good first issue help wanted
Projects
None yet
Development

No branches or pull requests

8 participants