Fix #848 - new [SQLFeatureStore] Deleting features fails for certain feature type prefix configurations

.travis.yml

@@ -12,4 +12,4 @@ script: "travis_retry mvn verify -Pintegration-tests -B -V"
 env: MAVEN_OPTS="-XX:MaxPermSize=256m"
 cache:
   directories:
-    - $HOME/.m2/repository/
+    - $HOME/.m2/repository/

deegree-core/deegree-core-commons/src/main/java/org/deegree/commons/utils/ArrayUtils.java

@@ -38,11 +38,7 @@
 import static java.lang.Double.parseDouble;
 import static java.lang.Float.parseFloat;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.List;
-import java.util.StringTokenizer;
+import java.util.*;
 
 /**
  * This is a collection of some methods that work with arrays and lists, like join or removeAll. It is complementary to
@@ -339,4 +335,19 @@ public static float[] splitAsFloats( String str, String delim )
         return fs;
     }
 
+    /**
+     * Sorts all strings by length, descending
+     *
+     * @param strings array of non-null strings, but strings can be empty
+     */
+    public static void sortByLengthDescending( String[] strings ) {
+        Arrays.sort( strings, new Comparator<String>() {
+            @Override
+            public int compare( String o1, String o2 ) {
+                int len1 = o1.length();
+                int len2 = o2.length();
+                return ( len1 > len2 ? -1 : ( len1 == len2 ? 0 : 1 ) );
+            }
+        } );
+    }
 }

deegree-core/deegree-core-commons/src/test/java/org/deegree/commons/utils/ArrayToolsTest.java → deegree-core/deegree-core-commons/src/test/java/org/deegree/commons/utils/ArrayUtilsTest.java

@@ -35,29 +35,26 @@
  ----------------------------------------------------------------------------*/
 package org.deegree.commons.utils;
 
-import static org.deegree.commons.utils.ArrayUtils.join;
-import static org.deegree.commons.utils.ArrayUtils.removeAll;
-import static org.junit.Assert.assertEquals;
+import org.junit.Test;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
 
-import org.junit.Test;
+import static org.deegree.commons.utils.ArrayUtils.*;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
 
 /**
- * 
- * 
  * @author <a href="mailto:tonnhofer@lat-lon.de">Oliver Tonnhofer</a>
  * @author last edited by: $Author$
- * 
  * @version $Revision$, $Date$
- * 
  */
