Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Update resolveDocumentProfile to return a Proxy for attaching context
Browse files Browse the repository at this point in the history
davismcphee committed Jun 5, 2024
1 parent ce85a6a commit 113f66f
Showing 5 changed files with 55 additions and 65 deletions.
Original file line number Diff line number Diff line change
@@ -68,10 +68,7 @@ export const fetchDocuments = (
filter((res) => !isRunningResponse(res)),
map((res) => {
return buildDataTableRecordList(res.rawResponse.hits.hits as EsHitRecord[], dataView, {
processRecord: (record) => {
services.profilesManager.resolveDocumentProfile({ record });
return record;
},
processRecord: (record) => services.profilesManager.resolveDocumentProfile({ record }),
});
})
);
Original file line number Diff line number Diff line change
@@ -78,9 +78,7 @@ export function fetchEsql(
flattened: row,
};

profilesManager.resolveDocumentProfile({ record });

return record;
return profilesManager.resolveDocumentProfile({ record });
});
}
});
Original file line number Diff line number Diff line change
@@ -25,8 +25,9 @@ const {

profilesManagerMock.resolveRootProfile({});
profilesManagerMock.resolveDataSourceProfile({});
profilesManagerMock.resolveDocumentProfile({ record: contextRecordMock });
profilesManagerMock.resolveDocumentProfile({ record: contextRecordMock2 });

const record = profilesManagerMock.resolveDocumentProfile({ record: contextRecordMock });
const record2 = profilesManagerMock.resolveDocumentProfile({ record: contextRecordMock2 });

discoverServiceMock.profilesManager = profilesManagerMock;

@@ -35,7 +36,7 @@ const getProfiles$Spy = jest.spyOn(discoverServiceMock.profilesManager, 'getProf

const render = () => {
return renderHook((props) => useProfiles(props), {
initialProps: { record: contextRecordMock } as GetProfilesOptions,
initialProps: { record } as GetProfilesOptions,
wrapper: ({ children }) => (
<KibanaContextProvider services={discoverServiceMock}>{children}</KibanaContextProvider>
),
@@ -63,11 +64,11 @@ describe('useProfiles', () => {
expect(getProfilesSpy).toHaveBeenCalledTimes(2);
expect(getProfiles$Spy).toHaveBeenCalledTimes(1);
const prevResult = result.current;
rerender({ record: contextRecordMock });
rerender({ record });
expect(getProfilesSpy).toHaveBeenCalledTimes(2);
expect(getProfiles$Spy).toHaveBeenCalledTimes(1);
expect(result.current).toBe(prevResult);
rerender({ record: contextRecordMock2 });
rerender({ record: record2 });
expect(getProfilesSpy).toHaveBeenCalledTimes(3);
expect(getProfiles$Spy).toHaveBeenCalledTimes(2);
expect(result.current).toBe(prevResult);
@@ -78,7 +79,7 @@ describe('useProfiles', () => {
expect(getProfilesSpy).toHaveBeenCalledTimes(2);
expect(getProfiles$Spy).toHaveBeenCalledTimes(1);
const prevResult = result.current;
rerender({ record: contextRecordMock });
rerender({ record });
expect(getProfilesSpy).toHaveBeenCalledTimes(2);
expect(getProfiles$Spy).toHaveBeenCalledTimes(1);
expect(result.current).toBe(prevResult);
Original file line number Diff line number Diff line change
@@ -39,16 +39,20 @@ describe('ProfilesManager', () => {
});

it('should resolve document profile', async () => {
mocks.profilesManagerMock.resolveDocumentProfile({ record: mocks.contextRecordMock });
const profiles = mocks.profilesManagerMock.getProfiles({ record: mocks.contextRecordMock });
const record = mocks.profilesManagerMock.resolveDocumentProfile({
record: mocks.contextRecordMock,
});
const profiles = mocks.profilesManagerMock.getProfiles({ record });
expect(profiles).toEqual([{}, {}, mocks.documentProfileProviderMock.profile]);
});

it('should resolve multiple profiles', async () => {
await mocks.profilesManagerMock.resolveRootProfile({});
await mocks.profilesManagerMock.resolveDataSourceProfile({});
mocks.profilesManagerMock.resolveDocumentProfile({ record: mocks.contextRecordMock });
const profiles = mocks.profilesManagerMock.getProfiles({ record: mocks.contextRecordMock });
const record = mocks.profilesManagerMock.resolveDocumentProfile({
record: mocks.contextRecordMock,
});
const profiles = mocks.profilesManagerMock.getProfiles({ record });
expect(profiles).toEqual([
mocks.rootProfileProviderMock.profile,
mocks.dataSourceProfileProviderMock.profile,
@@ -58,23 +62,23 @@ describe('ProfilesManager', () => {

it('should expose profiles as an observable', async () => {
const getProfilesSpy = jest.spyOn(mocks.profilesManagerMock, 'getProfiles');
const profiles$ = mocks.profilesManagerMock.getProfiles$({ record: mocks.contextRecordMock });
const record = mocks.profilesManagerMock.resolveDocumentProfile({
record: mocks.contextRecordMock,
});
const profiles$ = mocks.profilesManagerMock.getProfiles$({ record });
const next = jest.fn();
profiles$.subscribe(next);
expect(getProfilesSpy).toHaveBeenCalledTimes(1);
expect(next).toHaveBeenCalledWith([{}, {}, {}]);
expect(next).toHaveBeenCalledWith([{}, {}, mocks.documentProfileProviderMock.profile]);
await mocks.profilesManagerMock.resolveRootProfile({});
expect(getProfilesSpy).toHaveBeenCalledTimes(2);
expect(next).toHaveBeenCalledWith([mocks.rootProfileProviderMock.profile, {}, {}]);
await mocks.profilesManagerMock.resolveDataSourceProfile({});
expect(getProfilesSpy).toHaveBeenCalledTimes(3);
expect(next).toHaveBeenCalledWith([
mocks.rootProfileProviderMock.profile,
mocks.dataSourceProfileProviderMock.profile,
{},
mocks.documentProfileProviderMock.profile,
]);
mocks.profilesManagerMock.resolveDocumentProfile({ record: mocks.contextRecordMock });
expect(getProfilesSpy).toHaveBeenCalledTimes(4);
await mocks.profilesManagerMock.resolveDataSourceProfile({});
expect(getProfilesSpy).toHaveBeenCalledTimes(3);
expect(next).toHaveBeenCalledWith([
mocks.rootProfileProviderMock.profile,
mocks.dataSourceProfileProviderMock.profile,
@@ -157,15 +161,19 @@ describe('ProfilesManager', () => {
});

it('should log an error and fall back to the default profile if document profile resolution fails', () => {
mocks.profilesManagerMock.resolveDocumentProfile({ record: mocks.contextRecordMock });
let profiles = mocks.profilesManagerMock.getProfiles({ record: mocks.contextRecordMock });
const record = mocks.profilesManagerMock.resolveDocumentProfile({
record: mocks.contextRecordMock,
});
let profiles = mocks.profilesManagerMock.getProfiles({ record });
expect(profiles).toEqual([{}, {}, mocks.documentProfileProviderMock.profile]);
const resolveSpy = jest.spyOn(mocks.documentProfileProviderMock, 'resolve');
resolveSpy.mockImplementation(() => {
throw new Error('Failed to resolve');
});
mocks.profilesManagerMock.resolveDocumentProfile({ record: mocks.contextRecordMock2 });
profiles = mocks.profilesManagerMock.getProfiles({ record: mocks.contextRecordMock2 });
const record2 = mocks.profilesManagerMock.resolveDocumentProfile({
record: mocks.contextRecordMock2,
});
profiles = mocks.profilesManagerMock.getProfiles({ record: record2 });
expect(addLog).toHaveBeenCalledWith(
'[ProfilesManager] document context resolution failed with params: {\n "recordId": "logstash-2014.09.09::388::"\n}',
new Error('Failed to resolve')
58 changes: 22 additions & 36 deletions src/plugins/discover/public/context_awareness/profiles_manager.ts
Original file line number Diff line number Diff line change
@@ -8,16 +8,8 @@

import type { DataTableRecord } from '@kbn/discover-utils';
import { isOfAggregateQueryType } from '@kbn/es-query';
import { isEqual, memoize } from 'lodash';
import {
BehaviorSubject,
combineLatest,
distinctUntilChanged,
filter,
map,
Observable,
startWith,
} from 'rxjs';
import { isEqual } from 'lodash';
import { BehaviorSubject, combineLatest, map } from 'rxjs';
import { DataSourceType, isDataSourceType } from '../../common/data_sources';
import { addLog } from '../utils/add_log';
import type {
@@ -53,7 +45,6 @@ export interface GetProfilesOptions {
export class ProfilesManager {
private readonly rootContext$: BehaviorSubject<ContextWithProfileId<RootContext>>;
private readonly dataSourceContext$: BehaviorSubject<ContextWithProfileId<DataSourceContext>>;
private readonly recordId$ = new BehaviorSubject<string | undefined>(undefined);

private prevRootProfileParams?: SerializedRootProfileParams;
private prevDataSourceProfileParams?: SerializedDataSourceProfileParams;
@@ -124,21 +115,27 @@ export class ProfilesManager {
}

public resolveDocumentProfile(params: DocumentProfileProviderParams) {
Object.defineProperty(params.record, 'context', {
get: memoize(() => {
let context = this.documentProfileService.defaultContext;

try {
context = this.documentProfileService.resolve(params);
} catch (e) {
logResolutionError(ContextType.Document, { recordId: params.record.id }, e);
let context: ContextWithProfileId<DocumentContext> | undefined;

return new Proxy(params.record, {
has: (target, prop) => prop === 'context' || Reflect.has(target, prop),
get: (target, prop, receiver) => {
if (prop !== 'context') {
return Reflect.get(target, prop, receiver);
}

if (!context) {
try {
context = this.documentProfileService.resolve(params);
} catch (e) {
logResolutionError(ContextType.Document, { recordId: params.record.id }, e);
context = this.documentProfileService.defaultContext;
}
}

return context;
}),
},
});

this.recordId$.next(params.record.id);
}

public getProfiles({ record }: GetProfilesOptions = {}) {
@@ -152,20 +149,9 @@ export class ProfilesManager {
}

public getProfiles$(options: GetProfilesOptions = {}) {
const observables: Array<Observable<unknown>> = [this.rootContext$, this.dataSourceContext$];
const record = options.record;

if (record) {
observables.push(
this.recordId$.pipe(
startWith(record.id),
distinctUntilChanged(),
filter((recordId) => recordId === record.id)
)
);
}

return combineLatest(observables).pipe(map(() => this.getProfiles(options)));
return combineLatest([this.rootContext$, this.dataSourceContext$]).pipe(
map(() => this.getProfiles(options))
);
}
}

0 comments on commit 113f66f

Please sign in to comment.