Skip to content

Commit

Permalink
[Enterprise Search] Add read-only mode interceptor and error handler (e…
Browse files Browse the repository at this point in the history
…lastic#77569) (elastic#77782)

* Add readOnlyMode prop + callout to Layout component

* Update HttpLogic to initialize readOnlyMode from config_data

+ update App Search & Workplace Search layout to pass readOnlyMode state
- update passed props test to not refer to readOnlyMode, so as not to confuse distinction between props.readOnlyMode (passed on init, can grow stale) and HttpLogic.values.readOnlyMode (will update on every http call)

- DRY out HttpLogic initializeHttp type defs

* Update enterpriseSearchRequestHandler to pass read-only mode header

+ add a custom 503 API response for read-only mode errors that come back from API endpoints (e.g. when attempting to create/edit a document) - this is so we correctly display a flash message error instead of the generic "Error Connecting" state

+ note that we still need to send back read only mode on ALL headers, not just on handleReadOnlyModeError however - this is so that the read-only mode state can updates dynamically on all API polls (e.g. on a 200 GET)

* Add HttpLogic read-only mode interceptor

- which should now dynamically listen / update state every time an Enterprise Search API call is made

+ DRY out isEnterpriseSearchApi helper and making wrapping/branching clearer

* PR feedback: Copy
  • Loading branch information
Constance authored Sep 17, 2020
1 parent 456760b commit 1145806
Show file tree
Hide file tree
Showing 15 changed files with 298 additions and 59 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/enterprise_search/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,6 @@ export const JSON_HEADER = {
Accept: 'application/json', // Required for Enterprise Search APIs
};

export const READ_ONLY_MODE_HEADER = 'x-ent-search-read-only-mode';

export const ENGINES_PAGE_SIZE = 10;
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,17 @@ describe('AppSearchConfigured', () => {
const wrapper = shallow(<AppSearchConfigured />);

expect(wrapper.find(Layout)).toHaveLength(1);
expect(wrapper.find(Layout).prop('readOnlyMode')).toBeFalsy();
expect(wrapper.find(EngineOverview)).toHaveLength(1);
});

it('initializes app data with passed props', () => {
const initializeAppData = jest.fn();
(useActions as jest.Mock).mockImplementation(() => ({ initializeAppData }));

shallow(<AppSearchConfigured readOnlyMode={true} />);
shallow(<AppSearchConfigured ilmEnabled={true} />);

expect(initializeAppData).toHaveBeenCalledWith({ readOnlyMode: true });
expect(initializeAppData).toHaveBeenCalledWith({ ilmEnabled: true });
});

it('does not re-initialize app data', () => {
Expand All @@ -83,6 +84,14 @@ describe('AppSearchConfigured', () => {

expect(wrapper.find(ErrorConnecting)).toHaveLength(1);
});

it('passes readOnlyMode state', () => {
(useValues as jest.Mock).mockImplementation(() => ({ readOnlyMode: true }));

const wrapper = shallow(<AppSearchConfigured />);

expect(wrapper.find(Layout).prop('readOnlyMode')).toEqual(true);
});
});

describe('AppSearchNav', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const AppSearchUnconfigured: React.FC = () => (
export const AppSearchConfigured: React.FC<IInitialAppData> = (props) => {
const { hasInitialized } = useValues(AppLogic);
const { initializeAppData } = useActions(AppLogic);
const { errorConnecting } = useValues(HttpLogic);
const { errorConnecting, readOnlyMode } = useValues(HttpLogic);

useEffect(() => {
if (!hasInitialized) initializeAppData(props);
Expand All @@ -63,7 +63,7 @@ export const AppSearchConfigured: React.FC<IInitialAppData> = (props) => {
<SetupGuide />
</Route>
<Route>
<Layout navigation={<AppSearchNav />}>
<Layout navigation={<AppSearchNav />} readOnlyMode={readOnlyMode}>
{errorConnecting ? (
<ErrorConnecting />
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ export const renderApp = (
>
<LicenseProvider license$={plugins.licensing.license$}>
<Provider store={store}>
<HttpProvider http={core.http} errorConnecting={errorConnecting} />
<HttpProvider
http={core.http}
errorConnecting={errorConnecting}
readOnlyMode={initialData.readOnlyMode}
/>
<FlashMessagesProvider history={params.history} />
<Router history={params.history}>
<App {...initialData} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe('HttpLogic', () => {
http: null,
httpInterceptors: [],
errorConnecting: false,
readOnlyMode: false,
};

beforeEach(() => {
Expand All @@ -31,12 +32,17 @@ describe('HttpLogic', () => {
describe('initializeHttp()', () => {
it('sets values based on passed props', () => {
HttpLogic.mount();
HttpLogic.actions.initializeHttp({ http: mockHttp, errorConnecting: true });
HttpLogic.actions.initializeHttp({
http: mockHttp,
errorConnecting: true,
readOnlyMode: true,
});

expect(HttpLogic.values).toEqual({
http: mockHttp,
httpInterceptors: [],
errorConnecting: true,
readOnlyMode: true,
});
});
});
Expand All @@ -52,50 +58,110 @@ describe('HttpLogic', () => {
});
});

describe('setReadOnlyMode()', () => {
it('sets readOnlyMode value', () => {
HttpLogic.mount();
HttpLogic.actions.setReadOnlyMode(true);
expect(HttpLogic.values.readOnlyMode).toEqual(true);

HttpLogic.actions.setReadOnlyMode(false);
expect(HttpLogic.values.readOnlyMode).toEqual(false);
});
});

describe('http interceptors', () => {
describe('initializeHttpInterceptors()', () => {
beforeEach(() => {
HttpLogic.mount();
jest.spyOn(HttpLogic.actions, 'setHttpInterceptors');
jest.spyOn(HttpLogic.actions, 'setErrorConnecting');
HttpLogic.actions.initializeHttp({ http: mockHttp });

HttpLogic.actions.initializeHttpInterceptors();
});

it('calls http.intercept and sets an array of interceptors', () => {
mockHttp.intercept.mockImplementationOnce(() => 'removeInterceptorFn' as any);
mockHttp.intercept
.mockImplementationOnce(() => 'removeErrorInterceptorFn' as any)
.mockImplementationOnce(() => 'removeReadOnlyInterceptorFn' as any);
HttpLogic.actions.initializeHttpInterceptors();

expect(mockHttp.intercept).toHaveBeenCalled();
expect(HttpLogic.actions.setHttpInterceptors).toHaveBeenCalledWith(['removeInterceptorFn']);
expect(HttpLogic.actions.setHttpInterceptors).toHaveBeenCalledWith([
'removeErrorInterceptorFn',
'removeReadOnlyInterceptorFn',
]);
});

describe('errorConnectingInterceptor', () => {
let interceptedResponse: any;

beforeEach(() => {
interceptedResponse = mockHttp.intercept.mock.calls[0][0].responseError;
jest.spyOn(HttpLogic.actions, 'setErrorConnecting');
});

it('handles errors connecting to Enterprise Search', async () => {
const { responseError } = mockHttp.intercept.mock.calls[0][0] as any;
const httpResponse = { response: { url: '/api/app_search/engines', status: 502 } };
await expect(responseError(httpResponse)).rejects.toEqual(httpResponse);
const httpResponse = {
response: { url: '/api/app_search/engines', status: 502 },
};
await expect(interceptedResponse(httpResponse)).rejects.toEqual(httpResponse);

expect(HttpLogic.actions.setErrorConnecting).toHaveBeenCalled();
});

it('does not handle non-502 Enterprise Search errors', async () => {
const { responseError } = mockHttp.intercept.mock.calls[0][0] as any;
const httpResponse = { response: { url: '/api/workplace_search/overview', status: 404 } };
await expect(responseError(httpResponse)).rejects.toEqual(httpResponse);
const httpResponse = {
response: { url: '/api/workplace_search/overview', status: 404 },
};
await expect(interceptedResponse(httpResponse)).rejects.toEqual(httpResponse);

expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
});

it('does not handle errors for unrelated calls', async () => {
const { responseError } = mockHttp.intercept.mock.calls[0][0] as any;
const httpResponse = { response: { url: '/api/some_other_plugin/', status: 502 } };
await expect(responseError(httpResponse)).rejects.toEqual(httpResponse);
it('does not handle errors for non-Enterprise Search API calls', async () => {
const httpResponse = {
response: { url: '/api/some_other_plugin/', status: 502 },
};
await expect(interceptedResponse(httpResponse)).rejects.toEqual(httpResponse);

expect(HttpLogic.actions.setErrorConnecting).not.toHaveBeenCalled();
});
});

describe('readOnlyModeInterceptor', () => {
let interceptedResponse: any;

beforeEach(() => {
interceptedResponse = mockHttp.intercept.mock.calls[1][0].response;
jest.spyOn(HttpLogic.actions, 'setReadOnlyMode');
});

it('sets readOnlyMode to true if the response header is true', async () => {
const httpResponse = {
response: { url: '/api/app_search/engines', headers: { get: () => 'true' } },
};
await expect(interceptedResponse(httpResponse)).resolves.toEqual(httpResponse);

expect(HttpLogic.actions.setReadOnlyMode).toHaveBeenCalledWith(true);
});

it('sets readOnlyMode to false if the response header is false', async () => {
const httpResponse = {
response: { url: '/api/workplace_search/overview', headers: { get: () => 'false' } },
};
await expect(interceptedResponse(httpResponse)).resolves.toEqual(httpResponse);

expect(HttpLogic.actions.setReadOnlyMode).toHaveBeenCalledWith(false);
});

it('does not handle headers for non-Enterprise Search API calls', async () => {
const httpResponse = {
response: { url: '/api/some_other_plugin/', headers: { get: () => 'true' } },
};
await expect(interceptedResponse(httpResponse)).resolves.toEqual(httpResponse);

expect(HttpLogic.actions.setReadOnlyMode).not.toHaveBeenCalled();
});
});
});

it('sets httpInterceptors and calls all valid remove functions on unmount', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,32 @@

import { kea, MakeLogicType } from 'kea';

import { HttpSetup, HttpInterceptorResponseError } from 'src/core/public';
import { HttpSetup, HttpInterceptorResponseError, HttpResponse } from 'src/core/public';
import { IHttpProviderProps } from './http_provider';

import { READ_ONLY_MODE_HEADER } from '../../../../common/constants';

export interface IHttpValues {
http: HttpSetup;
httpInterceptors: Function[];
errorConnecting: boolean;
readOnlyMode: boolean;
}
export interface IHttpActions {
initializeHttp({
http,
errorConnecting,
}: {
http: HttpSetup;
errorConnecting?: boolean;
}): { http: HttpSetup; errorConnecting?: boolean };
initializeHttp({ http, errorConnecting, readOnlyMode }: IHttpProviderProps): IHttpProviderProps;
initializeHttpInterceptors(): void;
setHttpInterceptors(httpInterceptors: Function[]): { httpInterceptors: Function[] };
setErrorConnecting(errorConnecting: boolean): { errorConnecting: boolean };
setReadOnlyMode(readOnlyMode: boolean): { readOnlyMode: boolean };
}

export const HttpLogic = kea<MakeLogicType<IHttpValues, IHttpActions>>({
actions: {
initializeHttp: ({ http, errorConnecting }) => ({ http, errorConnecting }),
initializeHttp: (props) => props,
initializeHttpInterceptors: () => null,
setHttpInterceptors: (httpInterceptors) => ({ httpInterceptors }),
setErrorConnecting: (errorConnecting) => ({ errorConnecting }),
setReadOnlyMode: (readOnlyMode) => ({ readOnlyMode }),
},
reducers: {
http: [
Expand All @@ -53,20 +53,27 @@ export const HttpLogic = kea<MakeLogicType<IHttpValues, IHttpActions>>({
setErrorConnecting: (_, { errorConnecting }) => errorConnecting,
},
],
readOnlyMode: [
false,
{
initializeHttp: (_, { readOnlyMode }) => !!readOnlyMode,
setReadOnlyMode: (_, { readOnlyMode }) => readOnlyMode,
},
],
},
listeners: ({ values, actions }) => ({
initializeHttpInterceptors: () => {
const httpInterceptors = [];

const errorConnectingInterceptor = values.http.intercept({
responseError: async (httpResponse) => {
const { url, status } = httpResponse.response!;
const hasErrorConnecting = status === 502;
const isApiResponse =
url.includes('/api/app_search/') || url.includes('/api/workplace_search/');
if (isEnterpriseSearchApi(httpResponse)) {
const { status } = httpResponse.response!;
const hasErrorConnecting = status === 502;

if (isApiResponse && hasErrorConnecting) {
actions.setErrorConnecting(true);
if (hasErrorConnecting) {
actions.setErrorConnecting(true);
}
}

// Re-throw error so that downstream catches work as expected
Expand All @@ -75,7 +82,23 @@ export const HttpLogic = kea<MakeLogicType<IHttpValues, IHttpActions>>({
});
httpInterceptors.push(errorConnectingInterceptor);

// TODO: Read only mode interceptor
const readOnlyModeInterceptor = values.http.intercept({
response: async (httpResponse) => {
if (isEnterpriseSearchApi(httpResponse)) {
const readOnlyMode = httpResponse.response!.headers.get(READ_ONLY_MODE_HEADER);

if (readOnlyMode === 'true') {
actions.setReadOnlyMode(true);
} else {
actions.setReadOnlyMode(false);
}
}

return Promise.resolve(httpResponse);
},
});
httpInterceptors.push(readOnlyModeInterceptor);

actions.setHttpInterceptors(httpInterceptors);
},
}),
Expand All @@ -87,3 +110,11 @@ export const HttpLogic = kea<MakeLogicType<IHttpValues, IHttpActions>>({
},
}),
});

/**
* Small helper that checks whether or not an http call is for an Enterprise Search API
*/
const isEnterpriseSearchApi = (httpResponse: HttpResponse) => {
const { url } = httpResponse.response!;
return url.includes('/api/app_search/') || url.includes('/api/workplace_search/');
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('HttpProvider', () => {
const props = {
http: {} as any,
errorConnecting: false,
readOnlyMode: false,
};
const initializeHttp = jest.fn();
const initializeHttpInterceptors = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ import { HttpSetup } from 'src/core/public';

import { HttpLogic } from './http_logic';

interface IHttpProviderProps {
export interface IHttpProviderProps {
http: HttpSetup;
errorConnecting?: boolean;
readOnlyMode?: boolean;
}

export const HttpProvider: React.FC<IHttpProviderProps> = (props) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,15 @@
padding: $euiSize;
}
}

&__readOnlyMode {
margin: -$euiSizeM 0 $euiSizeL;

@include euiBreakpoint('m') {
margin: 0 0 $euiSizeL;
}
@include euiBreakpoint('xs', 's') {
margin: 0;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import React from 'react';
import { shallow } from 'enzyme';
import { EuiPageSideBar, EuiButton, EuiPageBody } from '@elastic/eui';
import { EuiPageSideBar, EuiButton, EuiPageBody, EuiCallOut } from '@elastic/eui';

import { Layout, INavContext } from './layout';

Expand Down Expand Up @@ -55,6 +55,12 @@ describe('Layout', () => {
expect(wrapper.find(EuiPageSideBar).prop('className')).not.toContain('--isOpen');
});

it('renders a read-only mode callout', () => {
const wrapper = shallow(<Layout navigation={null} readOnlyMode={true} />);

expect(wrapper.find(EuiCallOut)).toHaveLength(1);
});

it('renders children', () => {
const wrapper = shallow(
<Layout navigation={null}>
Expand Down
Loading

0 comments on commit 1145806

Please sign in to comment.