Initial work on new swagger based REST API

[nelsonsilva]

I added a new REST API frontend based on swagger. It delegates the API calls to the existing functions so it's merely "cosmetic". The great thing about this is that you get a cool UI and a code generator. You can try this out by checking out my branch and going to http://nelsonsilva.github.com/etherpad-lite-rest-ui/

[JohnMcLear]

Do we need an APIKey to test it out on your URL?

[nelsonsilva]

I don't have a publicly available Etherpad instance so you have to run etherpad locally and put your api key in the text input.

[nelsonsilva]

I added a first version of a java client code generator here: https://github.com/nelsonsilva/etherpad-lite-codegen

[JohnMcLear]

Afaik there is another java client for Etherpad, are you aware?

[nelsonsilva]

Yes i am. I'm using it for now. But I had to try swagger-codegen (https://github.com/wordnik/swagger-codegen) to see if this works.

Swagger gives you a cool REST UI so you can document your endpoints and play with them (have you played with my version yet?).

Also, by using swagger-codegen you can automatically create clients for Scala, Flash, Java, Objc, PHP, Python, Python3 and Ruby which would be nice since these can be built using any CI service and would always be up to date. For instance for my first java client version i'm using drone.io.

[JohnMcLear]

Ahh, I see, nice! :)

[disy-mk]

this is fancy!

[nelsonsilva]

I think it would be nice way to start shaping a new REST API for Etherpad Lite. I grouped the operations in resources (similar to what you have in the docs). I also moved the stuff that was under "data" in the responses to the top level object ex: {"code":0, "message":"", "padId":"dsdsds"}. This was because swagger does not support nested containers (lists) and I wanted to have proper JSON schema for the responses.

[JohnMcLear]

I agree with a REST API, my only concern is that we really need the core issue of that the API doesn't allow for different versions of API endpoints right now (there is an open issue for this) and I think we need to address this first before adding fancy wrappers :)

That said, I'm really happy with the work you are doing :) +1

[jhollinger]

+1 for REST. As an EPLite API Client author the codegen suggestion makes me sad - like a factory worker losing his job to automation. But "The needs of the many..." so +1 again.

[disy-mk]

I'm going to have a look at the PHP version of the client, as this is what I am working currently with.

[nelsonsilva]

@jhollinger This is still WIP so it'll probably be a while before this is merged (if it's even merged) Still, the codegen uses mustache templates so we still need someone to make nice templates for each of the clients... so I guess the factory worker gets to keep its job ;)

[nelsonsilva]

I came up with a quick "hack" to have a REST endpoint per version (instead of a single one for the latest version). Basically I load the swagger module once per version by removing it from the require.cache. This allows me to have several swagger "instances" which would otherwise be impossible since node caches the modules and swagger's not wrapped in a function. This is cool since we can generate a client for each API version (see here for an example: https://github.com/nelsonsilva/etherpad-lite-codegen).

[marcelklehr]

IMHO, the current API (v1) must still be usable after pulling this (thus, this should become version 2)

[nelsonsilva]

@marcelklehr this is served in a different endpoint /rest/ so the current API is still usable

[marcelklehr]

ah, okay.

[JohnMcLear]

Hey @nelsonsilva can you please update this so it can be automatically merged then it's going into develop (after we release 1.2.81) thanks :)

[nelsonsilva]

@JohnMcLear done. Rebased to develop.

[JohnMcLear]

Thanks :)

[JohnMcLear]

Hey man, can you please push some documentation or so for this new API?

[nelsonsilva]

@JohnMcLear

1. Start etherpad-lite
2. Go to: http://nelsonsilva.github.com/etherpad-lite-rest-ui/

:P

[andypost]

Awesome! it works

[disy-mk]

I cannot test this btw; as soon as i change the port, the host changes to 0.0.0.0.

0 : error http://0.0.0.0:9876/rest/1.2.7/api/pad?api_key=blahblah

---

