[PATCH] DataIntegrityProblemException: Primitive must be part of the dataset

[gaben]

What steps will reproduce the problem?

1. Set taggingpreset.validator=true
2. Create two connected ways
3. Select way1, the common node and way2
4. Create a turn restriction between them
5. In the newly opened relation editor click on the header

What is the expected result?

Changing the restriction type possible.

What happens instead?

Exception appears.

Please provide any additional information below. Attach a screenshot if possible.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2023-09-21 20:54:28 +0200 (Thu, 21 Sep 2023)
Build-Date:2023-09-22 01:30:58
Revision:18841
Relative:URL: ^/trunk

Identification: JOSM/1.5 (18841 hu) Windows 10 64-Bit
OS Build number: Windows 10 Pro for Workstations 2009 (19045)
Memory Usage: 2396 MB / 7266 MB (1433 MB allocated, but free)
Java version: 1.8.0_382-b05, Azul Systems, Inc., OpenJDK 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920×1200 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1200
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1250
System property sun.jnu.encoding: Cp1250
Locale info: hu_HU
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Dicedtea-web.bin.name=javaws.exe, -Dicedtea-web.bin.location=C:\Program Files\IcedTeaWeb\WebStart\bin\javaws.exe]
Dataset consistency test: No problems found


=== REPORTED CRASH DATA ===
BugReportExceptionHandler#handleException:
No data collected.

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: AWT-EventQueue-1 (33) of JOSM (development version)
org.openstreetmap.josm.data.osm.DataIntegrityProblemException: Primitive must be part of the dataset: {Relation id=-63 version=0 VT [vonal -610, pont -25375, vonal -611]}
	at org.openstreetmap.josm.data.osm.OsmPrimitive.checkDataset(OsmPrimitive.java:198)
	at java.util.ArrayList.forEach(ArrayList.java:1259)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.createPanel(TaggingPreset.java:450)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.showDialog(TaggingPreset.java:604)
	at org.openstreetmap.josm.gui.dialogs.properties.PresetListPanel$LabelMouseAdapter.mouseClicked(PresetListPanel.java:42)
	at java.awt.AWTEventMulticaster.mouseClicked(AWTEventMulticaster.java:270)
	at java.awt.Component.processMouseEvent(Component.java:6542)
	at javax.swing.JComponent.processMouseEvent(JComponent.java:3324)
	at java.awt.Component.processEvent(Component.java:6304)
	at java.awt.Container.processEvent(Container.java:2239)
	at java.awt.Component.dispatchEventImpl(Component.java:4889)
	at java.awt.Container.dispatchEventImpl(Container.java:2297)
	at java.awt.Component.dispatchEvent(Component.java:4711)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4904)
	at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4544)
	at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4476)
	at java.awt.Container.dispatchEventImpl(Container.java:2283)
	at java.awt.Window.dispatchEventImpl(Window.java:2746)
	at java.awt.Component.dispatchEvent(Component.java:4711)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:760)
	at java.awt.EventQueue.access$500(EventQueue.java:97)
	at java.awt.EventQueue$3.run(EventQueue.java:709)
	at java.awt.EventQueue$3.run(EventQueue.java:703)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:84)
	at java.awt.EventQueue$4.run(EventQueue.java:733)
	at java.awt.EventQueue$4.run(EventQueue.java:731)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:730)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

[gaben]

The taggingpreset.validator=true setting causing the issue.

[gaben]

The root cause is that the relation does not exist in the dataset at that point. It becomes part of the dataset once the relation editor is closed with the OK button. At least that's how I understand it.

[taylor.smock]

I could have sworn I fixed that. I guess not, or it is a slightly different problem with a slightly different set of steps to reproduce.

[gaben]

The only solution I can think of at the moment is:

src/org/openstreetmap/josm/gui/tagging/presets/TaggingPreset.java

@@ -444 +444 @@
             GuiHelper.setEnabledRec(itemPanel, false);
         }
 
