From b9340b2a1c479debedf4766484b188d56ec8cc0c Mon Sep 17 00:00:00 2001 From: Quinn Slack Date: Fri, 2 Aug 2024 16:59:49 -0700 Subject: [PATCH] accept client providers via Observable/AsyncGenerator This lets that value change. --- client/vscode-lib/package.json | 2 +- client/vscode-lib/src/controller.ts | 5 +- lib/client/package.json | 2 +- lib/client/src/api.ts | 25 +++++- lib/client/src/client/client.test.ts | 61 ++++++++++--- lib/client/src/client/client.ts | 23 +++-- lib/client/src/client/util.test.ts | 123 ++++++++++++++++++++++++++- lib/client/src/client/util.ts | 38 ++++++++- lib/client/src/index.ts | 5 +- 9 files changed, 258 insertions(+), 26 deletions(-) diff --git a/client/vscode-lib/package.json b/client/vscode-lib/package.json index 5540c102..4e7d42fd 100644 --- a/client/vscode-lib/package.json +++ b/client/vscode-lib/package.json @@ -1,6 +1,6 @@ { "name": "@openctx/vscode-lib", - "version": "0.0.17", + "version": "0.0.18", "description": "OpenCtx library for VS Code extensions", "license": "Apache-2.0", "repository": { diff --git a/client/vscode-lib/src/controller.ts b/client/vscode-lib/src/controller.ts index 9380d948..84e42e22 100644 --- a/client/vscode-lib/src/controller.ts +++ b/client/vscode-lib/src/controller.ts @@ -65,7 +65,10 @@ export function createController({ outputChannel: vscode.OutputChannel getAuthInfo?: (secrets: vscode.SecretStorage, providerUri: string) => Promise features: { annotations?: boolean; statusBar?: boolean } - providers?: ImportedProviderConfiguration[] + providers?: + | ImportedProviderConfiguration[] + | Observable + | (() => AsyncGenerator) mergeConfiguration?: (configuration: ClientConfiguration) => Promise preloadDelay?: number }): { diff --git a/lib/client/package.json b/lib/client/package.json index 9cf10b56..73ef0b34 100644 --- a/lib/client/package.json +++ b/lib/client/package.json @@ -1,6 +1,6 @@ { "name": "@openctx/client", - "version": "0.0.22", + "version": "0.0.23", "description": "OpenCtx client library", "license": "Apache-2.0", "repository": { diff --git a/lib/client/src/api.ts b/lib/client/src/api.ts index dd3c8274..79abc060 100644 --- a/lib/client/src/api.ts +++ b/lib/client/src/api.ts @@ -16,6 +16,7 @@ import { combineLatest, defer, distinctUntilChanged, + filter, from, map, mergeMap, @@ -72,6 +73,14 @@ function observeProviderCall( fn: (provider: ProviderClientWithSettings) => Observable, { emitPartial, errorHook, logger }: Pick, 'logger'> & ObserveOptions, ): Observable> { + // This sentinel value lets us avoid emitting a "fake" partial result when `emitPartial` is + // true. We use `combineLatest` below, which waits until all providers have emitted until it + // emits a value, so we need to use `startWith(null)`. But this means that upon subscription, we + // get a meaningless `[null, null, null, ...]` value. Using this sentinel value instead of + // `null` lets us detect that case and filter it out so our caller doesn't see it. But in the + // case where there are no providers, we still want to emit [] because that is a true result. + const EMIT_PARTIAL_SENTINEL: 'emit-partial-sentinel' = {} as any + return providerClients.pipe( mergeMap(providerClients => providerClients && providerClients.length > 0 @@ -79,7 +88,7 @@ function observeProviderCall( providerClients.map(({ uri, providerClient, settings }) => defer(() => fn({ uri, providerClient, settings })) .pipe( - emitPartial ? startWith(null) : tap(), + emitPartial ? startWith(EMIT_PARTIAL_SENTINEL) : tap(), catchError(error => { if (errorHook) { errorHook(uri, error) @@ -92,14 +101,24 @@ function observeProviderCall( ) .pipe( map(items => - (items || []).map(item => ({ ...item, providerUri: uri })), + items === EMIT_PARTIAL_SENTINEL + ? items + : (items || []).map(item => ({ ...item, providerUri: uri })), ), ), ), ) : of([]), ), - map(result => result.filter((v): v is EachWithProviderUri => v !== null).flat()), + filter( + result => + !emitPartial || result.length === 0 || result.some(v => v !== EMIT_PARTIAL_SENTINEL), + ), + map(result => + result + .filter((v): v is EachWithProviderUri => v !== null && v !== EMIT_PARTIAL_SENTINEL) + .flat(), + ), distinctUntilChanged((a, b) => JSON.stringify(a) === JSON.stringify(b)), tap(items => { if (LOG_VERBOSE) { diff --git a/lib/client/src/client/client.test.ts b/lib/client/src/client/client.test.ts index 8856c5aa..44d55b65 100644 --- a/lib/client/src/client/client.test.ts +++ b/lib/client/src/client/client.test.ts @@ -4,7 +4,7 @@ import { Observable, firstValueFrom, of } from 'rxjs' import { TestScheduler } from 'rxjs/testing' import { describe, expect, test } from 'vitest' import type { Annotation, EachWithProviderUri } from '../api.js' -import type { ConfigurationUserInput } from '../configuration.js' +import type { ConfigurationUserInput, ImportedProviderConfiguration } from '../configuration.js' import { type Client, type ClientEnv, createClient } from './client.js' function testdataFileUri(file: string): string { @@ -45,6 +45,48 @@ describe('Client', () => { new TestScheduler((actual, expected) => expect(actual).toStrictEqual(expected)) describe('meta', () => { + test('meta with async generator providers option', async () => { + const client = createTestClient({ + configuration: () => of({}), + providers: async function* (): AsyncGenerator { + yield [ + { + provider: { meta: () => ({ name: 'my-provider-1' }) }, + providerUri: 'u1', + settings: true, + }, + ] + await new Promise(resolve => setTimeout(resolve)) + yield [ + { + provider: { meta: () => ({ name: 'my-provider-2' }) }, + providerUri: 'u2', + settings: true, + }, + { + provider: { meta: () => ({ name: 'my-provider-3' }) }, + providerUri: 'u3', + settings: true, + }, + ] + }, + }) + + const values: EachWithProviderUri[] = [] + const signal = new AbortController().signal + for await (const value of client.metaChanges__asyncGenerator({}, {}, signal)) { + values.push(value) + } + expect(values).toStrictEqual([ + [{ name: 'my-provider-1', providerUri: 'u1' }], + [{ name: 'my-provider-2', providerUri: 'u2' }], + [ + { name: 'my-provider-2', providerUri: 'u2' }, + { name: 'my-provider-3', providerUri: 'u3' }, + ], + ]) + }) + test('metaChanges__asyncGenerator', async () => { const client = createTestClient({ configuration: () => @@ -54,11 +96,13 @@ describe('Client', () => { enable: true, providers: { [testdataFileUri('simpleMeta.js')]: { nameSuffix: '1' } }, }) - observer.next({ - enable: true, - providers: { [testdataFileUri('simpleMeta.js')]: { nameSuffix: '2' } }, + setTimeout(() => { + observer.next({ + enable: true, + providers: { [testdataFileUri('simpleMeta.js')]: { nameSuffix: '2' } }, + }) + observer.complete() }) - observer.complete() }), }) @@ -68,7 +112,6 @@ describe('Client', () => { values.push(value) } expect(values).toStrictEqual([ - [], [{ name: 'simpleMeta-1', providerUri: testdataFileUri('simpleMeta.js') }], [{ name: 'simpleMeta-2', providerUri: testdataFileUri('simpleMeta.js') }], ]) @@ -118,8 +161,7 @@ describe('Client', () => { }), }, }).itemsChanges(FIXTURE_ITEMS_PARAMS), - ).toBe('(0a)', { - '0': [], + ).toBe('a', { a: [{ ...fixtureItem('a'), providerUri: testdataFileUri('simple.js') }], } satisfies Record>) }) @@ -166,8 +208,7 @@ describe('Client', () => { getProviderClient: () => ({ annotations: () => of([fixtureAnn('a')]) }), }, }).annotationsChanges(FIXTURE_ANNOTATIONS_PARAMS), - ).toBe('(0a)', { - '0': [], + ).toBe('a', { a: [{ ...fixtureAnn('a'), providerUri: testdataFileUri('simple.js') }], } satisfies Record>) }) diff --git a/lib/client/src/client/client.ts b/lib/client/src/client/client.ts index 5b35f598..6c0f344f 100644 --- a/lib/client/src/client/client.ts +++ b/lib/client/src/client/client.ts @@ -20,6 +20,7 @@ import { distinctUntilChanged, firstValueFrom, from, + isObservable, map, mergeMap, of, @@ -45,7 +46,7 @@ import { } from '../configuration.js' import type { Logger } from '../logger.js' import { type ProviderClient, createProviderClient } from '../providerClient/createProviderClient.js' -import { observableToAsyncGenerator } from './util.js' +import { asyncGeneratorToObservable, observableToAsyncGenerator } from './util.js' /** * Hooks for the OpenCtx {@link Client} to access information about the environment, such as the @@ -75,7 +76,10 @@ export interface ClientEnv { /** * The list of providers already resolved and imported. */ - providers?: ImportedProviderConfiguration[] + providers?: + | ImportedProviderConfiguration[] + | Observable + | (() => AsyncGenerator) /** * The authentication info for the provider. @@ -337,17 +341,22 @@ export function createClient(env: ClientEnv): Client { } function providerClientsWithSettings(resource?: string): Observable { - return from(env.configuration(resource)) + const providers = isObservable(env.providers) + ? env.providers + : env.providers instanceof Function + ? asyncGeneratorToObservable(env.providers()) + : of(env.providers) + return combineLatest([env.configuration(resource), providers]) .pipe( - map(config => { + map(([config, providers]) => { if (!config.enable) { config = { ...config, providers: {} } } - return configurationFromUserInput(config, env.providers) + return [configurationFromUserInput(config, providers), providers] as const }), ) .pipe( - mergeMap(configuration => + mergeMap(([configuration, providers]) => configuration.providers.length > 0 ? combineLatest( configuration.providers.map(({ providerUri, settings }) => @@ -363,7 +372,7 @@ export function createClient(env: ClientEnv): Client { logger, importProvider: env.importProvider, }, - env.providers?.find( + providers?.find( provider => provider.providerUri === providerUri, )?.provider, ), diff --git a/lib/client/src/client/util.test.ts b/lib/client/src/client/util.test.ts index 91a12548..2e7da29e 100644 --- a/lib/client/src/client/util.test.ts +++ b/lib/client/src/client/util.test.ts @@ -1,6 +1,6 @@ import { Observable } from 'rxjs' import { describe, expect, test } from 'vitest' -import { observableToAsyncGenerator } from './util.js' +import { asyncGeneratorToObservable, isAsyncGenerator, observableToAsyncGenerator } from './util.js' describe('observableToAsyncGenerator', () => { test('observable that emits and completes', async () => { @@ -69,3 +69,124 @@ describe('observableToAsyncGenerator', () => { expect(results).toEqual([1, 2, 3, 4, 5]) }) }) +describe('asyncGeneratorToObservable', () => { + function readObservable(observable: Observable): Promise { + return new Promise(resolve => { + const results: number[] = [] + observable.subscribe({ + next: value => results.push(value), + complete: () => resolve(results), + }) + }) + } + + test('async generator that yields and completes', async () => { + const observable = asyncGeneratorToObservable( + (async function* () { + yield 1 + yield 2 + yield 3 + })(), + ) + expect(await readObservable(observable)).toEqual([1, 2, 3]) + }) + + test('async generator that throws an error', async () => { + const ERROR = new Error('Test error') + async function* generator() { + yield 1 + throw ERROR + } + + const observable = asyncGeneratorToObservable(generator()) + const results: number[] = [] + let error: Error | null = null + + await new Promise(resolve => { + observable.subscribe({ + next: value => results.push(value), + error: err => { + error = err + resolve() + }, + complete: () => resolve(), + }) + }) + + expect(results).toEqual([1]) + expect(error).toBe(ERROR) + }) + + test('async generator with no yields', async () => { + async function* generator() { + // Empty generator + } + + const observable = asyncGeneratorToObservable(generator()) + let completed = false + + await new Promise(resolve => { + observable.subscribe({ + next: () => expect.fail('should not yield any values'), + complete: () => { + completed = true + resolve() + }, + }) + }) + + expect(completed).toBe(true) + }) +}) + +describe('isAsyncGenerator', () => { + test('true for valid async generator', () => { + async function* validAsyncGenerator() { + yield 1 + } + expect(isAsyncGenerator(validAsyncGenerator())).toBe(true) + }) + + test('false for other values', () => { + expect(isAsyncGenerator(42)).toBe(false) + expect(isAsyncGenerator('string')).toBe(false) + expect(isAsyncGenerator(true)).toBe(false) + expect(isAsyncGenerator(undefined)).toBe(false) + expect(isAsyncGenerator(null)).toBe(false) + expect(isAsyncGenerator({})).toBe(false) + expect(isAsyncGenerator(function regularFunction() {})).toBe(false) + }) + + test('false for async functions', () => { + async function asyncFunction() {} + expect(isAsyncGenerator(asyncFunction)).toBe(false) + }) + + test('false for non-async generator functions', () => { + function* generatorFunction() { + yield 1 + } + expect(isAsyncGenerator(generatorFunction())).toBe(false) + }) + + test('false for objects with some but not all required methods', () => { + const incompleteObject = { + next: () => {}, + throw: () => {}, + [Symbol.asyncIterator]: function () { + return this + }, + } + expect(isAsyncGenerator(incompleteObject)).toBe(false) + }) + + test('false for objects with all methods but incorrect Symbol.asyncIterator implementation', () => { + const incorrectObject = { + next: () => {}, + throw: () => {}, + return: () => {}, + [Symbol.asyncIterator]: () => ({}), + } + expect(isAsyncGenerator(incorrectObject)).toBe(false) + }) +}) diff --git a/lib/client/src/client/util.ts b/lib/client/src/client/util.ts index ba1bfed1..e17fc7da 100644 --- a/lib/client/src/client/util.ts +++ b/lib/client/src/client/util.ts @@ -1,4 +1,4 @@ -import type { Observable } from 'rxjs' +import { Observable } from 'rxjs' export async function* observableToAsyncGenerator( observable: Observable, @@ -61,3 +61,39 @@ export async function* observableToAsyncGenerator( removeAbortListener?.() } } + +export function asyncGeneratorToObservable(asyncGenerator: AsyncGenerator): Observable { + return new Observable(observer => { + ;(async () => { + try { + for await (const value of asyncGenerator) { + observer.next(value) + } + observer.complete() + } catch (error) { + observer.error(error) + } + })() + + return () => { + // If the AsyncGenerator has a return method, call it to clean up + if (asyncGenerator.return) { + asyncGenerator.return() + } + } + }) +} + +export function isAsyncGenerator(value: any): value is AsyncGenerator { + if (value === null || typeof value !== 'object') { + return false + } + + return ( + typeof value.next === 'function' && + typeof value.throw === 'function' && + typeof value.return === 'function' && + typeof value[Symbol.asyncIterator] === 'function' && + value[Symbol.asyncIterator]() === value + ) +} diff --git a/lib/client/src/index.ts b/lib/client/src/index.ts index 054b900e..df102a16 100644 --- a/lib/client/src/index.ts +++ b/lib/client/src/index.ts @@ -9,6 +9,9 @@ export { type ClientEnv, type ProviderMethodOptions, } from './client/client.js' -export { type ConfigurationUserInput as ClientConfiguration } from './configuration.js' +export { + type ConfigurationUserInput as ClientConfiguration, + type ImportedProviderConfiguration, +} from './configuration.js' export type { Logger } from './logger.js' export { fetchProviderSource } from './providerClient/transport/module.js'