[FIXED JENKINS-16976] Option to disable nested directory creation for…

… broken FTP servers.
jenkinsci · Mar 3, 2013 · d34aa3f · d34aa3f
1 parent a972ce7
commit d34aa3f
Show file tree

Hide file tree

Showing 11 changed files with 90 additions and 8 deletions.
diff --git a/src/main/java/jenkins/plugins/publish_over_ftp/BapFtpClient.java b/src/main/java/jenkins/plugins/publish_over_ftp/BapFtpClient.java
@@ -44,12 +44,17 @@ public class BapFtpClient extends BPDefaultClient<BapFtpTransfer> {
 
     private BPBuildInfo buildInfo;
     private FTPClient ftpClient;
+    private boolean disableMakeNestedDirs;
 
     public BapFtpClient(final FTPClient ftpClient, final BPBuildInfo buildInfo) {
         this.ftpClient = ftpClient;
         this.buildInfo = buildInfo;
     }
 
+    public void setDisableMakeNestedDirs(final boolean disableMakeNestedDirs) {
+        this.disableMakeNestedDirs = disableMakeNestedDirs;
+    }
+
     public FTPClient getFtpClient() { return ftpClient; }
     public void setFtpClient(final FTPClient ftpClient) { this.ftpClient = ftpClient; }
 
@@ -66,6 +71,7 @@ public boolean changeDirectory(final String directory) {
 
     public boolean makeDirectory(final String directory) {
         try {
+            if (disableMakeNestedDirs && directory.contains("/")) return false;
             return ftpClient.makeDirectory(directory);
         } catch (IOException ioe) {
             throw new BapPublisherException(Messages.exception_mkdirException(directory), ioe);

diff --git a/src/main/java/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration.java b/src/main/java/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration.java
@@ -56,15 +56,17 @@ public class BapFtpHostConfiguration extends BPHostConfiguration<BapFtpClient, O
     private int timeout;
     private boolean useActiveData;
     private final String controlEncoding;
+    private final boolean disableMakeNestedDirs;
 
     @DataBoundConstructor
     public BapFtpHostConfiguration(final String name, final String hostname, final String username, final String encryptedPassword,
                                    final String remoteRootDir, final int port, final int timeout, final boolean useActiveData,
-                                   final String controlEncoding) {
+                                   final String controlEncoding, final boolean disableMakeNestedDirs) {
         super(name, hostname, username, encryptedPassword, remoteRootDir, port);
         this.timeout = timeout;
         this.useActiveData = useActiveData;
         this.controlEncoding = Util.fixEmptyAndTrim(controlEncoding);
+        this.disableMakeNestedDirs = disableMakeNestedDirs;
     }
 
     protected final String getPassword() {
@@ -81,6 +83,10 @@ public String getControlEncoding() {
         return controlEncoding;
     }
 
+    public boolean isDisableMakeNestedDirs() {
+        return disableMakeNestedDirs;
+    }
+
     @Override
     public BapFtpClient createClient(final BPBuildInfo buildInfo) {
         final BapFtpClient client = new BapFtpClient(createFTPClient(), buildInfo);
@@ -100,6 +106,7 @@ public FTPClient createFTPClient() {
     private void init(final BapFtpClient client) throws IOException {
         final FTPClient ftpClient = client.getFtpClient();
         final BPBuildInfo buildInfo = client.getBuildInfo();
+        client.setDisableMakeNestedDirs(disableMakeNestedDirs);
         PrintCommandListener commandPrinter = null;
         if (buildInfo.isVerbose()) {
             commandPrinter = new PrintCommandListener(new PrintWriter(buildInfo.getListener().getLogger()));

diff --git a/src/main/resources/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/config.jelly b/src/main/resources/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/config.jelly
@@ -53,6 +53,9 @@
             <f:entry title="${%useActiveData}" field="useActiveData">
               <f:checkbox/>
             </f:entry>
+            <f:entry title="${%disableMakeNestedDirs}" field="disableMakeNestedDirs">
+              <f:checkbox/>
+            </f:entry>
             <f:entry title="${%controlEncoding}" field="controlEncoding">
               <f:textbox/>
             </f:entry>

diff --git a/...main/resources/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/config.properties b/...main/resources/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/config.properties
@@ -24,3 +24,4 @@
 
 useActiveData=Use active data mode
 controlEncoding=Control encoding
+disableMakeNestedDirs=Don''t make nested dirs
diff --git a/...esources/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/config_no_BV.properties b/...esources/jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/config_no_BV.properties
@@ -24,3 +24,4 @@
 
 useActiveData=U*e a*t*v* d*t* m*d*
 controlEncoding=C*n*r*l e*c*d*n*
+disableMakeNestedDirs=D*n''* m*k* n*s*e* d*r*
diff --git a/.../jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/help-disableMakeNestedDirs.html b/.../jenkins/plugins/publish_over_ftp/BapFtpHostConfiguration/help-disableMakeNestedDirs.html
@@ -0,0 +1,35 @@
+<!--
+  ~ The MIT License
+  ~
+  ~ Copyright (C) 2010-2011 by Anthony Robinson
+  ~
+  ~ Permission is hereby granted, free of charge, to any person obtaining a copy
+  ~ of this software and associated documentation files (the "Software"), to deal
+  ~ in the Software without restriction, including without limitation the rights
+  ~ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+  ~ copies of the Software, and to permit persons to whom the Software is
+  ~ furnished to do so, subject to the following conditions:
+  ~
+  ~ The above copyright notice and this permission notice shall be included in
+  ~ all copies or substantial portions of the Software.
+  ~
+  ~ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+  ~ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+  ~ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+  ~ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+  ~ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+  ~ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+  ~ THE SOFTWARE.
+  -->
+
+<div>
+    <p>Do not try to create nested directories with a single MKD command</p>
+    <p>The default behaviour when creating directories is to try to create the target directory
+        including any missing directories with a single command. If this fails, the plugin
+        will fall back to creating each directory individually. Some FTP servers, when asked to create
+        nested directories will respond with a success code, but will not create the directories
+        correctly. This will result in directories being created in incorrect branches of the directory
+        tree.</p>
+    <p>This option will not prevent the creation of nested directories, it is just a fix for badly
+        behaved FTP servers.</p>
+</div>
diff --git a/src/test/java/jenkins/plugins/publish_over_ftp/BapFtpClientTest.java b/src/test/java/jenkins/plugins/publish_over_ftp/BapFtpClientTest.java
@@ -175,6 +175,20 @@ public void setUp() throws Exception {
         mockControl.verify();
     }
 
+    @Test public void testMakeDirectoryWillAttemptNested() throws Exception {
+        expect(mockFTPClient.makeDirectory(DIRECTORY)).andReturn(false);
+        mockControl.replay();
+        assertFalse(bapFtpClient.makeDirectory(DIRECTORY));
+        mockControl.verify();
+    }
+
+    @Test public void testMakeDirectoryWillNotAttemptNested() throws Exception {
+        bapFtpClient.setDisableMakeNestedDirs(true);
+        mockControl.replay();
+        assertFalse(bapFtpClient.makeDirectory(DIRECTORY));
+        mockControl.verify();
+    }
+
     @Test public void testMakeDirectoryIOException() throws Exception {
         expect(mockFTPClient.makeDirectory(DIRECTORY)).andThrow(IO_EXCEPTION);
         mockControl.replay();

diff --git a/src/test/java/jenkins/plugins/publish_over_ftp/BapHostConfigurationTest.java b/src/test/java/jenkins/plugins/publish_over_ftp/BapHostConfigurationTest.java
@@ -40,6 +40,7 @@
 import java.io.IOException;
 
 import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
 import static org.easymock.EasyMock.expect;
 
 @SuppressWarnings({ "PMD.SignatureDeclareThrowsException", "PMD.TooManyMethods" })
@@ -58,7 +59,7 @@ public static void after() {
     private final transient BPBuildInfo buildInfo = new BPBuildInfo(TaskListener.NULL, "", new FilePath(new File("")), null, null);
     private final transient IMocksControl mockControl = EasyMock.createStrictControl();
     private final transient FTPClient mockFTPClient = mockControl.createMock(FTPClient.class);
-    private final transient BapFtpHostConfiguration bapFtpHostConfiguration = new BapFtpHostConfigurationWithMockFTPClient(mockFTPClient);
+    private transient BapFtpHostConfiguration bapFtpHostConfiguration = new BapFtpHostConfigurationWithMockFTPClient(mockFTPClient);
 
     @Test public void testChangeToRootDir() throws Exception {
         assertChangeToInitialDirectory("/");
@@ -119,6 +120,17 @@ private void assertChangeToInitialDirectory(final String remoteRoot, final boole
         assertCreateSession();
     }
 
+    @Test public void testDisableMakeNestedDirs() throws Exception {
+        bapFtpHostConfiguration = new BapFtpHostConfigurationWithMockFTPClient(mockFTPClient, true);
+        expectConnectAndLogin();
+        expect(mockFTPClient.printWorkingDirectory()).andReturn("/");
+        final BapFtpClient client = assertCreateSession();
+        mockControl.reset();
+        mockControl.replay();
+        assertFalse(client.makeDirectory("more/than/one"));
+        mockControl.verify();
+    }
+
     private void expectConnectAndLogin() throws Exception {
         mockFTPClient.setDefaultTimeout(bapFtpHostConfiguration.getTimeout());
         mockFTPClient.setDataTimeout(bapFtpHostConfiguration.getTimeout());
@@ -146,10 +158,13 @@ private static class BapFtpHostConfigurationWithMockFTPClient extends BapFtpHost
         private static final String TEST_USERNAME = "myTestUsername";
         private static final String TEST_PASSWORD = "myTestPassword";
         private final transient FTPClient ftpClient;
-        BapFtpHostConfigurationWithMockFTPClient(final FTPClient ftpClient) {
-            super(TEST_CFG_NAME, TEST_HOSTNAME, TEST_USERNAME, TEST_PASSWORD, "", DEFAULT_PORT, DEFAULT_TIMEOUT, false, null);
+        BapFtpHostConfigurationWithMockFTPClient(final FTPClient ftpClient, final boolean disableMakeNestedDirs) {
+            super(TEST_CFG_NAME, TEST_HOSTNAME, TEST_USERNAME, TEST_PASSWORD, "", DEFAULT_PORT, DEFAULT_TIMEOUT, false, null, disableMakeNestedDirs);
             this.ftpClient = ftpClient;
         }
+        BapFtpHostConfigurationWithMockFTPClient(final FTPClient ftpClient) {
+            this(ftpClient, false);
+        }
         @Override
         public FTPClient createFTPClient() {
             return ftpClient;

diff --git a/src/test/java/jenkins/plugins/publish_over_ftp/jenkins/CurrentConfigurationTest.java b/src/test/java/jenkins/plugins/publish_over_ftp/jenkins/CurrentConfigurationTest.java
@@ -52,8 +52,8 @@ public void testTestsAreDisabled() throws Exception {
     }
 //    @TODO figure out why this no longer works
     public void dontTestRoundTrip() throws Exception {
-        final BapFtpHostConfiguration configA = new BapFtpHostConfiguration("host A", "", "", "", "", 0, 0, false, null);
-        final BapFtpHostConfiguration configB = new BapFtpHostConfiguration("host B", "", "", "", "", 0, 0, false, null);
+        final BapFtpHostConfiguration configA = new BapFtpHostConfiguration("host A", "", "", "", "", 0, 0, false, null, false);
+        final BapFtpHostConfiguration configB = new BapFtpHostConfiguration("host B", "", "", "", "", 0, 0, false, null, false);
         final FreeStyleProject project = createFreeStyleProject();
         new JenkinsTestHelper().setGlobalConfig(configA, configB);
         final BapFtpPublisherPlugin plugin = createPlugin(configA.getName(), configB.getName());

diff --git a/src/test/java/jenkins/plugins/publish_over_ftp/jenkins/IntegrationTest.java b/src/test/java/jenkins/plugins/publish_over_ftp/jenkins/IntegrationTest.java
@@ -64,7 +64,7 @@ public void testIntegration() throws Exception {
         final int port = 21;
         final int timeout = 3000;
         final BapFtpHostConfiguration testHostConfig = new BapFtpHostConfiguration("testConfig", "testHostname", "testUsername",
-                                                                        TEST_PASSWORD, "/testRemoteRoot", port, timeout, false, null) {
+                                                                        TEST_PASSWORD, "/testRemoteRoot", port, timeout, false, null, false) {
             @Override
             public FTPClient createFTPClient() {
                 return mockFTPClient;

diff --git a/src/test/java/jenkins/plugins/publish_over_ftp/jenkins/LegacyConfigurationTest.java b/src/test/java/jenkins/plugins/publish_over_ftp/jenkins/LegacyConfigurationTest.java
@@ -86,7 +86,7 @@ private BapFtpPublisherPlugin getConfiguredPlugin() {
     public BapFtpHostConfiguration createHostConfiguration(final String suffix, final int port,
                                                            final int timeout, final boolean useActiveData) {
         return new BapFtpHostConfiguration("Config " + suffix, "hostname." + suffix, "username." + suffix,
-            "password." + suffix, "remoteRoot." + suffix, port, timeout, useActiveData, null);
+            "password." + suffix, "remoteRoot." + suffix, port, timeout, useActiveData, null, false);
     }
 
 }