From be4b70696e4051ecebea500db66a4cfe320e403f Mon Sep 17 00:00:00 2001 From: David de Boer Date: Fri, 6 Mar 2026 19:58:50 +0100 Subject: [PATCH] refactor(sparql-importer): replace import result classes with discriminated union - Replace ImportSuccessful, ImportFailed, NotSupported classes with interfaces carrying a `type` discriminant ('successful', 'failed', 'not-supported') - Add ImportResult union type for the Importer.import() return type - Update sparql-qlever Importer to return plain objects instead of class instances - Replace instanceof checks with type-property checks in pipeline's ImportResolver and all tests - Change value imports to type-only imports where possible --- .../src/distribution/importResolver.ts | 5 +- .../test/distribution/importResolver.test.ts | 99 +++++++++---------- .../pipeline/test/distribution/report.test.ts | 8 +- .../distribution/resolveDistributions.test.ts | 38 +++---- packages/sparql-importer/src/index.ts | 37 ++++--- packages/sparql-qlever/src/importer.ts | 20 ++-- packages/sparql-qlever/test/importer.test.ts | 3 +- 7 files changed, 103 insertions(+), 107 deletions(-) diff --git a/packages/pipeline/src/distribution/importResolver.ts b/packages/pipeline/src/distribution/importResolver.ts index 662206cb..4e57c5c2 100644 --- a/packages/pipeline/src/distribution/importResolver.ts +++ b/packages/pipeline/src/distribution/importResolver.ts @@ -1,6 +1,5 @@ import { type Dataset, Distribution } from '@lde/dataset'; import type { Importer } from '@lde/sparql-importer'; -import { ImportFailed, ImportSuccessful } from '@lde/sparql-importer'; import type { SparqlServer } from '@lde/sparql-server'; import { type DistributionResolver, @@ -66,7 +65,7 @@ export class ImportResolver implements DistributionResolver { const importStart = Date.now(); const importResult = await this.options.importer.import(dataset); - if (importResult instanceof ImportSuccessful) { + if (importResult.type === 'successful') { await this.options.server.start(); const distribution = Distribution.sparql( @@ -87,7 +86,7 @@ export class ImportResolver implements DistributionResolver { dataset, 'No SPARQL endpoint or importable data dump available', probeResults, - importResult instanceof ImportFailed ? importResult : undefined, + importResult.type === 'failed' ? importResult : undefined, ); } diff --git a/packages/pipeline/test/distribution/importResolver.test.ts b/packages/pipeline/test/distribution/importResolver.test.ts index c35f0a1a..89e73eae 100644 --- a/packages/pipeline/test/distribution/importResolver.test.ts +++ b/packages/pipeline/test/distribution/importResolver.test.ts @@ -6,7 +6,7 @@ import { type DistributionResolver, } from '../../src/distribution/index.js'; import { Dataset, Distribution } from '@lde/dataset'; -import { ImportSuccessful, ImportFailed } from '@lde/sparql-importer'; +import type { ImportSuccessful, ImportFailed } from '@lde/sparql-importer'; import type { SparqlServer } from '@lde/sparql-server'; import { describe, it, expect, vi } from 'vitest'; @@ -78,14 +78,13 @@ describe('ImportResolver', () => { ); const mockImporter = { - import: vi - .fn() - .mockResolvedValue( - new ImportSuccessful( - Distribution.sparql(new URL('http://localhost:7878/sparql')), - 'test-graph', - ), + import: vi.fn().mockResolvedValue({ + type: 'successful', + distribution: Distribution.sparql( + new URL('http://localhost:7878/sparql'), ), + identifier: 'test-graph', + } satisfies ImportSuccessful), }; const server = makeServer(); @@ -118,11 +117,11 @@ describe('ImportResolver', () => { new URL('http://localhost:7878/sparql'), ); const mockImporter = { - import: vi - .fn() - .mockResolvedValue( - new ImportSuccessful(importedDistribution, 'test-graph'), - ), + import: vi.fn().mockResolvedValue({ + type: 'successful', + distribution: importedDistribution, + identifier: 'test-graph', + } satisfies ImportSuccessful), }; const server = makeServer(); @@ -163,17 +162,14 @@ describe('ImportResolver', () => { ); const mockImporter = { - import: vi - .fn() - .mockResolvedValue( - new ImportFailed( - new Distribution( - new URL('http://example.org/data.nt'), - 'application/n-triples', - ), - 'Parse error', - ), + import: vi.fn().mockResolvedValue({ + type: 'failed', + distribution: new Distribution( + new URL('http://example.org/data.nt'), + 'application/n-triples', ), + error: 'Parse error', + } satisfies ImportFailed), }; const resolver = new ImportResolver(inner, { @@ -184,7 +180,7 @@ describe('ImportResolver', () => { expect(result).toBeInstanceOf(NoDistributionAvailable); const noDistribution = result as NoDistributionAvailable; - expect(noDistribution.importFailed).toBeInstanceOf(ImportFailed); + expect(noDistribution.importFailed?.type).toBe('failed'); expect(noDistribution.importFailed!.error).toBe('Parse error'); expect(noDistribution.probeResults).toHaveLength(1); }); @@ -201,14 +197,13 @@ describe('ImportResolver', () => { const inner = makeInnerResolver(resolved); const mockImporter = { - import: vi - .fn() - .mockResolvedValue( - new ImportSuccessful( - Distribution.sparql(new URL('http://localhost:7878/sparql')), - 'test-graph', - ), + import: vi.fn().mockResolvedValue({ + type: 'successful', + distribution: Distribution.sparql( + new URL('http://localhost:7878/sparql'), ), + identifier: 'test-graph', + } satisfies ImportSuccessful), }; const server = makeServer(); @@ -241,17 +236,14 @@ describe('ImportResolver', () => { const inner = makeInnerResolver(resolved); const mockImporter = { - import: vi - .fn() - .mockResolvedValue( - new ImportFailed( - new Distribution( - new URL('http://example.org/data.nt'), - 'application/n-triples', - ), - 'Parse error', - ), + import: vi.fn().mockResolvedValue({ + type: 'failed', + distribution: new Distribution( + new URL('http://example.org/data.nt'), + 'application/n-triples', ), + error: 'Parse error', + } satisfies ImportFailed), }; const resolver = new ImportResolver(inner, { @@ -264,7 +256,7 @@ describe('ImportResolver', () => { expect(result).toBeInstanceOf(NoDistributionAvailable); const noDistribution = result as NoDistributionAvailable; expect(noDistribution.probeResults).toHaveLength(1); - expect(noDistribution.importFailed).toBeInstanceOf(ImportFailed); + expect(noDistribution.importFailed?.type).toBe('failed'); }); it('default strategy preserves existing sparql-first behaviour', async () => { @@ -296,14 +288,13 @@ describe('ImportResolver', () => { ); const mockImporter = { - import: vi - .fn() - .mockResolvedValue( - new ImportSuccessful( - Distribution.sparql(new URL('http://localhost:7878/sparql')), - 'test-graph', - ), + import: vi.fn().mockResolvedValue({ + type: 'successful', + distribution: Distribution.sparql( + new URL('http://localhost:7878/sparql'), ), + identifier: 'test-graph', + } satisfies ImportSuccessful), }; const server = makeServer(); @@ -355,11 +346,11 @@ describe('ImportResolver', () => { importedDistribution.subjectFilter = '?s a .'; const mockImporter = { - import: vi - .fn() - .mockResolvedValue( - new ImportSuccessful(importedDistribution, 'test-graph'), - ), + import: vi.fn().mockResolvedValue({ + type: 'successful', + distribution: importedDistribution, + identifier: 'test-graph', + } satisfies ImportSuccessful), }; const server = makeServer(); diff --git a/packages/pipeline/test/distribution/report.test.ts b/packages/pipeline/test/distribution/report.test.ts index c2b7313c..f74562a9 100644 --- a/packages/pipeline/test/distribution/report.test.ts +++ b/packages/pipeline/test/distribution/report.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; import { Store } from 'n3'; import { Distribution } from '@lde/dataset'; -import { ImportFailed } from '@lde/sparql-importer'; +import type { ImportFailed } from '@lde/sparql-importer'; import { probeResultsToQuads, NetworkError, @@ -179,7 +179,11 @@ describe('probeResultsToQuads', () => { headers: { 'Content-Length': '1000' }, }), ); - const importError = new ImportFailed(distribution, 'Parse error'); + const importError: ImportFailed = { + type: 'failed', + distribution, + error: 'Parse error', + }; const store = await collect( probeResultsToQuads( diff --git a/packages/pipeline/test/distribution/resolveDistributions.test.ts b/packages/pipeline/test/distribution/resolveDistributions.test.ts index f0c70ab0..8694892a 100644 --- a/packages/pipeline/test/distribution/resolveDistributions.test.ts +++ b/packages/pipeline/test/distribution/resolveDistributions.test.ts @@ -5,7 +5,7 @@ import { type DistributionResolver, } from '../../src/distribution/index.js'; import { Dataset, Distribution } from '@lde/dataset'; -import { ImportFailed } from '@lde/sparql-importer'; +import type { ImportFailed } from '@lde/sparql-importer'; import { SparqlProbeResult, DataDumpProbeResult, @@ -22,7 +22,7 @@ async function collect(iterable: AsyncIterable): Promise { } function mockResolver( - result: ResolvedDistribution | NoDistributionAvailable + result: ResolvedDistribution | NoDistributionAvailable, ): DistributionResolver { return { resolve: async () => result }; } @@ -30,21 +30,21 @@ function mockResolver( describe('resolveDistributions', () => { it('returns resolved distribution and probe report quads', async () => { const distribution = Distribution.sparql( - new URL('http://example.org/sparql') + new URL('http://example.org/sparql'), ); const probeResult = new SparqlProbeResult( 'http://example.org/sparql', new Response('{}', { status: 200, headers: { 'Content-Type': 'application/sparql-results+json' }, - }) + }), ); const dataset = new Dataset({ iri: new URL('http://example.org/dataset'), distributions: [distribution], }); const resolver = mockResolver( - new ResolvedDistribution(distribution, [probeResult]) + new ResolvedDistribution(distribution, [probeResult]), ); const result = await resolveDistributions(dataset, resolver); @@ -64,14 +64,14 @@ describe('resolveDistributions', () => { }); const networkError = new NetworkError( 'http://example.org/sparql', - 'Connection refused' + 'Connection refused', ); const resolver = mockResolver( new NoDistributionAvailable( dataset, 'No SPARQL endpoint or importable data dump available', - [networkError] - ) + [networkError], + ), ); const result = await resolveDistributions(dataset, resolver); @@ -81,7 +81,7 @@ describe('resolveDistributions', () => { const quads = await collect(result.quads); expect(quads.length).toBeGreaterThan(0); const errorQuad = quads.find( - (q) => q.predicate.value === 'https://schema.org/error' + (q) => q.predicate.value === 'https://schema.org/error', ); expect(errorQuad).toBeDefined(); expect(errorQuad!.object.value).toBe('Connection refused'); @@ -90,7 +90,7 @@ describe('resolveDistributions', () => { it('returns null distribution and import error quads when importer fails', async () => { const dataDumpDistribution = new Distribution( new URL('http://example.org/data.nt'), - 'application/n-triples' + 'application/n-triples', ); const dataset = new Dataset({ iri: new URL('http://example.org/dataset'), @@ -101,16 +101,20 @@ describe('resolveDistributions', () => { new Response('', { status: 200, headers: { 'Content-Length': '1000' }, - }) + }), ); - const importFailed = new ImportFailed(dataDumpDistribution, 'Parse error'); + const importFailed: ImportFailed = { + type: 'failed', + distribution: dataDumpDistribution, + error: 'Parse error', + }; const resolver = mockResolver( new NoDistributionAvailable( dataset, 'No SPARQL endpoint or importable data dump available', [probeResult], - importFailed - ) + importFailed, + ), ); const result = await resolveDistributions(dataset, resolver); @@ -122,21 +126,21 @@ describe('resolveDistributions', () => { const errorQuad = quads.find( (q) => q.predicate.value === 'https://schema.org/error' && - q.object.value === 'Parse error' + q.object.value === 'Parse error', ); expect(errorQuad).toBeDefined(); }); it('works with a custom DistributionResolver implementation', async () => { const distribution = Distribution.sparql( - new URL('http://custom.org/sparql') + new URL('http://custom.org/sparql'), ); const probeResult = new SparqlProbeResult( 'http://custom.org/sparql', new Response('{}', { status: 200, headers: { 'Content-Type': 'application/sparql-results+json' }, - }) + }), ); const dataset = new Dataset({ iri: new URL('http://custom.org/dataset'), diff --git a/packages/sparql-importer/src/index.ts b/packages/sparql-importer/src/index.ts index 607919a5..fb6ffc36 100644 --- a/packages/sparql-importer/src/index.ts +++ b/packages/sparql-importer/src/index.ts @@ -7,35 +7,32 @@ import { Dataset, Distribution } from '@lde/dataset'; * This assumes that all Distributions of the Dataset are roughly equivalent: * https://docs.nde.nl/requirements-datasets/#dataset-distributions. */ -// export interface Importer extends EventEmitter { export interface Importer { /** * Import a {@link Dataset} to a SPARQL server. */ - import( - dataset: Dataset - ): Promise; + import(dataset: Dataset): Promise; } -// interface Events { -// imported: [statements: number]; -// end: [statements: number]; -// } +/** Discriminated union of all possible import outcomes. */ +export type ImportResult = ImportSuccessful | ImportFailed | NotSupported; -export class ImportSuccessful { - constructor( - public readonly distribution: Distribution, - public readonly identifier?: string - ) {} +/** The distribution was successfully imported. */ +export interface ImportSuccessful { + readonly type: 'successful'; + readonly distribution: Distribution; + readonly identifier?: string; } -export class ImportFailed { - constructor( - public readonly distribution: Distribution, - public readonly error: string - ) {} +/** The import was attempted but failed. */ +export interface ImportFailed { + readonly type: 'failed'; + readonly distribution: Distribution; + readonly error: string; } -export class NotSupported { - constructor(public readonly distribution?: Distribution) {} +/** The importer does not support this dataset's distributions. */ +export interface NotSupported { + readonly type: 'not-supported'; + readonly distribution?: Distribution; } diff --git a/packages/sparql-qlever/src/importer.ts b/packages/sparql-qlever/src/importer.ts index c5717180..c0bc6fb2 100644 --- a/packages/sparql-qlever/src/importer.ts +++ b/packages/sparql-qlever/src/importer.ts @@ -1,8 +1,8 @@ -import { +import type { Importer as ImporterInterface, ImportFailed, + ImportResult, ImportSuccessful, - NotSupported, } from '@lde/sparql-importer'; import { Dataset, Distribution } from '@lde/dataset'; import { @@ -45,9 +45,7 @@ export class Importer implements ImporterInterface { }; } - public async import( - dataset: Dataset, - ): Promise { + public async import(dataset: Dataset): Promise { const downloadDistributions = dataset .getDownloadDistributions() .filter( @@ -55,14 +53,14 @@ export class Importer implements ImporterInterface { distribution.mimeType !== undefined, ); if (downloadDistributions.length === 0) { - return new NotSupported(); + return { type: 'not-supported' }; } let result!: ImportSuccessful | ImportFailed; for (const downloadDistribution of downloadDistributions) { try { result = await this.doImport(downloadDistribution); - if (result instanceof ImportSuccessful) { + if (result.type === 'successful') { return result; } } catch (error) { @@ -72,7 +70,11 @@ export class Importer implements ImporterInterface { } else { errorMessage = (error as Error).message; } - result = new ImportFailed(downloadDistribution, errorMessage); + result = { + type: 'failed', + distribution: downloadDistribution, + error: errorMessage, + }; } } @@ -88,7 +90,7 @@ export class Importer implements ImporterInterface { this.fileFormatFromMimeType(distribution.mimeType), ); - return new ImportSuccessful(distribution); + return { type: 'successful', distribution }; } private fileFormatFromMimeType(mimeType: string): fileFormat { diff --git a/packages/sparql-qlever/test/importer.test.ts b/packages/sparql-qlever/test/importer.test.ts index a44770f6..16ea7a53 100644 --- a/packages/sparql-qlever/test/importer.test.ts +++ b/packages/sparql-qlever/test/importer.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect } from 'vitest'; import { Importer } from '../src/importer.js'; import { DockerTaskRunner } from '@lde/task-runner-docker'; -import { ImportSuccessful } from '@lde/sparql-importer'; import { Dataset, Distribution } from '@lde/dataset'; import { resolve } from 'node:path'; @@ -35,7 +34,7 @@ describe('Importer', () => { }); const result = await importer.import(dataset); - expect(result).toBeInstanceOf(ImportSuccessful); + expect(result.type).toBe('successful'); }, 30_000); }); });