-
Notifications
You must be signed in to change notification settings - Fork 2
Add search features API #95
Changes from 16 commits
efa285d
5e4fe5b
de15667
f5fd2be
e10a9fc
25ca64b
c76d18c
8a1c7aa
39590b9
4847f2b
13e53ba
4bbef15
c112fa7
61756bc
f1326e3
480071b
4c57cae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { from, Observable, Subscription } from 'rxjs' | ||
import { ExtSearch } from 'src/extension/api/search' | ||
import { Connection } from 'src/protocol/jsonrpc2/connection' | ||
import { createProxyAndHandleRequests } from '../../common/proxy' | ||
import { TransformQuerySignature } from '../providers/queryTransformer' | ||
import { FeatureProviderRegistry } from '../providers/registry' | ||
import { SubscriptionMap } from './common' | ||
|
||
/** @internal */ | ||
export interface SearchAPI { | ||
$registerQueryTransformer(id: number): void | ||
$unregister(id: number): void | ||
} | ||
|
||
/** @internal */ | ||
export class Search implements SearchAPI { | ||
private subscriptions = new Subscription() | ||
private registrations = new SubscriptionMap() | ||
private proxy: ExtSearch | ||
|
||
constructor( | ||
connection: Connection, | ||
private queryTransformerRegistry: FeatureProviderRegistry<{}, TransformQuerySignature> | ||
) { | ||
this.subscriptions.add(this.registrations) | ||
|
||
this.proxy = createProxyAndHandleRequests('search', connection, this) | ||
} | ||
|
||
public $registerQueryTransformer(id: number): void { | ||
this.registrations.add( | ||
id, | ||
this.queryTransformerRegistry.registerProvider( | ||
{}, | ||
(query: string): Observable<string> => from(this.proxy.$transformQuery(id, query)) | ||
) | ||
) | ||
} | ||
|
||
public $unregister(id: number): void { | ||
this.registrations.remove(id) | ||
} | ||
|
||
public unsubscribe(): void { | ||
this.subscriptions.unsubscribe() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
import * as assert from 'assert' | ||
import { of } from 'rxjs' | ||
import { TestScheduler } from 'rxjs/testing' | ||
import { transformQuery, TransformQuerySignature } from './queryTransformer' | ||
|
||
const scheduler = () => new TestScheduler((a, b) => assert.deepStrictEqual(a, b)) | ||
|
||
const FIXTURE_INPUT = 'foo' | ||
const FIXTURE_RESULT = 'bar' | ||
const FIXTURE_RESULT_TWO = 'qux' | ||
const FIXTURE_RESULT_MERGED = 'foo bar qux' | ||
|
||
describe('transformQuery', () => { | ||
describe('0 providers', () => { | ||
it('returns original query', () => | ||
scheduler().run(({ cold, expectObservable }) => | ||
expectObservable( | ||
transformQuery(cold<TransformQuerySignature[]>('-a-|', { a: [] }), FIXTURE_INPUT) | ||
).toBe('-a-|', { | ||
a: FIXTURE_INPUT, | ||
}) | ||
)) | ||
}) | ||
|
||
describe('1 provider', () => { | ||
it('returns result from provider', () => | ||
scheduler().run(({ cold, expectObservable }) => | ||
expectObservable( | ||
transformQuery( | ||
cold<TransformQuerySignature[]>('-a-|', { | ||
a: [q => of(FIXTURE_RESULT)], | ||
}), | ||
FIXTURE_INPUT | ||
) | ||
).toBe('-a-|', { a: FIXTURE_RESULT }) | ||
)) | ||
}) | ||
|
||
describe('2 providers', () => { | ||
it('returns a single query transformed by both providers', () => | ||
scheduler().run(({ cold, expectObservable }) => | ||
expectObservable( | ||
transformQuery( | ||
cold<TransformQuerySignature[]>('-a-|', { | ||
a: [q => of(`${q} ${FIXTURE_RESULT}`), q => of(`${q} ${FIXTURE_RESULT_TWO}`)], | ||
}), | ||
FIXTURE_INPUT | ||
) | ||
).toBe('-a-|', { a: FIXTURE_RESULT_MERGED }) | ||
)) | ||
}) | ||
|
||
describe('Multiple emissions', () => { | ||
it('returns stream of results', () => | ||
scheduler().run(({ cold, expectObservable }) => | ||
expectObservable( | ||
transformQuery( | ||
cold<TransformQuerySignature[]>('-a-b-|', { | ||
a: [q => of(`${q} ${FIXTURE_RESULT}`)], | ||
b: [q => of(`${q} ${FIXTURE_RESULT_TWO}`)], | ||
}), | ||
FIXTURE_INPUT | ||
) | ||
).toBe('-a-b-|', { | ||
a: `${FIXTURE_INPUT} ${FIXTURE_RESULT}`, | ||
b: `${FIXTURE_INPUT} ${FIXTURE_RESULT_TWO}`, | ||
}) | ||
)) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { Observable, of } from 'rxjs' | ||
import { flatMap, map, switchMap } from 'rxjs/operators' | ||
import { FeatureProviderRegistry } from './registry' | ||
|
||
export type TransformQuerySignature = (query: string) => Observable<string> | ||
|
||
/** Transforms search queries using registered query transformers from extensions. */ | ||
export class QueryTransformerRegistry extends FeatureProviderRegistry<{}, TransformQuerySignature> { | ||
public transformQuery(query: string): Observable<string> { | ||
return transformQuery(this.providers, query) | ||
} | ||
} | ||
|
||
/** | ||
* Returns an observable that emits a query transformed by all providers whenever any of the last-emitted set of providers emits | ||
* a query. | ||
* | ||
* Most callers should use QueryTransformerRegistry's transformQuery method, which uses the registered query transformers | ||
* | ||
*/ | ||
export function transformQuery(providers: Observable<TransformQuerySignature[]>, query: string): Observable<string> { | ||
return providers.pipe( | ||
switchMap(providers => { | ||
if (providers.length === 0) { | ||
return [query] | ||
} | ||
return providers.reduce( | ||
(currentQuery, transformQuery) => | ||
currentQuery.pipe(flatMap(q => transformQuery(q).pipe(map(transformedQuery => transformedQuery)))), | ||
of(query) | ||
) | ||
}) | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import { Unsubscribable } from 'rxjs' | ||
import { QueryTransformer } from 'sourcegraph' | ||
import { SearchAPI } from 'src/client/api/search' | ||
import { ProviderMap } from './common' | ||
|
||
export interface ExtSearchAPI { | ||
$transformQuery: (id: number, query: string) => Promise<string> | ||
} | ||
|
||
export class ExtSearch implements ExtSearchAPI { | ||
private registrations = new ProviderMap<QueryTransformer>(id => this.proxy.$unregister(id)) | ||
constructor(private proxy: SearchAPI) {} | ||
|
||
public registerQueryTransformer(provider: QueryTransformer): Unsubscribable { | ||
const { id, subscription } = this.registrations.add(provider) | ||
this.proxy.$registerQueryTransformer(id) | ||
return subscription | ||
} | ||
|
||
public $transformQuery(id: number, query: string): Promise<string> { | ||
const provider = this.registrations.get<QueryTransformer>(id) | ||
return Promise.resolve(provider.transformQuery(query)) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import * as assert from 'assert' | ||
import { take } from 'rxjs/operators' | ||
import { integrationTestContext } from './helpers.test' | ||
|
||
describe('search (integration)', () => { | ||
it('registers a query transformer', async () => { | ||
const { clientController, extensionHost, ready } = await integrationTestContext() | ||
|
||
// Register the provider and call it | ||
const unsubscribe = extensionHost.search.registerQueryTransformer({ transformQuery: () => 'bar' }) | ||
await ready | ||
assert.deepStrictEqual( | ||
await clientController.registries.queryTransformer | ||
.transformQuery('foo') | ||
.pipe(take(1)) | ||
.toPromise(), | ||
'bar' | ||
) | ||
|
||
// Unregister the provider and ensure it's removed. | ||
unsubscribe.unsubscribe() | ||
assert.deepStrictEqual( | ||
await clientController.registries.queryTransformer | ||
.transformQuery('foo') | ||
.pipe(take(1)) | ||
.toPromise(), | ||
'foo' | ||
) | ||
}) | ||
|
||
it('supports multiple query transformers', async () => { | ||
const { clientController, extensionHost, ready } = await integrationTestContext() | ||
|
||
// Register the provider and call it | ||
extensionHost.search.registerQueryTransformer({ transformQuery: q => `${q} bar` }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor, but |
||
extensionHost.search.registerQueryTransformer({ transformQuery: q => `${q} qux` }) | ||
await ready | ||
assert.deepStrictEqual( | ||
await clientController.registries.queryTransformer | ||
.transformQuery('foo') | ||
.pipe(take(1)) | ||
.toPromise(), | ||
'foo bar qux' | ||
) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -785,6 +785,37 @@ declare module 'sourcegraph' { | |
): Unsubscribable | ||
} | ||
|
||
/** | ||
* A query transformer alters a user's search query before executing a search. | ||
* | ||
* Query transformers allow extensions to define new search query operators and syntax, for example, | ||
* by matching strings in a query (e.g. `go.imports:`) and replacing them with a regular expression or string. | ||
*/ | ||
export interface QueryTransformer { | ||
/** | ||
* Transforms a search query into another, valid query. If there are no transformations to be made | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd replace "the original query is returned" with ", it should return the original query" |
||
* the original query is returned. | ||
* | ||
* @param query A search query provided by a client. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just "A search query" is fine, no need to say "provided by a client" (not sure what that adds) |
||
*/ | ||
transformQuery(query: string): string | Promise<string> | ||
} | ||
|
||
/** | ||
* API for extensions to augment search functionality. | ||
*/ | ||
export namespace search { | ||
/** | ||
* Registers a query transformer. | ||
* | ||
* Multiple transformers can be registered. In that case, all transformers will be executed | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is more idiomatic to say a transformation is applied, not executed. So replace "executed" with "applied". Also, the order in which multiple transformers are applied is undefined (i.e., the extension API makes no guarantee). So I'd state that. |
||
* and the result is a single query that has been altered by all transformers. | ||
* | ||
* @param provider A query transformer. | ||
*/ | ||
export function registerQueryTransformer(provider: QueryTransformer): Unsubscribable | ||
} | ||
|
||
/** | ||
* Commands are functions that are implemented and registered by extensions. Extensions can invoke any command | ||
* (including commands registered by other extensions). The extension can also define contributions (in | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add
/** @internal */
tsdoc directives as in https://sourcegraph.com/github.com/sourcegraph/sourcegraph-extension-api/-/blob/src/client/api/configuration.ts#L8:1 and other files because this type is part of the internal API (so we want to communicate that to readers and to tools like doc generators).