Skip to content

Commit

Permalink
Merge branch 'praszuk/fix/share-nodes-with-not-a-building'
Browse files Browse the repository at this point in the history
  • Loading branch information
praszuk committed May 30, 2022
2 parents f0d0420 + a841a0d commit 87ff710
Show file tree
Hide file tree
Showing 8 changed files with 247 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package org.openstreetmap.josm.plugins.plbuildings.command;

import org.openstreetmap.josm.command.Command;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.osm.*;
import org.openstreetmap.josm.plugins.plbuildings.BuildingsSettings;
import org.openstreetmap.josm.plugins.plbuildings.exceptions.ImportBuildingDuplicateException;
import org.openstreetmap.josm.plugins.plbuildings.utils.SharedNodesUtils;
import org.openstreetmap.josm.tools.Logging;

import java.util.ArrayList;
Expand All @@ -13,6 +13,8 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;

import static org.openstreetmap.josm.plugins.plbuildings.utils.SharedNodesUtils.getBBox;
import static org.openstreetmap.josm.plugins.plbuildings.utils.SharedNodesUtils.isCloseNode;
import static org.openstreetmap.josm.tools.I18n.tr;


Expand All @@ -38,34 +40,6 @@ public Way getCreatedBuilding() {
return createdBuilding;
}

/**
* Create bbox based on list of nodes and their positions.
* It will produce bbox expanded by offset to e.g. match all very close nodes when importing
* semidetached_house/terrace buildings
* It works only for positive lat/lon values, because this plugin is only for Poland.
*/
public static BBox getBBox(List<Node> nodes, double bboxOffset) {
BBox bbox = new BBox();
nodes.forEach(bbox::add);

LatLon topLeft = bbox.getTopLeft();
LatLon bottomRight = bbox.getBottomRight();
bbox.add(new LatLon(topLeft.lat() + bboxOffset, topLeft.lon() - bboxOffset));
bbox.add(new LatLon(bottomRight.lat() - bboxOffset, bottomRight.lon() + bboxOffset));

return bbox;
}

/**
* Check if node1 is close to node2 where max distance is maxOffset (inclusive)
* Both (latitude and longitude) values must be close to return true.
*/
public static boolean isCloseNode(Node node1, Node node2, double maxOffset) {
boolean isLatOk = Math.abs(node1.lat() - node2.lat()) <= maxOffset;
boolean isLonOk = Math.abs(node1.lon() - node2.lon()) <= maxOffset;

return isLatOk && isLonOk;
}

