Skip to content

Commit

Permalink
fix: .paginate() with results namespace (GET /installation/repositori…
Browse files Browse the repository at this point in the history
…es, single page response)
  • Loading branch information
gr2m committed Apr 14, 2019
1 parent 706af24 commit 27e2977
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 52 deletions.
4 changes: 4 additions & 0 deletions plugins/pagination/iterator.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module.exports = iterator

const normalizePaginatedListResponse = require('./normalize-paginated-list-response')

function iterator (octokit, options) {
const headers = options.headers
let url = octokit.request.endpoint(options).url
Expand All @@ -14,6 +16,8 @@ function iterator (octokit, options) {
return octokit.request({ url, headers })

.then((response) => {
normalizePaginatedListResponse(octokit, url, response)

// `response.headers.link` format:
// '<https://api.github.com/users/aseemk/followers?page=2>; rel="next", <https://api.github.com/users/aseemk/followers?page=2>; rel="last"'
// sets `url` to undefined if "next" URL is not present or `link` header is not set
Expand Down
88 changes: 88 additions & 0 deletions plugins/pagination/normalize-paginated-list-response.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* Some “list” response that can be paginated have a different response structure
*
* They have a `total_count` key in the response (search also has `incomplete_results`,
* /installation/repositories also has `repository_selection`), as well as a key with
* the list of the items which name varies from endpoint to endpoint:
*
* - https://developer.github.com/v3/search/#example (key `items`)
* - https://developer.github.com/v3/checks/runs/#response-3 (key: `check_runs`)
* - https://developer.github.com/v3/checks/suites/#response-1 (key: `check_suites`)
* - https://developer.github.com/v3/apps/installations/#list-repositories (key: `repositories`)
*
* Octokit normalizes these responses so that paginated results are always returned following
* the same structure. One challenge is that if the list response has only one page, no Link
* header is provided, so this header alone is not sufficient to check wether a response is
* paginated or not. For the exceptions with the namespace, a fallback check for the route
* paths has to be added in order to normalize the response. We cannot check for the total_count
* property because it also exists in the response of Get the combined status for a specific ref.
*/

module.exports = normalizePaginatedListResponse

const Deprecation = require('deprecation')
const once = require('once')

const deprecateIncompleteResults = once((log, deprecation) => log.warn(deprecation))
const deprecateTotalCount = once((log, deprecation) => log.warn(deprecation))
const deprecateNamespace = once((log, deprecation) => log.warn(deprecation))

const REGEX_IS_SEARCH_PATH = /^\/search\//
const REGEX_IS_CHECKS_PATH = /^\/repos\/[^/]+\/[^/]+\/commits\/[^/]+\/(check-runs|check-suites)/
const REGEX_IS_INSTALLATION_REPOSITORIES_PATH = /^\/installation\/repositories/

function normalizePaginatedListResponse (octokit, url, response) {
const path = url.replace(octokit.request.endpoint.DEFAULTS.baseUrl, '')
if (
!REGEX_IS_SEARCH_PATH.test(path) &&
!REGEX_IS_CHECKS_PATH.test(path) &&
!REGEX_IS_INSTALLATION_REPOSITORIES_PATH.test(path)
) {
return
}

// keep the additional properties intact to avoid a breaking change,
// but log a deprecation warning when accessed
const incompleteResults = response.data.incomplete_results
const repositorySelection = response.data.repository_selection
const totalCount = response.data.total_count
delete response.data.incomplete_results
delete response.data.repository_selection
delete response.data.total_count

const namespaceKey = Object.keys(response.data)[0]

response.data = response.data[namespaceKey]

Object.defineProperty(response.data, namespaceKey, {
get () {
deprecateNamespace(octokit.log, new Deprecation(`[@octokit/rest] "result.data.${namespaceKey}" is deprecated. Use "result.data" instead`))
return response.data
}
})

if (typeof incompleteResults !== 'undefined') {
Object.defineProperty(response.data, 'incomplete_results', {
get () {
deprecateIncompleteResults(octokit.log, new Deprecation('[@octokit/rest] "result.data.incomplete_results" is deprecated.'))
return incompleteResults
}
})
}

if (typeof repositorySelection !== 'undefined') {
Object.defineProperty(response.data, 'repository_selection', {
get () {
deprecateTotalCount(octokit.log, new Deprecation('[@octokit/rest] "result.data.repository_selection" is deprecated.'))
return repositorySelection
}
})
}

Object.defineProperty(response.data, 'total_count', {
get () {
deprecateTotalCount(octokit.log, new Deprecation('[@octokit/rest] "result.data.total_count" is deprecated.'))
return totalCount
}
})
}
52 changes: 0 additions & 52 deletions plugins/pagination/paginate.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
module.exports = paginate

const Deprecation = require('deprecation')
const once = require('once')

const deprecateIncompleteResults = once((log, deprecation) => log.warn(deprecation))
const deprecateTotalCount = once((log, deprecation) => log.warn(deprecation))
const deprecateNamespace = once((log, deprecation) => log.warn(deprecation))

const iterator = require('./iterator')

function paginate (octokit, route, options, mapFn) {
Expand All @@ -30,51 +23,6 @@ function gather (octokit, results, iterator, mapFn) {
earlyExit = true
}

// normalize list responses with { total_count, incomplete_results, items } keys
// https:/octokit/rest.js/issues/1283
if ('total_count' in result.value.data && result.value.headers.link) {
const incompleteResults = result.value.data.incomplete_results
const repositorySelection = result.value.data.repository_selection
const totalCount = result.value.data.total_count
delete result.value.data.incomplete_results
delete result.value.data.repository_selection
delete result.value.data.total_count

const namespaceKey = Object.keys(result.value.data)[0]

result.value.data = result.value.data[namespaceKey]

Object.defineProperty(result.value.data, namespaceKey, {
get () {
deprecateNamespace(octokit.log, new Deprecation(`[@octokit/rest] "result.data.${namespaceKey}" is deprecated. Use "result.data" instead`))
return result.value.data
}
})

if (typeof incompleteResults !== 'undefined') {
Object.defineProperty(result.value.data, 'incomplete_results', {
get () {
deprecateIncompleteResults(octokit.log, new Deprecation('[@octokit/rest] "result.data.incomplete_results" is deprecated.'))
return incompleteResults
}
})
}

Object.defineProperty(result.value.data, 'repository_selection', {
get () {
deprecateTotalCount(octokit.log, new Deprecation('[@octokit/rest] "result.data.repository_selection" is deprecated.'))
return repositorySelection
}
})

Object.defineProperty(result.value.data, 'total_count', {
get () {
deprecateTotalCount(octokit.log, new Deprecation('[@octokit/rest] "result.data.total_count" is deprecated.'))
return totalCount
}
})
}

results = results.concat(mapFn ? mapFn(result.value, done) : result.value.data)

if (earlyExit) {
Expand Down

0 comments on commit 27e2977

Please sign in to comment.