diff --git a/.changeset/callback-api-deprecations.md b/.changeset/callback-api-deprecations.md new file mode 100644 index 000000000..53400cfc3 --- /dev/null +++ b/.changeset/callback-api-deprecations.md @@ -0,0 +1,7 @@ +--- +"oc": patch +"oc-fastify-server-adapter": patch +"oc-metadata-adapters-utils": patch +--- + +Add promise-first adapter boundaries and one-time deprecation warnings for legacy registry, plugin, storage, metadata, and HTTP server callback APIs while preserving their 0.x behavior. diff --git a/V1.md b/V1.md index 99f298bb5..6ce41c0e3 100644 --- a/V1.md +++ b/V1.md @@ -123,15 +123,15 @@ pages?" โ€” **no**. | # | Change | Rationale | Migration (registry operator) | Status | |---|--------|-----------|-------------------------------|--------| -| R1 | **Promise-only registry API** | `registry.start/close/register`, plugin `register`, and adapter methods are callback-based behind a `universalify` shim. This is registry-boot code, not app code. | `await registry.start()` etc.; callback signatures removed. | ๐Ÿšง In progress โ€” the 0.x additive `start/close/register` precursor is implemented in [#1539](https://github.com/opencomponents/oc/pull/1539) (tracked in [#1528](https://github.com/opencomponents/oc/issues/1528)); plugin and adapter promise APIs plus the v1 callback removal remain open. | +| R1 | **Promise-only registry API** | `registry.start/close/register`, plugin `register`, and adapter methods are callback-based behind a `universalify` shim. This is registry-boot code, not app code. | `await registry.start()` etc.; callback signatures removed. | ๐Ÿšง In progress โ€” the 0.x additive registry lifecycle, plugin, and adapter promise paths now exist with callback warnings; v1 callback removal remains. | | R2 | **ESM-only registry packages + `exports` map** | The registry runs as an app you deploy; ESM-only is an acceptable registry break and kills dual-build complexity. Locks down deep imports. | Registry deployment uses ESM (or dynamic `import()`); only documented entry points importable. | โฌœ Not started | | R3 | **Adapter-native hook/config types** | `beforePublish`, `publishValidation`, `conf.routes` handlers, and `Authentication` leak Express `Request`/`Response`; a global `Express` augmentation ships in types. | Hooks receive `OcRequest`/`OcResponse`; Express becomes one adapter (Fastify, etc.). Express-compat shim provided. | โœ… Done โ€” `HttpServerAdapter` interface + in-core Express adapter (default) and a first-class `oc-fastify-server-adapter` package are shipped ([#1507](https://github.com/opencomponents/oc/pull/1507), [#1508](https://github.com/opencomponents/oc/pull/1508), [#1509](https://github.com/opencomponents/oc/pull/1509), [#1510](https://github.com/opencomponents/oc/pull/1510), [#1511](https://github.com/opencomponents/oc/pull/1511), [#1512](https://github.com/opencomponents/oc/pull/1512), [#1513](https://github.com/opencomponents/oc/pull/1513), [#1518](https://github.com/opencomponents/oc/pull/1518)). Follow-up hardening (trailing-slash routing default) tracked in [#1515](https://github.com/opencomponents/oc/issues/1515), partially landed via [#1521](https://github.com/opencomponents/oc/pull/1521). | | R4 | **Package split** | Monorepo already; a major is the moment. `oc-registry` (server), `oc-cli` (tooling), `oc-core` (shared types/runtime); `oc` = thin umbrella/CLI. | Update imports once (`import { Registry } from 'oc-registry'`). `oc` still installs the CLI. | ๐Ÿšง In progress โ€” Turborepo/Changesets monorepo foundation ([#1476](https://github.com/opencomponents/oc/pull/1476)); storage adapters ([#1505](https://github.com/opencomponents/oc/pull/1505)) and metadata adapters ([#1503](https://github.com/opencomponents/oc/pull/1503)) already extracted into their own `packages/*`. The actual `oc-registry`/`oc-cli`/`oc-core` split of the core package has not started. | -| R5 | **Promise-only plugins + typed errors** | Plugin `register(options, deps, next)` is callback-style; code throws raw strings in places. | Plugins return promises; errors become typed `Error` subclasses. | โฌœ Not started | +| R5 | **Promise-only plugins + typed errors** | Plugin `register(options, deps, next)` is callback-style; code throws raw strings in places. | Plugins return promises; errors become typed `Error` subclasses. | ๐Ÿšง In progress โ€” promise registration and one-time callback deprecation warnings are shipped on 0.x; typed errors and v1 removal remain. | | R6 | **Remove global mutable state** | `events-handler` subscriptions and `plugins-initialiser` `deferredLoads` are process-global, so multiple registries in one process interfere. | Per-instance event/plugin state. Subtle behavior change; documented. | โฌœ Not started | | R7 | **Metadata store = source of truth; bundle a default** | The metadata-adapter subsystem is newer and superior; flat-file is legacy. A bundled default keeps zero-config working. | No-config registries get the default store automatically. | ๐Ÿšง In progress โ€” pluggable metadata store landed as **opt-in** ([#1503](https://github.com/opencomponents/oc/pull/1503)); storage-only remains the default. Bundling a default store + making it the source of truth is still open. | | R8 | **Flat-file `components.json` = export-only legacy** | It's now a projection of the metadata store. | File is still *exported* for legacy consumers via `exportLegacyFiles`; no data loss. | โฌœ Not started (depends on R7) | -| R9 | **Promise-only storage/metadata adapters** | Drop the callback-adapter conversion shim. | Adapter authors return promises; operators use current promise-based adapter versions. | โฌœ Not started | +| R9 | **Promise-only storage/metadata adapters** | Drop the callback-adapter conversion shim. | Adapter authors return promises; operators use current promise-based adapter versions. | ๐Ÿšง In progress โ€” storage and metadata callback shims now warn once and preserve 0.x behavior; v1 removal remains. | | R10 | **Extract legacy jade/handlebars runtime to an opt-in compat package** | Keep core lean; old components must still render. | Operators still serving pre-`oc-template-*` components install/enable the compat package (registry-config action, not a component rewrite). | โฌœ Not started | | R11 | **Configurable CORS / security headers; keep URLs + Accept contract** | CORS is hardcoded today; clients/components depend on route shapes so URLs stay. | New optional `cors` config; defaults preserve today's behavior. No URL versioning. | โฌœ Not started โ€” `registry/middleware/cors.ts` still hardcodes headers | | R12 | **Remove the `oc-cli-version` publish gate** | Dates to `fcf87069` ("Added preventing old oc versions to publish") when CLI + registry shipped lockstep. It rejects any CLI whose `major.minor` is behind the registry โ€” a `1.0.0` registry would reject **all** v0.x CLIs on publish. `template.minOcVersion` + package-structure validation now cover the real need. | Publishing no longer fails on CLI semver skew. Keep `node-version` + `template.minOcVersion` checks. | โฌœ Not started โ€” gate still present in `registry/routes/publish.ts` | @@ -148,7 +148,7 @@ pages?" โ€” **no**. | # | Change | Rationale | Migration (web app owner) | Status | |---|--------|-----------|---------------------------|--------| | P1 | **Browser `oc-client.js` stays fully drop-in (guarantee, not a change)** | A web app embeds it and renders ``. Breaking it means editing every app. | **Nothing.** Same script URL, stable `window.oc` API, auto-injected importmap, unchanged `` markup (see ยง2b). | โœ… Holding โ€” guarantee upheld; the browser client was extracted into its own `oc-client-browser` package ([#1478](https://github.com/opencomponents/oc/pull/1478)) with drop-in behavior preserved (e.g. [#1516](https://github.com/opencomponents/oc/pull/1516) fixed a DOM-move unmount regression, [#1521](https://github.com/opencomponents/oc/pull/1521) started removing trailing slashes from `oc.build` hrefs). | -| P2 | **Node SSR `oc-client` โ†’ promise API + TS types + ESM** | Separate repo, callback-based, untyped. This is the single intentional app-layer break, done once per SSR app. | `await client.renderComponent(...)`; adopt ESM/types. Smoothed by a 0.x callback shim + deprecation warnings. | โฌœ Not started (lives in the separate `opencomponents/oc-client` repo) | +| P2 | **Node SSR `oc-client` โ†’ promise API + TS types + ESM** | Separate repo, callback-based, untyped. This is the single intentional app-layer break, done once per SSR app. | `await client.renderComponent(...)`; adopt ESM/types. Smoothed by a 0.x callback shim + deprecation warnings. | ๐Ÿšง In progress โ€” the 0.x promise/callback shim and registry integration warning are implemented; ESM/types remain for v1 (client lives in the separate `opencomponents/oc-client-node` repo). | | P3 | **Node `oc-client` keeps a conservative engine floor (`>=20`)** | Don't force SSR apps to upgrade their Node runtime just to consume components. | SSR apps stay on Node 20+ even though the registry requires 22+. | โฌœ Not started | ### 4.3 Component layer `[COMPONENT-ADDITIVE]` โ€” additive + opt-in only @@ -203,11 +203,9 @@ the client and in the (opt-in) component authoring path, not in the host page. ### Track 1 โ€” `0.x` (non-breaking, ships continuously, now) Land everything that can be additive so v1 becomes mostly *removals*: -1. ๐Ÿšง Add promise-returning variants of `registry.start/close/register`, plugins, adapters, - and the node `oc-client` (keep callbacks working, emit deprecation warnings). โ€” tracked in - [#1528](https://github.com/opencomponents/oc/issues/1528) (registry `start/close/register`), - implemented in [#1539](https://github.com/opencomponents/oc/pull/1539); plugin, adapter, - and node `oc-client` promise variants remain open. +1. โœ… Add promise-returning variants of `registry.start/close/register`, plugins, adapters, + and the node `oc-client` (keep callbacks working, emit one-time deprecation warnings). + Registry lifecycle is tracked in [#1528](https://github.com/opencomponents/oc/issues/1528). 2. โฌœ Migrate registry internals to ESM incrementally. โ€” not started. 3. โฌœ Introduce the modern component runtime as **opt-in** and make it the `oc init` default, while keeping legacy authoring fully supported. โ€” not started. @@ -220,10 +218,10 @@ Land everything that can be additive so v1 becomes mostly *removals*: node `oc-client` API. โ€” the central `deprecate()` utility plus warnings for `s3`, `refreshInterval`, boolean `discovery`, and `oc.json` `mocks` are **merged** via [#1532](https://github.com/opencomponents/oc/pull/1532) (tracked in - [#1529](https://github.com/opencomponents/oc/issues/1529)). The callback registry API - warning is implemented in [#1539](https://github.com/opencomponents/oc/pull/1539). - Still to cover: bare `jade`/`handlebars` on `oc init` and the callback node `oc-client` - API. + [#1529](https://github.com/opencomponents/oc/issues/1529)). Registry lifecycle callbacks + are implemented in [#1539](https://github.com/opencomponents/oc/pull/1539); this pass adds + plugin, storage, metadata, HTTP adapter, and node SSR client callback warnings. Still to + cover: bare `jade`/`handlebars` on `oc init`. **Additional registry-layer groundwork landed ahead of/alongside the plan above** (see ยง4.1 for detail): the `HttpServerAdapter` abstraction with Express (default) and Fastify adapters @@ -317,8 +315,9 @@ storage/metadata adapter package extraction (R4, ๐Ÿšง in progress โ€” [#1476](ht - [#1529](https://github.com/opencomponents/oc/issues/1529) โ€” Central deprecation-warning pass for v1 removals โ€” Track 1 item 5. Core utility + config-option warnings merged via [#1532](https://github.com/opencomponents/oc/pull/1532); callback registry warnings are - implemented in [#1539](https://github.com/opencomponents/oc/pull/1539), while bare - `jade`/`handlebars` and callback node `oc-client` warnings remain open. + implemented in [#1539](https://github.com/opencomponents/oc/pull/1539), and this pass covers + plugin, adapter, and node SSR client callbacks. Bare `jade`/`handlebars` warnings remain + open. - [#1515](https://github.com/opencomponents/oc/issues/1515) โ€” Make the Fastify adapter's `ignoreTrailingSlash` configurable (default `false`) and stop OC from emitting trailing-slash URLs โ€” follow-on hardening for R3. diff --git a/packages/oc-fastify-server-adapter/README.md b/packages/oc-fastify-server-adapter/README.md index a009081b3..f2a9c0258 100644 --- a/packages/oc-fastify-server-adapter/README.md +++ b/packages/oc-fastify-server-adapter/README.md @@ -8,7 +8,7 @@ Fastify HTTP server adapter for the OC registry. The adapter is opt-in and imple npm install oc-fastify-server-adapter fastify ``` -`oc` is a peer dependency. Use this adapter with an OC version that exports the HTTP server adapter types (`>=0.50.56`). +Use this adapter with an OC version that supports the HTTP server adapter contract (`>=0.50.56`). ## Usage diff --git a/packages/oc-fastify-server-adapter/src/index.ts b/packages/oc-fastify-server-adapter/src/index.ts index 1146eefd2..8994dc701 100644 --- a/packages/oc-fastify-server-adapter/src/index.ts +++ b/packages/oc-fastify-server-adapter/src/index.ts @@ -158,15 +158,25 @@ const ocResponseSym = Symbol('ocResponse'); const timingStartSym = Symbol('timingStart'); const multipartParsedSym = Symbol('multipartParsed'); const defaultBodyLimit = 100 * 1024; -const warnedDeprecations = new Set(); +const warningStoreKey = Symbol.for('opencomponents.deprecation-warnings'); +const callbackWarningId = 'http-server-adapter-callbacks'; const warnAboutCallback = () => { - const id = 'http-server-adapter-callbacks'; - if (warnedDeprecations.has(id)) { + const processWithWarningStore = process as typeof process & { + [key: symbol]: unknown; + }; + let warned = processWithWarningStore[warningStoreKey] as + | Set + | undefined; + if (!warned) { + warned = new Set(); + processWithWarningStore[warningStoreKey] = warned; + } + if (warned.has(callbackWarningId)) { return; } - warnedDeprecations.add(id); + warned.add(callbackWarningId); process.emitWarning( 'The HTTP server adapter callback API is deprecated and will be removed in OpenComponents v1 - use the returned promises instead.', 'DeprecationWarning' diff --git a/packages/oc-metadata-adapters-utils/src/index.ts b/packages/oc-metadata-adapters-utils/src/index.ts index 9cc590915..315a5bdce 100644 --- a/packages/oc-metadata-adapters-utils/src/index.ts +++ b/packages/oc-metadata-adapters-utils/src/index.ts @@ -35,6 +35,22 @@ export interface MetadataStore { }>; } +/** A promise- or callback-based metadata store accepted on the 0.x line. */ +export type MetadataStoreLike = { + adapterType: string; + isValid(): boolean; + initialise: (...args: any[]) => unknown; + getAllComponents: (...args: any[]) => unknown; + addVersion: (...args: any[]) => unknown; + reserveVersion: (...args: any[]) => unknown; + commitVersion: (...args: any[]) => unknown; + abortVersion: (...args: any[]) => unknown; + getChangeToken?: (...args: any[]) => unknown; + close?: (...args: any[]) => unknown; + removeVersion?: (...args: any[]) => unknown; + changesSince?: (...args: any[]) => unknown; +}; + export interface VersionAlreadyExistsError extends Error { code: typeof VERSION_ALREADY_EXISTS | typeof VERSION_PUBLISH_IN_PROGRESS; cause?: unknown; diff --git a/packages/oc/README.md b/packages/oc/README.md index 6fa69a43b..376559487 100644 --- a/packages/oc/README.md +++ b/packages/oc/README.md @@ -80,8 +80,9 @@ metadata store. Publishing reserves the metadata row first, uploads package file to storage only after the reservation succeeds, then commits the row. Duplicate or in-progress metadata rows are treated as the existing "component version already exists" publish error. When the registry is shut down via -`registry.close(callback)`, the metadata adapter's optional `close()` hook is -invoked so the adapter can release its connection pool. +`await registry.close()`, the metadata adapter's optional `close()` hook is +invoked so the adapter can release its connection pool. The callback form remains +available on 0.x but is deprecated. Custom metadata adapters should implement the shared contract exported by `oc-metadata-adapters-utils`: 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/index.ts b/packages/oc/src/index.ts index 6caa62e8e..7ceb35948 100644 --- a/packages/oc/src/index.ts +++ b/packages/oc/src/index.ts @@ -8,7 +8,9 @@ export type { ExpressMiddleware, HttpServerAdapter, HttpServerAdapterFactory, + HttpServerAdapterLike, HttpServerListenOptions, + LegacyHttpServerAdapter, Method, NativeApp, OcHandler, diff --git a/packages/oc/src/registry/domain/http-server/types.ts b/packages/oc/src/registry/domain/http-server/types.ts index fd8ef581a..86277235b 100644 --- a/packages/oc/src/registry/domain/http-server/types.ts +++ b/packages/oc/src/registry/domain/http-server/types.ts @@ -156,6 +156,12 @@ interface CallbackHttpServerAdapterLifecycle { export type PromiseHttpServerAdapter = HttpServerAdapterBase & PromiseHttpServerAdapterLifecycle; +export type LegacyHttpServerAdapter = + HttpServerAdapterBase & CallbackHttpServerAdapterLifecycle; + export type HttpServerAdapter = | PromiseHttpServerAdapter - | (HttpServerAdapterBase & CallbackHttpServerAdapterLifecycle); + | LegacyHttpServerAdapter; + +export type HttpServerAdapterLike = + HttpServerAdapter; 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..fa599e1b0 --- /dev/null +++ b/packages/oc/src/registry/domain/metadata-adapter.ts @@ -0,0 +1,128 @@ +import type { MetadataStore, MetadataStoreLike } from '../../types'; +import deprecate from '../../utils/deprecate'; + +type MetadataMethod = + | 'initialise' + | 'getAllComponents' + | 'addVersion' + | 'reserveVersion' + | 'commitVersion' + | 'abortVersion' + | 'getChangeToken' + | 'close' + | 'removeVersion' + | 'changesSince'; + +const metadataMethods = new Set([ + 'initialise', + 'getAllComponents', + 'addVersion', + 'reserveVersion', + 'commitVersion', + 'abortVersion', + 'getChangeToken', + 'close', + 'removeVersion', + 'changesSince' +]); + +const isPromiseLike = (value: unknown): value is PromiseLike => + typeof (value as PromiseLike | undefined)?.then === 'function'; + +const warnAboutCallbacks = () => + deprecate({ + id: 'metadata-adapter-callbacks', + subject: 'Metadata adapter callbacks', + replacement: 'promise-based metadata adapter methods' + }); + +const callMetadataMethod = ( + method: (...args: any[]) => unknown, + receiver: MetadataStoreLike, + args: unknown[] +): Promise => + new Promise((resolve, reject) => { + const suppliedCallback = + typeof args[args.length - 1] === 'function' + ? (args.pop() as (error?: unknown, value?: unknown) => void) + : undefined; + if (suppliedCallback) { + warnAboutCallbacks(); + } + let callbackCalled = false; + let callbackError: unknown; + let callbackValue: unknown; + let useCallback = false; + let suppliedCallbackCalled = false; + + const notifySuppliedCallback = (error?: unknown, value?: unknown) => { + if (suppliedCallback && !suppliedCallbackCalled) { + suppliedCallbackCalled = true; + suppliedCallback(error, value); + } + }; + + const callback = (error?: unknown, value?: unknown) => { + callbackCalled = true; + callbackError = error; + callbackValue = value; + + notifySuppliedCallback(error, value); + + if (useCallback) { + callbackError ? reject(callbackError) : resolve(callbackValue); + } + }; + + let result: unknown; + try { + result = method.apply(receiver, [...args, callback]); + } catch (error) { + notifySuppliedCallback(error); + reject(error); + return; + } + + if (isPromiseLike(result)) { + void result.then( + (value) => { + notifySuppliedCallback(undefined, value); + resolve(value); + }, + (error) => { + notifySuppliedCallback(error); + reject(error); + } + ); + return; + } + + useCallback = true; + if (!suppliedCallback) { + warnAboutCallbacks(); + } + + if (callbackCalled) { + callbackError ? reject(callbackError) : resolve(callbackValue); + } + }); + +export default function getPromiseBasedMetadataAdapter( + adapter: MetadataStoreLike +): MetadataStore { + return new Proxy(adapter, { + get(target, property, receiver) { + const value = Reflect.get(target, property, receiver); + if ( + typeof property !== 'string' || + !metadataMethods.has(property as MetadataMethod) || + typeof value !== 'function' + ) { + return value; + } + + return (...args: unknown[]) => + callMetadataMethod(value as (...args: any[]) => unknown, target, args); + } + }) as unknown as MetadataStore; +} diff --git a/packages/oc/src/registry/domain/plugins-initialiser.ts b/packages/oc/src/registry/domain/plugins-initialiser.ts index b69bf36c5..ef69070f9 100644 --- a/packages/oc/src/registry/domain/plugins-initialiser.ts +++ b/packages/oc/src/registry/domain/plugins-initialiser.ts @@ -65,10 +65,24 @@ const registerPlugin = ( let callbackCalled = false; let callbackError: Error | undefined; let useCallback = false; + let callbackWarningEmitted = false; let result: unknown; + const warnAboutCallback = () => { + if (callbackWarningEmitted) { + return; + } + callbackWarningEmitted = true; + deprecate({ + id: 'plugin-register-callback', + subject: 'Plugin register callbacks', + replacement: 'an async register(options, dependencies) function' + }); + }; + try { result = register(plugin.options || {}, dependencies, (error?: Error) => { + warnAboutCallback(); callbackCalled = true; callbackError = error; @@ -85,11 +99,7 @@ const registerPlugin = ( void result.then(resolve, reject); } else { useCallback = true; - deprecate({ - id: 'plugin-register-callback', - subject: 'Plugin register callbacks', - replacement: 'an async register(options, dependencies) function' - }); + warnAboutCallback(); if (callbackCalled) { callbackError ? reject(callbackError) : resolve(); 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/server-adapter.ts b/packages/oc/src/registry/domain/server-adapter.ts index 653e0b825..f36ebf2eb 100644 --- a/packages/oc/src/registry/domain/server-adapter.ts +++ b/packages/oc/src/registry/domain/server-adapter.ts @@ -1,8 +1,25 @@ -import type { HttpServerAdapter } from './http-server/types'; +import deprecate from '../../utils/deprecate'; +import type { + HttpServerAdapter, + HttpServerAdapterFactory, + HttpServerAdapterLike, + LegacyHttpServerAdapter, + PromiseHttpServerAdapter +} from './http-server/types'; -type HttpServerAdapterFactory = (options?: T) => HttpServerAdapter; +const isPromiseLike = (value: unknown): value is PromiseLike => + typeof (value as PromiseLike | undefined)?.then === 'function'; -function isHttpServerAdapter(adapter: unknown): adapter is HttpServerAdapter { +const warnAboutCallbacks = () => + deprecate({ + id: 'http-server-adapter-callbacks', + subject: 'The HTTP server adapter callback API', + replacement: 'the returned promises' + }); + +function isHttpServerAdapter( + adapter: unknown +): adapter is HttpServerAdapterLike { return ( !!adapter && typeof adapter === 'object' && @@ -12,12 +29,112 @@ function isHttpServerAdapter(adapter: unknown): adapter is HttpServerAdapter { ); } +const toPromise = ( + method: (...args: any[]) => unknown, + args: unknown[] +): Promise => + new Promise((resolve, reject) => { + let callbackCalled = false; + let callbackError: Error | undefined; + let useCallback = false; + let settled = false; + const settle = (error?: Error) => { + if (settled) { + return; + } + settled = true; + error ? reject(error) : resolve(); + }; + const callback = (error?: Error) => { + warnAboutCallbacks(); + callbackCalled = true; + callbackError = error; + if (useCallback) { + settle(error); + } + }; + + let result: unknown; + try { + result = method(...args, callback); + } catch (error) { + settle(error as Error); + return; + } + + if (isPromiseLike(result)) { + void result.then( + () => settle(), + (error) => settle(error as Error) + ); + return; + } + + useCallback = true; + warnAboutCallbacks(); + if (callbackCalled) { + settle(callbackError); + } + }); + +const normaliseAdapter = ( + adapter: HttpServerAdapterLike +): PromiseHttpServerAdapter => { + if ( + adapter.supportsPromiseLifecycle === true || + (adapter.supportsPromiseLifecycle !== false && + typeof (adapter as any).close !== 'function') + ) { + return adapter as PromiseHttpServerAdapter; + } + + return new Proxy(adapter, { + get(target, property) { + if (property === 'supportsPromiseLifecycle') { + return true; + } + if (property === 'listen') { + return (options: unknown, callback?: (error?: Error) => void) => { + if (callback) { + warnAboutCallbacks(); + return (target as LegacyHttpServerAdapter).listen( + options as any, + callback + ); + } + + return toPromise( + (target as LegacyHttpServerAdapter).listen.bind(target), + [options] + ); + }; + } + if (property === 'close') { + return (callback?: (error?: Error) => void) => { + if (callback) { + warnAboutCallbacks(); + return (target as LegacyHttpServerAdapter).close(callback); + } + + return toPromise( + (target as LegacyHttpServerAdapter).close.bind(target), + [] + ); + }; + } + + const value = Reflect.get(target, property, target); + return typeof value === 'function' ? value.bind(target) : value; + } + }) as unknown as PromiseHttpServerAdapter; +}; + export default function getHttpServerAdapter( - adapter: HttpServerAdapter | HttpServerAdapterFactory, + adapter: HttpServerAdapterLike | HttpServerAdapterFactory, options?: T -): HttpServerAdapter { +): PromiseHttpServerAdapter { if (isHttpServerAdapter(adapter)) { - return adapter; + return normaliseAdapter(adapter); } const instance = adapter(options); @@ -25,5 +142,5 @@ export default function getHttpServerAdapter( throw new Error('Invalid HTTP server adapter'); } - return instance; + return normaliseAdapter(instance); } diff --git a/packages/oc/src/registry/domain/storage-adapter.ts b/packages/oc/src/registry/domain/storage-adapter.ts index 7897d9c8e..fec3fb3b7 100644 --- a/packages/oc/src/registry/domain/storage-adapter.ts +++ b/packages/oc/src/registry/domain/storage-adapter.ts @@ -1,5 +1,6 @@ import type { StorageAdapter } from 'oc-storage-adapters-utils'; import { fromCallback } from 'universalify'; +import deprecate from '../../utils/deprecate'; type RemovePromiseOverload = T extends { (...args: infer B): void; @@ -48,35 +49,50 @@ function isLegacyAdapter( } function convertLegacyAdapter(adapter: LegacyStorageAdapter): StorageAdapter { + const toPromise = (method: unknown) => + typeof method === 'function' + ? fromCallback((method as (...args: any[]) => void).bind(adapter)) + : method; + 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, + getFile: toPromise(adapter.getFile), + getJson: toPromise(adapter.getJson), + listSubDirectories: toPromise(adapter.listSubDirectories), + putDir: toPromise(adapter.putDir), + putFile: toPromise(adapter.putFile), + putFileContent: toPromise(adapter.putFileContent), + removeDir: toPromise(adapter.removeDir), + removeFile: toPromise(adapter.removeFile), + getUrl: adapter.getUrl.bind(adapter), maxConcurrentRequests: adapter.maxConcurrentRequests, - adapterType: adapter.adapterType + adapterType: adapter.adapterType, + isValid: adapter.isValid?.bind(adapter) } as any; } +const warnAboutCallbacks = (adapter: LegacyStorageAdapter) => { + if (isOfficialAdapter(adapter)) { + const pkg = officialAdapters[adapter.adapterType]; + deprecate({ + id: `storage-adapter-callbacks-${adapter.adapterType}`, + subject: `The callback API of ${pkg.name}`, + replacement: `the promise API from ${pkg.name}@${pkg.firstPromiseBasedVersion} or newer` + }); + return; + } + + deprecate({ + id: 'storage-adapter-callbacks', + subject: 'Storage adapter callbacks', + replacement: 'promise-based storage adapter methods' + }); +}; + export default function getPromiseBasedAdapter( adapter: StorageAdapter | LegacyStorageAdapter ): 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' - ); - } + warnAboutCallbacks(adapter); return convertLegacyAdapter(adapter); } 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/registry/routes/helpers/get-component.ts b/packages/oc/src/registry/routes/helpers/get-component.ts index a7ee0521e..f3baa2aaa 100644 --- a/packages/oc/src/registry/routes/helpers/get-component.ts +++ b/packages/oc/src/registry/routes/helpers/get-component.ts @@ -10,6 +10,7 @@ import { fromPromise } from 'universalify'; import strings from '../../../resources'; import settings from '../../../resources/settings'; import type { Component, Config, PluginContext, Plugins } from '../../../types'; +import deprecate from '../../../utils/deprecate'; import isTemplateLegacy from '../../../utils/is-template-legacy'; import eventsHandler from '../../domain/events-handler'; import type { CookieOptions } from '../../domain/http-server/types'; @@ -555,26 +556,50 @@ export default function getComponent(conf: Config, repository: Repository) { data.id = id; const returnResult = (template: any) => { + const renderSuccess = (html: string) => + callback({ + status: 200, + ...(responseHeaders ? { headers: responseHeaders } : {}), + response: Object.assign(response, { html }) + }); + const renderFailure = (err: Error) => + callback({ + status: 500, + response: { + code: 'INTERNAL_SERVER_ERROR', + error: err + } + }); + + if ((client as any).supportsPromiseApi === true) { + Promise.resolve() + .then(() => + (client as any).renderTemplate( + template, + data, + renderOptions + ) + ) + .then(renderSuccess, renderFailure); + return; + } + + deprecate({ + id: 'node-oc-client-callback-api', + subject: 'The callback API of the Node.js oc-client', + replacement: 'the promise-based oc-client API' + }); client.renderTemplate( template, data, renderOptions, (err: Error, html: string) => { if (err) { - return callback({ - status: 500, - response: { - code: 'INTERNAL_SERVER_ERROR', - error: err - } - }); + renderFailure(err); + return; } - callback({ - status: 200, - ...(responseHeaders ? { headers: responseHeaders } : {}), - response: Object.assign(response, { html }) - }); + renderSuccess(html); } ); }; diff --git a/packages/oc/src/types.ts b/packages/oc/src/types.ts index 06fe141f7..3017e56c6 100644 --- a/packages/oc/src/types.ts +++ b/packages/oc/src/types.ts @@ -1,5 +1,8 @@ import type { NextFunction, Request, Response } from 'express'; -import type { MetadataStore as MetadataStoreType } from 'oc-metadata-adapters-utils'; +import type { + MetadataStoreLike, + MetadataStore as MetadataStoreType +} from 'oc-metadata-adapters-utils'; import type { StorageAdapter } from 'oc-storage-adapters-utils'; import type { PackageJson } from 'type-fest'; import type { @@ -7,7 +10,11 @@ import type { HttpServerAdapterOptions } from './registry/domain/http-server/types'; -export type { ComponentRow, MetadataStore } from 'oc-metadata-adapters-utils'; +export type { + ComponentRow, + MetadataStore, + MetadataStoreLike +} from 'oc-metadata-adapters-utils'; type Middleware = (req: Request, res: Response, next: NextFunction) => void; @@ -82,7 +89,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) => MetadataStoreType | MetadataStoreLike; options: T; /** * Controls whether OC asks the metadata adapter to manage its schema/table. diff --git a/packages/oc/src/utils/deprecate.ts b/packages/oc/src/utils/deprecate.ts index 05a15d56f..2b1f9f280 100644 --- a/packages/oc/src/utils/deprecate.ts +++ b/packages/oc/src/utils/deprecate.ts @@ -1,6 +1,17 @@ -const warned = new Set(); +const warningStoreKey = Symbol.for('opencomponents.deprecation-warnings'); +const processWithWarningStore = process as typeof process & { + [key: symbol]: unknown; +}; +let warned = processWithWarningStore[warningStoreKey] as + | Set + | undefined; +if (!warned) { + warned = new Set(); + processWithWarningStore[warningStoreKey] = warned; +} +const warnedSet = warned; -interface DeprecationNotice { +export interface DeprecationNotice { /** Stable identifier used to only warn once per process for this deprecation. */ id: string; /** The option/API being removed. */ @@ -24,11 +35,11 @@ export default function deprecate({ subject, replacement }: DeprecationNotice): void { - if (warned.has(id)) { + if (warnedSet.has(id)) { return; } - warned.add(id); + warnedSet.add(id); process.emitWarning( `${subject} is deprecated and will be removed in OpenComponents v1 - use ${replacement} instead.`, 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..75eb9eaaa --- /dev/null +++ b/packages/oc/test/unit/registry-domain-metadata-adapter.js @@ -0,0 +1,97 @@ +const expect = require('chai').expect; +const injectr = require('injectr'); +const sinon = require('sinon'); + +const initialise = () => { + const warned = new Set(); + const emitWarning = sinon.stub(); + const deprecate = sinon.stub().callsFake(({ id, subject, replacement }) => { + if (!warned.has(id)) { + warned.add(id); + emitWarning( + `${subject} is deprecated and will be removed in OpenComponents v1 - use ${replacement} instead.`, + 'DeprecationWarning' + ); + } + }); + const getPromiseBasedMetadataAdapter = injectr( + '../../dist/registry/domain/metadata-adapter.js', + { '../../utils/deprecate': { __esModule: true, default: deprecate } } + ).default; + + return { deprecate, emitWarning, getPromiseBasedMetadataAdapter }; +}; + +describe('registry : domain : metadata-adapter', () => { + it('preserves promise methods without warning', async () => { + const { deprecate, getPromiseBasedMetadataAdapter } = initialise(); + const adapter = { + adapterType: 'test-metadata', + isValid: () => 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() + }; + + const parsed = getPromiseBasedMetadataAdapter(adapter); + + await parsed.initialise(); + expect(deprecate.called).to.be.false; + }); + + it('bridges callback methods and warns once while preserving results', async () => { + const { emitWarning, getPromiseBasedMetadataAdapter } = initialise(); + const adapter = { + adapterType: 'legacy-metadata', + isValid: () => true, + initialise: sinon.stub().yields(null), + getAllComponents: sinon.stub().yields(null, [{ name: 'hello' }]), + addVersion: sinon.stub().yields(null), + reserveVersion: sinon.stub().yields(null, { token: 'token' }), + commitVersion: sinon.stub().yields(null), + abortVersion: sinon.stub().yields(null) + }; + + const parsed = getPromiseBasedMetadataAdapter(adapter); + const callback = sinon.spy(); + + await parsed.initialise(callback); + const rows = await parsed.getAllComponents(); + const reservation = await parsed.reserveVersion({ name: 'hello' }); + + expect(callback.calledOnceWithExactly(null, undefined)).to.be.true; + expect(rows).to.eql([{ name: 'hello' }]); + expect(reservation).to.eql({ token: 'token' }); + expect(emitWarning.calledOnce).to.be.true; + expect(emitWarning.firstCall.args[0]).to.contain( + 'Metadata adapter callbacks' + ); + }); + + it('bridges callback errors and rejects the promise', async () => { + const { getPromiseBasedMetadataAdapter } = initialise(); + const error = new Error('metadata failed'); + const adapter = { + adapterType: 'legacy-metadata', + isValid: () => true, + initialise: sinon.stub().yields(error), + getAllComponents: sinon.stub(), + addVersion: sinon.stub(), + reserveVersion: sinon.stub(), + commitVersion: sinon.stub(), + abortVersion: sinon.stub() + }; + + let result; + try { + await getPromiseBasedMetadataAdapter(adapter).initialise(); + } catch (caught) { + result = caught; + } + + expect(result).to.equal(error); + }); +}); diff --git a/packages/oc/test/unit/registry-domain-plugins-initialiser.js b/packages/oc/test/unit/registry-domain-plugins-initialiser.js index 6e617e6f6..653c6ae30 100644 --- a/packages/oc/test/unit/registry-domain-plugins-initialiser.js +++ b/packages/oc/test/unit/registry-domain-plugins-initialiser.js @@ -1,7 +1,25 @@ const expect = require('chai').expect; +const injectr = require('injectr'); +const sinon = require('sinon'); describe('registry : domain : plugins-initialiser', () => { const pluginsInitialiser = require('../../dist/registry/domain/plugins-initialiser'); + const emitWarning = sinon.stub(); + const warned = new Set(); + const deprecate = sinon.stub().callsFake(({ id, subject, replacement }) => { + if (warned.has(id)) { + return; + } + warned.add(id); + emitWarning( + `${subject} is deprecated and will be removed in OpenComponents v1 - use ${replacement} instead.`, + 'DeprecationWarning' + ); + }); + const pluginsInitialiserWithWarning = injectr( + '../../dist/registry/domain/plugins-initialiser.js', + { '../../utils/deprecate': { __esModule: true, default: deprecate } } + ); describe('when initialising not valid plugins', () => { describe('when plugin not registered correctly', () => { @@ -232,6 +250,54 @@ describe('registry : domain : plugins-initialiser', () => { }); }); + describe('when initialising callback-based plugins', () => { + beforeEach(() => { + deprecate.resetHistory(); + emitWarning.resetHistory(); + warned.clear(); + }); + + it('warns once while preserving callback registration', async () => { + const result = await pluginsInitialiserWithWarning.init([ + { + name: 'callbackPluginA', + register: { + register: (_options, _dependencies, next) => next(), + execute: () => 'a' + } + }, + { + name: 'callbackPluginB', + register: { + register: (_options, _dependencies, next) => next(), + execute: () => 'b' + } + } + ]); + + expect(result.callbackPluginA.handler()).to.equal('a'); + expect(result.callbackPluginB.handler()).to.equal('b'); + expect(emitWarning.calledOnce).to.be.true; + expect(emitWarning.firstCall.args[0]).to.contain( + 'Plugin register callbacks' + ); + }); + + it('does not warn for promise-based registration', async () => { + await pluginsInitialiserWithWarning.init([ + { + name: 'promisePlugin', + register: { + register: async () => {}, + execute: () => 'promise' + } + } + ]); + + expect(deprecate.called).to.be.false; + }); + }); + describe('when plugin specifies dependencies', () => { let passedDeps; let flag; diff --git a/packages/oc/test/unit/registry-domain-server-adapter.js b/packages/oc/test/unit/registry-domain-server-adapter.js index 6f53e1943..6db6ffc04 100644 --- a/packages/oc/test/unit/registry-domain-server-adapter.js +++ b/packages/oc/test/unit/registry-domain-server-adapter.js @@ -1,9 +1,25 @@ const expect = require('chai').expect; +const injectr = require('injectr'); const sinon = require('sinon'); describe('registry : domain : server-adapter', () => { - const getHttpServerAdapter = - require('../../dist/registry/domain/server-adapter').default; + const deprecate = sinon.stub(); + const emitWarning = sinon.stub(); + const warned = new Set(); + deprecate.callsFake(({ id, subject, replacement }) => { + if (warned.has(id)) { + return; + } + warned.add(id); + emitWarning( + `${subject} is deprecated and will be removed in OpenComponents v1 - use ${replacement} instead.`, + 'DeprecationWarning' + ); + }); + const getHttpServerAdapter = injectr( + '../../dist/registry/domain/server-adapter.js', + { '../../utils/deprecate': { __esModule: true, default: deprecate } } + ).default; const createAdapter = () => ({ native: sinon.stub(), @@ -11,6 +27,20 @@ describe('registry : domain : server-adapter', () => { httpServer: sinon.stub() }); + const createPromiseAdapter = () => ({ + ...createAdapter(), + isListening: sinon.stub(), + close: sinon.stub(), + onServerError: sinon.stub(), + supportsPromiseLifecycle: true + }); + + const createLegacyAdapter = () => { + const adapter = createPromiseAdapter(); + adapter.supportsPromiseLifecycle = false; + return adapter; + }; + describe('when given an adapter factory', () => { it('should return the created adapter', () => { const adapter = createAdapter(); @@ -37,4 +67,81 @@ describe('registry : domain : server-adapter', () => { ); }); }); + + it('bridges a legacy adapter lifecycle to promises and warns once', async () => { + deprecate.resetHistory(); + warned.clear(); + emitWarning.resetHistory(); + const adapter = createLegacyAdapter(); + adapter.listen.callsFake((_options, callback) => callback()); + adapter.close.callsFake((callback) => callback()); + const factory = sinon.stub().returns(adapter); + + const parsed = getHttpServerAdapter(factory); + + expect(parsed.supportsPromiseLifecycle).to.be.true; + await parsed.listen({ port: 0, timeout: 1000 }); + await parsed.close(); + + expect(adapter.listen.calledOnce).to.be.true; + expect(adapter.close.calledOnce).to.be.true; + expect(emitWarning.calledOnce).to.be.true; + }); + + it('does not warn for unmarked promise lifecycle methods', async () => { + deprecate.resetHistory(); + warned.clear(); + emitWarning.resetHistory(); + const adapter = createPromiseAdapter(); + delete adapter.supportsPromiseLifecycle; + adapter.listen.resolves(); + adapter.close.resolves(); + + const parsed = getHttpServerAdapter(adapter); + await parsed.listen({ port: 0, timeout: 1000 }); + await parsed.close(); + + expect(emitWarning.called).to.be.false; + }); + + it('uses a returned promise when a lifecycle method also calls back', async () => { + const error = new Error('promise failed'); + const adapter = createLegacyAdapter(); + adapter.listen.callsFake((_options, callback) => { + callback(); + return Promise.reject(error); + }); + const parsed = getHttpServerAdapter(adapter); + + let actualError; + try { + await parsed.listen({ port: 0, timeout: 1000 }); + } catch (caught) { + actualError = caught; + } + + expect(actualError).to.equal(error); + }); + + it('preserves legacy lifecycle callbacks and warns once', (done) => { + deprecate.resetHistory(); + warned.clear(); + emitWarning.resetHistory(); + const adapter = createLegacyAdapter(); + adapter.listen.callsFake((_options, callback) => callback()); + adapter.close.callsFake((callback) => callback()); + const parsed = getHttpServerAdapter(sinon.stub().returns(adapter)); + + parsed.listen({ port: 0, timeout: 1000 }, (listenError) => { + expect(listenError).to.be.undefined; + parsed.close((closeError) => { + expect(closeError).to.be.undefined; + expect(emitWarning.calledOnce).to.be.true; + expect(emitWarning.firstCall.args[0]).to.contain( + 'HTTP server adapter callback API' + ); + done(); + }); + }); + }); }); diff --git a/packages/oc/test/unit/registry-domain-storage-adapter.js b/packages/oc/test/unit/registry-domain-storage-adapter.js index a6e2b549a..1f6570357 100644 --- a/packages/oc/test/unit/registry-domain-storage-adapter.js +++ b/packages/oc/test/unit/registry-domain-storage-adapter.js @@ -24,19 +24,54 @@ function mockLegacyAdapter() { putDir: sinon.stub().yields(), putFile: sinon.stub().yields(), putFileContent: sinon.stub().yields(), + removeDir: sinon.stub().yields(), + removeFile: sinon.stub().yields(), getUrl: sinon.stub().returns(''), maxConcurrentRequests: 20, - adapterType: 's3' + adapterType: 's3', + isValid: sinon.stub().returns(true) }; } let process; -function initialise(adapter) { +function initialiseWithRealAdapter(adapter) { process = { emitWarning: sinon.stub() }; const adapterParser = injectr( '../../dist/registry/domain/storage-adapter.js', - { universalify: { fromCallback: sinon.stub().returns('promisified') } }, + { + universalify: require('universalify'), + '../../utils/deprecate': { + __esModule: true, + default: ({ subject, replacement }) => + process.emitWarning( + `${subject} is deprecated and will be removed in OpenComponents v1 - use ${replacement} instead.`, + 'DeprecationWarning' + ) + } + }, + { process } + ).default; + + return adapterParser(adapter); +} + +function initialise(adapter, warningProcess, deprecate) { + process = warningProcess || { emitWarning: sinon.stub() }; + deprecate = deprecate || sinon.stub().callsFake(({ id, subject, replacement }) => { + process.emitWarning( + `${subject} is deprecated and will be removed in OpenComponents v1 - use ${replacement} instead.`, + 'DeprecationWarning' + ); + deprecate.ids = deprecate.ids || []; + deprecate.ids.push(id); + }); + const adapterParser = injectr( + '../../dist/registry/domain/storage-adapter.js', + { + universalify: { fromCallback: sinon.stub().returns('promisified') }, + '../../utils/deprecate': { __esModule: true, default: deprecate } + }, { process } ).default; @@ -75,7 +110,7 @@ describe('registry : domain : 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.' + 'Storage adapter callbacks' ); expect(process.emitWarning.args[0][1]).to.contain('DeprecationWarning'); }); @@ -92,6 +127,60 @@ describe('registry : domain : adapter', () => { expect(parsed.putDir).to.be.equal('promisified'); expect(parsed.putFile).to.be.equal('promisified'); expect(parsed.putFileContent).to.be.equal('promisified'); + expect(parsed.removeDir).to.be.equal('promisified'); + expect(parsed.removeFile).to.be.equal('promisified'); + expect(parsed.isValid()).to.equal(true); + }); + + it('only warns once for repeated legacy adapter construction', () => { + const warningProcess = { emitWarning: sinon.stub() }; + const warned = new Set(); + const deprecate = sinon.stub().callsFake(({ id, subject, replacement }) => { + if (warned.has(id)) { + return; + } + warned.add(id); + warningProcess.emitWarning( + `${subject} is deprecated and will be removed in OpenComponents v1 - use ${replacement} instead.`, + 'DeprecationWarning' + ); + }); + initialise(mockLegacyAdapter(), warningProcess, deprecate); + initialise(mockLegacyAdapter(), warningProcess, deprecate); + + expect(warningProcess.emitWarning.calledOnce).to.be.true; }); + + it('preserves callback results through the promise adapter', async () => { + const adapter = initialiseWithRealAdapter({ + ...mockLegacyAdapter(), + getFile: sinon.stub().yields(null, 'file contents') + }); + + const result = await adapter.getFile('path'); + + expect(result).to.equal('file contents'); }); + + it('preserves the legacy adapter receiver', async () => { + const legacyAdapter = { + ...mockLegacyAdapter(), + prefix: 'stored', + getFile(filePath, callback) { + callback(null, `${this.prefix}:${filePath}`); + }, + getUrl() { + return this.prefix; + }, + isValid() { + return this.prefix === 'stored'; + } + }; + const adapter = initialiseWithRealAdapter(legacyAdapter); + + expect(await adapter.getFile('path')).to.equal('stored:path'); + expect(adapter.getUrl()).to.equal('stored'); + expect(adapter.isValid()).to.be.true; + }); +}); }); diff --git a/packages/oc/test/unit/registry-routes-helpers-get-component.js b/packages/oc/test/unit/registry-routes-helpers-get-component.js index bf3f8bd67..6dcf9fec1 100644 --- a/packages/oc/test/unit/registry-routes-helpers-get-component.js +++ b/packages/oc/test/unit/registry-routes-helpers-get-component.js @@ -12,8 +12,10 @@ describe('registry : routes : helpers : get-component', () => { 'oc-template-jade': require('oc-template-jade'), 'oc-template-handlebars': require('oc-template-handlebars') }; - const initialise = (params) => { + let deprecateStub; + const initialise = (params, clientFactory) => { fireStub = sinon.stub(); + deprecateStub = sinon.stub(); GetComponent = injectr( '../../dist/registry/routes/helpers/get-component.js', { @@ -21,16 +23,27 @@ describe('registry : routes : helpers : get-component', () => { on: () => {}, fire: fireStub }, - 'oc-client': () => { - const client = Client(); - return { - renderTemplate: (template, data, renderOptions, cb) => { - if (renderOptions.templateType === 'oc-template-supported') { - renderOptions.templateType = 'oc-template-jade'; + 'oc-client': + clientFactory || + (() => { + const client = Client(); + return { + renderTemplate: (template, data, renderOptions, cb) => { + if (renderOptions.templateType === 'oc-template-supported') { + renderOptions.templateType = 'oc-template-jade'; + } + return client.renderTemplate( + template, + data, + renderOptions, + cb + ); } - return client.renderTemplate(template, data, renderOptions, cb); - } - }; + }; + }), + '../../../utils/deprecate': { + __esModule: true, + default: deprecateStub } }, { console, Buffer, clearTimeout, setTimeout } @@ -189,6 +202,108 @@ describe('registry : routes : helpers : get-component', () => { }); }); + describe('when using the legacy node oc-client renderTemplate API', () => { + it('emits one deprecation notice while preserving rendering', (done) => { + initialise(mockedComponents['async-error2-component']); + const getComponent = GetComponent({}, mockedRepository); + + getComponent( + { + name: 'async-error2-component', + headers: {}, + parameters: {}, + version: '1.X.X', + conf: { baseUrl: 'http://components.com/' } + }, + () => { + expect(deprecateStub.calledOnce).to.be.true; + expect(deprecateStub.firstCall.args[0]).to.include({ + id: 'node-oc-client-callback-api' + }); + done(); + } + ); + }); + }); + + describe('when using the promise node oc-client renderTemplate API', () => { + it('renders without a deprecation notice', (done) => { + initialise(mockedComponents['async-error2-component'], () => ({ + supportsPromiseApi: true, + renderTemplate: sinon.stub().resolves('

