From 82d580d60b73d9a52442861401f9ff5c5514ffc7 Mon Sep 17 00:00:00 2001 From: Andrew MacBean Date: Mon, 15 Sep 2014 14:16:31 +0000 Subject: QPID-6075: [Java Broker] Deleting VHN fails to delete underlying store files if VHN has not been started git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1625039 13f79535-47bb-0310-9956-ffa450edef68 --- .../store/berkeleydb/BDBConfigurationStore.java | 34 +++++++++++----------- .../server/store/berkeleydb/BDBMessageStore.java | 32 ++++++++++---------- .../store/berkeleydb/BDBMessageStoreTest.java | 6 +++- .../store/ManagementModeStoreHandler.java | 2 +- .../store/AbstractJDBCConfigurationStore.java | 2 +- .../server/store/AbstractJDBCMessageStore.java | 2 +- .../qpid/server/store/AbstractMemoryStore.java | 2 +- .../server/store/DurableConfigurationStore.java | 2 +- .../qpid/server/store/JsonFileConfigStore.java | 20 ++++++++----- .../qpid/server/store/MemoryMessageStore.java | 2 +- .../org/apache/qpid/server/store/MessageStore.java | 2 +- .../apache/qpid/server/store/NullMessageStore.java | 2 +- .../server/virtualhost/AbstractVirtualHost.java | 2 +- .../virtualhostnode/AbstractVirtualHostNode.java | 2 +- .../store/BrokerStoreUpgraderAndRecovererTest.java | 2 +- .../qpid/server/store/JsonFileConfigStoreTest.java | 3 +- .../store/derby/DerbyConfigurationStore.java | 25 +++++++++------- .../qpid/server/store/derby/DerbyMessageStore.java | 27 +++++++++-------- .../apache/qpid/server/store/derby/DerbyUtils.java | 20 ------------- .../server/store/derby/DerbyMessageStoreTest.java | 5 +++- .../server/store/jdbc/JDBCMessageStoreTest.java | 2 +- 21 files changed, 98 insertions(+), 98 deletions(-) diff --git a/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBConfigurationStore.java b/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBConfigurationStore.java index bbcf6db2fe..2caa069891 100644 --- a/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBConfigurationStore.java +++ b/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBConfigurationStore.java @@ -44,6 +44,7 @@ import org.apache.log4j.Logger; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.store.ConfiguredObjectRecord; import org.apache.qpid.server.store.DurableConfigurationStore; +import org.apache.qpid.server.store.FileBasedSettings; import org.apache.qpid.server.store.MessageStore; import org.apache.qpid.server.store.MessageStoreProvider; import org.apache.qpid.server.store.SizeMonitoringSettings; @@ -479,27 +480,26 @@ public class BDBConfigurationStore implements MessageStoreProvider, DurableConfi } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { - if (!isConfigurationStoreOpen() && !_providedMessageStore.isMessageStoreOpen()) + if (LOGGER.isDebugEnabled()) { - if (_storeLocation != null) - { - if (LOGGER.isDebugEnabled()) - { - LOGGER.debug("Deleting store " + _storeLocation); - } + LOGGER.debug("Deleting store " + _storeLocation); + } - File location = new File(_storeLocation); - if (location.exists()) - { - if (!FileUtils.delete(location, true)) - { - LOGGER.error("Cannot delete " + _storeLocation); - } - } + FileBasedSettings fileBasedSettings = (FileBasedSettings)parent; + String storePath = fileBasedSettings.getStorePath(); + + if (storePath != null) + { + File configFile = new File(storePath); + if (!FileUtils.delete(configFile, true)) + { + LOGGER.info("Failed to delete the store at location " + storePath); } } + + _storeLocation = null; } private boolean isConfigurationStoreOpen() @@ -562,7 +562,7 @@ public class BDBConfigurationStore implements MessageStoreProvider, DurableConfi } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { // Nothing to do, message store will be deleted when configuration store is deleted } diff --git a/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStore.java b/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStore.java index 9a3734b5ca..5facf616e5 100644 --- a/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStore.java +++ b/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStore.java @@ -27,6 +27,7 @@ import com.sleepycat.je.DatabaseException; import org.apache.log4j.Logger; import org.apache.qpid.server.model.ConfiguredObject; +import org.apache.qpid.server.store.FileBasedSettings; import org.apache.qpid.server.store.SizeMonitoringSettings; import org.apache.qpid.server.store.StoreException; import org.apache.qpid.util.FileUtils; @@ -102,27 +103,26 @@ public class BDBMessageStore extends AbstractBDBMessageStore } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { - if (!_messageStoreOpen.get()) + if (LOGGER.isDebugEnabled()) { - if (_storeLocation != null) - { - if (LOGGER.isDebugEnabled()) - { - LOGGER.debug("Deleting store " + _storeLocation); - } + LOGGER.debug("Deleting store " + _storeLocation); + } - File location = new File(_storeLocation); - if (location.exists()) - { - if (!FileUtils.delete(location, true)) - { - LOGGER.error("Cannot delete " + _storeLocation); - } - } + FileBasedSettings fileBasedSettings = (FileBasedSettings)parent; + String storePath = fileBasedSettings.getStorePath(); + + if (storePath != null) + { + File configFile = new File(storePath); + if (!FileUtils.delete(configFile, true)) + { + LOGGER.info("Failed to delete the store at location " + storePath); } } + + _storeLocation = null; } @Override diff --git a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java index bace585e56..3ddfd0bbdf 100644 --- a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java +++ b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java @@ -402,7 +402,11 @@ public class BDBMessageStoreTest extends MessageStoreTestCase getStore().closeMessageStore(); assertTrue("Store does not exist at " + storeLocation, location.exists()); - getStore().onDelete(); + BDBVirtualHost mockVH = mock(BDBVirtualHost.class); + when(mockVH.getStorePath()).thenReturn(getStore().getStoreLocation()); + + getStore().onDelete(mockVH); + assertFalse("Store exists at " + storeLocation, location.exists()); } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java b/java/broker-core/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java index 44b76cd5c8..4032ae2f39 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java @@ -254,7 +254,7 @@ public class ManagementModeStoreHandler implements DurableConfigurationStore } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java index b3d0428bb3..52072678e8 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCConfigurationStore.java @@ -912,7 +912,7 @@ public abstract class AbstractJDBCConfigurationStore implements MessageStoreProv protected abstract String getBlobAsString(ResultSet rs, int col) throws SQLException; @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { // TODO should probably check we are closed try diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java index 71754b21c9..9633e32408 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractJDBCMessageStore.java @@ -1964,7 +1964,7 @@ public abstract class AbstractJDBCMessageStore implements MessageStore protected abstract void storedSizeChange(int storeSizeIncrease); @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { // TODO should probably check we are closed try diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractMemoryStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractMemoryStore.java index a9af138a02..54999551e0 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractMemoryStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/AbstractMemoryStore.java @@ -130,7 +130,7 @@ public abstract class AbstractMemoryStore implements DurableConfigurationStore, } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/DurableConfigurationStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/DurableConfigurationStore.java index 588edd9cab..1bf83162e6 100755 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/DurableConfigurationStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/DurableConfigurationStore.java @@ -93,5 +93,5 @@ public interface DurableConfigurationStore * has not be opened, then this call will be ignored. The store should be closed * before making this call. */ - void onDelete(); + void onDelete(ConfiguredObject parent); } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java index 5001086010..b00428064d 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/JsonFileConfigStore.java @@ -40,6 +40,7 @@ import java.util.Map; import java.util.UUID; import org.apache.log4j.Logger; +import org.apache.qpid.util.FileUtils; import org.codehaus.jackson.JsonGenerator; import org.codehaus.jackson.JsonProcessingException; import org.codehaus.jackson.Version; @@ -388,7 +389,7 @@ public class JsonFileConfigStore implements DurableConfigurationStore { File tmpFile = File.createTempFile("cfg","tmp", new File(_directoryName)); tmpFile.deleteOnExit(); - _objectMapper.writeValue(tmpFile,data); + _objectMapper.writeValue(tmpFile, data); renameFile(tmpFile.getName(),_configFileName); tmpFile.delete(); } @@ -565,17 +566,22 @@ public class JsonFileConfigStore implements DurableConfigurationStore } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { - if (_configFileName != null) + FileBasedSettings fileBasedSettings = (FileBasedSettings)parent; + String storePath = fileBasedSettings.getStorePath(); + + if (storePath != null) { - File configFile = new File(_directoryName, _configFileName); - if (!configFile.delete()) + File configFile = new File(storePath); + if (!FileUtils.delete(configFile, true)) { - _logger.info("Failed to delete JSON file config store: " + _configFileName); + _logger.info("Failed to delete the store at location " + storePath); } - _configFileName = null; } + + _configFileName = null; + _directoryName = null; } private void releaseFileLock() diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/MemoryMessageStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/MemoryMessageStore.java index 9c0a5118ff..c108918d34 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/MemoryMessageStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/MemoryMessageStore.java @@ -260,7 +260,7 @@ public class MemoryMessageStore implements MessageStore } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/MessageStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/MessageStore.java index a4eaf48353..1629454cde 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/MessageStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/MessageStore.java @@ -79,5 +79,5 @@ public interface MessageStore */ void closeMessageStore(); - void onDelete(); + void onDelete(ConfiguredObject parent); } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/store/NullMessageStore.java b/java/broker-core/src/main/java/org/apache/qpid/server/store/NullMessageStore.java index 2e6c437e95..b6f4ea52ce 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/store/NullMessageStore.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/store/NullMessageStore.java @@ -117,7 +117,7 @@ public abstract class NullMessageStore implements MessageStore, DurableConfigura } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { } diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java index eacc4f2458..2d58cb9327 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java @@ -1175,7 +1175,7 @@ public abstract class AbstractVirtualHost> exte { try { - ms.onDelete(); + ms.onDelete(this); } catch (Exception e) { diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java index e866effc54..45a3c1eb5a 100644 --- a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java +++ b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/AbstractVirtualHostNode.java @@ -191,7 +191,7 @@ public abstract class AbstractVirtualHostNode parent) { } diff --git a/java/broker-core/src/test/java/org/apache/qpid/server/store/JsonFileConfigStoreTest.java b/java/broker-core/src/test/java/org/apache/qpid/server/store/JsonFileConfigStoreTest.java index 72e14c7a4b..d76ba0bc76 100644 --- a/java/broker-core/src/test/java/org/apache/qpid/server/store/JsonFileConfigStoreTest.java +++ b/java/broker-core/src/test/java/org/apache/qpid/server/store/JsonFileConfigStoreTest.java @@ -34,6 +34,7 @@ import java.util.HashMap; import java.util.Map; import java.util.UUID; +import org.apache.qpid.server.model.VirtualHostNode; import org.mockito.ArgumentMatcher; import org.mockito.InOrder; @@ -360,7 +361,7 @@ public class JsonFileConfigStoreTest extends QpidTestCase _store.closeConfigurationStore(); assertTrue("JSON store should exist after close", expectedJsonFile.exists()); - _store.onDelete(); + _store.onDelete(_parent); assertFalse("JSON store should not exist after delete", expectedJsonFile.exists()); } diff --git a/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java b/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java index 5866319985..31089356a8 100644 --- a/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java +++ b/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyConfigurationStore.java @@ -21,6 +21,7 @@ package org.apache.qpid.server.store.derby; +import java.io.File; import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; @@ -31,6 +32,7 @@ import org.apache.log4j.Logger; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.store.*; +import org.apache.qpid.util.FileUtils; /** * Implementation of a DurableConfigurationStore backed by Apache Derby @@ -139,7 +141,7 @@ public class DerbyConfigurationStore extends AbstractJDBCConfigurationStore } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { if (_providedMessageStore.isMessageStoreOpen()) { @@ -151,18 +153,19 @@ public class DerbyConfigurationStore extends AbstractJDBCConfigurationStore LOGGER.debug("Deleting store " + _storeLocation); } - try - { - DerbyUtils.deleteDatabaseLocation(_storeLocation); - } - catch (StoreException se) - { - LOGGER.debug("Failed to delete the store at location " + _storeLocation); - } - finally + FileBasedSettings fileBasedSettings = (FileBasedSettings)parent; + String storePath = fileBasedSettings.getStorePath(); + + if (storePath != null) { - _storeLocation = null; + File configFile = new File(storePath); + if (!FileUtils.delete(configFile, true)) + { + LOGGER.info("Failed to delete the store at location " + storePath); + } } + + _storeLocation = null; } @Override diff --git a/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyMessageStore.java b/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyMessageStore.java index d9948fe21e..9c6baa52ba 100644 --- a/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyMessageStore.java +++ b/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyMessageStore.java @@ -21,6 +21,7 @@ package org.apache.qpid.server.store.derby; +import java.io.File; import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; @@ -30,6 +31,7 @@ import org.apache.log4j.Logger; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.store.FileBasedSettings; import org.apache.qpid.server.store.StoreException; +import org.apache.qpid.util.FileUtils; /** * Implementation of a MessageStore backed by Apache Derby. @@ -71,11 +73,11 @@ public class DerbyMessageStore extends AbstractDerbyMessageStore } @Override - public void onDelete() + public void onDelete(ConfiguredObject parent) { if (isMessageStoreOpen()) { - throw new IllegalStateException("Cannot delete the store as the message store is still open"); + throw new IllegalStateException("Cannot delete the store as the provided message store is still open"); } if (LOGGER.isDebugEnabled()) @@ -83,18 +85,19 @@ public class DerbyMessageStore extends AbstractDerbyMessageStore LOGGER.debug("Deleting store " + _storeLocation); } - try - { - DerbyUtils.deleteDatabaseLocation(_storeLocation); - } - catch (StoreException se) - { - LOGGER.debug("Failed to delete the store at location " + _storeLocation); - } - finally + FileBasedSettings fileBasedSettings = (FileBasedSettings)parent; + String storePath = fileBasedSettings.getStorePath(); + + if (storePath != null) { - _storeLocation = null; + File configFile = new File(storePath); + if (!FileUtils.delete(configFile, true)) + { + LOGGER.info("Failed to delete the store at location " + storePath); + } } + + _storeLocation = null; } @Override diff --git a/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyUtils.java b/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyUtils.java index b0f4a137f2..9bdce9af1c 100644 --- a/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyUtils.java +++ b/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/store/derby/DerbyUtils.java @@ -101,26 +101,6 @@ public class DerbyUtils } } - public static void deleteDatabaseLocation(String storeLocation) - { - if (MEMORY_STORE_LOCATION.equals(storeLocation)) - { - return; - } - - if (storeLocation != null) - { - File location = new File(storeLocation); - if (location.exists()) - { - if (!FileUtils.delete(location, true)) - { - throw new StoreException("Failed to delete the store at location : " + storeLocation); - } - } - } - } - public static String getBlobAsString(ResultSet rs, int col) throws SQLException { Blob blob = rs.getBlob(col); diff --git a/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreTest.java b/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreTest.java index ad3246290c..0b1847bb59 100644 --- a/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreTest.java +++ b/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreTest.java @@ -57,7 +57,10 @@ public class DerbyMessageStoreTest extends MessageStoreTestCase getStore().closeMessageStore(); assertTrue("Store does not exist at " + _storeLocation, location.exists()); - getStore().onDelete(); + DerbyVirtualHost mockVH = mock(DerbyVirtualHost.class); + when(mockVH.getStorePath()).thenReturn(_storeLocation); + + getStore().onDelete(mockVH); assertFalse("Store exists at " + _storeLocation, location.exists()); } diff --git a/java/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java b/java/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java index 207c5a8325..3a85ae3257 100644 --- a/java/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java +++ b/java/broker-plugins/jdbc-store/src/test/java/org/apache/qpid/server/store/jdbc/JDBCMessageStoreTest.java @@ -59,7 +59,7 @@ public class JDBCMessageStoreTest extends MessageStoreTestCase assertTablesExist(expectedTables, true); getStore().closeMessageStore(); assertTablesExist(expectedTables, true); - getStore().onDelete(); + getStore().onDelete(mock(JDBCVirtualHost.class)); assertTablesExist(expectedTables, false); } -- cgit v1.2.1