Fix retry when file download fails1.0.2

When the Datafile collector is unable to download the file from an xNF it now retries to collect the file.

Change-Id: I61f68f9cf7af1a7fab160b0e936daafd1a23aaf8
Issue-ID: DCAEGEN2-864
Signed-off-by: elinuxhenrik <henrik.b.andersson@est.tech>

9 files changed, 214 insertions, 60 deletions

diff --git a/datafile-dmaap-client/pom.xml b/datafile-dmaap-client/pom.xml
index 21839ab1..9eb232ba 100644
--- a/datafile-dmaap-client/pom.xml
+++ b/datafile-dmaap-client/pom.xml
@@ -24,7 +24,7 @@
   <parent>
     <groupId>org.onap.dcaegen2.collectors</groupId>
     <artifactId>datafile</artifactId>
-    <version>1.0.0-SNAPSHOT</version>
+    <version>1.0.2-SNAPSHOT</version>
   </parent>
 
   <groupId>org.onap.dcaegen2.collectors.datafile</groupId>
diff --git a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/ErrorData.java b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/ErrorData.java
new file mode 100644
index 00000000..2585bbce
--- /dev/null
+++ b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/ErrorData.java
@@ -0,0 +1,41 @@
+/*
+ * ============LICENSE_START======================================================================
+ * Copyright (C) 2018 Nordix Foundation. All rights reserved.
+ * ===============================================================================================
+ * Licensed 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.
+ * ============LICENSE_END========================================================================
+ */
+
+package org.onap.dcaegen2.collectors.datafile.ftp;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class ErrorData {
+    private List<String> errorMessages = new ArrayList<>();
+    private List<Throwable> errorCauses = new ArrayList<>();
+
+    public void addError(String errorMessage, Throwable errorCause) {
+        errorMessages.add(errorMessage);
+        errorCauses.add(errorCause);
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder message = new StringBuilder();
+        for (int i = 0; i < errorMessages.size(); i++) {
+            message.append(errorMessages.get(i)).append(" Cause: ").append(errorCauses.get(i)).append("\n");
+        }
+        return message.toString();
+    }
+}
diff --git a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FileCollectClient.java b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FileCollectClient.java
new file mode 100644
index 00000000..b50b0457
--- /dev/null
+++ b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FileCollectClient.java
@@ -0,0 +1,53 @@
+/*
+ * ============LICENSE_START======================================================================
+ * Copyright (C) 2018 Nordix Foundation. All rights reserved.
+ * ===============================================================================================
+ * Licensed 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.
+ * ============LICENSE_END========================================================================
+ */
+
+package org.onap.dcaegen2.collectors.datafile.ftp;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * @author
+ *
+ */
+public abstract class FileCollectClient {
+    protected static final Logger logger = LoggerFactory.getLogger(FtpsClient.class);
+
+    protected FileServerData fileServerData;
+    protected String remoteFile;
+    protected String localFile;
+    protected ErrorData errorData;
+
+    public FileCollectResult collectFile(FileServerData fileServerData, String remoteFile, String localFile) {
+        logger.trace("collectFile called with fileServerData: {}, remoteFile: {}, localFile: {}", fileServerData,
+                remoteFile, localFile);
+
+        this.fileServerData = fileServerData;
+        this.remoteFile = remoteFile;
+        this.localFile = localFile;
+
+        return retryCollectFile();
+    }
+
+    public abstract FileCollectResult retryCollectFile();
+
+    protected void addError(String errorMessage, Throwable errorCause) {
+        if (errorData == null) {
+            errorData = new ErrorData();
+        }
+        errorData.addError(errorMessage, errorCause);
+    }
+}
diff --git a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FileCollectResult.java b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FileCollectResult.java
new file mode 100644
index 00000000..6cd048ac
--- /dev/null
+++ b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FileCollectResult.java
@@ -0,0 +1,46 @@
+/*
+ * ============LICENSE_START======================================================================
+ * Copyright (C) 2018 Nordix Foundation. All rights reserved.
+ * ===============================================================================================
+ * Licensed 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.
+ * ============LICENSE_END========================================================================
+ */
+
+package org.onap.dcaegen2.collectors.datafile.ftp;
+
+public class FileCollectResult {
+    private boolean result;
+    private ErrorData errorData;
+
+    public FileCollectResult() {
+        this.result = true;
+    }
+
+    public FileCollectResult(ErrorData errorData) {
+        this.errorData = errorData;
+        result = false;
+    }
+
+    public boolean downloadSuccessful() {
+        return result;
+    }
+
+    public String getErrorData() {
+        return errorData.toString();
+    }
+
+    @Override
+    public String toString() {
+        return "Download successful: " + result + " Error data: " + getErrorData();
+    }
+}
diff --git a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java
index 719013ea..a88072c7 100644
--- a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java
+++ b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java
@@ -36,8 +36,6 @@ import org.onap.dcaegen2.collectors.datafile.ssl.ITrustManagerFactory;
 import org.onap.dcaegen2.collectors.datafile.ssl.KeyManagerUtilsWrapper;
 import org.onap.dcaegen2.collectors.datafile.ssl.KeyStoreWrapper;
 import org.onap.dcaegen2.collectors.datafile.ssl.TrustManagerFactoryWrapper;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Component;
 
 /**
@@ -46,9 +44,7 @@ import org.springframework.stereotype.Component;
  * @author <a href="mailto:martin.c.yan@est.tech">Martin Yan</a>
  */
 @Component
