[GEOS-10824] Replace TestDirectoryStoreFactorySpi by a DataAccessFactoryProducer

src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/RestconfigWfsTest.java

@@ -88,6 +88,7 @@ protected void onTearDown(SystemTestData testData) throws Exception {
                     + "<workspace>" //
                     + "<name>gsml</name>" //
                     + "</workspace>" //
+                    + "<type>Application Schema DataAccess</type>" //
                     + "<connectionParameters>" //
                     + "<entry key='dbtype'>app-schema</entry>" //
                     + "<entry key='url'>file:workspaces/gsml/MappedFeature/MappedFeature.xml</entry>" //

src/main/src/main/java/applicationContext.xml

@@ -358,4 +358,5 @@
   <bean id="xmlViewParamsFormatParser" class="org.geoserver.ows.kvp.XMLViewParamsFormatParser">
   </bean>
 
+  <bean id="defaultDataAccessFactoryProducer" class="org.geoserver.data.DefaultDataAccessFactoryProducer"/>
 </beans>

src/main/src/main/java/org/geoserver/catalog/ResourcePool.java

@@ -705,15 +705,12 @@ private void disableStoreInfoIfNeeded(
         connectionParameters =
                 ResourcePool.getParams(connectionParameters, catalog.getResourceLoader());
 
-        // obtain the factory
+        // obtain the factory, either using the "type", or if not found, using the parameters
         DataAccessFactory factory = null;
         try {
             factory = getDataStoreFactory(info);
         } catch (IOException e) {
-            throw new IOException(
-                    "Failed to find the datastore factory for "
-                            + info.getName()
-                            + ", did you forget to install the store extension jar?");
+            // ignoring since the error message is the same as for the null factory, see line below
         }
         if (factory == null) {
             throw new IOException(
@@ -725,9 +722,8 @@ private void disableStoreInfoIfNeeded(
 
         // ensure that the namespace parameter is set for the datastore
         if (!connectionParameters.containsKey("namespace") && params != null) {
-            // if we grabbed the factory, check that the factory actually supports
-            // a namespace parameter, if we could not get the factory, assume that
-            // it does
+            // if we grabbed the factory, check that the factory actually supports a namespace
+            // parameter, if we could not get the factory, assume that it does
             boolean supportsNamespace = false;
 
             for (Param p : params) {
@@ -749,8 +745,7 @@ private void disableStoreInfoIfNeeded(
             }
         }
 
-        // see if the store has a repository param, if so, pass the one wrapping
-        // the store
+        // see if the store has a repository param, if so, pass the one wrapping the store
         if (params != null) {
             for (Param p : params) {
                 if (Repository.class.equals(p.getType())) {
@@ -772,7 +767,19 @@ private void disableStoreInfoIfNeeded(
             }
         }
 
-        dataStore = DataStoreUtils.getDataAccess(connectionParameters);
+        // use the factory obtained through the lookup first
+        try {
+            dataStore = DataStoreUtils.getDataAccess(factory, connectionParameters);
+        } catch (IOException e) {
+            LOGGER.log(
+                    Level.INFO,
+                    String.format(
+                            "Failed to create the store using the configured factory (%s), will try a generic lookup now.",
+                            factory.getClass()),
+                    e);
+            dataStore = DataStoreUtils.getDataAccess(connectionParameters);
+        }
+
         if (dataStore == null) {
             /*
              * Preserve DataStore retyping behaviour by calling

src/main/src/main/java/org/geoserver/catalog/util/LegacyCatalogImporter.java

@@ -325,6 +325,12 @@ void importDataStores(CatalogFactory factory, Map dataStores) {
             dataStore.getConnectionParameters().put("namespace", ns.getURI());
 
             dataStore.setEnabled((Boolean) map.get("enabled"));
+
+            // some tolerance for legacy app-schema tests
+            if ("app-schema".equals(connectionParams.get("dbtype"))) {
+                dataStore.setType("Application Schema DataAccess");
+            }
+
             catalog.add(dataStore);
 
             if (dataStore.isEnabled()) {

src/main/src/main/java/org/geoserver/data/DataAccessFactoryProducer.java

@@ -6,6 +6,7 @@
 package org.geoserver.data;
 
 import java.util.List;
+import org.geoserver.platform.ExtensionPriority;
 import org.geotools.api.data.DataAccessFactory;
 
 /**
@@ -16,8 +17,13 @@
  *
  * @author Justin Deoliveira, OpenGeo
  */
-public interface DataAccessFactoryProducer {
+public interface DataAccessFactoryProducer extends ExtensionPriority {
 
     /** Returns the list of factories. */
     List<DataAccessFactory> getDataStoreFactories();
+
+    @Override
+    default int getPriority() {
+        return LOWEST;
+    }
 }

src/main/src/main/java/org/geoserver/data/DefaultDataAccessFactoryProducer.java

@@ -0,0 +1,20 @@
+/* (c) 2024 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.data;
+
+import com.google.common.collect.Streams;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.geotools.api.data.DataAccessFactory;
+import org.geotools.api.data.DataAccessFinder;
+
+public class DefaultDataAccessFactoryProducer implements DataAccessFactoryProducer {
+
+    @Override
+    public List<DataAccessFactory> getDataStoreFactories() {
+        return Streams.stream(DataAccessFinder.getAvailableDataStores())
+                .collect(Collectors.toList());
+    }
+}

src/main/src/main/java/org/vfny/geoserver/util/DataStoreUtils.java

@@ -11,7 +11,6 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -23,12 +22,12 @@
 import org.geoserver.catalog.WMTSLayerInfo;
 import org.geoserver.data.DataAccessFactoryProducer;
 import org.geoserver.data.DataStoreFactoryInitializer;
+import org.geoserver.data.DefaultDataAccessFactoryProducer;
 import org.geoserver.feature.retype.RetypingDataStore;
 import org.geoserver.platform.GeoServerExtensions;
 import org.geotools.api.data.DataAccess;
 import org.geotools.api.data.DataAccessFactory;
 import org.geotools.api.data.DataAccessFactory.Param;
-import org.geotools.api.data.DataAccessFinder;
 import org.geotools.api.data.DataStore;
 import org.geotools.api.feature.Feature;
 import org.geotools.api.feature.type.FeatureType;
@@ -62,6 +61,16 @@ public abstract class DataStoreUtils {
             return null;
         }
 
+        return getDataAccess(factory, params);
+    }
+
+    /**
+     * Creates a {@link DataAccess} using the given params and factory, verbatim, and then
+     * eventually wraps it into a renaming wrapper so that feature type names are good ones from the
+     * wfs point of view (that is, no ":" in the type names)
+     */
+    public static DataAccess<? extends FeatureType, ? extends Feature> getDataAccess(
+            DataAccessFactory factory, Map<String, Serializable> params) throws IOException {
         DataAccess<? extends FeatureType, ? extends Feature> store =
                 factory.createDataStore(params);
         if (store == null) {
@@ -242,13 +251,11 @@ public static Map<String, Object> toConnectionParams(
 
     public static Collection<DataAccessFactory> getAvailableDataStoreFactories() {
         List<DataAccessFactory> factories = new ArrayList<>();
-        Iterator<DataAccessFactory> it = DataAccessFinder.getAvailableDataStores();
-        while (it.hasNext()) {
-            factories.add(it.next());
-        }
 
-        for (DataAccessFactoryProducer producer :
-                GeoServerExtensions.extensions(DataAccessFactoryProducer.class)) {
+        List<DataAccessFactoryProducer> producers =
+                GeoServerExtensions.extensions(DataAccessFactoryProducer.class);
+        if (producers.isEmpty()) producers = List.of(new DefaultDataAccessFactoryProducer());
+        for (DataAccessFactoryProducer producer : producers) {
             try {
                 factories.addAll(producer.getDataStoreFactories());
             } catch (Throwable t) {

src/main/src/test/java/org/geoserver/catalog/ResourcePoolTest.java

@@ -59,24 +59,29 @@
 import org.easymock.Capture;
 import org.easymock.CaptureType;
 import org.easymock.EasyMock;
+import org.geoserver.catalog.ResourcePoolTest.TestDirectoryStoreFactorySpi.TestDirectoryStore;
 import org.geoserver.catalog.impl.DataStoreInfoImpl;
 import org.geoserver.catalog.impl.StyleInfoImpl;
 import org.geoserver.catalog.impl.WMSStoreInfoImpl;
 import org.geoserver.catalog.util.ReaderUtils;
 import org.geoserver.config.GeoServer;
 import org.geoserver.config.GeoServerDataDirectory;
 import org.geoserver.config.GeoServerInfo;
+import org.geoserver.data.DataAccessFactoryProducer;
 import org.geoserver.data.test.MockData;
 import org.geoserver.data.test.SystemTestData;
 import org.geoserver.data.test.TestData;
 import org.geoserver.platform.GeoServerEnvironment;
 import org.geoserver.platform.GeoServerExtensions;
+import org.geoserver.platform.GeoServerExtensionsHelper;
 import org.geoserver.test.GeoServerSystemTestSupport;
 import org.geoserver.test.RunTestSetup;
 import org.geoserver.test.SystemTest;
 import org.geotools.api.coverage.grid.GridCoverageReader;
 import org.geotools.api.data.DataAccess;
+import org.geotools.api.data.DataAccessFactory;
 import org.geotools.api.data.DataStore;
+import org.geotools.api.data.DataStoreFactorySpi;
 import org.geotools.api.data.FeatureSource;
 import org.geotools.api.data.Query;
 import org.geotools.api.data.SimpleFeatureLocking;
@@ -102,6 +107,9 @@
 import org.geotools.coverage.grid.io.GridCoverage2DReader;
 import org.geotools.coverage.grid.io.StructuredGridCoverage2DReader;
 import org.geotools.coverage.util.CoverageUtilities;
+import org.geotools.data.directory.DirectoryDataStore;
+import org.geotools.data.memory.MemoryDataStore;
+import org.geotools.data.shapefile.ShapefileDirectoryFactory;
 import org.geotools.data.simple.SimpleFeatureCollection;
 import org.geotools.data.wfs.WFSDataStoreFactory;
 import org.geotools.factory.CommonFactoryFinder;
@@ -123,6 +131,7 @@
 import org.geotools.util.Version;
 import org.geotools.util.factory.GeoTools;
 import org.geotools.util.factory.Hints;
+import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.Test;
@@ -199,6 +208,7 @@ protected void onSetUp(SystemTestData testData) throws Exception {
         }
         CatalogBuilder cb = new CatalogBuilder(catalog);
         DataStoreInfo dataStoreInfo = cb.buildDataStore("mini-states");
+        dataStoreInfo.setType("Shapefile");
         dataStoreInfo.getConnectionParameters().put("url", "file:data/mini-states/mini-states.shp");
         catalog.add(dataStoreInfo);
         DataStore ds = (DataStore) dataStoreInfo.getDataStore(null);
@@ -1376,6 +1386,7 @@ public void testAutodisableOnConnfailure() {
         ds.setWorkspace(ws);
         ds.setEnabled(true);
         ds.setDisableOnConnFailure(true);
+        ds.setType("H2");
         Map<String, Serializable> params = ds.getConnectionParameters();
         params.put("dbtype", "h2");
         params.put("database", "");
@@ -1527,4 +1538,101 @@ public void testCustomizeAttributesCRS() throws Exception {
         CoordinateReferenceSystem crs = schema.getCoordinateReferenceSystem();
         assertEquals(CRS.decode("EPSG:4326", true), crs);
     }
+
+    @Test
+    public void testAcceptAllStore() throws Exception {
+
+        DataAccessFactoryProducer testFactoryProducer =
+                new DataAccessFactoryProducer() {
+                    @Override
+                    public int getPriority() {
+                        return HIGHEST;
+                    }
+
+                    @Override
+                    public List<DataAccessFactory> getDataStoreFactories() {
+                        return List.of(
+                                new TestDirectoryStoreFactorySpi(),
+                                new ShapefileDirectoryFactory());
+                    }
+                };
+        GeoServerExtensionsHelper.singleton(
+                "testDataAccessFactoryProducer",
+                testFactoryProducer,
+                DataAccessFactoryProducer.class);
+
+        try {
+            // now create a store that would be caught by the test factory, if it wasn't for the
+            // type
+            // being used to lookup the shape factory
+            Catalog catalog = getCatalog();
+            CatalogBuilder cb = new CatalogBuilder(catalog);
+            DataStoreInfo dataStoreInfo = cb.buildDataStore("mini-states-dir");
+            dataStoreInfo.setType("Directory of spatial files (shapefiles)");
+            dataStoreInfo.getConnectionParameters().put("url", "file:data/mini-states");
+            catalog.add(dataStoreInfo);
+
+            // check this is the right type of store
+            DataStore ds = (DataStore) dataStoreInfo.getDataStore(null);
+            assertThat(ds, Matchers.instanceOf(DirectoryDataStore.class));
+
+            DataStoreInfo testDsInfo = cb.buildDataStore("test-ds");
+            testDsInfo.setType(TestDirectoryStoreFactorySpi.DISPLAY_NAME);
+            dataStoreInfo.getConnectionParameters().put("url", "file:data/fake");
+            DataStore testDs = (DataStore) testDsInfo.getDataStore(null);
+            assertThat(testDs, Matchers.instanceOf(TestDirectoryStore.class));
+        } finally {
+            GeoServerExtensionsHelper.clear();
+        }
+    }
+
+    /**
+     * A fake store that returns nothing, used to similate the FGB store interaction with directory
+     * data store
+     */
+    static class TestDirectoryStoreFactorySpi implements DataStoreFactorySpi {
+
+        static final String DISPLAY_NAME = "Test store accepting a simple URL";
+        static final Param URL_PARAM =
+                new Param("url", URL.class, "A file or directory", true, null);
+
+        @Override
+        public DataStore createDataStore(Map<String, ?> params) throws IOException {
+            return new TestDirectoryStore();
+        }
+
+        @Override
+        public String getDisplayName() {
+            return DISPLAY_NAME;
+        }
+
+        @Override
+        public String getDescription() {
+            return "A test factory with no parameters and one fixed data type";
+        }
+
+        @Override
+        public Param[] getParametersInfo() {
+            return new Param[] {URL_PARAM};
+        }
+
+        @Override
+        public boolean isAvailable() {
+            return true;
+        }
+
+        @Override
+        public DataStore createNewDataStore(Map<String, ?> params) throws IOException {
+            return createDataStore(params);
+        }
+
+        /**
+         * Empty subclass, just to have a clearer name on test failures, e.g., "failed to cast to
+         * TestDirectoryStore" is better than "failed to case to MemoryDataStore", e.g., it would
+         * lead immediately to this file. In case you're seeing this, it's likely that you created a
+         * DataStoreInfo without setting the "type" attribute to the desired data store factory
+         * displayName.
+         */
+        public static class TestDirectoryStore extends MemoryDataStore {}
+    }
 }

src/restconfig/src/main/java/org/geoserver/rest/catalog/DataStoreController.java

@@ -146,14 +146,16 @@ public ResponseEntity<String> dataStorePost(
         }
 
         // attempt to set the datastore type
-        try {
-            DataAccessFactory factory =
-                    DataStoreUtils.aquireFactory(dataStore.getConnectionParameters());
-            dataStore.setType(factory.getDisplayName());
-        } catch (Exception e) {
-            LOGGER.warning("Unable to determine datastore type from connection parameters");
-            if (LOGGER.isLoggable(Level.FINE)) {
-                LOGGER.log(Level.FINE, "", e);
+        if (dataStore.getType() == null) {
+            try {
+                DataAccessFactory factory =
+                        DataStoreUtils.aquireFactory(dataStore.getConnectionParameters());
+                dataStore.setType(factory.getDisplayName());
+            } catch (Exception e) {
+                LOGGER.warning("Unable to determine datastore type from connection parameters");
+                if (LOGGER.isLoggable(Level.FINE)) {
+                    LOGGER.log(Level.FINE, "", e);
+                }
             }
         }