Skip to content

Commit

Permalink
migrate version_check to new client
Browse files Browse the repository at this point in the history
  • Loading branch information
pgayvallet committed Jun 30, 2020
1 parent 4156411 commit 5a3fd9c
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 74 deletions.
24 changes: 24 additions & 0 deletions src/core/server/elasticsearch/client/client_facade.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,30 @@
* under the License.
*/

import { ApiResponse } from '@elastic/elasticsearch';
import { TransportRequestPromise } from '@elastic/elasticsearch/lib/Transport';
import { ClientFacade } from './client_facade';

const createApiResponse = <T>(body: T): TransportRequestPromise<ApiResponse<T>> => {
const response: ApiResponse<T> = {
body,
statusCode: 200,
warnings: [],
headers: {},
meta: {} as any,
};
const promise = Promise.resolve(response);
(promise as TransportRequestPromise<ApiResponse<T>>).abort = () => undefined;

return promise as TransportRequestPromise<ApiResponse<T>>;
};

const createApiError = (err: any): TransportRequestPromise<never> => {
const promise = Promise.reject(err);
(promise as TransportRequestPromise<never>).abort = () => undefined;
return promise as TransportRequestPromise<never>;
};

const createFacadeMock = () => {
const mock: DeeplyMockedKeys<ClientFacade> = {
transport: {
Expand Down Expand Up @@ -396,4 +418,6 @@ const createFacadeMock = () => {

export const clientFacadeMock = {
create: createFacadeMock,
createApiResponse,
createApiError,
};
2 changes: 2 additions & 0 deletions src/core/server/elasticsearch/client/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,6 @@ export const elasticsearchClientMock = {
createCustomClusterClient: createCustomClusterClientMock,
createScopedClusterClient: createScopedClusterClientMock,
createFacade: clientFacadeMock.create,
createClientResponse: clientFacadeMock.createApiResponse,
createClientError: clientFacadeMock.createApiError,
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@
* under the License.
*/

export const MockLegacyClusterClient = jest.fn();
jest.mock('./legacy/cluster_client', () => ({ LegacyClusterClient: MockLegacyClusterClient }));

export const MockClusterClient = jest.fn();
jest.mock('./legacy/cluster_client', () => ({ LegacyClusterClient: MockClusterClient }));
jest.mock('./client/cluster_client', () => ({ ClusterClient: MockClusterClient }));
88 changes: 48 additions & 40 deletions src/core/server/elasticsearch/elasticsearch_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import { first } from 'rxjs/operators';

import { MockClusterClient } from './elasticsearch_service.test.mocks';
import { MockLegacyClusterClient, MockClusterClient } from './elasticsearch_service.test.mocks';

import { BehaviorSubject } from 'rxjs';
import { Env } from '../config';
Expand All @@ -31,6 +31,7 @@ import { httpServiceMock } from '../http/http_service.mock';
import { ElasticsearchConfig } from './elasticsearch_config';
import { ElasticsearchService } from './elasticsearch_service';
import { elasticsearchServiceMock } from './elasticsearch_service.mock';
import { elasticsearchClientMock } from './client/mocks';
import { duration } from 'moment';

const delay = async (durationMs: number) =>
Expand All @@ -56,11 +57,23 @@ configService.atPath.mockReturnValue(
let env: Env;
let coreContext: CoreContext;
const logger = loggingSystemMock.create();

let mockClusterClientInstance: ReturnType<typeof elasticsearchClientMock.createCustomClusterClient>;
let mockLegacyClusterClientInstance: ReturnType<typeof elasticsearchServiceMock.createCustomClusterClient>;

beforeEach(() => {
env = Env.createDefault(getEnvOptions());

coreContext = { coreId: Symbol(), env, logger, configService: configService as any };
elasticsearchService = new ElasticsearchService(coreContext);

MockLegacyClusterClient.mockClear();
MockClusterClient.mockClear();

mockLegacyClusterClientInstance = elasticsearchServiceMock.createCustomClusterClient();
MockLegacyClusterClient.mockImplementation(() => mockLegacyClusterClientInstance);
mockClusterClientInstance = elasticsearchClientMock.createCustomClusterClient();
MockClusterClient.mockImplementation(() => mockClusterClientInstance);
});

afterEach(() => jest.clearAllMocks());
Expand All @@ -74,31 +87,28 @@ describe('#setup', () => {
);
});

it('returns elasticsearch client as a part of the contract', async () => {
const mockClusterClientInstance = elasticsearchServiceMock.createClusterClient();
MockClusterClient.mockImplementationOnce(() => mockClusterClientInstance);

it('returns legacy elasticsearch client as a part of the contract', async () => {
const setupContract = await elasticsearchService.setup(deps);
const client = setupContract.legacy.client;

expect(mockClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(0);
expect(mockLegacyClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(0);
await client.callAsInternalUser('any');
expect(mockClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(mockLegacyClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(1);
});

describe('#createClient', () => {
describe('#createLegacyClient', () => {
it('allows to specify config properties', async () => {
const setupContract = await elasticsearchService.setup(deps);

const mockClusterClientInstance = { close: jest.fn() };
MockClusterClient.mockImplementation(() => mockClusterClientInstance);
// reset all mocks called during setup phase
MockLegacyClusterClient.mockClear();

const customConfig = { logQueries: true };
const clusterClient = setupContract.legacy.createClient('some-custom-type', customConfig);

expect(clusterClient).toBe(mockClusterClientInstance);
expect(clusterClient).toBe(mockLegacyClusterClientInstance);

expect(MockClusterClient).toHaveBeenCalledWith(
expect(MockLegacyClusterClient).toHaveBeenCalledWith(
expect.objectContaining(customConfig),
expect.objectContaining({ context: ['elasticsearch', 'some-custom-type'] }),
expect.any(Function)
Expand All @@ -107,8 +117,9 @@ describe('#setup', () => {

it('falls back to elasticsearch default config values if property not specified', async () => {
const setupContract = await elasticsearchService.setup(deps);

// reset all mocks called during setup phase
MockClusterClient.mockClear();
MockLegacyClusterClient.mockClear();

const customConfig = {
hosts: ['http://8.8.8.8'],
Expand All @@ -117,7 +128,7 @@ describe('#setup', () => {
};
setupContract.legacy.createClient('some-custom-type', customConfig);

const config = MockClusterClient.mock.calls[0][0];
const config = MockLegacyClusterClient.mock.calls[0][0];
expect(config).toMatchInlineSnapshot(`
Object {
"healthCheckDelay": "PT0.01S",
Expand All @@ -137,12 +148,13 @@ describe('#setup', () => {
});
it('falls back to elasticsearch config if custom config not passed', async () => {
const setupContract = await elasticsearchService.setup(deps);

// reset all mocks called during setup phase
MockClusterClient.mockClear();
MockLegacyClusterClient.mockClear();

setupContract.legacy.createClient('another-type');

const config = MockClusterClient.mock.calls[0][0];
const config = MockLegacyClusterClient.mock.calls[0][0];
expect(config).toMatchInlineSnapshot(`
Object {
"healthCheckDelay": "PT0.01S",
Expand Down Expand Up @@ -178,8 +190,9 @@ describe('#setup', () => {
);
elasticsearchService = new ElasticsearchService(coreContext);
const setupContract = await elasticsearchService.setup(deps);

// reset all mocks called during setup phase
MockClusterClient.mockClear();
MockLegacyClusterClient.mockClear();

const customConfig = {
hosts: ['http://8.8.8.8'],
Expand All @@ -188,7 +201,7 @@ describe('#setup', () => {
};
setupContract.legacy.createClient('some-custom-type', customConfig);

const config = MockClusterClient.mock.calls[0][0];
const config = MockLegacyClusterClient.mock.calls[0][0];
expect(config).toMatchInlineSnapshot(`
Object {
"healthCheckDelay": "PT2S",
Expand All @@ -209,66 +222,61 @@ describe('#setup', () => {
});

it('esNodeVersionCompatibility$ only starts polling when subscribed to', async (done) => {
const clusterClientInstance = elasticsearchServiceMock.createClusterClient();
MockClusterClient.mockImplementationOnce(() => clusterClientInstance);

clusterClientInstance.callAsInternalUser.mockRejectedValue(new Error());
const mockedClient = elasticsearchClientMock.createFacade();
mockedClient.nodes.info.mockRejectedValue(new Error());
mockClusterClientInstance.asInternalUser.mockReturnValue(mockedClient);

const setupContract = await elasticsearchService.setup(deps);
await delay(10);

expect(clusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(0);
expect(mockedClient.nodes.info).toHaveBeenCalledTimes(0);
setupContract.esNodesCompatibility$.subscribe(() => {
expect(clusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
done();
});
});

it('esNodeVersionCompatibility$ stops polling when unsubscribed from', async (done) => {
const mockClusterClientInstance = elasticsearchServiceMock.createClusterClient();
MockClusterClient.mockImplementationOnce(() => mockClusterClientInstance);

mockClusterClientInstance.callAsInternalUser.mockRejectedValue(new Error());
const mockedClient = elasticsearchClientMock.createFacade();
mockedClient.nodes.info.mockRejectedValue(new Error());
mockClusterClientInstance.asInternalUser.mockReturnValue(mockedClient);

const setupContract = await elasticsearchService.setup(deps);

expect(mockClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(0);
expect(mockedClient.nodes.info).toHaveBeenCalledTimes(0);
const sub = setupContract.esNodesCompatibility$.subscribe(async () => {
sub.unsubscribe();
await delay(100);
expect(mockClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
done();
});
});
});

describe('#stop', () => {
it('stops both admin and data clients', async () => {
const mockClusterClientInstance = { close: jest.fn() };
MockClusterClient.mockImplementationOnce(() => mockClusterClientInstance);

it('stops both legacy and new clients', async () => {
await elasticsearchService.setup(deps);
await elasticsearchService.stop();

expect(mockLegacyClusterClientInstance.close).toHaveBeenCalledTimes(1);
expect(mockClusterClientInstance.close).toHaveBeenCalledTimes(1);
});

it('stops pollEsNodeVersions even if there are active subscriptions', async (done) => {
expect.assertions(2);
const mockClusterClientInstance = elasticsearchServiceMock.createCustomClusterClient();

MockClusterClient.mockImplementationOnce(() => mockClusterClientInstance);

mockClusterClientInstance.callAsInternalUser.mockRejectedValue(new Error());
const mockedClient = elasticsearchClientMock.createFacade();
mockedClient.nodes.info.mockRejectedValue(new Error());
mockClusterClientInstance.asInternalUser.mockReturnValue(mockedClient);

const setupContract = await elasticsearchService.setup(deps);

setupContract.esNodesCompatibility$.subscribe(async () => {
expect(mockClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);

await elasticsearchService.stop();
await delay(100);
expect(mockClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(mockedClient.nodes.info).toHaveBeenCalledTimes(1);
done();
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/elasticsearch/elasticsearch_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export class ElasticsearchService
this.legacyClient = this.createLegacyClusterClient('data', config, deps.http.getAuthHeaders);

const esNodesCompatibility$ = pollEsNodesVersion({
callWithInternalUser: this.legacyClient.callAsInternalUser,
internalClient: this.client.asInternalUser(),
log: this.log,
ignoreVersionMismatch: config.ignoreVersionMismatch,
esVersionCheckInterval: config.healthCheckDelay.asMilliseconds(),
Expand Down
Loading

0 comments on commit 5a3fd9c

Please sign in to comment.