Support jsonapi.org

[conrad-vanl]

Is there any way, or plans, to configure how JSON responses (and therefore requests) are serialized? For instance, it would be very nice to support a standard, such as http://jsonapi.org/ so that clients can take advantage of generalized tooling. Looking at the current API, it would appear to me the biggest thing would be for models to be serialized in root objects...

This:

{
   "user": {
      "id": "...",
      "name": "Conrad"
   }
}

As opposed to:

{
   "id": "...",
   "name": "Conrad"
}

There would also be other considerations in how associations and relations are serialized.

[raymondfeng]

BTW, let's refactor https://github.com/strongloop/strong-remoting/blob/master/lib/http-context.js so that it can expose hook/extension point to allow plugin of media type handlers, for example.

remoteObjects.registerMediaType('application/vnd.api+json',  {
  unmarshal: function(ctx, next) {
    // parse the http request
  },
  marshal: function(ctx, next) {
    // build the http response
  }
});

[digitalsadhu]

thanks @raymondfeng that refactor would be very useful as I don't want to have to instruct people to configure the body parser separately from the module i'm writing. remoteObjects.registerMediaType would be excellent. I currently have a working solution here https://github.com/digitalsadhu/loopback-component-jsonapi/blob/master/headers.js#L4-L26 but its not ideal, bodyparser would be better

[bajtos]

I would personally submit a patch to body-parser so that it recognises application/vnd.api+json as JSON by default. The other approaches listed above are fine with me too.

[digitalsadhu]

K, thanks @bajtos I can look at doing this.

[digitalsadhu]

@bajtos looks like we need to support it in loopback or perhaps just in loopback-component-jsonapi. bodyparser don't wish to support it directly. See. https://github.com/expressjs/body-parser/issues/131

[digitalsadhu]

@bajtos @raymondfeng To support JSON API correctly, I need to translate all error responses from:

{ error: {...} }

to

{ errors: [{...}, {...}] }

I've noticed that not all errors go through the remote method afterError hook.

My validation errors are nicely formatted for JSON API as above which is half the battle but other errors such as 404s and some 500s seem to avoid the hook.

Any pointers re: where I might go to try to handle those? Would I need to define a middleware to catch them before the regular error handler?

[digitalsadhu]

@bajtos @raymondfeng Disregard last question. Solved by a combination of a custom error handler middleware to catch some errors and a custom rest api middleware function as described here: https://docs.strongloop.com/display/public/LB/Environment-specific+configuration#Environment-specificconfiguration-CustomizingRESTerrorhandling to catch the rest. In the end I needed to transform errors in 3 different places so im wondering if I could have done a better job of this...

Final hacky implementation here: https://github.com/digitalsadhu/loopback-component-jsonapi/blob/master/errors.js

[bajtos]

looks like we need to support it in loopback or perhaps just in loopback-component-jsonapi. bodyparser don't wish to support it directly. See. expressjs/body-parser#131

IIUC, it should be ok to add another instance of body-parser.json middleware and configure it to parse application/vnd.api+json request bodies. This can be done from a component setup function via app.middleware().

To support JSON API correctly, I need to translate all error responses (...) I've noticed that not all errors go through the remote method afterError hook. (...) In the end I needed to transform errors in 3 different places so im wondering if I could have done a better job of this..

I don't think you can get rid of handling the errors in two places - REST error handler and then generic app error handler.

However, I think you can get rid of remotes.afterError hook and call the error-transformer from the other two places instead.

To simplify your implementation, I think we can add an option to strong-remoting's RestAdapter to let it pass through all errors to the top-level app error handler. That will allow you to simplify your implementation and apply any transformation in the error handler middleware.

Thoughts?

[digitalsadhu]

That would be pretty nice. A single error handler would simplify things. I can at the very least try to refactor out the afterError handler and tidy things up.

On Mon, 12 Oct 2015 at 17:21, Miroslav Bajtoš notifications@github.com wrote:

looks like we need to support it in loopback or perhaps just in loopback-component-jsonapi. bodyparser don't wish to support it directly. See. expressjs/body-parser#131 https://github.com/expressjs/body-parser/issues/131

IIUC, it should be ok to add another instance of body-parser.json middleware and configure it to parse application/vnd.api+json request bodies. This can be done from a component setup function via app.middleware().

To support JSON API correctly, I need to translate all error responses (...) I've noticed that not all errors go through the remote method afterError hook. (...) In the end I needed to transform errors in 3 different places so im wondering if I could have done a better job of this..

I don't think you can get rid of handling the errors in two places - REST error handler and then generic app error handler.

However, I think you can get rid of remotes.afterError hook and call the error-transformer from the other two places instead.

To simplify your implementation, I think we can add an option to strong-remoting's RestAdapter to let it pass through all errors to the top-level app error handler. That will allow you to simplify your implementation and apply any transformation in the error handler middleware.

Thoughts?

— Reply to this email directly or view it on GitHub https://github.com/strongloop/loopback/issues/445#issuecomment-147433394 .

[bajtos]

That would be pretty nice. A single error handler would simplify things.

@digitalsadhu Cool. Would you mind contributing this change yourself? Here is the code to change: lib/rest-adapter.js#L306-L308. You can use the implementation of handleUnknownPaths as an inspiration, it's few lines above.

[listepo]

Hi guys I need help. Why this.definition.relations is empty?

remotes.before('**', function(ctx, next) {
  console.log(this.definition.relations);
  next();
});

[listepo]

When creating a need to get all relations. Get information about the relationship by name. To properly set the data. Also, I do not see a single method that returns the foreign key if it is set or created by default.

[raymondfeng]

You should use modelClass.definition.relations. I assume the model class is part of ctx. The this might be the model class or prototype.

[listepo]

@raymondfeng thanks, how to set plural for all models?

[listepo]

@raymondfeng this.definition.settings.relations

[listepo]

I think you need to fill and other parameters as, property, foreignKey

[digitalsadhu]

@bajtos earlier you mentioned:

Yes, it's not possible to change the list of content types at a boot script level now. The change should be pretty easy, we need to extend loopback-swagger to read consumes/produces from remoting metadata of each method (if it's provided).

I'd be happy to have a crack at submitting the PR for this. Any chance you can give me a bit of guidance?

[digitalsadhu]

@bajtos @raymondfeng @ritch
I'm running into another issue that specifying 'application/vnd.api+json' as the accept type is not handled in strong remoting here: https://github.com/strongloop/strong-remoting/blob/4575e92142aa32ee4a568862f93b394e7d034127/lib/http-context.js#L561-L593 I end up with a 406 Not Acceptable

The only option I can think of at the moment is to detect application/vnd.api+json and rewrite it to json or application/json

Any chance I can submit a PR to add support for application/vnd.api+json?

[ritch]

Any chance I can submit a PR to add support for application/vnd.api+json?

Sure. You should be able to add it as a case.

[digitalsadhu]

@ritch awesome. Will do.

[digitalsadhu]

@ritch PR submitted. https://github.com/strongloop/strong-remoting/pull/249

[bajtos]

@digitalsadhu

Yes, it's not possible to change the list of content types at a boot script level now. The change should be pretty easy, we need to extend loopback-swagger to read consumes/produces from remoting metadata of each method (if it's provided).

I'd be happy to have a crack at submitting the PR for this. Any chance you can give me a bit of guidance?

Awesome! You can start looking here: https://github.com/strongloop/loopback-swagger/blob/b6e39252640732e31c26d6831bf86c5b6593aff7/lib/specgen/route-helper.js#L161-L162. The default list of produces/consumes types is initialised here and then used here.

[digitalsadhu]

Nice! On my list!

On Thu, 5 Nov 2015 at 17:02, Miroslav Bajtoš notifications@github.com wrote:

@digitalsadhu https://github.com/digitalsadhu

Yes, it's not possible to change the list of content types at a boot script level now. The change should be pretty easy, we need to extend loopback-swagger to read consumes/produces from remoting metadata of each method (if it's provided).

I'd be happy to have a crack at submitting the PR for this. Any chance you can give me a bit of guidance?

Awesome! You can start looking here: https://github.com/strongloop/loopback-swagger/blob/b6e39252640732e31c26d6831bf86c5b6593aff7/lib/specgen/route-helper.js#L161-L162. The default list of produces/consumes types is initialised here https://github.com/strongloop/loopback-swagger/blob/b6e39252640732e31c26d6831bf86c5b6593aff7/lib/specgen/swagger-spec-generator.js#L25-L35 and then used [here]( https://github.com/strongloop/loopback-swagger/blob/b6e39252640732e31c26d6831bf86c5b6593aff7/lib/specgen/swagger-spec-generator.js#L124-L1250 .

— Reply to this email directly or view it on GitHub https://github.com/strongloop/loopback/issues/445#issuecomment-154102862 .

[digitalsadhu]

@bajtos I've been digging into this a bit. My first thought was to try something like this:

var entry = {
      path: routeHelper.convertPathFragments(route.path),
      method: routeHelper.convertVerb(route.verb),
      operation: {
        tags: tags,
        summary: typeConverter.convertText(route.description),
        description: typeConverter.convertText(route.notes),
        // [bajtos] We used to remove leading model name from the operation
        // name for Swagger Spec 1.2. Swagger Spec 2.0 requires
        // operation ids to be unique, thus we have to include the model name.
        operationId: route.method,
        // [bajtos] we are omitting consumes and produces, as they are same
        // for all methods and they are already specified in top-level fields
        parameters: accepts,
        responses: responseMessages,
        deprecated: !!route.deprecated,
        // TODO: security
      }
    };

    if (route.consumes) {
      entry.operation.consumes = route.consumes;
    }

    if (route.produces) {
      entry.operation.produces = route.produces;
    }

But 2 problems,

1. produces and consumes aren't coming through from say a remoteMethod definition so I'm thinking I don't fully understand the nature of the route object. (Though it looks very similar to a remote method definition)
2. Even if 1 above worked, boot scripts are run after the loopback-component-explorer is setup and so doing the following would have no effect:

module.exports = function (app) {
  let remotes = app.remotes();
  let methods = remotes.methods();
  methods.forEach(function (method) {
    method.consumes = ['application/vnd.api+json'];
    method.produces = ['application/vnd.api+json'];
  });
}

Perhaps I'm barking up the wrong tree and rather than trying to allow defining consumes and produces per method I should just be trying to allow a setting on app or strong-remoting setup?

Pointers, suggestions appreciated.

[bajtos]

produces and consumes aren't coming through from say a remoteMethod definition so I'm thinking I don't fully understand the nature of the route object. (Though it looks very similar to a remote method definition)

strong-remoting's SharedMethod copies only a subset of properties from the original options, see the constructor: https://github.com/strongloop/strong-remoting/blob/65479ddb63524274193ea16d5e2ea5fbbc5c8b03/lib/shared-method.js#L81-L141 and also fromFunction method: https://github.com/strongloop/strong-remoting/blob/65479ddb63524274193ea16d5e2ea5fbbc5c8b03/lib/shared-method.js#L158-L169

Even if 1 above worked, boot scripts are run after the loopback-component-explorer is setup and so doing the following would have no effect:

This is a problem with remotes.methods() which creates a temporary short-lived SharedMethod instances. Any changes made to these instances are discarder :(

Perhaps I'm barking up the wrong tree and rather than trying to allow defining consumes and produces per method I should just be trying to allow a setting on app or strong-remoting setup?

Yes, that's what crossed my mind too. It looks like a good strategy, considering that JSON-API implementation almost certainly wants to change consumes/produces globally.

We already have two global options that are related to your work: rest.supportedTypes and rest.xml (docs). Perhaps it's time to add rest.jsonApi?

@digitalsadhu what do you think?

[digitalsadhu]

Thanks @bajtos that makes sense, thanks for clarification.

How were you imagining rest.jsonApi would work?

Perhaps something like, if rest.jsonApi=true the following:

• 'application/vnd.api+json' added to supportedTypes
• 'application/vnd.api+json' added to consumes/produces list

Perhaps also:

• bodyParser modified to treat Content-Type 'application/vnd.api+json' as json

[digitalsadhu]

@bajtos also, how hard is it to get access to strong-remoting rest options inside loopback-swagger?

[bajtos]

How were you imagining rest.jsonApi would work?

Your proposal above looks good to me.

how hard is it to get access to strong-remoting rest options inside loopback-swagger

I'd say it's easy. Swagger specgen has access to remotes and the rest handler, see https://github.com/strongloop/loopback-swagger/blob/b6e39252640732e31c26d6831bf86c5b6593aff7/lib/specgen/swagger-spec-generator.js#L40-L41

The handler has options property, see https://github.com/strongloop/strong-remoting/blob/32efdc433f8bd12f1ecc7872c855216b99c0911e/lib/rest-adapter.js#L40.

[digitalsadhu]

Ok excellent. I'll make that my next push. Thanks for the feedback

[teckays]

I'm waiting for this feature more than for a Christmas present. +100

[digitalsadhu]

@teckays Have a look at the component we are working on here https://github.com/digitalsadhu/loopback-component-jsonapi

It's not quite feature complete for use with ember. (Side loading in the works) and there are still most likely some bugs to work out but it's worth giving it a go. We'd certainly appreciate any big reports etc.

[teckays]

@digitalsadhu I will definitely give it a try and report any bugs/improvements I find. Thank you.

[digitalsadhu]

@teckays we released v0.11.0 of the component yesterday which adds support for side loading and that pretty much rounds out the main features needed for ember users.

For anyone else reading this, https://github.com/digitalsadhu/loopback-component-jsonapi is ready for anyone willing to try it out and submit bugs and such. We are pretty quick to respond and keen to find and fix any issues so please give it a go.

[digitalsadhu]

@bajtos I'm wondering if theres any chance anyone from Strongloop would have the time to have a look through https://github.com/digitalsadhu/loopback-component-jsonapi and offer any feedback? (I know you guys are busy and understand if it's not possible)

Theres a lot of code improvement we'd like to do when we get a chance and we have a bank of integration tests in place so that we can refactor with confidence as needed.

We'd also like to continue adding additional configuration options to make the component more flexible for various real world use cases.

One thing i'm a bit worried about is that as loopback changes we may constantly end up fixing things that break (So far this hasn't happened) so in general i'm hoping theres ways we can make the code base a bit more robust.

[digitalsadhu]

Just an update on loopback-component-jsonapi, we are getting pretty stable at version v0.17.2 We are using it in production in multiple projects at work now. I'd recommend people give it a go and give us feedback, report bugs etc.

[teckays]

@digitalsadhu +1

[vforvalerio87]

+1

[ashishtilara]

+1

[celicoo]

+1

[bajtos]

Closing this issue as done - see https://www.npmjs.com/package/loopback-component-jsonapi