Skip to content

Commit

Permalink
[Fix Suggestions] Intersecting Buildings Check (osmlab#640)
Browse files Browse the repository at this point in the history
* add fix suggestion

* updated tests

* spotless

* check
  • Loading branch information
brianjor authored Dec 8, 2021
1 parent e7416af commit 8632e73
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -151,26 +154,32 @@ protected Optional<CheckFlag> 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()));
}
// If object is smaller than otherBuilding, the instruction states otherBuilding
// 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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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());
});
}

Expand All @@ -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());
});
}

Expand Down Expand Up @@ -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<FeatureChange> 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<FeatureChange> 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<FeatureChange> thirdSuggestions = thirdFlag.getFixSuggestions();
Assert.assertTrue(this.suggestionsContainIds(thirdSuggestions, List.of(3234567L)));
});
}

Expand All @@ -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());
});
}

Expand All @@ -115,4 +140,11 @@ public void testSmallIntersectionBuildingsAtlasWithIncreasedIntersectionLowerLim
"{\"IntersectingBuildingsCheck\": {\"intersection.lower.limit\": 0.15}}")));
this.verifier.verifyEmpty();
}

private boolean suggestionsContainIds(final Set<FeatureChange> suggestions,
final List<Long> ids)
{
return suggestions.stream().map(FeatureChange::getAfterView)
.map(AtlasEntity::getOsmIdentifier).allMatch(ids::contains);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down

0 comments on commit 8632e73

Please sign in to comment.