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 b203948fa..7c330fb78 100644
--- a/geowebcache/core/src/main/java/org/geowebcache/config/XMLConfiguration.java
+++ b/geowebcache/core/src/main/java/org/geowebcache/config/XMLConfiguration.java
@@ -15,6 +15,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.annotations.VisibleForTesting;
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.io.xml.DomReader;
import java.io.FileNotFoundException;
@@ -290,7 +291,8 @@ public void setDefaultValues(TileLayer layer) {
}
}
- private GeoWebCacheConfiguration loadConfiguration() throws ConfigurationException {
+ @VisibleForTesting
+ GeoWebCacheConfiguration loadConfiguration() throws ConfigurationException {
Assert.isTrue(resourceProvider.hasInput(), "Resource provider must have an input");
try {
try (InputStream in = resourceProvider.in()) {
@@ -969,19 +971,44 @@ public synchronized void addBlobStore(BlobStoreInfo info) {
} catch (IOException ioe) {
// save failed, roll back the add
blobStores.remove(info);
- throw new ConfigurationPersistenceException("Unable to add BlobStoreInfo \"%s\"".formatted(info), ioe);
+ ConfigurationPersistenceException thrown =
+ new ConfigurationPersistenceException("Unable to add BlobStoreInfo \"%s\"".formatted(info), ioe);
+
+ try { // reverting the stored config by re-saving
+ save();
+ } catch (IOException suppressed) {
+ log.log(
+ Level.WARNING,
+ "Unable to revert stored configuration after failure to store BlobStore " + info,
+ suppressed);
+ thrown.addSuppressed(thrown);
+ }
+ throw thrown;
}
try {
blobStoreListeners.safeForEach(listener -> {
listener.handleAddBlobStore(info);
});
} catch (IOException | GeoWebCacheException e) {
+ ConfigurationPersistenceException thrown;
if (ExceptionUtils.isOrSuppresses(e, UnsuitableStorageException.class)) {
// Can't store here, roll back
blobStores.remove(info);
- throw new ConfigurationPersistenceException("Unable to add BlobStoreInfo \"%s\"".formatted(info), e);
+ thrown = new ConfigurationPersistenceException("Unable to add BlobStoreInfo \"%s\"".formatted(info), e);
+ try { // reverting the stored config by re-saving
+ save();
+ log.log(Level.CONFIG, "Configuration reverted after failure to add BlobStore " + info);
+ } catch (IOException suppress) {
+ log.log(
+ Level.WARNING,
+ "Unexpected error saving the reverted configuration after failure to add BlobStore " + info,
+ suppress);
+ thrown.addSuppressed(thrown);
+ }
+ } else {
+ thrown = new ConfigurationPersistenceException(e);
}
- throw new ConfigurationPersistenceException(e);
+ throw thrown;
}
}
@@ -1050,7 +1077,21 @@ public synchronized void modifyBlobStore(BlobStoreInfo info) {
// Can't store here, roll back
blobStores.remove(info);
blobStores.add(infoToRemove);
- throw new ConfigurationPersistenceException("Unable to modify BlobStoreInfo \"%s\"".formatted(info), e);
+ ConfigurationPersistenceException thrown = new ConfigurationPersistenceException(
+ "Unable to modify BlobStoreInfo \"%s\"".formatted(info), e);
+ try {
+ // roll-back stored config, leaving an invalid stored config will prevent application startup
+ save();
+ log.log(Level.CONFIG, "Configuration reverted after failure to modify BlobStore " + info);
+ } catch (IOException suppress) {
+ log.log(
+ Level.WARNING,
+ "Unexpected error saving the reverted configuration after failure to modify BlobStore "
+ + info,
+ suppress);
+ thrown.addSuppressed(suppress);
+ }
+ throw thrown;
}
throw new ConfigurationPersistenceException(e);
}
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 3b3151116..e921fad22 100644
--- a/geowebcache/core/src/test/java/org/geowebcache/config/XMLConfigurationTest.java
+++ b/geowebcache/core/src/test/java/org/geowebcache/config/XMLConfigurationTest.java
@@ -16,6 +16,7 @@
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
+import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.greaterThan;
@@ -31,12 +32,15 @@
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.io.File;
import java.io.FileInputStream;
+import java.io.FilterOutputStream;
import java.io.IOException;
+import java.io.OutputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
@@ -67,13 +71,16 @@
import org.geowebcache.grid.SRS;
import org.geowebcache.layer.TileLayer;
import org.geowebcache.layer.wms.WMSLayer;
+import org.geowebcache.storage.UnsuitableStorageException;
import org.geowebcache.util.TestUtils;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
+import org.mockito.Mockito;
import org.springframework.context.ApplicationContext;
+import org.springframework.web.context.WebApplicationContext;
import org.xml.sax.SAXParseException;
public class XMLConfigurationTest {
@@ -477,7 +484,7 @@ public void testNoBlobStores() throws Exception {
}
@Test
- public void testSaveBlobStores() throws Exception {
+ public void testAddBlobStores() throws Exception {
FileBlobStoreInfo store1 = new FileBlobStoreInfo();
store1.setName("store1");
store1.setDefault(true);
@@ -517,6 +524,150 @@ public void testSaveBlobStores() throws Exception {
assertEquals(store2, stores.get(1));
}
+ @Test
+ public void testAddBlobStoreExceptionSaving() throws Exception {
+
+ XMLFileResourceProvider resourceProvider =
+ new XMLFileResourceProvider(
+ XMLConfiguration.DEFAULT_CONFIGURATION_FILE_NAME,
+ (WebApplicationContext) null,
+ this.configDir.getAbsolutePath(),
+ null) {
+
+ // throw an ioexception the first time close() is called, the second time is the roll-back
+ private boolean thrown = false;
+
+ @Override
+ public OutputStream out() throws IOException {
+ OutputStream real = super.out();
+ return new FilterOutputStream(real) {
+
+ @Override
+ public void close() throws IOException {
+ real.close();
+ if (thrown) {
+ return;
+ }
+ thrown = true;
+ throw new IOException("forced io exception");
+ }
+ };
+ }
+ };
+
+ config = new XMLConfiguration(null, resourceProvider);
+ config.setGridSetBroker(gridSetBroker);
+
+ FileBlobStoreInfo store1 = new FileBlobStoreInfo();
+ store1.setName("store1");
+ store1.setDefault(true);
+ store1.setEnabled(true);
+ store1.setFileSystemBlockSize(8096);
+ store1.setBaseDirectory("/tmp/test");
+
+ assertThrows(ConfigurationPersistenceException.class, () -> config.addBlobStore(store1));
+ assertEquals(0, config.getBlobStoreCount());
+ GeoWebCacheConfiguration configuration = config.loadConfiguration();
+ assertTrue("store shouldn't be saved", configuration.getBlobStores().isEmpty());
+ }
+
+ /**
+ * Verifies the blobstore configuration is rolled back from the persisted configuration if a
+ * {@link BlobStoreConfigurationListener#handleAddBlobStore} throws an {@link UnsuitableStorageException}.
+ *
+ * Note I'm not sure why XMLConfiguration.addBlobStore() only rolls-back on UnsuitableStorageException and not on
+ * IOException or GeoWebCacheException
+ */
+ @Test
+ public void testAddBlobStoreExceptionFromListener() throws Exception {
+ FileBlobStoreInfo store1 = new FileBlobStoreInfo();
+ store1.setName("store1");
+ store1.setDefault(true);
+ store1.setEnabled(true);
+ store1.setFileSystemBlockSize(8096);
+ store1.setBaseDirectory("/tmp/test");
+
+ BlobStoreConfigurationListener listener;
+
+ listener = mock(BlobStoreConfigurationListener.class);
+ doThrow(new UnsuitableStorageException("fake")).when(listener).handleAddBlobStore(Mockito.any());
+ config.addBlobStoreListener(listener);
+
+ assertAddBlobStoreFails(store1, UnsuitableStorageException.class);
+
+ // note, I'm not sure why XMLConfiguration.addBlobStore() only rolls-back on UnsuitableStorageException and not
+ // on IOException or GeoWebCacheException
+ // doThrow(new IOException("fake")).when(listener).handleAddBlobStore(Mockito.any());
+ // assertAddBlobStoreFails(store1, IOException.class);
+ //
+ // doThrow(new GeoWebCacheException("fake")).when(listener).handleAddBlobStore(Mockito.any());
+ // assertAddBlobStoreFails(store1, GeoWebCacheException.class);
+ }
+
+ private void assertAddBlobStoreFails(FileBlobStoreInfo store, Class expectedCause)
+ throws ConfigurationException {
+ ConfigurationPersistenceException expected;
+ expected = assertThrows(ConfigurationPersistenceException.class, () -> config.addBlobStore(store));
+ assertThat(expected.getCause(), instanceOf(expectedCause));
+ assertEquals(0, config.getBlobStoreCount());
+ GeoWebCacheConfiguration configuration = config.loadConfiguration();
+ assertTrue("store shouldn't be saved", configuration.getBlobStores().isEmpty());
+ }
+
+ /**
+ * Verifies the blobstore configuration is rolled back from the persisted configuration if a
+ * {@link BlobStoreConfigurationListener#handleModifyBlobStore(BlobStoreInfo)} throws an
+ * {@link UnsuitableStorageException}.
+ *
+ *
Note I'm not sure why XMLConfiguration.modifyBobstore() only rolls-back on UnsuitableStorageException and not
+ * on IOException or GeoWebCacheException
+ */
+ @Test
+ public void testModifyBlobStoreExceptionFromListener() throws Exception {
+ FileBlobStoreInfo original = new FileBlobStoreInfo();
+ original.setName("store1");
+ original.setDefault(true);
+ original.setEnabled(true);
+ original.setFileSystemBlockSize(8096);
+ original.setBaseDirectory("/tmp/test");
+
+ config.addBlobStore(original);
+
+ BlobStoreConfigurationListener listener;
+
+ listener = mock(BlobStoreConfigurationListener.class);
+ doThrow(new UnsuitableStorageException("fake")).when(listener).handleModifyBlobStore(Mockito.any());
+ config.addBlobStoreListener(listener);
+
+ assertModifyBlobStoreFails(original, UnsuitableStorageException.class);
+
+ // note, I'm not sure why XMLConfiguration.addBlobStore() only rolls-back on UnsuitableStorageException and not
+ // on IOException or GeoWebCacheException
+ // doThrow(new IOException("fake")).when(listener).handleModifyBlobStore(Mockito.any());
+ // assertModifyBlobStoreFails(original, IOException.class);
+ //
+ // doThrow(new GeoWebCacheException("fake")).when(listener).handleModifyBlobStore(Mockito.any());
+ // assertModifyBlobStoreFails(original, GeoWebCacheException.class);
+ }
+
+ private void assertModifyBlobStoreFails(FileBlobStoreInfo original, Class expectedCause)
+ throws ConfigurationException {
+
+ FileBlobStoreInfo modified = (FileBlobStoreInfo) original.clone();
+ modified.setBaseDirectory("/tmp/test2");
+
+ assertEquals(1, config.getBlobStoreCount());
+
+ ConfigurationPersistenceException expected;
+ expected = assertThrows(ConfigurationPersistenceException.class, () -> config.modifyBlobStore(modified));
+ assertThat(expected.getCause(), instanceOf(expectedCause));
+ GeoWebCacheConfiguration reloaded = config.loadConfiguration();
+ assertEquals(1, reloaded.getBlobStores().size());
+
+ BlobStoreInfo stored = reloaded.getBlobStores().get(0);
+ assertEquals("store shouldn't be saved", original, stored);
+ }
+
@Test
public void testSaveCurrentVersion() throws Exception {