@Override
public void fillModifiedData(
Expand Down Expand Up @@ -135,6 +109,7 @@ private void prepareBuilding() throws ImportBuildingDuplicateException {

List<Node> closeNodes = dataSet.searchNodes(bbox).stream()
.filter(n -> !n.isDeleted())
.filter(SharedNodesUtils::isShareableNode)
.collect(Collectors.toList());
List<Node> buildingNodes = new ArrayList<>();
List<Node> nodesToAddToDataSet = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.openstreetmap.josm.plugins.plbuildings.utils;

import java.util.AbstractMap;
import java.util.List;
import java.util.Map;

import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.osm.BBox;
import org.openstreetmap.josm.data.osm.Node;
import org.openstreetmap.josm.data.osm.TagMap;

public class SharedNodesUtils {

/**
* Create bbox based on list of nodes and their positions.
* It will produce bbox expanded by offset to e.g. match all very close nodes when importing
* semidetached_house/terrace buildings
* It works only for positive lat/lon values, because this plugin is only for Poland.
*/
public static BBox getBBox(List<Node> nodes, double bboxOffset) {
BBox bbox = new BBox();
nodes.forEach(bbox::add);

LatLon topLeft = bbox.getTopLeft();
LatLon bottomRight = bbox.getBottomRight();
bbox.add(new LatLon(topLeft.lat() + bboxOffset, topLeft.lon() - bboxOffset));
bbox.add(new LatLon(bottomRight.lat() - bboxOffset, bottomRight.lon() + bboxOffset));

return bbox;
}

/**
* Check if node1 is close to node2 where max distance is maxOffset (inclusive)
* Both (latitude and longitude) values must be close to return true.
*/
public static boolean isCloseNode(Node node1, Node node2, double maxOffset) {
boolean isLatOk = Math.abs(node1.lat() - node2.lat()) <= maxOffset;
boolean isLonOk = Math.abs(node1.lon() - node2.lon()) <= maxOffset;

return isLatOk && isLonOk;
}

/**
* Check if node can be shared with importing building. Check membership of node and parent object's tags.
* @return true if node can be shared (reused) with importing building else false
*/
public static boolean isShareableNode(Node node){
if (!node.isReferredByWays(1)){ // not member of any way
return false;
}
return SharedNodesUtils.isNodeStickToWayWithTag(node, new AbstractMap.SimpleEntry<>("building", "*"));
}


/**
*
* @param node which is element of ways to check theirs tags
* @param entryRequired key-value pair as OSM tag which is checked if exists in any shared (referred) way.
* To check only key, use "*" or empty string as value.
* @return true if at least 1 way has given tag (entryRequired) else false
*/
public static boolean isNodeStickToWayWithTag(Node node, Map.Entry<String, String> entryRequired){
return node.isReferredByWays(1) && node.getParentWays().stream().anyMatch(parentWay -> {
TagMap wayTags = parentWay.getKeys();

if (wayTags.containsKey(entryRequired.getKey())){
if (entryRequired.getValue().equals("*") || entryRequired.getValue().isEmpty()){
return true;
}
else return wayTags.get(entryRequired.getKey()).equals(entryRequired.getValue());
}
return false;
});
}
}
17 changes: 17 additions & 0 deletions test/data/share_nodes_with_object/import_building.osm
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version='1.0' encoding='UTF-8'?>
<osm version='0.6' generator='JOSM'>
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='CGImap 0.8.6 (1354445 spike-07.openstreetmap.org)' />
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='OpenStreetMap server' />
<node id='-101869' action='modify' visible='true' lat='50.33973987901' lon='19.49013168728' />
<node id='-101870' action='modify' visible='true' lat='50.33934776422' lon='19.49012060908' />
<node id='-101871' action='modify' visible='true' lat='50.33933971654' lon='19.49081588666' />
<node id='-101872' action='modify' visible='true' lat='50.33973183125' lon='19.49082697058' />
<way id='-102374' action='modify' visible='true'>
<nd ref='-101869' />
<nd ref='-101870' />
<nd ref='-101871' />
<nd ref='-101872' />
<nd ref='-101869' />
<tag k='building' v='detached' />
</way>
</osm>
8 changes: 8 additions & 0 deletions test/data/share_nodes_with_object/node_shop.osm
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version='1.0' encoding='UTF-8'?>
<osm version='0.6' generator='JOSM'>
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='CGImap 0.8.6 (1354445 spike-07.openstreetmap.org)' />
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='OpenStreetMap server' />
<node id='-101954' action='modify' visible='true' lat='50.33934776394' lon='19.49012060894'>
<tag k='shop' v='convenience' />
</node>
</osm>
16 changes: 16 additions & 0 deletions test/data/share_nodes_with_object/way_barrier.osm
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version='1.0' encoding='UTF-8'?>
<osm version='0.6' generator='JOSM'>
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='CGImap 0.8.6 (1354445 spike-07.openstreetmap.org)' />
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='OpenStreetMap server' />
<node id='-101935' action='modify' visible='true' lat='50.33912764473' lon='19.49012170189' />
<node id='-101936' action='modify' visible='true' lat='50.33934777586' lon='19.4901206162' />
<node id='-101937' action='modify' visible='true' lat='50.33973987609' lon='19.49013168725' />
<node id='-101938' action='modify' visible='true' lat='50.34002810377' lon='19.49013757663' />
<way id='-102731' action='modify' visible='true'>
<nd ref='-101935' />
<nd ref='-101936' />
<nd ref='-101937' />
<nd ref='-101938' />
<tag k='barrier' v='fence' />
</way>
</osm>
17 changes: 17 additions & 0 deletions test/data/share_nodes_with_object/way_building.osm
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version='1.0' encoding='UTF-8'?>
<osm version='0.6' generator='JOSM'>
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='CGImap 0.8.6 (1354445 spike-07.openstreetmap.org)' />
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='OpenStreetMap server' />
<node id='-101926' action='modify' visible='true' lat='50.33934776418' lon='19.49012060776' />
<node id='-101927' action='modify' visible='true' lat='50.33973987789' lon='19.49013168725' />
<node id='-101928' action='modify' visible='true' lat='50.33934921461' lon='19.48952072093' />
<node id='-101929' action='modify' visible='true' lat='50.33973924315' lon='19.4895236035' />
<way id='-102444' action='modify' visible='true'>
<nd ref='-101926' />
<nd ref='-101928' />
<nd ref='-101929' />
<nd ref='-101927' />
<nd ref='-101926' />
<tag k='building' v='yes' />
</way>
</osm>
16 changes: 16 additions & 0 deletions test/data/share_nodes_with_object/way_waterway.osm
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version='1.0' encoding='UTF-8'?>
<osm version='0.6' generator='JOSM'>
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='CGImap 0.8.6 (1354445 spike-07.openstreetmap.org)' />
<bounds minlat='50.3389967' minlon='19.4891453' maxlat='50.3400923' maxlon='19.4917202' origin='OpenStreetMap server' />
<node id='-101930' action='modify' visible='true' lat='50.33912764473' lon='19.49012170189' />
<node id='-101931' action='modify' visible='true' lat='50.33934776521' lon='19.49012060937' />
<node id='-101932' action='modify' visible='true' lat='50.33973987878' lon='19.49013168725' />
<node id='-101933' action='modify' visible='true' lat='50.34002810377' lon='19.49013757663' />
<way id='-102727' action='modify' visible='true'>
<nd ref='-101930' />
<nd ref='-101931' />
<nd ref='-101932' />
<nd ref='-101933' />
<tag k='waterway' v='ditch' />
</way>
</osm>
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package org.openstreetmap.josm.plugins.plbuildings;

import mockit.Mock;
import mockit.MockUp;
import org.junit.Rule;
import org.junit.Test;
import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.data.osm.Way;
import org.openstreetmap.josm.testutils.JOSMTestRules;

import java.io.File;

import static org.junit.Assert.*;
import static org.openstreetmap.josm.plugins.plbuildings.ImportUtils.importOsmFile;

public class ShareNodesWithObjectTest {
@Rule
public JOSMTestRules rules = new JOSMTestRules().main();

static {
new MockUp<BuildingsAction>(){
@Mock
public DataSet getBuildingsAtCurrentLocation(){
DataSet importData = importOsmFile(
new File("test/data/share_nodes_with_object/import_building.osm"),
""
);
assertNotNull(importData);

return importOsmFile(new File("test/data/share_nodes_with_object/import_building.osm"), "");
}
};
}
@Test
public void testImportBuildingShareTwoNodesWithBuilding(){
DataSet ds = importOsmFile(new File("test/data/share_nodes_with_object/way_building.osm"), "");
assertNotNull(ds);

BuildingsAction.performBuildingImport(ds);

Way building = (Way) ds.getWays().toArray()[0]; // doesn't matter which one
assertEquals(
building.getNodes().stream()
.skip(1)
.filter(node -> node.isReferredByWays(2))
.count(),
2);
}

@Test
public void testImportBuildingNotShareNodesWithNotBuildingWay(){
DataSet ds = importOsmFile(new File("test/data/share_nodes_with_object/way_waterway.osm"), "");
assertNotNull(ds);

BuildingsAction.performBuildingImport(ds);

Way building = (Way) ds.getWays().stream().filter(way -> way.hasKey("building")).toArray()[0];
assertEquals(
building.getNodes().stream()
.skip(1)
.filter(node -> node.isReferredByWays(2))
.count(),
0);
}

@Test
public void testImportBuildingNotShareNodesWithBarrier(){
DataSet ds = importOsmFile(new File("test/data/share_nodes_with_object/way_barrier.osm"), "");
assertNotNull(ds);

BuildingsAction.performBuildingImport(ds);

Way building = (Way) ds.getWays().stream().filter(way -> way.hasKey("building")).toArray()[0];
assertEquals(
building.getNodes().stream()
.skip(1)
.filter(node -> node.isReferredByWays(2))
.count(),
0);
}

@Test
public void testImportBuildingNotShareNodeWithNodeObject(){
DataSet ds = importOsmFile(new File("test/data/share_nodes_with_object/node_shop.osm"), "");
assertNotNull(ds);

BuildingsAction.performBuildingImport(ds);

Way building = (Way) ds.getWays().stream().filter(way -> way.hasKey("building")).toArray()[0];

// -1 is for avoid duplicate of nodes from closed way and +1 is for looking for node object
assertEquals(ds.getNodes().size(), (building.getNodesCount() - 1) + 1);
}
}

0 comments on commit 87ff710

Please sign in to comment.