Feat: allow path parameters on controllers

[vincentvanderweele]

All Submissions:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Have you written unit tests?
• Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
• This PR is associated with an existing issue?

Closing issues

Close #261

If this is a new feature submission:

• Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the goals and philosophy of the project?

Potential Problems With The Approach

This is quite a simple solution. I am not sure if passing down the controller path this way is the correct way to solve it, or if it would be better to get the parent information from within the MethodGenerator. I created this as a starting point for discussion; I'd happily update the approach if preferred.

In the related issue #318 Luke Autry wrote

The tricky thing would be verifying that each method within the controller properly accepts the path parameter (e.g. {companyId}).

I don't think that is implemented on method level yet either, so I did not add support for that here. Since the full path is in scope when handling the @Path decorator, I don't think this solution makes that validation harder, if someone would want to add that later.

Test plan

I verified that the correct parameters are generated and that both controller parameters and method parameters can be used in all 3 generated server frameworks.

[vincentvanderweele]

I ran the tests locally and they passed there. Did I forget to register something somewhere?

[WoH]

Looks very similar to the approach I was working on. Would be good to add one more test as outlined.

I ran the tests locally and they passed there. Did I forget to register something somewhere?

I haven't looked at your Path Parameter lookup, but it seems to be failing:

  1) Sub resource route generation
       should generate a path parameter for method without path parameter:
     AssertionError: Path object for /SubResourceTest/{mainResourceId}/SubResource route wasn't generated.: expected undefined to exist
      at Object.VerifyPath (unit/utilities/verifyPath.ts:16:71)
      at getValidatedParameters (unit/swagger/pathGeneration/subResourceRoutes.spec.ts:14:18)
      at Context.<anonymous> (unit/swagger/pathGeneration/subResourceRoutes.spec.ts:26:24)
      at processImmediate (internal/timers.js:461:21)

  2) Sub resource route generation
       should generate two path parameters for method with path parameter:
     AssertionError: Path object for /SubResourceTest/{mainResourceId}/SubResource/{subResourceId} route wasn't generated.: expected undefined to exist
      at Object.VerifyPath (unit/utilities/verifyPath.ts:16:71)
      at getValidatedParameters (unit/swagger/pathGeneration/subResourceRoutes.spec.ts:14:18)
      at Context.<anonymous> (unit/swagger/pathGeneration/subResourceRoutes.spec.ts:31:24)
      at processImmediate (internal/timers.js:461:21)

E: You can use the current file mocha debug config to check, but I think these are likely reproducible locally. Maybe run yarn run build && yarn run pretest to make sure.

[WoH]

Copy this to the base hapi and koa server.

[vincentvanderweele]

I'm not sure if I correctly understand this comment. Do you mean I should copy-paste the exact same code to the other two tests (hapi-server and koa-server)? Or is there a shared place to copy this to so I don't need to copy-paste the same test three times? I did the former but if you meant the latter I might need a bit more guidance.

[vincentvanderweele]

It seems that my unit tests are still failing although they pass on my machine. I'll have a closer look tomorrow what's going on there.