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

Amenity editor cannot handle 64 bit node ids? #3

Closed
Maturion opened this Issue Nov 5, 2016 · 8 comments

Comments

Projects
None yet
7 participants
@Maturion

Maturion commented Nov 5, 2016

Apparently, Amenity Editor cannot deal with node ids larger than 32 bit.

Here is a changeset that originally concerned a mosque in Morocco, but affected a node belonging to the coastline of the Volga river in Central Russia:
https://www.openstreetmap.org/changeset/43411845
The node that should have been changed is this (4483045489) one, the one that was actually changed is that (188078193) one, however.

Looking at the respective node ids, it seems like Amenity Editor cannot handle node ids larger than 32 bit:

4483045489 - 2^32 = 188078193

This would explain why a completely unrelated node was affected by the changeset mentioned above. I haven't looked at the code yet, but if this is true, Amenity Editor is not really safe for making live edits anymore.

@simonpoole

This comment has been minimized.

Show comment
Hide comment
@simonpoole

simonpoole Nov 5, 2016

Seems as if the issue may be related to the use of Types.INTEGER in https://github.com/grundid/amenity-editor/blob/master/src/main/java/org/osmsurround/ae/dao/NodeUpdate.java the rest of the code seems to be 64bit ready.

PS: while fixing this issue is of highest concern, there are a few other things that should be looked at if the editor is supposed to stay in operation.

simonpoole commented Nov 5, 2016

Seems as if the issue may be related to the use of Types.INTEGER in https://github.com/grundid/amenity-editor/blob/master/src/main/java/org/osmsurround/ae/dao/NodeUpdate.java the rest of the code seems to be 64bit ready.

PS: while fixing this issue is of highest concern, there are a few other things that should be looked at if the editor is supposed to stay in operation.

@Maturion Maturion changed the title from Amenity editor cannot handle with 64 bit node ids? to Amenity editor cannot handle 64 bit node ids? Nov 5, 2016

@scaidermern

This comment has been minimized.

Show comment
Hide comment
@scaidermern

scaidermern Nov 5, 2016

Another amenity editor changeset where a node has been moved a huge distance: https://www.openstreetmap.org/changeset/43411665
The node that should have been edited: https://www.openstreetmap.org/node/4483039394
The node that has been edited instead: https://www.openstreetmap.org/node/188072098

And more affected changesets, just created a few minutes ago:
https://www.openstreetmap.org/changeset/43421842
https://www.openstreetmap.org/changeset/43421755
https://www.openstreetmap.org/changeset/43422128

Note: I've already wrote user osm10005 a friendly message to stop using amenity editor for updating the position of existing amenities. Not sure if he/she reads the message, though.

scaidermern commented Nov 5, 2016

Another amenity editor changeset where a node has been moved a huge distance: https://www.openstreetmap.org/changeset/43411665
The node that should have been edited: https://www.openstreetmap.org/node/4483039394
The node that has been edited instead: https://www.openstreetmap.org/node/188072098

And more affected changesets, just created a few minutes ago:
https://www.openstreetmap.org/changeset/43421842
https://www.openstreetmap.org/changeset/43421755
https://www.openstreetmap.org/changeset/43422128

Note: I've already wrote user osm10005 a friendly message to stop using amenity editor for updating the position of existing amenities. Not sure if he/she reads the message, though.

@rickmastfan67

This comment has been minimized.

Show comment
Hide comment
@rickmastfan67

rickmastfan67 Nov 5, 2016

Another example where the node isn't moved. Pretty sure there isn't a mosque here in the middle of nowhere in Missouri, USA.
https://www.openstreetmap.org/node/188895896/history

rickmastfan67 commented Nov 5, 2016

Another example where the node isn't moved. Pretty sure there isn't a mosque here in the middle of nowhere in Missouri, USA.
https://www.openstreetmap.org/node/188895896/history

@tomhughes

This comment has been minimized.

Show comment
Hide comment
@tomhughes

tomhughes Nov 5, 2016

The OAuth client ID being used used by the public install at http://ae.osmsurround.org/ has now been invalidated and all existing tokens revoked so it can no longer make edits - a new OAuth ID will need to be created once this bug has been fixed.

tomhughes commented Nov 5, 2016

The OAuth client ID being used used by the public install at http://ae.osmsurround.org/ has now been invalidated and all existing tokens revoked so it can no longer make edits - a new OAuth ID will need to be created once this bug has been fixed.

@grundid

This comment has been minimized.

Show comment
Hide comment
@grundid

grundid Nov 16, 2016

Owner

sorry guys. the amenity editor was designed with long nodeIds from the start. unfortunately the conversion into 32-bit integers happened during the write operations into the db.

Owner

grundid commented Nov 16, 2016

sorry guys. the amenity editor was designed with long nodeIds from the start. unfortunately the conversion into 32-bit integers happened during the write operations into the db.

@grundid grundid closed this Nov 16, 2016

@stephan75

This comment has been minimized.

Show comment
Hide comment
@stephan75

stephan75 Nov 17, 2016

@simonpoole ... what other things you have in mind?

Minor like osm data is not under cc-by-sa any more?

Or big things like using leaflet or openlayers 3 ?

stephan75 commented Nov 17, 2016

@simonpoole ... what other things you have in mind?

Minor like osm data is not under cc-by-sa any more?

Or big things like using leaflet or openlayers 3 ?

@simonpoole

This comment has been minimized.

Show comment
Hide comment
@simonpoole

simonpoole Nov 17, 2016

@stephan75 how is the question related to the amenity-editor?

simonpoole commented Nov 17, 2016

@stephan75 how is the question related to the amenity-editor?

@stephan75

This comment has been minimized.

Show comment
Hide comment
@stephan75

stephan75 Nov 17, 2016

@simonpoole
I read your comment in this isue from 12 days ago ... there you mention that the editor should be improved more ...

stephan75 commented Nov 17, 2016

@simonpoole
I read your comment in this isue from 12 days ago ... there you mention that the editor should be improved more ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment