Loopback: 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.

[meng-zhang]

+1

[imjoshholloway]

+1 for this

[ritch]

You can modify sharedMethods to have data returned like you showed above. In 2.x this is possible by modifying sharedMethod.returns.

var sharedMethod = MyModel.sharedClass.find('findById', true);
sharedMethod.returns[0].root = false;

Although I don't think this is a good long term solution. I think we should add the ability to define a global response format setting. What would be helpful is to know what other formats are useful.

[ritch]

As far as supporting jsonapi, I'd like to see what else we need to change and if we can maintain computability with 2.x. Maybe a jsonapi flag that would modify all the right settings to support jsonapi.org conventions?

[eabruzzese]

+1

[bemosior]

+1 as well.

[alagunambi]

+1, integration with ember apps would be cake walk if implemented.

[kaizhu256]

+1

[bradplank]

+1, and happy Ember developers we would be!

[bajtos]

Does Ember include application/vnd.api+json in the accepts headers of the request?

strong-remoting can already serve different response depending on what the client accepts, it should be reasonably easy to add a new case for application/vnd.api+json - see strong-remoting/lib/http-context.js

The difficult part is how to determine the name of the root key (e.g. user for User.findById, users for User.find, but also category for Product.__get__category).

[bradplank]

By default, Ember uses the following in the request header:
Accept:application/json, text/javascript, */*; q=0.01

However, the RESTAdapter can easily be extended to add application/vnd.api+json as an Accept header.

For the root key, wouldn't the model's name / plural name be used? I see the difficulty with the hasManyThrough relationship, as the API explorer shows the incorrect model. (I've been meaning to see if I could fix that issue.)

[bajtos]

For the root key, wouldn't the model's name / plural name be used? I see the difficulty with the hasManyThrough relationship, as the API explorer shows the incorrect model. (I've been meaning to see if I could fix that issue.)

I would make the initial implementation easier if we could use a constant key name. The jsonapi spec explicitly supports that:

The primary resource(s) SHOULD be keyed either by their resource type or the generic key "data".

Does Ember support this too?

[bradplank]

Unfortunately, it seems Ember does not treat the generic key 'data' as a special key. I reviewed the code, and tested just to make sure. The following error message is generated as there is no 'data' model defined:

Uncaught Error: Assertion Failed: Error: No model was found for 'datum'

[NOTE: I am currently using Ember version 1.7.0, however, I do not see any changes in 1.8.1 that would use the generic key 'data'.]

[sergiolepore]

@bradplank hmm... Seems like the Ember Inflector is trying to pluralize the word "data". Try this:

Ember.Inflector.inflector.uncountable('data');

[seriousben]

:+1:

[seriousben]

jsonapi will soon hit 1.0 and only backward compatible changes are planned after that.
If Loopback started supporting it, it would make it very interesting to companies.

[BenjaminHorn]

+1

[Panman82]

Now that json api is 1.0, is there any further news on loopback support? And to note, there is strong movement ATM to make Ember Data natively handle json api.

[sahin]

+1

[sahin]

IS there any update on this?

[Keeo]

:+1:

[raiskila]

Very interested in this!

[adamdilek]

+1

[digitalsadhu]

+1 :+1:

[digitalsadhu]

I've begun writing a bootscript to add jsonapi support. Couple questions: @bajtos @ritch

• Is it possible to modify the list of content type options in the loopback explorer? For testing purposes it would be nice to be able to select application/vnd.api+json
• JSON API specifies the use of PATCH instead of PUT. Is it possible to modify loopback explorer to send PATCHs in place of PUTs?

[bajtos]

Is it possible to modify the list of content type options in the loopback explorer?

ATM, it's not possible to customise the list of content types at route (method) level - see https://github.com/strongloop/loopback-swagger/blob/8eb73acbaae2cd600d03a95dcf55aee6f7560980/lib/specgen/route-helper.js#L161-L162.

However, you can customise this list at global level via options.consumes and options.produces passed to loopback-component-explorer, see https://github.com/strongloop/loopback-swagger/blob/8eb73acbaae2cd600d03a95dcf55aee6f7560980/lib/specgen/swagger-spec-generator.js#L22-L36

JSON API specifies the use of PATCH instead of PUT. Is it possible to modify loopback explorer to send PATCHs in place of PUTs?

LoopBack explorer sends exactly the same HTTP verb which is specified in remoting metadata and used by the rest adapter. You should modify remoting data of your methods to specify a different HTTP verb (method), loopback-explorer will then pick it up automatically.

[digitalsadhu]

Legend @bajtos, thanks!

[bajtos]

Legend @bajtos, thanks!

You are welcome :)

As for customising list of content-types at route (method) level, it's not implemented yet but I am happy to land such patch if you (or somebody else) would be interested in contributing the enhancement.

[digitalsadhu]

My main hope is to be able to make all Jsonapi support changes from a boot script so that anyone could drop the boot script into a loopback app and have instant json API support. I'm about 70% done I think with pretty complete output support including relationships and some basic create delete and update support. With your suggestions above I should be able to get things mostly there but I want to polish, add tests, figure out the best way to release it as a loopback add on etc.

When you say at route level is not yet supported, would that mean it's not possible to modify at a boot script level yet?

[digitalsadhu]

Ps, @bajtos I'd be happy to work toward converting from a boot script to adding to core if I can get some suggestions where/how best to do that.

[drewclauson]

@digitalsadhu :+1:

[bajtos]

When you say at route level is not yet supported, would that mean it's not possible to modify at a boot script level yet?

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).

If your module exported function(app, options), then it could be registered via server/component-config.json and people would not have to copy any files at all.

I'd be happy to work toward converting from a boot script to adding to core if I can get some suggestions where/how best to do that.

Cool! Let's see what you come up with in the form of a boot script (or component) and then we can take a look what's the best way for bringing this into core.

[digitalsadhu]

Ok nice. Will keep at it and report back.

[digitalsadhu]

I've created a loopback component called loopback-component-jsonapi and put my work on this so far there.

Here's the repo:
https://github.com/digitalsadhu/loopback-component-jsonapi

I'd love help from anyone else interested in seeing this completed.

[digitalsadhu]

@bajtos I tried the naive approach of:

remotes.methods().forEach(function (method) {
  if (method.http.verb === 'put') {
    method.http.verb = 'patch';
  }
})

This seems to get me part the way there with some methods showing up as PATCH in the explorer. Relationship updates and such are still PUT eg. cat.prototype.__link__dogs is still PUT as is cat.prototype.__updateById__dogs

Any chance you could shed some light on what I'm not quite understanding?

Cheers.

[bajtos]

@digitalsadhu IIRC, relation methods are set up in a later turn of the event loop, only after the model has been attached to a datasource. IIRC you can listen for Model.on('attached') to get notified about that.

[digitalsadhu]

ohh, k cheers, will pursue that

[digitalsadhu]

@bajtos
I tried this:

//common/models/cat.js
module.exports = function(Cat) {
  Cat.on('attached', function () {
    var remotes = Cat.app.remotes()
    remotes.methods().forEach(function (method) {
      if (method.http.verb === 'put') {
        method.http.verb = 'patch';
      }
    })
  })
};

No change in behaviour. Was this what you were thinking?

[digitalsadhu]

@bajtos I've been digging into this using node inspector and finding the following (can you shed any light?)
in the chrome console:

remotes = app.remotes();
rm13 = remotes.methods()[13]
> SharedMethod
rm13.http.verb
"put"
rm13.http.verb = "patch"
"patch"
rm13.http.verb
"patch"
remotes.methods()[13].http.verb
"put"

I thought remotes() was a singleton and would always return the same objects? It seems that calling it a second time returns objects that have relationship methods returned to "put"

Welcome any advice...

UPDATE
Several hours of node inspecting later and it looks to me like each time you run app.remotes().methods(), the relationship methods get redefined (eg. here https://github.com/strongloop/loopback/blob/master/lib/model.js#L532-L544)
This redefining only happens for relationship remote methods such as cats/:id/dog/:id

I was able to show this by first calling app.remotes().methods() and watching strong remotings shared method module and see the shared methods all get created for all remote methods. A subsequent call to app.remotes().methods() showed only the relationship remote methods being recreated which the other remote methods are returned unchanged.

So as far as I can tell this means it's currently just not possible to use a boot script or component to overwrite the http verb (of relationship remote methods) as any later calls to app.remotes().methods() will overwrite changes.

[bajtos]

@digitalsadhu I thought remotes() was a singleton and would always return the same objects?

AFAICT, remotes returns the same singleton: https://github.com/strongloop/loopback/blob/6fa57754abb959459e68c052b8eac6ad67350bbf/lib/application.js#L53-L65

When it comes to methods(), it creates a new array every time, but that should be still ok. https://github.com/strongloop/strong-remoting/blob/5f0f81c5f96c22792dd45fba0e044bf2adfcbb64/lib/remote-objects.js#L217-L227

I'd say the problem is in SharedClass.prototype.methods() which builds new SharedMethod objects for each remoted function that was not registered via the new 2.x API - this is in order to preserve backwards compatibility with [email protected] apps and I guess relation methods are registered this way. https://github.com/strongloop/strong-remoting/blob/5f0f81c5f96c22792dd45fba0e044bf2adfcbb64/lib/shared-class.js#L99-L121

I think you'll have to iterate through all functions on each model & its prototype and fix remoting metadata there (see https://github.com/strongloop/strong-remoting/blob/5f0f81c5f96c22792dd45fba0e044bf2adfcbb64/lib/shared-class.js#L239-L263).

Mockup:

// mockup, I did not test it myself
app.models().forEach(function(m) {
  eachRemoteFunctionInObject(m, fixHttpMethod);
  eachRemoteFunctionInObject(m.prototype, fixHttpMethod);
});

function fixHttpMethod(fn, name) {
  if (fn.http && fn.http.verb.toLowerCase() === 'put') fn.http.verb = 'patch';
}

// copy implementation of eachRemoteFunctionInObject() from strong-remoting's lib/shared-class.js

If this code proves to be the correct solution, then we can investigate how to improve strong-remoting's API to allow you to iterate all remote functions in a way that allows you to modify their remoting metada, regardless of the way how they are defined.

[bajtos]

@digitalsadhu the example code above patches only the relation ([email protected]) functions, you will still have to iterate through app.remotes().methods() to patch the functions defined in the new style.

[digitalsadhu]

@bajtos
Looks like your approach wont quite work. Debugging through sharedClass here:

https://github.com/strongloop/strong-remoting/blob/5f0f81c5f96c22792dd45fba0e044bf2adfcbb64/lib/shared-class.js#L100-L121

// static methods
  eachRemoteFunctionInObject(ctor, function(fn, name) {
//breakpoint set here is never hit
    if (functionIndex.indexOf(fn) === -1) {
      functionIndex.push(fn);
    } else {
      var sharedMethod = find(methods, fn);
      sharedMethod.addAlias(name);
      return;
    }
    methods.push(SharedMethod.fromFunction(fn, name, sc, true));
  });

  // instance methods
  eachRemoteFunctionInObject(ctor.prototype, function(fn, name) {
//breakpoint set here is never hit
    if (functionIndex.indexOf(fn) === -1) {
      functionIndex.push(fn);
    } else {
      var sharedMethod = find(methods, fn);
      sharedMethod.addAlias(name);
      return;
    }
    methods.push(SharedMethod.fromFunction(fn, name, sc));
  });

The eachRemoteFunctionInObject method never gets called. Actually ever so far as I can tell... In all my debugging with a breakpoint inside the callback from eachRemoteFunctionInObject i've never seen it get hit...

What seems to be the key to setting up the relations is the resolver is this:
https://github.com/strongloop/strong-remoting/blob/5f0f81c5f96c22792dd45fba0e044bf2adfcbb64/lib/shared-class.js#L123-L126

//at this point methods = []

// resolvers
this._resolvers.forEach(function(resolver) {
  resolver.call(this, _define.bind(sc, methods)); 
});

//at this point methods is an array of relationship remote methods

Before that is called methods is an empty array. Afterwards the relationship remote methods have been setup. It resets these up each time you call methods() starting with the ModelCtor.relations definitions. I'm still chasing this approach down and hoping theres some way I can get in to redefine the verb before the resolvers are run

[digitalsadhu]

@bajtos It looks to me like at present because the relationship methods are recreated on the fly each time .methods() is called, the only way to do this right now is to overwrite the relationship methods on the Model class in loopback since thats where the remote method definitions are. See:

https://github.com/strongloop/loopback/blob/master/lib/model.js#L485-L492

The define method is passed in from strong remotings sharedClass but the definitions are hard coded in methods like Model.hasOneRemoting and its from there that the remote methods are created each time you call methods()

[digitalsadhu]

@bajtos It looks to me like at present because the relationship methods are recreated on the fly each time .methods() is called, the only way to do this right now is to overwrite the relationship methods on the Model class in loopback since thats where the remote method definitions are. See:

https://github.com/strongloop/loopback/blob/master/lib/model.js#L485-L492

The define method is passed in from strong remotings sharedClass but the definitions are hard coded in methods like Model.hasOneRemoting and its from there that the remote methods are created each time you call methods()

Final working solution, basically duplicate a tonne of code out of model.js just to change "put" to "patch"

function convertNullToNotFoundError(toModelName, ctx, cb) {
  if (ctx.result !== null) return cb();

  var fk = ctx.getArgByName('fk');
  var msg = 'Unknown "' + toModelName + '" id "' + fk + '".';
  var error = new Error(msg);
  error.statusCode = error.status = 404;
  error.code = 'MODEL_NOT_FOUND';
  cb(error);
}

function fixHttpMethod(fn, name) {
  if (fn.http && fn.http.verb && fn.http.verb.toLowerCase() === 'put') fn.http.verb = 'patch';
}

module.exports = function (app, options) {
  app.models().forEach(function(ctor) {
    ctor.hasOneRemoting = function(relationName, relation, define) {
      var pathName = (relation.options.http && relation.options.http.path) || relationName;
      var toModelName = relation.modelTo.modelName;

      define('__get__' + relationName, {
        isStatic: false,
        http: {verb: 'get', path: '/' + pathName},
        accepts: {arg: 'refresh', type: 'boolean', http: {source: 'query'}},
        description: 'Fetches hasOne relation ' + relationName + '.',
        accessType: 'READ',
        returns: {arg: relationName, type: relation.modelTo.modelName, root: true},
        rest: {after: convertNullToNotFoundError.bind(null, toModelName)}
      });

      define('__create__' + relationName, {
        isStatic: false,
        http: {verb: 'post', path: '/' + pathName},
        accepts: {arg: 'data', type: toModelName, http: {source: 'body'}},
        description: 'Creates a new instance in ' + relationName + ' of this model.',
        accessType: 'WRITE',
        returns: {arg: 'data', type: toModelName, root: true}
      });

      define('__update__' + relationName, {
        isStatic: false,
        http: {verb: 'patch', path: '/' + pathName},
        accepts: {arg: 'data', type: toModelName, http: {source: 'body'}},
        description: 'Update ' + relationName + ' of this model.',
        accessType: 'WRITE',
        returns: {arg: 'data', type: toModelName, root: true}
      });

      define('__destroy__' + relationName, {
        isStatic: false,
        http: {verb: 'delete', path: '/' + pathName},
        description: 'Deletes ' + relationName + ' of this model.',
        accessType: 'WRITE'
      });
    };

    ctor.hasManyRemoting = function(relationName, relation, define) {
      var pathName = (relation.options.http && relation.options.http.path) || relationName;
      var toModelName = relation.modelTo.modelName;

      var findByIdFunc = this.prototype['__findById__' + relationName];
      define('__findById__' + relationName, {
        isStatic: false,
        http: {verb: 'get', path: '/' + pathName + '/:fk'},
        accepts: {arg: 'fk', type: 'any',
          description: 'Foreign key for ' + relationName, required: true,
          http: {source: 'path'}},
        description: 'Find a related item by id for ' + relationName + '.',
        accessType: 'READ',
        returns: {arg: 'result', type: toModelName, root: true},
        rest: {after: convertNullToNotFoundError.bind(null, toModelName)}
      }, findByIdFunc);

      var destroyByIdFunc = this.prototype['__destroyById__' + relationName];
      define('__destroyById__' + relationName, {
        isStatic: false,
        http: {verb: 'delete', path: '/' + pathName + '/:fk'},
        accepts: {arg: 'fk', type: 'any',
          description: 'Foreign key for ' + relationName, required: true,
          http: {source: 'path'}},
        description: 'Delete a related item by id for ' + relationName + '.',
        accessType: 'WRITE',
        returns: []
      }, destroyByIdFunc);

      var updateByIdFunc = this.prototype['__updateById__' + relationName];
      define('__updateById__' + relationName, {
        isStatic: false,
        http: {verb: 'patch', path: '/' + pathName + '/:fk'},
        accepts: [
          {arg: 'fk', type: 'any',
            description: 'Foreign key for ' + relationName, required: true,
            http: {source: 'path'}},
          {arg: 'data', type: toModelName, http: {source: 'body'}}
        ],
        description: 'Update a related item by id for ' + relationName + '.',
        accessType: 'WRITE',
        returns: {arg: 'result', type: toModelName, root: true}
      }, updateByIdFunc);

      if (relation.modelThrough || relation.type === 'referencesMany') {
        var modelThrough = relation.modelThrough || relation.modelTo;

        var accepts = [];
        if (relation.type === 'hasMany' && relation.modelThrough) {
          // Restrict: only hasManyThrough relation can have additional properties
          accepts.push({arg: 'data', type: modelThrough.modelName, http: {source: 'body'}});
        }

        var addFunc = this.prototype['__link__' + relationName];
        define('__link__' + relationName, {
          isStatic: false,
          http: {verb: 'patch', path: '/' + pathName + '/rel/:fk'},
          accepts: [{arg: 'fk', type: 'any',
            description: 'Foreign key for ' + relationName, required: true,
            http: {source: 'path'}}].concat(accepts),
          description: 'Add a related item by id for ' + relationName + '.',
          accessType: 'WRITE',
          returns: {arg: relationName, type: modelThrough.modelName, root: true}
        }, addFunc);

        var removeFunc = this.prototype['__unlink__' + relationName];
        define('__unlink__' + relationName, {
          isStatic: false,
          http: {verb: 'delete', path: '/' + pathName + '/rel/:fk'},
          accepts: {arg: 'fk', type: 'any',
            description: 'Foreign key for ' + relationName, required: true,
            http: {source: 'path'}},
          description: 'Remove the ' + relationName + ' relation to an item by id.',
          accessType: 'WRITE',
          returns: []
        }, removeFunc);

        // FIXME: [rfeng] How to map a function with callback(err, true|false) to HEAD?
        // true --> 200 and false --> 404?
        var existsFunc = this.prototype['__exists__' + relationName];
        define('__exists__' + relationName, {
          isStatic: false,
          http: {verb: 'head', path: '/' + pathName + '/rel/:fk'},
          accepts: {arg: 'fk', type: 'any',
            description: 'Foreign key for ' + relationName, required: true,
            http: {source: 'path'}},
          description: 'Check the existence of ' + relationName + ' relation to an item by id.',
          accessType: 'READ',
          returns: {arg: 'exists', type: 'boolean', root: true},
          rest: {
            // After hook to map exists to 200/404 for HEAD
            after: function(ctx, cb) {
              if (ctx.result === false) {
                var modelName = ctx.method.sharedClass.name;
                var id = ctx.getArgByName('id');
                var msg = 'Unknown "' + modelName + '" id "' + id + '".';
                var error = new Error(msg);
                error.statusCode = error.status = 404;
                error.code = 'MODEL_NOT_FOUND';
                cb(error);
              } else {
                cb();
              }
            }
          }
        }, existsFunc);
      }
    };
  });

  app.remotes().methods().forEach(fixHttpMethod);
}

[bajtos]

@digitalsadhu I see. Sorry for offering you a wrong advice.

It makes me wonder if it is possible to override SharedMethod.prototype.methods(), so that you can intercept the returned list of methods and apply the modifications as necessary, without having to duplicate all that code.

// mockup, I did not test it myself
app.models().forEach(function(m) {
  var originalFn = m.sharedClass.methods;
  m.sharedClass.methods = function() {
    var result = originalFn.apply(this, arguments);
    result.forEach(fixHttpMethod);
    return result;
  };
});

[digitalsadhu]

Thanks @bajtos. Ill have a look at that. One thing that I've come to realise though (reading and reading json api spec) is that the way loopback does relationships isn't really going to be compatible out of the box with json api. (not as simple as modifying the output and the payloads) I think i'm going to have to add and remove remote methods for relationships to make it work. This probably means that blanket modifying the relationship verbs won't be needed as such.

For relationships

This stuff should stay the same:

has many

GET posts/{id}/comments

has one

GET posts/{id}/author
etc.

This stuff needs to be added/changed:

//links an author to a post, may create the author if author doesn't exist (author details specified in payload)
POST /posts/{id}/relationships/author

//links a different author to a post (author details specified in payload including id for author)
PATCH /posts/{id}/relationships/author

//deletes/unlinks an author from a post
DELETE /posts/{id}/relationships/author

So my current thinking now is that I may need to completely disable various loopback methods (theres no support for upsert in JSON API) I may need to change some methods eg. relationship methods like link and unlink. And I may need to add entirely new methods eg. updating relationship links.

It's looking a little bit complex and my plan is to sit down and plan out how it might all look before trying to tackle it once I complete reviewing the basic (non relational) stuff I have already see: https://github.com/digitalsadhu/loopback-component-jsonapi/issues/9 and adding tests

[tsteuwer]

@digitalsadhu you are amazing. If loopback could support json api then I know a ton of ember sites that would love to switch over to loopback

[digitalsadhu]

Thanks a lot @tsteuwer ! Progress is going well. Adding tests as we speak (My wife has our little son off for a walk so I'm squeezing in a couple hours work on the project)

[digitalsadhu]

@bajtos Currently bodyparser isn't supporting content-type application/vnd.api+json and ctx.req.body === [] if I don't set content-type specifically to application/json. I wonder what the best way to fix this is? Should I submit a patch to loopback core to support application/vnd.api+json body parsing?

[raymondfeng]

@digitalsadhu You can configure the json body parser so that it can support extra types. See https://github.com/expressjs/body-parser#type

[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š [email protected]
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š [email protected]
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