-        if (selected.size() == 1 && Boolean.TRUE.equals(USE_VALIDATOR.get())) {
+        if (selected.size() == 1 && Boolean.TRUE.equals(USE_VALIDATOR.get()) && selected.stream().map(OsmPrimitive::getDataSet)
+                .noneMatch(Objects::isNull)) {
             // Fail early -- validateAsync requires the primitive(s) to be part of a dataset. Failing later in validateAsync ''does not'' give us
-            // a usable stack trace. See #21829 for details.
-            selected.forEach(OsmPrimitive::checkDataset);
+            // a usable stack trace. See #21829 and #23196 for details.
             itemGuiSupport.addListener((source, key, newValue) ->
                     TaggingPresetValidation.validateAsync(selected.iterator().next(), validationLabel, getChangedTags()));
         }

The drawback is that it disables the async validation for newly created objects, which is the point of the feature... Please do not commit this, I'm just thinking loudly.

[taylor.smock]

The stacktrace if we get rid of the selected.forEach(OsmPrimitive::checkDataset) is

org.openstreetmap.josm.data.osm.DataIntegrityProblemException: Primitive must be part of the dataset: {Relation id=-142 version=0 VT [way -648, node 236132035, way -649]}
	at org.openstreetmap.josm.data.osm.OsmPrimitive.checkDataset(OsmPrimitive.java:198)
	at org.openstreetmap.josm.data.osm.OsmPrimitive.referrers(OsmPrimitive.java:702)
	at org.openstreetmap.josm.data.osm.OsmPrimitive.getReferrers(OsmPrimitive.java:728)
	at org.openstreetmap.josm.data.osm.OsmPrimitive.getReferrers(OsmPrimitive.java:734)
	at org.openstreetmap.josm.data.osm.FilterModel.getAffectedPrimitives(FilterModel.java:442)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPresetValidation.applyChangedTags(TaggingPresetValidation.java:81)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPresetValidation.validateAsync(TaggingPresetValidation.java:45)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.lambda$createPanel$2(TaggingPreset.java:454)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItemGuiSupport.lambda$fireItemValueModified$1(TaggingPresetItemGuiSupport.java:182)
	at org.openstreetmap.josm.tools.ListenerList.fireEvent(ListenerList.java:155)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItemGuiSupport.fireItemValueModified(TaggingPresetItemGuiSupport.java:182)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.createPanel(TaggingPreset.java:464)
	at org.openstreetmap.josm.gui.tagging.presets.TaggingPreset.showDialog(Unknown Source)
	at org.openstreetmap.josm.gui.dialogs.properties.PresetListPanel$LabelMouseAdapter.mouseClicked(PresetListPanel.java:42)

The "best" solution is to add the relation to the dataset, so something like

src/org/openstreetmap/josm/gui/tagging/presets/TaggingPreset.java

@@ -545 +548 @@
             if (r.isMultipolygon() && r != calculated) {
                 r.setMembers(RelationSorter.sortMembersByConnectivity(r.getMembers()));
             }
+            // Add the new relation to the dataset
+            final Relation toEdit;
+            if (r != calculated) {
+                final DataSet ds = primitives.stream().map(OsmPrimitive::getDataSet).filter(Objects::nonNull).findFirst()
+                        .orElse(OsmDataManager.getInstance().getEditDataSet());
+                UndoRedoHandler.getInstance().add(new AddPrimitivesCommand(Collections.singletonList(r.save()), ds));
+                toEdit = (Relation) ds.getPrimitiveById(r);
+            } else {
+                toEdit = r;
+            }
             SwingUtilities.invokeLater(() -> RelationEditor.getEditor(
-                    MainApplication.getLayerManager().getEditLayer(), r, members).setVisible(true));
+                    MainApplication.getLayerManager().getEditLayer(), toEdit, members).setVisible(true));
         }
         if (!primitives.isEmpty()) {
             DataSet ds = primitives.iterator().next().getDataSet();

would work.

[gaben]

I like it and at the same time feels weird to use. Probably just I need to get used to it.

[taylor.smock]

In 18866/josm:

Fix #23196: DataIntegrityProblemException: Primitive must be part of the dataset

It turns out that FilterModel doesn't care whether a primitive's referrers are
in the dataset or not.

This additionally adds a non-regression test and modifies WindowMocker to cover
some more headless exceptions. This additionally lets us stop mocking
ExtendedDialog#setupDialog.

[gaben]

That's even more simple, great!

[taylor.smock]

In 18867/josm:

See #16567: Update to JUnit 5

This adds the @TaggingPresets annotation used by r18866 (see #23196). The
annotation tries to only initialize the presets when the current preset list does
not have the same hashCode as the default preset list. In order for this to work,
TaggingPresets#getTaggingPresets had to return a List (the method contract
has not changed), which calculates the hashCode in a deterministic manner.

[taylor.smock]

Ticket #23247 has been marked as a duplicate of this ticket.

[gaben]

fix typo

[taylor.smock]

Ticket retargeted after milestone deleted