Modify

Opened 15 months ago

Closed 15 months ago

Last modified 13 months ago

#23165 closed defect (fixed)

Cannot assign a changesetId > 0 to a new primitive. Value of changesetId is 1

Reported by: jochen.hebbrecht Owned by: jochen.hebbrecht
Priority: normal Milestone: 23.11
Component: Core Version:
Keywords: Cc:

Description

Hi,

We have a problem opening a PBF in JOSM as of version 18700 (released 20th of March). Up & until that day, we always used the PBF plugin to open PBF files in JOSM.

As of version 18700, we can use the built-in support now for opening PBF files. This is thanks to this commit: https://github.com/JOSM/josm/commit/fbcc55340eba456e816989bca5b81015126e4c03

However, a PBF file can suddenly no longer be opened because of:

2023-08-30 22:25:43.922 SEVERE: java.lang.IllegalStateException: Cannot assign a changesetId > 0 to a new primitive. Value of changesetId is 1
java.lang.IllegalStateException: Cannot assign a changesetId > 0 to a new primitive. Value of changesetId is 1
	at org.openstreetmap.josm.data.osm.AbstractPrimitive.setChangesetId(AbstractPrimitive.java:303)
	at org.openstreetmap.josm.io.OsmPbfReader.setOsmPrimitiveData(OsmPbfReader.java:841)
	at org.openstreetmap.josm.io.OsmPbfReader.parseWay(OsmPbfReader.java:672)
	at org.openstreetmap.josm.io.OsmPbfReader.parsePrimitiveGroup(OsmPbfReader.java:449)

The file itself could perfectly be opened with an older version of JOSM. I'm trying to see if this correlates to a bug on your side, or a data issue on our side which is now exposed thanks to recent code changes?
As an intermediate conclusion, it seems that the built-in support is dealing PBF reading differently.

I have 2 questions:

  • Looking at the stacktrace, it seems that JOSM is treating the 'failing' element as "new", meaning the ID is negative. However, in our PBF, there's not a single OSM element containing a negative ID. Can somebody explain me why JOSM converts positive ids into negative ids while reading the PBF?
  • This is an example of what we have as data in our PBF
    <node id="2488317" version="1" timestamp="2023-08-24T00:00:00Z" uid="1" user="userName" changeset="1" lat="43.7325063" lon="7.4188489">
      <tag k="some_key" v="some_value/">
    </node>
    
    It seems that JOSM no longer likes the fact that we add changeset=1 there. Any reason this check in your codebase? https://github.com/JOSM/josm/blob/master/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java#L301. Why can't the changeset value not be 1 for example?

Thanks for your help!

Jochen

Attachments (1)

24165.osm.pbf (434 bytes ) - added by jochen.hebbrecht 15 months ago.
Example PBF file which throws the exception when trying to open in JOSM

Download all attachments as: .zip

Change History (15)

comment:1 by jochen.hebbrecht, 15 months ago

Type: taskdefect

comment:2 by taylor.smock, 15 months ago

Owner: changed from team to jochen.hebbrecht
Status: newneedinfo

Can you please either (a) upload a copy of the PBF or (b) link to a copy of the PBF?

I think I just need to move the changeset id set to after the location where I set the version, but I'd like to double check with a file known to cause the issue.

by jochen.hebbrecht, 15 months ago

Attachment: 24165.osm.pbf added

Example PBF file which throws the exception when trying to open in JOSM

comment:3 by jochen.hebbrecht, 15 months ago

Hi Taylor,

Thanks for your fast feedback!
I've just uploaded a very small dataset (in the format of PBF file) which you can use to simulate the problem.

Jochen

comment:4 by taylor.smock, 15 months ago

It looks like I may have messed up the id parsing code. I'm calling intValue instead of longValue in some locations. I've got a fix, but I'm working on a test for it.
Specifically, I'm using the equivalent of

<osm version="0.6">
  <node id="21" version="1" timestamp="2023-08-24T00:00:00Z" uid="1" user="user" changeset="1" lat="0" lon="1"/>
  <node id="9223372036854775806" version="1" timestamp="2023-08-24T00:00:00Z" uid="1" user="user" changeset="1" lat="-1" lon="-1"/>
  <way id="9223372036854775806" version="1" timestamp="2023-08-24T00:00:00Z" uid="2147483647" user="user" changeset="2147483648">
    <nd ref="10"/>
    <nd ref="21"/>
    <tag k="highway" v="road"/>
  </way>
  <relation id="9223372036854775806" version="2147483647" timestamp="2023-08-24T00:00:00Z" uid="1" user="user" changeset="1"/>
</osm>

Yes, I know that I've got a bad nd ref="10" in there. I don't know how JOSM will react to that, since my test is having issues with node ids that large, and may have other problems that I may need to fix.

comment:5 by jochen.hebbrecht, 15 months ago

Thanks Taylor. That makes sense what you say! Now I understand from where the negative ids are coming, because I could not find them in the PBF (and now I also understand why JOSM expects that changeset=0 when the id is negative - because a negative id is treated as a new map element).

The test case you described above seems good! Hoping to have your changes in pretty soon! :-). Is there somewhere I can maybe support/assist you?

comment:6 by taylor.smock, 15 months ago

Not really. If you've got other corner cases that don't work, that would be helpful, since I'm looking at the code anyway.

There is some kind of parsing issue going on whereby really large numbers are failing. An edge case, but something I should fix while I'm thinking about it.

It looks like I might be messing up the array values somewhere. Maybe. I'll have to check, but I'm not seeing what I expected in the byte array that is getting decoded/decompressed for the id.

FTR, the node id array is [0x2A, 0xD2, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01]
The two node ids I'm expecting are 21 and 9223372036854775806.

0x2A -> 42 -> decodeZigZag -> 21 (good)
0xD2, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01 -> -46 -> -23 (bad -- there is a long overflow issue here)

For the latter, this is what I expected (using other people's code):
9223372036854775806 -> encodeZigZag -> 18446744069414584323 -> convertToByteArray -> [0x83, 0x80, 0x80, 0x80, 0xF0, 0xFF, 0xFF, 0xFF, 0xFF, 0x01]

Difference:

- 0xD2, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01
+ 0x83, 0x80, 0x80, 0x80, 0xF0, 0xFF, 0xFF, 0xFF, 0xFF, 0x01

comment:7 by jochen.hebbrecht, 15 months ago

For the moment, I don't have any corner cases. So it's really this specific case that fails. As a workaround, we could use changeset="0" in our PBF to walk around the bug :-). But it would be good to have it properly fixed of course.

Last edited 15 months ago by jochen.hebbrecht (previous) (diff)

comment:8 by taylor.smock, 15 months ago

I need to correct myself:

0xD2, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x01 -> -46 (18446744073709551570 unsigned)-> -23 (9223372036854775785 unsigned; add 21 due to delta encoding).

So the encoding is correct. I was borking the bitshift (>> -> >>>).

comment:9 by taylor.smock, 15 months ago

Resolution: fixed
Status: needinfoclosed

In 18826/josm:

Fix #23165: Cannot assign a changesetId > 0 to a new primitive

There were two problems:

  1. Using Number.intValue instead of Number.longValue
  2. Using >> instead of >>> (bit shift)

comment:10 by taylor.smock, 15 months ago

Milestone: 23.09

comment:11 by taylor.smock, 15 months ago

In 18827/josm:

See #23165: Use test file with smaller (Integer.MAX_VALUE) changeset id

comment:12 by jochen.hebbrecht, 15 months ago

Thanks! I've tested version 18826 and I can confirm I no longer have the problem! :-)

comment:13 by taylor.smock, 14 months ago

Milestone: 23.0923.10

Ticket retargeted after milestone deleted

comment:14 by taylor.smock, 13 months ago

Milestone: 23.1023.11

Ticket retargeted after milestone deleted

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain jochen.hebbrecht.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.