From cde1e3f60ac58fcb48d628ff46b4f5a11204f07b Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Wed, 10 Jun 2020 16:02:48 -0600 Subject: [PATCH] Split some classes into their own files and fix some sonarlint issues. Signed-off-by: Taylor Smock --- .../BoundingBoxMapWithAIDownloader.java | 3 +- .../data/mapwithai/MapWithAICategory.java | 104 +++++++++++++ .../data/mapwithai/MapWithAIInfo.java | 146 ++---------------- .../data/mapwithai/MapWithAILayerInfo.java | 65 ++++---- .../data/mapwithai/MapWithAIType.java | 43 ++++++ .../validation/tests/RoutingIslandsTest.java | 2 +- .../mapwithai/AddMapWithAIPanel.java | 10 +- .../MapWithAIDefaultLayerTableModel.java | 2 +- .../mapwithai/MapWithAIProvidersPanel.java | 9 +- .../io/mapwithai/ESRISourceReader.java | 11 +- .../io/mapwithai/MapWithAISourceReader.java | 16 +- .../MapWithAISourceReaderTestIT.java | 4 +- .../data/mapwithai/MapWithAIInfoTest.java | 7 +- .../io/mapwithai/ESRISourceReaderTest.java | 16 +- 14 files changed, 241 insertions(+), 197 deletions(-) create mode 100644 src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAICategory.java create mode 100644 src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIType.java diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java index ef53e47..f617769 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java @@ -21,6 +21,7 @@ import org.openstreetmap.josm.io.OsmApiException; import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.plugins.mapwithai.io.mapwithai.OsmReaderCustom; import org.openstreetmap.josm.tools.HttpClient; @@ -96,7 +97,7 @@ class BoundingBoxMapWithAIDownloader extends BoundingBoxDownloader { String contentType = this.activeConnection.getResponse().getHeaderField("Content-Type"); if (contentType.contains("text/xml")) { ds = OsmReaderCustom.parseDataSet(source, progressMonitor, true); - } else if (MapWithAIInfo.MapWithAIType.ESRI_FEATURE_SERVER.equals(this.info.getSourceType())) { + } else if (MapWithAIType.ESRI_FEATURE_SERVER == this.info.getSourceType()) { ds = GeoJSONReader.parseDataSet(source, progressMonitor); if (info.getReplacementTags() != null) { GetDataRunnable.replaceKeys(ds, info.getReplacementTags()); diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAICategory.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAICategory.java new file mode 100644 index 0000000..15e9e10 --- /dev/null +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAICategory.java @@ -0,0 +1,104 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.plugins.mapwithai.data.mapwithai; + +import static org.openstreetmap.josm.tools.I18n.marktr; + +import java.util.Collections; +import java.util.Comparator; +import java.util.EnumMap; +import java.util.Locale; +import java.util.Map; + +import javax.swing.ImageIcon; + +import org.openstreetmap.josm.data.sources.ISourceCategory; +import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets; +import org.openstreetmap.josm.tools.ImageProvider; +import org.openstreetmap.josm.tools.ImageProvider.ImageSizes; + +/** + * The category for a MapWithAI source + * + * @author Taylor Smock + * + */ +public enum MapWithAICategory implements ISourceCategory { + + BUILDING("data/closedway", "buildings", marktr("Buildings")), + HIGHWAY("presets/transport/way/way_road", "highways", marktr("Roads")), + ADDRESS("presets/misc/housenumber_small", "addresses", marktr("Addresses")), + POWER("presets/power/pole", "pole", marktr("Power")), PRESET("dialogs/search", "presets", marktr("Presets")), + FEATURED("presets/service/network-wireless.svg", "featured", marktr("Featured")), + OTHER(null, "other", marktr("Other")); + + private static final Map> iconCache = Collections + .synchronizedMap(new EnumMap<>(ImageSizes.class)); + + private final String category; + private final String description; + private final String icon; + + MapWithAICategory(String icon, String category, String description) { + this.category = category; + this.icon = icon == null || icon.trim().isEmpty() ? "mapwithai" : icon; + this.description = description; + } + + @Override + public final String getCategoryString() { + return category; + } + + @Override + public final String getDescription() { + return description; + } + + @Override + public final ImageIcon getIcon(ImageSizes size) { + return iconCache.computeIfAbsent(size, x -> Collections.synchronizedMap(new EnumMap<>(MapWithAICategory.class))) + .computeIfAbsent(this, x -> ImageProvider.get(x.icon, size)); + } + + public static MapWithAICategory fromString(String s) { + for (MapWithAICategory category : MapWithAICategory.values()) { + if (category.getCategoryString().equals(s)) { + return category; + } + } + if (s != null && !s.trim().isEmpty()) { + // fuzzy match + String tmp = s.toLowerCase(Locale.ROOT); + for (MapWithAICategory type : MapWithAICategory.values()) { + if (tmp.contains(type.getDescription().toLowerCase(Locale.ROOT)) + || type.getDescription().toLowerCase(Locale.ROOT).contains(tmp)) { + return type; + } + } + // Check if it matches a preset + if (TaggingPresets.getPresetKeys().stream().map(String::toLowerCase) + .anyMatch(m -> tmp.contains(m) || m.contains(tmp))) { + return PRESET; + } + } + return OTHER; + } + + public static class DescriptionComparator implements Comparator { + + @Override + public int compare(MapWithAICategory o1, MapWithAICategory o2) { + return (o1 == null || o2 == null) ? 1 : o1.getDescription().compareTo(o2.getDescription()); + } + } + + @Override + public MapWithAICategory getDefault() { + return OTHER; + } + + @Override + public MapWithAICategory getFromString(String s) { + return fromString(s); + } +} 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 c08f271..cd69d2d 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 @@ -1,16 +1,14 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.mapwithai.data.mapwithai; -import static org.openstreetmap.josm.tools.I18n.marktr; import static org.openstreetmap.josm.tools.I18n.tr; import java.io.StringReader; +import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; -import java.util.EnumMap; import java.util.HashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -20,145 +18,20 @@ import javax.json.Json; import javax.json.JsonArray; import javax.json.JsonObject; import javax.json.stream.JsonParser; -import javax.swing.ImageIcon; import org.openstreetmap.josm.data.StructUtils.StructEntry; import org.openstreetmap.josm.data.imagery.ImageryInfo; import org.openstreetmap.josm.data.imagery.ImageryInfo.ImageryBounds; import org.openstreetmap.josm.data.imagery.Shape; -import org.openstreetmap.josm.data.sources.ISourceCategory; -import org.openstreetmap.josm.data.sources.ISourceType; import org.openstreetmap.josm.data.sources.SourceInfo; import org.openstreetmap.josm.data.sources.SourcePreferenceEntry; -import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo.MapWithAIPreferenceEntry; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.CheckParameterUtil; -import org.openstreetmap.josm.tools.ImageProvider; -import org.openstreetmap.josm.tools.ImageProvider.ImageSizes; import org.openstreetmap.josm.tools.Logging; public class MapWithAIInfo extends - SourceInfo { - - /** - * Type of MapWithAI entry - */ - public enum MapWithAIType implements ISourceType { - FACEBOOK("facebook"), THIRD_PARTY("thirdParty"), ESRI("esri"), ESRI_FEATURE_SERVER("esriFeatureServer"); - - private final String typeString; - - MapWithAIType(String typeString) { - this.typeString = typeString; - } - - @Override - public String getTypeString() { - return typeString; - } - - public static MapWithAIType fromString(String s) { - for (MapWithAIType type : MapWithAIType.values()) { - if (type.getTypeString().equals(s)) { - return type; - } - } - return null; - } - - @Override - public MapWithAIType getDefault() { - return THIRD_PARTY; - } - - @Override - public MapWithAIType getFromString(String s) { - return fromString(s); - } - } - - public enum MapWithAICategory implements ISourceCategory { - - BUILDING("data/closedway", "buildings", marktr("Buildings")), - HIGHWAY("presets/transport/way/way_road", "highways", marktr("Roads")), - ADDRESS("presets/misc/housenumber_small", "addresses", marktr("Addresses")), - POWER("presets/power/pole", "pole", marktr("Power")), PRESET("dialogs/search", "presets", marktr("Presets")), - FEATURED("presets/service/network-wireless.svg", "featured", marktr("Featured")), - OTHER(null, "other", marktr("Other")); - - private static final Map> iconCache = Collections - .synchronizedMap(new EnumMap<>(ImageSizes.class)); - - private final String category; - private final String description; - private final String icon; - - MapWithAICategory(String icon, String category, String description) { - this.category = category; - this.icon = icon == null || icon.trim().isEmpty() ? "mapwithai" : icon; - this.description = description; - } - - @Override - public final String getCategoryString() { - return category; - } - - @Override - public final String getDescription() { - return description; - } - - @Override - public final ImageIcon getIcon(ImageSizes size) { - return iconCache - .computeIfAbsent(size, x -> Collections.synchronizedMap(new EnumMap<>(MapWithAICategory.class))) - .computeIfAbsent(this, x -> ImageProvider.get(x.icon, size)); - } - - public static MapWithAICategory fromString(String s) { - for (MapWithAICategory category : MapWithAICategory.values()) { - if (category.getCategoryString().equals(s)) { - return category; - } - } - if (s != null && !s.trim().isEmpty()) { - // fuzzy match - String tmp = s.toLowerCase(Locale.ROOT); - for (MapWithAICategory type : MapWithAICategory.values()) { - if (tmp.contains(type.getDescription().toLowerCase(Locale.ROOT)) - || type.getDescription().toLowerCase(Locale.ROOT).contains(tmp)) { - return type; - } - } - // Check if it matches a preset - if (TaggingPresets.getPresetKeys().stream().map(String::toLowerCase) - .anyMatch(m -> tmp.contains(m) || m.contains(tmp))) { - return PRESET; - } - } - return OTHER; - } - - public static class DescriptionComparator implements Comparator { - - @Override - public int compare(MapWithAICategory o1, MapWithAICategory o2) { - return (o1 == null || o2 == null) ? 1 : o1.getDescription().compareTo(o2.getDescription()); - } - } - - @Override - public MapWithAICategory getDefault() { - return OTHER; - } - - @Override - public MapWithAICategory getFromString(String s) { - return fromString(s); - } - } + SourceInfo { private List categories; private JsonArray parameters; @@ -176,7 +49,6 @@ public class MapWithAIInfo extends * {@link #MapWithAIInfo(ImageryInfo) MapWithAIInfo constructor} * {@link #equalsPref(MapWithAIPreferenceEntry) equalsPref method} **/ - public static class MapWithAIPreferenceEntry extends SourcePreferenceEntry { @StructEntry String parameters; @@ -298,7 +170,7 @@ public class MapWithAIInfo extends setEulaAcceptanceRequired(e.eula); if (e.parameters != null) { try (JsonParser parser = Json.createParser(new StringReader(e.parameters))) { - if (parser.hasNext() && JsonParser.Event.START_ARRAY.equals(parser.next())) { + if (parser.hasNext() && JsonParser.Event.START_ARRAY == parser.next()) { setParameters(parser.getArray()); } } @@ -341,7 +213,7 @@ public class MapWithAIInfo extends setConflationUrl(e.conflationUrl); if (e.conflationParameters != null) { try (JsonParser parser = Json.createParser(new StringReader(e.conflationParameters))) { - if (parser.hasNext() && JsonParser.Event.START_ARRAY.equals(parser.next())) { + if (parser.hasNext() && JsonParser.Event.START_ARRAY == parser.next()) { setConflationParameters(parser.getArray()); } } @@ -411,11 +283,11 @@ public class MapWithAIInfo extends } public void setParameters(JsonArray parameters) { - this.parameters = parameters; + this.parameters = parameters != null ? Json.createArrayBuilder(parameters).build() : null; } public JsonArray getParameters() { - return parameters; + return parameters != null ? Json.createArrayBuilder(parameters).build() : null; } /** @@ -472,7 +344,7 @@ public class MapWithAIInfo extends StringBuilder sb = new StringBuilder(); if (url != null && !url.trim().isEmpty()) { sb.append(url); - if (MapWithAIType.ESRI_FEATURE_SERVER.equals(sourceType)) { + if (MapWithAIType.ESRI_FEATURE_SERVER == sourceType) { if (!url.endsWith("/")) { sb.append('/'); } @@ -522,7 +394,7 @@ public class MapWithAIInfo extends * @param parameters Set the conflation parameters */ public void setConflationParameters(JsonArray parameters) { - this.conflationParameters = parameters; + this.conflationParameters = parameters != null ? Json.createArrayBuilder(parameters).build() : null; } /** @@ -531,7 +403,7 @@ public class MapWithAIInfo extends * @param categories The categories to set */ public void setAdditionalCategories(List categories) { - this.categories = categories; + this.categories = categories != null ? new ArrayList<>(categories) : null; } /** diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java index 84f4b0c..b678d72 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java @@ -198,12 +198,8 @@ public class MapWithAILayerInfo { allDefaultLayers.clear(); defaultLayers.addAll(newLayers); for (MapWithAIInfo layer : newLayers) { - if (MapWithAIInfo.MapWithAIType.ESRI.equals(layer.getSourceType())) { - try (ESRISourceReader reader = new ESRISourceReader(layer)) { - allDefaultLayers.addAll(reader.parse()); - } catch (IOException e) { - Logging.error(e); - } + if (MapWithAIType.ESRI == layer.getSourceType()) { + allDefaultLayers.addAll(parseEsri(layer)); } else { allDefaultLayers.add(layer); } @@ -222,30 +218,45 @@ public class MapWithAILayerInfo { listener.onFinish(); } } - } - /** - * Build the mapping of unique ids to {@link ImageryInfo}s. - * - * @param lst input list - * @param idMap output map - */ - private static void buildIdMap(List lst, Map idMap) { - idMap.clear(); - Set notUnique = new HashSet<>(); - for (MapWithAIInfo i : lst) { - if (i.getId() != null) { - if (idMap.containsKey(i.getId())) { - notUnique.add(i.getId()); - Logging.error("Id ''{0}'' is not unique - used by ''{1}'' and ''{2}''!", i.getId(), i.getName(), - idMap.get(i.getId()).getName()); - continue; - } - idMap.put(i.getId(), i); + /** + * Parse an Esri layer + * + * @param layer The layer to parse + * @return The Feature Servers for the ESRI layer + */ + private Collection parseEsri(MapWithAIInfo layer) { + try (ESRISourceReader esriReader = new ESRISourceReader(layer)) { + return esriReader.parse(); + } catch (IOException e) { + Logging.error(e); } + return Collections.emptyList(); } - for (String i : notUnique) { - idMap.remove(i); + + /** + * Build the mapping of unique ids to {@link ImageryInfo}s. + * + * @param lst input list + * @param idMap output map + */ + private void buildIdMap(List lst, Map idMap) { + idMap.clear(); + Set notUnique = new HashSet<>(); + for (MapWithAIInfo i : lst) { + if (i.getId() != null) { + if (idMap.containsKey(i.getId())) { + notUnique.add(i.getId()); + Logging.error("Id ''{0}'' is not unique - used by ''{1}'' and ''{2}''!", i.getId(), i.getName(), + idMap.get(i.getId()).getName()); + continue; + } + idMap.put(i.getId(), i); + } + } + for (String i : notUnique) { + idMap.remove(i); + } } } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIType.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIType.java new file mode 100644 index 0000000..1516b6c --- /dev/null +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIType.java @@ -0,0 +1,43 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.plugins.mapwithai.data.mapwithai; + +import org.openstreetmap.josm.data.sources.ISourceType; + +/** + * Type of MapWithAI entry + * + * @author Taylor Smock + */ +public enum MapWithAIType implements ISourceType { + FACEBOOK("facebook"), THIRD_PARTY("thirdParty"), ESRI("esri"), ESRI_FEATURE_SERVER("esriFeatureServer"); + + private final String typeString; + + MapWithAIType(String typeString) { + this.typeString = typeString; + } + + @Override + public String getTypeString() { + return typeString; + } + + public static MapWithAIType fromString(String s) { + for (MapWithAIType type : MapWithAIType.values()) { + if (type.getTypeString().equals(s)) { + return type; + } + } + return null; + } + + @Override + public MapWithAIType getDefault() { + return THIRD_PARTY; + } + + @Override + public MapWithAIType getFromString(String s) { + return fromString(s); + } +} diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/RoutingIslandsTest.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/RoutingIslandsTest.java index c142d04..1fc634e 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/RoutingIslandsTest.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/RoutingIslandsTest.java @@ -106,7 +106,7 @@ public class RoutingIslandsTest extends Test { .message(tr("MapWithAI (experimental)"), marktr("Routable way not connected to other ways")) .build()); } else if ((ValidatorPrefHelper.PREF_OTHER.get() || ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() - || !Severity.OTHER.equals(SEVERITY_MAP.get(ROUTING_ISLAND))) && !isBeforeUpload) { + || Severity.OTHER != SEVERITY_MAP.get(ROUTING_ISLAND)) && !isBeforeUpload) { if (way.hasKey(HIGHWAY) && !IGNORE_TAGS_HIGHWAY.contains(way.get(HIGHWAY))) { potentialHighways.add(way); } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/AddMapWithAIPanel.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/AddMapWithAIPanel.java index f577a38..938b67b 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/AddMapWithAIPanel.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/AddMapWithAIPanel.java @@ -36,7 +36,7 @@ import org.openstreetmap.josm.gui.preferences.imagery.HeadersTable; import org.openstreetmap.josm.gui.widgets.JosmTextArea; import org.openstreetmap.josm.gui.widgets.JosmTextField; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; -import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo.MapWithAIType; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Pair; @@ -59,10 +59,10 @@ class AddMapWithAIPanel extends JPanel { private JSpinner minimumCacheExpiry; private JComboBox minimumCacheExpiryUnit; private TimeUnit currentUnit; - private MapWithAIInfo.MapWithAIType type; + private MapWithAIType type; private MapWithAIInfo info; - private JComboBox typeBox; + private JComboBox typeBox; protected AddMapWithAIPanel(LayoutManager layout) { super(layout); @@ -120,8 +120,8 @@ class AddMapWithAIPanel extends JPanel { add(new JLabel(tr("{0} Enter name for this source", "3.")), GBC.eol()); add(name, GBC.eol().fill(GBC.HORIZONTAL)); add(new JLabel(tr("{0} What is the type of this source?", "4.")), GBC.eol()); - typeBox = new JComboBox<>(MapWithAIInfo.MapWithAIType.values()); - typeBox.setSelectedItem(MapWithAIInfo.MapWithAIType.THIRD_PARTY); + typeBox = new JComboBox<>(MapWithAIType.values()); + typeBox.setSelectedItem(MapWithAIType.THIRD_PARTY); typeBox.addItemListener(l -> { type = (MapWithAIType) typeBox.getSelectedItem(); notifyListeners(); diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIDefaultLayerTableModel.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIDefaultLayerTableModel.java index 42ef032..926fd20 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIDefaultLayerTableModel.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIDefaultLayerTableModel.java @@ -14,8 +14,8 @@ import java.util.stream.Stream; import javax.swing.table.DefaultTableModel; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAICategory; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; -import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo.MapWithAICategory; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAILayerInfo; /** diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java index f3696de..a8b2706 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java @@ -58,9 +58,10 @@ import org.openstreetmap.josm.gui.preferences.imagery.ImageryProvidersPanel; import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.gui.widgets.FilterField; import org.openstreetmap.josm.gui.widgets.HtmlPanel; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAICategory; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; -import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo.MapWithAICategory; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAILayerInfo; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.plugins.mapwithai.io.mapwithai.ESRISourceReader; import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.ImageProvider; @@ -392,7 +393,7 @@ public class MapWithAIProvidersPanel extends JPanel { activeToolbar.setFloatable(false); activeToolbar.setBorderPainted(false); activeToolbar.setOpaque(false); - activeToolbar.add(new NewEntryAction(MapWithAIInfo.MapWithAIType.THIRD_PARTY)); + activeToolbar.add(new NewEntryAction(MapWithAIType.THIRD_PARTY)); activeToolbar.add(edit); activeToolbar.add(remove); if (showActive) { @@ -556,7 +557,7 @@ public class MapWithAIProvidersPanel extends JPanel { private class NewEntryAction extends AbstractAction { private static final long serialVersionUID = 7451336680150337942L; - NewEntryAction(MapWithAIInfo.MapWithAIType type) { + NewEntryAction(MapWithAIType type) { putValue(NAME, type.toString()); putValue(SHORT_DESCRIPTION, tr("Add a new {0} entry by entering the URL", type.toString())); String icon = /* ICON(dialogs/) */ "add"; @@ -572,7 +573,7 @@ public class MapWithAIProvidersPanel extends JPanel { if (addDialog.getValue() == 1) { try { MapWithAIInfo info = p.getSourceInfo(); - if (MapWithAIInfo.MapWithAIType.ESRI == info.getSourceType()) { + if (MapWithAIType.ESRI == info.getSourceType()) { try (ESRISourceReader reader = new ESRISourceReader(info)) { for (MapWithAIInfo i : reader.parse()) { activeModel.addRow(i); diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReader.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReader.java index 05c145d..020e95a 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReader.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReader.java @@ -24,8 +24,9 @@ import javax.json.JsonValue; import org.openstreetmap.josm.data.imagery.ImageryInfo.ImageryBounds; import org.openstreetmap.josm.io.CachedFile; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAICategory; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; -import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo.MapWithAICategory; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.tools.HttpClient; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -86,7 +87,7 @@ public class ESRISourceReader implements Closeable { JsonReader reader = Json.createReader(i)) { layers.setFastFail(fastFail); JsonStructure parser = reader.read(); - if (parser.getValueType().equals(JsonValue.ValueType.OBJECT)) { + if (parser.getValueType() == JsonValue.ValueType.OBJECT) { JsonObject obj = parser.asJsonObject(); next = obj.getString("nextStart", "-1"); searchUrl = startReplace.matcher(search).replaceAll(next); @@ -124,7 +125,7 @@ public class ESRISourceReader implements Closeable { ImageryBounds imageryBounds = new ImageryBounds(String.join(",", extent[1], extent[0], extent[3], extent[2]), ","); newInfo.setBounds(imageryBounds); - newInfo.setSourceType(MapWithAIInfo.MapWithAIType.ESRI_FEATURE_SERVER); + newInfo.setSourceType(MapWithAIType.ESRI_FEATURE_SERVER); newInfo.setTermsOfUseText(feature.getString("licenseInfo", null)); if (feature.containsKey("thumbnail")) { newInfo.setAttributionImageURL( @@ -135,8 +136,8 @@ public class ESRISourceReader implements Closeable { .stream().map(JsonString::getString).map(s -> s.replace("/Categories/", "")) .map(MapWithAICategory::fromString).filter(Objects::nonNull) .collect(Collectors.toCollection(ArrayList::new)); - MapWithAICategory category = categories.stream().filter(c -> !MapWithAICategory.FEATURED.equals(c)) - .findFirst().orElse(MapWithAICategory.OTHER); + MapWithAICategory category = categories.stream().filter(c -> MapWithAICategory.FEATURED != c).findFirst() + .orElse(MapWithAICategory.OTHER); newInfo.setCategory(category); categories.remove(category); newInfo.setAdditionalCategories(categories); diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReader.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReader.java index f3640b3..da00a1a 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReader.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReader.java @@ -28,6 +28,7 @@ import org.openstreetmap.josm.data.imagery.Shape; import org.openstreetmap.josm.data.osm.BBox; import org.openstreetmap.josm.io.CachedFile; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.tools.DefaultGeoProperty; import org.openstreetmap.josm.tools.GeoPropertyIndex; import org.openstreetmap.josm.tools.HttpClient; @@ -49,6 +50,9 @@ public class MapWithAISourceReader implements Closeable { private CachedFile cachedFile; private boolean fastFail; + private static final int MIN_NODE_FOR_CLOSED_WAY = 2; + private static final int COORD_ARRAY_SIZE = 6; + /** * Constructs a {@code ImageryReader} from a given filename, URL or internal * resource. @@ -93,7 +97,7 @@ public class MapWithAISourceReader implements Closeable { try (JsonReader reader = Json.createReader(cachedFile.setMaxAge(CachedFile.DAYS) .setCachingStrategy(CachedFile.CachingStrategy.IfModifiedSince).getContentReader())) { JsonStructure struct = reader.read(); - if (JsonValue.ValueType.OBJECT.equals(struct.getValueType())) { + if (JsonValue.ValueType.OBJECT == struct.getValueType()) { JsonObject jsonObject = struct.asJsonObject(); entries = parseJson(jsonObject); } @@ -103,17 +107,17 @@ public class MapWithAISourceReader implements Closeable { private static MapWithAIInfo parse(Map.Entry entry) { String name = entry.getKey(); - if (JsonValue.ValueType.OBJECT.equals(entry.getValue().getValueType())) { + if (JsonValue.ValueType.OBJECT == entry.getValue().getValueType()) { JsonObject values = entry.getValue().asJsonObject(); String url = values.getString("url", ""); - String type = values.getString("type", MapWithAIInfo.MapWithAIType.THIRD_PARTY.getTypeString()); + String type = values.getString("type", MapWithAIType.THIRD_PARTY.getTypeString()); String eula = values.getString("eula", ""); boolean conflation = values.getBoolean("conflate", false); String conflationUrl = values.getString("conflationUrl", null); String id = values.getString("id", name.replace(" ", "_")); JsonValue countries = values.getOrDefault("countries", JsonValue.EMPTY_JSON_OBJECT); List bounds = new ArrayList<>(); - if (JsonValue.ValueType.OBJECT.equals(countries.getValueType())) { + if (JsonValue.ValueType.OBJECT == countries.getValueType()) { Set codes = Territories.getKnownIso3166Codes(); for (Map.Entry country : countries.asJsonObject().entrySet()) { if (codes.contains(country.getKey())) { @@ -159,14 +163,14 @@ public class MapWithAISourceReader implements Closeable { Collection shapes = new ArrayList<>(); float[] moveTo = null; while (!iterator.isDone()) { - float[] coords = new float[6]; + float[] coords = new float[COORD_ARRAY_SIZE]; int type = iterator.currentSegment(coords); if (type == PathIterator.SEG_MOVETO || type == PathIterator.SEG_LINETO) { if (type == PathIterator.SEG_MOVETO) { moveTo = coords; } defaultShape.addPoint(Float.toString(coords[1]), Float.toString(coords[0])); - } else if (type == PathIterator.SEG_CLOSE && moveTo != null && moveTo.length >= 2) { + } else if (type == PathIterator.SEG_CLOSE && moveTo != null && moveTo.length >= MIN_NODE_FOR_CLOSED_WAY) { defaultShape.addPoint(Float.toString(moveTo[1]), Float.toString(moveTo[0])); shapes.add(defaultShape); defaultShape = new Shape(); diff --git a/test/integration/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReaderTestIT.java b/test/integration/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReaderTestIT.java index aa1aa18..47acf64 100644 --- a/test/integration/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReaderTestIT.java +++ b/test/integration/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/MapWithAISourceReaderTestIT.java @@ -13,7 +13,7 @@ import org.junit.Rule; import org.junit.Test; import org.openstreetmap.josm.plugins.mapwithai.backend.DataAvailability; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; -import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo.MapWithAIType; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.testutils.JOSMTestRules; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -27,7 +27,7 @@ public class MapWithAISourceReaderTestIT { public void testDefaultSourceIT() throws IOException { try (MapWithAISourceReader source = new MapWithAISourceReader(DataAvailability.getReleaseUrl())) { List infoList = source.parse(); - assertFalse(infoList.isEmpty()); + assertFalse(infoList.isEmpty(), "There should be viable sources"); for (MapWithAIType type : Arrays.asList(MapWithAIType.FACEBOOK, MapWithAIType.THIRD_PARTY)) { assertTrue(infoList.stream().filter(i -> type.equals(i.getSourceType())).count() > 0, tr("Type {0} should have more than 0 sources", type.getTypeString())); diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java index caa9ab6..3da76b4 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIInfoTest.java @@ -13,8 +13,7 @@ public class MapWithAIInfoTest { @ParameterizedTest @MethodSource("provideMapWithAIInfoInitializers") - public void assertInitializersWorked(MapWithAIInfo i, String name, String url, String id, - MapWithAIInfo.MapWithAIType type) { + public void assertInitializersWorked(MapWithAIInfo i, String name, String url, String id, MapWithAIType type) { assertEquals(name, i.getName()); assertEquals(id, i.getId()); assertEquals(url, i.getUrl()); @@ -31,8 +30,8 @@ public class MapWithAIInfoTest { String url = "https://test.url"; String id = "a;lkdjfadl;ksfj"; String eula = ""; - MapWithAIInfo.MapWithAIType type = MapWithAIInfo.MapWithAIType.FACEBOOK; - MapWithAIInfo.MapWithAIType defaultType = MapWithAIInfo.MapWithAIType.THIRD_PARTY; + MapWithAIType type = MapWithAIType.FACEBOOK; + MapWithAIType defaultType = MapWithAIType.THIRD_PARTY; MapWithAIInfo tempInfo = new MapWithAIInfo(name, url, id); return Stream.of(Arguments.of(new MapWithAIInfo(), null, null, null, defaultType), Arguments.of(new MapWithAIInfo(name), name, null, null, defaultType), diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReaderTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReaderTest.java index 8e09c3e..81e43ad 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReaderTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReaderTest.java @@ -11,13 +11,19 @@ import java.util.Collection; import org.junit.Rule; import org.junit.Test; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; -import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo.MapWithAIType; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules; public class ESRISourceReaderTest { @Rule public MapWithAITestRules rule = (MapWithAITestRules) new MapWithAITestRules().wiremock().projection(); + /** + * Test that ESRI servers are properly added + * + * @throws IOException If there is an issue with reading the network + * file/wiremocked file + */ @Test public void testAddEsriLayer() throws IOException { // TODO wiremock @@ -28,9 +34,11 @@ public class ESRISourceReaderTest { info.setUrl(url); try (ESRISourceReader reader = new ESRISourceReader(info)) { Collection layers = reader.parse(); - assertFalse(layers.isEmpty()); - assertTrue(layers.stream().noneMatch(i -> info.getUrl().equals(i.getUrl()))); - assertTrue(layers.stream().allMatch(i -> MapWithAIType.ESRI_FEATURE_SERVER.equals(i.getSourceType()))); + assertFalse(layers.isEmpty(), "There should be a MapWithAI layer"); + assertTrue(layers.stream().noneMatch(i -> info.getUrl().equals(i.getUrl())), + "The ESRI server should be expanded to feature servers"); + assertTrue(layers.stream().allMatch(i -> MapWithAIType.ESRI_FEATURE_SERVER.equals(i.getSourceType())), + "There should only be ESRI feature servers"); } } }