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 #23676: NumberFormatException in MapillaryImageUtils.getKey #232

Merged
merged 3 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ plugins {
id("application")
id("com.diffplug.spotless") version "6.25.0"
id("com.github.ben-manes.versions") version "0.51.0"
id("com.github.spotbugs") version "6.0.12"
id("com.github.spotbugs") version "6.0.17"
id("net.ltgt.errorprone") version "3.1.0"
id("org.openstreetmap.josm") version "0.8.2"
id("org.sonarqube") version "4.3.0.3225"
Expand Down Expand Up @@ -57,24 +57,24 @@ java.targetCompatibility = JavaVersion.VERSION_17

val versions = mapOf(
"awaitility" to "4.2.1",
"errorprone" to "2.26.1",
"errorprone" to "2.28.0",
"jdatepicker" to "1.3.4",
"jmockit" to "1.49",
"junit" to "5.10.2",
"pmd" to "6.42.0",
"spotbugs" to "4.8.4",
"wiremock" to "2.35.1"
"spotbugs" to "4.8.6",
"wiremock" to "3.6.0"
)

dependencies {
errorprone("com.google.errorprone:error_prone_core:${versions["errorprone"]}")
testImplementation ("org.openstreetmap.josm:josm-unittest:SNAPSHOT"){ isChanging = true }
testImplementation("com.github.tomakehurst:wiremock-jre8:${versions["wiremock"]}")
testImplementation("org.wiremock:wiremock:${versions["wiremock"]}")

testImplementation("org.junit.jupiter:junit-jupiter-api:${versions["junit"]}")
testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${versions["junit"]}")
// This can be removed once JOSM drops all JUnit4 support. Nothing remaining in Mapillary uses JUnit4.
testImplementation("org.junit.jupiter:junit-jupiter-params:${versions["junit"]}")
// This can be removed once JOSM drops all JUnit4 support. Nothing remaining in Mapillary uses JUnit4.
testImplementation("org.junit.vintage:junit-vintage-engine:${versions["junit"]}")

testImplementation("org.awaitility:awaitility:${versions["awaitility"]}")
Expand Down
1 change: 1 addition & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ plugin.main.version=18940
plugin.compile.version=18940
# The datepicker plugin is currently in the source tree. TODO fix
plugin.requires=apache-commons
plugin.minimum.java.version=17

# Character encoding of Gradle files
systemProp.file.encoding=utf-8
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
<Plugin-Link>${plugin.link}</Plugin-Link>
<Plugin-Icon>${plugin.icon}</Plugin-Icon>
<Plugin-Canloadatruntime>${plugin.canloadatruntime}</Plugin-Canloadatruntime>
<Plugin-Minimum-Java-Version>${plugin.minimum.java.version}</Plugin-Minimum-Java-Version>
</manifestEntries>
</archive>
</configuration>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary;

import java.util.stream.Stream;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public class MapObjectLayerAction extends JosmAction {

public MapObjectLayerAction() {
super(
I18n.tr(ACTION_NAME), MapillaryPlugin.LOGO.setSize(ImageSizes.DEFAULT), I18n.tr(DESCRIPTION),
Shortcut.registerShortcut("mapillary:trafficSignLayer", ACTION_NAME, KeyEvent.CHAR_UNDEFINED, Shortcut.NONE),
I18n.tr(ACTION_NAME), MapillaryPlugin.LOGO.setSize(ImageSizes.DEFAULT), I18n.tr(DESCRIPTION), Shortcut
.registerShortcut("mapillary:trafficSignLayer", ACTION_NAME, KeyEvent.CHAR_UNDEFINED, Shortcut.NONE),
false, "mapillary:trafficSignLayer", true);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.actions;

import static org.openstreetmap.josm.tools.I18n.marktr;
import static org.openstreetmap.josm.tools.I18n.tr;

import java.awt.event.ActionEvent;
import java.awt.event.KeyEvent;

import org.openstreetmap.josm.actions.JosmAction;
import org.openstreetmap.josm.gui.MainApplication;
import org.openstreetmap.josm.gui.layer.Layer;
Expand All @@ -11,12 +17,6 @@
import org.openstreetmap.josm.tools.ImageProvider.ImageSizes;
import org.openstreetmap.josm.tools.Shortcut;

import java.awt.event.ActionEvent;
import java.awt.event.KeyEvent;

import static org.openstreetmap.josm.tools.I18n.marktr;
import static org.openstreetmap.josm.tools.I18n.tr;

/**
* An action to create a layer for Mapillary point objects (not traffic signs)
*/
Expand All @@ -27,8 +27,8 @@ public class MapPointObjectLayerAction extends JosmAction {
"Displays the layer displaying the map point objects detected by Mapillary");

public MapPointObjectLayerAction() {
super(tr(ACTION_NAME), MapillaryPlugin.LOGO.setSize(ImageSizes.DEFAULT), tr(TOOLTIP),
Shortcut.registerShortcut("mapillary:pointFeaturesLayer", tr(ACTION_NAME), KeyEvent.CHAR_UNDEFINED, Shortcut.NONE),
super(tr(ACTION_NAME), MapillaryPlugin.LOGO.setSize(ImageSizes.DEFAULT), tr(TOOLTIP), Shortcut
.registerShortcut("mapillary:pointFeaturesLayer", tr(ACTION_NAME), KeyEvent.CHAR_UNDEFINED, Shortcut.NONE),
false, "mapillary:pointFeaturesLayer", true);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.command;

import static org.openstreetmap.josm.tools.I18n.tr;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.command;

import java.util.Collection;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.data.mapillary;

import java.util.Map;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.data.mapillary;

import org.openstreetmap.josm.data.osm.event.IDataSelectionListener;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public final class MapillaryFilterDialog extends ToggleDialog
private static final String[] TIME_LIST = { tr("Years"), tr("Months"), tr("Days") };

private static final long[] TIME_FACTOR = new long[] { 31_536_000_000L, // = 365 * 24 * 60 * 60 * 1000 = number of
// ms in a year
// ms in a year
2_592_000_000L, // = 30 * 24 * 60 * 60 * 1000 = number of ms in a month
86_400_000 // = 24 * 60 * 60 * 1000 = number of ms in a day
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.gui.layer;

import java.awt.Color;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.gui.widget;

import java.util.ArrayList;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.io.download;

import org.openstreetmap.gui.jmapviewer.Tile;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.io.download;

import org.openstreetmap.gui.jmapviewer.Tile;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.spi.preferences;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,17 @@ public static long getKey(@Nullable IPrimitive image, boolean ignoreId) {
}
// This should always be a parseable integer according to API docs. By not checking that all characters are
// digits, we save 55.6 MB of allocations in the test area during download.
if (image.hasKey(ImageProperties.ID.toString())) {
final var id = Long.parseLong(image.get(ImageProperties.ID.toString()));
if (id > 0) {
image.setOsmId(id, 1);
try {
if (image.hasKey(ImageProperties.ID.toString())) {
final var id = Long.parseLong(image.get(ImageProperties.ID.toString()));
if (id > 0) {
image.setOsmId(id, 1);
}
return id;
}
return id;
} catch (NumberFormatException numberFormatException) {
// This does actually happen; see #23734
Logging.error(numberFormatException);
}
}
return 0;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.utils;

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.utils;

import org.openstreetmap.josm.plugins.mapillary.cache.Caches;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.command;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.data.mapillary;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.gui.imageinfo;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ class MapillaryDownloader implements AfterEachCallback {
@Override
public void afterEach(ExtensionContext context) {
AtomicBoolean done = new AtomicBoolean();
new org.openstreetmap.josm.plugins.mapillary.gui.workers.MapillaryNodesDownloader(nodes -> done.set(true), 1).execute();
new org.openstreetmap.josm.plugins.mapillary.gui.workers.MapillaryNodesDownloader(nodes -> done.set(true),
1).execute();
Awaitility.await().atMost(Durations.FIVE_SECONDS).untilTrue(done);
done.set(false);
new org.openstreetmap.josm.plugins.mapillary.gui.workers.MapillarySequenceDownloader("", chunks -> done.set(true)).execute();
new org.openstreetmap.josm.plugins.mapillary.gui.workers.MapillarySequenceDownloader("",
chunks -> done.set(true)).execute();
Awaitility.await().atMost(Durations.FIVE_SECONDS).untilTrue(done);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.testutils.annotations;

import static org.junit.jupiter.api.Assertions.fail;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.testutils.annotations;

import java.lang.annotation.Documented;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,6 @@

import javax.imageio.ImageIO;

import org.junit.jupiter.api.extension.AfterAllCallback;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeAllCallback;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.platform.commons.support.AnnotationSupport;
import org.openstreetmap.josm.TestUtils;
import org.openstreetmap.josm.plugins.mapillary.spi.preferences.IMapillaryUrls;
import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryConfig;
import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryUrls;
import org.openstreetmap.josm.tools.Utils;

import com.github.tomakehurst.wiremock.WireMockServer;
import com.github.tomakehurst.wiremock.client.WireMock;
import com.github.tomakehurst.wiremock.common.TextFile;
Expand All @@ -64,6 +52,17 @@
import jakarta.json.JsonReader;
import jakarta.json.JsonString;
import jakarta.json.JsonValue;
import org.junit.jupiter.api.extension.AfterAllCallback;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeAllCallback;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.platform.commons.support.AnnotationSupport;
import org.openstreetmap.josm.TestUtils;
import org.openstreetmap.josm.plugins.mapillary.spi.preferences.IMapillaryUrls;
import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryConfig;
import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryUrls;
import org.openstreetmap.josm.tools.Utils;

/**
* Mock Mapillary API calls
Expand Down Expand Up @@ -226,13 +225,10 @@ public Response transform(Response response, ServeEvent serveEvent) {
if (request.queryParameter("image_ids").isPresent()) {
// Not implemented currently since I don't know if I need to split on `,` or `%2C`
final List<String> imageIds = request.queryParameter("image_ids").values().stream()
.map(str -> str.split(",", -1)).flatMap(Stream::of).filter(Objects::nonNull)
.toList();
final List<TextFile> imageText = imageIds.stream()
.map(image -> Path.of(TestUtils.getTestDataRoot(), "__files", "api", "v4", "responses", "graph", image + ".json"))
.map(Path::toUri)
.map(TextFile::new)
.toList();
.map(str -> str.split(",", -1)).flatMap(Stream::of).filter(Objects::nonNull).toList();
final List<TextFile> imageText = imageIds.stream().map(image -> Path.of(TestUtils.getTestDataRoot(),
"__files", "api", "v4", "responses", "graph", image + ".json")).map(Path::toUri)
.map(TextFile::new).toList();
// We need to get the actual bytes prior to returning, so we need to read the files.
final String body = imageText.stream().map(TextFile::readContentsAsString)
.collect(Collectors.joining(",", "{\"data\":[", "]}"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapillary.testutils.annotations;

import java.security.Permission;
Expand Down
Loading