From 4b1d7fe8343f236f9b324eed424dfc3c53b25785 Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Tue, 27 Oct 2026 13:19:18 +0500 Subject: [PATCH 1/3] fix: stop uploading the static/ folder with the build No server step reads it, and an image-heavy static/ pushed the upload past the size limit. portal generate now zips src/ in the action, as sdk and plugin generate do, so a failed zip names its reason instead of the generic error. Co-Authored-By: Claude Opus 5.5 --- src/actions/plugin/generate.ts | 2 +- src/actions/portal/prepare-project.ts | 9 +++- .../services/portal-artifacts-service.ts | 13 ++--- src/infrastructure/zip-service.ts | 20 ++++--- src/prompts/api/validate.ts | 3 +- src/prompts/plugin/generate.ts | 3 +- src/prompts/portal/prepare-project.ts | 6 +++ src/prompts/sdk/generate.ts | 3 +- src/prompts/sdk/publish.ts | 3 +- src/prompts/upload.ts | 5 ++ src/types/project-context.ts | 2 +- src/types/project-layout.ts | 3 ++ src/types/temp-context.ts | 18 +++++-- test/actions/plugin/generate.test.ts | 13 ++++- test/actions/portal/prepare-project.test.ts | 53 ++++++++++++++++++- test/actions/sdk/generate.test.ts | 2 +- test/actions/sdk/publish.test.ts | 3 +- .../services/portal-artifacts-service.test.ts | 29 +++++----- test/infrastructure/zip-service.test.ts | 21 ++++++++ test/prompts/portal/prepare-project.test.ts | 13 +++++ test/types/project-context.test.ts | 20 +++++-- 21 files changed, 195 insertions(+), 49 deletions(-) create mode 100644 src/prompts/upload.ts diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index 48099a76a..54c623dc1 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -102,7 +102,7 @@ export class PluginGenerateAction { return await written .asyncAndThen(() => new ResultAsync(configContext.stageUpload(tempDirectory, selection))) .mapErr((failure): PluginGenerationProblem => ({ kind: 'configNotPrepared', failure })) - .andThen((staged) => new ResultAsync(tempContext.zip(staged))) + .andThen((staged) => new ResultAsync(tempContext.zipBuild(staged))) .andThen((upload) => new ResultAsync( this.prompts.generatePlugin( diff --git a/src/actions/portal/prepare-project.ts b/src/actions/portal/prepare-project.ts index fe380f6b5..9922e97eb 100644 --- a/src/actions/portal/prepare-project.ts +++ b/src/actions/portal/prepare-project.ts @@ -8,6 +8,7 @@ import { DirectoryPath } from '../../types/file/directoryPath.js'; import { PortalArtifacts } from '../../types/portal/portal-artifacts.js'; import { PortalSource } from '../../types/portal/portal-source.js'; import { ProjectContext } from '../../types/project-context.js'; +import { TempContext } from '../../types/temp-context.js'; import { ActionResult } from '../action-result.js'; /** What the caller does within the shared run, in the order the run does it. */ @@ -81,9 +82,15 @@ export class PreparePortalProjectAction { // The artifacts live in this directory for as long as the caller needs them, so it wraps // everything that reads them rather than being opened and closed around the call. return await withDirPath(async (artifactsDirectory) => { + const build = await new TempContext(artifactsDirectory).zipBuild(sourceDirectory); + if (build.isErr()) { + this.prompts.sourceNotZipped(build.error, sourceDirectory); + return ActionResult.failed(); + } + const artifacts = await this.prompts.generateArtifacts( this.artifactsService.generate( - sourceDirectory, + build.value, artifactsDirectory, this.configDir, this.commandMetadata, diff --git a/src/infrastructure/services/portal-artifacts-service.ts b/src/infrastructure/services/portal-artifacts-service.ts index 43f6d1d93..ee561441f 100644 --- a/src/infrastructure/services/portal-artifacts-service.ts +++ b/src/infrastructure/services/portal-artifacts-service.ts @@ -50,12 +50,11 @@ export class PortalArtifactsService { * One run of `/portal-artifacts`: the SDKs, the code-sample catalogs and the context plugin, * delivered as a single archive and unpacked into `into`. * - * `source` is the `src/` directory, which is zipped and uploaded as the build. What the run - * produces is decided entirely by the `apimatic.json` inside it — the languages it names, and - * whether it carries a `plugin` block. + * `build` is the zipped `src/` directory. What the run produces is decided entirely by the + * `apimatic.json` inside it — the languages it names, and whether it carries a `plugin` block. */ public async generate( - source: DirectoryPath, + build: FilePath, into: DirectoryPath, configDir: DirectoryPath, commandMetadata: CommandMetadata, @@ -69,12 +68,6 @@ export class PortalArtifactsService { return err(ServiceError.unauthorizedWithHint(null)); } - const build = new FilePath(into, new FileName('build.zip')); - const archived = await this.zipService.archive(source, build); - if (archived.isErr()) { - return err(ServiceError.InvalidResponse); - } - const initiated = await this.initiateGeneration(build, commandMetadata, token); if (initiated.isErr()) { return err(initiated.error); diff --git a/src/infrastructure/zip-service.ts b/src/infrastructure/zip-service.ts index 08b6f3742..8f16c1f62 100644 --- a/src/infrastructure/zip-service.ts +++ b/src/infrastructure/zip-service.ts @@ -10,9 +10,14 @@ import { errorMessage } from '../utils/error-utils.js'; type ZipEntry = { file: FilePath; metadataPath: string }; export class ZipService { - public async archive(sourceDir: DirectoryPath, outputZipPath: FilePath): Promise> { + /** `excluding` names top-level entries of `sourceDir` to leave out of the archive. */ + public async archive( + sourceDir: DirectoryPath, + outputZipPath: FilePath, + excluding: readonly string[] = [] + ): Promise> { try { - await this.write(sourceDir, outputZipPath); + await this.write(sourceDir, outputZipPath, excluding); return ok(undefined); } catch (error) { await fs.promises.rm(outputZipPath.toString(), { force: true }).catch(() => undefined); @@ -29,8 +34,8 @@ export class ZipService { } } - private async write(sourceDir: DirectoryPath, outputZipPath: FilePath): Promise { - const entries = this.entriesUnder(sourceDir, ''); + private async write(sourceDir: DirectoryPath, outputZipPath: FilePath, excluding: readonly string[]): Promise { + const entries = this.entriesUnder(sourceDir, '', excluding); const zipfile = new yazl.ZipFile(); return new Promise((resolve, reject) => { @@ -46,12 +51,15 @@ export class ZipService { }); } - private entriesUnder(dir: DirectoryPath, relativePrefix: string): ZipEntry[] { + private entriesUnder(dir: DirectoryPath, relativePrefix: string, excluding: readonly string[]): ZipEntry[] { return fs.readdirSync(dir.toString(), { withFileTypes: true }).flatMap((entry) => { // Always use forward slashes as metadataPath — zip format requires it const metadataPath = relativePrefix ? `${relativePrefix}/${entry.name}` : entry.name; + if (excluding.includes(metadataPath)) { + return []; + } return entry.isDirectory() - ? this.entriesUnder(dir.join(entry.name), metadataPath) + ? this.entriesUnder(dir.join(entry.name), metadataPath, excluding) : [{ file: new FilePath(dir, new FileName(entry.name)), metadataPath }]; }); } diff --git a/src/prompts/api/validate.ts b/src/prompts/api/validate.ts index 8b611832b..f85a895ef 100644 --- a/src/prompts/api/validate.ts +++ b/src/prompts/api/validate.ts @@ -6,6 +6,7 @@ import { Result } from 'neverthrow'; import { ValidateApiResult, ValidationEntry, ValidationSummary } from '@apimatic/sdk'; import { FilePath } from '../../types/file/filePath.js'; import { ResolveProblem } from '../../types/resource-context.js'; +import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; import { withSpinner } from '../prompt.js'; @@ -106,7 +107,7 @@ export class ApiValidatePrompts { return; } case 'zipFailed': { - log.error(`${f.path(problem.specDirectory)} could not be zipped for upload: ${problem.reason}`); + log.error(notZippedForUpload(problem.specDirectory, problem.reason)); return; } case 'symlinks': { diff --git a/src/prompts/plugin/generate.ts b/src/prompts/plugin/generate.ts index 2f03a13ff..39a79befe 100644 --- a/src/prompts/plugin/generate.ts +++ b/src/prompts/plugin/generate.ts @@ -4,6 +4,7 @@ import { ServiceError } from '../../infrastructure/service-error.js'; import { PublishingProfileItem } from '../../types/publish-api/publishing-profile-item.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FilePath } from '../../types/file/filePath.js'; +import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; import { withSpinner } from '../prompt.js'; import { APIMATIC_CONFIG_FILE_NAME } from '../../types/apimatic-config/document.js'; @@ -24,7 +25,7 @@ function describeGenerationProblem(problem: PluginGenerationProblem, sourceDirec case 'generationFailed': return problem.error.errorMessage; case 'zipFailed': - return `${f.path(sourceDirectory)} could not be zipped for upload: ${problem.reason}`; + return notZippedForUpload(sourceDirectory, problem.reason); case 'unzipFailed': return `${ServiceError.InvalidResponse.errorMessage}\n${problem.reason}`; } diff --git a/src/prompts/portal/prepare-project.ts b/src/prompts/portal/prepare-project.ts index a9b65963e..253b4387d 100644 --- a/src/prompts/portal/prepare-project.ts +++ b/src/prompts/portal/prepare-project.ts @@ -4,11 +4,13 @@ import { ServiceError } from '../../infrastructure/service-error.js'; import { PortalAuthorizationFailure } from '../../infrastructure/services/portal-authorization-service.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FileName } from '../../types/file/fileName.js'; +import { FileProblem } from '../../types/file/file-problem.js'; import { FilePath } from '../../types/file/filePath.js'; import { ContentNotices } from '../../types/portal/content-notices.js'; import { MissingArtifacts } from '../../types/portal/generated-pages.js'; import { PortalArtifacts } from '../../types/portal/portal-artifacts.js'; import { PortalSourceProblem, SharedEndpoint } from '../../types/portal/portal-source.js'; +import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; import { describeMissingArtifacts, generateArtifacts } from './artifacts.js'; import { reportAuthorizationFailure } from './authorization.js'; @@ -39,6 +41,10 @@ export class PreparePortalProjectPrompts { reportAuthorizationFailure(failure); } + public sourceNotZipped(problem: FileProblem, sourceDirectory: DirectoryPath) { + log.error(notZippedForUpload(sourceDirectory, problem.reason)); + } + public generateArtifacts(fn: Promise>) { return generateArtifacts(fn); } diff --git a/src/prompts/sdk/generate.ts b/src/prompts/sdk/generate.ts index e3c1442e3..3b5e6f655 100644 --- a/src/prompts/sdk/generate.ts +++ b/src/prompts/sdk/generate.ts @@ -1,6 +1,7 @@ import { isCancel, confirm, log, select } from '@clack/prompts'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FileProblem } from '../../types/file/file-problem.js'; +import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; import { Result } from 'neverthrow'; import { withSpinner } from '../prompt.js'; @@ -68,7 +69,7 @@ export class SdkGeneratePrompts { } public srcDirNotZipped(sourceDirectory: DirectoryPath, problem: FileProblem) { - log.error(`${f.path(sourceDirectory)} could not be zipped for upload: ${problem.reason}`); + log.error(notZippedForUpload(sourceDirectory, problem.reason)); } public sdkNotUnzipped(problem: FileProblem) { diff --git a/src/prompts/sdk/publish.ts b/src/prompts/sdk/publish.ts index 94988baf1..017ee770b 100644 --- a/src/prompts/sdk/publish.ts +++ b/src/prompts/sdk/publish.ts @@ -8,6 +8,7 @@ import { PublishType } from '../../types/publish-api/publishing-profile-item.js' import { SemVersion } from '../../types/publish/version.js'; import { Language, Stability } from '../../types/sdk/generate.js'; import { noteWrapped, withSpinner } from '../prompt.js'; +import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; import { PublishingProfile } from '../../types/publish/publishing-profile.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; @@ -25,7 +26,7 @@ export class SdkPublishPrompts { } public sdkNotZipped(sdkDirectory: DirectoryPath, problem: FileProblem) { - log.error(`The SDK in ${f.path(sdkDirectory)} could not be zipped for upload: ${problem.reason}`); + log.error(`The SDK in ${notZippedForUpload(sdkDirectory, problem.reason)}`); } public dryRunNotice(publishingSummary: string): void { diff --git a/src/prompts/upload.ts b/src/prompts/upload.ts new file mode 100644 index 000000000..bea37d7b2 --- /dev/null +++ b/src/prompts/upload.ts @@ -0,0 +1,5 @@ +import { DirectoryPath } from '../types/file/directoryPath.js'; +import { format as f } from './format.js'; + +export const notZippedForUpload = (directory: DirectoryPath, reason: string): string => + `${f.path(directory)} could not be zipped for upload: ${reason}`; diff --git a/src/types/project-context.ts b/src/types/project-context.ts index 9c6b25021..11339846a 100644 --- a/src/types/project-context.ts +++ b/src/types/project-context.ts @@ -122,7 +122,7 @@ export class ProjectContext { if (packageSettingsDirectory) { await this.fileService.copyDirectoryContents(packageSettingsDirectory, staged.join('package-settings')); } - return await new TempContext(tempDirectory).zip(staged); + return await new TempContext(tempDirectory).zipBuild(staged); } public async isVersioned(): Promise { diff --git a/src/types/project-layout.ts b/src/types/project-layout.ts index c080a7bdc..c80918b62 100644 --- a/src/types/project-layout.ts +++ b/src/types/project-layout.ts @@ -12,6 +12,9 @@ export const CONTENT_DIRECTORY_NAME = 'content'; export const STATIC_DIRECTORY_NAME = 'static'; +/** The folders of `src/` the local site build copies to the site's root as they are, so no build upload sends them. */ +export const LOCAL_ONLY_DIRECTORY_NAMES = [STATIC_DIRECTORY_NAME] as const; + export const OUTPUT_DIRECTORY_NAMES = { sdk: 'sdk', portal: 'portal', diff --git a/src/types/temp-context.ts b/src/types/temp-context.ts index 2e0d6ba97..fe0dc881a 100644 --- a/src/types/temp-context.ts +++ b/src/types/temp-context.ts @@ -5,6 +5,7 @@ import { DirectoryPath } from './file/directoryPath.js'; import { FilePath } from './file/filePath.js'; import { FileName } from './file/fileName.js'; import { FileProblem } from './file/file-problem.js'; +import { LOCAL_ONLY_DIRECTORY_NAMES } from './project-layout.js'; import { randomUUID } from 'node:crypto'; export class TempContext { @@ -19,9 +20,11 @@ export class TempContext { } public async zip(directory: DirectoryPath): Promise> { - const tempFile = this.getTempFileName; - const archived = await this.zipService.archive(directory, tempFile); - return archived.map(() => tempFile).mapErr((reason): FileProblem => ({ kind: 'zipFailed', reason })); + return await this.zipWithout(directory, []); + } + + public async zipBuild(directory: DirectoryPath): Promise> { + return await this.zipWithout(directory, LOCAL_ONLY_DIRECTORY_NAMES); } public async save(portalStream: NodeJS.ReadableStream): Promise { @@ -29,4 +32,13 @@ export class TempContext { await this.fileService.writeFile(tempFile, portalStream); return tempFile; } + + private async zipWithout( + directory: DirectoryPath, + excluding: readonly string[] + ): Promise> { + const tempFile = this.getTempFileName; + const archived = await this.zipService.archive(directory, tempFile, excluding); + return archived.map(() => tempFile).mapErr((reason): FileProblem => ({ kind: 'zipFailed', reason })); + } } diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index bd150024c..dc5787fae 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -178,6 +178,17 @@ describe('PluginGenerateAction', () => { expect(uploaded['spec/openapi.json']).to.equal('{"openapi": "3.0.0"}'); }); + it('keeps content/ and leaves out static/', async () => { + generated(); + await fsExtra.outputFile(path.join(sourceDirectory, 'content', 'index.md'), '# Home'); + await fsExtra.outputFile(path.join(sourceDirectory, 'static', 'images', 'logo.png'), 'png'); + + await execute(); + + expect(uploaded['content/index.md']).to.equal('# Home'); + expect(Object.keys(uploaded).filter((entry) => entry.startsWith('static/'))).to.be.empty; + }); + it('synthesizes nothing: no plugin-config.json is written for the server', async () => { generated(); @@ -644,7 +655,7 @@ describe('PluginGenerateAction', () => { it('reports a source directory it could not zip, without generating', async () => { const generatePlugin = sinon.stub(PluginService.prototype, 'generatePlugin'); const problem: FileProblem = { kind: 'zipFailed', reason: 'EACCES: permission denied' }; - sinon.stub(TempContext.prototype, 'zip').resolves(err(problem)); + sinon.stub(TempContext.prototype, 'zipBuild').resolves(err(problem)); const pluginNotGenerated = sinon.stub(PluginGeneratePrompts.prototype, 'pluginNotGenerated'); expect((await execute()).isFailed()).to.be.true; diff --git a/test/actions/portal/prepare-project.test.ts b/test/actions/portal/prepare-project.test.ts index 912fab47b..61442ed45 100644 --- a/test/actions/portal/prepare-project.test.ts +++ b/test/actions/portal/prepare-project.test.ts @@ -1,13 +1,18 @@ import os from 'os'; +import AdmZip from 'adm-zip'; import sinon from 'sinon'; import { expect } from 'chai'; -import { ok } from 'neverthrow'; +import { err, ok } from 'neverthrow'; +import { ActionResult } from '../../../src/actions/action-result'; import { PreparePortalProjectAction } from '../../../src/actions/portal/prepare-project'; import { PortalAuthorizationService } from '../../../src/infrastructure/services/portal-authorization-service'; import { DirectoryPath } from '../../../src/types/file/directoryPath'; +import { FileProblem } from '../../../src/types/file/file-problem'; +import { FilePath } from '../../../src/types/file/filePath'; import { CommandMetadata } from '../../../src/types/common/command-metadata'; import { ProjectContext } from '../../../src/types/project-context'; -import { stubPreparePortalProject } from './prepare-project-stubs'; +import { TempContext } from '../../../src/types/temp-context'; +import { completeArtifacts, PreparePortalProjectStubs, stubPreparePortalProject } from './prepare-project-stubs'; const COMMAND_METADATA: CommandMetadata = { commandName: 'portal generate', shell: 'test' }; @@ -34,4 +39,48 @@ describe('PreparePortalProjectAction', () => { expect(authorize.called).to.be.false; expect(onPrepared.called).to.be.false; }); + + describe('the build it uploads', () => { + const FIXTURE = new DirectoryPath(process.cwd()).join('test/resources/portal-inputs/default'); + let shared: PreparePortalProjectStubs; + let onPrepared: sinon.SinonStub; + + const execute = () => + new PreparePortalProjectAction(new DirectoryPath(os.tmpdir()), COMMAND_METADATA).execute( + ProjectContext.in(FIXTURE), + { onPrepared } + ); + + beforeEach(() => { + shared = stubPreparePortalProject(); + sinon.stub(PortalAuthorizationService.prototype, 'authorize').resolves(ok(undefined)); + onPrepared = sinon.stub().resolves(ActionResult.success()); + }); + + it('is src/ with content/ and without static/', async () => { + let uploaded: string[] = []; + // Read while the stub runs: the build's temporary directory is gone once the action returns. + shared.artifacts.callsFake(async (build: FilePath) => { + uploaded = new AdmZip(build.toString()).getEntries().map((entry) => entry.entryName); + return ok(completeArtifacts()); + }); + + await execute(); + + expect(uploaded).to.include.members(['apimatic.json', 'content/index.md']); + expect(uploaded.filter((entry) => entry.startsWith('static/'))).to.be.empty; + }); + + it('reports a source it could not zip, without asking for the artifacts', async () => { + const problem: FileProblem = { kind: 'zipFailed', reason: 'EBUSY: resource busy or locked' }; + sinon.stub(TempContext.prototype, 'zipBuild').resolves(err(problem)); + + const result = await execute(); + + expect(result.isFailed()).to.be.true; + expect(shared.prompts.sourceNotZipped.calledOnceWith(problem, FIXTURE.join('src'))).to.be.true; + expect(shared.artifacts.called).to.be.false; + expect(onPrepared.called).to.be.false; + }); + }); }); diff --git a/test/actions/sdk/generate.test.ts b/test/actions/sdk/generate.test.ts index 8fc68c54d..64489b585 100644 --- a/test/actions/sdk/generate.test.ts +++ b/test/actions/sdk/generate.test.ts @@ -69,7 +69,7 @@ describe('GenerateAction (sdk)', () => { it('reports a source directory it could not zip, without generating', async () => { const problem: FileProblem = { kind: 'zipFailed', reason: 'EACCES: permission denied' }; - sinon.stub(TempContext.prototype, 'zip').resolves(err(problem)); + sinon.stub(TempContext.prototype, 'zipBuild').resolves(err(problem)); const srcDirNotZipped = sinon.stub(SdkGeneratePrompts.prototype, 'srcDirNotZipped'); expect((await execute()).isFailed()).to.be.true; diff --git a/test/actions/sdk/publish.test.ts b/test/actions/sdk/publish.test.ts index aec005420..286467ac5 100644 --- a/test/actions/sdk/publish.test.ts +++ b/test/actions/sdk/publish.test.ts @@ -74,8 +74,7 @@ describe('SdkPublishAction', () => { it('reports an SDK it could not zip, without publishing', async () => { const problem: FileProblem = { kind: 'zipFailed', reason: 'EACCES: permission denied' }; - const zip = sinon.stub(TempContext.prototype, 'zip').callThrough(); - zip.onSecondCall().resolves(err(problem)); + sinon.stub(TempContext.prototype, 'zip').resolves(err(problem)); const publishSdkPackage = sinon.stub(PublishingApiService.prototype, 'publishSdkPackage'); const sdkNotZipped = sinon.stub(SdkPublishPrompts.prototype, 'sdkNotZipped'); diff --git a/test/infrastructure/services/portal-artifacts-service.test.ts b/test/infrastructure/services/portal-artifacts-service.test.ts index e7d0574b5..1ef46dac9 100644 --- a/test/infrastructure/services/portal-artifacts-service.test.ts +++ b/test/infrastructure/services/portal-artifacts-service.test.ts @@ -25,14 +25,14 @@ const CATALOG = { describe('PortalArtifactsService', () => { let server: http.Server; let workDir: string; - let source: DirectoryPath; + let build: FilePath; let configDir: DirectoryPath; let service: PortalArtifactsService; /** What the endpoint will answer the download with, built fresh per test. */ let artifactsZip: Buffer; let respondToStatus: (res: http.ServerResponse) => void; - let postedBodyBytes: number; + let postedBody: Buffer; const json = (res: http.ServerResponse, statusCode: number, body: unknown) => { res.writeHead(statusCode, { 'Content-Type': 'application/json' }); @@ -58,7 +58,7 @@ describe('PortalArtifactsService', () => { const generate = async (into?: string) => { const destination = into ?? fs.mkdtempSync(path.join(workDir, 'into-')); - return await service.generate(source, new DirectoryPath(destination), configDir, metadata, AUTH_KEY); + return await service.generate(build, new DirectoryPath(destination), configDir, metadata, AUTH_KEY); }; before(async () => { @@ -66,9 +66,12 @@ describe('PortalArtifactsService', () => { const url = req.url ?? ''; if (req.method === 'POST') { - postedBodyBytes = 0; - req.on('data', (chunk: Buffer) => (postedBodyBytes += chunk.length)); - req.on('end', () => json(res, 202, { id: GENERATION_ID })); + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => chunks.push(chunk)); + req.on('end', () => { + postedBody = Buffer.concat(chunks); + json(res, 202, { id: GENERATION_ID }); + }); return; } @@ -93,12 +96,13 @@ describe('PortalArtifactsService', () => { workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'portal-artifacts-')); - // The source directory the service zips and uploads. + // The build the service uploads: a zipped source directory. const sourceDir = path.join(workDir, 'src'); fs.mkdirSync(path.join(sourceDir, 'spec'), { recursive: true }); fs.writeFileSync(path.join(sourceDir, 'apimatic.json'), JSON.stringify({ schemaVersion: 1 })); fs.writeFileSync(path.join(sourceDir, 'spec', 'openapi.json'), '{}'); - source = new DirectoryPath(sourceDir); + build = new FilePath(new DirectoryPath(workDir), new FileName('build.zip')); + (await new ZipService().archive(new DirectoryPath(sourceDir), build))._unsafeUnwrap(); // No config.json here, so the explicit authKey is used. configDir = new DirectoryPath(workDir); @@ -201,11 +205,10 @@ describe('PortalArtifactsService', () => { }); }); - it('uploads the source directory as the build', async () => { + it('uploads the build it is given', async () => { await generate(); - // The zip carries the two files written above, so a body this size cannot be an empty archive. - expect(postedBodyBytes).to.be.greaterThan(100); + expect(postedBody.includes(fs.readFileSync(build.toString()))).to.be.true; }); describe('a run that fails', () => { @@ -256,7 +259,7 @@ describe('PortalArtifactsService', () => { const error = ( await impatient.generate( - source, + build, new DirectoryPath(fs.mkdtempSync(path.join(workDir, 'into-'))), configDir, metadata, @@ -299,7 +302,7 @@ describe('PortalArtifactsService', () => { it('asks for an auth key it does not have rather than calling without one', async () => { const error = ( await service.generate( - source, + build, new DirectoryPath(fs.mkdtempSync(path.join(workDir, 'into-'))), configDir, metadata, diff --git a/test/infrastructure/zip-service.test.ts b/test/infrastructure/zip-service.test.ts index 959f18e31..9068bb8d3 100644 --- a/test/infrastructure/zip-service.test.ts +++ b/test/infrastructure/zip-service.test.ts @@ -43,6 +43,27 @@ describe('ZipService', () => { expect(entries).to.have.members(['openapi.json', 'paths/pets.json']); }); + it('leaves nothing out unless told to', async () => { + write('index.html', ''); + write(path.join('static', 'logo.png'), 'png'); + + expect((await new ZipService().archive(source, archive)).isOk()).to.be.true; + + const entries = new AdmZip(archive.toString()).getEntries().map((entry) => entry.entryName); + expect(entries).to.have.members(['index.html', 'static/logo.png']); + }); + + it('leaves out an entry it is told to, and keeps a nested folder with the same name', async () => { + write(path.join('spec', 'openapi.json'), '{}'); + write(path.join('static', 'images', 'logo.png'), 'png'); + write(path.join('content', 'static', 'page.md'), '# Page'); + + expect((await new ZipService().archive(source, archive, ['static'])).isOk()).to.be.true; + + const entries = new AdmZip(archive.toString()).getEntries().map((entry) => entry.entryName); + expect(entries).to.have.members(['spec/openapi.json', 'content/static/page.md']); + }); + it('reports a source directory that is not there', async () => { const archived = await new ZipService().archive(source.join('missing'), archive); diff --git a/test/prompts/portal/prepare-project.test.ts b/test/prompts/portal/prepare-project.test.ts index f73b7ba94..419d40e96 100644 --- a/test/prompts/portal/prepare-project.test.ts +++ b/test/prompts/portal/prepare-project.test.ts @@ -35,6 +35,19 @@ describe('PreparePortalProjectPrompts', () => { ); }); + it('names the source directory it could not zip, and why', () => { + const error = sinon.stub(log, 'error'); + + new PreparePortalProjectPrompts().sourceNotZipped( + { kind: 'zipFailed', reason: 'EBUSY: resource busy or locked' }, + new DirectoryPath('project').join('src') + ); + + expect(stripVTControlCharacters(String(error.firstCall.args[0]))).to.contain( + "src' could not be zipped for upload: EBUSY: resource busy or locked" + ); + }); + describe('a source the build refuses', () => { const source = new DirectoryPath('project').join('src'); const example = diff --git a/test/types/project-context.test.ts b/test/types/project-context.test.ts index 016c78b7a..81fe0aa6b 100644 --- a/test/types/project-context.test.ts +++ b/test/types/project-context.test.ts @@ -87,17 +87,29 @@ describe('ProjectContext', () => { expect(await project().specsExist()).to.be.true; }); - it('zips the source directory for the generators, with the package settings when given', async () => { + it('zips src/ for the generators with the package settings and content/, leaving out static/', async () => { write('src/spec/openapi.json', '{}'); + write('src/content/index.md', '# Home'); + write('src/static/images/logo.png', 'png'); const temp = new DirectoryPath(inProject('temp')); const settings = new DirectoryPath(inProject('settings')); write('settings/package.json', '{}'); const zip = (await project().srcDirZip(temp, settings))._unsafeUnwrap(); - expect(fs.existsSync(zip.toString())).to.be.true; - expect(fs.existsSync(inProject('temp', 'src', 'spec', 'openapi.json'))).to.be.true; - expect(fs.existsSync(inProject('temp', 'src', 'package-settings', 'package.json'))).to.be.true; + const entries = new AdmZip(zip.toString()).getEntries().map((entry) => entry.entryName); + expect(entries).to.have.members(['spec/openapi.json', 'content/index.md', 'package-settings/package.json']); + }); + + it('keeps a spec folder named static: only a build upload leaves static/ out', async () => { + write('src/spec/openapi.yaml', 'openapi: 3.0.3'); + write('src/spec/static/pet.yaml', 'type: object'); + fs.mkdirSync(inProject('temp')); + + const zip = (await project().specZip(new DirectoryPath(inProject('temp'))))._unsafeUnwrap(); + + const entries = new AdmZip(zip.toString()).getEntries().map((entry) => entry.entryName); + expect(entries).to.have.members(['openapi.yaml', 'static/pet.yaml']); }); it('zips all of spec/ for validation, so a spec split across files keeps the ones it references', async () => { From acd7bff207cbf30c1bfc8ae6a2519998bcb7592c Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Tue, 27 Oct 2026 13:55:21 +0500 Subject: [PATCH 2/3] test: fold the static/ check into the existing spec zip test Sonar counted the separate test as duplicated lines, over the 3% gate for new code. Co-Authored-By: Claude Opus 5.5 --- test/types/project-context.test.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/test/types/project-context.test.ts b/test/types/project-context.test.ts index 81fe0aa6b..e71764572 100644 --- a/test/types/project-context.test.ts +++ b/test/types/project-context.test.ts @@ -101,27 +101,18 @@ describe('ProjectContext', () => { expect(entries).to.have.members(['spec/openapi.json', 'content/index.md', 'package-settings/package.json']); }); - it('keeps a spec folder named static: only a build upload leaves static/ out', async () => { - write('src/spec/openapi.yaml', 'openapi: 3.0.3'); - write('src/spec/static/pet.yaml', 'type: object'); - fs.mkdirSync(inProject('temp')); - - const zip = (await project().specZip(new DirectoryPath(inProject('temp'))))._unsafeUnwrap(); - - const entries = new AdmZip(zip.toString()).getEntries().map((entry) => entry.entryName); - expect(entries).to.have.members(['openapi.yaml', 'static/pet.yaml']); - }); - + // A folder named static is kept too: only a build upload leaves static/ out. it('zips all of spec/ for validation, so a spec split across files keeps the ones it references', async () => { write('src/spec/openapi.yaml', 'openapi: 3.0.3'); write('src/spec/schemas/pet.yaml', 'type: object'); + write('src/spec/static/owner.yaml', 'type: object'); write('src/APIMATIC-BUILD.json', '{}'); fs.mkdirSync(inProject('temp')); const zip = (await project().specZip(new DirectoryPath(inProject('temp'))))._unsafeUnwrap(); const entries = new AdmZip(zip.toString()).getEntries().map((entry) => entry.entryName); - expect(entries).to.have.members(['openapi.yaml', 'schemas/pet.yaml']); + expect(entries).to.have.members(['openapi.yaml', 'schemas/pet.yaml', 'static/owner.yaml']); }); it('zips nothing when spec/ is empty, and names the directory it looked in', async () => { From 55fbae3d57fb1e1ff2063e7fdf7f14be7c6fe825 Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Tue, 27 Oct 2026 17:17:05 +0500 Subject: [PATCH 3/3] fix: make every build upload from one copy of src/ without static/ ProjectContext.buildZip and pluginBuildZip now make the upload for the portal, sdk and plugin commands. They copy src/ without static/ (in any letter case), following links, and report a failed copy like a failed zip, once the copy has stopped. TempContext and ZipService are back to what they are on dev, and the shared zip-failure sentence lives in prompts/prompt.ts. Co-Authored-By: Claude Opus 5.5 --- src/actions/plugin/generate.ts | 3 +- src/actions/portal/prepare-project.ts | 3 +- src/actions/sdk/generate.ts | 8 +-- src/infrastructure/file-service.ts | 28 +++++++- src/infrastructure/zip-service.ts | 20 ++---- src/prompts/api/validate.ts | 3 +- src/prompts/plugin/generate.ts | 3 +- src/prompts/portal/prepare-project.ts | 2 +- src/prompts/prompt.ts | 5 ++ src/prompts/sdk/generate.ts | 3 +- src/prompts/sdk/publish.ts | 3 +- src/prompts/upload.ts | 5 -- src/types/plugin-config-context.ts | 21 ++---- src/types/plugin/generation-problem.ts | 7 +- src/types/project-context.ts | 54 +++++++++++++-- src/types/temp-context.ts | 18 +---- test/actions/plugin/generate.test.ts | 2 +- test/actions/portal/prepare-project.test.ts | 2 +- test/actions/sdk/generate.test.ts | 2 +- test/actions/sdk/publish.test.ts | 3 +- test/infrastructure/file-service.test.ts | 74 +++++++++++++++++++++ test/infrastructure/zip-service.test.ts | 21 ------ test/types/project-context.test.ts | 20 ++++-- 23 files changed, 199 insertions(+), 111 deletions(-) delete mode 100644 src/prompts/upload.ts diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index 54c623dc1..aed7caf05 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -100,9 +100,8 @@ export class PluginGenerateAction { const tempContext = new TempContext(tempDirectory); const written = await configContext.recordLanguages(selection); return await written - .asyncAndThen(() => new ResultAsync(configContext.stageUpload(tempDirectory, selection))) .mapErr((failure): PluginGenerationProblem => ({ kind: 'configNotPrepared', failure })) - .andThen((staged) => new ResultAsync(tempContext.zipBuild(staged))) + .asyncAndThen(() => new ResultAsync(project.pluginBuildZip(tempDirectory, selection))) .andThen((upload) => new ResultAsync( this.prompts.generatePlugin( diff --git a/src/actions/portal/prepare-project.ts b/src/actions/portal/prepare-project.ts index 9922e97eb..ccb3f0c19 100644 --- a/src/actions/portal/prepare-project.ts +++ b/src/actions/portal/prepare-project.ts @@ -8,7 +8,6 @@ import { DirectoryPath } from '../../types/file/directoryPath.js'; import { PortalArtifacts } from '../../types/portal/portal-artifacts.js'; import { PortalSource } from '../../types/portal/portal-source.js'; import { ProjectContext } from '../../types/project-context.js'; -import { TempContext } from '../../types/temp-context.js'; import { ActionResult } from '../action-result.js'; /** What the caller does within the shared run, in the order the run does it. */ @@ -82,7 +81,7 @@ export class PreparePortalProjectAction { // The artifacts live in this directory for as long as the caller needs them, so it wraps // everything that reads them rather than being opened and closed around the call. return await withDirPath(async (artifactsDirectory) => { - const build = await new TempContext(artifactsDirectory).zipBuild(sourceDirectory); + const build = await project.buildZip(artifactsDirectory); if (build.isErr()) { this.prompts.sourceNotZipped(build.error, sourceDirectory); return ActionResult.failed(); diff --git a/src/actions/sdk/generate.ts b/src/actions/sdk/generate.ts index 97ac31e31..a40f46a33 100644 --- a/src/actions/sdk/generate.ts +++ b/src/actions/sdk/generate.ts @@ -71,15 +71,15 @@ export class GenerateAction { return await withDirPath(async (tempDirectory) => { const tempContext = new TempContext(tempDirectory); - const srcDirZip = await buildFrom.srcDirZip(tempDirectory, packageSettingsDirectory); - if (srcDirZip.isErr()) { - this.prompts.srcDirNotZipped(sourceDirectory, srcDirZip.error); + const build = await buildFrom.buildZip(tempDirectory, packageSettingsDirectory); + if (build.isErr()) { + this.prompts.srcDirNotZipped(sourceDirectory, build.error); return ActionResult.failed(); } const response = await this.prompts.generateSdk( this.sdkGenerationService.generateSdk( - srcDirZip.value, + build.value, language, stability, this.configDir, diff --git a/src/infrastructure/file-service.ts b/src/infrastructure/file-service.ts index 85692a796..f7a078ec6 100644 --- a/src/infrastructure/file-service.ts +++ b/src/infrastructure/file-service.ts @@ -242,6 +242,20 @@ export class FileService { await this.forEachEntry(source, destination, (from, to) => fsExtra.copy(from, to)); } + /** Copies what links point to, so relative ones still resolve. `excluding` ignores case, as Windows and macOS do. */ + public async copyDirectoryContentsFollowingLinks( + source: DirectoryPath, + destination: DirectoryPath, + excluding: readonly FileName[] = [] + ) { + await this.forEachEntry( + source, + destination, + (from, to) => fsExtra.copy(from, to, { dereference: true }), + excluding + ); + } + public async moveDirectoryContents(source: DirectoryPath, destination: DirectoryPath) { await this.forEachEntry(source, destination, (from, to) => fsExtra.move(from, to, { overwrite: true })); } @@ -249,12 +263,20 @@ export class FileService { private async forEachEntry( source: DirectoryPath, destination: DirectoryPath, - operation: (from: string, to: string) => Promise + operation: (from: string, to: string) => Promise, + excluding: readonly FileName[] = [] ) { const entries = await fsExtra.readdir(source.toString()); - await Promise.all( - entries.map((entry) => operation(path.join(source.toString(), entry), path.join(destination.toString(), entry))) + // Settled rather than raced, so no entry is still being written when the caller cleans up after a failure. + const outcomes = await Promise.allSettled( + entries + .filter((entry) => !excluding.some((name) => name.is(entry))) + .map((entry) => operation(path.join(source.toString(), entry), path.join(destination.toString(), entry))) ); + const failure = outcomes.find((outcome): outcome is PromiseRejectedResult => outcome.status === 'rejected'); + if (failure) { + throw failure.reason; + } } public async deleteFile(filePath: FilePath): Promise { diff --git a/src/infrastructure/zip-service.ts b/src/infrastructure/zip-service.ts index 8f16c1f62..08b6f3742 100644 --- a/src/infrastructure/zip-service.ts +++ b/src/infrastructure/zip-service.ts @@ -10,14 +10,9 @@ import { errorMessage } from '../utils/error-utils.js'; type ZipEntry = { file: FilePath; metadataPath: string }; export class ZipService { - /** `excluding` names top-level entries of `sourceDir` to leave out of the archive. */ - public async archive( - sourceDir: DirectoryPath, - outputZipPath: FilePath, - excluding: readonly string[] = [] - ): Promise> { + public async archive(sourceDir: DirectoryPath, outputZipPath: FilePath): Promise> { try { - await this.write(sourceDir, outputZipPath, excluding); + await this.write(sourceDir, outputZipPath); return ok(undefined); } catch (error) { await fs.promises.rm(outputZipPath.toString(), { force: true }).catch(() => undefined); @@ -34,8 +29,8 @@ export class ZipService { } } - private async write(sourceDir: DirectoryPath, outputZipPath: FilePath, excluding: readonly string[]): Promise { - const entries = this.entriesUnder(sourceDir, '', excluding); + private async write(sourceDir: DirectoryPath, outputZipPath: FilePath): Promise { + const entries = this.entriesUnder(sourceDir, ''); const zipfile = new yazl.ZipFile(); return new Promise((resolve, reject) => { @@ -51,15 +46,12 @@ export class ZipService { }); } - private entriesUnder(dir: DirectoryPath, relativePrefix: string, excluding: readonly string[]): ZipEntry[] { + private entriesUnder(dir: DirectoryPath, relativePrefix: string): ZipEntry[] { return fs.readdirSync(dir.toString(), { withFileTypes: true }).flatMap((entry) => { // Always use forward slashes as metadataPath — zip format requires it const metadataPath = relativePrefix ? `${relativePrefix}/${entry.name}` : entry.name; - if (excluding.includes(metadataPath)) { - return []; - } return entry.isDirectory() - ? this.entriesUnder(dir.join(entry.name), metadataPath, excluding) + ? this.entriesUnder(dir.join(entry.name), metadataPath) : [{ file: new FilePath(dir, new FileName(entry.name)), metadataPath }]; }); } diff --git a/src/prompts/api/validate.ts b/src/prompts/api/validate.ts index f85a895ef..699055275 100644 --- a/src/prompts/api/validate.ts +++ b/src/prompts/api/validate.ts @@ -6,9 +6,8 @@ import { Result } from 'neverthrow'; import { ValidateApiResult, ValidationEntry, ValidationSummary } from '@apimatic/sdk'; import { FilePath } from '../../types/file/filePath.js'; import { ResolveProblem } from '../../types/resource-context.js'; -import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; -import { withSpinner } from '../prompt.js'; +import { notZippedForUpload, withSpinner } from '../prompt.js'; export class ApiValidatePrompts { public async validateApi(fn: Promise>) { diff --git a/src/prompts/plugin/generate.ts b/src/prompts/plugin/generate.ts index 39a79befe..73360657a 100644 --- a/src/prompts/plugin/generate.ts +++ b/src/prompts/plugin/generate.ts @@ -4,9 +4,8 @@ import { ServiceError } from '../../infrastructure/service-error.js'; import { PublishingProfileItem } from '../../types/publish-api/publishing-profile-item.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FilePath } from '../../types/file/filePath.js'; -import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; -import { withSpinner } from '../prompt.js'; +import { notZippedForUpload, withSpinner } from '../prompt.js'; import { APIMATIC_CONFIG_FILE_NAME } from '../../types/apimatic-config/document.js'; import { PluginConfig } from '../../types/plugin-config-context.js'; import { PluginGenerationProblem } from '../../types/plugin/generation-problem.js'; diff --git a/src/prompts/portal/prepare-project.ts b/src/prompts/portal/prepare-project.ts index 253b4387d..1e51865da 100644 --- a/src/prompts/portal/prepare-project.ts +++ b/src/prompts/portal/prepare-project.ts @@ -10,8 +10,8 @@ import { ContentNotices } from '../../types/portal/content-notices.js'; import { MissingArtifacts } from '../../types/portal/generated-pages.js'; import { PortalArtifacts } from '../../types/portal/portal-artifacts.js'; import { PortalSourceProblem, SharedEndpoint } from '../../types/portal/portal-source.js'; -import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; +import { notZippedForUpload } from '../prompt.js'; import { describeMissingArtifacts, generateArtifacts } from './artifacts.js'; import { reportAuthorizationFailure } from './authorization.js'; import { reportUnplacedSamples } from './code-samples.js'; diff --git a/src/prompts/prompt.ts b/src/prompts/prompt.ts index fe3028cc0..406a8e602 100644 --- a/src/prompts/prompt.ts +++ b/src/prompts/prompt.ts @@ -4,6 +4,8 @@ import pc from 'picocolors'; import { getColumns } from '@clack/core'; import { log, note, NoteOptions, S_BAR_H, S_CONNECT_LEFT, spinner } from '@clack/prompts'; import { Result } from 'neverthrow'; +import { DirectoryPath } from '../types/file/directoryPath.js'; +import { format as f } from './format.js'; /** A fixed message, or one built from what the operation returned. */ type SpinnerMessage = string | ((value: T) => string); @@ -56,6 +58,9 @@ export const noteWrapped = (message: string, title: string) => { } }; +export const notZippedForUpload = (directory: DirectoryPath, reason: string): string => + `${f.path(directory)} could not be zipped for upload: ${reason}`; + type ColumnStyle = 'primary' | 'secondary' | 'item'; export function buildTableWithHeading( diff --git a/src/prompts/sdk/generate.ts b/src/prompts/sdk/generate.ts index 3b5e6f655..417a0eb8e 100644 --- a/src/prompts/sdk/generate.ts +++ b/src/prompts/sdk/generate.ts @@ -1,10 +1,9 @@ import { isCancel, confirm, log, select } from '@clack/prompts'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FileProblem } from '../../types/file/file-problem.js'; -import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; import { Result } from 'neverthrow'; -import { withSpinner } from '../prompt.js'; +import { notZippedForUpload, withSpinner } from '../prompt.js'; import { ServiceError } from '../../infrastructure/service-error.js'; import { AVAILABLE_LANGUAGES, Language, languageLabel, UPCOMING_LANGUAGES } from '../../types/sdk/generate.js'; import { VersionProblem } from '../../types/project-context.js'; diff --git a/src/prompts/sdk/publish.ts b/src/prompts/sdk/publish.ts index 017ee770b..94988baf1 100644 --- a/src/prompts/sdk/publish.ts +++ b/src/prompts/sdk/publish.ts @@ -8,7 +8,6 @@ import { PublishType } from '../../types/publish-api/publishing-profile-item.js' import { SemVersion } from '../../types/publish/version.js'; import { Language, Stability } from '../../types/sdk/generate.js'; import { noteWrapped, withSpinner } from '../prompt.js'; -import { notZippedForUpload } from '../upload.js'; import { format as f } from '../format.js'; import { PublishingProfile } from '../../types/publish/publishing-profile.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; @@ -26,7 +25,7 @@ export class SdkPublishPrompts { } public sdkNotZipped(sdkDirectory: DirectoryPath, problem: FileProblem) { - log.error(`The SDK in ${notZippedForUpload(sdkDirectory, problem.reason)}`); + log.error(`The SDK in ${f.path(sdkDirectory)} could not be zipped for upload: ${problem.reason}`); } public dryRunNotice(publishingSummary: string): void { diff --git a/src/prompts/upload.ts b/src/prompts/upload.ts deleted file mode 100644 index bea37d7b2..000000000 --- a/src/prompts/upload.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { DirectoryPath } from '../types/file/directoryPath.js'; -import { format as f } from './format.js'; - -export const notZippedForUpload = (directory: DirectoryPath, reason: string): string => - `${f.path(directory)} could not be zipped for upload: ${reason}`; diff --git a/src/types/plugin-config-context.ts b/src/types/plugin-config-context.ts index 2956e98ac..d3360028f 100644 --- a/src/types/plugin-config-context.ts +++ b/src/types/plugin-config-context.ts @@ -1,5 +1,4 @@ import { ok, Result, ResultAsync } from 'neverthrow'; -import { FileService } from '../infrastructure/file-service.js'; import { ApimaticConfigContext, ApimaticConfigWriteFailure } from './apimatic-config-context.js'; import { ApimaticConfigDocument, ConfigBlockName, findingClause } from './apimatic-config/document.js'; import { isPublished, languagesOf, PluginLanguageEntry, PluginLanguages } from './apimatic-config/languages-block.js'; @@ -13,7 +12,6 @@ import { PluginMetadata } from './plugin/plugin-config.js'; import { PortalLanguages } from './portal/portal-languages.js'; -import { SOURCE_DIRECTORY_NAME } from './project-layout.js'; import { SemVersion } from './publish/version.js'; import { isAvailableLanguage, Language } from './sdk/generate.js'; @@ -147,7 +145,6 @@ const withMetadata = ( export class PluginConfigContext { private readonly configContext: ApimaticConfigContext; - private readonly fileService = new FileService(); constructor(private readonly sourceDirectory: DirectoryPath) { this.configContext = new ApimaticConfigContext(this.sourceDirectory); @@ -206,25 +203,15 @@ export class PluginConfigContext { return await this.merge((document) => document.with('languages', recorded(document, languages, keepsRecord))); } - /** - * The `src/` to upload: a copy whose `languages` names exactly what the plugin covers. The - * user's file keeps the published entries this run leaves out; the service reads the copy. - */ - public async stageUpload( - into: DirectoryPath, - languages: readonly Language[] - ): Promise> { - const staged = into.join(SOURCE_DIRECTORY_NAME); - await this.fileService.copyDirectoryContents(this.sourceDirectory, staged); - - const config = new ApimaticConfigContext(staged); - const covered = await config.merge(OWNED_BLOCKS, (document) => + /** Only for the build's copy: `languages` comes to name just what the plugin covers, dropping published entries. */ + public async coverOnly(languages: readonly Language[]): Promise> { + const covered = await this.configContext.merge(OWNED_BLOCKS, (document) => document.with('languages', recorded(document, languages, isUnavailableLanguage)) ); // The merge writes nothing when it changes nothing, so a mark on the copy can outlive it — // and the service reads this file with a parser that will not look past one. - return await covered.asyncAndThen(() => new ResultAsync(config.removeByteOrderMark())).map(() => staged); + return await covered.asyncAndThen(() => new ResultAsync(this.configContext.removeByteOrderMark())); } private async merge( diff --git a/src/types/plugin/generation-problem.ts b/src/types/plugin/generation-problem.ts index f5dd3ccde..6f2b6c105 100644 --- a/src/types/plugin/generation-problem.ts +++ b/src/types/plugin/generation-problem.ts @@ -2,7 +2,6 @@ import { ServiceError } from '../../infrastructure/service-error.js'; import { FileProblem } from '../file/file-problem.js'; import { PluginConfigWriteFailure } from '../plugin-config-context.js'; -export type PluginGenerationProblem = - | FileProblem - | { kind: 'configNotPrepared'; failure: PluginConfigWriteFailure } - | { kind: 'generationFailed'; error: ServiceError }; +export type PluginBuildProblem = FileProblem | { kind: 'configNotPrepared'; failure: PluginConfigWriteFailure }; + +export type PluginGenerationProblem = PluginBuildProblem | { kind: 'generationFailed'; error: ServiceError }; diff --git a/src/types/project-context.ts b/src/types/project-context.ts index 11339846a..dadff7452 100644 --- a/src/types/project-context.ts +++ b/src/types/project-context.ts @@ -1,5 +1,6 @@ import { err, ok, Result } from 'neverthrow'; import { FileService } from '../infrastructure/file-service.js'; +import { errorMessage } from '../utils/error-utils.js'; import { ApimaticConfigContext } from './apimatic-config-context.js'; import { BuildConfig } from './build/build.js'; import { DirectoryPath } from './file/directoryPath.js'; @@ -7,8 +8,14 @@ import { FileName } from './file/fileName.js'; import { FilePath } from './file/filePath.js'; import { FileProblem } from './file/file-problem.js'; import { PluginConfigContext } from './plugin-config-context.js'; +import { PluginBuildProblem } from './plugin/generation-problem.js'; import { PortalSourceContext } from './portal-source-context.js'; -import { OUTPUT_DIRECTORY_NAMES, SOURCE_DIRECTORY_NAME, SPEC_DIRECTORY_NAME } from './project-layout.js'; +import { + LOCAL_ONLY_DIRECTORY_NAMES, + OUTPUT_DIRECTORY_NAMES, + SOURCE_DIRECTORY_NAME, + SPEC_DIRECTORY_NAME +} from './project-layout.js'; import { Language } from './sdk/generate.js'; import { SdkContext } from './sdk-context.js'; import { SpecContext } from './spec-context.js'; @@ -113,16 +120,30 @@ export class ProjectContext { ); } - public async srcDirZip( + public async buildZip( tempDirectory: DirectoryPath, packageSettingsDirectory?: DirectoryPath ): Promise> { - const staged = tempDirectory.join(SOURCE_DIRECTORY_NAME); - await this.fileService.copyDirectoryContents(this.source, staged); - if (packageSettingsDirectory) { - await this.fileService.copyDirectoryContents(packageSettingsDirectory, staged.join('package-settings')); + const staged = await this.stagedBuild(tempDirectory, packageSettingsDirectory); + if (staged.isErr()) { + return err(staged.error); } - return await new TempContext(tempDirectory).zipBuild(staged); + return await new TempContext(tempDirectory).zip(staged.value); + } + + public async pluginBuildZip( + tempDirectory: DirectoryPath, + languages: readonly Language[] + ): Promise> { + const staged = await this.stagedBuild(tempDirectory); + if (staged.isErr()) { + return err(staged.error); + } + const covered = await new PluginConfigContext(staged.value).coverOnly(languages); + if (covered.isErr()) { + return err({ kind: 'configNotPrepared', failure: covered.error }); + } + return await new TempContext(tempDirectory).zip(staged.value); } public async isVersioned(): Promise { @@ -200,6 +221,25 @@ export class ProjectContext { return ok(undefined); } + // A copy that fails is reported as a zip that fails: either way, the build could not be packed. + private async stagedBuild( + tempDirectory: DirectoryPath, + packageSettingsDirectory?: DirectoryPath + ): Promise> { + const staged = tempDirectory.join(SOURCE_DIRECTORY_NAME); + const localOnly = LOCAL_ONLY_DIRECTORY_NAMES.map((name) => new FileName(name)); + try { + await this.fileService.copyDirectoryContentsFollowingLinks(this.source, staged, localOnly); + if (packageSettingsDirectory) { + const packageSettings = staged.join('package-settings'); + await this.fileService.copyDirectoryContentsFollowingLinks(packageSettingsDirectory, packageSettings); + } + return ok(staged); + } catch (error) { + return err({ kind: 'zipFailed', reason: errorMessage(error) }); + } + } + private reading(versionDirectory: DirectoryPath): ProjectContext { return new ProjectContext(this.projectDirectory, versionDirectory, versionDirectory.leafName()); } diff --git a/src/types/temp-context.ts b/src/types/temp-context.ts index fe0dc881a..2e0d6ba97 100644 --- a/src/types/temp-context.ts +++ b/src/types/temp-context.ts @@ -5,7 +5,6 @@ import { DirectoryPath } from './file/directoryPath.js'; import { FilePath } from './file/filePath.js'; import { FileName } from './file/fileName.js'; import { FileProblem } from './file/file-problem.js'; -import { LOCAL_ONLY_DIRECTORY_NAMES } from './project-layout.js'; import { randomUUID } from 'node:crypto'; export class TempContext { @@ -20,11 +19,9 @@ export class TempContext { } public async zip(directory: DirectoryPath): Promise> { - return await this.zipWithout(directory, []); - } - - public async zipBuild(directory: DirectoryPath): Promise> { - return await this.zipWithout(directory, LOCAL_ONLY_DIRECTORY_NAMES); + const tempFile = this.getTempFileName; + const archived = await this.zipService.archive(directory, tempFile); + return archived.map(() => tempFile).mapErr((reason): FileProblem => ({ kind: 'zipFailed', reason })); } public async save(portalStream: NodeJS.ReadableStream): Promise { @@ -32,13 +29,4 @@ export class TempContext { await this.fileService.writeFile(tempFile, portalStream); return tempFile; } - - private async zipWithout( - directory: DirectoryPath, - excluding: readonly string[] - ): Promise> { - const tempFile = this.getTempFileName; - const archived = await this.zipService.archive(directory, tempFile, excluding); - return archived.map(() => tempFile).mapErr((reason): FileProblem => ({ kind: 'zipFailed', reason })); - } } diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index dc5787fae..f44defff5 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -655,7 +655,7 @@ describe('PluginGenerateAction', () => { it('reports a source directory it could not zip, without generating', async () => { const generatePlugin = sinon.stub(PluginService.prototype, 'generatePlugin'); const problem: FileProblem = { kind: 'zipFailed', reason: 'EACCES: permission denied' }; - sinon.stub(TempContext.prototype, 'zipBuild').resolves(err(problem)); + sinon.stub(TempContext.prototype, 'zip').resolves(err(problem)); const pluginNotGenerated = sinon.stub(PluginGeneratePrompts.prototype, 'pluginNotGenerated'); expect((await execute()).isFailed()).to.be.true; diff --git a/test/actions/portal/prepare-project.test.ts b/test/actions/portal/prepare-project.test.ts index 61442ed45..cbb8aa5bb 100644 --- a/test/actions/portal/prepare-project.test.ts +++ b/test/actions/portal/prepare-project.test.ts @@ -73,7 +73,7 @@ describe('PreparePortalProjectAction', () => { it('reports a source it could not zip, without asking for the artifacts', async () => { const problem: FileProblem = { kind: 'zipFailed', reason: 'EBUSY: resource busy or locked' }; - sinon.stub(TempContext.prototype, 'zipBuild').resolves(err(problem)); + sinon.stub(TempContext.prototype, 'zip').resolves(err(problem)); const result = await execute(); diff --git a/test/actions/sdk/generate.test.ts b/test/actions/sdk/generate.test.ts index 64489b585..8fc68c54d 100644 --- a/test/actions/sdk/generate.test.ts +++ b/test/actions/sdk/generate.test.ts @@ -69,7 +69,7 @@ describe('GenerateAction (sdk)', () => { it('reports a source directory it could not zip, without generating', async () => { const problem: FileProblem = { kind: 'zipFailed', reason: 'EACCES: permission denied' }; - sinon.stub(TempContext.prototype, 'zipBuild').resolves(err(problem)); + sinon.stub(TempContext.prototype, 'zip').resolves(err(problem)); const srcDirNotZipped = sinon.stub(SdkGeneratePrompts.prototype, 'srcDirNotZipped'); expect((await execute()).isFailed()).to.be.true; diff --git a/test/actions/sdk/publish.test.ts b/test/actions/sdk/publish.test.ts index 286467ac5..aec005420 100644 --- a/test/actions/sdk/publish.test.ts +++ b/test/actions/sdk/publish.test.ts @@ -74,7 +74,8 @@ describe('SdkPublishAction', () => { it('reports an SDK it could not zip, without publishing', async () => { const problem: FileProblem = { kind: 'zipFailed', reason: 'EACCES: permission denied' }; - sinon.stub(TempContext.prototype, 'zip').resolves(err(problem)); + const zip = sinon.stub(TempContext.prototype, 'zip').callThrough(); + zip.onSecondCall().resolves(err(problem)); const publishSdkPackage = sinon.stub(PublishingApiService.prototype, 'publishSdkPackage'); const sdkNotZipped = sinon.stub(SdkPublishPrompts.prototype, 'sdkNotZipped'); diff --git a/test/infrastructure/file-service.test.ts b/test/infrastructure/file-service.test.ts index 67944017b..5bb2f8074 100644 --- a/test/infrastructure/file-service.test.ts +++ b/test/infrastructure/file-service.test.ts @@ -220,4 +220,78 @@ describe('FileService', () => { expect(missing._unsafeUnwrapErr()).to.contain('ENOENT'); }); }); + + describe('copyDirectoryContentsFollowingLinks', () => { + const fileService = new FileService(); + let root: DirectoryPath; + const inRoot = (...parts: string[]) => path.join(root.toString(), ...parts); + // Copied one level deeper than the source, so a relative link kept as a link would point at nothing. + const copy = () => root.join('elsewhere', 'copy'); + const folderLink = process.platform === 'win32' ? 'junction' : 'dir'; + + beforeEach(() => { + root = new DirectoryPath(fs.mkdtempSync(path.join(os.tmpdir(), 'file-service-'))); + fsExtra.outputFileSync(inRoot('shared', 'notes.md'), '# notes'); + fs.mkdirSync(inRoot('source')); + }); + + afterEach(() => { + fs.rmSync(root.toString(), { recursive: true, force: true }); + }); + + it('copies a linked folder as the folder it points to', async () => { + fs.symlinkSync(inRoot('shared'), inRoot('source', 'linked'), folderLink); + + await fileService.copyDirectoryContentsFollowingLinks(root.join('source'), copy()); + + expect(fs.lstatSync(copy().join('linked').toString()).isSymbolicLink()).to.be.false; + expect(fs.readFileSync(path.join(copy().toString(), 'linked', 'notes.md'), 'utf-8')).to.equal('# notes'); + }); + + it('copies a relative file link as the file it points to', async function () { + try { + fs.symlinkSync(path.join('..', 'shared', 'notes.md'), inRoot('source', 'notes.md'), 'file'); + } catch { + this.skip(); // Windows makes a file link only in Developer Mode or for an administrator. + } + + await fileService.copyDirectoryContentsFollowingLinks(root.join('source'), copy()); + + expect(fs.readFileSync(path.join(copy().toString(), 'notes.md'), 'utf-8')).to.equal('# notes'); + }); + + it('fails on a link to nothing only once every other entry is copied', async () => { + for (let file = 0; file < 400; file++) { + fsExtra.outputFileSync(inRoot('source', 'content', `${file}.md`), ''); + } + fs.symlinkSync(inRoot('missing'), inRoot('source', 'broken'), folderLink); + + let thrown: unknown; + try { + await fileService.copyDirectoryContentsFollowingLinks(root.join('source'), copy()); + } catch (error) { + thrown = error; + } + + expect(thrown).to.be.an('error'); + expect(fs.readdirSync(path.join(copy().toString(), 'content'))).to.have.length(400); + }); + + it('leaves out a top-level entry it is told to, in any case, and keeps a nested one of that name', async () => { + for (const file of ['Static/logo.png', 'content/static/page.md', 'spec/openapi.json']) { + fsExtra.outputFileSync(inRoot('source', file), ''); + } + + await fileService.copyDirectoryContentsFollowingLinks(root.join('source'), copy(), [new FileName('static')]); + + const copied = fs.readdirSync(copy().toString(), { recursive: true, encoding: 'utf-8' }); + expect(copied.map((entry) => entry.split(path.sep).join('/'))).to.have.members([ + 'content', + 'content/static', + 'content/static/page.md', + 'spec', + 'spec/openapi.json' + ]); + }); + }); }); diff --git a/test/infrastructure/zip-service.test.ts b/test/infrastructure/zip-service.test.ts index 9068bb8d3..959f18e31 100644 --- a/test/infrastructure/zip-service.test.ts +++ b/test/infrastructure/zip-service.test.ts @@ -43,27 +43,6 @@ describe('ZipService', () => { expect(entries).to.have.members(['openapi.json', 'paths/pets.json']); }); - it('leaves nothing out unless told to', async () => { - write('index.html', ''); - write(path.join('static', 'logo.png'), 'png'); - - expect((await new ZipService().archive(source, archive)).isOk()).to.be.true; - - const entries = new AdmZip(archive.toString()).getEntries().map((entry) => entry.entryName); - expect(entries).to.have.members(['index.html', 'static/logo.png']); - }); - - it('leaves out an entry it is told to, and keeps a nested folder with the same name', async () => { - write(path.join('spec', 'openapi.json'), '{}'); - write(path.join('static', 'images', 'logo.png'), 'png'); - write(path.join('content', 'static', 'page.md'), '# Page'); - - expect((await new ZipService().archive(source, archive, ['static'])).isOk()).to.be.true; - - const entries = new AdmZip(archive.toString()).getEntries().map((entry) => entry.entryName); - expect(entries).to.have.members(['spec/openapi.json', 'content/static/page.md']); - }); - it('reports a source directory that is not there', async () => { const archived = await new ZipService().archive(source.join('missing'), archive); diff --git a/test/types/project-context.test.ts b/test/types/project-context.test.ts index e71764572..2cc05f452 100644 --- a/test/types/project-context.test.ts +++ b/test/types/project-context.test.ts @@ -95,24 +95,36 @@ describe('ProjectContext', () => { const settings = new DirectoryPath(inProject('settings')); write('settings/package.json', '{}'); - const zip = (await project().srcDirZip(temp, settings))._unsafeUnwrap(); + const zip = (await project().buildZip(temp, settings))._unsafeUnwrap(); const entries = new AdmZip(zip.toString()).getEntries().map((entry) => entry.entryName); expect(entries).to.have.members(['spec/openapi.json', 'content/index.md', 'package-settings/package.json']); }); - // A folder named static is kept too: only a build upload leaves static/ out. + it('reports a source it could not copy for the generators as one it could not zip', async () => { + write('src/spec/openapi.json', '{}'); + fs.symlinkSync( + inProject('missing'), + inProject('src', 'broken'), + process.platform === 'win32' ? 'junction' : 'dir' + ); + + const problem = (await project().buildZip(new DirectoryPath(inProject('temp'))))._unsafeUnwrapErr(); + + expect(problem.kind).to.equal('zipFailed'); + expect(problem.reason).to.contain('ENOENT'); + }); + it('zips all of spec/ for validation, so a spec split across files keeps the ones it references', async () => { write('src/spec/openapi.yaml', 'openapi: 3.0.3'); write('src/spec/schemas/pet.yaml', 'type: object'); - write('src/spec/static/owner.yaml', 'type: object'); write('src/APIMATIC-BUILD.json', '{}'); fs.mkdirSync(inProject('temp')); const zip = (await project().specZip(new DirectoryPath(inProject('temp'))))._unsafeUnwrap(); const entries = new AdmZip(zip.toString()).getEntries().map((entry) => entry.entryName); - expect(entries).to.have.members(['openapi.yaml', 'schemas/pet.yaml', 'static/owner.yaml']); + expect(entries).to.have.members(['openapi.yaml', 'schemas/pet.yaml']); }); it('zips nothing when spec/ is empty, and names the directory it looked in', async () => {