K, tracked down the problem to my settings file; to ease things I bind to 0.0.0.0 resulting in getting redirected to that ip

[JohnMcLear]

1cfc8eda19176f9359a1bce6ff62d3c6680d3395 breaks import somehow..

[JohnMcLear]

http://debian:9001/rest/1.2.81/api/pad?api_key=test

Should work right?

I get

Cannot GET /rest/1.2.81/api/pad?api_key=test

[disy-mk]

The API versions do not match… ?

http://debian:9001/rest/1.2.81/api/pad?api_key=test Should work right?

I get

Cannot GET /rest/1.2.7/api/pad?api_key=test — Reply to this email directly or view it on GitHub.

[JohnMcLear]

updated..

[JohnMcLear]

http://debian:9001/rest/1.2.7/api/pad?api_key=test works

[JohnMcLear]

/rest/1.2.9/api/pad?api_key doesn't work..

So what am I missing here?

[disy-mk]

1.2.7 is the latest API in APIHandler.js;

Am I missing something?

/rest/1.2.9/api/pad?api_key doesn't work..

So what am I missing here?

[JohnMcLear]

I see :) Totally forgot it doesn't track actual v#

[disy-mk]

I already had some thought about updating the API logic in a way that API(1.2.7) is like += API(1.2) + new functions

or something like that and a list list of currently available API versions; currently the copy pasting of currently existing functions from API vera. to the next one is weird ^^

I see :) Totally forgot it doesn't track actual v#

[marcelklehr]

@ manuel: I agree

[andypost]

suppose "test" method is a nice way to tell about current API version that server supports. And it would be nice feature to introduce a method to enumerate available API methods - so plugins could register their own methods for external usage (for example to get some data from external system)

[disy-mk]

@marcelklehr: see #1638

[nelsonsilva]

The long term plan is to resort to something like I've done in https://github.com/ether/etherpad-lite/pull/1528 I do not want to tie the API specification to the swagger structure but having:

API[">=1.2.7"].getHTML = { parameters:[...], response:{...}, method:["GET"], description:"....", action: function(...){...} }

should allow us to have everything in a single place.

The versioning uses semver and the full list of methods for each API version is determined at runtime based on the rules.

I just need some feedback on the JSON schema for the responses. I've removed the "data" atribute and now have proper models for the responses. This is also due to some limitations in swagger where nested "typed" list are not supported so I ended up flattening the models as much as possible.

This move is especially interesting to allow automatic client generation based on the API spec.

I've added a simple Java client example here: https://github.com/nelsonsilva/etherpad-lite-codegen

[disy-mk]

Yeah, I'm aware of your work. But as far as I could tell my PR doesn't interfere with either of your commits.

[nelsonsilva]

In https://github.com/ether/etherpad-lite/pull/1638 you provide another way to tackle API versioning, by concating the list of functions explicitly:

API_V1_1 = apiConcat(API_V1, API_V1_1) ; etc...

which I've done in https://github.com/ether/etherpad-lite/pull/1528 :

// got through each of the semver rules for(var version in API) { // if the rule satisfies the required version if (semver.satisfies(apiVersion, version)) { // Add these functions for (var fn in API[version]) { _CACHE[apiVersion][fn] = API[version][fn]; } } }

I'm not by any means saying that my way is the way but these two PR serve the same purpose and we must agree on an approach for this. Personally I prefer the semver rules approach as you have a more semantic specification...

[disy-mk]

OK, just had a look at APIhandler and as neither of your commits touched the huge list "version" I tried to compact that construction

[nelsonsilva]

@disy-mk The plan is to remove the that list. With the semver rules in each of the functions we'd just need a list of released versions with their numbering

versions = ["1", "1.1", ...]

So the getVersions function would just have to return that list...

[disy-mk]

OK, I understand;

maybe #1638 might still be nice if we chose a way of providing compatibility for existing clients (which we were afraid of getting broken by your work). That way there would be enough time to adapt the existing clients appropriately until we have all clients getting auto-built.