Fixed relation return types in DynamicRelation.allKnownOsmMembers() (#…

…227) * Fixed relation return types in allKnownOsmMembers * Added a unit test to verify
osmlab · Sep 25, 2018 · 47c6e7f · 47c6e7f
1 parent 36f9816
commit 47c6e7f
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 26 deletions.
diff --git a/src/main/java/org/openstreetmap/atlas/geography/atlas/dynamic/DynamicRelation.java b/src/main/java/org/openstreetmap/atlas/geography/atlas/dynamic/DynamicRelation.java
@@ -31,11 +31,7 @@ protected DynamicRelation(final DynamicAtlas atlas, final long identifier)
     @Override
     public RelationMemberList allKnownOsmMembers()
     {
-        /*
-         * TODO this will return AtlasEntities which are not of type DynamicX. Ideally, we should be
-         * recreating returned entities as DynamicX instead of the underlying PackedX or MultiX.
-         */
-        return subRelation().allKnownOsmMembers();
+        return getRelationMembersAsDynamicEntities(subRelation().allKnownOsmMembers());
     }
 
     @Override
@@ -61,10 +57,34 @@ public Map<String, String> getTags()
     @Override
     public RelationMemberList members()
     {
-        final RelationMemberList subRelationMemberList = subRelation().members();
+        return getRelationMembersAsDynamicEntities(subRelation().members());
+    }
+
+    @Override
+    public long osmRelationIdentifier()
+    {
+        return subRelation().osmRelationIdentifier();
+    }
+
+    @Override
+    public Set<Relation> relations()
+    {
+        return subRelation().relations().stream()
+                .map(relation -> new DynamicRelation(dynamicAtlas(), relation.getIdentifier()))
+                .collect(Collectors.toSet());
+    }
+
+    private DynamicAtlas dynamicAtlas()
+    {
+        return (DynamicAtlas) this.getAtlas();
+    }
+
+    private RelationMemberList getRelationMembersAsDynamicEntities(
+            final RelationMemberList memberList)
+    {
         final List<RelationMember> newMemberList = new ArrayList<>();
 
-        for (final RelationMember member : subRelationMemberList)
+        for (final RelationMember member : memberList)
         {
             final AtlasEntity entity = member.getEntity();
             AtlasEntity dynamicEntity = null;
@@ -98,25 +118,6 @@ public RelationMemberList members()
         return new RelationMemberList(newMemberList);
     }
 
-    @Override
-    public long osmRelationIdentifier()
-    {
-        return subRelation().osmRelationIdentifier();
-    }
-
-    @Override
-    public Set<Relation> relations()
-    {
-        return subRelation().relations().stream()
-                .map(relation -> new DynamicRelation(dynamicAtlas(), relation.getIdentifier()))
-                .collect(Collectors.toSet());
-    }
-
-    private DynamicAtlas dynamicAtlas()
-    {
-        return (DynamicAtlas) this.getAtlas();
-    }
-
     private Relation subRelation()
     {
         final Relation result = dynamicAtlas().subRelation(this.identifier);

diff --git a/src/test/java/org/openstreetmap/atlas/geography/atlas/dynamic/DynamicAtlasTest.java b/src/test/java/org/openstreetmap/atlas/geography/atlas/dynamic/DynamicAtlasTest.java
@@ -19,6 +19,7 @@
 import org.openstreetmap.atlas.geography.atlas.dynamic.rules.DynamicAtlasTestRule;
 import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity;
 import org.openstreetmap.atlas.geography.atlas.items.Relation;
+import org.openstreetmap.atlas.geography.atlas.items.RelationMember;
 import org.openstreetmap.atlas.geography.atlas.multi.MultiAtlas;
 import org.openstreetmap.atlas.geography.atlas.multi.MultiRelation;
 import org.openstreetmap.atlas.geography.atlas.packed.PackedRelation;
@@ -318,4 +319,50 @@ public void testRuleIntegrity()
         Assert.assertEquals("Found differences: " + new AtlasDelta(atlas, multiAtlas).toString(),
                 atlas, multiAtlas);
     }
+
+    @Test
+    public void testTypeOfReturnedRelationMembers()
+    {
+        final DynamicAtlas localDynamicAtlas;
+        final Map<Shard, Atlas> localStore = new HashMap<>();
+        localStore.put(new SlippyTile(0, 0, 0), this.rule.getAtlasForRelationsTest());
+        final Supplier<DynamicAtlasPolicy> localPolicySupplier = () -> new DynamicAtlasPolicy(
+                shard ->
+                {
+                    if (localStore.containsKey(shard))
+                    {
+                        return Optional.of(localStore.get(shard));
+                    }
+                    else
+                    {
+                        return Optional.empty();
+                    }
+                }, new SlippyTileSharding(0), new SlippyTile(0, 0, 0), Rectangle.MAXIMUM);
+        localDynamicAtlas = new DynamicAtlas(localPolicySupplier.get());
+
+        for (final Relation relation : localDynamicAtlas.relations())
+        {
+            for (final RelationMember member : relation.allKnownOsmMembers())
+            {
+                Assert.assertTrue(atlasEntityIsADynamicEntity(member.getEntity()));
+            }
+            for (final RelationMember member : relation.members())
+            {
+                Assert.assertTrue(atlasEntityIsADynamicEntity(member.getEntity()));
+            }
+        }
+    }
+
+    private boolean atlasEntityIsADynamicEntity(final AtlasEntity entity)
+    {
+        final boolean isDynamicPoint = entity instanceof DynamicPoint;
+        final boolean isDynamicLine = entity instanceof DynamicLine;
+        final boolean isDynamicArea = entity instanceof DynamicArea;
+        final boolean isDynamicNode = entity instanceof DynamicNode;
+        final boolean isDynamicEdge = entity instanceof DynamicEdge;
+        final boolean isDynamicRelation = entity instanceof DynamicRelation;
+
+        return isDynamicPoint || isDynamicLine || isDynamicArea || isDynamicNode || isDynamicEdge
+                || isDynamicRelation;
+    }
 }