Index: src/com/vividsolutions/jcs/conflate/polygonmatch/AngleHistogramMatcher.java =================================================================== --- src/com/vividsolutions/jcs/conflate/polygonmatch/AngleHistogramMatcher.java (revision 267) +++ src/com/vividsolutions/jcs/conflate/polygonmatch/AngleHistogramMatcher.java (working copy) @@ -84,7 +84,7 @@ * weighted by segment length */ protected Histogram angleHistogram(Geometry g, int binCount) { - Geometry clone = (Geometry) g.clone(); + Geometry clone = g.copy(); //#normalize makes linestrings and polygons use a standard orientation. //[Jon Aquino] clone.normalize(); Index: src/com/vividsolutions/jcs/conflate/polygonmatch/CentroidAligner.java =================================================================== --- src/com/vividsolutions/jcs/conflate/polygonmatch/CentroidAligner.java (revision 267) +++ src/com/vividsolutions/jcs/conflate/polygonmatch/CentroidAligner.java (working copy) @@ -16,7 +16,7 @@ } private Geometry align(Geometry original) { - Geometry aligned = (Geometry) original.clone(); + Geometry aligned = original.copy(); MatcherUtil.align(aligned, aligned.getCentroid().getCoordinate()); return aligned; } Index: src/com/vividsolutions/jcs/conflate/polygonmatch/Matches.java =================================================================== --- src/com/vividsolutions/jcs/conflate/polygonmatch/Matches.java (revision 267) +++ src/com/vividsolutions/jcs/conflate/polygonmatch/Matches.java (working copy) @@ -31,10 +31,14 @@ */ package com.vividsolutions.jcs.conflate.polygonmatch; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Set; import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.util.Assert; @@ -47,8 +51,8 @@ * A FeatureCollection that stores the "score" of each Feature. The score is * a number between 0.0 and 1.0 that indicates the confidence of a match. */ -public class Matches implements FeatureCollection, Cloneable { - +public class Matches extends AbstractMap implements FeatureCollection, Cloneable { + private final Set> entrySet = new HashSet<>(); /** * Creates a Matches object. * @param schema metadata applicable to the features that will be stored in @@ -80,8 +84,8 @@ } } - private FeatureDataset dataset; - private List scores = new ArrayList<>(); + private final FeatureDataset dataset; + private final List scores = new ArrayList<>(); /** * This method is not supported, because added features need to be associated @@ -135,6 +139,24 @@ throw new UnsupportedOperationException(); } + @Override + public Set> entrySet() { + if (this.size() == this.entrySet.size()) { + return this.entrySet; + } + return updateEntrySet(); + } + + private synchronized Set> updateEntrySet() { + if (this.size() != this.entrySet.size()) { + this.entrySet.clear(); + for (int i = 0; i < this.size(); i++) { + this.entrySet.add(new SimpleEntry<>(this.getFeature(i), this.getScore(i))); + } + } + return this.entrySet; + } + /** * This method is not supported, because Matches should not normally need to * have matches removed. @@ -167,7 +189,7 @@ if (score == 0) { return; } - scores.add(Double.valueOf(score)); + scores.add(score); dataset.add(feature); if (score > topScore) { topScore = score; @@ -195,7 +217,7 @@ * @return the confidence of the ith match */ public double getScore(int i) { - return scores.get(i).doubleValue(); + return scores.get(i); } @Override Index: src/com/vividsolutions/jcs/conflate/polygonmatch/SymDiffMatcher.java =================================================================== --- src/com/vividsolutions/jcs/conflate/polygonmatch/SymDiffMatcher.java (revision 267) +++ src/com/vividsolutions/jcs/conflate/polygonmatch/SymDiffMatcher.java (working copy) @@ -53,8 +53,8 @@ */ @Override public double match(Geometry target, Geometry candidate) { - Geometry targetGeom = (Geometry) target.clone(); - Geometry candidateGeom = (Geometry) candidate.clone(); + Geometry targetGeom = target.copy(); + Geometry candidateGeom = candidate.copy(); if (targetGeom.isEmpty() || candidateGeom.isEmpty()) { return 0; //avoid div by 0 in centre-of-mass calc [Jon Aquino] } Index: src/com/vividsolutions/jcs/conflate/polygonmatch/WeightedMatcher.java =================================================================== --- src/com/vividsolutions/jcs/conflate/polygonmatch/WeightedMatcher.java (revision 267) +++ src/com/vividsolutions/jcs/conflate/polygonmatch/WeightedMatcher.java (working copy) @@ -33,6 +33,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.TreeMap; import org.locationtech.jts.util.Assert; @@ -48,9 +49,10 @@ /** * Creates a WeightedMatcher with the given matchers and their weights. - * @param matchersAndWeights alternates between FeatureMatchers and Doubles + * @param matchersAndWeights alternates between Doubles and FeatureMatchers */ - public WeightedMatcher(Object[] matchersAndWeights) { + public WeightedMatcher(Object... matchersAndWeights) { + Objects.requireNonNull(matchersAndWeights); Assert.isTrue(matchersAndWeights.length % 2 == 0); for (int i = 0; i < matchersAndWeights.length; i += 2) { add((FeatureMatcher) matchersAndWeights[i+1], @@ -70,10 +72,10 @@ if (weight == 0) { return; } - matcherToWeightMap.put(matcher, Double.valueOf(weight)); + this.matcherToWeightMap.put(matcher, weight); } - private Map matcherToWeightMap = new HashMap<>(); + private final Map matcherToWeightMap = new HashMap<>(); /** * Searches a collection of candidate features for those that match the given @@ -93,10 +95,7 @@ private Matches toMatches(Map featureToScoreMap, FeatureSchema schema) { Matches matches = new Matches(schema); - for (Feature feature : featureToScoreMap.keySet()) { - double score = featureToScoreMap.get(feature).doubleValue(); - matches.add(feature, score); - } + featureToScoreMap.forEach(matches::add); return matches; } @@ -111,36 +110,24 @@ private Map featureToScoreMap(Map matcherToMatchesMap) { Map featureToScoreMap = new TreeMap<>(); - for (FeatureMatcher matcher : matcherToMatchesMap.keySet()) { - Matches matches = matcherToMatchesMap.get(matcher); - addToFeatureToScoreMap(matches, matcher, featureToScoreMap); - } + matcherToMatchesMap.forEach((matcher, matches) -> this.addToFeatureToScoreMap(matches, matcher, featureToScoreMap)); return featureToScoreMap; } private void addToFeatureToScoreMap(Matches matches, FeatureMatcher matcher, Map featureToScoreMap) { - for (int i = 0; i < matches.size(); i++) { - double score = matches.getScore(i) * normalizedWeight(matcher); - addToFeatureToScoreMap(matches.getFeature(i), score, featureToScoreMap); + + for (Map.Entry entry : matches.entrySet()) { + double score = entry.getValue() * this.normalizedWeight(matcher); + featureToScoreMap.compute(entry.getKey(), (key, value) -> value == null ? score : value + score); } } - private void addToFeatureToScoreMap(Feature feature, double score, Map featureToScoreMap) { - Double oldScore = featureToScoreMap.get(feature); - if (oldScore == null) { oldScore = Double.valueOf(0); } - featureToScoreMap.put(feature, Double.valueOf(oldScore.doubleValue() + score)); - } - private double normalizedWeight(FeatureMatcher matcher) { - return matcherToWeightMap.get(matcher).doubleValue() / weightTotal(); + return this.matcherToWeightMap.get(matcher) / this.weightTotal(); } private double weightTotal() { - double weightTotal = 0; - for (Double weight : matcherToWeightMap.values()) { - weightTotal += weight.doubleValue(); - } - return weightTotal; + return this.matcherToWeightMap.values().stream().mapToDouble(Double::doubleValue).sum(); } } Index: src/com/vividsolutions/jump/feature/AbstractBasicFeature.java =================================================================== --- src/com/vividsolutions/jump/feature/AbstractBasicFeature.java (revision 267) +++ src/com/vividsolutions/jump/feature/AbstractBasicFeature.java (working copy) @@ -182,7 +182,7 @@ for (int i = 0; i < schema.getAttributeCount(); i++) { if (schema.getAttributeType(i) == AttributeType.GEOMETRY) { clone.setAttribute(i, - deep ? getGeometry().clone() : getGeometry()); + deep ? getGeometry().copy() : getGeometry()); } else { clone.setAttribute(i, getAttribute(i)); } Index: src/com/vividsolutions/jump/util/CoordinateArrays.java =================================================================== --- src/com/vividsolutions/jump/util/CoordinateArrays.java (revision 267) +++ src/com/vividsolutions/jump/util/CoordinateArrays.java (working copy) @@ -34,7 +34,7 @@ import java.util.ArrayList; import java.util.List; -import org.locationtech.jts.algorithm.CGAlgorithms; +import org.locationtech.jts.algorithm.Orientation; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryCollection; @@ -128,7 +128,7 @@ Coordinate[] shell = poly.getExteriorRing().getCoordinates(); if (orientPolygons) { - shell = ensureOrientation(shell, CGAlgorithms.CLOCKWISE); + shell = ensureOrientation(shell, Orientation.CLOCKWISE); } coordArrayList.add(shell); @@ -137,7 +137,7 @@ Coordinate[] hole = poly.getInteriorRingN(i).getCoordinates(); if (orientPolygons) { - hole = ensureOrientation(hole, CGAlgorithms.COUNTERCLOCKWISE); + hole = ensureOrientation(hole, Orientation.COUNTERCLOCKWISE); } coordArrayList.add(hole); @@ -168,8 +168,8 @@ return coord; } - int orientation = CGAlgorithms.isCCW(coord) ? CGAlgorithms.COUNTERCLOCKWISE - : CGAlgorithms.CLOCKWISE; + int orientation = Orientation.isCCW(coord) ? Orientation.COUNTERCLOCKWISE + : Orientation.CLOCKWISE; if (orientation != desiredOrientation) { Coordinate[] reverse = coord.clone(); Index: src/org/openstreetmap/josm/plugins/conflation/OsmFeature.java =================================================================== --- src/org/openstreetmap/josm/plugins/conflation/OsmFeature.java (revision 267) +++ src/org/openstreetmap/josm/plugins/conflation/OsmFeature.java (working copy) @@ -3,6 +3,7 @@ package org.openstreetmap.josm.plugins.conflation; import java.util.Map; +import java.util.Objects; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.plugins.jts.JTSConverter; @@ -9,12 +10,12 @@ import com.vividsolutions.jump.feature.AbstractBasicFeature; import com.vividsolutions.jump.feature.AttributeType; +import com.vividsolutions.jump.feature.Feature; import com.vividsolutions.jump.feature.FeatureSchema; public class OsmFeature extends AbstractBasicFeature { private Object[] attributes; - private OsmPrimitive primitive; - private JTSConverter converter; + private final OsmPrimitive primitive; /** * Create a copy of the OSM geometry @@ -22,14 +23,15 @@ */ public OsmFeature(OsmPrimitive prim, JTSConverter jtsConverter) { super(new FeatureSchema()); - primitive = prim; + this.primitive = Objects.requireNonNull(prim); Map keys = prim.getKeys(); - attributes = new Object[keys.size() + 1]; - getSchema().addAttribute("__GEOMETRY__", AttributeType.GEOMETRY); - for (String key : keys.keySet()) { - getSchema().addAttribute(key, AttributeType.STRING); - setAttribute(key, keys.get(key)); - } + this.attributes = new Object[keys.size() + 1]; + this.getSchema().addAttribute("__GEOMETRY__", AttributeType.GEOMETRY); + keys.forEach((key, value) -> { + this.getSchema().addAttribute(key, AttributeType.STRING); + setAttribute(key, value); + }); + final JTSConverter converter; if (jtsConverter != null) converter = jtsConverter; else @@ -68,4 +70,29 @@ // objects with the same id return (int) primitive.getUniqueId(); } + + @Override + public int compareTo(Feature abstractBasicFeature) { + // Rather unfortunately, we cannot implement the interface with OsmFeature + // So we are going to special case osm features, and compare ids. + // The super.compareTo only looks at geometry. If the geometry is the same, then it returns 0. + final int superCompare = super.compareTo(abstractBasicFeature); + if (superCompare == 0 && abstractBasicFeature instanceof OsmFeature) { + final OsmFeature other = (OsmFeature) abstractBasicFeature; + return this.primitive.compareTo(other.primitive); + } + return superCompare; + } + + @Override + public boolean equals(Object other) { + return other != null && this.getClass().equals(other.getClass()) + && Objects.equals(((OsmFeature) other).primitive, this.primitive); + } + + @Override + public int hashCode() { + // No superclasses implement hashCode + return this.primitive.hashCode(); + } } Index: test/unit/com/vividsolutions/jcs/conflate/polygonmatch/WeightedMatcherTest.java =================================================================== --- test/unit/com/vividsolutions/jcs/conflate/polygonmatch/WeightedMatcherTest.java (nonexistent) +++ test/unit/com/vividsolutions/jcs/conflate/polygonmatch/WeightedMatcherTest.java (working copy) @@ -0,0 +1,60 @@ +package com.vividsolutions.jcs.conflate.polygonmatch; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import java.util.Arrays; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.plugins.conflation.OsmFeature; +import org.openstreetmap.josm.plugins.jts.JTSConverter; +import org.openstreetmap.josm.testutils.JOSMTestRules; +import org.openstreetmap.josm.testutils.annotations.BasicPreferences; + +import com.vividsolutions.jump.feature.Feature; +import com.vividsolutions.jump.feature.FeatureCollection; +import com.vividsolutions.jump.feature.FeatureDataset; +import com.vividsolutions.jump.feature.FeatureSchema; +import com.vividsolutions.jump.feature.IndexedFeatureCollection; + +/** + * Test class for {@link WeightedMatcher} + * @author Taylor Smock + */ +@BasicPreferences +class WeightedMatcherTest { + @RegisterExtension + JOSMTestRules josmTestRules = new JOSMTestRules().projection(); + /** + * Non-regression test for JOSM #21788 + * This occurred when two {@link org.openstreetmap.josm.plugins.conflation.OsmFeature} objects had {@link Comparable} + * equality. + */ + @Test + void testNonRegression21788() { + final Node target = new Node(); + target.setCoor(LatLon.ZERO); + final Node node1 = new Node(1, 1); + node1.setCoor(LatLon.ZERO); + final Node node2 = new Node(2, 1); + node2.setCoor(LatLon.ZERO); + final JTSConverter converter = new JTSConverter(true); + final WeightedMatcher weightedMatcher = new WeightedMatcher(1, new UnityMatcher()); + assertDoesNotThrow(() -> weightedMatcher.match(new OsmFeature(target, converter), + new FeatureDataset(Arrays.asList(new OsmFeature(node1, converter), new OsmFeature(node2, converter)), new FeatureSchema()))); + } + + /** + * This matcher always gives a score of 1. This is to make it easier for getting out of bounds. + */ + private static final class UnityMatcher implements FeatureMatcher { + @Override + public Matches match(Feature target, FeatureCollection candidates) { + final Matches matches = new Matches(target.getSchema()); + candidates.forEach(feature -> matches.add(feature, 1)); + return matches; + } + } +} Index: test/unit/org/openstreetmap/josm/plugins/conflation/OsmFeatureTest.java =================================================================== --- test/unit/org/openstreetmap/josm/plugins/conflation/OsmFeatureTest.java (nonexistent) +++ test/unit/org/openstreetmap/josm/plugins/conflation/OsmFeatureTest.java (working copy) @@ -0,0 +1,79 @@ +package org.openstreetmap.josm.plugins.conflation; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Arrays; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.plugins.jts.JTSConverter; +import org.openstreetmap.josm.testutils.JOSMTestRules; +import org.openstreetmap.josm.testutils.annotations.BasicPreferences; + +import com.vividsolutions.jump.feature.AttributeType; +import com.vividsolutions.jump.feature.FeatureSchema; +import nl.jqno.equalsverifier.EqualsVerifier; + +/** + * Test class for {@link OsmFeature} + * @author Taylor Smock + */ +@BasicPreferences +class OsmFeatureTest { + @RegisterExtension + JOSMTestRules josmTestRules = new JOSMTestRules().projection(); + /** + * This checks that two osm features (points) do not match if they have the same geometry but different ids. + * This is a partial non-regression test for JOSM #21788. The root of the issue is that TreeMap uses the Comparable + * interface in put operations, and the {@link Comparable} interface specifies that {@code 0} implies equality. + */ + @Test + void testCompareTo() { + final JTSConverter converter = new JTSConverter(true); + + final Node node1 = new Node(1, 1); + final Node node2 = new Node(2, 1); + // This is deliberate -- we want to make certain that different equal objects work (i.e., no one accidentally + // uses == ). + final Node node3 = new Node(1, 1); + Arrays.asList(node1, node2, node3).forEach(node -> node.setCoor(LatLon.ZERO)); + + final OsmFeature osmFeature1 = new OsmFeature(node1, converter); + final OsmFeature osmFeature2 = new OsmFeature(node2, converter); + final OsmFeature osmFeature3 = new OsmFeature(node3, converter); + + assertEquals(osmFeature1, osmFeature3, "The two nodes are equal"); + assertNotEquals(osmFeature1, osmFeature2, "The two nodes are not equal"); + assertEquals(0, osmFeature1.compareTo(osmFeature3)); + assertEquals(0, osmFeature3.compareTo(osmFeature1)); + assertNotEquals(0, osmFeature1.compareTo(osmFeature2)); + assertNotEquals(0, osmFeature2.compareTo(osmFeature3)); + } + + @Test + void testEqualsContract() { + final Node redNode = new Node(1, 1); + final Node blueNode = new Node(2, 2); + redNode.setCoor(LatLon.NORTH_POLE); + blueNode.setCoor(LatLon.SOUTH_POLE); + + final FeatureSchema redSchema = new FeatureSchema(); + final FeatureSchema blueSchema = new FeatureSchema(); + redSchema.addAttribute("red", AttributeType.STRING); + blueSchema.addAttribute("blue", AttributeType.STRING); + EqualsVerifier.forClass(OsmFeature.class) + .usingGetClass() + .withNonnullFields("primitive") + .withIgnoredFields("attributes" /* mutable */, + "schema" /* mutable */, + "id" /* not used in class */) + .withPrefabValues(OsmPrimitive.class, redNode, blueNode) + .withPrefabValues(FeatureSchema.class, redSchema, blueSchema) + .verify(); + } +}