QPID-6446: Verify that store path is a writable folder on creation of derby virtual host and node

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1670629 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 263 insertions, 4 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/FileHelper.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/FileHelper.java
index 0e1a28f220..a13e3f52f9 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/FileHelper.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/util/FileHelper.java
@@ -130,4 +130,30 @@ public class FileHelper
             }
         }
     }
+
+    public boolean isWritableDirectory(String path)
+    {
+        File storePath = new File(path);
+        if (storePath.exists())
+        {
+            if (!storePath.isDirectory())
+            {
+                return false;
+            }
+        }
+        else
+        {
+            do
+            {
+                storePath = storePath.getParentFile();
+                if (storePath == null)
+                {
+                    return false;
+                }
+            }
+            while (!storePath.exists());
+        }
+        return storePath.canWrite();
+    }
+
 }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
index 76fbb4f48e..e0b0d37cbe 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
@@ -297,7 +297,7 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte
 
 
 
-    private void validateMessageStoreCreation()
+    protected void validateMessageStoreCreation()
     {
         MessageStore store = createMessageStore();
         if (store != null)
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/FileHelperTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/FileHelperTest.java
index 9d47ed496a..27581fa60f 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/FileHelperTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/util/FileHelperTest.java
@@ -27,10 +27,11 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.PosixFileAttributeView;
 import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.PosixFilePermissions;
 import java.util.Set;
 
 import org.apache.qpid.test.utils.QpidTestCase;
+import org.apache.qpid.test.utils.TestFileUtils;
+import org.apache.qpid.util.FileUtils;
 
 public class FileHelperTest extends QpidTestCase
 {
@@ -121,6 +122,43 @@ public class FileHelperTest extends QpidTestCase
         }
     }
 
+    public void testIsWritableDirectoryForFilePath() throws Exception
+    {
+        File workDir = TestFileUtils.createTestDirectory("test", true);
+        try
+        {
+            File file = new File(workDir, getTestName());
+            file.createNewFile();
+            assertFalse("Should return false for a file", _fileHelper.isWritableDirectory(file.getAbsolutePath()));
+        }
+        finally
+        {
+            FileUtils.delete(workDir, true);
+        }
+    }
+
+
+    public void testIsWritableDirectoryForNonWritablePath() throws Exception
+    {
+        File workDir = TestFileUtils.createTestDirectory("test", true);
+        try
+        {
+            if (Files.getFileStore(workDir.toPath()).supportsFileAttributeView(PosixFileAttributeView.class))
+            {
+                File file = new File(workDir, getTestName());
+                file.mkdirs();
+                if (file.setWritable(false, false))
+                {
+                    assertFalse("Should return false for non writable folder",
+                            _fileHelper.isWritableDirectory(new File(file, "test").getAbsolutePath()));
+                }
+            }
+        }
+        finally
+        {
+            FileUtils.delete(workDir, true);
+        }
+    }
 
     private void assertPermissions(Path path) throws IOException
     {
diff --git a/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhost/derby/DerbyVirtualHostImpl.java b/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhost/derby/DerbyVirtualHostImpl.java
index 5089090643..dbaf5e55b7 100644
--- a/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhost/derby/DerbyVirtualHostImpl.java
+++ b/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhost/derby/DerbyVirtualHostImpl.java
@@ -20,12 +20,14 @@
  */
 package org.apache.qpid.server.virtualhost.derby;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.VirtualHostNode;
 import org.apache.qpid.server.store.MessageStore;
 import org.apache.qpid.server.store.derby.DerbyMessageStore;
+import org.apache.qpid.server.util.FileHelper;
 import org.apache.qpid.server.virtualhost.AbstractVirtualHost;
 
 import java.util.Map;
@@ -75,4 +77,13 @@ public class DerbyVirtualHostImpl extends AbstractVirtualHost<DerbyVirtualHostIm
     {
         return _storeOverfullSize;
     }
+
+    @Override
+    protected void validateMessageStoreCreation()
+    {
+        if (!new FileHelper().isWritableDirectory(getStorePath()))
+        {
+            throw new IllegalConfigurationException("The store path is not writable directory");
+        }
+    }
 }
