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

Support more RESTful API #91

Open
mmindenhall opened this issue Jun 12, 2015 · 20 comments
Open

Support more RESTful API #91

mmindenhall opened this issue Jun 12, 2015 · 20 comments

Comments

@mmindenhall
Copy link
Collaborator

Hello,

I'm just starting to use tiedot for an application on a low memory footprint device. The results so far seem very promising!

The current API is definitely usable, but I was wondering if it would be possible to support a more RESTful API? There are a couple of issues that this would solve.

  1. Any HTTP method can be used for any request, which violates the intended use of the HTTP methods. A RESTful API would use only the intended method for each request.
  2. Documents are JSON (and only JSON), so it seems unnatural to URL encode them and submit them either with a GET (as URL encoded query params) or POST/PUT (as application/x-www-form-urlencoded content). A RESTful API would use path parameters and application/json encoding only, which would simplify things. Also, unnecessary overhead of URL decoding the documents would be eliminated. For example, in the tutorial a document is inserted like this (lines omitted for brevity):
$ curl -v --data-ascii doc='{"a": 1, "b": 2}' "http://localhost:8080/insert?col=Feeds"
> POST /insert?col=Feeds HTTP/1.1
> Content-Length: 20
> Content-Type: application/x-www-form-urlencoded
>
* upload completely sent off: 20 out of 20 bytes
< HTTP/1.1 201 Created
< Cache-Control: must-revalidate
< Content-Type: text/plain
< Content-Length: 19
* Connection #0 to host localhost left intact
3587505840683728602

This would instead become something like:

POST /v1/insert/Feeds HTTP/1.1
Content-Type: application/json
{"a": 1, "b": 2}

The v1 prefix makes this a new API version so the existing API can still be supported. The new version would use query path parameters, application/json content, and restrict to appropriate HTTP verbs. For example:

# Collection management
POST   /v1/create                 # param(s) in request body JSON
POST   /v1/create/:col            # empty request body
POST   /v1/create/:col/:numparts  # empty request body
GET    /v1/all
PUT    /v1/rename                 # param(s) in request body JSON
PUT    /v1/rename/:old/:new       # empty request body
DELETE /v1/:col                   # empty request body
GET    /v1/scrub/:col
GET    /v1/sync

# Document management
POST   /v1/insert/:col
GET    /v1/get/:col/:id
PUT    /v1/update/:col/:id   # replace entire existing document
PATCH  /v1/update/:col/:id   # (optional) modify only specified fields in document
DELETE /v1/delete/:col/:id
GET    /v1/approxdoccount/:col
GET    /v1/getpage/:col/:page/:total

# Etc.

I can offer to help with documentation, but this project is my first foray into golang, so I doubt I could help much with implementation at this point. Thanks!

@HouzuoGuo
Copy link
Owner

I'm very glad to hear that tiedot could be useful to you. I'll think through it and get back to you soon.

@HouzuoGuo
Copy link
Owner

I like the idea very much, but unfortunately at the moment I do not have time working on this feature. Please take a look at:
https://github.com/HouzuoGuo/tiedot/blob/master/httpapi/srv.go

Would you like to give it a try at implementing these APIs?

@omeid
Copy link
Collaborator

omeid commented Jun 16, 2015

👍 for the suggested API, it will work great with Angular Resource which will make a new control panel development and other tiedot + angular apps very smooth.

@Dr-Terrible
Copy link
Collaborator

👍

As a starter, I think it would be quite useful to have a simple RESTful API with a minimal feature set for basic operations. Since tiedot is using JSON internally, then json:api and RAML could be a good fit for modelling the API.

@mmindenhall
Copy link
Collaborator Author

I might be able to work on this once I get a bit more experience with golang and some more free time. We're just starting to port an existing Java application to golang in order to be able to run in a smaller footprint. FYI, we're using tiedot to replace Hazelcast in the new architecture.

@HouzuoGuo
Copy link
Owner

I just added all of you into collaborator list, please feel free to experiment with Restful API implementations however you like!
@mmindenhall if you see any vital feature of hazelcast missing from tiedot, or if you have any questions, feel free to ask!

@Dr-Terrible
Copy link
Collaborator

In the next few weeks I have some free time to spare and I'd like to tackle this issue. Anyone of the previous commenters is welcome to join me in the effort.

As a start I was thinking to model the API using RAML or OpenAPI Spec (ex Swagger) formats; it seems that OpenAPI/Swagger 2.0 is well supported by several Golang tools, while RAML isn't.

So, I vouch for OpenAPI/Swagger by using go-swagger. With go-swagger we can automate the creation of all the boilerplate code necessary for the client / server API, and automate the API validation too. All we need to do is to manually implement the models of the API. This way go-swagger can save us tons of time and the usual useless drama about API stability and its validation process.

Any preference / suggestion / alternative about it?

@HouzuoGuo
Copy link
Owner

Hey Doctor.
I have heard about swagger and how easy it is to manage the entire API lifecycle with it, in fact another project of mine is also making use of swagger. It would definitely be a nice addition to tiedot, please feel very free to go ahead with implementing it.

@prods
Copy link
Collaborator

prods commented Jul 31, 2017

I decided to work on this enhancement using httprouter.

Below you will see my proposed set of routes. They may change, but this is just the starting point.

Method Routes Original Routes
POST /collection/:collection_name /create
PUT /collection/:collection_name /rename
DELETE /collection/:collection_name /drop
GET /collections /all
GET /collection/:collection_name/scrub /scrub
POST /collection/:collection_name/query /query
GET /collection/:collection_name/count /count
POST /collection/:collection_name/doc /inert
GET /collection/:collection_name/doc/:id /get
PUT /collection/:collection_name/doc/:id /update
DELETE /collection/:collection_name/doc/:id /delete
GET /collection/:collection_name/page/:page_total /getpage
GET /collection/:collection_name/approxdoccount /approxdoccount
POST /collection/:collection_name/index/:index_path /index
POST /collection/:collection_name/unindex/:index_path /unindex
GET /collection/:collection_name/indexes /indexes
GET /sync /sync
GET /shutdown /shutdown
GET /dump /dump

Considerations

Query Routes as a POST and not a GET
The query route should be setup as POST. This will make the query object more scalable over time.

API Routes Versioning
I did not add a version number to the route. I think braking changes or reasons for a major version change should be controlled at the application level and not the API. Versioning an API may have its advantages on a service API, but I am not sure it will be beneficial for a database interface since it may create technical debt before getting to another major version. This should be controlled with tags and new version releases. Thoughts?

API Backwards Compatibility
I added an boolean argument "legacy-api-support" to the server in order to enable the old API routes. This should allows anyone with existing code to continue using the old routes while they migrate.

If everything goes well I may have a full working version soon.

@HouzuoGuo
Copy link
Owner

Thanks very much @prods, please proceed with the proposal and I've added you to collaborator list.

Would it be more appropriate to use POST action for /sync, /shutdown, and /dump?

@prods
Copy link
Collaborator

prods commented Jul 31, 2017

Thanks! and Yes, it makes perfect sense. Database engine specific operations should run on POSTS. I will update the route list and I will keep you posted. Let me know if you find anything else or you have any recommendation.

@prods
Copy link
Collaborator

prods commented Jul 31, 2017

Below is an updated version of the API Routes.

Method Routes Original Routes Status
GET /collections /all Tested
POST /collection/:collection_name /create Tested
PUT /collection/:collection_name/rename/:new_collection_name /rename Tested
DELETE /collection/:collection_name /drop Tested
POST /collection/:collection_name/scrub /scrub Tested
POST /collection/:collection_name/query /query Tested
POST /collection/:collection_name/count /count Tested
POST /collection/:collection_name/doc /insert Pending
GET /collection/:collection_name/doc/:id /get Pending
GET /collection/:collection_name/page/:page_total /getpage Pending
PUT /collection/:collection_name/doc/:id /update Pending
DELETE /collection/:collection_name/doc/:id /delete Pending
GET /collection/:collection_name/approxdoccount /approxdoccount Pending
POST /collection/:collection_name/index/:index_path /index Pending
POST /collection/:collection_name/unindex/:index_path /unindex Pending
GET /collection/:collection_name/indexes /indexes Pending
POST /sync /sync Pending
POST /shutdown /shutdown Pending
POST /dump /dump Pending

I did set the updates in bold.

In other hand after closer review I realized that error messages are being returned as plain text and I would like to propose, as part of this api change, to upgrade all api results to JSON objects. See my initial proposal below... I kind of like verbose api errors but the idea is to keep it as simple as possible...

{
  "collection": "<COLLECTION_NAME>",
  "operation": "<OPERATION>",
  "error": "<ERROR MESSAGE>"
}

Sample:

{
  "collection": "test_collection",
  "operation": "delete",
  "error": "Collection test_collection does not exist"
}

Thoughts?

@prods
Copy link
Collaborator

prods commented Aug 1, 2017

Adding. Below is a better list of proposed error objects.

Collection Operations

{
  "collection": "test_collection",
  "operation": "delete",
  "error": "Collection does not exist"
}

Document Operations

{
  "collection": "test_collection",
  "document": 19238928282,
  "operation": "update",
  "error": "Document does not exist"
}

Engine Operations

{
  "operation": "dump",
  "error": "Specified Path cannot be found"
}

The error messages are not necessarily real. I added them in order to provide some examples.

Let me know.

@prods
Copy link
Collaborator

prods commented Aug 1, 2017

Ok, just another update. I already created all the routes and successfully completed a first round of testing. Below you will see a few more changes in the proposal. As before I marked in bold the changes and included some possible response payloads for functions that are silent today.

Method Routes Original Routes Request Response Status
GET /collections /all N/A [ "c_name1", "c_name2", ... ] Created
POST /collection/:collection_name /create N/A { "done" : true } (Proposal) Created
PUT /collection/:collection_name/rename/:new_collection_name /rename N/A { "done" : true } (Proposal) Created
DELETE /collection/:collection_name /drop N/A { "done" : true } (Proposal) Created
POST /collection/:collection_name/scrub /scrub N/A { "done" : true } (Proposal) Created
POST /collection/:collection_name/query /query { "c" : [...] } { "<id>" : {}, ... } Created
POST /collection/:collection_name/count /count { "c" : [...] } { "count" : 100 } (Proposal) Created
POST /collection/:collection_name/doc /insert { } { "id" : "" } (Proposal) Created
GET /collection/:collection_name/doc/:id /get N/A { } Created
GET /collection/:collection_name/page/:page/of/:total /getpage N/A { "<id>" : {}, ... } Created
PUT /collection/:collection_name/doc/:id /update { } { "done" : true } (Proposal) Created
DELETE /collection/:collection_name/doc/:id /delete N/A { "done" : true } (Proposal) Created
GET /collection/:collection_name/count/approx /approxdoccount N/A { "count" : 100 } (Proposal) Created
POST /collection/:collection_name/index /index { "path" : "" } { "done" : true } (Proposal) Created
DELETE /collection/:collection_name/index /unindex { "path" : "" } { "done" : true } (Proposal) Created
GET /collection/:collection_name/indexes /indexes N/A { "", ... } Created
POST /sync /sync N/A N/A Created
POST /shutdown /shutdown N/A { "done" : true } (Proposal) Created
POST /dump /dump { "destination : "" } { "done": true } (Proposal) Created

I will continue the testing and I will keep you posted. Please feel free to post any idea, comment and/or concern...

@HouzuoGuo
Copy link
Owner

Looks really good so far!

@prods
Copy link
Collaborator

prods commented Aug 12, 2017

Just an update. This continues to move forward. Finding time here and there to complete it. You can take a pick at My Fork Branch. The API is now fully working and I am close to jump into documentation and final testing. I will keep you posted.

@prods
Copy link
Collaborator

prods commented Aug 12, 2017

Forgot mention, please make sure to run it using the new boolean use-new-api argument. This will enable the new API. You will find a postman collection in the doc folder that you can use to test-drive it.

@HouzuoGuo
Copy link
Owner

Many thanks for your effort, the code is looking very good already.

Would you mind creating a wiki article describing the usage of new API as well? Afterwards please merge the changes whenever you're ready.

I must admit that I have sounded quite lazy so far, not offering much help, my excuse would be that this project https://github.com/HouzuoGuo/laitos is keeping me busy at the moment.

@prods
Copy link
Collaborator

prods commented Aug 15, 2017

No problem, already started working on the documentation. Will definitely add a Wiki article with all the details before merging the changes. Thanks! I will take a look at the other project, looks interesting from what I briefly saw.

@HouzuoGuo
Copy link
Owner

thank You!
coincidentally, according to cloc program, the laitos project has 12345 lines of go code as of now 😄

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

No branches or pull requests

5 participants