Get building data from RapiD

* Also move a method to RapiDDataUtils
	* Also fix Issue #2
	* Remove data from RapiD layer when added to OSM layer

Signed-off-by: Taylor Smock <smocktaylor@gmail.com>

src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDAction.java

@@ -61,6 +61,8 @@ public class RapiDAction extends JosmAction {
 		for (final Bounds bound : editSetBounds) {
 			if (!rapidBounds.contains(bound)) {
 				final DataSet newData = RapiDDataUtils.getData(bound.toBBox());
+				/* Microsoft buildings don't have a source, so we add one */
+				RapiDDataUtils.addSourceTags(newData, "building", "Microsoft");
 				synchronized (layer) {
 					layer.unlock();
 					layer.mergeFrom(newData);

src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDAddCommand.java

@@ -17,7 +17,6 @@ import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
 import org.openstreetmap.josm.data.osm.PrimitiveData;
-import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.osm.visitor.MergeSourceBuildingVisitor;
 import org.openstreetmap.josm.plugins.rapid.RapiDPlugin;
@@ -54,11 +53,12 @@ public class RapiDAddCommand extends Command {
 			throw new IllegalArgumentException();
 		}
 		primitives = new HashSet<>(primitives);
-		addPrimitivesToCollection(/* collection= */ primitives, /* primitives= */ primitives);
+		RapiDDataUtils.addPrimitivesToCollection(/* collection= */ primitives, /* primitives= */ primitives);
 		synchronized (this) {
 			rapid.unlock();
 			Collection<OsmPrimitive> newPrimitives = new TreeSet<>(moveCollection(rapid, editable, primitives));
 			createConnections(editable, newPrimitives);
+			RapiDDataUtils.removePrimitivesFromDataSet(primitives);
 			rapid.lock();
 		}
 		return true;
@@ -67,6 +67,8 @@ public class RapiDAddCommand extends Command {
 	/**
 	 * Create connections based off of current RapiD syntax
 	 *
+	 * @param dataSet    The {@link DataSet} that should have the primitives we are
+	 *                   connecting to
 	 * @param collection The primitives with connection information (currently only
 	 *                   checks Nodes)
 	 */
@@ -75,7 +77,7 @@ public class RapiDAddCommand extends Command {
 		for (Node node : nodes) {
 			if (node.hasKey("conn")) {
 				// Currently w<way id>,n<node1>,n<node2>
-				String[] connections = node.get("conn").split(",");
+				String[] connections = node.get("conn").split(",", -1);
 				OsmPrimitive[] primitiveConnections = new OsmPrimitive[connections.length];
 				for (int i = 0; i < connections.length; i++) {
 					String member = connections[i];
@@ -113,8 +115,6 @@ public class RapiDAddCommand extends Command {
 	 * @param first     The first node in a waysegment (the node is between this and
 	 *                  the second node)
 	 * @param second    The second node in a waysegemnt
-	 * @param recursion The recursion (how many times this has called itself). Use 0
-	 *                  when calling.
 	 */
 	public static void addNodesToWay(Node toAddNode, Way way, Node first, Node second) {
 		int index = Math.max(way.getNodes().indexOf(first), way.getNodes().indexOf(second));
@@ -146,24 +146,6 @@ public class RapiDAddCommand extends Command {
 		return addPrimitivesCommand.getParticipatingPrimitives();
 	}
 
-	/**
-	 * Add primitives and their children to a collection
-	 *
-	 * @param collection A collection to add the primitives to
-	 * @param primitives The primitives to add to the collection
-	 */
-	public static void addPrimitivesToCollection(Collection<OsmPrimitive> collection,
-			Collection<OsmPrimitive> primitives) {
-		for (OsmPrimitive primitive : primitives) {
-			if (primitive instanceof Way) {
-				collection.addAll(((Way) primitive).getNodes());
-			} else if (primitive instanceof Relation) {
-				addPrimitivesToCollection(collection, ((Relation) primitive).getMemberPrimitives());
-			}
-			collection.add(primitive);
-		}
-	}
-
 	@Override
 	public String getDescriptionText() {
 		return tr("Add object from RapiD");

src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDDataUtils.java

@@ -4,11 +4,17 @@ package org.openstreetmap.josm.plugins.rapid.backend;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.TreeSet;
 
 import org.openstreetmap.josm.data.osm.BBox;
 import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.io.IllegalDataException;
 import org.openstreetmap.josm.io.OsmReader;
 import org.openstreetmap.josm.plugins.rapid.RapiDPlugin;
@@ -20,12 +26,13 @@ import org.openstreetmap.josm.tools.Logging;
  */
 public final class RapiDDataUtils {
 	private static final String RAPID_API_TOKEN = "ASZUVdYpCkd3M6ZrzjXdQzHulqRMnxdlkeBJWEKOeTUoY_Gwm9fuEd2YObLrClgDB_xfavizBsh0oDfTWTF7Zb4C";
-	private static Set<String> API_LIST = new HashSet<>();
+	private static final Set<String> API_LIST = new HashSet<>();
 	static {
 		addRapidApi(new StringBuilder().append("https://www.facebook.com/maps/ml_roads?").append("conflate_with_osm=")
 				.append(true).append("&").append("theme=")
 				.append("ml_road_vector").append("&").append("collaborator=").append("fbid").append("&")
 				.append("token=").append(RAPID_API_TOKEN).append("&").append("hash=").append("ASYM8LPNy8k1XoJiI7A")
+				.append("&").append("result_type=").append("road_building_vector_xml")
 				.append("&").append("bbox={bbox}").toString());
 	}
 
@@ -33,7 +40,12 @@ public final class RapiDDataUtils {
 		// Hide the constructor
 	}
 
-	@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
+	/**
+	 * Get a dataset from the API servers using a bbox
+	 *
+	 * @param bbox The bbox from which to get data
+	 * @return A DataSet with data inside the bbox
+	 */
 	public static DataSet getData(BBox bbox) {
 		InputStream inputStream = null;
 		DataSet dataSet = new DataSet();
@@ -59,7 +71,71 @@ public final class RapiDDataUtils {
 		return dataSet;
 	}
 
+	/**
+	 * Add specified source tags to objects without a source tag that also have a
+	 * specific key
+	 *
+	 * @param dataSet    The {#link DataSet} to look through
+	 * @param primaryKey The primary key that must be in the {@link OsmPrimitive}
+	 * @param source     The specified source value (not tag)
+	 */
+	public static void addSourceTags(DataSet dataSet, String primaryKey, String source) {
+		dataSet.allPrimitives().stream().filter(p -> p.hasKey(primaryKey) && !p.hasKey("source")).forEach(p -> {
+			p.put("source", source);
+			p.save();
+		});
+	}
+
+	/**
+	 * Add a url to the the API_LIST
+	 *
+	 * @param url A url with a "{bbox}" inside it (this is what gets replaced in {@link RapiDDataUtils#getData})
+	 */
 	public static void addRapidApi(String url) {
 		API_LIST.add(url);
 	}
+
+	/**
+	 * Remove primitives and their children from a dataset.
+	 *
+	 * @param primitives The primitives to remove
+	 */
+	public static void removePrimitivesFromDataSet(Collection<OsmPrimitive> primitives) {
+		for (OsmPrimitive primitive : primitives) {
+			if (primitive instanceof Relation) {
+				removePrimitivesFromDataSet(((Relation) primitive).getMemberPrimitives());
+			} else if (primitive instanceof Way) {
+				for (Node node : ((Way) primitive).getNodes()) {
+					DataSet ds = node.getDataSet();
+					if (ds != null) {
+						ds.removePrimitive(node);
+					}
+				}
+			}
+			DataSet ds = primitive.getDataSet();
+			if (ds != null) {
+				ds.removePrimitive(primitive);
+			}
+		}
+	}
+
+	/**
+	 * Add primitives and their children to a collection
+	 *
+	 * @param collection A collection to add the primitives to
+	 * @param primitives The primitives to add to the collection
+	 */
+	public static void addPrimitivesToCollection(Collection<OsmPrimitive> collection,
+			Collection<OsmPrimitive> primitives) {
+		Collection<OsmPrimitive> temporaryCollection = new TreeSet<>();
+		for (OsmPrimitive primitive : primitives) {
+			if (primitive instanceof Way) {
+				temporaryCollection.addAll(((Way) primitive).getNodes());
+			} else if (primitive instanceof Relation) {
+				addPrimitivesToCollection(temporaryCollection, ((Relation) primitive).getMemberPrimitives());
+			}
+			temporaryCollection.add(primitive);
+		}
+		collection.addAll(temporaryCollection);
+	}
 }

test/unit/org/openstreetmap/josm/plugins/rapid/RapiDAddComandTest.java

@@ -1,9 +1,8 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.rapid;
 
-import java.util.Collection;
+import java.util.Arrays;
 import java.util.Collections;
-import java.util.TreeSet;
 
 import org.junit.Assert;
 import org.junit.Rule;
@@ -12,7 +11,6 @@ import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
-import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.plugins.rapid.backend.RapiDAddCommand;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
@@ -22,7 +20,7 @@ public class RapiDAddComandTest {
 	public JOSMTestRules test = new JOSMTestRules();
 
 	@Test
-	public void testMoveCollection() {
+	public void testMoveCollectionSingleParent() {
 		DataSet ds1 = new DataSet();
 		DataSet ds2 = new DataSet();
 		Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)),
@@ -37,17 +35,33 @@ public class RapiDAddComandTest {
 		Assert.assertTrue(ds2.containsWay(way1));
 		Assert.assertTrue(ds2.containsNode(way1.firstNode()));
 		Assert.assertTrue(ds2.containsNode(way1.lastNode()));
-		// Assert.assertFalse(ds1.containsWay(way1)); // Only if we delete ways from
+		Assert.assertFalse(ds1.containsWay(way1));
-		// rapid dataset
 	}
 
 	@Test
-	public void testAddPrimitivesToCollection() {
+	public void testMoveCollectionMultipleParent() {
+
+		DataSet ds1 = new DataSet();
+		DataSet ds2 = new DataSet();
 		Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)), new Node(new LatLon(0, 0.1)));
-		Collection<OsmPrimitive> collection = new TreeSet<>();
+		Way way2 = TestUtils.newWay("highway=residential", new Node(new LatLon(-0.1, -0.2)), way1.firstNode());
-		Assert.assertEquals(0, collection.size());
+		for (Node node : way1.getNodes()) {
-		RapiDAddCommand.addPrimitivesToCollection(collection, Collections.singletonList(way1));
+			ds1.addPrimitive(node);
-		Assert.assertEquals(3, collection.size());
+		}
+		for (Node node : way2.getNodes()) {
+			if (!ds1.containsNode(node)) {
+				ds1.addPrimitive(node);
+			}
+		}
+		ds1.addPrimitive(way1);
+		ds1.addPrimitive(way2);
+		ds1.lock();
+		RapiDAddCommand command = new RapiDAddCommand(ds1, ds2, Arrays.asList(way1, way2));
+		command.executeCommand();
+		Assert.assertTrue(ds2.containsWay(way1));
+		Assert.assertTrue(ds2.containsNode(way1.firstNode()));
+		Assert.assertTrue(ds2.containsNode(way1.lastNode()));
+		Assert.assertFalse(ds1.containsWay(way1));
 	}
 
 	@Test

test/unit/org/openstreetmap/josm/plugins/rapid/RapiDDataUtilsTest.java

@@ -1,11 +1,20 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.rapid;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.TreeSet;
+
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
+import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.BBox;
 import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.plugins.rapid.backend.RapiDDataUtils;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
@@ -18,10 +27,54 @@ public class RapiDDataUtilsTest {
 	 */
 	@Test
 	public void testGetData() {
-		BBox testBBox = new BBox();
+		BBox testBBox = getTestBBox();
-		testBBox.add(new LatLon(39.0666521, -108.5707187));
-		testBBox.add(new LatLon(39.0664016, -108.5702225));
 		DataSet ds = new DataSet(RapiDDataUtils.getData(testBBox));
 		Assert.assertEquals(1, ds.getWays().size());
 	}
+
+	@Test
+	public void testAddSourceTags() {
+		BBox testBBox = getTestBBox();
+		DataSet ds = new DataSet(RapiDDataUtils.getData(testBBox));
+		Assert.assertEquals(1, ds.getWays().size());
+		Way way1 = (Way) ds.getWays().toArray()[0];
+		String originalSource = way1.get("source");
+		Assert.assertNotNull(originalSource);
+
+		way1.remove("source");
+		Assert.assertNull(way1.get("source"));
+
+		RapiDDataUtils.addSourceTags(ds, "highway", originalSource);
+		Assert.assertEquals(originalSource, way1.get("source"));
+	}
+
+	private static BBox getTestBBox() {
+		BBox testBBox = new BBox();
+		testBBox.add(new LatLon(39.0666521, -108.5707187));
+		testBBox.add(new LatLon(39.0664016, -108.5702225));
+		return testBBox;
+	}
+
+	@Test
+	public void testAddPrimitivesToCollection() {
+		Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)), new Node(new LatLon(0, 0.1)));
+		Collection<OsmPrimitive> collection = new TreeSet<>();
+		Assert.assertEquals(0, collection.size());
+		RapiDDataUtils.addPrimitivesToCollection(collection, Collections.singletonList(way1));
+		Assert.assertEquals(3, collection.size());
+	}
+
+	@Test
+	public void testRemovePrimitivesFromDataSet() {
+		Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)), new Node(new LatLon(0, 0.1)));
+		DataSet ds1 = new DataSet();
+		for (Node node : way1.getNodes()) {
+			ds1.addPrimitive(node);
+		}
+		ds1.addPrimitive(way1);
+
+		Assert.assertEquals(3, ds1.allPrimitives().size());
+		RapiDDataUtils.removePrimitivesFromDataSet(Collections.singleton(way1));
+		Assert.assertEquals(0, ds1.allPrimitives().size());
+	}
 }