From 88a48a5fdb2558af99f3c23baacf75f21b600cbd Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Tue, 11 Oct 2022 10:46:43 -0600 Subject: [PATCH] Fix #22440: NPE in MapWithAIInfo#getConflationUrl Signed-off-by: Taylor Smock --- .../data/mapwithai/MapWithAIInfo.java | 24 +++++- .../data/mapwithai/MapWithAIInfoTest.java | 79 +++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java index 3a29d3f..6c597af 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfo.java @@ -453,7 +453,29 @@ public class MapWithAIInfo extends return getNonConflatedUrl(); } StringBuilder sb = new StringBuilder(); - sb.append(conflationUrl.replace("{id}", this.id)); + if (this.conflationUrl.contains("{id}") && this.id != null) { + sb.append(conflationUrl.replace("{id}", this.id)); + } else if (this.conflationUrl.contains("{id}")) { + // We need to trigger synchronization. This means that the current download + // may won't be conflated. + // But this should automatically correct the behavior for the next attempt. + final MapWithAILayerInfo mwli = MapWithAILayerInfo.getInstance(); + mwli.load(false, () -> { + Optional defaultLayer = mwli.getAllDefaultLayers().stream().filter(this::equals) + .findFirst(); + if (defaultLayer.isPresent()) { + this.id = defaultLayer.get().id; + } else { + MapWithAIInfo newInfo = mwli.getAllDefaultLayers().stream() + .filter(layer -> Objects.equals(this.url, layer.url) && Objects.nonNull(layer.id)) + .findFirst().orElse(this); + this.id = newInfo.id; + } + }); + return getNonConflatedUrl(); + } else { + sb.append(conflationUrl); + } List parametersString = getConflationParameterString(); if (!parametersString.isEmpty()) { diff --git a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java index a20fb7d..f8b96f3 100644 --- a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java +++ b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java @@ -1,7 +1,11 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.mapwithai.data.mapwithai; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; import javax.json.Json; import javax.json.JsonArray; @@ -15,15 +19,21 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Random; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.tools.Logging; +import mockit.Mock; +import mockit.MockUp; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Warning; @@ -117,4 +127,73 @@ class MapWithAIInfoTest { EqualsVerifier.forClass(MapWithAIInfo.class).suppress(Warning.NONFINAL_FIELDS) .withOnlyTheseFields("url", "sourceType").usingGetClass().verify(); } + + /** + * Non-regression test for #22440: NPE in + * {@link MapWithAIInfo#getConflationUrl}. This is caused by the + * {@link MapWithAIInfo#getId()} being {@code null}. + */ + @Test + void testNonRegression22440UpdateFallback() { + TestUtils.assumeWorkingJMockit(); + MapWithAIInfo info = new MapWithAIInfo("22440", "https://test.example", null); + info.setConflationUrl("https://test.example/{id}"); + info.setConflation(true); + MapWithAILayerInfo.getInstance().clear(); + MapWithAILayerInfo.getInstance().add(info); + AtomicBoolean updateCalled = new AtomicBoolean(); + new MockUp() { + @Mock + public void load(boolean fastFail, MapWithAILayerInfo.FinishListener listener) { + updateCalled.set(true); + listener.onFinish(); + } + }; + assertDoesNotThrow(info::getUrlExpanded); + MainApplication.worker.submit(() -> { + /* Sync thread */ }); + GuiHelper.runInEDTAndWait(() -> { + /* Sync thread */ }); + assertTrue(updateCalled.get()); + assertEquals(1, MapWithAILayerInfo.getInstance().getLayers().size()); + assertSame(info, MapWithAILayerInfo.getInstance().getLayers().get(0)); + } + + /** + * Non-regression test for #22440: NPE in + * {@link MapWithAIInfo#getConflationUrl}. This is caused by the + * {@link MapWithAIInfo#getId()} being {@code null}. + */ + @Test + void testNonRegression22440Update() { + TestUtils.assumeWorkingJMockit(); + MapWithAIInfo info = new MapWithAIInfo("22440", "https://test.example", null); + info.setConflationUrl("https://test.example/{id}"); + info.setConflation(true); + MapWithAILayerInfo.getInstance().clear(); + MapWithAILayerInfo.getInstance().add(info); + AtomicBoolean updateCalled = new AtomicBoolean(); + new MockUp() { + @Mock + public void load(boolean fastFail, MapWithAILayerInfo.FinishListener listener) { + updateCalled.set(true); + listener.onFinish(); + } + + @Mock + public List getAllDefaultLayers() { + return Collections.singletonList(new MapWithAIInfo("22440Update", "https://test.example", "22")); + } + }; + assertNull(info.getId()); + assertDoesNotThrow(info::getUrlExpanded); + MainApplication.worker.submit(() -> { + /* Sync thread */ }); + GuiHelper.runInEDTAndWait(() -> { + /* Sync thread */ }); + assertTrue(updateCalled.get()); + assertEquals(1, MapWithAILayerInfo.getInstance().getLayers().size()); + assertSame(info, MapWithAILayerInfo.getInstance().getLayers().get(0)); + assertEquals("22", info.getId()); + } }