REST-ful API notes

[broofa]

[First of all, a huge thank you for hosting this site over the years! I've gotten a tremendous amount of value from it and very much appreciate all the hard work you've put into it. I'm actually really pleased to discover the new API and, even better, that you're running on a JS stack that I'm pretty familiar with. Here's hoping I can help contribute at some point.]

So, regarding the new API - speaking specifically of the REST-ful JSON api which is what I expect to be using - there are a few things that stand out as being a bit "unexpected". For your consideration...

• [ ] Endpoints like /search, /download, and /motorguide that query the server for data ("pulling" data) should only support the GET method. It looks like these endpoints currently support both the GET and POST methods. However, with the exception of /metadata, they are all documented as using POST in the Swagger docs. At a minimum, the documentation should prefer the GET method, and document how the search params can be supplied as query parameters rather than what request body.
• [ ] Search parameters should be case-insensitive. I was surprised that "estes" didn't return results, but "Estes" did.
• [ ] /getrockets should just be rockets
• [ ] Endpoints should not overload fetching of public and restricted data. E.g. There should be separate endpoints for publically accessible rockets vs. a users's personal rockets. Among other things, it makes it easier to set up authentication based on the resource path. (See comments below)
• [ ] A truly "REST-ful" api does not have verbs in the resource paths. For example
  • /search would be better expressed as /motors (e.g. GET /motors)
  • /getrockets would be better expressed as /rockets (e.g. GET /rockets to fetch public rocket data)
  • ... and GET /users/:userId/rockets (requires authentication) for a user's personal collection of rockets
  • ... and instead of /saverockets, callers could POST /users/:userId/rockets to add a rocket, or PUT /users/:userId/rockets/:rocketId to update an existing rocket.
  • ... and so on.

I hope this doesn't come across as pushy. I'm sure there are good reasons for the shape of the current API. I'm mostly just hoping to convey a sense of what I think most developers familiar with REST would expect to find here.

Again, thank you for all your hard work!

[JohnCoker]

Support for both GET and POST is mostly a matter of reducing code. (All requests handle both for simplicity.) I don't see a reason to not support POST, but don't feel strongly about it for the JSON API.

Fields like the manufacturer are enums, and the /metadata endpoint is called to enumerate the values. Case insensitivity isn't the only thing here, there are also manufacturer abbreviations and aliases ("Cesaroni", "CTI").

I agree that ideally the structure of the JSON API should change to be more modern, but this should be added in a backwards-compatible way, at least in terms of not breaking the XML API. I had intended to add a /api/v2 path that was totally modern at some point, but just never got around to it. Note that the existing support is in api_v1.js.

The XML APIs have been in place since 2008 and are used by external programs such as OpenRocket and RockSim so should not change. (It's also of course used by the phone app.) There is a test in /spec/api which can be used to verify that nothing breaks. (It's not currently run by Travis, but must be run manually with the local server running.)