Merge pull request #1737 from smartdevicelink/bugfix/issue_1736_multi_file_upload

Fix file operations check file system at queue time instead of run time

Author: Robert Henigan

android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/file/FileManagerTests.java

@@ -850,6 +850,42 @@ public void run() {
         });
     }
 
+    /**
+     * Tests to make sure files are not being uploaded to head unit multiple times in a row
+     */
+    @Test
+    public void testFileNotOnHmi() {
+        final ISdl internalInterface = createISdlMock();
+
+        doAnswer(onListFilesSuccess).when(internalInterface).sendRPC(any(ListFiles.class));
+        doAnswer(onPutFileSuccess).when(internalInterface).sendRPC(any(PutFile.class));
+
+        final SdlArtwork validFile2 = new SdlArtwork(TestValues.GENERAL_STRING + "2", FileType.GRAPHIC_JPEG, TestValues.GENERAL_STRING.getBytes(), false);
+
+        final List<SdlArtwork> list = Arrays.asList(validFile2, validFile2);
+
+        FileManagerConfig fileManagerConfig = new FileManagerConfig();
+
+        final FileManager fileManager = new FileManager(internalInterface, mTestContext, fileManagerConfig);
+        fileManager.start(new CompletionListener() {
+            @Override
+            public void onComplete(boolean success) {
+                fileManager.uploadArtworks(list, new MultipleFileCompletionListener() {
+                    @Override
+                    public void onComplete(final Map<String, String> errors) {
+                        assertOnMainThread(new Runnable() {
+                            @Override
+                            public void run() {
+                                verify(internalInterface, times(1)).sendRPC(any(PutFile.class));
+                            }
+                        });
+                    }
+                });
+            }
+        });
+    }
+
+
     /**
      * Test custom overridden SdlFile equals method
      */

android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/TextAndGraphicManagerTests.java

