From 3e63db8bae453a41c875a9a41df8ee448656d12e Mon Sep 17 00:00:00 2001 From: Michael Malave Date: Tue, 2 Jun 2026 12:07:06 -0700 Subject: [PATCH] Refactor domains:index, features:index, and spaces:index list flows to use in-command pagination patterns, remove the shared paginator utility, and update unit tests to cover the new pagination behavior --- src/commands/domains/index.ts | 5 +- src/commands/features/index.ts | 6 +- src/commands/spaces/index.ts | 10 +-- src/lib/utils/paginator.ts | 32 -------- test/unit/commands/domains/index.unit.test.ts | 67 +++++++++------- .../unit/commands/features/index.unit.test.ts | 78 +++++++++++++++---- test/unit/commands/spaces/index.unit.test.ts | 62 ++++++++------- test/unit/utils/paginator.unit.test.ts | 47 ----------- 8 files changed, 148 insertions(+), 159 deletions(-) delete mode 100644 src/lib/utils/paginator.ts delete mode 100644 test/unit/utils/paginator.unit.test.ts diff --git a/src/commands/domains/index.ts b/src/commands/domains/index.ts index a787c18b83..b07761042c 100644 --- a/src/commands/domains/index.ts +++ b/src/commands/domains/index.ts @@ -1,12 +1,12 @@ import {Command, flags} from '@heroku-cli/command' import * as Heroku from '@heroku-cli/schema' import {color, hux} from '@heroku/heroku-cli-util' +import {HerokuSDK} from '@heroku/sdk' import {ux} from '@oclif/core/ux' import {orderBy} from 'natural-orderby' import Uri from 'urijs' import parseKeyValue from '../../lib/utils/key-value-parser.js' -import {paginateRequest} from '../../lib/utils/paginator.js' import {huxTableNoWrapOptions} from '../../lib/utils/table-utils.js' export default class DomainsIndex extends Command { @@ -175,7 +175,8 @@ www.example.com CNAME www.example.herokudns.com`] async run() { const {flags} = await this.parse(DomainsIndex) - const domains = await paginateRequest(this.heroku, `/apps/${flags.app}/domains`, 1000) + const {platform} = new HerokuSDK() + const domains = await platform.domain.list(flags.app) as Heroku.Domain[] const herokuDomain = domains.find((domain: Heroku.Domain) => domain.kind === 'heroku') let customDomains = domains.filter((domain: Heroku.Domain) => domain.kind === 'custom') let displayTotalDomains = false diff --git a/src/commands/features/index.ts b/src/commands/features/index.ts index 7de4a7f562..785ab1e74f 100644 --- a/src/commands/features/index.ts +++ b/src/commands/features/index.ts @@ -1,6 +1,7 @@ import {Command, flags} from '@heroku-cli/command' -import * as Heroku from '@heroku-cli/schema' +import type {AppFeature} from '@heroku/types/3.sdk' import {color, hux} from '@heroku/heroku-cli-util' +import {HerokuSDK} from '@heroku/sdk' import {ux} from '@oclif/core/ux' export default class Features extends Command { @@ -15,7 +16,8 @@ export default class Features extends Command { const {flags} = await this.parse(Features) const {app, json} = flags - let {body: features} = await this.heroku.get(`/apps/${app}/features`) + const {platform} = new HerokuSDK() + let features = await platform.appFeature.list(app) as AppFeature[] features = features.filter(f => f.state === 'general') features = features.sort((a, b) => (a.name || '').localeCompare(b.name || '')) diff --git a/src/commands/spaces/index.ts b/src/commands/spaces/index.ts index 6dc5088228..f38179b9c4 100644 --- a/src/commands/spaces/index.ts +++ b/src/commands/spaces/index.ts @@ -1,5 +1,6 @@ import {Command, flags as Flags} from '@heroku-cli/command' import {color, hux} from '@heroku/heroku-cli-util' +import {HerokuSDK} from '@heroku/sdk' import {ux} from '@oclif/core/ux' import {getGeneration} from '../../lib/apps/generation.js' @@ -47,11 +48,10 @@ export default class Index extends Command { public async run(): Promise { const {flags} = await this.parse(Index) const {json, team} = flags - let {body: spaces} = await this.heroku.get('/spaces', { - headers: { - Accept: 'application/vnd.heroku+json; version=3.sdk', - }, - }) + const {platform} = new HerokuSDK() + let spaces = await platform + .withHeaders({Accept: 'application/vnd.heroku+json; version=3.sdk'}) + .space.list() as SpaceArray if (team) { spaces = spaces.filter(s => s.team.name === team) } diff --git a/src/lib/utils/paginator.ts b/src/lib/utils/paginator.ts deleted file mode 100644 index 8ff89d15c6..0000000000 --- a/src/lib/utils/paginator.ts +++ /dev/null @@ -1,32 +0,0 @@ -// page size ranges from 200 - 1000 seen here -// https://devcenter.heroku.com/articles/platform-api-reference#ranges - -// This paginator uses status code to determine passing the Next-Range header -import {APIClient} from '@heroku-cli/command' - -export async function paginateRequest(client: APIClient, url: string, pageSize = 200): Promise { - let isPartial = true - let isFirstRequest = true - let nextRange: string | undefined = '' - let aggregatedResponseBody: T[] = [] - - while (isPartial) { - const response = await client.get(url, { - headers: { - Range: `${(isPartial && !isFirstRequest) ? `${nextRange}` : `id ..; max=${pageSize};`}`, - }, - partial: true, - }) - - aggregatedResponseBody = [...response.body, ...aggregatedResponseBody] - isFirstRequest = false - - if (response.statusCode === 206) { - nextRange = response.headers['next-range'] as string - } else { - isPartial = false - } - } - - return aggregatedResponseBody -} diff --git a/test/unit/commands/domains/index.unit.test.ts b/test/unit/commands/domains/index.unit.test.ts index 7825282872..8d7077824d 100644 --- a/test/unit/commands/domains/index.unit.test.ts +++ b/test/unit/commands/domains/index.unit.test.ts @@ -1,27 +1,37 @@ import {runCommand} from '@heroku-cli/test-utils' import {hux} from '@heroku/heroku-cli-util' +import {HerokuSDK} from '@heroku/sdk' import {expect} from 'chai' -import nock from 'nock' +import * as sinon from 'sinon' import {SinonStub, stub} from 'sinon' import DomainsIndex from '../../../../src/commands/domains/index.js' import removeAllWhitespace from '../../../helpers/utils/remove-whitespaces.js' import {unwrap} from '../../../helpers/utils/unwrap.js' +type FakePlatform = { + domain: {list: sinon.SinonStub} +} + +function buildFakePlatform(): FakePlatform { + return { + domain: {list: sinon.stub()}, + } +} + describe('domains', function () { let confirmStub: SinonStub - let api: nock.Scope + let fakePlatform: FakePlatform beforeEach(function () { - api = nock('https://api.heroku.com') + fakePlatform = buildFakePlatform() + sinon.stub(HerokuSDK.prototype, 'platform').get(() => fakePlatform) confirmStub = stub(DomainsIndex.prototype, 'confirmDisplayAllDomains') .resolves(true) }) afterEach(function () { - api.done() - confirmStub.restore() - nock.cleanAll() + sinon.restore() }) const herokuOnlyDomainsResponse = [ @@ -114,16 +124,17 @@ describe('domains', function () { ] it('does not show the custom domain header if there are no custom domains', async function () { - api.get('/apps/myapp/domains').reply(200, herokuOnlyDomainsResponse) + fakePlatform.domain.list.resolves(herokuOnlyDomainsResponse) const {stdout} = await runCommand(DomainsIndex, ['--app', 'myapp']) expect(stdout).to.contain('=== ⬢ myapp Heroku Domain\n\nmyapp.herokuapp.com') expect(stdout).to.contain('myapp.herokuapp.com') expect(stdout).to.not.contain('=== ⬢ myapp Custom Domains') + expect(fakePlatform.domain.list.calledOnceWithExactly('myapp')).to.equal(true) }) it('shows a list of domains and their DNS targets when there are custom domains', async function () { - api.get('/apps/myapp/domains').reply(200, herokuAndCustomDomainsResponse) + fakePlatform.domain.list.resolves(herokuAndCustomDomainsResponse) const {stdout} = await runCommand(DomainsIndex, ['--app', 'myapp']) const actual = removeAllWhitespace(stdout) @@ -137,7 +148,7 @@ describe('domains', function () { }) it('shows the SNI endpoint column when multiple sni endpoints are enabled', async function () { - api.get('/apps/myapp/domains').reply(200, herokuDomainWithSniEndpoint) + fakePlatform.domain.list.resolves(herokuDomainWithSniEndpoint) const {stdout} = await runCommand(DomainsIndex, ['--app', 'myapp']) const actual = removeAllWhitespace(stdout) @@ -146,25 +157,23 @@ describe('domains', function () { }) it('shows warning message for over 100 domains', async function () { - api.get('/apps/myapp/domains').reply(200, () => { - const domainData = { - acm_status: null, - acm_status_reason: null, - app: { - id: '01234567-89ab-cdef-0123-456789abcdef', - name: 'myapp', - }, - cname: null, - created_at: '2012-01-01T12:00:00Z', - hostname: 'example.com', - id: '11434567-89ab-cdef-0123-456789abcdef', - kind: 'custom', - status: 'succeeded', - updated_at: '2012-01-01T12:00:00Z', - } - - return new Array(1000).fill(domainData) // eslint-disable-line unicorn/no-new-array - }) + const domainData = { + acm_status: null, + acm_status_reason: null, + app: { + id: '01234567-89ab-cdef-0123-456789abcdef', + name: 'myapp', + }, + cname: null, + created_at: '2012-01-01T12:00:00Z', + hostname: 'example.com', + id: '11434567-89ab-cdef-0123-456789abcdef', + kind: 'custom', + status: 'succeeded', + updated_at: '2012-01-01T12:00:00Z', + } + + fakePlatform.domain.list.resolves(new Array(1000).fill(domainData)) // eslint-disable-line unicorn/no-new-array const {stderr, stdout} = await runCommand(DomainsIndex, ['--app', 'myapp']) expect(stdout).to.contain('=== ⬢ myapp Heroku Domain') @@ -172,7 +181,7 @@ describe('domains', function () { }) it('passes no-wrap option through to table rendering', async function () { - api.get('/apps/myapp/domains').reply(200, herokuAndCustomDomainsResponse) + fakePlatform.domain.list.resolves(herokuAndCustomDomainsResponse) const tableStub = stub(hux, 'table') await runCommand(DomainsIndex, ['--app', 'myapp', '--no-wrap']) diff --git a/test/unit/commands/features/index.unit.test.ts b/test/unit/commands/features/index.unit.test.ts index d6f4facba4..d2a91c7751 100644 --- a/test/unit/commands/features/index.unit.test.ts +++ b/test/unit/commands/features/index.unit.test.ts @@ -1,32 +1,41 @@ import {runCommand} from '@heroku-cli/test-utils' +import {HerokuSDK} from '@heroku/sdk' import {expect} from 'chai' -import nock from 'nock' +import * as sinon from 'sinon' import Features from '../../../../src/commands/features/index.js' +type FakePlatform = { + appFeature: {list: sinon.SinonStub} +} + +function buildFakePlatform(): FakePlatform { + return { + appFeature: {list: sinon.stub()}, + } +} + describe('features', function () { - let api: nock.Scope + let fakePlatform: FakePlatform beforeEach(function () { - api = nock('https://api.heroku.com') + fakePlatform = buildFakePlatform() + sinon.stub(HerokuSDK.prototype, 'platform').get(() => fakePlatform) }) afterEach(function () { - api.done() - nock.cleanAll() + sinon.restore() }) it('shows the app features', async function () { - api - .get('/apps/myapp/features') - .reply(200, [ - { - description: 'an app feature', - enabled: true, - name: 'feature a', - state: 'general', - }, - ]) + fakePlatform.appFeature.list.resolves([ + { + description: 'an app feature', + enabled: true, + name: 'feature a', + state: 'general', + }, + ]) const {stderr, stdout} = await runCommand(Features, ['--app', 'myapp']) @@ -35,5 +44,44 @@ describe('features', function () { [+] feature a an app feature `) + expect(fakePlatform.appFeature.list.calledOnceWithExactly('myapp')).to.equal(true) + }) + + it('filters out non-general features', async function () { + fakePlatform.appFeature.list.resolves([ + { + description: 'a general feature', + enabled: true, + name: 'feature a', + state: 'general', + }, + { + description: 'a beta feature', + enabled: false, + name: 'feature b', + state: 'beta', + }, + ]) + + const {stdout} = await runCommand(Features, ['--app', 'myapp']) + + expect(stdout).to.contain('feature a') + expect(stdout).to.not.contain('feature b') + }) + + it('shows features as json', async function () { + const features = [ + { + description: 'an app feature', + enabled: true, + name: 'feature a', + state: 'general', + }, + ] + fakePlatform.appFeature.list.resolves(features) + + const {stdout} = await runCommand(Features, ['--app', 'myapp', '--json']) + + expect(JSON.parse(stdout)).to.deep.equal(features) }) }) diff --git a/test/unit/commands/spaces/index.unit.test.ts b/test/unit/commands/spaces/index.unit.test.ts index 1086acede5..d558564253 100644 --- a/test/unit/commands/spaces/index.unit.test.ts +++ b/test/unit/commands/spaces/index.unit.test.ts @@ -1,12 +1,29 @@ import {runCommand} from '@heroku-cli/test-utils' +import {HerokuSDK} from '@heroku/sdk' import {Errors} from '@oclif/core' import ansis from 'ansis' import {expect} from 'chai' -import nock from 'nock' +import * as sinon from 'sinon' import Cmd from '../../../../src/commands/spaces/index.js' import removeAllWhitespace from '../../../helpers/utils/remove-whitespaces.js' +type FakePlatform = { + space: {list: sinon.SinonStub} + withHeaders: sinon.SinonStub +} + +function buildFakePlatform(): FakePlatform { + const spaceStub = {list: sinon.stub()} + const platform: FakePlatform = { + space: spaceStub, + withHeaders: sinon.stub(), + } + + platform.withHeaders.returns({space: spaceStub}) + return platform +} + describe('spaces', function () { const now = new Date() const spaces = [{ @@ -17,48 +34,43 @@ describe('spaces', function () { state: 'allocated', team: {name: 'my-team'}, }] - let api: nock.Scope + let fakePlatform: FakePlatform beforeEach(function () { - api = nock('https://api.heroku.com') + fakePlatform = buildFakePlatform() + sinon.stub(HerokuSDK.prototype, 'platform').get(() => fakePlatform) }) afterEach(function () { - api.done() - nock.cleanAll() + sinon.restore() }) it('shows spaces', async function () { - api - .get('/spaces') - .reply(200, spaces) + fakePlatform.space.list.resolves(spaces) const {stdout} = await runCommand(Cmd, []) const actual = removeAllWhitespace(stdout) expect(actual).to.include(removeAllWhitespace('Name Team Region State Generation Created At')) expect(actual).to.include(removeAllWhitespace(`⬡ my-space my-team my-region allocated cedar ${now.toISOString()}`)) + expect(fakePlatform.withHeaders.calledOnceWithExactly({Accept: 'application/vnd.heroku+json; version=3.sdk'})).to.equal(true) }) it('shows spaces with --json', async function () { - api - .get('/spaces') - .reply(200, spaces) + fakePlatform.space.list.resolves(spaces) const {stdout} = await runCommand(Cmd, ['--json']) expect(JSON.parse(stdout)).to.deep.eq(spaces) }) it('shows spaces scoped by teams', async function () { - api - .get('/spaces') - .reply(200, [...spaces, { - created_at: now.toISOString(), - generation: 'cedar', - name: 'other-space', - region: {name: 'my-region'}, - state: 'allocated', - team: {name: 'other-team'}, - }]) + fakePlatform.space.list.resolves([...spaces, { + created_at: now.toISOString(), + generation: 'cedar', + name: 'other-space', + region: {name: 'my-region'}, + state: 'allocated', + team: {name: 'other-team'}, + }]) const {stdout} = await runCommand(Cmd, ['--team', 'my-team']) const actual = removeAllWhitespace(stdout) @@ -67,9 +79,7 @@ describe('spaces', function () { }) it('shows spaces team error message', async function () { - api - .get('/spaces') - .reply(200, spaces) + fakePlatform.space.list.resolves(spaces) try { await runCommand(Cmd, ['--team', 'other-team']) @@ -80,9 +90,7 @@ describe('spaces', function () { }) it('shows spaces error message', async function () { - api - .get('/spaces') - .reply(200, []) + fakePlatform.space.list.resolves([]) try { await runCommand(Cmd, []) diff --git a/test/unit/utils/paginator.unit.test.ts b/test/unit/utils/paginator.unit.test.ts deleted file mode 100644 index 6bc0aa83e2..0000000000 --- a/test/unit/utils/paginator.unit.test.ts +++ /dev/null @@ -1,47 +0,0 @@ -import {APIClient} from '@heroku-cli/command' -import * as Heroku from '@heroku-cli/schema' -import {Config} from '@oclif/core' -import {expect} from 'chai' -import nock from 'nock' -import path from 'node:path' -import {fileURLToPath} from 'node:url' - -import {paginateRequest} from '../../../src/lib/utils/paginator.js' - -const __filename = fileURLToPath(import.meta.url) -const __dirname = path.dirname(__filename) -const root = path.resolve(__dirname, '../package.json') -const config = new Config({root}) -const exampleAPIClient = new APIClient(config) - -const requestUrl = '/apps/myapp/domains' - -describe('paginator', function () { - afterEach(function () { - nock.cleanAll() - }) - - it('paginates through 2 requests', async function () { - nock('https://api.heroku.com') - .get(requestUrl) - .reply(206, [{id: '1'}], {'next-range': 'id ..; max=200'}) - .get(requestUrl) - .reply(200, [{id: '2'}]) - - const results = await paginateRequest(exampleAPIClient, requestUrl, 200) - expect(results).to.have.length(2) - expect(results[0].id).to.equal('2') - expect(results[1].id).to.equal('1') - }) - - it('serves single requests', async function () { - nock('https://api.heroku.com') - .get(requestUrl) - .reply(200, [{id: '1'}]) - - const results = await paginateRequest(exampleAPIClient, requestUrl, 200) - expect(results).to.have.length(1) - expect(results).to.not.have.length(2) - expect(results[0].id).to.equal('1') - }) -})