diff --git a/src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/RestconfigWfsTest.java b/src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/RestconfigWfsTest.java index acfb61f881d..8b13b5a3114 100644 --- a/src/extension/app-schema/app-schema-test/src/test/java/org/geoserver/test/RestconfigWfsTest.java +++ b/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 { + "" // + "gsml" // + "" // + + "Application Schema DataAccess" // + "" // + "app-schema" // + "file:workspaces/gsml/MappedFeature/MappedFeature.xml" // diff --git a/src/main/src/main/java/org/geoserver/catalog/ResourcePool.java b/src/main/src/main/java/org/geoserver/catalog/ResourcePool.java index e941031374d..a6e593bdd27 100644 --- a/src/main/src/main/java/org/geoserver/catalog/ResourcePool.java +++ b/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 diff --git a/src/main/src/main/java/org/geoserver/catalog/util/LegacyCatalogImporter.java b/src/main/src/main/java/org/geoserver/catalog/util/LegacyCatalogImporter.java index ae0f21ee5cc..545f8694f93 100644 --- a/src/main/src/main/java/org/geoserver/catalog/util/LegacyCatalogImporter.java +++ b/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()) { diff --git a/src/main/src/main/java/org/vfny/geoserver/util/DataStoreUtils.java b/src/main/src/main/java/org/vfny/geoserver/util/DataStoreUtils.java index 7d96a8fc4e4..f212f331bac 100644 --- a/src/main/src/main/java/org/vfny/geoserver/util/DataStoreUtils.java +++ b/src/main/src/main/java/org/vfny/geoserver/util/DataStoreUtils.java @@ -62,6 +62,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 getDataAccess( + DataAccessFactory factory, Map params) throws IOException { DataAccess store = factory.createDataStore(params); if (store == null) { diff --git a/src/main/src/test/java/org/geoserver/catalog/ResourcePoolTest.java b/src/main/src/test/java/org/geoserver/catalog/ResourcePoolTest.java index 3d9e553bcff..07a74bcaa22 100644 --- a/src/main/src/test/java/org/geoserver/catalog/ResourcePoolTest.java +++ b/src/main/src/test/java/org/geoserver/catalog/ResourcePoolTest.java @@ -41,6 +41,7 @@ import java.net.URI; import java.net.URL; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.UUID; @@ -77,6 +78,8 @@ import org.geotools.api.coverage.grid.GridCoverageReader; import org.geotools.api.data.DataAccess; import org.geotools.api.data.DataStore; +import org.geotools.api.data.DataStoreFactorySpi; +import org.geotools.api.data.DataStoreFinder; import org.geotools.api.data.FeatureSource; import org.geotools.api.data.Query; import org.geotools.api.data.SimpleFeatureLocking; @@ -102,6 +105,8 @@ 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.shapefile.ShapefileDirectoryFactory; import org.geotools.data.simple.SimpleFeatureCollection; import org.geotools.data.wfs.WFSDataStoreFactory; import org.geotools.factory.CommonFactoryFinder; @@ -121,13 +126,18 @@ import org.geotools.util.SoftValueHashMap; import org.geotools.util.URLs; import org.geotools.util.Version; +import org.geotools.util.factory.FactoryRegistry; import org.geotools.util.factory.GeoTools; import org.geotools.util.factory.Hints; +import org.hamcrest.Matchers; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.Assume; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; import org.locationtech.jts.geom.Point; +import org.springframework.test.util.ReflectionTestUtils; import org.w3c.dom.Element; import org.xml.sax.SAXException; @@ -146,6 +156,8 @@ public class ResourcePoolTest extends GeoServerSystemTestSupport { private static final String HUMANS = "humans"; private static final String BAD_CONN_DATASTORE = "bad_conn_data_store"; + public static final DataStoreFactorySpi TEST_DIRECTORY_STORE_FACTORY_SPI = + new TestDirectoryStoreFactorySpi(); static { System.setProperty("ALLOW_ENV_PARAMETRIZATION", "true"); @@ -158,6 +170,17 @@ public class ResourcePoolTest extends GeoServerSystemTestSupport { private static final String EXTERNAL_ENTITIES = "externalEntities"; + @BeforeClass + public static void registerTestDirectoryStore() { + // a "catch-all" datastore that will use any File without requiring a filetype/dbtype + DataStoreFinder.registerFactrory(TEST_DIRECTORY_STORE_FACTORY_SPI); + } + + @AfterClass + public static void deregisterTestDirectoryStore() { + DataStoreFinder.deregisterFactrory(TEST_DIRECTORY_STORE_FACTORY_SPI); + } + @Override protected void onSetUp(SystemTestData testData) throws Exception { super.onSetUp(testData); @@ -199,6 +222,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 +1400,7 @@ public void testAutodisableOnConnfailure() { ds.setWorkspace(ws); ds.setEnabled(true); ds.setDisableOnConnFailure(true); + ds.setType("H2"); Map params = ds.getConnectionParameters(); params.put("dbtype", "h2"); params.put("database", ""); @@ -1527,4 +1552,44 @@ public void testCustomizeAttributesCRS() throws Exception { CoordinateReferenceSystem crs = schema.getCoordinateReferenceSystem(); assertEquals(CRS.decode("EPSG:4326", true), crs); } + + @Test + public void testAcceptAllStore() throws Exception { + // check we have both the shapefile directory and test store accepting URL without a dbtype + ShapefileDirectoryFactory shapeDirectorFactory = null; + TestDirectoryStoreFactorySpi testDirectoryFactory = null; + Iterator factoryIterator = DataStoreFinder.getAllDataStores(); + while (factoryIterator.hasNext()) { + DataStoreFactorySpi spi = factoryIterator.next(); + if (spi instanceof TestDirectoryStoreFactorySpi) + testDirectoryFactory = (TestDirectoryStoreFactorySpi) spi; + else if (spi instanceof ShapefileDirectoryFactory) + shapeDirectorFactory = (ShapefileDirectoryFactory) spi; + } + assertNotNull(shapeDirectorFactory); + assertNotNull(testDirectoryFactory); + + // Using reflection to grab the registry. The synchronization is there because the method + // is using assertions to check it's being called in a synchronized block (assertions + // are enabled when running tests with Maven) + FactoryRegistry registry; + synchronized (DataStoreFinder.class) { + registry = + ReflectionTestUtils.invokeMethod(DataStoreFinder.class, "getServiceRegistry"); + } + registry.setOrdering(DataStoreFactorySpi.class, testDirectoryFactory, shapeDirectorFactory); + + // 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)); + } } diff --git a/src/main/src/test/java/org/geoserver/catalog/TestDirectoryStoreFactorySpi.java b/src/main/src/test/java/org/geoserver/catalog/TestDirectoryStoreFactorySpi.java new file mode 100644 index 00000000000..b29b90f1f66 --- /dev/null +++ b/src/main/src/test/java/org/geoserver/catalog/TestDirectoryStoreFactorySpi.java @@ -0,0 +1,61 @@ +/* (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.catalog; + +import java.io.IOException; +import java.net.URL; +import java.util.Map; +import org.geotools.api.data.DataStore; +import org.geotools.api.data.DataStoreFactorySpi; +import org.geotools.data.memory.MemoryDataStore; + +/** + * A fake store that returns nothing, used to similate the FGB store interaction with directory data + * store + */ +public class TestDirectoryStoreFactorySpi implements DataStoreFactorySpi { + + public static final Param URL_PARAM = + new Param("url", URL.class, "A file or directory", true, null); + + @Override + public DataStore createDataStore(Map params) throws IOException { + return new TestDirectoryStore(); + } + + @Override + public String getDisplayName() { + return "Test store accepting a simple URL"; + } + + @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 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 {} +} diff --git a/src/restconfig/src/main/java/org/geoserver/rest/catalog/DataStoreController.java b/src/restconfig/src/main/java/org/geoserver/rest/catalog/DataStoreController.java index e67e3b42a16..1296e6a7f66 100644 --- a/src/restconfig/src/main/java/org/geoserver/rest/catalog/DataStoreController.java +++ b/src/restconfig/src/main/java/org/geoserver/rest/catalog/DataStoreController.java @@ -146,14 +146,16 @@ public ResponseEntity 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); + } } } diff --git a/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreControllerTest.java b/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreControllerTest.java index 03c48437cc8..6c2ceac8362 100644 --- a/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreControllerTest.java +++ b/src/restconfig/src/test/java/org/geoserver/rest/catalog/DataStoreControllerTest.java @@ -6,6 +6,7 @@ import static org.custommonkey.xmlunit.XMLAssert.assertXpathEvaluatesTo; import static org.custommonkey.xmlunit.XMLAssert.assertXpathExists; +import static org.geoserver.catalog.ResourcePoolTest.TEST_DIRECTORY_STORE_FACTORY_SPI; import static org.geoserver.rest.RestBaseController.ROOT_PATH; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.endsWith; @@ -33,10 +34,13 @@ import org.geoserver.data.test.SystemTestData; import org.geoserver.rest.RestBaseController; import org.geotools.api.data.DataStore; +import org.geotools.api.data.DataStoreFinder; import org.geotools.api.feature.type.FeatureType; import org.geotools.data.property.PropertyDataStore; import org.hamcrest.Matchers; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletResponse; @@ -46,6 +50,17 @@ public class DataStoreControllerTest extends CatalogRESTTestSupport { + @BeforeClass + public static void registerTestDirectoryStore() { + // a "catch-all" datastore that will use any File without requiring a filetype/dbtype + DataStoreFinder.registerFactrory(TEST_DIRECTORY_STORE_FACTORY_SPI); + } + + @AfterClass + public static void deregisterTestDirectoryStore() { + DataStoreFinder.deregisterFactrory(TEST_DIRECTORY_STORE_FACTORY_SPI); + } + @Before public void addDataStores() throws IOException { removeStore("sf", "newDataStore"); // may have been created by other tests