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/baseclient.js b/src/baseclient.js index d5f0117e2..3be9203d7 100644 --- a/src/baseclient.js +++ b/src/baseclient.js @@ -289,18 +289,18 @@ BaseClient.prototype = { * URL of an item in the ``/public`` folder. * * @param {string} path - Path relative to the module root. - * @returns {string} The full URL of the item, including the storage origin + * @returns {Promise} Resolves to t he full URL of the item, including the storage origin. */ getItemURL: function (path) { if (typeof(path) !== 'string') { - throw 'Argument \'path\' of baseClient.getItemURL must be a string'; + return Promise.reject('Argument \'path\' of baseClient.getItemURL must be a string'); } + let url; if (this.storage.connected) { path = this._cleanPath( this.makePath(path) ); - return this.storage.remote.href + path; - } else { - return undefined; + url = this.storage.remote.href + path; } + return Promise.resolve(url); }, /** diff --git a/src/dropbox.js b/src/dropbox.js index 139a70f42..1fda1a27a 100644 --- a/src/dropbox.js +++ b/src/dropbox.js @@ -60,6 +60,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); }; @@ -573,26 +575,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)} @@ -615,6 +643,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)); } @@ -978,7 +1009,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 * @@ -1000,7 +1032,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); @@ -1009,14 +1042,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); @@ -1064,9 +1099,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/baseclient-suite.js b/test/unit/baseclient-suite.js index f5417df27..6679b694e 100644 --- a/test/unit/baseclient-suite.js +++ b/test/unit/baseclient-suite.js @@ -403,8 +403,9 @@ define(['./src/config', './src/baseclient', 'test/helpers/mocks', 'tv4'], env.storage.connected = true; env.storage.remote = {href: 'http://example.com/test'}; - var itemURL = env.client.getItemURL('A%2FB /C/%bla//D'); - test.assert(itemURL, 'http://example.com/test/foo/A%252FB%20/C/%25bla/D'); + env.client.getItemURL('A%2FB /C/%bla//D').then((itemURL) => { + test.assert(itemURL, 'http://example.com/test/foo/A%252FB%20/C/%25bla/D'); + }); } }, @@ -414,11 +415,13 @@ define(['./src/config', './src/baseclient', 'test/helpers/mocks', 'tv4'], env.storage.connected = true; env.storage.remote = {href: 'http://example.com/test'}; - test.assert(env.client.getItemURL("Capture d'écran"), - 'http://example.com/test/foo/Capture%20d%27%C3%A9cran'); + env.client.getItemURL("Capture d'écran").then((itemURL) => { + test.assertAnd(itemURL, 'http://example.com/test/foo/Capture%20d%27%C3%A9cran'); - test.assert(env.client.getItemURL('So they said "hey"'), - 'http://example.com/test/foo/So%20they%20said%20%22hey%22'); + env.client.getItemURL('So they said "hey"').then((itemURL) => { + test.assert(itemURL, 'http://example.com/test/foo/So%20they%20said%20%22hey%22'); + }); + }); } }, diff --git a/test/unit/dropbox-suite.js b/test/unit/dropbox-suite.js index 3338a7b90..84becfe9d 100644 --- a/test/unit/dropbox-suite.js +++ b/test/unit/dropbox-suite.js @@ -791,6 +791,46 @@ 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(() => { + mockRequestSuccess({ + status: 200, + responseText: JSON.stringify({ + links: [] + }) + }); + }, 10); + + setTimeout(() => { + test.assertAnd(getMockRequestUrl(), 'https://api.dropboxapi.com/2/sharing/create_shared_link_with_settings'); + + mockRequestSuccess({ + status: 200, + responseText: JSON.stringify({ + '.tag': 'file', + url: 'http://example.com/public/foo', + }) + }); + }, 20); + } + }, + { desc: "requests are aborted if they aren't responded after the configured timeout", timeout: 2000, @@ -906,49 +946,6 @@ define(['require', './src/util', './src/dropbox', './src/wireclient', } }, - { - desc: "share gets called after getting a public path without touching the fullfilments", - run: function (env, test) { - oldShare = env.connectedClient.share; - env.connectedClient.share = function(path) { - oldShare.bind(env.connectedClient)(path) - .then(function (r) { - test.assert(env.connectedClient._itemRefs['/public/foo'],'http://dropbox.shareing/url'); - test.done(); - }) - .catch(function (err) { - test.fail(err); - }); - env.connectedClient.share = oldShare; - }; - - addMockRequestCallback(function (req) { - mockRequestSuccess({ - status: 200, - responseHeaders: { - 'Content-Type': 'text/plain; charset=UTF-8', - 'Dropbox-API-Result': JSON.stringify({rev: 'rev'}) - }, - arrayBuffer: new ArrayBufferMock('response-body') - }); - }); - addMockRequestCallback(function (req) { - mockRequestSuccess({ - status: 200, - responseText: JSON.stringify( { - url: 'http://dropbox.shareing/url' - }) - }); - }); - env.connectedClient.get('/public/foo').then(function (r){ - test.assertAnd(r.statusCode, 200, 'status = '+r.statusCode); - test.assertAnd(r.revision, 'rev',r.revision) - test.assertAnd(r.body, 'response-body', 'body = '+ r.body); - }) - } - }, - - { desc: "Dropbox adapter hooks itself into sync cycle when activated", run: function (env, test){