diff options
author | Robert Henigan <robert.henigan@livio.io> | 2021-09-30 17:12:31 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-09-30 17:12:31 -0400 |
commit | aad25f0c688f5f14aaed16474cf8f370b5ae4bca (patch) | |
tree | abd4ff9d06acc7a6fabded22a33004e3c6a4d6bf | |
parent | 3243c51ce4ec3a0c765e9d56e762181d88cf8f6c (diff) | |
parent | d65d18e709031a6469689ab0bda54e458b79a2e5 (diff) | |
download | sdl_android-aad25f0c688f5f14aaed16474cf8f370b5ae4bca.tar.gz |
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
9 files changed, 136 insertions, 58 deletions
diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/file/FileManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/file/FileManagerTests.java index 90bedd434..b3d489eaa 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/file/FileManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/file/FileManagerTests.java @@ -851,6 +851,42 @@ public class FileManagerTests { } /** + * 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 */ @Test diff --git a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/TextAndGraphicManagerTests.java b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/TextAndGraphicManagerTests.java index 9b948f808..3999e7d2c 100644 --- a/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/TextAndGraphicManagerTests.java +++ b/android/sdl_android/src/androidTest/java/com/smartdevicelink/managers/screen/TextAndGraphicManagerTests.java @@ -275,6 +275,7 @@ public class TextAndGraphicManagerTests { @Test public void testOperationManagement() { + textAndGraphicManager.transactionQueue.pause(); textAndGraphicManager.isDirty = true; textAndGraphicManager.updateOperation = null; textAndGraphicManager.update(null); diff --git a/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java b/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java index c8887d4e1..b46745917 100644 --- a/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java +++ b/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java @@ -40,7 +40,6 @@ import com.smartdevicelink.proxy.rpc.Image; 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 @@ public class SdlArtwork extends SdlFile implements Cloneable { */ @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; } diff --git a/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java b/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java index 13c7d3119..a0c0d9369 100644 --- a/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java +++ b/android/sdl_android/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java @@ -38,6 +38,7 @@ import androidx.annotation.NonNull; 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 @@ import java.util.Arrays; /** * 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 class SdlFile { // 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; + } } diff --git a/base/src/main/java/com/smartdevicelink/managers/file/BaseFileManager.java b/base/src/main/java/com/smartdevicelink/managers/file/BaseFileManager.java index 9412fb122..da6bae4e6 100644 --- a/base/src/main/java/com/smartdevicelink/managers/file/BaseFileManager.java +++ b/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 @@ abstract class BaseFileManager extends BaseSubManager { } 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 @@ abstract class BaseFileManager extends BaseSubManager { 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; } } diff --git a/base/src/main/java/com/smartdevicelink/managers/file/DeleteFileOperation.java b/base/src/main/java/com/smartdevicelink/managers/file/DeleteFileOperation.java index 21df7f7c4..522c5c7e9 100644 --- a/base/src/main/java/com/smartdevicelink/managers/file/DeleteFileOperation.java +++ b/base/src/main/java/com/smartdevicelink/managers/file/DeleteFileOperation.java @@ -38,8 +38,10 @@ import com.smartdevicelink.proxy.RPCResponse; 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 @@ class DeleteFileOperation extends Task { 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 @@ class DeleteFileOperation extends Task { 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); } diff --git a/base/src/main/java/com/smartdevicelink/managers/file/UploadFileOperation.java b/base/src/main/java/com/smartdevicelink/managers/file/UploadFileOperation.java index 4a9b785c6..aab50a280 100644 --- a/base/src/main/java/com/smartdevicelink/managers/file/UploadFileOperation.java +++ b/base/src/main/java/com/smartdevicelink/managers/file/UploadFileOperation.java @@ -44,6 +44,7 @@ import com.smartdevicelink.proxy.rpc.PutFile; 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 @@ class UploadFileOperation extends Task { 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); diff --git a/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java b/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java index affa4ec36..2c31c848a 100644 --- a/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java +++ b/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlArtwork.java @@ -37,7 +37,6 @@ import com.smartdevicelink.proxy.rpc.Image; 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 @@ public class SdlArtwork extends SdlFile implements Cloneable { */ @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; } diff --git a/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java b/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java index 6fd91b8a0..727413bf2 100644 --- a/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java +++ b/javaSE/javaSE/src/main/java/com/smartdevicelink/managers/file/filetypes/SdlFile.java @@ -35,6 +35,7 @@ import androidx.annotation.NonNull; 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 @@ import java.util.Arrays; /** * 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 class SdlFile { // 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; + } } |