search

NeotomaDB / api_nodetest

node.js and express implementation for the Neotoma API
MIT License
2 stars 0 forks source link

This change results in new tests in `neotoma_test` and the addition o… #44

Closed SimonGoring closed 7 years ago

SimonGoring commented 7 years ago

…f limit/offset terms in the queries.

It also results in the addition of new functionality, wherein an empty query re-directs users to the swagger documentation.

I want to check with @stryker-m that the behaviour of sending the user to swagger for an empty result (rather than returning a limited set) makes sense to him.

stryker-m commented 7 years ago

@SimonGoring I agree with change to return an empty object that matches the model for the requested data. This seems more helpful to the user than a null value. I think redirecting to the swagger api documention page for malformed requests is too disruptive. I'd prefer a response object similar to what's in v1: {"success": 0 , "message": "50a is not a valid siteid". We could add a "more_info": neotoam_swagger_url property to the response object instead of redirecting.

SimonGoring commented 7 years ago

This doesn't redirect on an error, only if there is nothing in the query string. I'm going to reject the change request and merge the pull request.

stryker-m commented 7 years ago

As I understand it, the redirect is still problematic. If an app makes a call to the api that returns no data, the api passes back the swagger landing page? Do I understand the behavior correctly? Why would we respond to an application with an interface designed for use by a person?

SimonGoring commented 7 years ago

No, only if the call comes in to the empty endpoint:

/v2/data/geopolitical/

without any query terms or identifiers.

stryker-m commented 7 years ago

That doesn't address the last question, why return html?

Similar case: request: http://api.neotomadb.org/v1/data/ Current response: {"success":0,"message":"Please pass a resource name."} Recommended enhancement to advertise the API documentation: request: /v2/data/geopolitical/ {"success":0, message":"Please pass a query parameter or identifier", "more-info": "Consult the API documentation at 'http://api-dev.neotomadb.org/api-docs/' "} or simply {"success":0, "message": "Please submit a valid request"}

The API should return json that can be handled by an app. Sending back HTML breaks that contract.

SimonGoring commented 7 years ago

I just raised an issue (#45) to deal with this separately. It looks like you can have the swagger docs replying with JSON content, which would give the machine what it wants as well. I'm working on the occurrence endpoint & then will address.