Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug in …/groupBy/tag and …/groupBy/key endpoints when key/values do not exist in keytables #291

Merged
merged 6 commits into from
Feb 9, 2023

Conversation

tyrasd
Copy link
Member

@tyrasd tyrasd commented Jan 11, 2023

Fixes a bug/crash in …/groupBy/tag requests triggered when using non-existing groupByKey or groupByValues are used in a query.

To Reproduce

curl -X GET "https://api.ohsome.org/v1/elements/count/groupBy/tag?bboxes=8.67%2C49.39%2C8.71%2C49.42&filter=type%3Away%20and%20building%3D*&format=json&groupByKey=building&groupByValues=DoesNotExist1&time=2023-01-01" -H  "accept: application/json"

This query is requesting results grouped by the tag building and value DoesNotExist1 (which does not exits in the keytables). The result, however contains building=yes as the groupByObject. When performing a query with two non-existing groupByValues (e.g. DoesNotExist1,DoesNotExist2), the results includes two entries with remainder as groupByObject. When requesting three or more non-existing groupByValues, the query crashes and a 500 server error is returned.

Similar bugs occur when performing a query with a non-existing tag key as the groupByKey.

The Problem and Fix

The issue is caused by a mistake in the code, where consecutive negative tag numbers are used as keyInt/valuesInt for OSM tags which do not exist in the keytables, but the hardcoded value -1 is also used to represent the remainder portion of the result. This is causing the mixup between the value DoesNotExist1 and the superfluous remainder entry in the example result above. Further, the negative pseudo-indices started at 0 which is also the index of the most common tag value for the corresponding tag. This is causing the mixup between the value DoesNotExist2 and the result entry for the value yes in the example above. The crashes for any additional non-existing values are caused by a null pointer exception where the (still) non-existing pseudo-indices are attempted to be converted back to strings using the tag translator.

This is fixed by starting the negative pseudo-indices at position -2 where confusion with the -1 value for the remainder entry is possible and using the input values to resolve the pseudo-indices back to strings.

Checklist

@tyrasd tyrasd added the bug Something isn't working label Jan 11, 2023
@tyrasd tyrasd marked this pull request as draft January 11, 2023 16:23
@tyrasd tyrasd force-pushed the bugfix/unknown-groupbytag-objects branch from 1cb126e to 07bbdaa Compare January 11, 2023 16:38
@tyrasd tyrasd changed the title fix bug in groupByTag endpoints when key/values do not exist in keytables fix bug in …/groupBy/tag and …/groupBy/key endpoints when key/values do not exist in keytables Jan 11, 2023
@tyrasd tyrasd force-pushed the bugfix/unknown-groupbytag-objects branch from 1a053c2 to 2ed312c Compare January 11, 2023 17:22
@tyrasd tyrasd marked this pull request as ready for review January 11, 2023 17:24
@tyrasd tyrasd added the waiting for review This pull request needs a code review label Jan 11, 2023
Copy link
Member

@rtroilo rtroilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

@tyrasd tyrasd merged commit 3fa7ecb into master Feb 9, 2023
@tyrasd tyrasd deleted the bugfix/unknown-groupbytag-objects branch February 9, 2023 16:26
@tyrasd tyrasd removed the waiting for review This pull request needs a code review label Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants