Data query response testing bugs

[fjugipe]

Some bugs in the tool I found in the data query response testing

1. Extent generation for the queries and general approach

   • The points_inside generate a diagonal corridor which starts quite close (but not) from the bottom left of the original API extent
     • The tests expect a response of 200 i.e. data should be found. But for example with large extents containing a single station far away from most of the data points (eq. Antartica), this will most likely never return data and will most certainly fail
     • One solution would be to generate a vertical corridor starting FROM the bottom left and ending at the top left of the original API extent, but this case does not necessarily work with position queries since the most south-est station might not also be the most eastern and the test currently takes the bottom left of the generated corridor for the position query.
   • Queries using points outside make valid queries but assert that a 400 would be correct in the case of data not found, I think the correct expected response would be 404 if the query is otherwise correct.

2. Area queries

   • f"?coords=POLYGON({inside_polygon}) should be f"?coords=POLYGON(({inside_polygon})) for it to be correct
   • WKT Polygons should be closed, so need to add the first point also as the last one in inside_polygon and outside_polygon for area queries
     

[havardf]

I will look into these. The issue with getting something else than 200 for requests inside the extent I might not have time to fix at the moment, since a proper fix might require more time than I currently have for this.

[havardf]

@fjugipe Regarding the 400 issue, the correct solution here might be a pragmatic one, to just accept both, or maybe even all 4XX requests. However, according to https://docs.ogc.org/DRAFTS/20-024.html#http-status-codes I interpret 400 to be correct.

As in

• 400: "The server cannot or will not process the request due to an apparent client error. For example, a query parameter had an incorrect value."
• 404: "The requested resource does not exist on the server. For example, a path parameter had an incorrect value."

And a data query request outside the extent is caused by wrong value to the query param coords. Unless of course they mean the name of the query parameter is wrong, not the value it is set to...

[fjugipe]

The specification truly leaves possibilities for different interpretations in many cases.. 😆 In this case, a pragmatic approach seems reasonable, since this tool is a profile validator, not a functionality tester per se.

In my opinion the difference is between "cannot or will not process" and "resource not found" and my interpretation is to follow the general http protocol elements like they are usually done.

Which would mean that if the query itself is erroneous (for example querying /area?coods=... with an incorrect query parameter). Should lead that the API doesn't even try to process the query and would result to 400, telling the user that your query is wrong and it will not work even if you change the parameter values.

in the other hand if the query is correct and the server is able to process it (using a POLYGON which doesn't have any data), but nothing is found would result to 404, which would indicate to change the query parameter values and try again.

[havardf]

@fjugipe Hm, looking at the code now, we already accept as correct any status code that is 4XX. So I am now not sure where you see the assert on exactly 400?

[fjugipe]

Sorry if I was not clear enough, there is no exactly 400 or 404 assert and that is what I wanted to indicate in the original issue description. It is just a nuance, but as I am planning to add this tool to https://github.com/EUMETNET/e-soh pipeline I wanted to point it out.

Currently when testing with the points outside, meaning that the query itself is correct, the validator will say that any status code that is 4XX is correct. But in my opinion in such cases it should assert that only 404 is correct, since there is nothing wrong with the query, just no data to be found.

if queries.outside != "":
                outside = requests.get(queries.outside, timeout=30)
                if outside.status_code < 400 or outside.status_code >= 500:
                    errors += f"Expected status code 4xx for query {queries.outside}; Got {outside.status_code}\n"

If the query would be somehow incorrect, like missing required query parameters, then it should assert that only 400 is correct.

[havardf]

Sorry if I was not clear enough, there is no exactly 400 or 404 assert and that is what I wanted to indicate in the original issue description. It is just a nuance, but as I am planning to add this tool to https://github.com/EUMETNET/e-soh pipeline I wanted to point it out.

Currently when testing with the points outside, meaning that the query itself is correct, the validator will say that any status code that is 4XX is correct. But in my opinion in such cases it should assert that only 404 is correct, since there is nothing wrong with the query, just no data to be found.

if queries.outside != "":
                outside = requests.get(queries.outside, timeout=30)
                if outside.status_code < 400 or outside.status_code >= 500:
                    errors += f"Expected status code 4xx for query {queries.outside}; Got {outside.status_code}\n"

If the query would be somehow incorrect, like missing required query parameters, then it should assert that only 400 is correct.

Right, ok. Your issue started a big discussion on our regular profile meeting about errors. I think we need to include some requirements in the profile itself about this. I have created an issue here: EUMETNET/metocean-edr-profile#49. Please comment there, would be great if we could really converge on some simple, but clear rules on this.