From 8ec6a838777f8e9874d2d7b9507e16c03e480cfe Mon Sep 17 00:00:00 2001 From: Ragnis Armus Date: Sat, 2 Jun 2018 18:59:43 +0300 Subject: [PATCH] Re-implement Dropbox#getItemURL --- .../dropbox-and-google-drive.rst | 2 +- doc/js-api/base-client.rst | 6 +- src/dropbox.js | 59 +++++++++++++++---- test/unit/dropbox-suite.js | 39 ++++++++++++ 4 files changed, 89 insertions(+), 17 deletions(-) diff --git a/doc/getting-started/dropbox-and-google-drive.rst b/doc/getting-started/dropbox-and-google-drive.rst index aaaa3495c..1c69a4bb4 100644 --- a/doc/getting-started/dropbox-and-google-drive.rst +++ b/doc/getting-started/dropbox-and-google-drive.rst @@ -49,7 +49,7 @@ Known issues * Listing and deleting folders with more than 10000 files will cause problems * Content-Type is not fully supported due to limitations of the Dropbox API * Dropbox preserves cases but is not case-sensitive -* ``getItemURL`` is not implemented yet (see issue :issue:`1052`) +* ``getItemURL`` works only for files which exist and are public Google Drive ------------ diff --git a/doc/js-api/base-client.rst b/doc/js-api/base-client.rst index 01f5fe24f..61e63df57 100644 --- a/doc/js-api/base-client.rst +++ b/doc/js-api/base-client.rst @@ -430,9 +430,9 @@ Other functions :short-name: .. WARNING:: - This method currently only works for remoteStorage - backends. The issues for implementing it for Dropbox and Google - Drive can be found at :issue:`1052` and :issue:`1054`. + This method currently only works for remoteStorage and Dropbox backends. + The issue for implementing it for Google Drive can be found at + :issue:`1054`. .. autofunction:: BaseClient#scope(path) :short-name: diff --git a/src/dropbox.js b/src/dropbox.js index a88407806..186cee671 100644 --- a/src/dropbox.js +++ b/src/dropbox.js @@ -59,6 +59,8 @@ var getDropboxPath = function (path) { return cleanPath(PATH_PREFIX + '/' + path).replace(/\/$/, ''); }; +const isPublicPath = path => path.match(/^\/public\/.*[^/]$/); + var compareApiError = function (response, expect) { return new RegExp('^' + expect.join('\\/') + '(\\/|$)').test(response.error_summary); }; @@ -575,26 +577,52 @@ Dropbox.prototype = { return this._deleteSimple(path); }, + /** + * Retrieve full, absolute URL of an item. Items which are non-public or do + * not exist always resolve to undefined. + * + * @returns {Promise} - resolves to an absolute URL of the item + * + * @protected + */ + getItemURL: function (path) { + if (!isPublicPath(path)) { + return Promise.resolve(undefined); + } + + let url = this._itemRefs[path]; + if (url !== undefined) { + return Promise.resolve(url); + } + + return this._getSharedLink(path).then((link) => { + if (link !== undefined) { + return link; + } + return this._share(path); + }); + }, + /** * Calls share, if the provided path resides in a public folder. * * @private */ _shareIfNeeded: function (path) { - if (path.match(/^\/public\/.*[^/]$/) && this._itemRefs[path] === undefined) { - this.share(path); + if (isPublicPath(path) && this._itemRefs[path] === undefined) { + this._share(path); } }, /** * Gets a publicly-accessible URL for the path from Dropbox and stores it - * in ``_itemRefs``. + * in ``_itemRefs``. Resolves to undefined if the path does not exist. * * @return {Promise} a promise for the URL * * @private */ - share: function (path) { + _share: function (path) { var url = 'https://api.dropboxapi.com/2/sharing/create_shared_link_with_settings'; var options = { body: {path: getDropboxPath(path)} @@ -617,6 +645,9 @@ Dropbox.prototype = { if (compareApiError(body, ['shared_link_already_exists'])) { return this._getSharedLink(path); } + if (compareApiError(body, ['path', 'not_found'])) { + return Promise.resolve(undefined); + } return Promise.reject(new Error('API error: ' + body.error_summary)); } @@ -980,7 +1011,8 @@ Dropbox.prototype = { }, /** - * Requests the link for an already-shared file or folder. + * Requests the link for a shared file or folder. Resolves to undefined if + * the requested file or folder has not bee shared. * * @param {string} path - path to the file or folder * @@ -1002,7 +1034,8 @@ Dropbox.prototype = { return Promise.reject(new Error('Invalid response status: ' + response.status)); } - var body; + let body; + let link; try { body = JSON.parse(response.responseText); @@ -1011,14 +1044,16 @@ Dropbox.prototype = { } if (response.status === 409) { + if (compareApiError(body, ['path', 'not_found'])) { + return Promise.resolve(undefined); + } return Promise.reject(new Error('API error: ' + response.error_summary)); } - if (!body.links.length) { - return Promise.reject(new Error('No links returned')); + if (body.links.length) { + link = body.links[0].url; } - - return Promise.resolve(body.links[0].url); + return Promise.resolve(link); }, (error) => { error.message = 'Could not get link to a shared file or folder ("' + path + '"): ' + error.message; return Promise.reject(error); @@ -1066,9 +1101,7 @@ function unHookSync(rs) { function hookGetItemURL (rs) { if (rs._origBaseClientGetItemURL) { return; } rs._origBaseClientGetItemURL = BaseClient.prototype.getItemURL; - BaseClient.prototype.getItemURL = function (/*path*/) { - throw new Error('getItemURL is not implemented for Dropbox yet'); - }; + BaseClient.prototype.getItemURL = rs.dropbox.getItemURL.bind(rs.dropbox); } /** diff --git a/test/unit/dropbox-suite.js b/test/unit/dropbox-suite.js index b280d3ce7..18b7bac5b 100644 --- a/test/unit/dropbox-suite.js +++ b/test/unit/dropbox-suite.js @@ -815,6 +815,45 @@ define(['require', './src/util', './src/dropbox', './src/wireclient', } }, + { + desc: "#getItemURL returns from cache", + run: function (env, test) { + env.connectedClient._itemRefs['/public/foo'] = 'http://example.com/public/foo'; + env.connectedClient.getItemURL('/public/foo').then((itemURL) => { + test.assert(itemURL, 'http://example.com/public/foo'); + }); + } + }, + + { + desc: "#getItemURL creates shared link if it does not exist", + run: function (env, test) { + env.connectedClient.getItemURL('/public/foo').then((itemURL) => { + test.assert(itemURL, 'http://example.com/public/foo'); + }); + + setTimeout(() => { + let req = XMLHttpRequest.instances.shift(); + test.assertFail(req, undefined); + req.status = 200; + req.responseText = JSON.stringify({ links: [] }); + req._onload(); + }, 10); + + setTimeout(() => { + req = XMLHttpRequest.instances.shift(); + test.assertFail(req, undefined); + test.assertAnd(req.url, 'https://api.dropboxapi.com/2/sharing/create_shared_link_with_settings'); + req.status = 200; + req.responseText = JSON.stringify({ + '.tag': 'file', + url: 'http://example.com/public/foo', + }); + req._onload(); + }, 20); + } + }, + { desc: "requests are aborted if they aren't responded after the configured timeout", timeout: 2000,