Skip to content
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

Remote methods mapping in JSON #209

Closed
nikita-leonov opened this issue Mar 2, 2014 · 13 comments · Fixed by #1366
Closed

Remote methods mapping in JSON #209

nikita-leonov opened this issue Mar 2, 2014 · 13 comments · Fixed by #1366
Assignees

Comments

@nikita-leonov
Copy link

Would be nice to have an option to define remote methods mapping to an actual classes in JSON. So we will have complete REST interface defined in one place with a logic and complex cases in JS. I do not see why some of such constructions could not be a part of JSON mapping:
ApplicationModel.authenticate,
{
accepts: [
{arg: 'appId', type: 'string', required: true},
{arg: 'key', type: 'string', required: true}
],
returns: {arg: 'accessToken', type: 'object', root: true},
http: {verb: 'post'}
}

@raymondfeng
Copy link
Member

Your proposal is aligned with the direction to make it possible to describe the remote methods using json. One idea is to have a routes.json to map http reqs (verb, url, content-type/accept) to nodejs methods.

@nikita-leonov
Copy link
Author

👍 thanks. Wold be helpful feature. As well as UI for simplifying this configs definition as a step after all configuration options will be available in json. I can clearly see how this solution will overcome parse.com and other MBAAS with such roadmap.

@raymondfeng raymondfeng added this to the 2.0.0 milestone Mar 3, 2014
@ritch
Copy link
Member

ritch commented Mar 4, 2014

@nikita-leonov (and anyone else reading)... proposals / drafts for a routes.js spec are very much welcomed. We are happy to take contributions 😄

@nikita-leonov
Copy link
Author

I would recommend to piggyback on existing solution instead of inventing some new standard. This is healthy for product, third-party community support and moving complete industry forward.

I know four solutions that could be somehow used for definition of resources mapping and :

  • WSDL (http://www.w3.org/TR/wsdl) — well known. Too complex :)
  • Mashery iodocs (https://github.com/mashery/iodocs) — main purpose is the same as for swagger, however I believe it could be used as a base as they represent all complexity of APIs within their standard.
  • RAML (http://raml.org) — RESTful API Modeling Language. This one is cool but so far lack of traction. Could be a nice idea to review.
  • WRML (https://github.com/wrml/wrml/) — Web Resource Modeling Language. Developed by Mark Masse, author of book "REST API Design Rulebook. Designing Consistent RESTful Web Service Interfaces". Nice book, nice thoughts. He had a plan to develop language for describing APIs, but it progressing not very fast. Possible we may try to reach him and cooperate. I requested specification for a standard, just for an option to compare with other solutions that we I listed. So far it is weakest option from a list ;(

I really like RAML. It is modular, extendable and so on. Current approach with json is really limiting as we forced to define everything within one json file. On a big APIs current approach will become a mess. Also Masher iodocs could be a nice option from a business development standpoint for strongloop and loopback. IODocs is weaker form a technology point of view as it is not a standard, it is a tool like swagger but with own specification of mappings for methods and resources. But at least they have all use cases covered. Btw currently swagger do not shop remote methods, so could be an option to solve multiple tasks at once — get specification for a mapping, get right tool for self-documented APIs and build relationship with Mashery.

@nikita-leonov
Copy link
Author

One more item in favor of RAML they have JavaScript based parser implemented — https://github.com/raml-org/raml-js-parser It capable for running in node.js.

@raymondfeng
Copy link
Member

Thanks for the ideas. We have been exploring most of them. Swagger is currently used for the API explorer.

There are probably a few layers in play:

  1. Define the data models (LDL)
  2. Define the method signature (as JS is weakly-typed lang)
  3. Define the mapping between a remoting transport/protocol and the Node.js method

Ideally, we should be able to declare/derive the metadata declaratively or programatically. Item 2 is more tied to the JS method, some sort of annotations should work. Item 3 could be further decoupled.

@nikita-leonov
Copy link
Author

Agree regarding data models. Not all the standards cover this. For example RAML is more API oriented rather than resources / models oriented specification. That is why I provided WRML as a reference as there work ongoing on defining APIs by using resources terms which is more in line with your current approach.

I will think a little bit more about this task in direction of leveraging existing technologies and standards. It is much harder to invent and develop something new, rather than leverage existing functionality. I strongly believe that there is technology available that will fit the goal.

@ritch
Copy link
Member

ritch commented Apr 8, 2014

@altsang @raymondfeng @bajtos and I discussed this a bit.

Points in our discussion:

  • LDL is for defining anything in loopback
  • We can output to these other formats (RAML, Swagger, etc) but LDL is the native input/output format for LoopBack.
  • We should support exporting other REST-DSL-ish formats like we currently do for Swagger in the explorer
  • We should also continue to provide hook APIs (you implemente a JS function) where a feature is impractical to implement in LDL.

@mrfelton
Copy link
Contributor

mrfelton commented Mar 4, 2015

fwiw I'm doing something like this in my own setup by using the unimplemented methods key in the model json schema to define the remote methods in the same way as you would normally in the remoteMethod() method, and then using that to setup the remote methods on the fly.

eg

common/models/my-model.json

  ...
  "relations": [],
  "acls": [],
  "methods": {
    "syncToRemote": {
      "doc": "Syncronise a person to a supported provider account.",
      "accepts": [{
        "arg": "id",
        "type": "number",
        "required": true,
        "description": "PersistedModel id"
      }, {
        "arg": "provider",
        "type": "string",
        "required": true,
        "http": {
          "source": "form"
        },
        "description": "The external account provider to update."
      }],
      "returns": {
        "arg": "person",
        "type": "object",
        "root": true,
        "description": "The updated person object."
      },
      "http": {
        "path": "/:id/syncToRemote",
        "verb": "post"
      }
    }
  }

common/models/my-model.js:

  // Set up remote methods from model config schema json.
  // TODO: Remove / Refactor this once loopback supports this natively.
  MyModel.on('attached', function () {
    var app = MyModel.app;
    _.each(app.models.MyModel.settings.methods, function (value, key) {
      MyModel.remoteMethod(key, value);
    });
  });

Seems to be working ok so far. My only gripe with it is that the remoteMethod call expects documentation for the properties to be in a description key, which is inconsistent with all the rest of the documentation in common/models/my-model.json which uses the doc key instead.

Would be great to have some support like this in loopback itself.

@bajtos
Copy link
Member

bajtos commented Mar 4, 2015

@mrfelton can you submit a pull request with your implementation? Remote methods should be defined by registry.createModel.

Few things to change in the second snippet:

  1. I believe you don't have wait for the model to be attached. In LoopBack 2.x, the remoting metadata is stored in a name-based lookup table, it's ok to describe a remote method before it's mixed into the object.

  2. The API of Model.remoteMethod() is suboptimal and we would like to fix it in 3.0, see Inconsistent API for remote methods and hooks #597. I would like the JSON-based registration to use the new way, meaning that the method name "ping" should be mapped to a static method (isStatic: true) and the method name "prototype.pong" should be mapped to a prototype method (isStatic: false). This is something you have to handle in your code until Model.remoteMethod() is fixed.

My only gripe with it is that the remoteMethod call expects documentation for the properties to be in a 'description' key, which is inconsistent with all the rest of the documentation in common/models/my-model.json which uses the 'doc' key instead.

The "doc" keyword is deprecated, you should use "description" in your model definitions too. See Model definitions JSON file docs.

@mrfelton
Copy link
Contributor

mrfelton commented Mar 4, 2015

Thanks @bajtos .

  1. I believe you don't have wait for the model to be attached. In LoopBack 2.x, the remoting metadata is stored in a name-based lookup table, it's ok to describe a remote method before it's mixed into the object.

The reason I wait until the model is attached is because I needed access to the app object so that I can inspect the structure and pull out the method details from there. I wasn't able to get a handle on a complete enough app object anywhere else. I'm guessing that if I move it toregistry.createModel. I'll have direct access to it from there.

  1. The API of Model.remoteMethod() is suboptimal and we would like to fix it in 3.0, see Inconsistent API for remote methods and hooks #597. I would like the JSON-based registration to use the new way, meaning that the method name "ping" should be mapped to a static method (isStatic: true) and the method name "prototype.pong" should be mapped to a prototype method (isStatic: false).

I'm not sure what you mean by ping and pong. Any docs on that you can point me towards?

The "doc" keyword is deprecated, you should use "description" in your model definitions too. See Model definitions JSON file docs.

Awesome!

@bajtos
Copy link
Member

bajtos commented Mar 4, 2015

  1. The API of Model.remoteMethod() is suboptimal and we would like to fix it in 3.0, see Inconsistent API for remote methods and hooks #597. I would like the JSON-based registration to use the new way, meaning that the method name "ping" should be mapped to a static method (isStatic: true) and the method name "prototype.pong" should be mapped to a prototype method (isStatic: false).

I'm not sure what you mean by ping and pong. Any docs on that you can point me towards?

They are just example method names. "ping" is a static method, "pong" is a prototype (instance) method.

The code sample in your comment above requires developers to register them in this way:

remoteMethods: {
  "ping": { isStatic: true, /*...*/ },
  "pong": { isStatic: false, /*...*/ }
}

What I would like to see instead:

remoteMethods: {
  "ping": { /*...*/ },
  "prototype.pong": { /*...*/ }
}

Does it make sense now?

@mrfelton
Copy link
Contributor

mrfelton commented Mar 4, 2015

gotcha. I read through #597 that you linked to also so it makes more since to me now. I actually hadn't come across the isStatic property before.

mrfelton pushed a commit to mrfelton/loopback that referenced this issue Mar 12, 2015
mrfelton pushed a commit to mrfelton/loopback that referenced this issue Mar 12, 2015
…e added to the model strongloop#209

Signed-off-by: Tom Kirkpatrick <tom@systemseed.com>
@bajtos bajtos assigned bajtos and unassigned ritch May 6, 2015
@bajtos bajtos added the #review label May 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants