Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accept client providers via Observable/AsyncGenerator #192

Merged
merged 1 commit into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/vscode-lib/package.json
Original file line number Diff line number Diff line change
@@ -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": {
Expand Down
5 changes: 4 additions & 1 deletion client/vscode-lib/src/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}): {
Expand Down
2 changes: 1 addition & 1 deletion lib/client/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openctx/client",
"version": "0.0.22",
"version": "0.0.23",
"description": "OpenCtx client library",
"license": "Apache-2.0",
"repository": {
Expand Down
25 changes: 22 additions & 3 deletions lib/client/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
combineLatest,
defer,
distinctUntilChanged,
filter,
from,
map,
mergeMap,
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
61 changes: 51 additions & 10 deletions lib/client/src/client/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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: () =>
Expand All @@ -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()
}),
})

Expand All @@ -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') }],
])
Expand Down Expand Up @@ -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[]>>)
})
Expand Down Expand Up @@ -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[]>>)
})
Expand Down
23 changes: 16 additions & 7 deletions lib/client/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
distinctUntilChanged,
firstValueFrom,
from,
isObservable,
map,
mergeMap,
of,
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 }) =>
Expand All @@ -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,
),
Expand Down
123 changes: 122 additions & 1 deletion lib/client/src/client/util.test.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand Down Expand Up @@ -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)
})
})
Loading
Loading