-public class FtpsClient {
-    private static final Logger logger = LoggerFactory.getLogger(FtpsClient.class);
-
+public class FtpsClient extends FileCollectClient {
     private String keyCertPath;
     private String keyCertPassword;
     private String trustedCAPath;
@@ -58,27 +54,32 @@ public class FtpsClient {
     private IKeyManagerUtils kmu;
     private IKeyStore keyStore;
     private ITrustManagerFactory trustManagerFactory;
-    private IFile localFile;
+    private IFile lf;
     private IFileSystemResource fileResource;
     private IOutputStream os;
 
-    public boolean collectFile(FileServerData fileServerData, String remoteFile, String localFile) {
-        logger.trace("collectFile called with fileServerData: {}, remoteFile: {}, localFile: {}", fileServerData,
-                remoteFile, localFile);
-        boolean result = true;
+    @Override
+    public FileCollectResult retryCollectFile() {
+        logger.trace("retryCollectFile called");
+
+        FileCollectResult fileCollectResult;
+
         IFTPSClient ftps = getFtpsClient();
 
         ftps.setNeedClientAuth(true);
 
-        if (setUpKeyManager(ftps) && setUpTrustedCA(ftps) && setUpConnection(fileServerData, ftps)) {
-            result = getFileFromxNF(remoteFile, localFile, ftps, fileServerData);
-
-            closeDownConnection(ftps);
+        if (setUpKeyManager(ftps) && setUpTrustedCA(ftps) && setUpConnection(ftps)) {
+            if (getFileFromxNF(ftps)) {
+                closeDownConnection(ftps);
+                fileCollectResult = new FileCollectResult();
+            } else {
+                fileCollectResult = new FileCollectResult(errorData);
+            }
         } else {
-            result = false;
+            fileCollectResult = new FileCollectResult(errorData);
         }
-        logger.trace("collectFile left with result: {}", result);
-        return result;
+        logger.trace("retryCollectFile left with result: {}", fileCollectResult);
+        return fileCollectResult;
     }
 
     private boolean setUpKeyManager(IFTPSClient ftps) {
@@ -88,7 +89,7 @@ public class FtpsClient {
             keyManagerUtils.setCredentials(keyCertPath, keyCertPassword);
             ftps.setKeyManager(keyManagerUtils.getClientKeyManager());
         } catch (GeneralSecurityException | IOException e) {
-            logger.error("Unable to use own key store {}", keyCertPath, e);
+            addError("Unable to use own key store " + keyCertPath, e);
             result = false;
         }
         return result;
@@ -108,13 +109,13 @@ public class FtpsClient {
             ftps.setTrustManager(tmf.getTrustManagers()[0]);
 
         } catch (Exception e) {
-            logger.error("Unable to trust xNF's CA, {}", trustedCAPath, e);
+            addError("Unable to trust xNF's CA, " + trustedCAPath, e);
             result = false;
         }
         return result;
     }
 
-    private boolean setUpConnection(FileServerData fileServerData, IFTPSClient ftps) {
+    private boolean setUpConnection(IFTPSClient ftps) {
         boolean result = true;
         try {
             ftps.connect(fileServerData.serverAddress(), fileServerData.port());
@@ -122,7 +123,7 @@ public class FtpsClient {
             boolean loginSuccesful = ftps.login(fileServerData.userId(), fileServerData.password());
             if (!loginSuccesful) {
                 ftps.logout();
-                logger.error("Unable to log in to xNF. {}", fileServerData);
+                addError("Unable to log in to xNF. " + fileServerData, null);
                 result = false;
             }
 
@@ -134,24 +135,23 @@ public class FtpsClient {
                 ftps.execPROT("P");
             } else {
                 ftps.disconnect();
-                logger.error("Unable to connect to xNF. {}", fileServerData);
+                addError("Unable to connect to xNF. " + fileServerData + "xNF reply code: " + ftps.getReplyCode(), null);
                 result = false;
             }
         } catch (Exception ex) {
-            logger.error("Unable to connect to xNF. Data: {}", fileServerData, ex);
+            addError("Unable to connect to xNF. Data: " + fileServerData, ex);
             result = false;
         }
         logger.trace("setUpConnection return value: {}", result);
         return result;
     }
 
-    private boolean getFileFromxNF(String remoteFile, String localFilePath, IFTPSClient ftps,
-            FileServerData fileServerData) {
+    private boolean getFileFromxNF(IFTPSClient ftps) {
         logger.trace("starting to getFile");
         boolean result = true;
+        IFile outfile = getFile();
         try {
-            IFile outfile = getFile();
-            outfile.setPath(localFilePath);
+            outfile.setPath(localFile);
             outfile.createNewFile();
 
             IOutputStream outputStream = getOutputStream();
@@ -160,9 +160,14 @@ public class FtpsClient {
             ftps.retrieveFile(remoteFile, output);
 
             output.close();
-            logger.debug("File {} Download Successfull from xNF", localFilePath);
+            logger.debug("File {} Download Successfull from xNF", localFile);
         } catch (IOException ex) {
-            logger.error("Unable to collect file from xNF. Data: {}", fileServerData, ex);
+            addError("Unable to collect file from xNF. Data: " + fileServerData, ex);
+            try {
+                outfile.delete();
+            } catch (Exception e) {
+                // Nothing
+            }
             result = false;
         }
         return result;
@@ -227,11 +232,11 @@ public class FtpsClient {
     }
 
     private IFile getFile() {
-        if (localFile == null) {
-            localFile = new FileWrapper();
+        if (lf == null) {
+            lf = new FileWrapper();
         }
 
-        return localFile;
+        return lf;
     }
 
     private IOutputStream getOutputStream() {
@@ -266,7 +271,7 @@ public class FtpsClient {
     }
 
     protected void setFile(IFile file) {
-        localFile = file;
+        lf = file;
     }
 
     protected void setOutputStream(IOutputStream outputStream) {
diff --git a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java
index 5bd95b16..18b28346 100644
--- a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java
+++ b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java
@@ -24,8 +24,6 @@ import com.jcraft.jsch.Session;
 import com.jcraft.jsch.SftpException;
 
 import org.apache.commons.io.FilenameUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Component;
 
 /**
@@ -35,11 +33,12 @@ import org.springframework.stereotype.Component;
  *
  */
 @Component
-public class SftpClient {
-    private static final Logger logger = LoggerFactory.getLogger(SftpClient.class);
+public class SftpClient extends FileCollectClient {
+    @Override
+    public FileCollectResult retryCollectFile() {
+        logger.trace("retryCollectFile called");
 
-    public boolean collectFile(FileServerData fileServerData, String remoteFile, String localFile) {
-        boolean result = true;
+        FileCollectResult result;
         Session session = setUpSession(fileServerData);
 
         if (session != null) {
@@ -47,20 +46,22 @@ public class SftpClient {
             if (sftpChannel != null) {
                 try {
                     sftpChannel.get(remoteFile, localFile);
+                    result = new FileCollectResult();
                     logger.debug("File {} Download Successfull from xNF", FilenameUtils.getName(localFile));
                 } catch (SftpException e) {
-                    logger.error("Unable to get file from xNF. Data: {}", fileServerData, e);
-                    result = false;
+                    addError("Unable to get file from xNF. Data: " + fileServerData, e);
+                    result = new FileCollectResult(errorData);
                 }
 
                 sftpChannel.exit();
             } else {
-                result = false;
+                result = new FileCollectResult(errorData);
             }
             session.disconnect();
         } else {
-            result = false;
+            result = new FileCollectResult(errorData);
         }
+        logger.trace("retryCollectFile left with result: {}", result);
         return result;
     }
 
@@ -74,7 +75,7 @@ public class SftpClient {
             session.setPassword(fileServerData.password());
             session.connect();
         } catch (JSchException e) {
-            logger.error("Unable to set up SFTP connection to xNF. Data: {}", fileServerData, e);
+            addError("Unable to set up SFTP connection to xNF. Data: " + fileServerData, e);
         }
         return session;
     }
@@ -87,7 +88,7 @@ public class SftpClient {
             channel.connect();
             sftpChannel = (ChannelSftp) channel;
         } catch (JSchException e) {
-            logger.error("Unable to get sftp channel to xNF. Data: {}", fileServerData, e);
+            addError("Unable to get sftp channel to xNF. Data: " + fileServerData, e);
         }
         return sftpChannel;
     }
diff --git a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/FileWrapper.java b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/FileWrapper.java
index f8c02f09..32b6c72f 100644
--- a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/FileWrapper.java
+++ b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/FileWrapper.java
@@ -41,4 +41,9 @@ public class FileWrapper implements IFile {
     public File getFile() {
         return file;
     }
+
+    @Override
+    public boolean delete() {
+        return file.delete();
+    }
 }
diff --git a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/IFile.java b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/IFile.java
index 47d868a0..a7094f69 100644
--- a/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/IFile.java
+++ b/datafile-dmaap-client/src/main/java/org/onap/dcaegen2/collectors/datafile/io/IFile.java
@@ -27,4 +27,6 @@ public interface IFile {
     public boolean createNewFile() throws IOException;
 
     public File getFile();
+
+    public boolean delete();
 }
diff --git a/datafile-dmaap-client/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java b/datafile-dmaap-client/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java
index 5d716a9b..e9e00c38 100644
--- a/datafile-dmaap-client/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java
+++ b/datafile-dmaap-client/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java
@@ -106,9 +106,9 @@ public class FtpsClientTest {
         ImmutableFileServerData fileServerData = ImmutableFileServerData.builder().serverAddress(XNF_ADDRESS)
                 .userId(USERNAME).password(PASSWORD).port(PORT).build();
 
-        boolean result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
+        FileCollectResult result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
 
-        assertTrue(result);
+        assertTrue(result.downloadSuccessful());
         verify(ftpsClientMock).setNeedClientAuth(true);
         verify(keyManagerUtilsMock).setCredentials(FTP_KEY_PATH, FTP_KEY_PASSWORD);
         verify(ftpsClientMock).setKeyManager(keyManagerMock);
@@ -140,9 +140,9 @@ public class FtpsClientTest {
         ImmutableFileServerData fileServerData = ImmutableFileServerData.builder().serverAddress(XNF_ADDRESS)
                 .userId(USERNAME).password(PASSWORD).port(PORT).build();
 
-        boolean result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
+        FileCollectResult result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
 
-        assertFalse(result);
+        assertFalse(result.downloadSuccessful());
     }
 
     @Test
@@ -156,9 +156,9 @@ public class FtpsClientTest {
         ImmutableFileServerData fileServerData = ImmutableFileServerData.builder().serverAddress(XNF_ADDRESS)
                 .userId(USERNAME).password(PASSWORD).port(PORT).build();
 
-        boolean result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
+        FileCollectResult result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
 
-        assertFalse(result);
+        assertFalse(result.downloadSuccessful());
     }
 
     @Test
@@ -172,10 +172,10 @@ public class FtpsClientTest {
         ImmutableFileServerData fileServerData = ImmutableFileServerData.builder().serverAddress(XNF_ADDRESS)
                 .userId(USERNAME).password(PASSWORD).port(PORT).build();
 
-        boolean result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
+        FileCollectResult result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
 
         verify(ftpsClientMock, times(1)).logout();
-        assertFalse(result);
+        assertFalse(result.downloadSuccessful());
     }
 
     @Test
@@ -190,10 +190,10 @@ public class FtpsClientTest {
         ImmutableFileServerData fileServerData = ImmutableFileServerData.builder().serverAddress(XNF_ADDRESS)
                 .userId(USERNAME).password(PASSWORD).port(PORT).build();
 
-        boolean result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
+        FileCollectResult result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
 
         verify(ftpsClientMock, times(1)).disconnect();
-        assertFalse(result);
+        assertFalse(result.downloadSuccessful());
     }
 
     @Test
@@ -208,9 +208,9 @@ public class FtpsClientTest {
         ImmutableFileServerData fileServerData = ImmutableFileServerData.builder().serverAddress(XNF_ADDRESS)
                 .userId(USERNAME).password(PASSWORD).port(PORT).build();
 
-        boolean result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
+        FileCollectResult result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
 
-        assertFalse(result);
+        assertFalse(result.downloadSuccessful());
     }
 
     @Test
@@ -227,8 +227,9 @@ public class FtpsClientTest {
         ImmutableFileServerData fileServerData = ImmutableFileServerData.builder().serverAddress(XNF_ADDRESS)
                 .userId(USERNAME).password(PASSWORD).port(PORT).build();
 
-        boolean result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
+        FileCollectResult result = clientUnderTest.collectFile(fileServerData, REMOTE_FILE_PATH, LOCAL_FILE_PATH);
 
-        assertFalse(result);
+        assertFalse(result.downloadSuccessful());
+        verify(localFileMock, times(1)).delete();
     }
 }
\ No newline at end of file