diff --git a/geowebcache/core/src/main/java/org/geowebcache/config/XMLConfiguration.java b/geowebcache/core/src/main/java/org/geowebcache/config/XMLConfiguration.java index 6206baaa3..9e6880e2a 100644 --- a/geowebcache/core/src/main/java/org/geowebcache/config/XMLConfiguration.java +++ b/geowebcache/core/src/main/java/org/geowebcache/config/XMLConfiguration.java @@ -788,14 +788,15 @@ private static Node applyTransform(Node oldRootNode, String xslFilename) { @Override public void afterPropertiesSet() throws GeoWebCacheException { - + if (!resourceProvider.hasInput()) { + throw new ConfigurationException( + String.format("The configuration resource provider is unable to provide a configuration file")); + } if (gridSetBroker == null) { throw new IllegalStateException("GridSetBroker has not been set"); } - if (resourceProvider.hasInput()) { - this.setGwcConfig(loadConfiguration()); - } + this.setGwcConfig(loadConfiguration()); log.config("Initializing GridSets from " + getIdentifier()); diff --git a/geowebcache/core/src/main/java/org/geowebcache/config/XMLFileResourceProvider.java b/geowebcache/core/src/main/java/org/geowebcache/config/XMLFileResourceProvider.java index 15c42a049..fe6b78d20 100644 --- a/geowebcache/core/src/main/java/org/geowebcache/config/XMLFileResourceProvider.java +++ b/geowebcache/core/src/main/java/org/geowebcache/config/XMLFileResourceProvider.java @@ -33,6 +33,7 @@ import org.geowebcache.util.ApplicationContextProvider; import org.geowebcache.util.GWCVars; import org.springframework.context.ApplicationContext; +import org.springframework.lang.NonNull; import org.springframework.web.context.WebApplicationContext; /** Default implementation of ConfigurationResourceProvider that uses the file system. */ @@ -49,9 +50,11 @@ public class XMLFileResourceProvider implements ConfigurationResourceProvider { private final WebApplicationContext context; /** Location of the configuration file */ + @NonNull private final File configDirectory; /** Name of the configuration file */ + @NonNull private final String configFileName; private String templateLocation; @@ -135,11 +138,27 @@ public XMLFileResourceProvider( this(configFileName, appCtx, getConfigDirVar(appCtx), storageDirFinder); } + /** + * {@inheritDoc} + * + *
If the configuration file doesn't exist and {@link #hasInput() == true}, the file will be first created from + * the {@link #setTemplate(String) template} + * + * @throws IOException if the file can't be created or copied from the template + */ @Override public InputStream in() throws IOException { return new FileInputStream(findOrCreateConfFile()); } + /** + * {@inheritDoc} + * + *
If the configuration file doesn't exist and {@link #hasOutput() == true}, the file will be first created from + * the {@link #setTemplate(String) template} + * + * @throws IOException if the file can't be created or copied from the template + */ @Override public OutputStream out() throws IOException { return new FileOutputStream(findOrCreateConfFile()); @@ -165,22 +184,13 @@ public void setTemplate(final String templateLocation) { } private File findConfigFile() throws IOException { - if (null == configDirectory) { - throw new IllegalStateException(); - } if (!configDirectory.exists() && !configDirectory.mkdirs()) { throw new IOException("TileLayerConfiguration directory does not exist and cannot be created: '" + configDirectory.getAbsolutePath() + "'"); } - if (!configDirectory.canWrite()) { - throw new IOException( - "TileLayerConfiguration directory is not writable: '" + configDirectory.getAbsolutePath() + "'"); - } - - File xmlFile = new File(configDirectory, configFileName); - return xmlFile; + return new File(configDirectory, configFileName); } @Override @@ -200,6 +210,11 @@ private File findOrCreateConfFile() throws IOException { if (xmlFile.exists()) { log.config("Found configuration file in " + configDirectory.getAbsolutePath()); } else if (templateLocation != null) { + if (!configDirectory.canWrite()) { + throw new IOException("TileLayerConfiguration directory is not writable: '" + + configDirectory.getAbsolutePath() + "'"); + } + log.warning("Found no configuration file in config directory, will create one at '" + xmlFile.getAbsolutePath() + "' from template " @@ -249,17 +264,40 @@ private void backUpConfig(final File xmlFile) throws IOException { log.fine("Config backup done"); } + /** + * Determines if the config file exists and is readable, or doesn't exist but can be created. + * + *
Calling this method has no side effects. The target file either exists, or can be created throught the + * {@link #setTemplate(String) template}, if a template has been set. In such case, it'll be created by either + * {@link #in()} or {@link #out()}. + */ @Override public boolean hasInput() { try { - return findOrCreateConfFile().exists(); + File file = findConfigFile(); + return file.exists() || (templateLocation != null && configDirectory.canWrite()); } catch (IOException e) { + log.log(Level.WARNING, "Error obtaining config file", e); return false; } } + /** + * Determines if the configuration can be {@link #out() written} to the {@link #getLocation() output file}. + * + *
Calling this method has no side effects. The target file may or may not exist. The target directory must be + * writable, and so must the target file in case it does exist. + * + * @return {@code true} if the {@link #getLocation() configuration file} can be written to. + */ @Override public boolean hasOutput() { - return true; + try { + File file = findConfigFile(); + return configDirectory.canWrite() && (!file.exists() || file.canWrite()); + } catch (IOException e) { + log.log(Level.WARNING, "Error obtaining config file", e); + return false; + } } } diff --git a/geowebcache/core/src/test/java/org/geowebcache/config/XMLConfigurationTest.java b/geowebcache/core/src/test/java/org/geowebcache/config/XMLConfigurationTest.java index 9d9097c9e..e35542aa5 100644 --- a/geowebcache/core/src/test/java/org/geowebcache/config/XMLConfigurationTest.java +++ b/geowebcache/core/src/test/java/org/geowebcache/config/XMLConfigurationTest.java @@ -18,6 +18,7 @@ import static org.easymock.EasyMock.replay; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertArrayEquals; @@ -27,6 +28,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; @@ -34,6 +36,7 @@ import java.io.File; import java.io.FileInputStream; +import java.io.IOException; import java.net.URL; import java.util.ArrayList; import java.util.Arrays; @@ -47,6 +50,7 @@ import java.util.logging.Logger; import org.apache.commons.io.FileUtils; import org.geotools.util.logging.Logging; +import org.geowebcache.GeoWebCacheException; import org.geowebcache.GeoWebCacheExtensions; import org.geowebcache.MockWepAppContextRule; import org.geowebcache.config.legends.LegendRawInfo; @@ -557,4 +561,32 @@ public void testWmtsCiteStrictComplianceIsActivated() throws Exception { // CITE strict compliance should be activated for WMTS assertThat(config.isWmtsCiteCompliant(), is(true)); } + + @Test + public void loadFromReadOnlyDirectory() throws GeoWebCacheException { + configDir.setWritable(false); + assertThat(configFile.exists(), is(true)); + try { + config = new XMLConfiguration(null, configDir.getAbsolutePath()); + config.setGridSetBroker(gridSetBroker); + config.afterPropertiesSet(); + assertThat(config.getLayerCount(), is(greaterThan(0))); + } finally { + configDir.setWritable(true); + } + } + + @Test + public void loadFromEmptyReadOnlyDirectoryFails() throws GeoWebCacheException, IOException { + File roDir = this.temp.newFolder(); + assertTrue(roDir.setWritable(false)); + try { + config = new XMLConfiguration(null, roDir.getAbsolutePath()); + config.setGridSetBroker(gridSetBroker); + + assertThrows(ConfigurationException.class, () -> config.afterPropertiesSet()); + } finally { + roDir.setWritable(true); + } + } }