resolving controller-interface type

[osher]

Following the discussion in:
#58

This PR is NOT READY YET, I just wanted you to have an early look and get feedback from you and from the builder.

The code implementation is more or less there
However - I cannot guarantee it before I add tests for all the cases we discussed.

I'll get to it in a few days.

[osher]

Peeweee. Haven't seen stack traces this long ever since the darker days of java... 😛
so ok, I broke the tests. need to run mocha with --recursive...
I added mocha.opts file

[osher]

baaah. more missing opts. ok.

[osher]

no. not missing opts. just a non-standard behavior.
I think i'got'cha now.
You don't want me to use mocha, you require me to use npm test
The way it's written does not work on windows. But I'll give you what you want.

moment

[coveralls]

Coverage decreased (-0.3%) to 96.104% when pulling f99c496 on osher:patch-1 into a44711b on theganyo:master.

[osher]

Ok. Build passes

BUT

The PR is not ready, I still have to add more tests.

I just would appreciate if you take a look and say a word

[coveralls]

Coverage increased (+0.07%) to 96.434% when pulling 94e3ef6 on osher:patch-1 into a44711b on theganyo:master.

[osher]

Well, I'll be damned. I could not help myself...

I think we're there.
I would like to add one more thing:
~/fittings/swagger_cors

module.exports = require('./cors')

Can I please do it here?

this should allow me to
config/default.yaml

swagger:
  fittingsDirs:             [ node_modules, lib/fittings ]

[theganyo]

Nice! I haven't had time to completely review, but this is looking super!

Re: cors... it's unrelated, should probably be a separate PR...

[osher]

Then I suppose the context.output structure is also a different issue.
Will be glad to hear what you think though.

Anyway - I admire your response times.
I'm off to the weekend. Weekends here are Fri/Sat so I'll be back on Sunday.
[edited]: Do you think you'll have time for it by then?

[theganyo]

Actually, on second thought, I'm ok with addressing them all in the same PR. As long as we can ensure all are backward compatible, I don't anticipate the need to accept piecemeal.

[theganyo]

Oh, and enjoy your weekend! Yes, I'll try to review before then.

[coveralls]

Coverage increased (+0.08%) to 96.44% when pulling 6ec101f on osher:patch-1 into a44711b on theganyo:master.

[coveralls]

Coverage increased (+0.08%) to 96.44% when pulling 6ec101f on osher:patch-1 into a44711b on theganyo:master.

[coveralls]

Coverage increased (+0.08%) to 96.44% when pulling 6ec101f on osher:patch-1 into a44711b on theganyo:master.

[osher]

ok - fittings/swagger_cors - is done.

I wanted to update the docs, but I saw the README is rather empty...
How do you intend to manage docs?
Perhaps I should create a wiki page about x-controller-interface?

[theganyo]

Yes, docs are horrible right now. This project was relying on the swagger project to maintain all the documentation, but that's no longer possible. So, yes, we should start a wiki. Many doc updates have been made only in the release notes until now, but it's not an acceptable way to continue.

[theganyo]

Oops. Forgot to finalize this over the weekend. Thanks for all your work on this!

[osher]

Scott - this is wonderful.
When should I expect it to be published on NPM?

[theganyo]

Did you happen to go ahead and write up any release notes?

[osher]

Where should I provide release notes?
I thought its some github feature only owners can...

Anyway - for my changes the release note may be in the spirit of:

This PR allows you to chose how you want to implement your controllers.
The default is like a middleware: operationMethod(req, res, next)

Alternatively, you can chose to write your controllers as a pipe: operationMethod(ctx, next)
In this case, the controller is passed with the pipeworks context, which have a reference to .request, to .response as seen in lib/connect_middleware, line 55.
The idea in this case is not to use ctx.response, but leave your response body on the ctx.output, the status on ctx.statusCode, and the headers on ctx.headers.
The benefit is improved isolation from web-context. You don't have to run a web server to run your tests. You just pass it a ctx and see what you find on the ctx once the callback is called.

Lastly, you can ask bagpipes to deduct the interface from the controller's method signature.
If it names 2 parameters - it's a pipe, if it names 3 parameters - it's a middleware.

This directive may be provided in few levels that cascade each other.

1. The default for the entire server - provided at config/default.yaml as swagger.bagpipes._router.controllersInterface.
2. If its more comfortable to you to express it in the swagger - you may use in your swagger.yaml the following directive on the root: x-controller-interface.
3. You can cascade the default for an entire path, suggesting that all methods of the controller that manages this path are implemented this way, using x-controller-interface on the path level.
4. lastly - you can cascade the directive per operation, using x-controller-interface on the operation level.
   The legal values are one of the values middleware, pipe, auto-detect, correspond to the behavior described above.

.

BTW - I think there's yet a small miss which I will be happy to fix before 0.7.1
We did not accomplished isolation from the web. Even if the user does not need to use ctx.response - he still needs ctx.request to access the ctx.request.swagger.params.
I think we should feature ctx.params
Edit - added line : I think it probably should be done in fittings/swagger_params_parser

.

Oh, I also see you did not reference your release notes for v0.7.0: 😮

readme.md ends with

https://github.com/theganyo/swagger-node-runner/releases/tag/v0.6.4
https://github.com/theganyo/swagger-node-runner/releases/tag/v0.6.10
https://github.com/theganyo/swagger-node-runner/releases/tag/v0.6.11