From 570ca00d0aeacf2a5a986a52cef37a47d35a3aa7 Mon Sep 17 00:00:00 2001 From: Alexander Zaslonov Date: Thu, 27 Aug 2026 16:14:15 -0700 Subject: [PATCH] fix: reconcile stale extraction artifacts safely Closes #106 --- docs/commands/extract.md | 1 + src/cli/extract-command.ts | 93 ++++-- src/clients/artifact-store.ts | 120 +++++++ src/clients/iartifact-store.ts | 10 + src/services/api-extractor.ts | 70 ++-- src/services/extract-service.ts | 11 +- src/services/filter-service.ts | 49 +++ src/services/product-extractor.ts | 30 +- src/services/workspace-extractor.ts | 6 +- .../azure-devops/extract-pipeline.ts | 6 +- .../github-actions/extract-workflow.ts | 6 +- tests/unit/cli/extract-command.test.ts | 86 ++++- tests/unit/clients/artifact-store.test.ts | 312 +++++++++++++++++- .../services/api-product-extractor.test.ts | 45 ++- tests/unit/services/extract-service.test.ts | 149 +++++++++ tests/unit/services/filter-service.test.ts | 31 ++ .../unit/services/workspace-extractor.test.ts | 104 ++++++ .../azure-devops/extract-pipeline.test.ts | 1 + .../github-actions/extract-workflow.test.ts | 1 + 19 files changed, 1066 insertions(+), 65 deletions(-) diff --git a/docs/commands/extract.md b/docs/commands/extract.md index 6d837d9b..c4cce83e 100644 --- a/docs/commands/extract.md +++ b/docs/commands/extract.md @@ -66,6 +66,7 @@ apiops extract \ | `--output ` | string | `./apim-artifacts` | No | Output directory path | | `--filter ` | string | — | No | Filter configuration YAML file | | `--no-transitive` | boolean | false (transitive ON) | No | Disable transitive dependency inclusion | +| `--remove-stale` | boolean | false | No | Remove stale managed artifacts in the selected filter and workspace scope after a fully successful extraction | ### Global flags diff --git a/src/cli/extract-command.ts b/src/cli/extract-command.ts index 37e4519a..8a5d5daf 100644 --- a/src/cli/extract-command.ts +++ b/src/cli/extract-command.ts @@ -8,26 +8,46 @@ */ import { Command } from 'commander'; -import { ExtractConfig } from '../models/config.js'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { ExtractConfig, FilterConfig } from '../models/config.js'; import { ApimServiceContext } from '../models/types.js'; import { runExtraction, ExtractionResult } from '../services/extract-service.js'; +import { shouldReconcileResource } from '../services/filter-service.js'; import { loadFilterConfig } from '../lib/config-loader.js'; import { logger, parseLogLevel } from '../lib/logger.js'; import { ApimClient } from '../clients/apim-client.js'; import { ArtifactStore } from '../clients/artifact-store.js'; +import { IArtifactStore } from '../clients/iartifact-store.js'; import { getCloudConfig, buildArmBaseUrl } from '../lib/cloud-config.js'; +import { EXIT_FATAL, EXIT_SUCCESS } from '../lib/exit-codes.js'; /** * Interface for extract command options (from CLI flags). */ -interface ExtractOptions { +export interface ExtractOptions { resourceGroup: string; serviceName: string; output: string; filter?: string; transitive: boolean; + removeStale?: boolean; } +export interface ExtractCommandDependencies { + runExtraction: typeof runExtraction; + createClient: (authScope: string) => ApimClient; + createStore: () => IArtifactStore; + exit: (code: number) => void; +} + +const DEFAULT_DEPENDENCIES: ExtractCommandDependencies = { + runExtraction, + createClient: (authScope) => new ApimClient(authScope), + createStore: () => new ArtifactStore(), + exit: (code) => process.exit(code), +}; + /** * Create and return the extract command for Commander. */ @@ -39,6 +59,7 @@ export function createExtractCommand(): Command { .option('--output ', 'Output directory path', './apim-artifacts') .option('--filter ', 'Filter configuration YAML file') .option('--no-transitive', 'Disable transitive dependency inclusion') + .option('--remove-stale', 'Remove stale managed artifacts in the selected scope') .action(async (options: ExtractOptions, command: Command) => { const globalOpts = command.optsWithGlobals<{ logLevel?: string; @@ -57,7 +78,7 @@ export function createExtractCommand(): Command { /** * Execute the extract command. */ -async function executeExtract( +export async function executeExtract( options: ExtractOptions, globalOpts: { logLevel?: string; @@ -65,13 +86,15 @@ async function executeExtract( cloud?: string; format?: string; apiVersion?: string; - } + }, + dependencies: ExtractCommandDependencies = DEFAULT_DEPENDENCIES ): Promise { const subscriptionId = globalOpts.subscriptionId ?? process.env.AZURE_SUBSCRIPTION_ID; if (!subscriptionId) { logger.error('Subscription ID required: use --subscription-id or set AZURE_SUBSCRIPTION_ID'); - process.exit(2); + dependencies.exit(2); + return; } // Build service context @@ -92,30 +115,49 @@ async function executeExtract( }; // Load filter config if specified - let filterConfig; + let filterConfig: FilterConfig | undefined; if (options.filter) { filterConfig = await loadFilterConfig(options.filter); if (!filterConfig) { logger.error(`Filter file not found: ${options.filter}`); - process.exit(2); + dependencies.exit(2); + return; } } - // Build extract config - const extractConfig: ExtractConfig = { - service: context, - outputDir: options.output, - filter: filterConfig, - includeTransitive: options.transitive, - logLevel: parseLogLevel(globalOpts.logLevel ?? 'info'), - }; - // Create client and store - const client = new ApimClient(cloudConfig.authScope); - const store = new ArtifactStore(); - - // Run extraction - const result = await runExtraction(client, store, extractConfig); + const client = dependencies.createClient(cloudConfig.authScope); + const store = dependencies.createStore(); + + const outputDir = path.resolve(options.output); + const outputParent = path.dirname(outputDir); + const stagingPrefix = path.join(outputParent, `.${path.basename(outputDir)}.apiops-extract-`); + await fs.mkdir(outputParent, { recursive: true }); + const stagingDir = await fs.mkdtemp(stagingPrefix); + + let result: ExtractionResult; + try { + const extractConfig: ExtractConfig = { + service: context, + outputDir: stagingDir, + filter: filterConfig, + includeTransitive: options.transitive, + logLevel: parseLogLevel(globalOpts.logLevel ?? 'info'), + }; + + result = await dependencies.runExtraction(client, store, extractConfig); + + if (result.exitCode !== EXIT_FATAL) { + await store.commitStagedExtraction( + stagingDir, + outputDir, + descriptor => shouldReconcileResource(descriptor, filterConfig), + shouldRemoveStaleArtifacts(result, options.removeStale ?? false) + ); + } + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } // Output results if (globalOpts.format === 'json') { @@ -124,7 +166,14 @@ async function executeExtract( outputText(result); } - process.exit(result.exitCode); + dependencies.exit(result.exitCode); +} + +export function shouldRemoveStaleArtifacts( + result: Pick, + requested: boolean +): boolean { + return requested && result.exitCode === EXIT_SUCCESS; } /** diff --git a/src/clients/artifact-store.ts b/src/clients/artifact-store.ts index 6c10cc8b..ccfd4599 100644 --- a/src/clients/artifact-store.ts +++ b/src/clients/artifact-store.ts @@ -17,10 +17,18 @@ import { buildSpecificationFilePath, buildAssociationFilePath, parseArtifactPath, + parseArtifactChangePath, } from '../lib/resource-path.js'; import { logger } from '../lib/logger.js'; +interface CommitFileSystem { + rename: typeof fs.rename; + rm: typeof fs.rm; +} + export class ArtifactStore implements IArtifactStore { + constructor(private readonly commitFileSystem: CommitFileSystem = fs) {} + async writeResource( baseDir: string, descriptor: ResourceDescriptor, @@ -218,6 +226,73 @@ export class ArtifactStore implements IArtifactStore { } } + async commitStagedExtraction( + stagingDir: string, + baseDir: string, + isInScope: (descriptor: ResourceDescriptor) => boolean, + removeStale: boolean + ): Promise { + const parentDir = path.dirname(baseDir); + const candidateDir = await fs.mkdtemp(path.join(parentDir, `.${path.basename(baseDir)}.candidate-`)); + const backupDir = path.join(parentDir, `.${path.basename(baseDir)}.backup-${path.basename(candidateDir)}`); + const baseExists = await this.fileExists(baseDir); + + try { + if (baseExists) { + await fs.cp(baseDir, candidateDir, { recursive: true, force: true }); + } + + if (removeStale && baseExists) { + const existingFiles = await this.listFiles(baseDir); + for (const existingFile of existingFiles) { + const descriptor = this.getOwningDescriptor(baseDir, existingFile); + if (!descriptor || !isInScope(descriptor)) { + continue; + } + + const relativePath = path.relative(baseDir, existingFile); + const stagedFile = path.join(stagingDir, relativePath); + if (!(await this.fileExists(stagedFile))) { + await this.commitFileSystem.rm(path.join(candidateDir, relativePath), { force: true }); + } + } + } + + await fs.cp(stagingDir, candidateDir, { recursive: true, force: true }); + + if (baseExists) { + await this.commitFileSystem.rename(baseDir, backupDir); + } + try { + await this.commitFileSystem.rename(candidateDir, baseDir); + } catch (commitError) { + if (baseExists) { + try { + await this.commitFileSystem.rename(backupDir, baseDir); + } catch (restoreError) { + throw new AggregateError( + [commitError, restoreError], + `Failed to commit staged extraction and restore the original output. Backup remains at ${backupDir}`, + { cause: restoreError } + ); + } + } + throw commitError; + } + + if (baseExists) { + try { + await this.commitFileSystem.rm(backupDir, { recursive: true, force: true }); + } catch (error) { + logger.warn(`Committed extraction but could not remove backup ${backupDir}: ${(error as Error).message}`); + } + } + logger.debug(`Committed staged extraction from ${stagingDir} to ${baseDir}`); + } finally { + await this.commitFileSystem.rm(candidateDir, { recursive: true, force: true }); + } + } + private async ensureDirectory(dirPath: string): Promise { try { await fs.mkdir(dirPath, { recursive: true }); @@ -251,6 +326,51 @@ export class ArtifactStore implements IArtifactStore { } } + private async listFiles(directory: string): Promise { + const files: string[] = []; + const entries = await fs.readdir(directory, { withFileTypes: true }); + + for (const entry of entries) { + const entryPath = path.join(directory, entry.name); + if (entry.isDirectory()) { + files.push(...await this.listFiles(entryPath)); + } else if (entry.isFile()) { + files.push(entryPath); + } + } + + return files; + } + + private getOwningDescriptor(baseDir: string, filePath: string): ResourceDescriptor | undefined { + const descriptor = parseArtifactChangePath(baseDir, filePath); + if (descriptor) { + return descriptor; + } + + const fileName = path.basename(filePath); + if (!['apis.json', 'groups.json', 'tags.json'].includes(fileName)) { + return undefined; + } + + const parentFile = fileName === 'apis.json' && path.basename(path.dirname(path.dirname(filePath))) === 'gateways' + ? 'gatewayInformation.json' + : 'productInformation.json'; + return parseArtifactPath(baseDir, path.join(path.dirname(filePath), parentFile)); + } + + private async fileExists(filePath: string): Promise { + try { + await fs.access(filePath); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return false; + } + throw error; + } + } + private descriptorEquals(a: ResourceDescriptor, b: ResourceDescriptor): boolean { return ( a.type === b.type && diff --git a/src/clients/iartifact-store.ts b/src/clients/iartifact-store.ts index 95c8f9cb..304f8617 100644 --- a/src/clients/iartifact-store.ts +++ b/src/clients/iartifact-store.ts @@ -88,4 +88,14 @@ export interface IArtifactStore { baseDir: string, descriptor: ResourceDescriptor ): Promise; + + /** + * Merge staged extraction output and optionally remove stale managed files. + */ + commitStagedExtraction( + stagingDir: string, + baseDir: string, + isInScope: (descriptor: ResourceDescriptor) => boolean, + removeStale: boolean + ): Promise; } diff --git a/src/services/api-extractor.ts b/src/services/api-extractor.ts index 961e3a21..4716aeef 100644 --- a/src/services/api-extractor.ts +++ b/src/services/api-extractor.ts @@ -25,6 +25,7 @@ import { isWorkspaceScope, extractNameFromLink } from '../lib/workspace-link.js' */ export interface ApiExtractionResult { apiName: string; + errorCount: number; revisions: ExtractedResource[]; specification: boolean; operations: ExtractedResource[]; @@ -58,6 +59,7 @@ export async function extractApiResources( const apiName = getNamePart(apiDescriptor.nameParts, 0); const result: ApiExtractionResult = { apiName, + errorCount: 0, revisions: [], specification: false, operations: [], @@ -78,6 +80,7 @@ export async function extractApiResources( result.revisions = await extractApiRevisions( client, store, context, apiName, outputDir, filter, workspace ); + result.errorCount += result.revisions.filter((revision) => revision.status === 'error').length; // Extract API schemas FIRST. For synthetic GraphQL APIs the SDL lives in an // ApiSchema resource; by extracting schemas first we can detect that case @@ -88,12 +91,15 @@ export async function extractApiResources( outputDir, filter, apiDescriptor, workspace ); result.schemas = schemaResult.extracted; + result.errorCount += schemaResult.errorCount; // Extract API specification (uses already-extracted schemas to detect // synthetic GraphQL without a second list call). - result.specification = await extractApiSpecification( + const specificationResult = await extractApiSpecification( client, store, context, apiDescriptor, apiJson, outputDir, result.schemas ); + result.specification = specificationResult.extracted; + result.errorCount += specificationResult.errorCount; // Extract API policy const policyContent = await extractApiPolicy( @@ -110,6 +116,7 @@ export async function extractApiResources( result.operations = opsResult.operations; result.operationPolicies = opsResult.operationPolicies; result.policies.push(...opsResult.policies); + result.errorCount += opsResult.errorCount; // Extract API tags // In workspace scope, the classic `apis/{api}/tags` endpoint returns HTTP 500. @@ -123,6 +130,7 @@ export async function extractApiResources( outputDir, filter, apiDescriptor, workspace ); result.tags = tagsResult.extracted; + result.errorCount += tagsResult.errorCount; } // Extract API diagnostics @@ -131,6 +139,7 @@ export async function extractApiResources( outputDir, filter, apiDescriptor, workspace ); result.diagnostics = diagResult.extracted; + result.errorCount += diagResult.errorCount; // Extract API releases const releaseResult = await extractResourceType( @@ -138,6 +147,7 @@ export async function extractApiResources( outputDir, filter, apiDescriptor, workspace ); result.releases = releaseResult.extracted; + result.errorCount += releaseResult.errorCount; // Extract API tag descriptions (not supported in workspace scope) if (!workspace) { @@ -146,12 +156,15 @@ export async function extractApiResources( outputDir, filter, apiDescriptor, workspace ); result.tagDescriptions = tagDescResult.extracted; + result.errorCount += tagDescResult.errorCount; } // Extract API wiki - result.wiki = await extractApiWiki( + const wikiResult = await extractApiWiki( client, store, context, apiDescriptor, outputDir ); + result.wiki = wikiResult.extracted; + result.errorCount += wikiResult.errorCount; // Extract GraphQL resolvers and their policies const resolverResult = await extractGraphQLResolvers( @@ -160,6 +173,7 @@ export async function extractApiResources( result.resolvers = resolverResult.resolvers; result.resolverPolicies = resolverResult.resolverPolicies; result.policies.push(...resolverResult.policies); + result.errorCount += resolverResult.errorCount; return result; } @@ -208,6 +222,13 @@ async function extractApiRevisions( await store.writeResource(outputDir, descriptor, revJson); results.push({ descriptor, json: revJson, status: 'success' }); logger.info(`Extracted revision ${buildResourceLabel(descriptor)}`); + } else { + results.push({ + descriptor, + json: {}, + status: 'error', + error: `Revision disappeared during extraction: ${buildResourceLabel(descriptor)}`, + }); } } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); @@ -221,7 +242,14 @@ async function extractApiRevisions( } } } catch (error) { - logger.warn(`Failed to list revisions for API "${apiName}": ${(error as Error).message}`); + const errorMessage = error instanceof Error ? error.message : String(error); + logger.warn(`Failed to list revisions for API "${apiName}": ${errorMessage}`); + results.push({ + descriptor: { type: ResourceType.Api, nameParts: [`${apiName};rev=?`], workspace }, + json: {}, + status: 'error', + error: errorMessage, + }); } return results; @@ -244,29 +272,29 @@ async function extractApiSpecification( apiJson: Record, outputDir: string, extractedSchemas: ExtractedResource[] -): Promise { +): Promise<{ extracted: boolean; errorCount: number }> { const properties = apiJson.properties as Record | undefined; const apiType = properties?.type as string | undefined; if (apiType?.toLowerCase() === 'websocket') { logger.debug(`OpenAPI does not apply to WebSocket APIs`); - return false; + return { extracted: false, errorCount: 0 }; } if (apiType?.toLowerCase() === 'mcp') { logger.debug(`Skipping spec export for MCP API "${getNamePart(apiDescriptor.nameParts, 0)}" — MCP APIs use the Model Context Protocol endpoint, not OpenAPI`); - return false; + return { extracted: false, errorCount: 0 }; } if (apiType?.toLowerCase() === 'a2a') { logger.debug(`Skipping spec export for A2A API "${getNamePart(apiDescriptor.nameParts, 0)}" — A2A APIs use JSON-RPC + agent card endpoints, not OpenAPI`); - return false; + return { extracted: false, errorCount: 0 }; } if (apiType?.toLowerCase() === 'graphql' && hasGraphQLSchema(extractedSchemas)) { logger.debug( `Skipping spec export for synthetic GraphQL API "${getNamePart(apiDescriptor.nameParts, 0)}" — schema is captured via ApiSchema` ); - return false; + return { extracted: false, errorCount: 0 }; } // REST APIs natively imported as Swagger 2.0 expose auto-generated schemas @@ -282,7 +310,7 @@ async function extractApiSpecification( const spec = await client.getApiSpecification(context, getNamePart(apiDescriptor.nameParts, 0), apiType, specDialect); if (!spec) { logger.debug(`No specification found for API "${getNamePart(apiDescriptor.nameParts, 0)}"`); - return false; + return { extracted: false, errorCount: 0 }; } await store.writeContent( @@ -294,11 +322,11 @@ async function extractApiSpecification( ); logger.info(`Extracted specification ${buildResourceLabel(apiDescriptor)} (${spec.format})`); - return true; + return { extracted: true, errorCount: 0 }; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); logger.warn(`Failed to extract specification ${buildResourceLabel(apiDescriptor)}: ${errorMessage}`); - return false; + return { extracted: false, errorCount: 1 }; } } @@ -393,6 +421,7 @@ async function extractApiOperations( operations: ExtractedResource[]; operationPolicies: ExtractedResource[]; policies: string[]; + errorCount: number; }> { const operations: ExtractedResource[] = []; const operationPolicies: ExtractedResource[] = []; @@ -434,7 +463,7 @@ async function extractApiOperations( } } - return { operations, operationPolicies, policies }; + return { operations, operationPolicies, policies, errorCount: opsResult.errorCount }; } /** @@ -446,7 +475,7 @@ async function extractApiWiki( context: ApimServiceContext, apiDescriptor: ResourceDescriptor, outputDir: string -): Promise { +): Promise<{ extracted: boolean; errorCount: number }> { const wikiDescriptor: ResourceDescriptor = { type: ResourceType.ApiWiki, nameParts: [...apiDescriptor.nameParts], @@ -456,7 +485,7 @@ async function extractApiWiki( try { const wikiJson = await client.getResource(context, wikiDescriptor); if (!wikiJson) { - return false; + return { extracted: false, errorCount: 0 }; } // Extract markdown content from wiki JSON @@ -474,11 +503,11 @@ async function extractApiWiki( await store.writeResource(outputDir, wikiDescriptor, wikiJson); } logger.info(`Extracted ${buildResourceLabel(wikiDescriptor)}`); - return true; + return { extracted: true, errorCount: 0 }; } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); logger.debug(`No wiki ${buildResourceLabel(wikiDescriptor)}: ${errorMessage}`); - return false; + return { extracted: false, errorCount: 1 }; } } @@ -498,6 +527,7 @@ async function extractGraphQLResolvers( resolvers: ExtractedResource[]; resolverPolicies: ExtractedResource[]; policies: string[]; + errorCount: number; }> { const resolvers: ExtractedResource[] = []; const resolverPolicies: ExtractedResource[] = []; @@ -507,7 +537,7 @@ async function extractGraphQLResolvers( const properties = apiJson.properties as Record | undefined; const apiType = properties?.type as string | undefined; if (apiType?.toLowerCase() !== 'graphql') { - return { resolvers, resolverPolicies, policies }; + return { resolvers, resolverPolicies, policies, errorCount: 0 }; } // Extract resolvers @@ -546,7 +576,7 @@ async function extractGraphQLResolvers( } } - return { resolvers, resolverPolicies, policies }; + return { resolvers, resolverPolicies, policies, errorCount: resolverResult.errorCount }; } /** @@ -596,8 +626,7 @@ export async function extractWorkspaceApiTags( for await (const linkJson of client.listResources(context, ResourceType.ApiTag, tagDescriptor)) { const apiName = extractNameFromLink(linkJson, linkProperty); if (!apiName) { - logger.warn(`Failed to extract API name from tag "${tagName}" apiLink response`); - continue; + throw new Error(`Failed to extract API name from tag "${tagName}" apiLink response`); } // Only create ApiTag artifacts for APIs that were extracted @@ -626,6 +655,7 @@ export async function extractWorkspaceApiTags( } } catch (error) { logger.warn(`Failed to list apiLinks for tag "${tagName}": ${(error as Error).message}`); + throw error; } } diff --git a/src/services/extract-service.ts b/src/services/extract-service.ts index e48eb20a..30225d2d 100644 --- a/src/services/extract-service.ts +++ b/src/services/extract-service.ts @@ -192,6 +192,12 @@ async function extractTier( result.extractedDescriptors.push(res.descriptor); } } + } else if (taskResult.status === 'rejected') { + const errorMessage = taskResult.reason instanceof Error + ? taskResult.reason.message + : String(taskResult.reason); + logger.error(`Resource type extraction failed: ${errorMessage}`); + result.totalErrors++; } } } @@ -252,6 +258,7 @@ async function extractApiSubResources( const { apiName, apiResult } = taskResult.value; result.apiResults.push(apiResult); + result.totalErrors += apiResult.errorCount; // Count sub-resources const subCount = @@ -356,6 +363,7 @@ async function extractProductSubResources( const { productName, prodResult } = taskResult.value; result.productResults.push(prodResult); + result.totalErrors += prodResult.errorCount; // Count sub-resources (associations + policy + wiki + tags) const subCount = @@ -451,13 +459,14 @@ async function extractGatewayAssociations( } } + await store.writeAssociation(outputDir, gw.descriptor, 'apis', apiNames); if (apiNames.length > 0) { - await store.writeAssociation(outputDir, gw.descriptor, 'apis', apiNames); result.totalExtracted++; logger.info(`Extracted ${apiNames.length} API associations for gateway "${getNamePart(gw.descriptor.nameParts, 0)}"`); } } catch (error) { logger.warn(`Failed to extract API associations for gateway "${getNamePart(gw.descriptor.nameParts, 0)}": ${(error as Error).message}`); + result.totalErrors++; } } } diff --git a/src/services/filter-service.ts b/src/services/filter-service.ts index e3a82962..0d7a5dc6 100644 --- a/src/services/filter-service.ts +++ b/src/services/filter-service.ts @@ -123,6 +123,55 @@ export function shouldIncludeResource( return true; } +export function shouldReconcileResource( + descriptor: ResourceDescriptor, + filter?: FilterConfig +): boolean { + if (!descriptor.workspace) { + return shouldIncludeResource(descriptor, filter); + } + + const workspaceDescriptor: ResourceDescriptor = { + type: ResourceType.Workspace, + nameParts: [descriptor.workspace], + }; + if (!shouldIncludeResource(workspaceDescriptor, filter)) { + return false; + } + + const workspaceFilter = getWorkspaceFilter(descriptor.workspace, filter); + return shouldIncludeResource(descriptor, workspaceFilter); +} + +function getWorkspaceFilter( + workspaceName: string, + filter?: FilterConfig +): FilterConfig | undefined { + const matchingKey = Object.keys(filter?.workspaceSubFilters ?? {}).find( + key => key.toLowerCase() === workspaceName.toLowerCase() + ); + if (!matchingKey) { + return undefined; + } + + const subFilter = filter?.workspaceSubFilters?.[matchingKey]; + return subFilter ? { + apis: subFilter.apis, + apiSubFilters: subFilter.apiSubFilters, + backends: subFilter.backends, + diagnostics: subFilter.diagnostics, + groups: subFilter.groups, + loggers: subFilter.loggers, + namedValues: subFilter.namedValues, + policyFragments: subFilter.policyFragments, + products: subFilter.products, + schemas: subFilter.schemas, + subscriptions: subFilter.subscriptions, + tags: subFilter.tags, + versionSets: subFilter.versionSets, + } : undefined; +} + /** * Get the fixed singleton name for a resource type from its ARM path. * E.g., ServicePolicy → "policy" diff --git a/src/services/product-extractor.ts b/src/services/product-extractor.ts index 6d2aa0a9..97118722 100644 --- a/src/services/product-extractor.ts +++ b/src/services/product-extractor.ts @@ -21,6 +21,7 @@ import { isWorkspaceScope, extractNameFromLink, extractLinkTarget } from '../lib */ export interface ProductExtractionResult { productName: string; + errorCount: number; apis: string[]; groups: string[]; policy: string | undefined; @@ -45,6 +46,7 @@ export async function extractProductResources( const productName = getNamePart(productDescriptor.nameParts, 0); const result: ProductExtractionResult = { productName, + errorCount: 0, apis: [], groups: [], policy: undefined, @@ -55,12 +57,12 @@ export async function extractProductResources( // Extract product API associations result.apis = await extractProductAssociations( - client, store, context, productDescriptor, outputDir, 'apis' + client, store, context, productDescriptor, outputDir, 'apis', () => result.errorCount++ ); // Extract product group associations result.groups = await extractProductAssociations( - client, store, context, productDescriptor, outputDir, 'groups' + client, store, context, productDescriptor, outputDir, 'groups', () => result.errorCount++ ); // Extract product policy @@ -73,7 +75,7 @@ export async function extractProductResources( // Extract product wiki result.wiki = await extractProductWiki( - client, store, context, productDescriptor, outputDir + client, store, context, productDescriptor, outputDir, () => result.errorCount++ ); // Extract product tags - store as tags.json association file. @@ -82,7 +84,7 @@ export async function extractProductResources( // in the workspace extractor. if (!isWorkspaceScope(context)) { result.tags = await extractProductTags( - client, store, context, productDescriptor, outputDir + client, store, context, productDescriptor, outputDir, () => result.errorCount++ ); } @@ -98,7 +100,8 @@ async function extractProductAssociations( context: ApimServiceContext, productDescriptor: ResourceDescriptor, outputDir: string, - associationType: 'apis' | 'groups' + associationType: 'apis' | 'groups', + onError: () => void ): Promise { const entries: AssociationEntry[] = []; const resourceType = associationType === 'apis' @@ -121,6 +124,7 @@ async function extractProductAssociations( const target = extractLinkTarget(json, linkProperty); if (!target) { logger.warn(`Failed to extract ${associationType} link target from workspace link response`); + onError(); continue; } entries.push({ name: target.name, scope: target.scope }); @@ -129,6 +133,7 @@ async function extractProductAssociations( } } catch (error) { logger.warn(`Failed to extract ${associationType} association name: ${(error as Error).message}`); + onError(); } } @@ -139,6 +144,7 @@ async function extractProductAssociations( } } catch (error) { logger.warn(`Failed to list ${associationType} for product "${getNamePart(productDescriptor.nameParts, 0)}": ${(error as Error).message}`); + onError(); } return entries.map(entry => entry.name); @@ -153,7 +159,8 @@ async function extractProductTags( store: IArtifactStore, context: ApimServiceContext, productDescriptor: ResourceDescriptor, - outputDir: string + outputDir: string, + onError: () => void ): Promise { const entries: AssociationEntry[] = []; const workspaceScoped = isWorkspaceScope(context); @@ -168,6 +175,7 @@ async function extractProductTags( const target = extractLinkTarget(json, linkProperty); if (!target) { logger.warn('Failed to extract tag name from workspace link response'); + onError(); continue; } entries.push({ name: target.name, scope: target.scope }); @@ -176,6 +184,7 @@ async function extractProductTags( } } catch (error) { logger.warn(`Failed to extract tag name: ${(error as Error).message}`); + onError(); } } @@ -186,6 +195,7 @@ async function extractProductTags( } } catch (error) { logger.warn(`Failed to list tags for product "${getNamePart(productDescriptor.nameParts, 0)}": ${(error as Error).message}`); + onError(); } return entries.map(entry => entry.name); @@ -233,7 +243,8 @@ async function extractProductWiki( store: IArtifactStore, context: ApimServiceContext, productDescriptor: ResourceDescriptor, - outputDir: string + outputDir: string, + onError: () => void ): Promise { try { const wikiDescriptor: ResourceDescriptor = { @@ -252,6 +263,7 @@ async function extractProductWiki( return true; } catch (error) { logger.debug(`No wiki for product "${getNamePart(productDescriptor.nameParts, 0)}": ${(error as Error).message}`); + onError(); return false; } } @@ -302,8 +314,7 @@ export async function extractWorkspaceProductTags( for await (const linkJson of client.listResources(context, ResourceType.ProductTag, tagDescriptor)) { const productName = extractNameFromLink(linkJson, linkProperty); if (!productName) { - logger.warn(`Failed to extract product name from tag "${tagName}" productLink response`); - continue; + throw new Error(`Failed to extract product name from tag "${tagName}" productLink response`); } if (!productTagsMap.has(productName)) { @@ -313,6 +324,7 @@ export async function extractWorkspaceProductTags( } } catch (error) { logger.warn(`Failed to list productLinks for tag "${tagName}": ${(error as Error).message}`); + throw error; } } diff --git a/src/services/workspace-extractor.ts b/src/services/workspace-extractor.ts index b3aa3d9e..221ef1e8 100644 --- a/src/services/workspace-extractor.ts +++ b/src/services/workspace-extractor.ts @@ -101,6 +101,7 @@ export async function extractWorkspaces( // before workspace-scoped children (named values, APIs, products, etc.). const wsDescriptor = { type: ResourceType.Workspace, nameParts: [wsName] }; const wsJson = await client.getResource(context, wsDescriptor); + const workspaceContainerError = wsJson ? 0 : 1; if (!wsJson) { logger.error( `Workspace container "${wsName}" was discovered but could not be read. Continuing with workspace child extraction.` @@ -113,6 +114,7 @@ export async function extractWorkspaces( client, store, context, wsName, outputDir, resolveWorkspaceFilter(wsName, filter) ); + wsResult.errorCount += workspaceContainerError; results.push(wsResult); } @@ -173,6 +175,7 @@ async function extractWorkspace( resourceCount += apiResult.operations.length + apiResult.tags.length + apiResult.schemas.length; + errorCount += apiResult.errorCount; } catch (error) { logger.warn(`Failed to extract API details for workspace "${workspaceName}": ${(error as Error).message}`); errorCount++; @@ -185,11 +188,12 @@ async function extractWorkspace( extractedProducts = result.extracted.filter((r) => r.status === 'success'); for (const product of extractedProducts) { try { - await extractProductResources( + const productResult = await extractProductResources( client, store, wsContext, product.descriptor, outputDir, filter, workspaceName ); resourceCount++; + errorCount += productResult.errorCount; } catch (error) { logger.warn(`Failed to extract product details for workspace "${workspaceName}": ${(error as Error).message}`); errorCount++; diff --git a/src/templates/azure-devops/extract-pipeline.ts b/src/templates/azure-devops/extract-pipeline.ts index 46737120..8071b649 100644 --- a/src/templates/azure-devops/extract-pipeline.ts +++ b/src/templates/azure-devops/extract-pipeline.ts @@ -101,7 +101,8 @@ steps: --resource-group "$(APIM_RESOURCE_GROUP)" \\ --service-name "$(APIM_SERVICE_NAME)" \\ --output ${config.artifactDir} \\ - --subscription-id "$(AZURE_SUBSCRIPTION_ID)" + --subscription-id "$(AZURE_SUBSCRIPTION_ID)" \\ + --remove-stale - task: AzureCLI@2 displayName: 'Run APIM Extract (With Configuration)' @@ -116,7 +117,8 @@ steps: --service-name "$(APIM_SERVICE_NAME)" \\ --output ${config.artifactDir} \\ --filter configuration.extractor.yaml \\ - --subscription-id "$(AZURE_SUBSCRIPTION_ID)" + --subscription-id "$(AZURE_SUBSCRIPTION_ID)" \\ + --remove-stale - task: PublishPipelineArtifact@1 displayName: 'Publish artifacts' diff --git a/src/templates/github-actions/extract-workflow.ts b/src/templates/github-actions/extract-workflow.ts index 78395349..8df7312a 100644 --- a/src/templates/github-actions/extract-workflow.ts +++ b/src/templates/github-actions/extract-workflow.ts @@ -86,7 +86,8 @@ jobs: --subscription-id \${{ secrets.AZURE_SUBSCRIPTION_ID }} \\ --resource-group \${{ env.APIM_RESOURCE_GROUP }} \\ --service-name \${{ env.APIM_SERVICE_NAME }} \\ - --output ${config.artifactDir} + --output ${config.artifactDir} \\ + --remove-stale - name: Run APIM Extract (With Configuration) if: \${{ github.event.inputs.CONFIGURATION_YAML_PATH != 'Extract All APIs' }} @@ -96,7 +97,8 @@ jobs: --resource-group \${{ env.APIM_RESOURCE_GROUP }} \\ --service-name \${{ env.APIM_SERVICE_NAME }} \\ --output ${config.artifactDir} \\ - --filter configuration.extractor.yaml + --filter configuration.extractor.yaml \\ + --remove-stale - name: Upload artifacts uses: actions/upload-artifact@v4 diff --git a/tests/unit/cli/extract-command.test.ts b/tests/unit/cli/extract-command.test.ts index bc3e98a9..4ac38221 100644 --- a/tests/unit/cli/extract-command.test.ts +++ b/tests/unit/cli/extract-command.test.ts @@ -4,8 +4,15 @@ * Unit tests for Extract command CLI registration */ -import { describe, it, expect } from 'vitest'; -import { createExtractCommand } from '../../../src/cli/extract-command.js'; +import { describe, it, expect, vi } from 'vitest'; +import { + createExtractCommand, + executeExtract, + shouldRemoveStaleArtifacts, +} from '../../../src/cli/extract-command.js'; +import { ExtractionResult } from '../../../src/services/extract-service.js'; +import { ApimClient } from '../../../src/clients/apim-client.js'; +import { IArtifactStore } from '../../../src/clients/iartifact-store.js'; describe('extract-command', () => { describe('createExtractCommand', () => { @@ -52,9 +59,84 @@ describe('extract-command', () => { expect(noTransOpt).toBeDefined(); }); + it('should have an opt-in --remove-stale option', () => { + const cmd = createExtractCommand(); + const option = cmd.options.find((candidate) => candidate.long === '--remove-stale'); + expect(option).toBeDefined(); + expect(option?.defaultValue).toBeUndefined(); + }); + it('should have a description', () => { const cmd = createExtractCommand(); expect(cmd.description()).toBeTruthy(); }); }); + + describe('shouldRemoveStaleArtifacts', () => { + it('should remove stale artifacts only after a successful extraction', () => { + expect(shouldRemoveStaleArtifacts({ exitCode: 0 }, true)).toBe(true); + expect(shouldRemoveStaleArtifacts({ exitCode: 0 }, false)).toBe(false); + expect(shouldRemoveStaleArtifacts({ exitCode: 1 }, true)).toBe(false); + expect(shouldRemoveStaleArtifacts({ exitCode: 2 }, true)).toBe(false); + }); + }); + + describe('executeExtract cleanup gating', () => { + it.each([ + { exitCode: 0, removeStale: true, expectedCommit: true, expectedCleanup: true }, + { exitCode: 0, removeStale: false, expectedCommit: true, expectedCleanup: false }, + { exitCode: 1, removeStale: true, expectedCommit: true, expectedCleanup: false }, + { exitCode: 2, removeStale: true, expectedCommit: false, expectedCleanup: false }, + ])('should gate stale cleanup for exit code $exitCode', async ({ + exitCode, + removeStale, + expectedCommit, + expectedCleanup, + }) => { + const commitStagedExtraction = vi.fn().mockResolvedValue(undefined); + const store = { commitStagedExtraction } as unknown as IArtifactStore; + const result: ExtractionResult = { + totalExtracted: exitCode === 2 ? 0 : 1, + totalErrors: exitCode === 0 ? 0 : 1, + typeResults: [], + apiResults: [], + productResults: [], + workspaceResults: [], + extractedDescriptors: [], + collectedPolicies: new Map(), + exitCode, + }; + const exit = vi.fn(); + const stdout = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + + try { + await executeExtract( + { + resourceGroup: 'rg-1', + serviceName: 'apim-1', + output: `test-output-${exitCode}`, + transitive: true, + removeStale, + }, + { subscriptionId: 'sub-1', format: 'json' }, + { + runExtraction: vi.fn().mockResolvedValue(result), + createClient: () => ({}) as ApimClient, + createStore: () => store, + exit, + } + ); + + if (expectedCommit) { + expect(commitStagedExtraction).toHaveBeenCalledOnce(); + expect(commitStagedExtraction.mock.calls[0]?.[3]).toBe(expectedCleanup); + } else { + expect(commitStagedExtraction).not.toHaveBeenCalled(); + } + expect(exit).toHaveBeenCalledWith(exitCode); + } finally { + stdout.mockRestore(); + } + }); + }); }); diff --git a/tests/unit/clients/artifact-store.test.ts b/tests/unit/clients/artifact-store.test.ts index 8abec384..25989e63 100644 --- a/tests/unit/clients/artifact-store.test.ts +++ b/tests/unit/clients/artifact-store.test.ts @@ -1,12 +1,19 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { ArtifactStore } from '../../../src/clients/artifact-store.js'; import { ResourceDescriptor } from '../../../src/models/types.js'; import { ResourceType } from '../../../src/models/resource-types.js'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import * as os from 'node:os'; +import { + buildArtifactFilePath, + buildAssociationFilePath, + buildPolicyFilePath, + buildSpecificationFilePath, +} from '../../../src/lib/resource-path.js'; +import { shouldReconcileResource } from '../../../src/services/filter-service.js'; describe('ArtifactStore', () => { let store: ArtifactStore; @@ -305,6 +312,309 @@ describe('ArtifactStore', () => { }); }); + describe('commitStagedExtraction', () => { + it('should remove stale in-scope artifacts and preserve excluded or unmanaged files', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const currentApi = { type: ResourceType.Api, nameParts: ['current-api'] }; + const staleApi = { type: ResourceType.Api, nameParts: ['stale-api'] }; + const excludedApi = { type: ResourceType.Api, nameParts: ['excluded-api'] }; + + try { + await store.writeResource(tmpDir, currentApi, { version: 'old' }); + await store.writeResource(tmpDir, staleApi, { version: 'stale' }); + await store.writeResource(tmpDir, excludedApi, { version: 'excluded' }); + await fs.writeFile(path.join(tmpDir, 'notes.md'), 'keep me', 'utf-8'); + await store.writeResource(stagingDir, currentApi, { version: 'new' }); + + await store.commitStagedExtraction( + stagingDir, + tmpDir, + descriptor => descriptor.nameParts[0] !== 'excluded-api', + true + ); + + await expect(store.readResource(tmpDir, currentApi)).resolves.toEqual({ version: 'new' }); + await expect(store.readResource(tmpDir, staleApi)).resolves.toBeUndefined(); + await expect(store.readResource(tmpDir, excludedApi)).resolves.toEqual({ version: 'excluded' }); + await expect(fs.readFile(path.join(tmpDir, 'notes.md'), 'utf-8')).resolves.toBe('keep me'); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it('should preserve stale artifacts when cleanup is disabled', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const staleApi = { type: ResourceType.Api, nameParts: ['stale-api'] }; + + try { + await store.writeResource(tmpDir, staleApi, { version: 'stale' }); + + await store.commitStagedExtraction(stagingDir, tmpDir, () => true, false); + + await expect(store.readResource(tmpDir, staleApi)).resolves.toEqual({ version: 'stale' }); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it('should remove a stale gateway association file omitted from staging', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const gateway = { type: ResourceType.Gateway, nameParts: ['gateway-1'] }; + + try { + await store.writeResource(tmpDir, gateway, { name: 'gateway-1' }); + await store.writeAssociation(tmpDir, gateway, 'apis', ['old-api']); + await store.writeResource(stagingDir, gateway, { name: 'gateway-1' }); + + await store.commitStagedExtraction(stagingDir, tmpDir, () => true, true); + + await expect(fs.access(buildAssociationFilePath(tmpDir, gateway, 'apis'))).rejects.toThrow(); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it('should remove stale API specifications and wikis', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const api = { type: ResourceType.Api, nameParts: ['api-1'] }; + const apiWiki = { type: ResourceType.ApiWiki, nameParts: ['api-1'] }; + const product = { type: ResourceType.Product, nameParts: ['product-1'] }; + const productWiki = { type: ResourceType.ProductWiki, nameParts: ['product-1'] }; + + try { + await store.writeResource(tmpDir, api, { name: 'api-1' }); + await store.writeContent(tmpDir, api, 'openapi: 3.0.0', 'specification', 'yaml'); + await store.writeResource(tmpDir, apiWiki, { name: 'default' }); + await store.writeResource(tmpDir, product, { name: 'product-1' }); + await store.writeResource(tmpDir, productWiki, { name: 'default' }); + await store.writeResource(stagingDir, api, { name: 'api-1' }); + await store.writeResource(stagingDir, product, { name: 'product-1' }); + + await store.commitStagedExtraction(stagingDir, tmpDir, () => true, true); + + await expect(store.readContent(tmpDir, api, 'specification')).resolves.toBeUndefined(); + await expect(store.readResource(tmpDir, apiWiki)).resolves.toBeUndefined(); + await expect(store.readResource(tmpDir, productWiki)).resolves.toBeUndefined(); + await expect(fs.access(buildSpecificationFilePath(tmpDir, api, 'yaml'))).rejects.toThrow(); + await expect(fs.access(buildArtifactFilePath(tmpDir, apiWiki)!)).rejects.toThrow(); + await expect(fs.access(buildArtifactFilePath(tmpDir, productWiki)!)).rejects.toThrow(); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it.each([undefined, 'workspace-1'])( + 'should remove every managed supplemental artifact in workspace %s', + async (workspace) => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const api = { type: ResourceType.Api, nameParts: ['api-1'], workspace }; + const apiPolicy = { type: ResourceType.ApiPolicy, nameParts: ['api-1'], workspace }; + const apiWiki = { type: ResourceType.ApiWiki, nameParts: ['api-1'], workspace }; + const product = { type: ResourceType.Product, nameParts: ['product-1'], workspace }; + const productPolicy = { type: ResourceType.ProductPolicy, nameParts: ['product-1'], workspace }; + const productWiki = { type: ResourceType.ProductWiki, nameParts: ['product-1'], workspace }; + const operationPolicy = { type: ResourceType.ApiOperationPolicy, nameParts: ['api-1', 'op-1'], workspace }; + const resolverPolicy = { type: ResourceType.GraphQLResolverPolicy, nameParts: ['api-1', 'resolver-1'], workspace }; + const gateway = { type: ResourceType.Gateway, nameParts: ['gateway-1'], workspace }; + const formats = ['yaml', 'json', 'graphql', 'wsdl', 'wadl'] as const; + + try { + await store.writeResource(tmpDir, api, { name: 'api-1' }); + await store.writeResource(tmpDir, apiWiki, { name: 'default' }); + await store.writeContent(tmpDir, apiPolicy, '', 'policy'); + await store.writeContent(tmpDir, operationPolicy, '', 'policy'); + await store.writeContent(tmpDir, resolverPolicy, '', 'policy'); + if (!workspace) { + await store.writeContent( + tmpDir, + { type: ResourceType.ServicePolicy, nameParts: [] }, + '', + 'policy' + ); + } + for (const format of formats) { + await store.writeContent(tmpDir, api, `spec-${format}`, 'specification', format); + } + await store.writeResource(tmpDir, product, { name: 'product-1' }); + await store.writeResource(tmpDir, productWiki, { name: 'default' }); + await store.writeContent(tmpDir, productPolicy, '', 'policy'); + await store.writeAssociation(tmpDir, product, 'apis', ['api-1']); + await store.writeAssociation(tmpDir, product, 'groups', ['group-1']); + await store.writeAssociation(tmpDir, product, 'tags', ['tag-1']); + await store.writeResource(tmpDir, gateway, { name: 'gateway-1' }); + await store.writeAssociation(tmpDir, gateway, 'apis', ['api-1']); + + await store.writeResource(stagingDir, api, { name: 'api-1' }); + await store.writeResource(stagingDir, product, { name: 'product-1' }); + await store.writeResource(stagingDir, gateway, { name: 'gateway-1' }); + + await store.commitStagedExtraction(stagingDir, tmpDir, () => true, true); + + for (const format of formats) { + await expect(fs.access(buildSpecificationFilePath(tmpDir, api, format))).rejects.toThrow(); + } + await expect(fs.access(buildPolicyFilePath(tmpDir, apiPolicy))).rejects.toThrow(); + await expect(fs.access(buildPolicyFilePath(tmpDir, operationPolicy))).rejects.toThrow(); + await expect(fs.access(buildPolicyFilePath(tmpDir, resolverPolicy))).rejects.toThrow(); + if (!workspace) { + await expect(fs.access(buildPolicyFilePath( + tmpDir, + { type: ResourceType.ServicePolicy, nameParts: [] } + ))).rejects.toThrow(); + } + await expect(fs.access(buildArtifactFilePath(tmpDir, apiWiki)!)).rejects.toThrow(); + await expect(fs.access(buildPolicyFilePath(tmpDir, productPolicy))).rejects.toThrow(); + await expect(fs.access(buildArtifactFilePath(tmpDir, productWiki)!)).rejects.toThrow(); + for (const association of ['apis', 'groups', 'tags'] as const) { + await expect(fs.access(buildAssociationFilePath(tmpDir, product, association))).rejects.toThrow(); + } + await expect(fs.access(buildAssociationFilePath(tmpDir, gateway, 'apis'))).rejects.toThrow(); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + } + ); + + it('should reconcile only selected workspace sub-filter artifacts', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const selected = { type: ResourceType.Api, nameParts: ['selected'], workspace: 'ws-1' }; + const excluded = { type: ResourceType.Api, nameParts: ['excluded'], workspace: 'ws-1' }; + const otherWorkspace = { type: ResourceType.Api, nameParts: ['selected'], workspace: 'ws-2' }; + const filter = { + workspaces: ['ws-1'], + workspaceSubFilters: { 'ws-1': { apis: ['selected'] } }, + }; + + try { + await store.writeResource(tmpDir, selected, { name: 'selected' }); + await store.writeResource(tmpDir, excluded, { name: 'excluded' }); + await store.writeResource(tmpDir, otherWorkspace, { name: 'selected' }); + + await store.commitStagedExtraction( + stagingDir, + tmpDir, + descriptor => shouldReconcileResource(descriptor, filter), + true + ); + + await expect(store.readResource(tmpDir, selected)).resolves.toBeUndefined(); + await expect(store.readResource(tmpDir, excluded)).resolves.toEqual({ name: 'excluded' }); + await expect(store.readResource(tmpDir, otherWorkspace)).resolves.toEqual({ name: 'selected' }); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it('should restore the original output when committing the candidate fails', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const descriptor = { type: ResourceType.Api, nameParts: ['api-1'] }; + const rename = vi.fn(async (oldPath, newPath) => { + if (path.basename(String(oldPath)).startsWith(`.${path.basename(tmpDir)}.candidate-`)) { + throw new Error('candidate rename failed'); + } + await fs.rename(oldPath, newPath); + }); + const failingStore = new ArtifactStore({ rename, rm: fs.rm }); + + try { + await store.writeResource(tmpDir, descriptor, { version: 'old' }); + await store.writeResource(stagingDir, descriptor, { version: 'new' }); + + await expect( + failingStore.commitStagedExtraction(stagingDir, tmpDir, () => true, true) + ).rejects.toThrow('candidate rename failed'); + await expect(store.readResource(tmpDir, descriptor)).resolves.toEqual({ version: 'old' }); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it('should preserve the original output when creating the backup fails', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const descriptor = { type: ResourceType.Api, nameParts: ['api-1'] }; + const rename = vi.fn().mockRejectedValue(new Error('backup rename failed')); + const failingStore = new ArtifactStore({ rename, rm: fs.rm }); + + try { + await store.writeResource(tmpDir, descriptor, { version: 'old' }); + await store.writeResource(stagingDir, descriptor, { version: 'new' }); + + await expect( + failingStore.commitStagedExtraction(stagingDir, tmpDir, () => true, true) + ).rejects.toThrow('backup rename failed'); + await expect(store.readResource(tmpDir, descriptor)).resolves.toEqual({ version: 'old' }); + expect(rename).toHaveBeenCalledOnce(); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it('should preserve the backup and report both errors when rollback fails', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const descriptor = { type: ResourceType.Api, nameParts: ['api-1'] }; + let renameCall = 0; + const rename = vi.fn(async (oldPath, newPath) => { + renameCall++; + if (renameCall > 1) { + throw new Error(renameCall === 2 ? 'candidate rename failed' : 'restore failed'); + } + await fs.rename(oldPath, newPath); + }); + const failingStore = new ArtifactStore({ rename, rm: fs.rm }); + + try { + await store.writeResource(tmpDir, descriptor, { version: 'old' }); + await store.writeResource(stagingDir, descriptor, { version: 'new' }); + + const error = await failingStore + .commitStagedExtraction(stagingDir, tmpDir, () => true, true) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(AggregateError); + expect((error as AggregateError).errors).toHaveLength(2); + expect((error as Error).message).toContain('Backup remains at'); + const backups = (await fs.readdir(path.dirname(tmpDir))) + .filter((entry) => entry.startsWith(`.${path.basename(tmpDir)}.backup-`)); + expect(backups).toHaveLength(1); + await expect(store.readResource(path.join(path.dirname(tmpDir), backups[0]), descriptor)) + .resolves.toEqual({ version: 'old' }); + await fs.rm(path.join(path.dirname(tmpDir), backups[0]), { recursive: true, force: true }); + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + + it('should succeed when removing the committed backup fails', async () => { + const stagingDir = await fs.mkdtemp(path.join(os.tmpdir(), 'apiops-staging-')); + const descriptor = { type: ResourceType.Api, nameParts: ['api-1'] }; + const remove = vi.fn(async (target, options) => { + if (String(target).includes('.backup-')) { + throw new Error('backup cleanup failed'); + } + await fs.rm(target, options); + }); + const resilientStore = new ArtifactStore({ rename: fs.rename, rm: remove }); + + try { + await store.writeResource(tmpDir, descriptor, { version: 'old' }); + await store.writeResource(stagingDir, descriptor, { version: 'new' }); + + await expect( + resilientStore.commitStagedExtraction(stagingDir, tmpDir, () => true, true) + ).resolves.toBeUndefined(); + await expect(store.readResource(tmpDir, descriptor)).resolves.toEqual({ version: 'new' }); + const backups = (await fs.readdir(path.dirname(tmpDir))) + .filter((entry) => entry.startsWith(`.${path.basename(tmpDir)}.backup-`)); + expect(backups).toHaveLength(1); + for (const backup of backups) { + await fs.rm(path.join(path.dirname(tmpDir), backup), { recursive: true, force: true }); + } + } finally { + await fs.rm(stagingDir, { recursive: true, force: true }); + } + }); + }); + describe('UTF-8 encoding', () => { it('should handle unicode resource names', async () => { const descriptor: ResourceDescriptor = { diff --git a/tests/unit/services/api-product-extractor.test.ts b/tests/unit/services/api-product-extractor.test.ts index 86dad408..6e5d7b17 100644 --- a/tests/unit/services/api-product-extractor.test.ts +++ b/tests/unit/services/api-product-extractor.test.ts @@ -653,6 +653,7 @@ describe('api-extractor', () => { ); expect(result.wiki).toBe(false); + expect(result.errorCount).toBe(1); }); it('should return specification=false and not throw when getApiSpecification throws', async () => { @@ -673,6 +674,7 @@ describe('api-extractor', () => { ); expect(result.specification).toBe(false); + expect(result.errorCount).toBe(1); }); it('should record error status when an individual revision getResource throws', async () => { @@ -701,7 +703,7 @@ describe('api-extractor', () => { expect(result.revisions[0]?.status).toBe('error'); }); - it('should return empty revisions and not throw when listApiRevisions throws', async () => { + it('should report an error revision and not throw when listApiRevisions throws', async () => { const client = createMockClient({ // eslint-disable-next-line require-yield listApiRevisions: async function* () { @@ -719,10 +721,12 @@ describe('api-extractor', () => { '/output' ); - expect(result.revisions).toHaveLength(0); + expect(result.revisions).toHaveLength(1); + expect(result.revisions[0]?.status).toBe('error'); + expect(result.errorCount).toBe(1); }); - it('should skip revision when getResource returns undefined (revision not found)', async () => { + it('should report an error when a listed revision disappears before fetch', async () => { const client = createMockClient({ listApiRevisions: async function* () { yield { apiRevision: '2' }; @@ -739,8 +743,10 @@ describe('api-extractor', () => { '/output' ); - // Revision listed but getResource returned undefined — not added to results - expect(result.revisions).toHaveLength(0); + expect(result.revisions).toHaveLength(1); + expect(result.revisions[0]?.status).toBe('error'); + expect(result.errorCount).toBe(1); + expect(store.writeResource).not.toHaveBeenCalled(); }); @@ -1186,6 +1192,35 @@ describe('product-extractor', () => { ); expect(result.tags).toEqual([]); + expect(result.errorCount).toBe(1); + }); + + it('counts failed product associations and wiki extraction', async () => { + const client = createMockClient(); + // eslint-disable-next-line require-yield + client.listResources = async function* (_ctx, type) { + if (type === ResourceType.ProductApi || type === ResourceType.ProductGroup) { + throw new Error(`${type} list failed`); + } + }; + client.getResource = vi.fn().mockImplementation(async (_ctx: unknown, desc: ResourceDescriptor) => { + if (desc.type === ResourceType.ProductWiki) { + throw new Error('product wiki failed'); + } + return undefined; + }); + const store = createMockStore(); + + const result = await extractProductResources( + client, store, testContext, + { type: ResourceType.Product, nameParts: ['starter'] }, + '/output' + ); + + expect(result.apis).toEqual([]); + expect(result.groups).toEqual([]); + expect(result.wiki).toBe(false); + expect(result.errorCount).toBe(3); }); }); }); diff --git a/tests/unit/services/extract-service.test.ts b/tests/unit/services/extract-service.test.ts index 3aeccc42..f846aff0 100644 --- a/tests/unit/services/extract-service.test.ts +++ b/tests/unit/services/extract-service.test.ts @@ -366,6 +366,155 @@ describe('extract-service', () => { expect(result.exitCode).toBeGreaterThanOrEqual(1); }); + it('should return partial when API specification extraction fails', async () => { + const client = createMockClient({ + [ResourceType.Api]: [{ name: 'echo-api', properties: {} }], + }); + client.getApiSpecification = vi.fn().mockRejectedValue(new Error('spec export failed')); + const store = createMockStore(); + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + includeTransitive: false, + logLevel: LogLevel.INFO, + }); + + expect(result.totalExtracted).toBeGreaterThan(0); + expect(result.totalErrors).toBeGreaterThan(0); + expect(result.exitCode).toBe(1); + }); + + it('should count a rejected resource type task as an extraction error', async () => { + const client = createMockClient({ + [ResourceType.Tag]: [{ name: 'tag-1', properties: {} }], + }); + const store = createMockStore(); + store.writeResource = vi.fn().mockRejectedValue(new Error('disk unavailable')); + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + includeTransitive: false, + logLevel: LogLevel.INFO, + }); + + expect(result.totalErrors).toBeGreaterThan(0); + expect(result.exitCode).toBe(2); + }); + + it('should return partial when a workspace container cannot be read', async () => { + const client = createMockClient({ + [ResourceType.Tag]: [{ name: 'tag-1', properties: {} }], + }); + const store = createMockStore(); + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + filter: { workspaces: ['ws-1'] }, + includeTransitive: false, + logLevel: LogLevel.INFO, + }); + + expect(result.totalExtracted).toBeGreaterThan(0); + expect(result.totalErrors).toBeGreaterThan(0); + expect(result.exitCode).toBe(1); + }); + + it('should aggregate API wiki and product supplemental failures', async () => { + const client = createMockClient({ + [ResourceType.Api]: [{ name: 'api-1', properties: {} }], + [ResourceType.Product]: [{ name: 'product-1', properties: {} }], + }); + const originalListResources = client.listResources; + client.listResources = async function* (context, type, parent) { + if ([ResourceType.ProductApi, ResourceType.ProductGroup, ResourceType.ProductTag].includes(type)) { + throw new Error(`${type} list failed`); + } + yield* originalListResources(context, type, parent); + }; + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor) => { + if (descriptor.type === ResourceType.ApiWiki || descriptor.type === ResourceType.ProductWiki) { + throw new Error(`${descriptor.type} read failed`); + } + return undefined; + }); + const store = createMockStore(); + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + includeTransitive: false, + logLevel: LogLevel.INFO, + }); + + expect(result.totalErrors).toBeGreaterThanOrEqual(5); + expect(result.exitCode).toBe(1); + }); + + it('should aggregate workspace tag-link failures', async () => { + const client = createMockClient({ + [ResourceType.Tag]: [{ name: 'service-tag', properties: {} }], + }); + const originalListResources = client.listResources; + client.listResources = async function* (context, type, parent) { + if (context.baseUrl.includes('/workspaces/')) { + if (type === ResourceType.Tag) yield { name: 'tag-1', properties: {} }; + if (type === ResourceType.Api) yield { name: 'api-1', properties: {} }; + if (type === ResourceType.ApiTag) throw new Error('apiLinks failed'); + return; + } + yield* originalListResources(context, type, parent); + }; + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor) => + descriptor.type === ResourceType.Workspace ? { name: 'ws-1', properties: {} } : undefined + ); + const store = createMockStore(); + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + filter: { workspaces: ['ws-1'] }, + includeTransitive: false, + logLevel: LogLevel.INFO, + }); + + expect(result.totalErrors).toBeGreaterThan(0); + expect(result.exitCode).toBe(1); + }); + + it('should aggregate workspace product tag-link failures', async () => { + const client = createMockClient({ + [ResourceType.Tag]: [{ name: 'service-tag', properties: {} }], + }); + const originalListResources = client.listResources; + client.listResources = async function* (context, type, parent) { + if (context.baseUrl.includes('/workspaces/')) { + if (type === ResourceType.Tag) yield { name: 'tag-1', properties: {} }; + if (type === ResourceType.Product) yield { name: 'product-1', properties: {} }; + if (type === ResourceType.ProductTag) throw new Error('productLinks failed'); + return; + } + yield* originalListResources(context, type, parent); + }; + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor) => + descriptor.type === ResourceType.Workspace ? { name: 'ws-1', properties: {} } : undefined + ); + const store = createMockStore(); + + const result = await runExtraction(client, store, { + service: testContext, + outputDir: '/output', + filter: { workspaces: ['ws-1'] }, + includeTransitive: false, + logLevel: LogLevel.INFO, + }); + + expect(result.totalErrors).toBeGreaterThan(0); + expect(result.exitCode).toBe(1); + }); + it('should handle empty APIM instance', async () => { const client = createMockClient({}); const store = createMockStore(); diff --git a/tests/unit/services/filter-service.test.ts b/tests/unit/services/filter-service.test.ts index be4ce7c1..169b9277 100644 --- a/tests/unit/services/filter-service.test.ts +++ b/tests/unit/services/filter-service.test.ts @@ -10,6 +10,7 @@ import { ResourceDescriptor } from '../../../src/models/types.js'; import { FilterConfig } from '../../../src/models/config.js'; import { shouldIncludeResource, + shouldReconcileResource, filterResources, extractRootApiName, isWildcardPattern, @@ -18,6 +19,36 @@ import { } from '../../../src/services/filter-service.js'; describe('filter-service', () => { + describe('shouldReconcileResource', () => { + const workspaceNamedValue: ResourceDescriptor = { + type: ResourceType.NamedValue, + nameParts: ['workspace-value'], + workspace: 'team-a', + }; + + it('should preserve resources from workspaces outside the selected scope', () => { + const filter: FilterConfig = { workspaces: ['team-b'] }; + + expect(shouldReconcileResource(workspaceNamedValue, filter)).toBe(false); + }); + + it('should use the workspace sub-filter instead of service-level filters', () => { + const filter: FilterConfig = { + namedValues: [], + workspaces: ['team-a'], + workspaceSubFilters: { + 'team-a': { namedValues: ['workspace-value'] }, + }, + }; + + expect(shouldReconcileResource(workspaceNamedValue, filter)).toBe(true); + expect(shouldReconcileResource( + { ...workspaceNamedValue, nameParts: ['excluded-value'] }, + filter + )).toBe(false); + }); + }); + describe('shouldIncludeResource', () => { it('should include all resources when no filter is provided', () => { const descriptor: ResourceDescriptor = { diff --git a/tests/unit/services/workspace-extractor.test.ts b/tests/unit/services/workspace-extractor.test.ts index c1aff29f..c3ecc01a 100644 --- a/tests/unit/services/workspace-extractor.test.ts +++ b/tests/unit/services/workspace-extractor.test.ts @@ -163,6 +163,110 @@ describe('workspace-extractor', () => { expect(results[0]?.errorCount).toBeGreaterThan(0); }); + it('should count a missing workspace container as an error', async () => { + const client = createMockClient(); + const store = createMockStore(); + + const results = await extractWorkspaces( + client, store, testContext, '/output', { workspaces: ['ws-1'] } + ); + + expect(results[0]?.errorCount).toBe(1); + }); + + it('should count workspace API supplemental failures', async () => { + const client = createMockClient(); + client.listResources = async function* (_ctx, type) { + if (type === ResourceType.Api) { + yield { name: 'ws-api', properties: {} }; + } + }; + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor) => + descriptor.type === ResourceType.Workspace ? { name: 'ws-1', properties: {} } : undefined + ); + client.getApiSpecification = vi.fn().mockRejectedValue(new Error('spec export failed')); + const store = createMockStore(); + + const results = await extractWorkspaces( + client, store, testContext, '/output', { workspaces: ['ws-1'] } + ); + + expect(results[0]?.errorCount).toBe(1); + }); + + it('should count workspace product supplemental failures', async () => { + const client = createMockClient(); + client.listResources = async function* (_ctx, type) { + if (type === ResourceType.Product) { + yield { name: 'ws-product', properties: {} }; + } else if (type === ResourceType.ProductApi || type === ResourceType.ProductGroup) { + throw new Error(`${type} list failed`); + } + }; + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor) => { + if (descriptor.type === ResourceType.Workspace) { + return { name: 'ws-1', properties: {} }; + } + if (descriptor.type === ResourceType.ProductWiki) { + throw new Error('wiki failed'); + } + return undefined; + }); + const store = createMockStore(); + + const results = await extractWorkspaces( + client, store, testContext, '/output', { workspaces: ['ws-1'] } + ); + + expect(results[0]?.errorCount).toBe(3); + }); + + it('should count workspace tag-link failures', async () => { + const client = createMockClient(); + client.listResources = async function* (_ctx, type) { + if (type === ResourceType.Tag) { + yield { name: 'tag-1', properties: {} }; + } else if (type === ResourceType.Api) { + yield { name: 'api-1', properties: {} }; + } else if (type === ResourceType.ApiTag) { + throw new Error('apiLinks failed'); + } + }; + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor) => + descriptor.type === ResourceType.Workspace ? { name: 'ws-1', properties: {} } : undefined + ); + const store = createMockStore(); + + const results = await extractWorkspaces( + client, store, testContext, '/output', { workspaces: ['ws-1'] } + ); + + expect(results[0]?.errorCount).toBe(1); + }); + + it('should count workspace product tag-link failures', async () => { + const client = createMockClient(); + client.listResources = async function* (_ctx, type) { + if (type === ResourceType.Tag) { + yield { name: 'tag-1', properties: {} }; + } else if (type === ResourceType.Product) { + yield { name: 'product-1', properties: {} }; + } else if (type === ResourceType.ProductTag) { + throw new Error('productLinks failed'); + } + }; + client.getResource = vi.fn().mockImplementation(async (_ctx, descriptor) => + descriptor.type === ResourceType.Workspace ? { name: 'ws-1', properties: {} } : undefined + ); + const store = createMockStore(); + + const results = await extractWorkspaces( + client, store, testContext, '/output', { workspaces: ['ws-1'] } + ); + + expect(results[0]?.errorCount).toBe(1); + }); + it('should extract API sub-resources within workspace', async () => { const client = createMockClient(); // Return APIs when listing in workspace context diff --git a/tests/unit/templates/azure-devops/extract-pipeline.test.ts b/tests/unit/templates/azure-devops/extract-pipeline.test.ts index 100ad106..16fe2448 100644 --- a/tests/unit/templates/azure-devops/extract-pipeline.test.ts +++ b/tests/unit/templates/azure-devops/extract-pipeline.test.ts @@ -124,6 +124,7 @@ describe('azure-devops/extract-pipeline', () => { const pipeline = generateExtractPipeline(defaultConfig); expect(pipeline).toContain('npm ci'); expect(pipeline).toContain('npx @azure-tools/apiops-cli extract'); + expect(pipeline).toContain('--remove-stale'); }); }); }); diff --git a/tests/unit/templates/github-actions/extract-workflow.test.ts b/tests/unit/templates/github-actions/extract-workflow.test.ts index a55db35c..a70bb8c7 100644 --- a/tests/unit/templates/github-actions/extract-workflow.test.ts +++ b/tests/unit/templates/github-actions/extract-workflow.test.ts @@ -121,6 +121,7 @@ describe('github-actions/extract-workflow', () => { const workflow = generateExtractWorkflow({ artifactDir: './apim-artifacts' }); expect(workflow).toContain('npm install'); expect(workflow).toContain('npx apiops extract'); + expect(workflow).toContain('--remove-stale'); }); }); });