You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[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!
The text was updated successfully, but these errors were encountered:
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.)
[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...
/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 theGET
method, and document how the search params can be supplied as query parameters rather than what request body./getrockets
should just berockets
/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)GET /users/:userId/rockets
(requires authentication) for a user's personal collection of rockets/saverockets
, callers couldPOST /users/:userId/rockets
to add a rocket, orPUT /users/:userId/rockets/:rocketId
to update an existing rocket.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!
The text was updated successfully, but these errors were encountered: