Implement getItemURL for Dropbox

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
 ------------

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:

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);
   },
 
   /**

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);
 }
 
 /**

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');
+            });
+          });
         }
       },
 

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){