accept client providers via Observable/AsyncGenerator (#192)

sqs · web-flow · commit 779a4b2ac1f1 · 2024-08-03T00:31:54.000Z
This lets that value change after initial client creation.
diff --git 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
@@ -65,7 +65,10 @@ export function createController({
     outputChannel: vscode.OutputChannel
     getAuthInfo?: (secrets: vscode.SecretStorage, providerUri: string) => Promise<AuthInfo | null>
     features: { annotations?: boolean; statusBar?: boolean }
-    providers?: ImportedProviderConfiguration[]
+    providers?:
+        | ImportedProviderConfiguration[]
+        | Observable<ImportedProviderConfiguration[]>
+        | (() => AsyncGenerator<ImportedProviderConfiguration[]>)
     mergeConfiguration?: (configuration: ClientConfiguration) => Promise<ClientConfiguration>
     preloadDelay?: number
 }): {
diff --git 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
@@ -16,6 +16,7 @@ import {
     combineLatest,
     defer,
     distinctUntilChanged,
+    filter,
     from,
     map,
     mergeMap,
@@ -72,14 +73,22 @@ function observeProviderCall<R>(
     fn: (provider: ProviderClientWithSettings) => Observable<R[] | null>,
     { emitPartial, errorHook, logger }: Pick<ClientEnv<never>, 'logger'> & ObserveOptions,
 ): Observable<EachWithProviderUri<R[]>> {
+    // 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
                 ? combineLatest(
                       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<R>(
                               )
                               .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<R[]> => v !== null).flat()),
+        filter(
+            result =>
+                !emitPartial || result.length === 0 || result.some(v => v !== EMIT_PARTIAL_SENTINEL),
+        ),
+        map(result =>
+            result
+                .filter((v): v is EachWithProviderUri<R[]> => 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
@@ -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<ImportedProviderConfiguration[]> {
+                    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<MetaResult[]>[] = []
+            const signal = new AbortController().signal
+            for await (const value of client.metaChanges__asyncGenerator({}, {}, signal)) {
+                values.push(value)
+            }
+            expect(values).toStrictEqual<typeof values>([
+                [{ 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<typeof values>([
-                [],
                 [{ 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<string, EachWithProviderUri<Item[]>>)
             })
@@ -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<string, EachWithProviderUri<Annotation[]>>)
             })
diff --git 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<R extends Range> {
     /**
      * The list of providers already resolved and imported.
      */
-    providers?: ImportedProviderConfiguration[]
+    providers?:
+        | ImportedProviderConfiguration[]
+        | Observable<ImportedProviderConfiguration[]>
+        | (() => AsyncGenerator<ImportedProviderConfiguration[]>)
 
     /**
      * The authentication info for the provider.
@@ -337,17 +341,22 @@ export function createClient<R extends Range>(env: ClientEnv<R>): Client<R> {
     }
 
     function providerClientsWithSettings(resource?: string): Observable<ProviderClientWithSettings[]> {
-        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<R extends Range>(env: ClientEnv<R>): Client<R> {
                                                         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
@@ -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<number>): Promise<number[]> {
+        return new Promise<number[]>(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<void>(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<void>(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
diff --git a/lib/client/src/index.ts b/lib/client/src/index.ts

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@openctx/vscode-lib",`
`3`		`- "version": "0.0.17",`
	`3`	`+ "version": "0.0.18",`
`4`	`4`	`"description": "OpenCtx library for VS Code extensions",`
`5`	`5`	`"license": "Apache-2.0",`
`6`	`6`	`"repository": {`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@openctx/client",`
`3`		`- "version": "0.0.22",`
	`3`	`+ "version": "0.0.23",`
`4`	`4`	`"description": "OpenCtx client library",`
`5`	`5`	`"license": "Apache-2.0",`
`6`	`6`	`"repository": {`