@@ -275,6 +275,7 @@ public void testDispose() {
 
     @Test
     public void testOperationManagement() {
+        textAndGraphicManager.transactionQueue.pause();
         textAndGraphicManager.isDirty = true;
         textAndGraphicManager.updateOperation = null;
         textAndGraphicManager.update(null);

android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java

@@ -40,7 +40,6 @@
 import com.smartdevicelink.proxy.rpc.enums.FileType;
 import com.smartdevicelink.proxy.rpc.enums.ImageType;
 import com.smartdevicelink.proxy.rpc.enums.StaticIconName;
-import com.smartdevicelink.util.DebugTool;
 
 /**
  * A class that extends SdlFile, representing artwork (JPEG, PNG, or BMP) to be uploaded to core
@@ -159,16 +158,10 @@ private Image createImageRPC() {
      */
     @Override
     public SdlArtwork clone() {
-        try {
-            SdlArtwork artwork = (SdlArtwork) super.clone();
-            if (artwork != null) {
-                artwork.imageRPC = artwork.createImageRPC();
-            }
+        SdlArtwork artwork = (SdlArtwork) super.clone();
+        if (artwork != null) {
+            artwork.imageRPC = artwork.createImageRPC();
             return artwork;
-        } catch (CloneNotSupportedException e) {
-            if (DebugTool.isDebugEnabled()) {
-                throw new RuntimeException("Clone not supported by super class");
-            }
         }
         return null;
     }

android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java

@@ -38,6 +38,7 @@
 
 import com.smartdevicelink.proxy.rpc.enums.FileType;
 import com.smartdevicelink.proxy.rpc.enums.StaticIconName;
+import com.smartdevicelink.util.DebugTool;
 
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
@@ -46,7 +47,7 @@
 /**
  * A class representing data to be uploaded to core
  */
-public class SdlFile {
+public class SdlFile implements Cloneable {
     private String fileName;
     private int id = -1;
     private Uri uri;
@@ -368,4 +369,22 @@ public boolean equals(Object o) {
         // return comparison
         return hashCode() == o.hashCode();
     }
+
+    /**
+     * Creates a deep copy of the object
+     *
+     * @return deep copy of the object, null if an exception occurred
+     */
+    @Override
+    public SdlFile clone() {
+        try {
+            SdlFile fileClone = (SdlFile) super.clone();
+            return fileClone;
+        } catch (CloneNotSupportedException e) {
+            if (DebugTool.isDebugEnabled()) {
+                throw new RuntimeException("Clone not supported by super class");
+            }
+        }
+        return null;
+    }
 }

base/src/main/java/com/smartdevicelink/managers/file/BaseFileManager.java

@@ -70,6 +70,8 @@ abstract class BaseFileManager extends BaseSubManager {
     private HashMap<String, Integer> failedFileUploadsCount;
     private final int maxFileUploadAttempts;
     private final int maxArtworkUploadAttempts;
+    final String fileManagerCannotOverwriteError = "Cannot overwrite remote file. The remote file system already has a file of this name, and the file manager is set to not automatically overwrite files.";
+
 
     /**
      * Constructor for BaseFileManager
@@ -194,13 +196,7 @@ public void onComplete(boolean success, int bytesAvailable, Collection<String> f
     }
 
     private void deleteRemoteFileWithNamePrivate(@NonNull final String fileName, final FileManagerCompletionListener listener) {
-        if (!mutableRemoteFileNames.contains(fileName) && listener != null) {
-            String errorMessage = "No such remote file is currently known";
-            listener.onComplete(false, bytesAvailable, mutableRemoteFileNames, errorMessage);
-            return;
-        }
-
-        DeleteFileOperation operation = new DeleteFileOperation(internalInterface, fileName, new FileManagerCompletionListener() {
+        DeleteFileOperation operation = new DeleteFileOperation(internalInterface, fileName, mutableRemoteFileNames, new FileManagerCompletionListener() {
             @Override
             public void onComplete(boolean success, int bytesAvailable, Collection<String> fileNames, String errorMessage) {
                 if (success) {
@@ -405,46 +401,29 @@ private void uploadFilePrivate(@NonNull final SdlFile file, final FileManagerCom
             return;
         }
 
-        // HAX: [#827](https://github.com/smartdevicelink/sdl_ios/issues/827) Older versions of Core
-        // had a bug where list files would cache incorrectly. This led to attempted uploads failing
-        // due to the system thinking they were already there when they were not. This is only needed
-        // if connecting to Core v4.3.1 or less which corresponds to RPC v4.3.1 or less
-        Version rpcVersion = new Version(internalInterface.getSdlMsgVersion());
-        if (!file.isPersistent() && !hasUploadedFile(file) && new Version(4, 4, 0).isNewerThan(rpcVersion) == 1) {
-            file.setOverwrite(true);
-        }
-
-        // Check our overwrite settings and error out if it would overwrite
-        if (!file.getOverwrite() && mutableRemoteFileNames.contains(file.getName())) {
-            String errorMessage = "Cannot overwrite remote file. The remote file system already has a file of this name, and the file manager is set to not automatically overwrite files.";
-            DebugTool.logWarning(TAG, errorMessage);
-            if (listener != null) {
-                listener.onComplete(true, bytesAvailable, null, errorMessage);
-            }
-            return;
-        }
-
         // If we didn't error out over the overwrite, then continue on
         sdl_uploadFilePrivate(file, listener);
     }
 
     private void sdl_uploadFilePrivate(@NonNull final SdlFile file, final FileManagerCompletionListener listener) {
-        final String fileName = file.getName();
+        final SdlFile fileClone = file.clone();
 
-        SdlFileWrapper fileWrapper = new SdlFileWrapper(file, new FileManagerCompletionListener() {
+        SdlFileWrapper fileWrapper = new SdlFileWrapper(fileClone, new FileManagerCompletionListener() {
             @Override
             public void onComplete(boolean success, int bytesAvailable, Collection<String> fileNames, String errorMessage) {
                 if (success) {
                     BaseFileManager.this.bytesAvailable = bytesAvailable;
-                    BaseFileManager.this.mutableRemoteFileNames.add(fileName);
-                    BaseFileManager.this.uploadedEphemeralFileNames.add(fileName);
+                    BaseFileManager.this.mutableRemoteFileNames.add(fileClone.getName());
+                    if (!file.isPersistent()) {
+                        BaseFileManager.this.uploadedEphemeralFileNames.add(fileClone.getName());
+                    }
                 } else {
-                    incrementFailedUploadCountForFileName(file.getName(), BaseFileManager.this.failedFileUploadsCount);
+                    incrementFailedUploadCountForFileName(fileClone.getName(), BaseFileManager.this.failedFileUploadsCount);
 
-                    int maxUploadCount = file instanceof SdlArtwork ? maxArtworkUploadAttempts : maxFileUploadAttempts;
-                    if (canFileBeUploadedAgain(file, maxUploadCount, failedFileUploadsCount)) {
+                    int maxUploadCount = fileClone instanceof SdlArtwork ? maxArtworkUploadAttempts : maxFileUploadAttempts;
+                    if (canFileBeUploadedAgain(fileClone, maxUploadCount, failedFileUploadsCount)) {
                         DebugTool.logInfo(TAG, String.format("Attempting to resend file with name %s after a failed upload attempt", file.getName()));
-                        sdl_uploadFilePrivate(file, listener);
+                        sdl_uploadFilePrivate(fileClone, listener);
                         return;
                     }
                 }

base/src/main/java/com/smartdevicelink/managers/file/DeleteFileOperation.java

@@ -38,8 +38,10 @@
 import com.smartdevicelink.proxy.rpc.DeleteFile;
 import com.smartdevicelink.proxy.rpc.DeleteFileResponse;
 import com.smartdevicelink.proxy.rpc.listeners.OnRPCResponseListener;
+import com.smartdevicelink.util.DebugTool;
 
 import java.lang.ref.WeakReference;
+import java.util.Set;
 
 /**
  * Created by Bilal Alsharifi on 12/1/20.
@@ -49,12 +51,14 @@ class DeleteFileOperation extends Task {
     private final WeakReference<ISdl> internalInterface;
     private String fileName;
     private FileManagerCompletionListener completionListener;
+    private Set<String> mutableRemoteFileNames;
 
-    DeleteFileOperation(ISdl internalInterface, String fileName, FileManagerCompletionListener completionListener) {
+    DeleteFileOperation(ISdl internalInterface, String fileName, Set<String> mutableRemoteFileNames, FileManagerCompletionListener completionListener) {
         super("DeleteFileOperation");
         this.internalInterface = new WeakReference<>(internalInterface);
         this.fileName = fileName;
         this.completionListener = completionListener;
+        this.mutableRemoteFileNames = mutableRemoteFileNames;
     }
 
     @Override
@@ -66,6 +70,15 @@ private void start() {
         if (getState() == Task.CANCELED) {
             return;
         }
+        if (!mutableRemoteFileNames.contains(fileName)) {
+            if (completionListener != null) {
+                String errorMessage = "File to delete is no longer on the head unit, aborting operation";
+                // Returning BaseFileManager.SPACE_AVAILABLE_MAX_VALUE for bytesAvaialble as a placeHolder, it will not get updated in BaseFileManager as long as success returned is false.
+                completionListener.onComplete(false, BaseFileManager.SPACE_AVAILABLE_MAX_VALUE, mutableRemoteFileNames, errorMessage);
+            }
+            onFinished();
+            return;
+        }
 
         deleteFile();
     }
@@ -77,12 +90,15 @@ private void deleteFile() {
             public void onResponse(int correlationId, RPCResponse response) {
                 DeleteFileResponse deleteFileResponse = (DeleteFileResponse) response;
                 boolean success = deleteFileResponse.getSuccess();
+                String errorMessage = success ? null : response.getInfo() + ": " + response.getResultCode();
+                if (errorMessage != null) {
+                    DebugTool.logInfo(TAG, "Error deleting file: " + errorMessage);
+                }
 
                 // If spaceAvailable is null, set it to the max value
                 int bytesAvailable = deleteFileResponse.getSpaceAvailable() != null ? deleteFileResponse.getSpaceAvailable() : BaseFileManager.SPACE_AVAILABLE_MAX_VALUE;
 
                 if (completionListener != null) {
-                    String errorMessage = success ? null : response.getInfo() + ": " + response.getResultCode();
                     completionListener.onComplete(success, bytesAvailable, null, errorMessage);
                 }
 

base/src/main/java/com/smartdevicelink/managers/file/UploadFileOperation.java

@@ -44,6 +44,7 @@
 import com.smartdevicelink.proxy.rpc.PutFileResponse;
 import com.smartdevicelink.proxy.rpc.listeners.OnRPCResponseListener;
 import com.smartdevicelink.util.DebugTool;
+import com.smartdevicelink.util.Version;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -81,6 +82,27 @@ private void start() {
             return;
         }
 
+        SdlFile file = fileWrapper.getFile();
+        // HAX: [#827](https://github.com/smartdevicelink/sdl_ios/issues/827) Older versions of Core
+        // had a bug where list files would cache incorrectly. This led to attempted uploads failing
+        // due to the system thinking they were already there when they were not. This is only needed
+        // if connecting to Core v4.3.1 or less which corresponds to RPC v4.3.1 or less
+        if (internalInterface.get() != null && fileManager.get() != null) {
+            Version rpcVersion = new Version(internalInterface.get().getSdlMsgVersion());
+            if (!file.isPersistent() && !fileManager.get().hasUploadedFile(file) && new Version(4, 4, 0).isNewerThan(rpcVersion) == 1) {
+                file.setOverwrite(true);
+            }
+            // Check our overwrite settings and error out if it would overwrite
+            if (!file.getOverwrite() && fileManager.get().mutableRemoteFileNames.contains(file.getName())) {
+                DebugTool.logWarning(TAG, fileManager.get().fileManagerCannotOverwriteError);
+                if (this.fileWrapper.getCompletionListener() != null) {
+                    this.fileWrapper.getCompletionListener().onComplete(true, bytesAvailable, null, fileManager.get().fileManagerCannotOverwriteError);
+                }
+                onFinished();
+                return;
+            }
+        }
+
         int mtuSize = 0;
         if (internalInterface.get() != null) {
             mtuSize = (int) internalInterface.get().getMtu(SessionType.RPC);

javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java

@@ -37,7 +37,6 @@
 import com.smartdevicelink.proxy.rpc.enums.FileType;
 import com.smartdevicelink.proxy.rpc.enums.ImageType;
 import com.smartdevicelink.proxy.rpc.enums.StaticIconName;
-import com.smartdevicelink.util.DebugTool;
 
 import java.net.URI;
 
@@ -159,16 +158,10 @@ private Image createImageRPC() {
      */
     @Override
     public SdlArtwork clone() {
-        try {
-            SdlArtwork artwork = (SdlArtwork) super.clone();
-            if (artwork != null) {
-                artwork.imageRPC = artwork.createImageRPC();
-            }
+        SdlArtwork artwork = (SdlArtwork) super.clone();
+        if (artwork != null) {
+            artwork.imageRPC = artwork.createImageRPC();
             return artwork;
-        } catch (CloneNotSupportedException e) {
-            if (DebugTool.isDebugEnabled()) {
-                throw new RuntimeException("Clone not supported by super class");
-            }
         }
         return null;
     }

javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java

@@ -35,6 +35,7 @@
 
 import com.smartdevicelink.proxy.rpc.enums.FileType;
 import com.smartdevicelink.proxy.rpc.enums.StaticIconName;
+import com.smartdevicelink.util.DebugTool;
 
 import java.net.URI;
 import java.security.MessageDigest;
@@ -44,7 +45,7 @@
 /**
  * A class representing data to be uploaded to core
  */
-public class SdlFile {
+public class SdlFile implements Cloneable {
     private String fileName;
     private String filePath;
     private URI uri;
@@ -366,4 +367,22 @@ public boolean equals(Object o) {
         // return comparison
         return hashCode() == o.hashCode();
     }
+
+    /**
+     * Creates a deep copy of the object
+     *
+     * @return deep copy of the object, null if an exception occurred
+     */
+    @Override
+    public SdlFile clone() {
+        try {
+            SdlFile fileClone = (SdlFile) super.clone();
+            return fileClone;
+        } catch (CloneNotSupportedException e) {
+            if (DebugTool.isDebugEnabled()) {
+                throw new RuntimeException("Clone not supported by super class");
+            }
+        }
+        return null;
+    }
 }