-
Notifications
You must be signed in to change notification settings - Fork 3
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
How to proxy params from request to ApiMethod #20
Comments
I came up with another solution. It is based on the fact that API methods will be part of applications that use one framework and the method can have a small piece of isolated, framework-dependent code. The solution uses the approach with separating the method parameters in two classes - passed by the user and environment provided. So the param definition in the API method file will require a way to distinguish the environment-provided ones:
The framework-dependent part in the API method definition should look something like this:
And the main code can remain framework-agnostic:
In the same time there will be no need for adding a global param-mapping function and each method can extract only environment data-fields that it will actually use. Pros:
Cons:
|
@dodev, looks good to me. |
Good approach. We can consider environment as a source of default params. .setOption('setDefaultParamsFromEnvironment', function (environment) {
return {
path: environment.pathname,
userCookie: environment.cookies.yp
};
}); It means that your don't mark parameters with a special flag and the method use a environment to fill default values for the parameters. Furthermore, we can allow the pass environment as a second parameter for execution on the server side:
In this case |
I think that parameters that can be set by the environment should be marked never the less. They can fall in the category 'optional', but that name can be deceiving for example a user can call a method from the server-side without passing the environment and not setting those params, because they are marked as 'optional'. This is not critical, but can lead to some nasty bugs. |
Yeah, actually you're right. Let's think about parameter name. .addParam({name: 'param', useEnvironment: true}) |
What about |
I think, we can achieve both method portability and powerful mechanism for obtaining some params from a context of an application or a request. Here is my suggestion. Let's add two new features:
If any parameter of the method is declared as an The When |
Solutions presented above look very complex for me but the issues is not a big deal actually. @lemmy- and I talked about this a little today and we agreed that we should always proxy express request as a second parameter to the method. From the server side you can pass express request as well:
If you decide to use express req, make sure that it's not null (an extra check in the api method). Also it makes it possible to define undefined optional parameters if you want to. Full freedom, guys 😈 If you work in the nmaps team, then you love to divide all methods into two groups: the server side and the frontend side. This approach will work, too, if we proxy request to a method. Anyway we need a special optional to mark parameters which will be provided by request (for docpage). thoughts? |
The motivation here is that this is the simplest solution we've come up with so far, it's easy to implement and does exactly what it's supposed to. The downsides are some new usage conventions. |
If an ApiMethod doesn't need environmental objects can I omit the req parameter in the exec method? |
I guess you can ignore it if you don't need it. |
By the way, "guys from nmaps team" have already been using such approach ) |
Proxy express request to a method. fixed #20
Sometimes you need to pass some parameters from the express request. For example, hostname, IP address, or a header.
It's impossible to pass these parameters from a browser; so, you need another way to get access to them. Currently, the express middleware tries to find method params using
req.param(name)
andreq.session
. Thus, you don't have a way to proxy custom request parameters to your api method.I have only one suggestion. Now Api class knows nothing about express and environment. It would be great to save this.
There are several possible solutions of this issue:
1. Add an option to the middleware.
prons:
cons:
2. Mark some parameters as an environment variables.
In a api method declaration add a special flag to the parameter.
It shows a special label in doc page and it could be ignored in server side.
Anyway we have to add a special option to the middleware.
prons:
cons:
3. Pass environment as a third parameter to the method
In the middleware you should build the environment object
prons:
cons:
4. Always proxy express request as a third parameter.
We also can enable this mechanism with a method option:
prons:
cons:
@dodev, @lemmy-, if I forget something to add, let me know.
Guys, we need your opinions.
//cc @mdevils, @razetdinov, @dfilatov
The text was updated successfully, but these errors were encountered: