From 8632e73020fa6722d05a8bf6389bde052df4f032 Mon Sep 17 00:00:00 2001 From: Brian Jorgenson <22751140+brianjor@users.noreply.github.com> Date: Wed, 8 Dec 2021 11:48:49 -0800 Subject: [PATCH] [Fix Suggestions] Intersecting Buildings Check (#640) * add fix suggestion * updated tests * spotless * check --- .../IntersectingBuildingsCheck.java | 9 +++++ .../IntersectingBuildingsCheckTest.java | 34 ++++++++++++++++++- .../IntersectingBuildingsTestCaseRule.java | 8 ++--- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheck.java index 0179ff338..2d1c38a84 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheck.java @@ -8,7 +8,10 @@ import org.openstreetmap.atlas.checks.flag.CheckFlag; import org.openstreetmap.atlas.checks.utility.IntersectionUtilities; import org.openstreetmap.atlas.geography.Polygon; +import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteEntity; import org.openstreetmap.atlas.geography.atlas.items.Area; +import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.tags.BuildingTag; import org.openstreetmap.atlas.utilities.configuration.Configuration; @@ -151,10 +154,12 @@ protected Optional flag(final AtlasObject object) // Get object and otherBuilding as a Surfaces final Surface objectAsSurface = ((Area) object).asPolygon().surface(); final Surface otherBuildingAsSurface = otherBuilding.asPolygon().surface(); + final FeatureChange featureChange; // If object is larger than otherBuilding, the instruction states object contains // otherBuilding if (objectAsSurface.isLargerThan(otherBuildingAsSurface)) { + featureChange = FeatureChange.remove(CompleteEntity.shallowFrom(otherBuilding)); flag.addObject(otherBuilding, this.getLocalizedInstruction(2, object.getOsmIdentifier(), otherBuilding.getOsmIdentifier())); } @@ -162,15 +167,19 @@ protected Optional flag(final AtlasObject object) // contains object else if (objectAsSurface.isLessThan(otherBuildingAsSurface)) { + featureChange = FeatureChange + .remove(CompleteEntity.shallowFrom((AtlasEntity) object)); flag.addObject(otherBuilding, this.getLocalizedInstruction(2, otherBuilding.getOsmIdentifier(), object.getOsmIdentifier())); } // If object and otherBuilding are equal, the instruction states that they overlap else { + featureChange = FeatureChange.remove(CompleteEntity.shallowFrom(otherBuilding)); flag.addObject(otherBuilding, this.getLocalizedInstruction(0, object.getOsmIdentifier(), otherBuilding.getOsmIdentifier())); } + flag.addFixSuggestion(featureChange); this.markAsFlagged(uniqueIdentifier); hadIntersection = true; } diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheckTest.java index f45154e8f..2aa65e805 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheckTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsCheckTest.java @@ -1,11 +1,18 @@ package org.openstreetmap.atlas.checks.validation.intersections; +import java.util.Arrays; +import java.util.List; +import java.util.Set; + import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver; import org.openstreetmap.atlas.checks.flag.CheckFlag; import org.openstreetmap.atlas.checks.validation.verifier.ConsumerBasedExpectedCheckVerifier; +import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; +import org.openstreetmap.atlas.geography.atlas.items.Area; +import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; /** * {@link IntersectingBuildingsCheck} unit test @@ -34,6 +41,10 @@ public void testContainsBuildingAtlas() Assert.assertEquals(2, flag.getFlaggedObjects().size()); Assert.assertTrue(flag.getInstructions() .contains("Building (id=1234567) contains building (id=2234567).")); + + final FeatureChange suggestion = flag.getFixSuggestions().iterator().next(); + final Area after = (Area) suggestion.getAfterView(); + Assert.assertEquals(2234567, after.getOsmIdentifier()); }); } @@ -46,7 +57,12 @@ public void testDuplicateBuildingsAtlas() this.verifier.verify(flag -> { Assert.assertEquals(2, flag.getFlaggedObjects().size()); - Assert.assertTrue(flag.getInstructions().contains("overlapped by another building")); + Assert.assertTrue(flag.getInstructions().contains( + "Building (id=1234567) is overlapped by another building (id=2234567).")); + + final FeatureChange suggestion = flag.getFixSuggestions().iterator().next(); + final Area after = (Area) suggestion.getAfterView(); + Assert.assertEquals(2234567, after.getOsmIdentifier()); }); } @@ -75,15 +91,23 @@ public void testSeveralDuplicateBuildingsAtlas() // First building overlaps with all other buildings final CheckFlag firstFlag = flags.get(0); Assert.assertEquals(4, firstFlag.getFlaggedObjects().size()); + final Set firstSuggestions = firstFlag.getFixSuggestions(); + Assert.assertTrue(this.suggestionsContainIds(firstSuggestions, + Arrays.asList(1234567L, 2234567L, 3234567L))); // Second building overlaps with the third and fourth one (it's overlap with first is // already flagged) final CheckFlag secondFlag = flags.get(1); Assert.assertEquals(3, secondFlag.getFlaggedObjects().size()); + final Set secondSuggestions = secondFlag.getFixSuggestions(); + Assert.assertTrue(this.suggestionsContainIds(secondSuggestions, + Arrays.asList(2234567L, 3234567L))); // Third building's overlap with fourth building will be flagged with the third flag final CheckFlag thirdFlag = flags.get(2); Assert.assertEquals(2, thirdFlag.getFlaggedObjects().size()); + final Set thirdSuggestions = thirdFlag.getFixSuggestions(); + Assert.assertTrue(this.suggestionsContainIds(thirdSuggestions, List.of(3234567L))); }); } @@ -104,6 +128,7 @@ public void testSmallIntersectionBuildingsAtlas() { Assert.assertEquals(2, flag.getFlaggedObjects().size()); Assert.assertTrue(flag.getInstructions().contains("intersects with another building")); + Assert.assertTrue(flag.getFixSuggestions().isEmpty()); }); } @@ -115,4 +140,11 @@ public void testSmallIntersectionBuildingsAtlasWithIncreasedIntersectionLowerLim "{\"IntersectingBuildingsCheck\": {\"intersection.lower.limit\": 0.15}}"))); this.verifier.verifyEmpty(); } + + private boolean suggestionsContainIds(final Set suggestions, + final List ids) + { + return suggestions.stream().map(FeatureChange::getAfterView) + .map(AtlasEntity::getOsmIdentifier).allMatch(ids::contains); + } } diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsTestCaseRule.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsTestCaseRule.java index 3d92f441b..c3ea884b8 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsTestCaseRule.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/IntersectingBuildingsTestCaseRule.java @@ -60,11 +60,11 @@ public class IntersectingBuildingsTestCaseRule extends CoreTestRule @TestAtlas(areas = { // a building - @Area(coordinates = { @Loc(value = TEST_1), @Loc(value = TEST_3), @Loc(value = TEST_4), - @Loc(value = TEST_2) }, tags = { "building=yes" }), + @Area(id = "1234567000000", coordinates = { @Loc(value = TEST_1), @Loc(value = TEST_3), + @Loc(value = TEST_4), @Loc(value = TEST_2) }, tags = { "building=yes" }), // another building with same footprint - @Area(coordinates = { @Loc(value = TEST_1), @Loc(value = TEST_3), @Loc(value = TEST_4), - @Loc(value = TEST_2) }, tags = { "building=yes" }) }) + @Area(id = "2234567000000", coordinates = { @Loc(value = TEST_1), @Loc(value = TEST_3), + @Loc(value = TEST_4), @Loc(value = TEST_2) }, tags = { "building=yes" }) }) private Atlas duplicateBuildingsAtlas; @TestAtlas(areas = {