diff --git a/packages/oc-azure-sql-metadata-adapter/src/index.ts b/packages/oc-azure-sql-metadata-adapter/src/index.ts index e6bb507b5..11120f879 100644 --- a/packages/oc-azure-sql-metadata-adapter/src/index.ts +++ b/packages/oc-azure-sql-metadata-adapter/src/index.ts @@ -3,10 +3,11 @@ import sql from 'mssql'; import { type ComponentRow, type MetadataStatus, - type MetadataStore, + type MetadataStoreWithCallbacks, VERSION_ALREADY_EXISTS, VERSION_PUBLISH_IN_PROGRESS, - type VersionAlreadyExistsError + type VersionAlreadyExistsError, + withCallbacks } from 'oc-metadata-adapters-utils'; export type { ComponentRow, MetadataStore } from 'oc-metadata-adapters-utils'; @@ -189,7 +190,7 @@ const addComponentRowInputs = ( export default function azureSqlMetadataAdapter( options?: AzureSqlMetadataAdapterOptions -): MetadataStore { +): MetadataStoreWithCallbacks { const adapterType = 'azure-sql'; const metadataOptions = options || ({} as AzureSqlMetadataAdapterOptions); const manageSchema = metadataOptions.manageSchema !== false; @@ -315,7 +316,8 @@ export default function azureSqlMetadataAdapter( `); }; - return { + const adapter = { + adapterApi: 'promise' as const, adapterType, isValid(): boolean { @@ -485,4 +487,37 @@ export default function azureSqlMetadataAdapter( } } }; + + return { + ...adapter, + initialise: withCallbacks( + adapter.initialise, + 'oc-azure-sql-metadata-adapter' + ), + getAllComponents: withCallbacks( + adapter.getAllComponents, + 'oc-azure-sql-metadata-adapter' + ), + addVersion: withCallbacks( + adapter.addVersion, + 'oc-azure-sql-metadata-adapter' + ), + reserveVersion: withCallbacks( + adapter.reserveVersion, + 'oc-azure-sql-metadata-adapter' + ), + commitVersion: withCallbacks( + adapter.commitVersion, + 'oc-azure-sql-metadata-adapter' + ), + abortVersion: withCallbacks( + adapter.abortVersion, + 'oc-azure-sql-metadata-adapter' + ), + getChangeToken: withCallbacks( + adapter.getChangeToken, + 'oc-azure-sql-metadata-adapter' + ), + close: withCallbacks(adapter.close, 'oc-azure-sql-metadata-adapter') + } as MetadataStoreWithCallbacks; } diff --git a/packages/oc-azure-sql-metadata-adapter/test/index.js b/packages/oc-azure-sql-metadata-adapter/test/index.js index 6fa6c64cf..0e88b628f 100644 --- a/packages/oc-azure-sql-metadata-adapter/test/index.js +++ b/packages/oc-azure-sql-metadata-adapter/test/index.js @@ -87,6 +87,17 @@ describe('oc-azure-sql-metadata-adapter', () => { }); }); + it('should support the legacy callback form', (done) => { + const { adapter } = createAdapter(); + const store = adapter({ server: 'localhost', database: 'oc' }); + + store.getAllComponents((error, rows) => { + expect(error).to.equal(null); + expect(rows).to.eql([]); + done(); + }); + }); + describe('initialise()', () => { it('should create the schema by default', async () => { const { adapter, pools, queryStub } = createAdapter(); diff --git a/packages/oc-azure-storage-adapter/src/index.ts b/packages/oc-azure-storage-adapter/src/index.ts index e87c25a26..85c349fed 100644 --- a/packages/oc-azure-storage-adapter/src/index.ts +++ b/packages/oc-azure-storage-adapter/src/index.ts @@ -13,9 +13,10 @@ import Cache from 'nice-cache'; import nodeDir, { type PathsResult } from 'node-dir'; import { getFileInfo, - type StorageAdapter, type StorageAdapterBaseConfig, - strings + type StorageAdapterWithCallbacks, + strings, + withCallbacks } from 'oc-storage-adapters-utils'; const getPaths: (path: string) => Promise = promisify( @@ -66,7 +67,9 @@ export interface AzureConfig extends StorageAdapterBaseConfig { | TokenCredential; } -export default function azureAdapter(conf: AzureConfig): StorageAdapter { +export default function azureAdapter( + conf: AzureConfig +): StorageAdapterWithCallbacks { const isValid = () => { if ( !conf.publicContainerName || @@ -291,19 +294,44 @@ export default function azureAdapter(conf: AzureConfig): StorageAdapter { }; return { - getFile, - getJson, + adapterApi: 'promise', + getFile: withCallbacks( + getFile, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['getFile'], + getJson: withCallbacks( + getJson, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['getJson'], getUrl, - listSubDirectories, + listSubDirectories: withCallbacks( + listSubDirectories, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['listSubDirectories'], maxConcurrentRequests: 20, - putDir, - putFile, - putFileContent, - removeFile, - removeDir, + putDir: withCallbacks( + putDir, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['putDir'], + putFile: withCallbacks( + putFile, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['putFile'], + putFileContent: withCallbacks( + putFileContent, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['putFileContent'], + removeFile: withCallbacks( + removeFile, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['removeFile'], + removeDir: withCallbacks( + removeDir, + 'oc-azure-storage-adapter' + ) as StorageAdapterWithCallbacks['removeDir'], adapterType: 'azure-blob-storage', isValid - }; + } as StorageAdapterWithCallbacks; } module.exports = azureAdapter; diff --git a/packages/oc-azure-storage-adapter/test/azure.test.ts b/packages/oc-azure-storage-adapter/test/azure.test.ts index e7544fe28..111c072e0 100644 --- a/packages/oc-azure-storage-adapter/test/azure.test.ts +++ b/packages/oc-azure-storage-adapter/test/azure.test.ts @@ -41,6 +41,16 @@ test('should expose the correct methods', () => { }); }); +test('should support the legacy callback form', (done) => { + const client = azure(validOptions); + + client.getFile('path/test.txt', (error, value) => { + expect(error).toBeNull(); + expect(value).toBe('Hello!'); + done(); + }); +}); + test('validate valid conf without credentials', () => { const options = { accountName: 'name', @@ -119,7 +129,7 @@ test('validate missing name', () => { test(`test getFile ${scenario.src}`, async () => { const client = azure(validOptions); const operation = () => - client[scenario.src.match(/\.json$/) ? 'getJson' : 'getFile']( + (client as any)[scenario.src.match(/\.json$/) ? 'getJson' : 'getFile']( scenario.src, false ); diff --git a/packages/oc-azure-table-metadata-adapter/src/index.ts b/packages/oc-azure-table-metadata-adapter/src/index.ts index cae7a9b5e..785f2dfb4 100644 --- a/packages/oc-azure-table-metadata-adapter/src/index.ts +++ b/packages/oc-azure-table-metadata-adapter/src/index.ts @@ -9,10 +9,11 @@ import { import { DefaultAzureCredential, type TokenCredential } from '@azure/identity'; import { type ComponentRow, - type MetadataStore, + type MetadataStoreWithCallbacks, VERSION_ALREADY_EXISTS, VERSION_PUBLISH_IN_PROGRESS, - type VersionAlreadyExistsError + type VersionAlreadyExistsError, + withCallbacks } from 'oc-metadata-adapters-utils'; export type { ComponentRow, MetadataStore } from 'oc-metadata-adapters-utils'; @@ -159,7 +160,7 @@ const getComponentEntity = ( export default function azureTableMetadataAdapter( options?: AzureTableMetadataAdapterOptions -): MetadataStore { +): MetadataStoreWithCallbacks { const adapterType = 'azure-table'; const opts = options || ({} as AzureTableMetadataAdapterOptions); const manageSchema = opts.manageSchema !== false; @@ -332,7 +333,8 @@ export default function azureTableMetadataAdapter( } }; - return { + const adapter = { + adapterApi: 'promise' as const, adapterType, isValid(): boolean { @@ -518,4 +520,37 @@ export default function azureTableMetadataAdapter( client = undefined; } }; + + return { + ...adapter, + initialise: withCallbacks( + adapter.initialise, + 'oc-azure-table-metadata-adapter' + ), + getAllComponents: withCallbacks( + adapter.getAllComponents, + 'oc-azure-table-metadata-adapter' + ), + addVersion: withCallbacks( + adapter.addVersion, + 'oc-azure-table-metadata-adapter' + ), + reserveVersion: withCallbacks( + adapter.reserveVersion, + 'oc-azure-table-metadata-adapter' + ), + commitVersion: withCallbacks( + adapter.commitVersion, + 'oc-azure-table-metadata-adapter' + ), + abortVersion: withCallbacks( + adapter.abortVersion, + 'oc-azure-table-metadata-adapter' + ), + getChangeToken: withCallbacks( + adapter.getChangeToken, + 'oc-azure-table-metadata-adapter' + ), + close: withCallbacks(adapter.close, 'oc-azure-table-metadata-adapter') + } as MetadataStoreWithCallbacks; } diff --git a/packages/oc-azure-table-metadata-adapter/test/index.js b/packages/oc-azure-table-metadata-adapter/test/index.js index 1e7c1dbff..0232590d6 100644 --- a/packages/oc-azure-table-metadata-adapter/test/index.js +++ b/packages/oc-azure-table-metadata-adapter/test/index.js @@ -223,6 +223,21 @@ describe('oc-azure-table-metadata-adapter', () => { }); }); + it('should support the legacy callback form', (done) => { + const { adapter } = createAdapter({ + listEntities: sinon.stub().returns({ + async *[Symbol.asyncIterator]() {} + }) + }); + const store = adapter({ connectionString: 'foo' }); + + store.getAllComponents((error, rows) => { + expect(error).to.equal(null); + expect(rows).to.eql([]); + done(); + }); + }); + describe('initialise()', () => { it('should create the table by default', async () => { const { adapter, createTableStub } = createAdapter(); diff --git a/packages/oc-gs-storage-adapter/src/index.ts b/packages/oc-gs-storage-adapter/src/index.ts index 093cb4023..797e2a19c 100644 --- a/packages/oc-gs-storage-adapter/src/index.ts +++ b/packages/oc-gs-storage-adapter/src/index.ts @@ -6,9 +6,10 @@ import Cache from 'nice-cache'; import nodeDir, { type PathsResult } from 'node-dir'; import { getFileInfo, - type StorageAdapter, type StorageAdapterBaseConfig, - strings + type StorageAdapterWithCallbacks, + strings, + withCallbacks } from 'oc-storage-adapters-utils'; import tmp from 'tmp'; @@ -32,7 +33,7 @@ export interface GsConfig extends StorageAdapterBaseConfig { maxAge?: boolean; } -export default function gsAdapter(conf: GsConfig): StorageAdapter { +export default function gsAdapter(conf: GsConfig): StorageAdapterWithCallbacks { const isValid = () => { if (!conf.bucket || !conf.projectId || !conf.path) { return false; @@ -332,19 +333,44 @@ export default function gsAdapter(conf: GsConfig): StorageAdapter { }; return { - getFile, - getJson, + adapterApi: 'promise', + getFile: withCallbacks( + getFile, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['getFile'], + getJson: withCallbacks( + getJson, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['getJson'], getUrl, - listSubDirectories, + listSubDirectories: withCallbacks( + listSubDirectories, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['listSubDirectories'], maxConcurrentRequests: 20, - putDir, - putFile, - putFileContent, - removeDir, - removeFile, + putDir: withCallbacks( + putDir, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['putDir'], + putFile: withCallbacks( + putFile, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['putFile'], + putFileContent: withCallbacks( + putFileContent, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['putFileContent'], + removeDir: withCallbacks( + removeDir, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['removeDir'], + removeFile: withCallbacks( + removeFile, + 'oc-gs-storage-adapter' + ) as StorageAdapterWithCallbacks['removeFile'], adapterType: 'gs', isValid - }; + } as StorageAdapterWithCallbacks; } module.exports = gsAdapter; diff --git a/packages/oc-gs-storage-adapter/test/gs.test.ts b/packages/oc-gs-storage-adapter/test/gs.test.ts index ef361a1c2..4c1371d61 100644 --- a/packages/oc-gs-storage-adapter/test/gs.test.ts +++ b/packages/oc-gs-storage-adapter/test/gs.test.ts @@ -53,6 +53,16 @@ test('should expose the correct methods', () => { }); }); +test('should support the legacy callback form', (done) => { + const client = gs(validOptions); + + client.getFile('path/test.txt', (error, value) => { + expect(error).toBeNull(); + expect(value).toBe('Hello!'); + done(); + }); +}); + test('validate valid conf', () => { const client = gs(validOptions); expect(client.isValid()).toBe(true); @@ -118,7 +128,7 @@ test('validate missing path conf', () => { test(`test getFile ${scenario.src}`, async () => { const client = gs(validOptions); const operation = () => - client[scenario.src.match(/\.json$/) ? 'getJson' : 'getFile']( + (client as any)[scenario.src.match(/\.json$/) ? 'getJson' : 'getFile']( scenario.src, false ); diff --git a/packages/oc-metadata-adapters-utils/src/index.ts b/packages/oc-metadata-adapters-utils/src/index.ts index 9cc590915..4e2b12da7 100644 --- a/packages/oc-metadata-adapters-utils/src/index.ts +++ b/packages/oc-metadata-adapters-utils/src/index.ts @@ -3,6 +3,76 @@ export const VERSION_PUBLISH_IN_PROGRESS = 'VERSION_PUBLISH_IN_PROGRESS'; export type MetadataStatus = 'publishing' | 'committed'; +export type MetadataAdapterCallback = ( + error: unknown, + value?: T +) => void; + +export type MetadataPromiseCallbackMethod< + Arguments extends unknown[], + ReturnValue +> = { + (...arguments_: Arguments): Promise; + (...arguments_: [...Arguments, MetadataAdapterCallback]): void; +}; + +const warnedCallbackAdapters = new Set(); + +const warnAboutCallbacks = (adapterId: string): void => { + if (warnedCallbackAdapters.has(adapterId)) { + return; + } + + warnedCallbackAdapters.add(adapterId); + const nodeProcess = ( + globalThis as typeof globalThis & { + process?: { emitWarning: (warning: string, type: string) => void }; + } + ).process; + nodeProcess?.emitWarning( + `Callback-based ${adapterId} adapter methods are deprecated and will be removed in OpenComponents v1 - use the returned promises instead.`, + 'DeprecationWarning' + ); +}; + +/** + * Adds the legacy callback form to a promise method without changing how + * promise callers observe results or errors. + */ +export const withCallbacks = ( + fn: (...arguments_: Arguments) => Promise, + adapterId = 'metadata' +): MetadataPromiseCallbackMethod => + function adapterMethod(this: unknown, ...arguments_: unknown[]) { + const callback = arguments_[arguments_.length - 1]; + + if (typeof callback !== 'function') { + return Promise.resolve().then(() => fn(...(arguments_ as Arguments))); + } + + warnAboutCallbacks(adapterId); + arguments_.pop(); + let settled = false; + const finish = (error: unknown, value?: ReturnValue) => { + if (settled) { + return; + } + settled = true; + (callback as MetadataAdapterCallback)(error, value); + }; + + try { + fn(...(arguments_ as Arguments)).then( + (value) => finish(null, value), + (error) => finish(error) + ); + } catch (error) { + finish(error); + } + + return undefined; + } as MetadataPromiseCallbackMethod; + export type ComponentRow = { name: string; version: string; @@ -12,7 +82,8 @@ export type ComponentRow = { publishToken?: string; }; -export interface MetadataStore { +export interface PromiseMetadataStore { + adapterApi?: 'promise'; adapterType: string; isValid(): boolean; initialise(): Promise; @@ -35,6 +106,127 @@ export interface MetadataStore { }>; } +export type MetadataStoreWithCallbacks = Omit< + PromiseMetadataStore, + | 'initialise' + | 'getAllComponents' + | 'addVersion' + | 'reserveVersion' + | 'commitVersion' + | 'abortVersion' + | 'getChangeToken' + | 'close' + | 'removeVersion' + | 'changesSince' +> & { + initialise: { + (): Promise; + (callback: MetadataAdapterCallback): void; + }; + getAllComponents: { + (): Promise; + (callback: MetadataAdapterCallback): void; + }; + addVersion: { + (row: ComponentRow): Promise; + (row: ComponentRow, callback: MetadataAdapterCallback): void; + }; + reserveVersion: { + (row: ComponentRow): Promise<{ token: string }>; + ( + row: ComponentRow, + callback: MetadataAdapterCallback<{ token: string }> + ): void; + }; + commitVersion: { + (name: string, version: string, token: string): Promise; + ( + name: string, + version: string, + token: string, + callback: MetadataAdapterCallback + ): void; + }; + abortVersion: { + (name: string, version: string, token: string): Promise; + ( + name: string, + version: string, + token: string, + callback: MetadataAdapterCallback + ): void; + }; + getChangeToken?: { + (): Promise; + (callback: MetadataAdapterCallback): void; + }; + close?: { + (): Promise; + (callback: MetadataAdapterCallback): void; + }; + removeVersion?: { + (name: string, version: string): Promise; + ( + name: string, + version: string, + callback: MetadataAdapterCallback + ): void; + }; + changesSince?: { + (cursor: string): Promise<{ rows: ComponentRow[]; cursor: string }>; + ( + cursor: string, + callback: MetadataAdapterCallback<{ + rows: ComponentRow[]; + cursor: string; + }> + ): void; + }; +}; + +export type MetadataStore = PromiseMetadataStore; + +export interface LegacyMetadataStore { + adapterApi?: 'callback'; + adapterType: string; + isValid(): boolean; + initialise: (callback: MetadataAdapterCallback) => void; + getAllComponents: (callback: MetadataAdapterCallback) => void; + addVersion: ( + row: ComponentRow, + callback: MetadataAdapterCallback + ) => void; + reserveVersion: ( + row: ComponentRow, + callback: MetadataAdapterCallback<{ token: string }> + ) => void; + commitVersion: ( + name: string, + version: string, + token: string, + callback: MetadataAdapterCallback + ) => void; + abortVersion: ( + name: string, + version: string, + token: string, + callback: MetadataAdapterCallback + ) => void; + getChangeToken?: (callback: MetadataAdapterCallback) => void; + close?: (callback: MetadataAdapterCallback) => void; + removeVersion?: ( + name: string, + version: string, + callback: MetadataAdapterCallback + ) => void; + changesSince?: ( + cursor: string, + callback: MetadataAdapterCallback<{ rows: ComponentRow[]; cursor: string }> + ) => void; +} + +export type MetadataStoreInput = PromiseMetadataStore | LegacyMetadataStore; + export interface VersionAlreadyExistsError extends Error { code: typeof VERSION_ALREADY_EXISTS | typeof VERSION_PUBLISH_IN_PROGRESS; cause?: unknown; diff --git a/packages/oc-s3-storage-adapter/src/index.ts b/packages/oc-s3-storage-adapter/src/index.ts index 9447c67be..199a8113d 100644 --- a/packages/oc-s3-storage-adapter/src/index.ts +++ b/packages/oc-s3-storage-adapter/src/index.ts @@ -14,9 +14,10 @@ import nodeDir, { type PathsResult } from 'node-dir'; import { getFileInfo, getNextYear, - type StorageAdapter, type StorageAdapterBaseConfig, - strings + type StorageAdapterWithCallbacks, + strings, + withCallbacks } from 'oc-storage-adapters-utils'; const getPaths: (path: string) => Promise = promisify( @@ -64,7 +65,7 @@ const streamToString = (stream: NodeJS.ReadableStream) => stream.on('end', () => resolve(Buffer.concat(chunks).toString('utf8'))); }); -export default function s3Adapter(conf: S3Config): StorageAdapter { +export default function s3Adapter(conf: S3Config): StorageAdapterWithCallbacks { const isValid = () => { if ( !conf.bucket || @@ -356,19 +357,44 @@ export default function s3Adapter(conf: S3Config): StorageAdapter { }; return { - getFile, - getJson, + adapterApi: 'promise', + getFile: withCallbacks( + getFile, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['getFile'], + getJson: withCallbacks( + getJson, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['getJson'], getUrl, - listSubDirectories, + listSubDirectories: withCallbacks( + listSubDirectories, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['listSubDirectories'], maxConcurrentRequests: 20, - putDir, - putFile, - putFileContent, - removeFile, - removeDir, + putDir: withCallbacks( + putDir, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['putDir'], + putFile: withCallbacks( + putFile, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['putFile'], + putFileContent: withCallbacks( + putFileContent, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['putFileContent'], + removeFile: withCallbacks( + removeFile, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['removeFile'], + removeDir: withCallbacks( + removeDir, + 'oc-s3-storage-adapter' + ) as StorageAdapterWithCallbacks['removeDir'], adapterType: 's3', isValid - }; + } as StorageAdapterWithCallbacks; } module.exports = s3Adapter; diff --git a/packages/oc-s3-storage-adapter/test/s3.test.ts b/packages/oc-s3-storage-adapter/test/s3.test.ts index 89c9aa005..ecc151d0e 100644 --- a/packages/oc-s3-storage-adapter/test/s3.test.ts +++ b/packages/oc-s3-storage-adapter/test/s3.test.ts @@ -40,6 +40,16 @@ test('should expose the correct methods', () => { }); }); +test('should support the legacy callback form', (done) => { + const client = s3(validOptions); + + client.getFile('path/test.txt', (error, value) => { + expect(error).toBeNull(); + expect(value).toBe('Hello!'); + done(); + }); +}); + test('validate valid conf', () => { const client = s3(validOptions); expect(client.isValid()).toBe(true); @@ -134,7 +144,7 @@ test('validate missing key/secret conf', () => { const client = s3(validOptions); const operation = () => - client[scenario.src.match(/\.json$/) ? 'getJson' : 'getFile']( + (client as any)[scenario.src.match(/\.json$/) ? 'getJson' : 'getFile']( scenario.src, false ); diff --git a/packages/oc-storage-adapters-utils/src/index.ts b/packages/oc-storage-adapters-utils/src/index.ts index c2d5cedae..da2251fa3 100644 --- a/packages/oc-storage-adapters-utils/src/index.ts +++ b/packages/oc-storage-adapters-utils/src/index.ts @@ -3,6 +3,74 @@ export { getMimeType } from './get-mime-type'; export { getNextYear } from './get-next-year'; export * as strings from './strings'; +export type AdapterApi = 'promise' | 'callback'; + +export type AdapterCallback = (error: unknown, value?: T) => void; + +export type PromiseCallbackMethod = { + (...arguments_: Arguments): Promise; + (...arguments_: [...Arguments, AdapterCallback]): void; +}; + +const warnedCallbackAdapters = new Set(); + +const warnAboutCallbacks = (adapterId: string): void => { + if (warnedCallbackAdapters.has(adapterId)) { + return; + } + + warnedCallbackAdapters.add(adapterId); + const nodeProcess = ( + globalThis as typeof globalThis & { + process?: { emitWarning: (warning: string, type: string) => void }; + } + ).process; + nodeProcess?.emitWarning( + `Callback-based ${adapterId} adapter methods are deprecated and will be removed in OpenComponents v1 - use the returned promises instead.`, + 'DeprecationWarning' + ); +}; + +/** + * Adds the legacy error-first callback form without changing the promise form. + * The returned function is deliberately one-shot: a misbehaving promise + * implementation cannot invoke a callback twice through this compatibility + * layer. + */ +export const withCallbacks = ( + fn: (...arguments_: Arguments) => Promise, + adapterId = 'storage' +): PromiseCallbackMethod => + function adapterMethod(this: unknown, ...arguments_: unknown[]) { + const callback = arguments_[arguments_.length - 1]; + + if (typeof callback !== 'function') { + return Promise.resolve().then(() => fn(...(arguments_ as Arguments))); + } + + warnAboutCallbacks(adapterId); + arguments_.pop(); + let settled = false; + const finish = (error: unknown, value?: ReturnValue) => { + if (settled) { + return; + } + settled = true; + (callback as AdapterCallback)(error, value); + }; + + try { + fn(...(arguments_ as Arguments)).then( + (value) => finish(null, value), + (error) => finish(error) + ); + } catch (error) { + finish(error); + } + + return undefined; + } as PromiseCallbackMethod; + export interface StorageAdapterBaseConfig { /** * Local folder that contains the compiled OC components ready to be uploaded @@ -29,7 +97,8 @@ export interface StorageAdapterBaseConfig { refreshInterval?: number; } -export interface StorageAdapter { +export interface PromiseStorageAdapter { + adapterApi?: 'promise'; adapterType: string; getFile(filePath: string, force?: boolean): Promise; getJson(filePath: string, force?: boolean): Promise; @@ -53,3 +122,165 @@ export interface StorageAdapter { removeFile(filePath: string, isPrivate: boolean): Promise; isValid: () => boolean; } + +export type StorageAdapterWithCallbacks = Omit< + PromiseStorageAdapter, + | 'getFile' + | 'getJson' + | 'listSubDirectories' + | 'putDir' + | 'putFile' + | 'putFileContent' + | 'removeDir' + | 'removeFile' +> & { + getFile: { + (filePath: string, force?: boolean): Promise; + (filePath: string, callback: AdapterCallback): void; + (filePath: string, force: boolean, callback: AdapterCallback): void; + }; + getJson: { + (filePath: string, force?: boolean): Promise; + (filePath: string, callback: AdapterCallback): void; + ( + filePath: string, + force: boolean, + callback: AdapterCallback + ): void; + }; + listSubDirectories: { + (dir: string): Promise; + (dir: string, callback: AdapterCallback): void; + }; + putDir: { + (folderPath: string, filePath: string): Promise; + ( + folderPath: string, + filePath: string, + callback: AdapterCallback + ): void; + }; + putFile: { + ( + filePath: string, + fileName: string, + isPrivate: boolean, + client?: unknown + ): Promise; + ( + filePath: string, + fileName: string, + isPrivate: boolean, + callback: AdapterCallback + ): void; + ( + filePath: string, + fileName: string, + isPrivate: boolean, + client: unknown, + callback: AdapterCallback + ): void; + }; + putFileContent: { + ( + data: unknown, + path: string, + isPrivate: boolean, + client?: unknown + ): Promise; + ( + data: unknown, + path: string, + isPrivate: boolean, + callback: AdapterCallback + ): void; + ( + data: unknown, + path: string, + isPrivate: boolean, + client: unknown, + callback: AdapterCallback + ): void; + }; + removeDir: { + (folderPath: string): Promise; + (folderPath: string, callback: AdapterCallback): void; + }; + removeFile: { + (filePath: string, isPrivate: boolean): Promise; + ( + filePath: string, + isPrivate: boolean, + callback: AdapterCallback + ): void; + }; +}; + +export type StorageAdapter = PromiseStorageAdapter; + +export interface LegacyStorageAdapter { + adapterApi?: 'callback'; + adapterType: string; + getFile: { + (filePath: string, callback: AdapterCallback): void; + (filePath: string, force: boolean, callback: AdapterCallback): void; + }; + getJson: { + (filePath: string, callback: AdapterCallback): void; + ( + filePath: string, + force: boolean, + callback: AdapterCallback + ): void; + }; + getUrl: (componentName: string, version: string, fileName: string) => string; + listSubDirectories: ( + dir: string, + callback: AdapterCallback + ) => void; + maxConcurrentRequests: number; + putDir: ( + folderPath: string, + filePath: string, + callback: AdapterCallback + ) => void; + putFile: { + ( + filePath: string, + fileName: string, + isPrivate: boolean, + callback: AdapterCallback + ): void; + ( + filePath: string, + fileName: string, + isPrivate: boolean, + client: unknown, + callback: AdapterCallback + ): void; + }; + putFileContent: { + ( + data: unknown, + path: string, + isPrivate: boolean, + callback: AdapterCallback + ): void; + ( + data: unknown, + path: string, + isPrivate: boolean, + client: unknown, + callback: AdapterCallback + ): void; + }; + removeDir?: (folderPath: string, callback: AdapterCallback) => void; + removeFile?: ( + filePath: string, + isPrivate: boolean, + callback: AdapterCallback + ) => void; + isValid: () => boolean; +} + +export type StorageAdapterInput = PromiseStorageAdapter | LegacyStorageAdapter; diff --git a/packages/oc/src/cli/facade/registry-migrate-metadata.ts b/packages/oc/src/cli/facade/registry-migrate-metadata.ts index 28d959441..31fdfc76a 100644 --- a/packages/oc/src/cli/facade/registry-migrate-metadata.ts +++ b/packages/oc/src/cli/facade/registry-migrate-metadata.ts @@ -1,6 +1,7 @@ import path from 'node:path'; import { pathToFileURL } from 'node:url'; import { fromPromise } from 'universalify'; +import getPromiseBasedMetadataAdapter from '../../registry/domain/metadata-adapter'; import getMetadataAdapterOptions from '../../registry/domain/metadata-adapter-options'; import { backfillMetadataFromStorageDetails } from '../../registry/domain/metadata-migration'; import sanitiseOptions, { @@ -69,8 +70,8 @@ const registryMigrateMetadata = ({ logger }: { logger: Logger }) => throw new Error('Registry config must include metadata options'); } - const metadataStore = conf.metadata.adapter( - getMetadataAdapterOptions(conf) + const metadataStore = getPromiseBasedMetadataAdapter( + conf.metadata.adapter(getMetadataAdapterOptions(conf)) ); const cdn = getPromiseBasedAdapter( conf.storage.adapter(conf.storage.options) diff --git a/packages/oc/src/registry/domain/metadata-adapter.ts b/packages/oc/src/registry/domain/metadata-adapter.ts new file mode 100644 index 000000000..ec6e83813 --- /dev/null +++ b/packages/oc/src/registry/domain/metadata-adapter.ts @@ -0,0 +1,139 @@ +import type { + LegacyMetadataStore, + MetadataStore, + MetadataStoreInput +} from 'oc-metadata-adapters-utils'; +import { fromCallback } from 'universalify'; +import deprecate from '../../utils/deprecate'; + +const warnAboutLegacyAdapter = (): void => { + deprecate({ + id: 'metadata-adapter-callback:custom', + subject: 'Callback-based metadata adapters', + replacement: 'a promise-returning metadata adapter' + }); +}; + +const convertMethod = (adapter: LegacyMetadataStore, method: string) => { + const implementation = adapter[method as keyof LegacyMetadataStore]; + return typeof implementation === 'function' + ? fromCallback((implementation as (...args: any[]) => void).bind(adapter)) + : implementation; +}; + +const convertLegacyAdapter = (adapter: LegacyMetadataStore): MetadataStore => + ({ + ...adapter, + adapterApi: 'promise', + initialise: convertMethod(adapter, 'initialise'), + getAllComponents: convertMethod(adapter, 'getAllComponents'), + addVersion: convertMethod(adapter, 'addVersion'), + reserveVersion: convertMethod(adapter, 'reserveVersion'), + commitVersion: convertMethod(adapter, 'commitVersion'), + abortVersion: convertMethod(adapter, 'abortVersion'), + getChangeToken: convertMethod(adapter, 'getChangeToken'), + close: convertMethod(adapter, 'close'), + removeVersion: convertMethod(adapter, 'removeVersion'), + changesSince: convertMethod(adapter, 'changesSince'), + isValid: adapter.isValid.bind(adapter) + }) as MetadataStore; + +const callUnmarkedMethod = ( + adapter: MetadataStoreInput, + method: (...args: any[]) => unknown, + args: unknown[] +): Promise => + new Promise((resolve, reject) => { + let settled = false; + const callback = (error: unknown, value?: ReturnValue) => { + warnAboutLegacyAdapter(); + if (settled) { + return; + } + settled = true; + if (error != null) { + reject(error); + } else { + resolve(value as ReturnValue); + } + }; + + let result: unknown; + try { + result = method.apply(adapter, [...args, callback]); + } catch (error) { + reject(error); + return; + } + + if (result && typeof (result as Promise).then === 'function') { + Promise.resolve(result).then( + (value) => { + if (!settled) { + settled = true; + resolve(value as ReturnValue); + } + }, + (error) => { + if (!settled) { + settled = true; + reject(error); + } + } + ); + } + }); + +const convertUnmarkedAdapter = (adapter: MetadataStoreInput): MetadataStore => { + const method = (name: keyof MetadataStore) => { + const implementation = adapter[name]; + return typeof implementation === 'function' + ? (...args: any[]) => + callUnmarkedMethod( + adapter, + implementation as (...args: any[]) => unknown, + args + ) + : implementation; + }; + + return { + ...adapter, + adapterApi: 'promise', + initialise: method('initialise'), + getAllComponents: method('getAllComponents'), + addVersion: method('addVersion'), + reserveVersion: method('reserveVersion'), + commitVersion: method('commitVersion'), + abortVersion: method('abortVersion'), + getChangeToken: method('getChangeToken'), + close: method('close'), + removeVersion: method('removeVersion'), + changesSince: method('changesSince'), + isValid: adapter.isValid.bind(adapter) + } as MetadataStore; +}; + +/** + * Returns the promise-first metadata contract used by registry internals. + * Explicitly marked adapters take the fast path. Unmarked custom adapters are + * wrapped lazily so either promise or callback implementations remain usable. + */ +export default function getPromiseBasedMetadataAdapter( + adapter: MetadataStoreInput +): MetadataStore { + if (adapter.adapterApi === 'promise') { + return adapter as MetadataStore; + } + + if (adapter.adapterApi === 'callback') { + warnAboutLegacyAdapter(); + return convertLegacyAdapter(adapter as LegacyMetadataStore); + } + + if (typeof adapter.initialise !== 'function') { + return adapter as MetadataStore; + } + + return convertUnmarkedAdapter(adapter); +} diff --git a/packages/oc/src/registry/domain/repository.ts b/packages/oc/src/registry/domain/repository.ts index d81628ed6..090a4ddc9 100644 --- a/packages/oc/src/registry/domain/repository.ts +++ b/packages/oc/src/registry/domain/repository.ts @@ -20,6 +20,7 @@ import errorToString from '../../utils/error-to-string'; import ComponentsCache from './components-cache'; import getComponentsDetails from './components-details'; import eventsHandler from './events-handler'; +import getPromiseBasedMetadataAdapter from './metadata-adapter'; import getMetadataAdapterOptions from './metadata-adapter-options'; import { createMetadataIndex, getComponentRow } from './metadata-index'; import { @@ -45,7 +46,9 @@ export default function repository(conf: Config) { : cdn.adapterType + ' cdn'; const metadataStore = !conf.local && conf.metadata - ? conf.metadata.adapter(getMetadataAdapterOptions(conf)) + ? getPromiseBasedMetadataAdapter( + conf.metadata.adapter(getMetadataAdapterOptions(conf)) + ) : undefined; const metadataIndex = metadataStore ? createMetadataIndex(metadataStore) diff --git a/packages/oc/src/registry/domain/storage-adapter.ts b/packages/oc/src/registry/domain/storage-adapter.ts index 7897d9c8e..66d8af59d 100644 --- a/packages/oc/src/registry/domain/storage-adapter.ts +++ b/packages/oc/src/registry/domain/storage-adapter.ts @@ -1,16 +1,10 @@ -import type { StorageAdapter } from 'oc-storage-adapters-utils'; +import type { + LegacyStorageAdapter, + StorageAdapter, + StorageAdapterInput +} from 'oc-storage-adapters-utils'; import { fromCallback } from 'universalify'; - -type RemovePromiseOverload = T extends { - (...args: infer B): void; - (...args: any[]): Promise; -} - ? (...args: B) => void - : T; - -type LegacyStorageAdapter = { - [P in keyof StorageAdapter]: RemovePromiseOverload; -}; +import deprecate from '../../utils/deprecate'; const officialAdapters = { s3: { name: 'oc-s3-storage-adapter', firstPromiseBasedVersion: '1.2.0' }, @@ -22,64 +16,86 @@ const officialAdapters = { }; type OfficialAdapter = keyof typeof officialAdapters; -function isOfficialAdapter( +const isOfficialAdapter = ( adapter: LegacyStorageAdapter -): adapter is LegacyStorageAdapter & { adapterType: OfficialAdapter } { - return Object.keys(officialAdapters).includes( - adapter.adapterType as OfficialAdapter - ); -} +): adapter is LegacyStorageAdapter & { adapterType: OfficialAdapter } => + Object.hasOwn(officialAdapters, adapter.adapterType); -function isPromiseBased(tryFunction: () => unknown) { - try { - (tryFunction as () => Promise)().catch(() => { - // To not throw unhandled promise exceptions - }); +const isPromiseBased = (adapter: StorageAdapterInput): boolean => { + if (adapter.adapterApi) { + return adapter.adapterApi === 'promise'; + } + + if (typeof adapter.getFile !== 'function') { return true; + } + + try { + const result = (adapter as StorageAdapter).getFile(''); + if (result && typeof result.then === 'function') { + void result.catch(() => undefined); + return true; + } } catch { return false; } -} -function isLegacyAdapter( - adapter: StorageAdapter | LegacyStorageAdapter -): adapter is LegacyStorageAdapter { - return !isPromiseBased(() => (adapter as StorageAdapter).getFile('')); -} + return false; +}; -function convertLegacyAdapter(adapter: LegacyStorageAdapter): StorageAdapter { - return { - getFile: fromCallback(adapter.getFile as any), - getJson: fromCallback(adapter.getJson as any), - listSubDirectories: fromCallback(adapter.listSubDirectories as any), - putDir: fromCallback(adapter.putDir as any), - putFile: fromCallback(adapter.putFile as any), - putFileContent: fromCallback(adapter.putFileContent as any), - getUrl: adapter.getUrl, - maxConcurrentRequests: adapter.maxConcurrentRequests, - adapterType: adapter.adapterType - } as any; -} +const warnAboutLegacyAdapter = (adapter: LegacyStorageAdapter): void => { + if (isOfficialAdapter(adapter)) { + const pkg = officialAdapters[adapter.adapterType]; + deprecate({ + id: `storage-adapter-callback:${adapter.adapterType}`, + subject: `The callback interface for ${pkg.name}`, + replacement: `${pkg.name} ${pkg.firstPromiseBasedVersion} or later` + }); + return; + } + deprecate({ + id: 'storage-adapter-callback:custom', + subject: 'Callback-based storage adapters', + replacement: 'a promise-returning storage adapter' + }); +}; + +const convertMethod = (adapter: LegacyStorageAdapter, method: string) => { + const implementation = adapter[method as keyof LegacyStorageAdapter]; + return typeof implementation === 'function' + ? fromCallback((implementation as (...args: any[]) => void).bind(adapter)) + : implementation; +}; + +const convertLegacyAdapter = (adapter: LegacyStorageAdapter): StorageAdapter => + ({ + ...adapter, + adapterApi: 'promise', + getFile: convertMethod(adapter, 'getFile'), + getJson: convertMethod(adapter, 'getJson'), + listSubDirectories: convertMethod(adapter, 'listSubDirectories'), + putDir: convertMethod(adapter, 'putDir'), + putFile: convertMethod(adapter, 'putFile'), + putFileContent: convertMethod(adapter, 'putFileContent'), + removeDir: convertMethod(adapter, 'removeDir'), + removeFile: convertMethod(adapter, 'removeFile'), + getUrl: adapter.getUrl.bind(adapter), + ...(adapter.isValid ? { isValid: adapter.isValid.bind(adapter) } : {}) + }) as StorageAdapter; + +/** + * Returns the promise-first storage contract used internally by the registry. + * Callback-only adapters remain supported on 0.x and are converted lazily by + * universalify, so their original errors and callback behavior are preserved. + */ export default function getPromiseBasedAdapter( - adapter: StorageAdapter | LegacyStorageAdapter + adapter: StorageAdapterInput ): StorageAdapter { - if (isLegacyAdapter(adapter)) { - if (isOfficialAdapter(adapter)) { - const pkg = officialAdapters[adapter.adapterType]; - process.emitWarning( - `Adapters now should work with promises. Consider upgrading your package ${pkg.name} to at least version ${pkg.firstPromiseBasedVersion}`, - 'DeprecationWarning' - ); - } else { - process.emitWarning( - 'Your adapter is using the old interface of working with callbacks. Consider upgrading it to work with promises, as the previous one will be deprecated.', - 'DeprecationWarning' - ); - } - - return convertLegacyAdapter(adapter); + if (isPromiseBased(adapter)) { + return adapter as StorageAdapter; } - return adapter; + warnAboutLegacyAdapter(adapter as LegacyStorageAdapter); + return convertLegacyAdapter(adapter as LegacyStorageAdapter); } diff --git a/packages/oc/src/registry/domain/validators/registry-configuration.ts b/packages/oc/src/registry/domain/validators/registry-configuration.ts index 0e185e8c7..e384b7c00 100644 --- a/packages/oc/src/registry/domain/validators/registry-configuration.ts +++ b/packages/oc/src/registry/domain/validators/registry-configuration.ts @@ -2,6 +2,7 @@ import strings from '../../../resources'; import type { Config, CorsOptions } from '../../../types'; import { validateCorsConfig } from '../../middleware/cors'; import * as auth from '../authentication'; +import getPromiseBasedMetadataAdapter from '../metadata-adapter'; import getMetadataAdapterOptions from '../metadata-adapter-options'; type ValidationResult = { isValid: true } | { isValid: false; message: string }; @@ -157,8 +158,8 @@ export default function registryConfiguration( } } - const metadataStore = conf.metadata.adapter( - getMetadataAdapterOptions(conf) + const metadataStore = getPromiseBasedMetadataAdapter( + conf.metadata.adapter(getMetadataAdapterOptions(conf)) ); if (!metadataStore.isValid()) { return returnError( diff --git a/packages/oc/src/types.ts b/packages/oc/src/types.ts index 9cb24cb84..636bed81e 100644 --- a/packages/oc/src/types.ts +++ b/packages/oc/src/types.ts @@ -1,6 +1,6 @@ import type { NextFunction, Request, Response } from 'express'; -import type { MetadataStore as MetadataStoreType } from 'oc-metadata-adapters-utils'; -import type { StorageAdapter } from 'oc-storage-adapters-utils'; +import type { MetadataStoreInput } from 'oc-metadata-adapters-utils'; +import type { StorageAdapterInput } from 'oc-storage-adapters-utils'; import type { PackageJson } from 'type-fest'; import type { HttpServerAdapterFactory, @@ -72,7 +72,7 @@ export interface MetadataConfig { * Factory for the metadata store. It must be side-effect free because config * validation may instantiate a throwaway store before registry startup. */ - adapter: (options: T) => MetadataStoreType; + adapter: (options: T) => MetadataStoreInput; options: T; /** * Controls whether OC asks the metadata adapter to manage its schema/table. @@ -468,7 +468,7 @@ export interface Config< * Low-level storage adapter used by the registry. */ storage: { - adapter: (options: T) => StorageAdapter; + adapter: (options: T) => StorageAdapterInput; options: T & { componentsDir: string }; }; /** diff --git a/packages/oc/test/types/adapter-registration.ts b/packages/oc/test/types/adapter-registration.ts new file mode 100644 index 000000000..774ff0320 --- /dev/null +++ b/packages/oc/test/types/adapter-registration.ts @@ -0,0 +1,40 @@ +import type { + LegacyMetadataStore, + MetadataStore, + MetadataStoreWithCallbacks +} from 'oc-metadata-adapters-utils'; +import type { + LegacyStorageAdapter, + StorageAdapter, + StorageAdapterWithCallbacks +} from 'oc-storage-adapters-utils'; + +declare const promiseStorageAdapter: StorageAdapter; +declare const dualStorageAdapter: StorageAdapterWithCallbacks; +declare const legacyStorageAdapter: LegacyStorageAdapter; +declare const promiseMetadataStore: MetadataStore; +declare const dualMetadataStore: MetadataStoreWithCallbacks; +declare const legacyMetadataStore: LegacyMetadataStore; + +promiseStorageAdapter.getFile('path'); +promiseStorageAdapter.getJson<{ ok: boolean }>('path'); +dualStorageAdapter.getFile('path'); +dualStorageAdapter.getFile('path', (error, value) => { + error; + value; +}); +legacyStorageAdapter.getFile('path', (error, value) => { + error; + value; +}); + +promiseMetadataStore.getAllComponents(); +dualMetadataStore.getAllComponents(); +dualMetadataStore.getAllComponents((error, rows) => { + error; + rows; +}); +legacyMetadataStore.getAllComponents((error, rows) => { + error; + rows; +}); diff --git a/packages/oc/test/unit/registry-domain-metadata-adapter.js b/packages/oc/test/unit/registry-domain-metadata-adapter.js new file mode 100644 index 000000000..ab7563e1a --- /dev/null +++ b/packages/oc/test/unit/registry-domain-metadata-adapter.js @@ -0,0 +1,129 @@ +const { expect } = require('chai'); +const injectr = require('injectr'); +const sinon = require('sinon'); + +const getParser = () => { + const emitWarning = sinon.stub(); + const parser = injectr( + '../../dist/registry/domain/metadata-adapter.js', + { + '../../utils/deprecate': { + __esModule: true, + default: ({ id, subject, replacement }) => + emitWarning( + `${id}: ${subject} is deprecated; use ${replacement}`, + 'DeprecationWarning' + ) + } + } + ).default; + + return { parser, emitWarning }; +}; + +const createPromiseStore = () => ({ + adapterApi: 'promise', + adapterType: 'test-metadata', + isValid: sinon.stub().returns(true), + initialise: sinon.stub().resolves(), + getAllComponents: sinon.stub().resolves([]), + addVersion: sinon.stub().resolves(), + reserveVersion: sinon.stub().resolves({ token: 'token' }), + commitVersion: sinon.stub().resolves(), + abortVersion: sinon.stub().resolves(), + close: sinon.stub().resolves() +}); + +const createLegacyStore = () => ({ + adapterApi: 'callback', + adapterType: 'legacy-metadata', + isValid: sinon.stub().returns(true), + initialise: sinon.stub().yields(null), + getAllComponents: sinon.stub().yields(null, []), + addVersion: sinon.stub().yields(null), + reserveVersion: sinon.stub().yields(null, { token: 'token' }), + commitVersion: sinon.stub().yields(null), + abortVersion: sinon.stub().yields(null), + close: sinon.stub().yields(null) +}); + +const createUnmarkedPromiseStore = () => { + const store = createPromiseStore(); + delete store.adapterApi; + return store; +}; + +const createUnmarkedRestCallbackStore = () => ({ + adapterType: 'legacy-rest-metadata', + isValid: sinon.stub().returns(true), + initialise: (...args) => args.at(-1)(null), + getAllComponents: (...args) => args.at(-1)(null, []), + addVersion: (...args) => args.at(-1)(null), + reserveVersion: (...args) => args.at(-1)(null, { token: 'token' }), + commitVersion: (...args) => args.at(-1)(null), + abortVersion: (...args) => args.at(-1)(null) +}); + +describe('registry : domain : metadata adapter', () => { + it('returns a promise metadata store unchanged', () => { + const { parser, emitWarning } = getParser(); + const store = createPromiseStore(); + + expect(parser(store)).to.equal(store); + expect(emitWarning.called).to.be.false; + }); + + it('supports unmarked promise stores', async () => { + const { parser, emitWarning } = getParser(); + + expect(await parser(createUnmarkedPromiseStore()).getAllComponents()).to.eql( + [] + ); + expect(emitWarning.called).to.be.false; + }); + + it('converts callback methods to promise methods and preserves errors', async () => { + const { parser, emitWarning } = getParser(); + const store = createLegacyStore(); + const parsed = parser(store); + + await parsed.initialise(); + expect(await parsed.getAllComponents()).to.eql([]); + expect(await parsed.reserveVersion({ + name: 'hello-world', + version: '1.0.0', + publishDate: 1 + })).to.eql({ token: 'token' }); + expect(emitWarning.calledOnce).to.be.true; + + const error = { code: 'METADATA_ERROR' }; + store.getAllComponents.resetBehavior(); + store.getAllComponents.callsFake((callback) => callback(error)); + let actualError; + try { + await parsed.getAllComponents(); + } catch (caughtError) { + actualError = caughtError; + } + expect(actualError).to.equal(error); + }); + + it('supports callback compatibility on the normalized store', (done) => { + const { parser } = getParser(); + const parsed = parser(createLegacyStore()); + + parsed.getAllComponents((error, rows) => { + expect(error).to.equal(null); + expect(rows).to.eql([]); + done(); + }); + }); + + it('supports unmarked callback stores with rest-parameter methods', async () => { + const { parser, emitWarning } = getParser(); + const parsed = parser(createUnmarkedRestCallbackStore()); + + expect(await parsed.getAllComponents()).to.eql([]); + expect(emitWarning.calledOnce).to.be.true; + }); +}); diff --git a/packages/oc/test/unit/registry-domain-repository.js b/packages/oc/test/unit/registry-domain-repository.js index ca133e092..7af1850ba 100644 --- a/packages/oc/test/unit/registry-domain-repository.js +++ b/packages/oc/test/unit/registry-domain-repository.js @@ -508,6 +508,22 @@ describe('registry : domain : repository', () => { } }); + const getCallbackMetadataStore = () => ({ + adapterApi: 'callback', + adapterType: 'legacy-metadata', + isValid: () => true, + initialise: (callback) => callback(null), + getAllComponents: (callback) => callback(null, []), + addVersion: (_row, callback) => callback(null), + reserveVersion: (_row, callback) => + callback(null, { token: 'publish-token' }), + commitVersion: (_name, _version, _token, callback) => + callback(null), + abortVersion: (_name, _version, _token, callback) => + callback(null), + close: (callback) => callback(null) + }); + const resetMetadataMocks = () => { metadataStoreMock.initialise = sinon.stub().resolves(); metadataStoreMock.getAllComponents = sinon.stub().resolves([]); @@ -551,6 +567,28 @@ describe('registry : domain : repository', () => { expect(componentsCacheMock.load.calledOnce).to.be.true; }); + it('should support callback metadata adapters through repository operations', async () => { + resetMetadataMocks(); + const callbackStore = getCallbackMetadataStore(); + const repository = Repository({ + ...cdnConfiguration, + metadata: { + adapter: () => callbackStore, + options: {} + } + }); + + await repository.init(); + await repository.publishComponent({ + componentName: 'hello-world', + componentVersion: '1.0.1', + pkgDetails: getPkg() + }); + await repository.close(); + + expect(s3Mock.putDir.calledOnce).to.be.true; + }); + it('should pass top-level manageSchema to the metadata adapter', () => { resetMetadataMocks(); const adapter = sinon.stub().returns(metadataStoreMock); diff --git a/packages/oc/test/unit/registry-domain-storage-adapter.js b/packages/oc/test/unit/registry-domain-storage-adapter.js index a6e2b549a..ef1c18896 100644 --- a/packages/oc/test/unit/registry-domain-storage-adapter.js +++ b/packages/oc/test/unit/registry-domain-storage-adapter.js @@ -2,96 +2,147 @@ const expect = require('chai').expect; const sinon = require('sinon'); const injectr = require('injectr'); -function mockAdapter() { +function mockPromiseAdapter() { return { - getFile: sinon.stub().resolves(), - getJson: sinon.stub().resolves(), - listSubDirectories: sinon.stub().resolves(), - putDir: sinon.stub().resolves(), - putFile: sinon.stub().resolves(), - putFileContent: sinon.stub().resolves(), + adapterApi: 'promise', + getFile: sinon.stub().resolves('file content'), + getJson: sinon.stub().resolves({ ok: true }), + listSubDirectories: sinon.stub().resolves(['1.0.0']), + putDir: sinon.stub().resolves(['uploaded']), + putFile: sinon.stub().resolves('uploaded'), + putFileContent: sinon.stub().resolves('uploaded'), + removeDir: sinon.stub().resolves(), + removeFile: sinon.stub().resolves(), getUrl: sinon.stub().returns(''), + isValid: sinon.stub().returns(true), maxConcurrentRequests: 20, adapterType: 's3' }; } -function mockLegacyAdapter() { +function mockLegacyAdapter(adapterType = 's3') { return { - getFile: sinon.stub().yields(), - getJson: sinon.stub().yields(), - listSubDirectories: sinon.stub().yields(), - putDir: sinon.stub().yields(), - putFile: sinon.stub().yields(), - putFileContent: sinon.stub().yields(), + adapterApi: 'callback', + getFile: sinon.stub().callsFake((filePath, callback) => + callback(null, `file:${filePath}`) + ), + getJson: sinon.stub().callsFake((_filePath, callback) => + callback(null, { ok: true }) + ), + listSubDirectories: sinon.stub().yields(null, ['1.0.0']), + putDir: sinon.stub().yields(null, ['uploaded']), + putFile: sinon.stub().yields(null, 'uploaded'), + putFileContent: sinon.stub().yields(null, 'uploaded'), + removeDir: sinon.stub().yields(null), + removeFile: sinon.stub().yields(null), getUrl: sinon.stub().returns(''), + isValid: sinon.stub().returns(true), maxConcurrentRequests: 20, - adapterType: 's3' + adapterType }; } -let process; - -function initialise(adapter) { - process = { emitWarning: sinon.stub() }; - const adapterParser = injectr( - '../../dist/registry/domain/storage-adapter.js', - { universalify: { fromCallback: sinon.stub().returns('promisified') } }, - { process } - ).default; +function getParser() { + const emitWarning = sinon.stub(); + const originalEmitWarning = process.emitWarning; + process.emitWarning = emitWarning; + const parser = injectr('../../dist/registry/domain/storage-adapter.js').default; - return adapterParser(adapter); + // Restore the process hook after each parser invocation in the test process. + const restore = () => { + process.emitWarning = originalEmitWarning; + }; + return { parser, emitWarning, restore }; } -describe('registry : domain : adapter', () => { - describe('when is not a legacy adapter', () => { - const adapter = mockAdapter(); - const parsed = initialise(adapter); +describe('registry : domain : storage adapter', () => { + it('returns a native promise adapter unchanged', () => { + const { parser, emitWarning, restore } = getParser(); + const adapter = mockPromiseAdapter(); - it('returns the same adapter', () => { - expect(parsed).to.be.equal(adapter); - }); + expect(parser(adapter)).to.equal(adapter); + expect(emitWarning.called).to.be.false; + restore(); }); - describe('when is a legacy adapter', () => { - describe('when is an official adapter', () => { - it('Shows a deprecation warning asking to upgrade', () => { - initialise(mockLegacyAdapter()); - - expect(process.emitWarning.called).to.be.true; - expect(process.emitWarning.args[0][0]).to.contain( - 'oc-s3-storage-adapter' - ); - expect(process.emitWarning.args[0][0]).to.contain('1.2.0'); - expect(process.emitWarning.args[0][1]).to.contain('DeprecationWarning'); - }); - }); + it('does not misclassify an unmarked promise adapter with optional arguments', async () => { + const { parser, emitWarning, restore } = getParser(); + const adapter = mockPromiseAdapter(); + delete adapter.adapterApi; + adapter.getFile = (filePath, _force = false) => + Promise.resolve(`file:${filePath}`); + + expect(await parser(adapter).getFile('path')).to.equal('file:path'); + expect(emitWarning.called).to.be.false; + restore(); + }); + + it('converts a callback adapter and preserves all adapter properties', async () => { + const { parser, emitWarning, restore } = getParser(); + const adapter = mockLegacyAdapter('azure-blob-storage'); + const parsed = parser(adapter); + + expect(parsed).not.to.equal(adapter); + expect(parsed.adapterType).to.equal('azure-blob-storage'); + expect(parsed.maxConcurrentRequests).to.equal(20); + expect(parsed.isValid()).to.be.true; + expect(await parsed.getFile('path')).to.equal('file:path'); + expect(await parsed.getJson('path')).to.eql({ ok: true }); + await parsed.removeDir('components'); + await parsed.removeFile('components/file', true); + expect(adapter.removeDir.calledOnce).to.be.true; + expect(adapter.removeFile.calledOnce).to.be.true; + expect(emitWarning.calledOnce).to.be.true; + restore(); + }); + + it('supports callback compatibility on converted methods', (done) => { + const { parser, restore } = getParser(); + const adapter = mockLegacyAdapter('gs'); + const parsed = parser(adapter); - describe('when is not an official adapter', () => { - it('Shows a deprecation warning about callbacks', () => { - initialise({ - ...mockLegacyAdapter(), - adapterType: 'non-official-adapter' - }); - expect(process.emitWarning.called).to.be.true; - expect(process.emitWarning.args[0][0]).to.contain( - 'Your adapter is using the old interface of working with callbacks. Consider upgrading it to work with promises, as the previous one will be deprecated.' - ); - expect(process.emitWarning.args[0][1]).to.contain('DeprecationWarning'); - }); + parsed.getFile('path', (error, value) => { + expect(error).to.equal(null); + expect(value).to.equal('file:path'); + restore(); + done(); }); + }); + + it('passes callback adapter errors through unchanged', (done) => { + const { parser, restore } = getParser(); + const error = { code: 'STORAGE_ERROR', message: 'backend failed' }; + const adapter = mockLegacyAdapter('gs'); + adapter.getFile.callsFake((_path, callback) => callback(error)); - it('returns a universalified adapter', () => { - const adapter = mockLegacyAdapter(); - const parsed = initialise(adapter); - - expect(parsed).not.to.be.equal(adapter); - expect(parsed.getFile).to.be.equal('promisified'); - expect(parsed.getJson).to.be.equal('promisified'); - expect(parsed.listSubDirectories).to.be.equal('promisified'); - expect(parsed.putDir).to.be.equal('promisified'); - expect(parsed.putFile).to.be.equal('promisified'); - expect(parsed.putFileContent).to.be.equal('promisified'); + parser(adapter).getFile('path').catch((actualError) => { + expect(actualError).to.equal(error); + restore(); + done(); }); }); + + it('warns once per adapter category', () => { + const { parser, emitWarning, restore } = getParser(); + + parser(mockLegacyAdapter()); + parser(mockLegacyAdapter()); + expect(emitWarning.calledOnce).to.be.true; + expect(emitWarning.args[0][0]).to.contain('oc-s3-storage-adapter'); + expect(emitWarning.args[0][0]).to.contain('1.2.0'); + expect(emitWarning.args[0][1]).to.equal('DeprecationWarning'); + restore(); + }); + + it('warns once for custom callback adapters', () => { + const { parser, emitWarning, restore } = getParser(); + + parser(mockLegacyAdapter('custom')); + parser(mockLegacyAdapter('custom')); + + expect(emitWarning.calledOnce).to.be.true; + expect(emitWarning.args[0][0]).to.contain('Callback-based storage adapters'); + restore(); + }); + }); diff --git a/packages/oc/test/unit/registry-domain-validator.js b/packages/oc/test/unit/registry-domain-validator.js index 2cd50fdae..dc9e52b1f 100644 --- a/packages/oc/test/unit/registry-domain-validator.js +++ b/packages/oc/test/unit/registry-domain-validator.js @@ -521,6 +521,27 @@ describe('registry : domain : validator', () => { }); }); + it('should accept a callback metadata adapter without invoking operations', () => { + const adapter = sinon.stub().returns({ + adapterApi: 'callback', + adapterType: 'legacy-metadata', + isValid: () => true, + initialise: (callback) => callback(null), + getAllComponents: (callback) => callback(null, []), + addVersion: (_row, callback) => callback(null), + reserveVersion: (_row, callback) => + callback(null, { token: 'token' }), + commitVersion: (_name, _version, _token, callback) => callback(null), + abortVersion: (_name, _version, _token, callback) => callback(null) + }); + const conf = { + s3: baseS3Conf, + metadata: { adapter, options: {} } + }; + + expect(validate(conf).isValid).to.be.true; + }); + it('should pass top-level manageSchema to the metadata adapter', () => { const adapter = sinon.stub().returns({ adapterType: 'test-metadata',