diff --git a/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhostnode/derby/DerbyVirtualHostNodeImpl.java b/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhostnode/derby/DerbyVirtualHostNodeImpl.java
index dc768aff09..8224a22929 100644
--- a/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhostnode/derby/DerbyVirtualHostNodeImpl.java
+++ b/qpid/java/broker-plugins/derby-store/src/main/java/org/apache/qpid/server/virtualhostnode/derby/DerbyVirtualHostNodeImpl.java
@@ -25,6 +25,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.logging.messages.ConfigStoreMessages;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ManagedAttributeField;
@@ -33,6 +34,7 @@ import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.store.DurableConfigurationStore;
 import org.apache.qpid.server.store.derby.DerbyConfigurationStore;
+import org.apache.qpid.server.util.FileHelper;
 import org.apache.qpid.server.virtualhostnode.AbstractStandardVirtualHostNode;
 
 @ManagedObject( category = false,
@@ -80,4 +82,13 @@ public class DerbyVirtualHostNodeImpl extends AbstractStandardVirtualHostNode<De
     {
         return Collections.singletonMap(VirtualHost.class.getSimpleName(), getSupportedVirtualHostTypes(true));
     }
+
+    @Override
+    public void validateOnCreate()
+    {
+        if (!new FileHelper().isWritableDirectory(getStorePath()))
+        {
+            throw new IllegalConfigurationException("The store path is not writable directory");
+        }
+    }
 }
