[data.search] Use versioned router (#158520)

## Summary Step 1 of #157095. Uses the new versioned router capabilities for the search routes (`POST` and `DELETE`). ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Matthias Wilhelm <[email protected]>
elastic · Jun 7, 2023 · 34ada8a · 34ada8a
1 parent b8b4f75
commit 34ada8a
Show file tree

Hide file tree

Showing 14 changed files with 235 additions and 78 deletions.
diff --git a/src/plugins/data/public/search/search_interceptor/search_interceptor.ts b/src/plugins/data/public/search/search_interceptor/search_interceptor.ts
@@ -298,10 +298,12 @@ export class SearchInterceptor {
           isSavedToBackground = true;
         });
 
-    const cancel = once(() => {
-      if (id && !isSavedToBackground) this.deps.http.delete(`/internal/search/${strategy}/${id}`);
+    const sendCancelRequest = once(() => {
+      this.deps.http.delete(`/internal/search/${strategy}/${id}`, { version: '1' });
     });
 
+    const cancel = () => id && !isSavedToBackground && sendCancelRequest();
+
     return pollSearch(search, cancel, {
       pollInterval: this.deps.searchConfig.asyncSearch.pollInterval,
       ...options,
@@ -346,6 +348,7 @@ export class SearchInterceptor {
       const { executionContext, strategy, ...searchOptions } = this.getSerializableOptions(options);
       return this.deps.http
         .post(`/internal/search/${strategy}${request.id ? `/${request.id}` : ''}`, {
+          version: '1',
           signal: abortSignal,
           context: executionContext,
           body: JSON.stringify({

diff --git a/src/plugins/data/server/search/routes/search.test.ts b/src/plugins/data/server/search/routes/search.test.ts
@@ -27,7 +27,15 @@ describe('Search service', () => {
     registerSearchRoute(mockCoreSetup.http.createRouter());
 
     const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
-    const handler = mockRouter.post.mock.calls[0][1];
+    const handler = mockRouter.versioned.post.mock.results[0].value.addVersion.mock.calls[0][1];
+    await handler(mockContext as unknown as RequestHandlerContext, mockRequest, mockResponse);
+  }
+
+  async function runMockDelete(mockContext: any, mockRequest: any, mockResponse: any) {
+    registerSearchRoute(mockCoreSetup.http.createRouter());
+
+    const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
+    const handler = mockRouter.versioned.delete.mock.results[0].value.addVersion.mock.calls[0][1];
     await handler(mockContext as unknown as RequestHandlerContext, mockRequest, mockResponse);
   }
 
@@ -156,4 +164,26 @@ describe('Search service', () => {
     expect(error.body.message).toBe('This is odd');
     expect(error.body.attributes).toBe(undefined);
   });
+
+  it('DELETE request calls cancel with the given ID and strategy', async () => {
+    const mockContext = {
+      search: {
+        cancel: jest.fn(),
+      },
+    };
+
+    const id = '1234';
+    const strategy = 'foo';
+    const params = { id, strategy };
+
+    const mockRequest = httpServerMock.createKibanaRequest({
+      params,
+    });
+    const mockResponse = httpServerMock.createResponseFactory();
+
+    await runMockDelete(mockContext, mockRequest, mockResponse);
+
+    expect(mockContext.search.cancel).toBeCalled();
+    expect(mockContext.search.cancel).toBeCalledWith(id, { strategy });
+  });
 });
diff --git a/src/plugins/data/server/search/routes/search.ts b/src/plugins/data/server/search/routes/search.ts
@@ -12,82 +12,97 @@ import { reportServerError } from '@kbn/kibana-utils-plugin/server';
 import { getRequestAbortedSignal } from '../../lib';
 import type { DataPluginRouter } from '../types';
 
-export function registerSearchRoute(router: DataPluginRouter): void {
-  router.post(
-    {
-      path: '/internal/search/{strategy}/{id?}',
-      validate: {
-        params: schema.object({
-          strategy: schema.string(),
-          id: schema.maybe(schema.string()),
-        }),
+export const SEARCH_API_BASE_URL = '/internal/search';
 
-        body: schema.object(
-          {
-            legacyHitsTotal: schema.maybe(schema.boolean()),
-            sessionId: schema.maybe(schema.string()),
-            isStored: schema.maybe(schema.boolean()),
-            isRestore: schema.maybe(schema.boolean()),
+export function registerSearchRoute(router: DataPluginRouter): void {
+  router.versioned
+    .post({
+      path: `${SEARCH_API_BASE_URL}/{strategy}/{id?}`,
+      access: 'internal',
+    })
+    .addVersion(
+      {
+        version: '1',
+        validate: {
+          request: {
+            params: schema.object({
+              strategy: schema.string(),
+              id: schema.maybe(schema.string()),
+            }),
+            body: schema.object(
+              {
+                legacyHitsTotal: schema.maybe(schema.boolean()),
+                sessionId: schema.maybe(schema.string()),
+                isStored: schema.maybe(schema.boolean()),
+                isRestore: schema.maybe(schema.boolean()),
+              },
+              { unknowns: 'allow' }
+            ),
           },
-          { unknowns: 'allow' }
-        ),
+        },
       },
-    },
-    async (context, request, res) => {
-      const {
-        legacyHitsTotal = true,
-        sessionId,
-        isStored,
-        isRestore,
-        ...searchRequest
-      } = request.body;
-      const { strategy, id } = request.params;
-      const abortSignal = getRequestAbortedSignal(request.events.aborted$);
+      async (context, request, res) => {
+        const {
+          legacyHitsTotal = true,
+          sessionId,
+          isStored,
+          isRestore,
+          ...searchRequest
+        } = request.body;
+        const { strategy, id } = request.params;
+        const abortSignal = getRequestAbortedSignal(request.events.aborted$);
 
-      try {
-        const search = await context.search;
-        const response = await search
-          .search(
-            { ...searchRequest, id },
-            {
-              abortSignal,
-              strategy,
-              legacyHitsTotal,
-              sessionId,
-              isStored,
-              isRestore,
-            }
-          )
-          .pipe(first())
-          .toPromise();
+        try {
+          const search = await context.search;
+          const response = await search
+            .search(
+              { ...searchRequest, id },
+              {
+                abortSignal,
+                strategy,
+                legacyHitsTotal,
+                sessionId,
+                isStored,
+                isRestore,
+              }
+            )
+            .pipe(first())
+            .toPromise();
 
-        return res.ok({ body: response });
-      } catch (err) {
-        return reportServerError(res, err);
+          return res.ok({ body: response });
+        } catch (err) {
+          return reportServerError(res, err);
+        }
       }
-    }
-  );
+    );
 
-  router.delete(
-    {
+  router.versioned
+    .delete({
       path: '/internal/search/{strategy}/{id}',
-      validate: {
-        params: schema.object({
-          strategy: schema.string(),
-          id: schema.string(),
-        }),
+      access: 'internal',
+    })
+    .addVersion(
+      {
+        version: '1',
+        validate: {
+          request: {
+            params: schema.object({
+              strategy: schema.string(),
+              id: schema.string(),
+            }),
+          },
+        },
       },
-    },
-    async (context, request, res) => {
-      const { strategy, id } = request.params;
+      async (context, request, res) => {
+        const { strategy, id } = request.params;
 
-      try {
-        const search = await context.search;
-        await search.cancel(id, { strategy });
-        return res.ok();
-      } catch (err) {
-        return reportServerError(res, err);
+        try {
+          const search = await context.search;
+          await search.cancel(id, { strategy });
+          return res.ok();
+        } catch (err) {
+          return reportServerError(res, err);
+        }
       }
-    }
-  );
+    );
 }
diff --git a/src/plugins/data_views/public/services/has_data.ts b/src/plugins/data_views/public/services/has_data.ts
@@ -82,6 +82,7 @@ export class HasData {
   }): Promise<boolean> =>
     http
       .post<IndicesViaSearchResponse>(`/internal/search/ese`, {
+        version: '1',
         body: JSON.stringify({
           params: {
             ignore_unavailable: true,

diff --git a/test/api_integration/apis/search/search.ts b/test/api_integration/apis/search/search.ts
@@ -6,6 +6,7 @@
  * Side Public License, v 1.
  */
 
+import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';
 import expect from '@kbn/expect';
 import { FtrProviderContext } from '../../ftr_provider_context';
 import { painlessErrReq } from './painless_err_req';
@@ -28,6 +29,7 @@ export default function ({ getService }: FtrProviderContext) {
       it('should return 200 when correctly formatted searches are provided', async () => {
         const resp = await supertest
           .post(`/internal/search/es`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
           .send({
             params: {
               body: {
@@ -43,11 +45,13 @@ export default function ({ getService }: FtrProviderContext) {
         expect(resp.body.isPartial).to.be(false);
         expect(resp.body.isRunning).to.be(false);
         expect(resp.body).to.have.property('rawResponse');
+        expect(resp.header).to.have.property(ELASTIC_HTTP_VERSION_HEADER, '1');
       });
 
       it('should return 200 if terminated early', async () => {
         const resp = await supertest
           .post(`/internal/search/es`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
           .send({
             params: {
               terminateAfter: 1,
@@ -66,11 +70,13 @@ export default function ({ getService }: FtrProviderContext) {
         expect(resp.body.isPartial).to.be(false);
         expect(resp.body.isRunning).to.be(false);
         expect(resp.body.rawResponse.terminated_early).to.be(true);
+        expect(resp.header).to.have.property(ELASTIC_HTTP_VERSION_HEADER, '1');
       });
 
       it('should return 404 when if no strategy is provided', async () => {
         const resp = await supertest
           .post(`/internal/search`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
           .send({
             body: {
               query: {
@@ -86,6 +92,7 @@ export default function ({ getService }: FtrProviderContext) {
       it('should return 404 when if unknown strategy is provided', async () => {
         const resp = await supertest
           .post(`/internal/search/banana`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
           .send({
             body: {
               query: {
@@ -97,11 +104,13 @@ export default function ({ getService }: FtrProviderContext) {
 
         verifyErrorResponse(resp.body, 404);
         expect(resp.body.message).to.contain('banana not found');
+        expect(resp.header).to.have.property(ELASTIC_HTTP_VERSION_HEADER, '1');
       });
 
       it('should return 400 with illegal ES argument', async () => {
         const resp = await supertest
           .post(`/internal/search/es`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
           .send({
             params: {
               timeout: 1, // This should be a time range string!
@@ -122,6 +131,7 @@ export default function ({ getService }: FtrProviderContext) {
       it('should return 400 with a bad body', async () => {
         const resp = await supertest
           .post(`/internal/search/es`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
           .send({
             params: {
               body: {
@@ -136,22 +146,35 @@ export default function ({ getService }: FtrProviderContext) {
       });
 
       it('should return 400 for a painless error', async () => {
-        const resp = await supertest.post(`/internal/search/es`).send(painlessErrReq).expect(400);
+        const resp = await supertest
+          .post(`/internal/search/es`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
+          .send(painlessErrReq)
+          .expect(400);
 
         verifyErrorResponse(resp.body, 400, 'search_phase_execution_exception', true);
       });
     });
 
     describe('delete', () => {
       it('should return 404 when no search id provided', async () => {
-        const resp = await supertest.delete(`/internal/search/es`).send().expect(404);
+        const resp = await supertest
+          .delete(`/internal/search/es`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
+          .send()
+          .expect(404);
         verifyErrorResponse(resp.body, 404);
       });
 
       it('should return 400 when trying a delete on a non supporting strategy', async () => {
-        const resp = await supertest.delete(`/internal/search/es/123`).send().expect(400);
+        const resp = await supertest
+          .delete(`/internal/search/es/123`)
+          .set(ELASTIC_HTTP_VERSION_HEADER, '1')
+          .send()
+          .expect(400);
         verifyErrorResponse(resp.body, 400);
         expect(resp.body.message).to.contain("Search strategy es doesn't support cancellations");
+        expect(resp.header).to.have.property(ELASTIC_HTTP_VERSION_HEADER, '1');
       });
     });
   });