promise result

') + })); + const getComponent = GetComponent({}, mockedRepository); + + getComponent( + { + name: 'async-error2-component', + headers: {}, + parameters: {}, + version: '1.X.X', + conf: { baseUrl: 'http://components.com/' } + }, + (result) => { + expect(result.status).to.equal(200); + expect(result.response.html).to.equal('

promise result

'); + expect(deprecateStub.called).to.be.false; + done(); + } + ); + }); + + it('turns promise and synchronous failures into server errors', async () => { + const error = new Error('render failed'); + for (const renderTemplate of [ + sinon.stub().rejects(error), + sinon.stub().throws(error) + ]) { + initialise(mockedComponents['async-error2-component'], () => ({ + supportsPromiseApi: true, + renderTemplate + })); + const getComponent = GetComponent({}, mockedRepository); + const result = await new Promise((resolve) => + getComponent( + { + name: 'async-error2-component', + headers: {}, + parameters: {}, + version: '1.X.X', + conf: { baseUrl: 'http://components.com/' } + }, + resolve + ) + ); + + expect(result.status).to.equal(500); + expect(result.response.error).to.equal(error); + } + }); + }); + + it('keeps unmarked rest-parameter clients on the callback path', (done) => { + initialise(mockedComponents['async-error2-component'], () => ({ + renderTemplate: (...args) => args.at(-1)(null, '