-public class ArrayToolsTest {
+public class ArrayUtilsTest {
 
     /**
-     * Test method for {@link org.deegree.commons.utils.ArrayUtils#join(String delimiter, String... strings )}.
+     * Test method for {@link org.deegree.commons.utils.ArrayUtils#join(String delimiter, String... strings)}.
      */
     @Test
     public void testJoin() {
@@ -68,7 +65,7 @@ public void testJoin() {
     }
 
     /**
-     * Test method for {@link org.deegree.commons.utils.ArrayUtils#join(String, List)}.
+     * Test method for {@link org.deegree.commons.utils.ArrayUtils#join(String, Collection)}.
      */
     @Test
     public void testJoinList() {
@@ -85,7 +82,7 @@ public void testJoinList() {
     }
 
     /**
-     * Test method for {@link org.deegree.commons.utils.ArrayUtils#join(String delimiter, int[] values )}.
+     * Test method for {@link org.deegree.commons.utils.ArrayUtils#join(String delimiter, int[] values)}.
      */
     @Test
     public void testjoinInts() {
@@ -114,4 +111,12 @@ private <T> void arrayCompare( T[] actual, T... expected ) {
             assertEquals( expected[i], actual[i] );
         }
     }
+
+    @Test
+    public void sortStringsByLength() throws Exception {
+        String[] strings = { "a", "ccc", "bb" };
+        sortByLengthDescending( strings );
+
+        assertArrayEquals( strings, new String[] { "ccc", "bb", "a" } );
+    }
 }

deegree-datastores/deegree-featurestores/deegree-featurestore-sql/src/main/java/org/deegree/feature/persistence/sql/id/IdAnalyzer.java

@@ -35,21 +35,22 @@
  ----------------------------------------------------------------------------*/
 package org.deegree.feature.persistence.sql.id;
 
-import java.util.HashMap;
-import java.util.Map;
-
 import org.deegree.feature.persistence.sql.FeatureTypeMapping;
 import org.deegree.feature.persistence.sql.MappedAppSchema;
 import org.deegree.feature.types.FeatureType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.deegree.commons.utils.ArrayUtils.sortByLengthDescending;
+
 /**
  * Helper class for analyzing if a given feature or geometry id can be attributed to a certain feature type.
- * 
+ *
  * @author <a href="mailto:schneider@lat-lon.de">Markus Schneider</a>
  * @author last edited by: $Author$
- * 
  * @version $Revision$, $Date$
  */
 public class IdAnalyzer {
@@ -58,13 +59,15 @@ public class IdAnalyzer {
 
     private final Map<String, FeatureType> prefixToFt = new HashMap<String, FeatureType>();
 
+    // this is used to match ids, so the best match (with the longest identical prefix) is found first
+    private final String [] prefixKeysSortedByLengthDesc;
+
     private final MappedAppSchema schema;
 
     /**
      * Creates a new {@link IdAnalyzer} instance for the given {@link MappedAppSchema}.
-     * 
-     * @param schema
-     *            application schema with mapping information, must not be <code>null</code>
+     *
+     * @param schema application schema with mapping information, must not be <code>null</code>
      */
     public IdAnalyzer( MappedAppSchema schema ) {
         this.schema = schema;
@@ -80,38 +83,44 @@ public IdAnalyzer( MappedAppSchema schema ) {
                 }
             }
         }
+        prefixKeysSortedByLengthDesc = prefixToFt.keySet().toArray( new String[0] );
+        sortByLengthDescending( prefixKeysSortedByLengthDesc );
     }
 
+    /**
+     * @param featureOrGeomId feature or geometry ID
+     * @return never <code>null</code>
+     * @throws IllegalArgumentException if given ID not found
+     */
     public IdAnalysis analyze( String featureOrGeomId ) {
+        FeatureType ft = getFeatureType( featureOrGeomId );
+        FIDMapping fidMapping = schema.getFtMapping( ft.getName() ).getFidMapping();
+        String idRemainder = featureOrGeomId.substring( fidMapping.getPrefix().length() );
+        return new IdAnalysis( ft, idRemainder, fidMapping );
+    }
 
-        FeatureType ft = null;
-        // TODO implement this more efficiently
-        for ( String prefix : prefixToFt.keySet() ) {
+    private FeatureType getFeatureType( String featureOrGeomId ) {
+        for ( String prefix : prefixKeysSortedByLengthDesc ) {
             if ( featureOrGeomId.startsWith( prefix ) ) {
-                ft = prefixToFt.get( prefix );
-                break;
+                return prefixToFt.get( prefix );
             }
         }
-        if ( ft == null ) {
-            StringBuilder sb = new StringBuilder( "Unable to determine feature type for id '" );
-            sb.append( featureOrGeomId );
-            sb.append( "'. Given id does not start with a configured identifier prefix. Known prefixes are: " );
-            boolean first = true;
-            for ( String prefix : prefixToFt.keySet() ) {
-                if ( !first ) {
-                    sb.append( ", " );
-                }
-                sb.append( "'" );
-                sb.append( prefix );
-                sb.append( "'" );
-                first = false;
+
+        StringBuilder errorMsg = new StringBuilder( "Unable to determine feature type for id '" );
+        errorMsg.append( featureOrGeomId );
+        errorMsg.append( "'. Given id does not start with a configured identifier prefix. Known prefixes are: " );
+        boolean first = true;
+        for ( String prefix : prefixToFt.keySet() ) {
+            if ( !first ) {
+                errorMsg.append( ", " );
             }
-            sb.append( "." );
-            throw new IllegalArgumentException( sb.toString() );
+            errorMsg.append( "'" );
+            errorMsg.append( prefix );
+            errorMsg.append( "'" );
+            first = false;
         }
-
-        FIDMapping fidMapping = schema.getFtMapping( ft.getName() ).getFidMapping();
-        String idRemainder = featureOrGeomId.substring( fidMapping.getPrefix().length() );
-        return new IdAnalysis( ft, idRemainder, fidMapping );
+        errorMsg.append( "." );
+        throw new IllegalArgumentException( errorMsg.toString() );
     }
+
 }

deegree-datastores/deegree-featurestores/deegree-featurestore-sql/src/test/java/org/deegree/feature/persistence/sql/id/IdAnalyzerTest.java

@@ -0,0 +1,133 @@
+/*----------------------------------------------------------------------------
+ This file is part of deegree, http://deegree.org/
+ Copyright (C) 2001-2017 by:
+ - Department of Geography, University of Bonn -
+ and
+ - lat/lon GmbH -
+
+ This library is free software; you can redistribute it and/or modify it under
+ the terms of the GNU Lesser General Public License as published by the Free
+ Software Foundation; either version 2.1 of the License, or (at your option)
+ any later version.
+ This library is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ details.
+ You should have received a copy of the GNU Lesser General Public License
+ along with this library; if not, write to the Free Software Foundation, Inc.,
+ 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+ Contact information:
+
+ lat/lon GmbH
+ Aennchenstr. 19, 53177 Bonn
+ Germany
+ http://lat-lon.de/
+
+ Department of Geography, University of Bonn
+ Prof. Dr. Klaus Greve
+ Postfach 1147, 53001 Bonn
+ Germany
+ http://www.geographie.uni-bonn.de/deegree/
+
+ e-mail: info@deegree.org
+ ----------------------------------------------------------------------------*/
+package org.deegree.feature.persistence.sql.id;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import javax.xml.namespace.QName;
+
+import org.deegree.commons.jdbc.SQLIdentifier;
+import org.deegree.commons.jdbc.TableName;
+import org.deegree.commons.tom.gml.property.PropertyType;
+import org.deegree.commons.tom.primitive.BaseType;
+import org.deegree.commons.utils.Pair;
+import org.deegree.feature.persistence.sql.FeatureTypeMapping;
+import org.deegree.feature.persistence.sql.MappedAppSchema;
+import org.deegree.feature.persistence.sql.rules.Mapping;
+import org.deegree.feature.types.FeatureType;
+import org.deegree.feature.types.GenericFeatureType;
+import org.junit.Test;
+
+/**
+ * {@link IdAnalyzer} tests for checking the correct deriving of a feature type from a feature id.
+ */
+public class IdAnalyzerTest {
+
+    private IdAnalyzer idAnalyzer;
+
+    @Test
+    public void analyzeFeatureIds() {
+        idAnalyzer = setupAnalyzerScenario( "APP_FEATURE1_", "APP_FEATURE2_", "APP_FEATURE3_" );
+        assertEquals( "APP_FEATURE1_", analyzeFeatureType( "APP_FEATURE1_1" ) );
+        assertEquals( "APP_FEATURE2_", analyzeFeatureType( "APP_FEATURE2_1" ) );
+        assertEquals( "APP_FEATURE3_", analyzeFeatureType( "APP_FEATURE3_1" ) );
+    }
+
+    @Test
+    public void analyzeGeometryIds() {
+        idAnalyzer = setupAnalyzerScenario( "APP_FEATURE1_", "APP_FEATURE2_", "APP_FEATURE3_" );
+        assertEquals( "APP_FEATURE1_", analyzeFeatureType( "APP_FEATURE1_1_APP_GEOM" ) );
+        assertEquals( "APP_FEATURE2_", analyzeFeatureType( "APP_FEATURE2_1_APP_GEOM" ) );
+        assertEquals( "APP_FEATURE3_", analyzeFeatureType( "APP_FEATURE3_1_APP_GEOM" ) );
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void analyzeNoMatch() {
+        idAnalyzer = setupAnalyzerScenario( "APP_FEATURE_" );
+        analyzeFeatureType( "BPP_FEATURE_1" );
+    }
+
+    @Test
+    public void analyzeFeatureIdMultiplePrefixMatches() {
+        // these tests used to fail with Java 8 (see https://github.com/deegree/deegree3/issues/848)
+        idAnalyzer = setupAnalyzerScenario( "APP_FEATURE_", "APP_FEATURE_X_" );
+        assertEquals( "APP_FEATURE_X_", analyzeFeatureType( "APP_FEATURE_X_1" ) );
+        // actual production case that used to fail
+        idAnalyzer = setupAnalyzerScenario( "IMRO_GEOMETRIESTRUCTUURVISIEOBJECT_", "IMRO_GEOMETRIESTRUCTUURVISIEOBJECT_P_" );
+        assertEquals( "IMRO_GEOMETRIESTRUCTUURVISIEOBJECT_P_", analyzeFeatureType( "IMRO_GEOMETRIESTRUCTUURVISIEOBJECT_P_1" ) );
+    }
+
+    private String analyzeFeatureType( final String fid ) {
+        return idAnalyzer.analyze( fid ).getFeatureType().getName().getLocalPart();
+    }
+
+    private IdAnalyzer setupAnalyzerScenario( final String... idPrefixes ) {
+        final FeatureType[] fts = new FeatureType[idPrefixes.length];
+        final FeatureTypeMapping[] ftMappings = new FeatureTypeMapping[idPrefixes.length];
+        int i = 0;
+        for ( final String idPrefix : idPrefixes ) {
+            fts[i] = buildFeatureType( idPrefix );
+            ftMappings[i] = buildFeatureTypeMapping( fts[i], idPrefix );
+            i++;
+        }
+        final MappedAppSchema schema = new MappedAppSchema( fts, null, null, null, ftMappings, null, null, null, false,
+                                                            null, null, null );
+        return new IdAnalyzer( schema );
+    }
+
+    private FeatureType buildFeatureType( final String localName ) {
+        return new GenericFeatureType( buildQName( localName ), Collections.<PropertyType> emptyList(), false );
+    }
+
+    private FeatureTypeMapping buildFeatureTypeMapping( final FeatureType ft, final String fidPrefix ) {
+        final FIDMapping fidMapping = buildFidMapping( fidPrefix );
+        return new FeatureTypeMapping( ft.getName(), new TableName( ft.getName().getLocalPart() ), fidMapping,
+                                       Collections.<Mapping> emptyList() );
+    }
+
+    private FIDMapping buildFidMapping( final String fidPrefix ) {
+        final List<Pair<SQLIdentifier, BaseType>> fidColumns = new ArrayList<Pair<SQLIdentifier, BaseType>>();
+        fidColumns.add( new Pair<SQLIdentifier, BaseType>( new SQLIdentifier( "id" ), BaseType.INTEGER ) );
+        return new FIDMapping( fidPrefix, "_", fidColumns, null );
+    }
+
+    private QName buildQName( final String localPart ) {
+        return new QName( "http://www.deegree.org/app", localPart, "app" );
+    }
+}