Skip to content

Commit 79aba1b

Browse files
committed
Fix case where file can have multiple upload/delete tasks created for it
1 parent e313f5e commit 79aba1b

4 files changed

Lines changed: 56 additions & 129 deletions

File tree

lib/js/src/manager/file/_DeleteFileOperation.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,14 @@ class _DeleteFileOperation extends _Task {
4040
* @class
4141
* @param {_LifecycleManager} lifecycleManager - An instance of _LifecycleManager.
4242
* @param {String} fileName - Name of the file to be deleted.
43+
* @param {String[]} remoteFileNames - A list of file names uploaded to the system
4344
* @param {Function} completionListener - Listener called when the operation has completed.
4445
*/
45-
constructor (lifecycleManager, fileName, completionListener) {
46+
constructor (lifecycleManager, fileName, remoteFileNames = [], completionListener) {
4647
super('DeleteFilesOperation');
4748
this._lifecycleManager = lifecycleManager;
4849
this._fileName = fileName;
50+
this._remoteFileNames = remoteFileNames;
4951
this._completionListener = completionListener;
5052
}
5153

@@ -66,6 +68,13 @@ class _DeleteFileOperation extends _Task {
6668
if (this.getState() === _Task.CANCELED) {
6769
return;
6870
}
71+
72+
if (!this._remoteFileNames.includes(this._fileName)) {
73+
const errorMessage = 'File to delete is no longer on the head unit, aborting the operation';
74+
console.log(errorMessage);
75+
this._completionListener(false, null, null, errorMessage);
76+
return this.onFinished();
77+
}
6978
await this._deleteFile();
7079
}
7180

@@ -96,6 +105,10 @@ class _DeleteFileOperation extends _Task {
96105
getName () {
97106
return `${super.getName()} - ${this.getId()}`;
98107
}
108+
109+
setRemoteFiles (remoteFiles) {
110+
this._remoteFileNames = remoteFiles;
111+
}
99112
}
100113

101114
export { _DeleteFileOperation };

lib/js/src/manager/file/_FileManagerBase.js

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,12 @@ class _FileManagerBase extends _SubManagerBase {
146146
return;
147147
}
148148

149-
const operation = new _DeleteFileOperation(this._lifecycleManager, fileName, (success, bytesAvailable, fileNames, errorMessage) => {
149+
const operation = new _DeleteFileOperation(this._lifecycleManager, fileName, this._remoteFiles, (success, bytesAvailable, fileNames, errorMessage) => {
150150
if (success) {
151151
this._bytesAvailable = bytesAvailable;
152152
this._remoteFiles = _ArrayTools.arrayRemove(this._remoteFiles, fileName);
153153
this._uploadedEphemeralFileNames = _ArrayTools.arrayRemove(this._uploadedEphemeralFileNames, fileName);
154+
this._updatePendingOperationsWithNewRemoteFiles();
154155
}
155156
listener(success, bytesAvailable, fileNames, errorMessage);
156157
});
@@ -170,6 +171,8 @@ class _FileManagerBase extends _SubManagerBase {
170171
this._deleteRemoteFileWithNamePrivate(fileName, (success, bytesAvailable, fileNames, errorMessage) => {
171172
if (!success) {
172173
failedDeletes.set(fileName, errorMessage);
174+
} else {
175+
this._bytesAvailable = bytesAvailable;
173176
}
174177
resolve();
175178
});
@@ -240,15 +243,6 @@ class _FileManagerBase extends _SubManagerBase {
240243
file.setOverwrite(true);
241244
}
242245

243-
if (!file.getOverwrite() && this._remoteFiles.includes(file.getName())) {
244-
const 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.';
245-
console.log(`FileManagerBase: ${errorMessage}`);
246-
if (typeof listener === 'function') {
247-
listener(false, this._bytesAvailable, null, errorMessage);
248-
}
249-
return;
250-
}
251-
252246
this._sdlUploadFilePrivate(file, listener);
253247
}
254248

@@ -264,8 +258,11 @@ class _FileManagerBase extends _SubManagerBase {
264258
if (success) {
265259
this._bytesAvailable = bytesAvailable;
266260
this._remoteFiles.push(fileName);
267-
this._uploadedEphemeralFileNames.push(fileName);
268-
} else {
261+
if (!file.isPersistent()) {
262+
this._uploadedEphemeralFileNames.push(fileName);
263+
}
264+
this._updatePendingOperationsWithNewRemoteFiles();
265+
} else if (errorMessage !== 'File is already on the head unit, aborting upload operation') {
269266
this._incrementFailedUploadCountForFileName(fileName, this._failedFileUploadsCount);
270267

271268
const maxUploadCount = (file instanceof SdlArtwork) ? this._maxArtworkUploadAttempts : this._maxFileUploadAttempts;
@@ -281,7 +278,7 @@ class _FileManagerBase extends _SubManagerBase {
281278
}
282279
});
283280

284-
const operation = new _UploadFileOperation(this._lifecycleManager, this, fileWrapper);
281+
const operation = new _UploadFileOperation(this._lifecycleManager, this, fileWrapper, this._remoteFiles);
285282
this._addTask(operation);
286283
}
287284

@@ -419,6 +416,17 @@ class _FileManagerBase extends _SubManagerBase {
419416
}
420417
return false;
421418
}
419+
420+
_updatePendingOperationsWithNewRemoteFiles () {
421+
for (const op of this._getTasks()) {
422+
if (op.getState() === _Task.IN_PROGRESS || op.getState() === _Task.CANCELED) {
423+
continue;
424+
}
425+
if (op instanceof _UploadFileOperation || op instanceof _DeleteFileOperation) {
426+
op.setRemoteFiles(this._remoteFiles);
427+
}
428+
}
429+
}
422430
}
423431

424432
const SPACE_AVAILABLE_MAX_VALUE = _FileManagerBase.SPACE_AVAILABLE_MAX_VALUE = 2000000000;

lib/js/src/manager/file/_UploadFileOperation.js

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ class _UploadFileOperation extends _Task {
1212
* @param {_LifecycleManager} lifecycleManager - An instance of _LifecycleManager.
1313
* @param {FileManager} fileManager - An instance of FileManager.
1414
* @param {_SdlFileWrapper} fileWrapper - An instance of _SdlFileWrapper.
15+
* @param {String[]} remoteFileNames - A list of file names uploaded to the system.
1516
*/
16-
constructor (lifecycleManager = null, fileManager = null, fileWrapper = null) {
17+
constructor (lifecycleManager = null, fileManager = null, fileWrapper = null, remoteFileNames = []) {
1718
super('UploadFileOperation');
1819
this._lifecycleManager = lifecycleManager;
1920
this._fileManager = fileManager;
2021
this._fileWrapper = fileWrapper;
22+
this._remoteFileNames = remoteFileNames;
2123
}
2224

2325
/**
@@ -38,6 +40,13 @@ class _UploadFileOperation extends _Task {
3840
return;
3941
}
4042

43+
if (!this._fileWrapper.getFile().getOverwrite() && this._remoteFileNames.includes(this._fileWrapper.getFile().getName())) {
44+
const errorMessage = 'File is already on the head unit, aborting upload operation';
45+
console.log(errorMessage);
46+
this._fileWrapper.getCompletionListener(false, null, null, errorMessage);
47+
return this.onFinished();
48+
}
49+
4150
let mtuSize = 0;
4251
if (this._lifecycleManager !== null) {
4352
mtuSize = this._lifecycleManager.getMtu(_ServiceType.RPC);
@@ -56,25 +65,16 @@ class _UploadFileOperation extends _Task {
5665
let bytesAvailable = _FileManagerBase.SPACE_AVAILABLE_MAX_VALUE;
5766
let highestCorrelationId = -1;
5867

59-
if (this.getState() === _Task.CANCELED) {
60-
const errorMessage = 'The file upload transaction was canceled before it could be completed.';
61-
completionListener(false, bytesAvailable, null, errorMessage);
62-
this.onFinished();
63-
return;
64-
}
65-
6668
if (file === null) {
6769
const errorMessage = 'The file manager was unable to send the file. This could be because the file does not exist at the specified file path or that passed data is invalid.';
6870
completionListener(false, bytesAvailable, null, errorMessage);
69-
this.onFinished();
70-
return;
71+
return this.onFinished();
7172
}
7273

7374
if (file.getName() === null || file.getName() === undefined) {
7475
const errorMessage = 'You must specify a file name in the SdlFile';
7576
completionListener(false, bytesAvailable, null, errorMessage);
76-
this.onFinished();
77-
return;
77+
return this.onFinished();
7878
}
7979

8080
let data;
@@ -84,16 +84,14 @@ class _UploadFileOperation extends _Task {
8484
if (data === null) {
8585
const errorMessage = 'File at path was empty';
8686
completionListener(false, bytesAvailable, null, errorMessage);
87-
this.onFinished();
88-
return;
87+
return this.onFinished();
8988
}
9089
} else if (file.getFileData() !== null && file.getFileData() !== undefined) {
9190
data = file.getFileData();
9291
} else {
9392
const errorMessage = 'The SdlFile to upload does not specify its file data.';
9493
completionListener(false, bytesAvailable, null, errorMessage);
95-
this.onFinished();
96-
return;
94+
return this.onFinished();
9795
}
9896
const fileSize = data.length;
9997

@@ -251,6 +249,10 @@ class _UploadFileOperation extends _Task {
251249
const maxJSONSize = this._getMaxJSONSize(file, fileSize);
252250
return mtuSize - (frameHeaderSize + binaryHeaderSize + maxJSONSize);
253251
}
252+
253+
setRemoteFiles (remoteFiles) {
254+
this._remoteFileNames = remoteFiles;
255+
}
254256
}
255257

256258
export { _UploadFileOperation };

tests/managers/file/FileManagerTests.js

Lines changed: 3 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ module.exports = function (appClient) {
9595
async function checkForUploadFailure (fileManager, sdlFile) {
9696
const operation = new SDL.manager.file._UploadFileOperation(sdlManager._lifecycleManager, sdlManager.getFileManager(), new SDL.manager.file._SdlFileWrapper(sdlFile, (success, bytesAvailable, fileNames, errorMessage) => {
9797
Validator.assertTrue(!success);
98-
}));
98+
}), sdlManager._fileManager.getRemoteFileNames());
9999
await operation.onExecute();
100100
}
101101

@@ -121,37 +121,6 @@ module.exports = function (appClient) {
121121
});
122122
}
123123

124-
let failEveryOtherRequest = true;
125-
/**
126-
* Handle multiple PutFile RPCs, succeeding in one and failing in the next.
127-
* @returns {Promise} - A promise.
128-
*/
129-
function onMultiplePutFileSuccess () {
130-
const response = new SDL.rpc.messages.PutFileResponse({
131-
functionName: SDL.rpc.enums.FunctionID.PutFiles,
132-
})
133-
.setSpaceAvailable(Test.GENERAL_INT);
134-
if (failEveryOtherRequest) {
135-
failEveryOtherRequest = !failEveryOtherRequest;
136-
response.setSuccess(true);
137-
// _handleRpc triggers the listener
138-
sdlManager._lifecycleManager._handleRpc(response);
139-
140-
return new Promise((resolve, reject) => {
141-
resolve(response);
142-
});
143-
} else {
144-
failEveryOtherRequest = !failEveryOtherRequest;
145-
response.setSuccess(false);
146-
// _handleRpc triggers the listener
147-
sdlManager._lifecycleManager._handleRpc(response);
148-
149-
return new Promise((resolve, reject) => {
150-
resolve(response);
151-
});
152-
}
153-
}
154-
155124
/**
156125
* Handle DeleteFile success.
157126
* @returns {Promise} - A promise.
@@ -411,12 +380,11 @@ module.exports = function (appClient) {
411380

412381
Validator.assertNull(await fileManager.deleteRemoteFilesWithNames([uploadedFileNames[0], uploadedFileNames[1]]));
413382
await new Promise (innerResolve => {
414-
const deleteOperation = new SDL.manager.file._DeleteFileOperation(sdlManager._lifecycleManager, uploadedFileNames[2], (success, bytesAvailable, fileNames, errorMessage) => {
383+
const deleteOperation = new SDL.manager.file._DeleteFileOperation(sdlManager._lifecycleManager, uploadedFileNames[2], [uploadedFileNames[2]], (success, bytesAvailable, fileNames, errorMessage) => {
415384
Validator.assertTrue(success);
416385
Validator.assertNull(errorMessage);
417386
innerResolve();
418387
});
419-
// await deleteOperation.onExecute();
420388
fileManager._addTask(deleteOperation);
421389
});
422390

@@ -426,70 +394,6 @@ module.exports = function (appClient) {
426394
stub.restore();
427395
});
428396

429-
it('testMultipleFileUploadPartialFailure', async function () {
430-
let firstFile = true;
431-
const expectSuccessThenFail = function (response) {
432-
if (firstFile) {
433-
Validator.assertTrue(response.getSuccess());
434-
} else {
435-
Validator.assertTrue(!response.getSuccess());
436-
}
437-
firstFile = !firstFile;
438-
};
439-
const expectSuccess = function (response) {
440-
Validator.assertTrue(response.getSuccess());
441-
};
442-
443-
sdlManager.addRpcListener(SDL.rpc.enums.FunctionID.ListFiles, expectSuccess);
444-
sdlManager.addRpcListener(SDL.rpc.enums.FunctionID.PutFile, expectSuccessThenFail);
445-
446-
const stub = sinon.stub(lifecycleManager, 'sendRpcResolve');
447-
stub.withArgs(sinon.match.instanceOf(SDL.rpc.messages.ListFiles))
448-
.callsFake(onListFilesSuccess);
449-
stub.withArgs(sinon.match.instanceOf(SDL.rpc.messages.PutFile))
450-
.callsFake(onMultiplePutFileSuccess);
451-
452-
await new Promise (resolve => {
453-
const listOperation = new SDL.manager.file._ListFilesOperation(sdlManager._lifecycleManager, (success, bytesAvailable, fileNames, errorMessage) => {
454-
resolve();
455-
});
456-
listOperation.onExecute();
457-
});
458-
459-
const baseFileName = 'file';
460-
let fileNum = 0;
461-
const filesToUpload = [];
462-
let sdlFile = new SDL.manager.file.filetypes.SdlFile();
463-
sdlFile.setName(baseFileName + fileNum++);
464-
sdlFile.setFilePath('./test_icon_1.png');
465-
filesToUpload.push(sdlFile);
466-
467-
sdlFile = new SDL.manager.file.filetypes.SdlFile();
468-
sdlFile.setName(baseFileName + fileNum++);
469-
sdlFile.setFileData(new Array(50));
470-
sdlFile.setPersistent(true);
471-
sdlFile.setType(SDL.rpc.enums.FileType.BINARY);
472-
filesToUpload.push(sdlFile);
473-
let uploadedFileNames = fileManager.getRemoteFileNames();
474-
475-
await fileManager.uploadFiles(filesToUpload[0]);
476-
await new Promise (resolve => {
477-
// this task will run after the previous
478-
const uploadOperation = new SDL.manager.file._UploadFileOperation(sdlManager._lifecycleManager, sdlManager._fileManager, new SDL.manager.file._SdlFileWrapper(filesToUpload[1], (success, bytesAvailable, fileNames, errorMessage) => {
479-
resolve();
480-
}));
481-
sdlManager._fileManager._addTask(uploadOperation);
482-
});
483-
stub.restore();
484-
485-
uploadedFileNames = fileManager.getRemoteFileNames();
486-
Validator.assertTrue(!uploadedFileNames.includes(filesToUpload[0].getName()));
487-
Validator.assertTrue(uploadedFileNames.includes(filesToUpload[1].getName()));
488-
489-
sdlManager.removeRpcListener(SDL.rpc.enums.FunctionID.PutFile, expectSuccessThenFail);
490-
sdlManager.removeRpcListener(SDL.rpc.enums.FunctionID.ListFiles, expectSuccess);
491-
});
492-
493397
it('testMultipleArtworkUploadSuccess', async function () {
494398
const expectSuccess = function (response) {
495399
Validator.assertTrue(response.getSuccess());
@@ -531,7 +435,7 @@ module.exports = function (appClient) {
531435
Validator.assertTrue(success);
532436
sdlManager._fileManager._remoteFiles.push(artworkToUpload[1].getName());
533437
resolve();
534-
}));
438+
}), sdlManager._fileManager.getRemoteFileNames());
535439
sdlManager._fileManager._addTask(uploadOperation);
536440
});
537441
stub.restore();

0 commit comments

Comments
 (0)