Cleanup page requests to use async / await

github-tools · Jul 16, 2017 · 957282f · 957282f
1 parent 03914f2
commit 957282f
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 24 deletions.
diff --git a/lib/Requestable.js b/lib/Requestable.js
@@ -235,15 +235,18 @@ class Requestable {
     * @param {string} path - the path to request
     * @param {Object} options - the query parameters to include
     * @param {Requestable.callback} [cb] - the function to receive the data. The returned data will always be an array.
-    * @param {Object[]} results - the partial results. This argument is intended for interal use only.
     * @return {Promise} - a promise which will resolve when all pages have been fetched
     * @deprecated This will be folded into {@link Requestable#_request} in the 2.0 release.
     */
-   _requestAllPages(path, options, cb, results) {
-      results = results || [];
+   async _requestAllPages(path, options, cb) {
+      let currentPath = path;
+      let results = [];
+      let response;
+
+      try {
+         do {
+            response = await this._request('GET', currentPath, options);
 
-      return this._request('GET', path, options)
-         .then((response) => {
             let thisGroup;
             if (response.data instanceof Array) {
                thisGroup = response.data;
@@ -255,19 +258,18 @@ class Requestable {
             }
             results.push(...thisGroup);
 
-            const nextUrl = getNextPage(response.headers.link);
-            if (nextUrl && typeof options.page !== 'number') {
-               log(`getting next page: ${nextUrl}`);
-               return this._requestAllPages(nextUrl, options, cb, results);
-            }
+            currentPath = getNextPage(response.headers.link);
+         } while(currentPath);
 
-            if (cb) {
-               cb(null, results, response);
-            }
+         if (cb) {
+            cb(null, results, response);
+         }
 
-            response.data = results;
-            return response;
-         }).catch(callbackErrorOrThrow(cb, path));
+         response.data = results;
+         return response;
+      } catch (err) {
+         return callbackErrorOrThrow(cb, path);
+      }
    }
 }
 
@@ -283,6 +285,7 @@ function methodHasNoBody(method) {
 
 function getNextPage(linksHeader = '') {
    const links = linksHeader.split(/\s*,\s*/); // splits and strips the urls
+   // TODO: Change this to early abort once it finds the link in question
    return links.reduce(function(nextUrl, link) {
       if (link.search(/rel="next"/) !== -1) {
          return (link.match(/<(.*)>/) || [])[1];

diff --git a/test/organization.spec.js b/test/organization.spec.js
@@ -21,7 +21,9 @@ describe('Organization', function() {
    });
 
    after(function() {
-      return github.getProject(createdProject.id).deleteProject();
+      if (createdProject) {
+         return github.getProject(createdProject.id).deleteProject();
+      }
    });
 
    describe('reading', function() {
@@ -95,13 +97,10 @@ describe('Organization', function() {
          }));
       });
 
-      it('should list the teams in the organization', function() {
-         return organization.getTeams()
-           .then(({data}) => {
-              const hasTeam = data.some((member) => member.slug === testRepoName);
-
-              expect(hasTeam).to.be.true();
-           });
+      it('should list the teams in the organization', async function() {
+         const {data} = await organization.getTeams();
+         const hasTeam = data.some((member) => member.slug === testRepoName);
+         expect(hasTeam).to.be.true();
       });
 
       it('should create a project', function(done) {