GIScience / ohsome-api

API for analysing OpenStreetMap history data
https://api.ohsome.org
GNU Affero General Public License v3.0
47 stars 8 forks source link

/ratio request gives nullpointer exception on missing filter2 parameter #26

Closed bonaparten closed 4 years ago

bonaparten commented 4 years ago

This request triggers a NullPointerException curl 'https://api.ohsome.org/v1/elements/count/ratio/groupBy/boundary?bboxes=8.684692%2C49.407669%2C8.688061%2C49.410310&filter=building=residential&format=json'

The problem is probably an empty result.

tyrasd commented 4 years ago

AFAICS, the problem is the missing filter2 parameter (see https://docs.ohsome.org/ohsome-api/v1/endpoints.html#post--elements-(aggregation)-ratio). The NPE is triggered at InputProcessingUtils.java#L388, but the null variable is coming from the filter2 variable set in ElementsRequestExecutor.java#L1377.

tyrasd commented 4 years ago

btw: here is a slightly simpler way to trigger this bug (/groupBy/geometry/ does not matter in this case):

curl 'https://api.ohsome.org/v1/elements/count/ratio?bboxes=8.684692%2C49.407669%2C8.688061%2C49.410310&filter=building=residential'
FabiKo117 commented 4 years ago

I've wrapped the value of the filter2 parameter now in inputProcessor.createEmptyStringIfNull(), but now the filter throws the following error message:

Invalid filter syntax. Please look at the additional info and examples about the filter parameter at https://docs.ohsome.org/ohsome-api. Detailed error message: line 1, column 1: whitespaces, not, (, KEY_STRING (a-z, 0-9, : or -), \", type or geometry expected, EOF encountered.

If I remember correctly the OSHDB accepts empty filters, right? So it basically does not apply any filtering in that case then. Do you think that's also feasible to implement in the filter library then @tyrasd or should I handle this in the ohsome API explicitly? edit: This is how it was handled thus far at least with the former keys-values syntax

tyrasd commented 4 years ago

I think the filter library should handle empty strings as "no filter". Can you open an issue for it?

Independently, maybe we should still not allow empty filters in the ohsome API, perhaps? I think it should be two slightly different questions:

  1. Should the ohsome API allow empty filter/filter2 parameters? example: …/elements/count?…&filter=&…
  2. What should the ohsome API do if the filter parameter is absent?

I guess the answer to question 1 is debatable: IMHO one could think especially for the aggregating endpoints that applying a transformation such as length, area (to some extent count as well) does make no sense, if one does not limit the stuff to measure to something which can actually be measured by that measure (e.g. length for "highway" objects). But sure, also the point can be made that counts of "all elements" could also make sense in some scenarios, especially when one may work with oshdb files generated not from OpenStreetMap data, but some other source (i.d.k., maybe a POI-only dataset).

But the answer to question 2, I think, is clearer: In order to prevent issues with incomplete queries that are executed by mistake (e.g. when one simply forgets to provide a filter or filter2 parameter), it is necessary that the ohsome API raises an exception if no filter (and/or filter2 if applicable) is provided. What do you think?

FabiKo117 commented 4 years ago

Issue is created 👍

Ok, I thought that the current /ratio implementation does NOT allow an empty filter, but apparently it does. The idea behind that is that you need to define some filter to compute a ratio against, otherwise if filter and filter2 are not set, the result is always just 1. I guess this somehow got lost during the implementation of the filter parameter, but I will add it now again, so that an exception is thrown if there is a null or empty filter parameter for a /ratio request.

IMO: 1) Yes, it should be possible to allow to set no specific OSM tag or type filter. Especially the filter2 parameter should be allowed to be null or empty, if e.g. someone wants to compute the ratio of polygonal features to all available features. For length and area requests, this is more debatable yes if it would make sense there.. I'd say we could still allow it there for now. 2) We could define a special syntax in the filter parameter that means "no filter" to avoid that this behavior can be caused accidentally and throw an exception, if it's completely absent (like it is already the case when a spatial filter is missing). Initially, I thought of something like filter=* but that could also be a bit misleading as it could be interpreted as "set a filter on everything" which should then always return 0 actually. Maybe something like filter=empty?

tyrasd commented 4 years ago

Maybe something like filter=empty?

why not just use the empty string for that? like in this example: https://api.ohsome.org/v1/elements/count…&filter=&…

FabiKo117 commented 4 years ago

Has already been fixed with this PR.