legacy result

') + })); + const getComponent = GetComponent({}, mockedRepository); + + getComponent( + { + name: 'async-error2-component', + headers: {}, + parameters: {}, + version: '1.X.X', + conf: { baseUrl: 'http://components.com/' } + }, + (result) => { + expect(result.status).to.equal(200); + expect(result.response.html).to.equal('

legacy result

'); + expect(deprecateStub.calledOnce).to.be.true; + done(); + } + ); + }); + describe('when the component sends a custom status code', () => { before((done) => { initialise(mockedComponents['async-custom-error-component']); diff --git a/packages/oc/test/unit/utils-deprecate.js b/packages/oc/test/unit/utils-deprecate.js index 8bc9ea96f..f117f9715 100644 --- a/packages/oc/test/unit/utils-deprecate.js +++ b/packages/oc/test/unit/utils-deprecate.js @@ -48,4 +48,24 @@ describe('utils : deprecate', () => { expect(emitWarning.calledTwice).to.be.true; }); + + it('shares warning identity across separately loaded utility modules', () => { + const emitWarning = sinon.stub(); + const processMock = { emitWarning }; + const first = injectr( + '../../dist/utils/deprecate.js', + {}, + { process: processMock } + ).default; + const second = injectr( + '../../dist/utils/deprecate.js', + {}, + { process: processMock } + ).default; + + first({ id: 'shared-id', subject: 'X', replacement: 'Y' }); + second({ id: 'shared-id', subject: 'X', replacement: 'Y' }); + + expect(emitWarning.calledOnce).to.be.true; + }); });