From 5e5d329e779dfe00cbdc093245357f536fc011a1 Mon Sep 17 00:00:00 2001 From: Joshua Chudy Date: Mon, 18 Mar 2024 17:46:46 -0700 Subject: [PATCH 1/4] update metadata request when a file exists but the new file to be uploaded has a different file name --- js/hatrac.js | 63 +++++++- test/specs/upload/tests/02.upload_obj.js | 189 +++++++++++++++++++---- 2 files changed, 225 insertions(+), 27 deletions(-) diff --git a/js/hatrac.js b/js/hatrac.js index 4a29ed87d..dd12d5a33 100644 --- a/js/hatrac.js +++ b/js/hatrac.js @@ -193,6 +193,7 @@ var ERMrest = (function(module) { })(ERMrest || {}); var ERMrest = (function(module) { + var FILENAME_REGEXP = /[^a-zA-Z0-9_.-]/ig; var allowedHttpErrors = [500, 503, 408, 401]; @@ -310,6 +311,7 @@ var ERMrest = (function(module) { this.http = this.reference._server.http; + this.updateDispositionOnly = false; this.isPaused = false; this.otherInfo = otherInfo; @@ -430,6 +432,7 @@ var ERMrest = (function(module) { var headers = module.getResponseHeader(response); var md5 = headers["content-md5"]; var length = headers["content-length"]; + var contentDisposition = headers["content-disposition"]; // If the file is not same, then simply resolve the promise without setting completed and jobDone if ((md5 != self.hash.md5_base64) || (length != self.file.size)) { @@ -437,6 +440,18 @@ var ERMrest = (function(module) { return; } + // hatrac only supports `filename*=UTF-8''` in the content disposition so check for this string to get the filename + var contentDispositionPrefix = "filename*=UTF-8''"; + var filenameIndex = contentDisposition.indexOf(contentDispositionPrefix) + contentDispositionPrefix.length; + + // check if filename in content disposition is different from filename being uploaded + // if it is, instead set a different flag and don't mark it as done + if (contentDisposition.substring(filenameIndex, contentDisposition.length) != self.file.name.replace(FILENAME_REGEXP, '_')) { + self.updateDispositionOnly = true; + deferred.resolve(self.url); + return; + } + self.isPaused = false; self.completed = true; self.jobDone = true; @@ -496,7 +511,7 @@ var ERMrest = (function(module) { "content-length": self.file.size, "content-type": self.file.type, "content-md5": self.hash.md5_base64, - "content-disposition": "filename*=UTF-8''" + self.file.name.replace(/[^a-zA-Z0-9_.-]/ig, '_') + "content-disposition": "filename*=UTF-8''" + self.file.name.replace(FILENAME_REGEXP, '_') }; if (!contextHeaderParams || !_isObject(contextHeaderParams)) { @@ -528,6 +543,52 @@ var ERMrest = (function(module) { return deferred.promise; }; + upload.prototype.createUpdateMetadataJob = function (contextHeaderParams) { + var self = this; + this.erred = false; + + var deferred = module._q.defer(); + + if (this.completed && this.jobDone) { + deferred.resolve(this.chunkUrl); + return deferred.promise; + } + + // Prepend the url with server uri if it is relative + var url = self._getAbsoluteUrl(self.url + ";metadata/content-disposition"); + + var data = "filename*=UTF-8''" + self.file.name.replace(FILENAME_REGEXP, '_'); + + if (!contextHeaderParams || !_isObject(contextHeaderParams)) { + contextHeaderParams = { + action: "upload/metadata/update", + referrer: self.reference.defaultLogInfo + }; + contextHeaderParams.referrer.column = self.column.name; + } + + var config = { + headers: _generateContextHeader(contextHeaderParams) + }; + config.headers['content-type'] = 'text/plain'; + + self.http.put(url, data, config).then(function (response) { + if (response) { + // mark as completed and job done since this is a metadata update request that doesn't transfer file data + self.isPaused = false; + self.completed = true; + self.jobDone = true; + + deferred.resolve(); + } + }, function(response) { + var error = module.responseToError(response); + deferred.reject(error); + }); + + return deferred.promise; + } + /** * @desc Call this function to start chunked upload to server. It reads the file and divides in into chunks diff --git a/test/specs/upload/tests/02.upload_obj.js b/test/specs/upload/tests/02.upload_obj.js index 30b154715..110ec1365 100644 --- a/test/specs/upload/tests/02.upload_obj.js +++ b/test/specs/upload/tests/02.upload_obj.js @@ -1,15 +1,16 @@ exports.execute = function (options) { var exec = require('child_process').execSync; - var FileAPI = require('file-api'), File = FileAPI.File; - File.prototype.jsdom = true; + var FileAPI = require('file-api'), File = FileAPI.File; + File.prototype.jsdom = true; describe("For verifying the upload object, ", function () { var schemaName = "upload", tableName = "file", columnName = "uri", chunkSize = 128000, - reference, column, filePath, uploadObj, chunkUrl; + reference, column, filePath, uploadObj, chunkUrl, + filePathDiffName, uploadObjDiffName; var serverUri = options.url.replace("/ermrest", ""); var baseUri = options.url + "/catalog/" + process.env.DEFAULT_CATALOG + "/entity/" @@ -24,6 +25,16 @@ exports.execute = function (options) { hash_64: "SxeHAOXzsVznmfLGwUZXQQ==" }; + // should be same file as + var fileDiffName = { + name: "diff_testfile500kb.png", + size: 512000, + displaySize: "500KB", + type: "image/png", + hash: "4b178700e5f3b15ce799f2c6c1465741", + hash_64: "SxeHAOXzsVznmfLGwUZXQQ==" + }; + var firstTime = Date.now(); var validRow = { timestamp: firstTime, @@ -34,15 +45,18 @@ exports.execute = function (options) { beforeAll(function (done) { filePath = "test/specs/upload/files/" + file.name; + filePathDiffName = "test/specs/upload/files/" + fileDiffName.name; exec("perl -e 'print \"\1\" x " + file.size + "' > " + filePath); + exec("cp " + filePath + " " + filePathDiffName); file.file = new File(filePath); + fileDiffName.file = new File(filePathDiffName); options.ermRest.resolve(baseUri, { cid: "test" }).then(function (response) { reference = response; - column = reference.columns.find(function(c) { return c.name == columnName; }); + column = reference.columns.find(function (c) { return c.name == columnName; }); if (!column) { console.log("Unable to find column " + columnName); @@ -91,7 +105,7 @@ exports.execute = function (options) { it("should verify the URL functions.", function (done) { expect(uploadObj.validateURL(validRow)).toBe(true); - uploadObj.calculateChecksum(validRow).then(function(url) { + uploadObj.calculateChecksum(validRow).then(function (url) { expect(url).toBe(serverFilePath, "File generated url is incorrect after calculating the checksum"); expect(validRow.filename).toBe(file.name, "Valid row name is incorrect"); @@ -110,40 +124,40 @@ exports.execute = function (options) { expect(uploadObj._getAbsoluteUrl(uploadObj.url)).toBe(serverUri + serverFilePath, "absolute url is incorrect"); done(); - }, function(err) { + }, function (err) { console.dir(err); done.fail(); }); }); it("should verify the job doesn't exist yet and neither does the file.", function (done) { - uploadObj._getExistingJobStatus().then(function(response) { + uploadObj._getExistingJobStatus().then(function (response) { expect(response).not.toBeDefined("Job status is defined"); return uploadObj.fileExists(); - }).then(function(response) { + }).then(function (response) { expect(response).toBe(serverFilePath, "Server file path is incorrect"); done(); - }).catch(function(err) { + }).catch(function (err) { console.dir(err); done.fail(); }); }); it("should verify the upload job is created.", function (done) { - uploadObj.createUploadJob().then(function(response) { + uploadObj.createUploadJob().then(function (response) { chunkUrl = response; expect(response.startsWith(serverFilePath)).toBeTruthy("Upload job file path is incorrect"); done(); - }, function(err) { + }, function (err) { console.dir(err); done.fail(); }); }); it("should verify the job now exists but the file does not yet.", function (done) { - uploadObj._getExistingJobStatus().then(function(response) { + uploadObj._getExistingJobStatus().then(function (response) { var data = response.data; expect(response).toBeDefined("Job status is not defined"); @@ -155,11 +169,11 @@ exports.execute = function (options) { expect(data["chunk-length"]).toBe(chunkSize, "Job status chunk size is incorrect"); return uploadObj.fileExists(); - }).then(function(response) { + }).then(function (response) { expect(response).toBe(serverFilePath, "Server file path is incorrect"); done(); - }).catch(function(err) { + }).catch(function (err) { console.dir(err); done.fail(); }); @@ -169,24 +183,24 @@ exports.execute = function (options) { // keeps track of each time notify is called, should be once per chunk var counter = 1; - uploadObj.start().then(function(response) { + uploadObj.start().then(function (response) { expect(response).toBe(serverFilePath, "Upload job file path is incorrect"); expect(uploadObj.isPaused).toBeFalsy("Upload job is paused"); done(); - }, function(err) { + }, function (err) { console.dir(err); done.fail(); }, function notify() { var chunks = uploadObj.chunks; // we get notified by updateProgressBar() after the chunk is PUT to the hatrac server - expect(chunks.length).toBe(file.size/chunkSize, "Upload job chunks is incorrect"); + expect(chunks.length).toBe(file.size / chunkSize, "Upload job chunks is incorrect"); // verify completed chunks var numCompleteChunks = 0; var numIncompleteChunks = 0 // the chunks get added in a random order in the array. sometimes chunk[2] is uploaded before chunk[1] - for(var i=0; i Date: Wed, 20 Mar 2024 11:12:56 -0700 Subject: [PATCH 2/4] comment for metadata function --- js/hatrac.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/js/hatrac.js b/js/hatrac.js index dd12d5a33..d4c0709e7 100644 --- a/js/hatrac.js +++ b/js/hatrac.js @@ -543,6 +543,12 @@ var ERMrest = (function(module) { return deferred.promise; }; + /** + * @desc Call this function to create an update metadata request for updating the content-disposition + * + * @returns {Promise} A promise resolved with no value or the url if it already exists + * or rejected with error + */ upload.prototype.createUpdateMetadataJob = function (contextHeaderParams) { var self = this; this.erred = false; From 57c681f0dac7daf8c553694f2d2335500c3a12e0 Mon Sep 17 00:00:00 2001 From: Joshua Chudy Date: Wed, 20 Mar 2024 18:41:22 -0700 Subject: [PATCH 3/4] move update logic to automatically happen in fileExists check instead of making chaise act on the response --- js/hatrac.js | 87 ++++++++---------------- test/specs/upload/tests/02.upload_obj.js | 14 ++-- 2 files changed, 37 insertions(+), 64 deletions(-) diff --git a/js/hatrac.js b/js/hatrac.js index d4c0709e7..de914a881 100644 --- a/js/hatrac.js +++ b/js/hatrac.js @@ -311,7 +311,6 @@ var ERMrest = (function(module) { this.http = this.reference._server.http; - this.updateDispositionOnly = false; this.isPaused = false; this.otherInfo = otherInfo; @@ -445,11 +444,37 @@ var ERMrest = (function(module) { var filenameIndex = contentDisposition.indexOf(contentDispositionPrefix) + contentDispositionPrefix.length; // check if filename in content disposition is different from filename being uploaded - // if it is, instead set a different flag and don't mark it as done + // if it is, create an update metadata request for updating the content-disposition if (contentDisposition.substring(filenameIndex, contentDisposition.length) != self.file.name.replace(FILENAME_REGEXP, '_')) { - self.updateDispositionOnly = true; - deferred.resolve(self.url); - return; + // Prepend the url with server uri if it is relative + var url = self._getAbsoluteUrl(self.url + ";metadata/content-disposition"); + + var data = "filename*=UTF-8''" + self.file.name.replace(FILENAME_REGEXP, '_'); + + if (!contextHeaderParams || !_isObject(contextHeaderParams)) { + contextHeaderParams = { + action: "upload/metadata/update", + referrer: self.reference.defaultLogInfo + }; + contextHeaderParams.referrer.column = self.column.name; + } + + var config = { + headers: _generateContextHeader(contextHeaderParams) + }; + config.headers['content-type'] = 'text/plain'; + + return self.http.put(url, data, config).then(function (response) { + // mark as completed and job done since this is a metadata update request that doesn't transfer file data + self.isPaused = false; + self.completed = true; + self.jobDone = true; + + deferred.resolve(self.url); + }, function(response) { + var error = module.responseToError(response); + deferred.reject(error); + }); } self.isPaused = false; @@ -543,58 +568,6 @@ var ERMrest = (function(module) { return deferred.promise; }; - /** - * @desc Call this function to create an update metadata request for updating the content-disposition - * - * @returns {Promise} A promise resolved with no value or the url if it already exists - * or rejected with error - */ - upload.prototype.createUpdateMetadataJob = function (contextHeaderParams) { - var self = this; - this.erred = false; - - var deferred = module._q.defer(); - - if (this.completed && this.jobDone) { - deferred.resolve(this.chunkUrl); - return deferred.promise; - } - - // Prepend the url with server uri if it is relative - var url = self._getAbsoluteUrl(self.url + ";metadata/content-disposition"); - - var data = "filename*=UTF-8''" + self.file.name.replace(FILENAME_REGEXP, '_'); - - if (!contextHeaderParams || !_isObject(contextHeaderParams)) { - contextHeaderParams = { - action: "upload/metadata/update", - referrer: self.reference.defaultLogInfo - }; - contextHeaderParams.referrer.column = self.column.name; - } - - var config = { - headers: _generateContextHeader(contextHeaderParams) - }; - config.headers['content-type'] = 'text/plain'; - - self.http.put(url, data, config).then(function (response) { - if (response) { - // mark as completed and job done since this is a metadata update request that doesn't transfer file data - self.isPaused = false; - self.completed = true; - self.jobDone = true; - - deferred.resolve(); - } - }, function(response) { - var error = module.responseToError(response); - deferred.reject(error); - }); - - return deferred.promise; - } - /** * @desc Call this function to start chunked upload to server. It reads the file and divides in into chunks diff --git a/test/specs/upload/tests/02.upload_obj.js b/test/specs/upload/tests/02.upload_obj.js index 110ec1365..5194fc055 100644 --- a/test/specs/upload/tests/02.upload_obj.js +++ b/test/specs/upload/tests/02.upload_obj.js @@ -298,7 +298,7 @@ exports.execute = function (options) { }); }); - it("should verify the job doesn't exist yet but a version of the same file does with a different filename.", function (done) { + it("should verify the job doesn't exist yet but a version of the same file does with a different filename. The file metadata update request should then run", function (done) { uploadObjDiffName._getExistingJobStatus().then(function (response) { expect(response).not.toBeDefined("Job status is defined"); @@ -306,7 +306,9 @@ exports.execute = function (options) { }).then(function (response) { expect(response).toBe(serverFilePath, "Server file path is incorrect"); - expect(uploadObjDiffName.updateDispositionOnly).toBeTruthy('update disposition only flag is incorrect'); + expect(uploadObjDiffName.isPaused).toBeFalsy("is paused is incorrect"); + expect(uploadObjDiffName.completed).toBeTruthy("completed is incorrect"); + expect(uploadObjDiffName.jobDone).toBeTruthy("job done is incorrect"); done(); }).catch(function (err) { @@ -315,11 +317,9 @@ exports.execute = function (options) { }); }); - it("should verify the update metadata job is created and succeeds.", function (done) { - uploadObjDiffName.createUpdateMetadataJob().then(function () { - expect(uploadObjDiffName.isPaused).toBeFalsy("is paused is incorrect"); - expect(uploadObjDiffName.completed).toBeTruthy("completed is incorrect"); - expect(uploadObjDiffName.jobDone).toBeTruthy("job done is incorrect"); + it("should verify the upload doesn't run and returns before getting existing job status.", function (done) { + uploadObjDiffName.createUploadJob().then(function (response) { + expect(response).toBeFalsy("create upload job returned the wrong response") done(); }, function (err) { From 7172adfd7b542268d38b7a056c67be8a197c23d6 Mon Sep 17 00:00:00 2001 From: Joshua Chudy Date: Fri, 22 Mar 2024 13:44:51 -0700 Subject: [PATCH 4/4] remove unnecessary conditional and set action regardless --- js/hatrac.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/js/hatrac.js b/js/hatrac.js index de914a881..8d8d149f1 100644 --- a/js/hatrac.js +++ b/js/hatrac.js @@ -448,16 +448,8 @@ var ERMrest = (function(module) { if (contentDisposition.substring(filenameIndex, contentDisposition.length) != self.file.name.replace(FILENAME_REGEXP, '_')) { // Prepend the url with server uri if it is relative var url = self._getAbsoluteUrl(self.url + ";metadata/content-disposition"); - var data = "filename*=UTF-8''" + self.file.name.replace(FILENAME_REGEXP, '_'); - - if (!contextHeaderParams || !_isObject(contextHeaderParams)) { - contextHeaderParams = { - action: "upload/metadata/update", - referrer: self.reference.defaultLogInfo - }; - contextHeaderParams.referrer.column = self.column.name; - } + contextHeaderParams.action = "upload/metadata/update" var config = { headers: _generateContextHeader(contextHeaderParams)