Skip to content

Commit 807fcc2

Browse files
[NP] Remove observables from es internal contract (#54556) (#54581)
* request context always uses the latest es client * update integration tests Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 105c452 commit 807fcc2

File tree

7 files changed

+22
-60
lines changed

7 files changed

+22
-60
lines changed

src/core/server/elasticsearch/elasticsearch_service.mock.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ const createInternalSetupContractMock = () => {
7474
legacy: {
7575
config$: new BehaviorSubject({} as ElasticsearchConfig),
7676
},
77-
adminClient$: new BehaviorSubject(createClusterClientMock()),
78-
dataClient$: new BehaviorSubject(createClusterClientMock()),
7977
};
8078
setupContract.adminClient.asScoped.mockReturnValue(createScopedClusterClientMock());
8179
setupContract.dataClient.asScoped.mockReturnValue(createScopedClusterClientMock());

src/core/server/elasticsearch/elasticsearch_service.test.ts

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { first } from 'rxjs/operators';
2121

2222
import { MockClusterClient } from './elasticsearch_service.test.mocks';
2323

24-
import { BehaviorSubject, combineLatest } from 'rxjs';
24+
import { BehaviorSubject } from 'rxjs';
2525
import { Env } from '../config';
2626
import { getEnvOptions } from '../config/__mocks__/env';
2727
import { CoreContext } from '../core_context';
@@ -91,44 +91,6 @@ describe('#setup', () => {
9191
expect(mockDataClusterClientInstance.callAsInternalUser).toHaveBeenCalledTimes(1);
9292
});
9393

94-
it('returns data and admin client observables as a part of the contract', async () => {
95-
const mockAdminClusterClientInstance = { close: jest.fn() };
96-
const mockDataClusterClientInstance = { close: jest.fn() };
97-
MockClusterClient.mockImplementationOnce(
98-
() => mockAdminClusterClientInstance
99-
).mockImplementationOnce(() => mockDataClusterClientInstance);
100-
101-
const setupContract = await elasticsearchService.setup(deps);
102-
103-
const [esConfig, adminClient, dataClient] = await combineLatest(
104-
setupContract.legacy.config$,
105-
setupContract.adminClient$,
106-
setupContract.dataClient$
107-
)
108-
.pipe(first())
109-
.toPromise();
110-
111-
expect(adminClient).toBe(mockAdminClusterClientInstance);
112-
expect(dataClient).toBe(mockDataClusterClientInstance);
113-
114-
expect(MockClusterClient).toHaveBeenCalledTimes(2);
115-
expect(MockClusterClient).toHaveBeenNthCalledWith(
116-
1,
117-
esConfig,
118-
expect.objectContaining({ context: ['elasticsearch', 'admin'] }),
119-
undefined
120-
);
121-
expect(MockClusterClient).toHaveBeenNthCalledWith(
122-
2,
123-
esConfig,
124-
expect.objectContaining({ context: ['elasticsearch', 'data'] }),
125-
expect.any(Function)
126-
);
127-
128-
expect(mockAdminClusterClientInstance.close).not.toHaveBeenCalled();
129-
expect(mockDataClusterClientInstance.close).not.toHaveBeenCalled();
130-
});
131-
13294
describe('#createClient', () => {
13395
it('allows to specify config properties', async () => {
13496
const setupContract = await elasticsearchService.setup(deps);

src/core/server/elasticsearch/elasticsearch_service.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,6 @@ export class ElasticsearchService implements CoreService<InternalElasticsearchSe
152152
return {
153153
legacy: { config$: clients$.pipe(map(clients => clients.config)) },
154154

155-
adminClient$,
156-
dataClient$,
157155
adminClient,
158156
dataClient,
159157

src/core/server/elasticsearch/types.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,4 @@ export interface InternalElasticsearchServiceSetup extends ElasticsearchServiceS
7777
readonly legacy: {
7878
readonly config$: Observable<ElasticsearchConfig>;
7979
};
80-
81-
readonly adminClient$: Observable<IClusterClient>;
82-
readonly dataClient$: Observable<IClusterClient>;
8380
}

src/core/server/http/integration_tests/core_service.test.mocks.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19+
import { elasticsearchServiceMock } from '../../elasticsearch/elasticsearch_service.mock';
1920

2021
export const clusterClientMock = jest.fn();
2122
jest.doMock('../../elasticsearch/scoped_cluster_client', () => ({
22-
ScopedClusterClient: clusterClientMock,
23+
ScopedClusterClient: clusterClientMock.mockImplementation(function() {
24+
return elasticsearchServiceMock.createScopedClusterClient();
25+
}),
2326
}));

src/core/server/http/integration_tests/core_services.test.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ describe('http service', () => {
133133
const { http } = await root.setup();
134134
const { registerAuth } = http;
135135

136-
await registerAuth((req, res, toolkit) => {
136+
registerAuth((req, res, toolkit) => {
137137
return toolkit.authenticated({ responseHeaders: authResponseHeader });
138138
});
139139

@@ -157,7 +157,7 @@ describe('http service', () => {
157157
const { http } = await root.setup();
158158
const { registerAuth } = http;
159159

160-
await registerAuth((req, res, toolkit) => {
160+
registerAuth((req, res, toolkit) => {
161161
return toolkit.authenticated({ responseHeaders: authResponseHeader });
162162
});
163163

@@ -222,12 +222,15 @@ describe('http service', () => {
222222
const { http } = await root.setup();
223223
const { registerAuth, createRouter } = http;
224224

225-
await registerAuth((req, res, toolkit) =>
226-
toolkit.authenticated({ requestHeaders: authHeaders })
227-
);
225+
registerAuth((req, res, toolkit) => toolkit.authenticated({ requestHeaders: authHeaders }));
228226

229227
const router = createRouter('/new-platform');
230-
router.get({ path: '/', validate: false }, (context, req, res) => res.ok());
228+
router.get({ path: '/', validate: false }, async (context, req, res) => {
229+
// it forces client initialization since the core creates them lazily.
230+
await context.core.elasticsearch.adminClient.callAsCurrentUser('ping');
231+
await context.core.elasticsearch.dataClient.callAsCurrentUser('ping');
232+
return res.ok();
233+
});
231234

232235
await root.start();
233236

@@ -247,7 +250,12 @@ describe('http service', () => {
247250
const { createRouter } = http;
248251

249252
const router = createRouter('/new-platform');
250-
router.get({ path: '/', validate: false }, (context, req, res) => res.ok());
253+
router.get({ path: '/', validate: false }, async (context, req, res) => {
254+
// it forces client initialization since the core creates them lazily.
255+
await context.core.elasticsearch.adminClient.callAsCurrentUser('ping');
256+
await context.core.elasticsearch.dataClient.callAsCurrentUser('ping');
257+
return res.ok();
258+
});
251259

252260
await root.start();
253261

src/core/server/server.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
* under the License.
1818
*/
1919

20-
import { take } from 'rxjs/operators';
2120
import { Type } from '@kbn/config-schema';
2221

2322
import {
@@ -216,9 +215,6 @@ export class Server {
216215
coreId,
217216
'core',
218217
async (context, req, res): Promise<RequestHandlerContext['core']> => {
219-
// it consumes elasticsearch observables to provide the same client throughout the context lifetime.
220-
const adminClient = await coreSetup.elasticsearch.adminClient$.pipe(take(1)).toPromise();
221-
const dataClient = await coreSetup.elasticsearch.dataClient$.pipe(take(1)).toPromise();
222218
const savedObjectsClient = coreSetup.savedObjects.getScopedClient(req);
223219
const uiSettingsClient = coreSetup.uiSettings.asScopedToClient(savedObjectsClient);
224220

@@ -230,8 +226,8 @@ export class Server {
230226
client: savedObjectsClient,
231227
},
232228
elasticsearch: {
233-
adminClient: adminClient.asScoped(req),
234-
dataClient: dataClient.asScoped(req),
229+
adminClient: coreSetup.elasticsearch.adminClient.asScoped(req),
230+
dataClient: coreSetup.elasticsearch.dataClient.asScoped(req),
235231
},
236232
uiSettings: {
237233
client: uiSettingsClient,

0 commit comments

Comments
 (0)