ref #9502 fix handling for multiple parents in distribution filter

diff --git a/cdmlib-services/src/main/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilities.java b/cdmlib-services/src/main/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilities.java
index fae3f42604be11d48426e5f7c12184434975a240..a529a3a7ff5a22662ba86de6924973dbf07ad4cc 100644 (file)
--- a/cdmlib-services/src/main/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilities.java
+++ b/cdmlib-services/src/main/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilities.java
@@ -870,7 +870,12 @@ public class DistributionServiceUtilities {
         return result;
     }
 
-
+    /**
+     * Removes all distributions that have an area being a parent of
+     * anothers distributions area. E.g. removes distribution for "Europe"
+     * if a distribution for "France" exists in the list, where Europe
+     * is a direct parent for France.
+     */
     private static void handleSubAreaPreferenceRule(SetMap<NamedArea, Distribution> filteredDistributions,
             TermTree<NamedArea> areaTree) {
 
@@ -880,11 +885,11 @@ public class DistributionServiceUtilities {
             if(removeCandidatesArea.contains(area)){
                 continue;
             }
-            //xx;
-            NamedArea parent = parentMap.getFirstValue(area);
-            if(parent != null && filteredDistributions.containsKey(parent)){
-                removeCandidatesArea.add(parent);
-            }
+            parentMap.get(area).forEach(parent->{
+                if(parent != null && filteredDistributions.containsKey(parent)){
+                    removeCandidatesArea.add(parent);
+                }
+            });
         }
         for(NamedArea removeKey : removeCandidatesArea){
             filteredDistributions.remove(removeKey);

diff --git a/cdmlib-services/src/test/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilitiesTest.java b/cdmlib-services/src/test/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilitiesTest.java
index 4523832661b626eeb5e6814e1de09f1f9215bf0d..ae2549f0f0698b43a4448a7243e2be55dcb6d627 100644 (file)
--- a/cdmlib-services/src/test/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilitiesTest.java
+++ b/cdmlib-services/src/test/java/eu/etaxonomy/cdm/api/service/geo/DistributionServiceUtilitiesTest.java
@@ -204,6 +204,8 @@ public class DistributionServiceUtilitiesTest extends TermTestBase {
         ileDeFranceDist = Distribution.NewInstance(ileDeFrance, PresenceAbsenceTerm.CULTIVATED());
         spainDist = Distribution.NewInstance(spain, PresenceAbsenceTerm.NATURALISED());
         westEuropeDist = null;
+        saarDist = null;
+        addDefaultDistributions();
     }
 
     private void createDefaultDistributionDtos() {
@@ -219,6 +221,27 @@ public class DistributionServiceUtilitiesTest extends TermTestBase {
         if (westEuropeDist != null) {
             distributionDtos.add(dist2Dto(westEuropeDist, parentAreaMap));
         }
+        if (saarDist != null) {
+            distributionDtos.add(dist2Dto(saarDist, parentAreaMap));
+        }
+    }
+
+    //only needed for testFilterDistributions_multipleParents which uses these data
+    private void addDefaultDistributions() {
+        distributions.clear();
+        distributions.add(europeDist);
+        distributions.add(germanyDist);
+        distributions.add(bawueDist);
+        distributions.add(berlinDist);
+        distributions.add(italyDist);
+        distributions.add(ileDeFranceDist);
+        distributions.add(spainDist);
+        if (westEuropeDist != null) {
+            distributions.add(westEuropeDist);
+        }
+        if (saarDist != null) {
+            distributions.add(saarDist);
+        }
     }
 
     private DistributionDto dist2Dto(Distribution distribution, SetMap<NamedArea, NamedArea> parentAreaMap) {
@@ -231,6 +254,13 @@ public class DistributionServiceUtilitiesTest extends TermTestBase {
         westEuropeDist = Distribution.NewInstance(westEurope, PresenceAbsenceTerm.NATURALISED());
     }
 
+    private void createAndAddSaarAndWestEuropeDistribution() {
+        createWesternEuropeDistribution();
+        saarDist = Distribution.NewInstance(saar, PresenceAbsenceTerm.NATURALISED());
+        distributions.add(westEuropeDist);
+        distributions.add(saarDist);
+    }
+
     @Test
     public void testFilterDistributions_aggregated(){
 
@@ -576,6 +606,46 @@ public class DistributionServiceUtilitiesTest extends TermTestBase {
         Assert.assertEquals(jugoslavia, filteredDistributions.iterator().next().getArea());
     }
 
+
+    @Test
+    public void testFilterDistributions_multipleParents(){
+        subAreaPreference = true;
+        statusOrderPreference = true;
+        boolean ignoreDistributionStatusUndefined = false;
+        boolean keepFallBackOnlyIfNoSubareaDataExists = true;
+
+        setupTreeTest();  //we use tree test setup here
+        TermTree<NamedArea> areaTree = this.areaTree;
+        createDefaultDistributions();
+
+        //test default (without 2 parents)
+        distributions.remove(bawueDist);
+        distributions.remove(berlinDist);
+        filteredDistributions = DistributionServiceUtilities.filterDistributions(distributions, areaTree,
+                hideMarkedAreas, NO_PREFER_AGGREGATED, statusOrderPreference, subAreaPreference,
+                keepFallBackOnlyIfNoSubareaDataExists, ignoreDistributionStatusUndefined);
+        Assert.assertEquals(4, filteredDistributions.size());
+        List<NamedArea> areaList = filteredDistributions.stream().map(fd->fd.getArea()).collect(toList);
+        Assert.assertTrue(areaList.contains(germany));
+        Assert.assertTrue(areaList.contains(ileDeFrance));
+        Assert.assertTrue(areaList.contains(italy));
+        Assert.assertTrue(areaList.contains(spain));
+
+        //add Saar which is child of Germany and West Europe
+        createAndAddSaarAndWestEuropeDistribution();
+        filteredDistributions = DistributionServiceUtilities.filterDistributions(distributions, areaTree,
+                hideMarkedAreas, NO_PREFER_AGGREGATED, statusOrderPreference, subAreaPreference,
+                keepFallBackOnlyIfNoSubareaDataExists, ignoreDistributionStatusUndefined);
+        Assert.assertEquals(4, filteredDistributions.size());
+        List<NamedArea> areaList2 = filteredDistributions.stream().map(fd->fd.getArea()).collect(toList);
+        Assert.assertTrue(areaList2.contains(saar));
+        Assert.assertFalse("Should not contain West Europe as it is Saar parent", areaList2.contains(westEurope));
+        Assert.assertFalse("Should not contain Germany as it is Saar parent", areaList2.contains(germany));
+        Assert.assertTrue(areaList2.contains(ileDeFrance));
+        Assert.assertTrue(areaList.contains(italy));
+        Assert.assertTrue(areaList.contains(spain));
+    }
+
     @Test
     public <TN extends TreeNode<Set<DistributionDto>,NamedAreaDto>> void testBuildOrderedTreeDto() {
         testBuildOrderedTreeDto(false);