diff --git a/qpid/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/virtualhostnode/derby/DerbyVirtualHostNodeTest.java b/qpid/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/virtualhostnode/derby/DerbyVirtualHostNodeTest.java
new file mode 100644
index 0000000000..5afbc7aceb
--- /dev/null
+++ b/qpid/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/virtualhostnode/derby/DerbyVirtualHostNodeTest.java
@@ -0,0 +1,175 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.qpid.server.virtualhostnode.derby;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.configuration.updater.TaskExecutor;
+import org.apache.qpid.server.configuration.updater.TaskExecutorImpl;
+import org.apache.qpid.server.logging.EventLogger;
+import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.BrokerModel;
+import org.apache.qpid.server.model.ConfiguredObjectFactoryImpl;
+import org.apache.qpid.server.model.SystemConfig;
+import org.apache.qpid.server.model.VirtualHostNode;
+import org.apache.qpid.server.model.adapter.BrokerAdapter;
+import org.apache.qpid.test.utils.QpidTestCase;
+import org.apache.qpid.test.utils.TestFileUtils;
+
+public class DerbyVirtualHostNodeTest extends QpidTestCase
+{
+    private TaskExecutor _taskExecutor;
+    private File _workDir;
+    private Broker<BrokerAdapter> _broker;
+
+    @Override
+    public void setUp() throws Exception
+    {
+        super.setUp();
+        _taskExecutor = new TaskExecutorImpl();
+        _taskExecutor.start();
+        _workDir = TestFileUtils.createTestDirectory("qpid.work_dir", true);
+        setTestSystemProperty("qpid.work_dir", _workDir.getAbsolutePath());
+        _broker = createBroker();
+    }
+
+    public void tearDown() throws Exception
+    {
+        try
+        {
+            _broker.close();
+        }
+        finally
+        {
+            _taskExecutor.stop();
+            TestFileUtils.delete(_workDir, true);
+            super.tearDown();
+        }
+    }
+
+    public void testCreateAndCloseVirtualHostNode() throws Exception
+    {
+        String nodeName = getTestName();
+        Map<String, Object> nodeData = new HashMap<>();
+        nodeData.put(VirtualHostNode.NAME, nodeName);
+        nodeData.put(VirtualHostNode.TYPE, DerbyVirtualHostNodeImpl.VIRTUAL_HOST_NODE_TYPE);
+
+        VirtualHostNode<?> virtualHostNode = (VirtualHostNode<?>)_broker.createChild(VirtualHostNode.class, nodeData);
+        virtualHostNode.start();
+        virtualHostNode.close();
+    }
+
+
+    public void testCreateDuplicateVirtualHostNodeAndClose() throws Exception
+    {
+
+        String nodeName = getTestName();
+        Map<String, Object> nodeData = new HashMap<>();
+        nodeData.put(VirtualHostNode.NAME, nodeName);
+        nodeData.put(VirtualHostNode.TYPE, DerbyVirtualHostNodeImpl.VIRTUAL_HOST_NODE_TYPE);
+
+        VirtualHostNode<?> virtualHostNode = (VirtualHostNode<?>)_broker.createChild(VirtualHostNode.class, nodeData);
+        virtualHostNode.start();
+
+        try
+        {
+            _broker.createChild(VirtualHostNode.class, nodeData);
+        }
+        catch(Exception e)
+        {
+            assertEquals("Unexpected message", "Child of type " + virtualHostNode.getClass().getSimpleName() + " already exists with name of " + getTestName(), e.getMessage());
+        }
+        virtualHostNode.close();
+    }
+
+    public void testOnCreateValidationForFileStorePath() throws Exception
+    {
+        File file = new File(_workDir, getTestName());
+        file.createNewFile();
+
+        String nodeName = getTestName();
+        Map<String, Object> nodeData = new HashMap<>();
+        nodeData.put(VirtualHostNode.NAME, nodeName);
+        nodeData.put(VirtualHostNode.TYPE, DerbyVirtualHostNodeImpl.VIRTUAL_HOST_NODE_TYPE);
+        nodeData.put(DerbyVirtualHostNodeImpl.STORE_PATH, file.getAbsolutePath());
+        try
+        {
+            _broker.createChild(VirtualHostNode.class, nodeData);
+            fail("Cannot create store for the file store path");
+        }
+        catch(IllegalConfigurationException e)
+        {
+            // pass
+        }
+
+    }
+
+
+    public void testOnCreateValidationForNonWritableStorePath() throws Exception
+    {
+        if (Files.getFileStore(_workDir.toPath()).supportsFileAttributeView(PosixFileAttributeView.class))
+        {
+            File file = new File(_workDir, getTestName());
+            file.mkdirs();
+            if (file.setWritable(false, false))
+            {
+                String nodeName = getTestName();
+                Map<String, Object> nodeData = new HashMap<>();
+                nodeData.put(VirtualHostNode.NAME, nodeName);
+                nodeData.put(VirtualHostNode.TYPE, DerbyVirtualHostNodeImpl.VIRTUAL_HOST_NODE_TYPE);
+                nodeData.put(DerbyVirtualHostNodeImpl.STORE_PATH, file.getAbsolutePath());
+                try
+                {
+                    _broker.createChild(VirtualHostNode.class, nodeData);
+                    fail("Cannot create store for the non writable store path");
+                }
+                catch (IllegalConfigurationException e)
+                {
+                    // pass
+                }
+            }
+        }
+    }
+
+    private BrokerAdapter createBroker()
+    {
+        Map<String, Object> brokerAttributes = Collections.<String, Object>singletonMap(Broker.NAME, "Broker");
+        SystemConfig parent = mock(SystemConfig.class);
+        when(parent.getEventLogger()).thenReturn(new EventLogger());
+        when(parent.getCategoryClass()).thenReturn(SystemConfig.class);
+        when(parent.getTaskExecutor()).thenReturn(_taskExecutor);
+        when(parent.getChildExecutor()).thenReturn(_taskExecutor);
+        when(parent.getModel()).thenReturn(BrokerModel.getInstance());
+        when(parent.getObjectFactory()).thenReturn(new ConfiguredObjectFactoryImpl(BrokerModel.getInstance()));
+        BrokerAdapter broker = new BrokerAdapter(brokerAttributes, parent);
+        broker.start();
+        return broker;
+    }
+}
diff --git a/qpid/java/test-profiles/JavaDerbyExcludes b/qpid/java/test-profiles/JavaDerbyExcludes
index 49db2ec00c..814406f04d 100644
--- a/qpid/java/test-profiles/JavaDerbyExcludes
+++ b/qpid/java/test-profiles/JavaDerbyExcludes
@@ -20,5 +20,3 @@
 org.apache.qpid.server.store.berkeleydb.*
 org.apache.qpid.server.store.berkeleydb.replication.*
 
-# QPID-6446: temporary disable the test until issue is resolved
-org.apache.qpid.systest.rest.VirtualHostNodeRestTest#testCreateVirtualHostNodeByPost