[TSVB] use new Search API for rollup search (#83275) (#83639)

* [TSVB] use new Search API for rollup search Closes: #82710 * remove unused code * rollup_search_strategy.test.js -> rollup_search_strategy.test.ts * default_search_capabilities.test.js -> default_search_capabilities.test.ts * remove getRollupService * fix CI * fix some types * update types * update codeowners * fix PR comments Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # .github/CODEOWNERS # x-pack/plugins/vis_type_timeseries_enhanced/server/search_strategies/rollup_search_capabilities.test.ts
elastic · Nov 18, 2020 · 98354d1 · 98354d1
1 parent 90f7699
commit 98354d1
Show file tree

Hide file tree

Showing 28 changed files with 452 additions and 482 deletions.
diff --git a/docs/developer/plugin-list.asciidoc b/docs/developer/plugin-list.asciidoc
@@ -554,6 +554,10 @@ in their infrastructure.
 |NOTE: This plugin contains implementation of URL drilldown. For drilldowns infrastructure code refer to ui_actions_enhanced plugin.
 
 
+|{kib-repo}blob/{branch}/x-pack/plugins/vis_type_timeseries_enhanced/README.md[visTypeTimeseriesEnhanced]
+|The vis_type_timeseries_enhanced plugin is the x-pack counterpart to the OSS vis_type_timeseries plugin.
+
+
 |{kib-repo}blob/{branch}/x-pack/plugins/watcher/README.md[watcher]
 |This plugins adopts some conventions in addition to or in place of conventions in Kibana (at the time of the plugin's creation):
 

diff --git a/src/plugins/vis_type_timeseries/server/index.ts b/src/plugins/vis_type_timeseries/server/index.ts
@@ -43,7 +43,9 @@ export {
   AbstractSearchStrategy,
   ReqFacade,
 } from './lib/search_strategies/strategies/abstract_search_strategy';
-// @ts-ignore
+
+export { VisPayload } from '../common/types';
+
 export { DefaultSearchCapabilities } from './lib/search_strategies/default_search_capabilities';
 
 export function plugin(initializerContext: PluginInitializerContext) {

diff --git a/src/plugins/vis_type_timeseries/server/lib/get_fields.ts b/src/plugins/vis_type_timeseries/server/lib/get_fields.ts
@@ -38,7 +38,7 @@ export async function getFields(
   // removes the need to refactor many layers of dependencies on "req", and instead just augments the top
   // level object passed from here. The layers should be refactored fully at some point, but for now
   // this works and we are still using the New Platform services for these vis data portions.
-  const reqFacade: ReqFacade = {
+  const reqFacade: ReqFacade<{}> = {
     requestContext,
     ...request,
     framework,

diff --git a/src/plugins/vis_type_timeseries/server/lib/get_vis_data.ts b/src/plugins/vis_type_timeseries/server/lib/get_vis_data.ts
@@ -64,7 +64,7 @@ export function getVisData(
   // removes the need to refactor many layers of dependencies on "req", and instead just augments the top
   // level object passed from here. The layers should be refactored fully at some point, but for now
   // this works and we are still using the New Platform services for these vis data portions.
-  const reqFacade: ReqFacade = {
+  const reqFacade: ReqFacade<GetVisDataOptions> = {
     requestContext,
     ...request,
     framework,

diff --git a/...egies/default_search_capabilities.test.js → ...egies/default_search_capabilities.test.ts b/...egies/default_search_capabilities.test.js → ...egies/default_search_capabilities.test.ts
@@ -17,13 +17,15 @@
  * under the License.
  */
 import { DefaultSearchCapabilities } from './default_search_capabilities';
+import { ReqFacade } from './strategies/abstract_search_strategy';
+import { VisPayload } from '../../../common/types';
 
 describe('DefaultSearchCapabilities', () => {
-  let defaultSearchCapabilities;
-  let req;
+  let defaultSearchCapabilities: DefaultSearchCapabilities;
+  let req: ReqFacade<VisPayload>;
 
   beforeEach(() => {
-    req = {};
+    req = {} as ReqFacade<VisPayload>;
     defaultSearchCapabilities = new DefaultSearchCapabilities(req);
   });
 
@@ -45,13 +47,13 @@ describe('DefaultSearchCapabilities', () => {
   });
 
   test('should return Search Timezone', () => {
-    defaultSearchCapabilities.request = {
+    defaultSearchCapabilities.request = ({
       payload: {
         timerange: {
           timezone: 'UTC',
         },
       },
-    };
+    } as unknown) as ReqFacade<VisPayload>;
 
     expect(defaultSearchCapabilities.searchTimezone).toEqual('UTC');
   });

diff --git a/...strategies/default_search_capabilities.js → ...strategies/default_search_capabilities.ts b/...strategies/default_search_capabilities.js → ...strategies/default_search_capabilities.ts
@@ -16,77 +16,80 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { Unit } from '@elastic/datemath';
 import {
   convertIntervalToUnit,
   parseInterval,
   getSuitableUnit,
 } from '../vis_data/helpers/unit_to_seconds';
 import { RESTRICTIONS_KEYS } from '../../../common/ui_restrictions';
+import { ReqFacade } from './strategies/abstract_search_strategy';
+import { VisPayload } from '../../../common/types';
 
-const getTimezoneFromRequest = (request) => {
+const getTimezoneFromRequest = (request: ReqFacade<VisPayload>) => {
   return request.payload.timerange.timezone;
 };
 
 export class DefaultSearchCapabilities {
-  constructor(request, fieldsCapabilities = {}) {
-    this.request = request;
-    this.fieldsCapabilities = fieldsCapabilities;
-  }
+  constructor(
+    public request: ReqFacade<VisPayload>,
+    public fieldsCapabilities: Record<string, any> = {}
+  ) {}
 
-  get defaultTimeInterval() {
+  public get defaultTimeInterval() {
     return null;
   }
 
-  get whiteListedMetrics() {
+  public get whiteListedMetrics() {
     return this.createUiRestriction();
   }
 
-  get whiteListedGroupByFields() {
+  public get whiteListedGroupByFields() {
     return this.createUiRestriction();
   }
 
-  get whiteListedTimerangeModes() {
+  public get whiteListedTimerangeModes() {
     return this.createUiRestriction();
   }
 
-  get uiRestrictions() {
+  public get uiRestrictions() {
     return {
       [RESTRICTIONS_KEYS.WHITE_LISTED_METRICS]: this.whiteListedMetrics,
       [RESTRICTIONS_KEYS.WHITE_LISTED_GROUP_BY_FIELDS]: this.whiteListedGroupByFields,
       [RESTRICTIONS_KEYS.WHITE_LISTED_TIMERANGE_MODES]: this.whiteListedTimerangeModes,
     };
   }
 
-  get searchTimezone() {
+  public get searchTimezone() {
     return getTimezoneFromRequest(this.request);
   }
 
-  createUiRestriction(restrictionsObject) {
+  createUiRestriction(restrictionsObject?: Record<string, any>) {
     return {
       '*': !restrictionsObject,
       ...(restrictionsObject || {}),
     };
   }
 
-  parseInterval(interval) {
+  parseInterval(interval: string) {
     return parseInterval(interval);
   }
 
-  getSuitableUnit(intervalInSeconds) {
+  getSuitableUnit(intervalInSeconds: string | number) {
     return getSuitableUnit(intervalInSeconds);
   }
 
-  convertIntervalToUnit(intervalString, unit) {
+  convertIntervalToUnit(intervalString: string, unit: Unit) {
     const parsedInterval = this.parseInterval(intervalString);
 
-    if (parsedInterval.unit !== unit) {
+    if (parsedInterval?.unit !== unit) {
       return convertIntervalToUnit(intervalString, unit);
     }
 
     return parsedInterval;
   }
 
-  getValidTimeInterval(intervalString) {
+  getValidTimeInterval(intervalString: string) {
     // Default search capabilities doesn't have any restrictions for the interval string
     return intervalString;
   }

diff --git a/...ugins/vis_type_timeseries/server/lib/search_strategies/search_strategies_registry.test.ts b/...ugins/vis_type_timeseries/server/lib/search_strategies/search_strategies_registry.test.ts
@@ -27,10 +27,10 @@ import { DefaultSearchCapabilities } from './default_search_capabilities';
 
 class MockSearchStrategy extends AbstractSearchStrategy {
   checkForViability() {
-    return {
+    return Promise.resolve({
       isViable: true,
       capabilities: {},
-    };
+    });
   }
 }
 
@@ -65,7 +65,7 @@ describe('SearchStrategyRegister', () => {
   });
 
   test('should add a strategy if it is an instance of AbstractSearchStrategy', () => {
-    const anotherSearchStrategy = new MockSearchStrategy('es');
+    const anotherSearchStrategy = new MockSearchStrategy();
     const addedStrategies = registry.addStrategy(anotherSearchStrategy);
 
     expect(addedStrategies.length).toEqual(2);
@@ -75,7 +75,7 @@ describe('SearchStrategyRegister', () => {
   test('should return a MockSearchStrategy instance', async () => {
     const req = {};
     const indexPattern = '*';
-    const anotherSearchStrategy = new MockSearchStrategy('es');
+    const anotherSearchStrategy = new MockSearchStrategy();
     registry.addStrategy(anotherSearchStrategy);
 
     const { searchStrategy, capabilities } = (await registry.getViableStrategy(req, indexPattern))!;

diff --git a/...s/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts b/...s/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts
@@ -46,16 +46,8 @@ export interface ReqFacade<T = unknown> extends FakeRequest {
   getEsShardTimeout: () => Promise<number>;
 }
 
-export class AbstractSearchStrategy {
-  public indexType?: string;
-  public additionalParams: any;
-
-  constructor(type?: string, additionalParams: any = {}) {
-    this.indexType = type;
-    this.additionalParams = additionalParams;
-  }
-
-  async search(req: ReqFacade<VisPayload>, bodies: any[], options = {}) {
+export abstract class AbstractSearchStrategy {
+  async search(req: ReqFacade<VisPayload>, bodies: any[], indexType?: string) {
     const requests: any[] = [];
     const { sessionId } = req.payload;
 
@@ -64,15 +56,13 @@ export class AbstractSearchStrategy {
         req.requestContext
           .search!.search(
             {
+              indexType,
               params: {
                 ...body,
-                ...this.additionalParams,
               },
-              indexType: this.indexType,
             },
             {
               sessionId,
-              ...options,
             }
           )
           .toPromise()
@@ -81,19 +71,23 @@ export class AbstractSearchStrategy {
     return Promise.all(requests);
   }
 
-  async getFieldsForWildcard(req: ReqFacade, indexPattern: string, capabilities: any) {
+  checkForViability(
+    req: ReqFacade<VisPayload>,
+    indexPattern: string
+  ): Promise<{ isViable: boolean; capabilities: unknown }> {
+    throw new TypeError('Must override method');
+  }
+
+  async getFieldsForWildcard<TPayload = unknown>(
+    req: ReqFacade<TPayload>,
+    indexPattern: string,
+    capabilities?: unknown
+  ) {
     const { indexPatternsService } = req.pre;
 
     return await indexPatternsService!.getFieldsForWildcard({
       pattern: indexPattern,
       fieldCapsOptions: { allow_no_indices: true },
     });
   }
-
-  checkForViability(
-    req: ReqFacade,
-    indexPattern: string
-  ): { isViable: boolean; capabilities: any } {
-    throw new TypeError('Must override method');
-  }
 }
diff --git a/...trategies/default_search_strategy.test.js → ...trategies/default_search_strategy.test.ts b/...trategies/default_search_strategy.test.js → ...trategies/default_search_strategy.test.ts
@@ -17,13 +17,15 @@
  * under the License.
  */
 import { DefaultSearchStrategy } from './default_search_strategy';
+import { ReqFacade } from './abstract_search_strategy';
+import { VisPayload } from '../../../../common/types';
 
 describe('DefaultSearchStrategy', () => {
-  let defaultSearchStrategy;
-  let req;
+  let defaultSearchStrategy: DefaultSearchStrategy;
+  let req: ReqFacade<VisPayload>;
 
   beforeEach(() => {
-    req = {};
+    req = {} as ReqFacade<VisPayload>;
     defaultSearchStrategy = new DefaultSearchStrategy();
   });
 
@@ -34,8 +36,8 @@ describe('DefaultSearchStrategy', () => {
     expect(defaultSearchStrategy.getFieldsForWildcard).toBeDefined();
   });
 
-  test('should check a strategy for viability', () => {
-    const value = defaultSearchStrategy.checkForViability(req);
+  test('should check a strategy for viability', async () => {
+    const value = await defaultSearchStrategy.checkForViability(req);
 
     expect(value.isViable).toBe(true);
     expect(value.capabilities).toEqual({

diff --git a/...ies/strategies/default_search_strategy.js → ...ies/strategies/default_search_strategy.ts b/...ies/strategies/default_search_strategy.js → ...ies/strategies/default_search_strategy.ts
@@ -17,16 +17,17 @@
  * under the License.
  */
 
-import { AbstractSearchStrategy } from './abstract_search_strategy';
+import { AbstractSearchStrategy, ReqFacade } from './abstract_search_strategy';
 import { DefaultSearchCapabilities } from '../default_search_capabilities';
+import { VisPayload } from '../../../../common/types';
 
 export class DefaultSearchStrategy extends AbstractSearchStrategy {
   name = 'default';
 
-  checkForViability(req) {
-    return {
+  checkForViability(req: ReqFacade<VisPayload>) {
+    return Promise.resolve({
       isViable: true,
       capabilities: new DefaultSearchCapabilities(req),
-    };
+    });
   }
 }
diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/get_bucket_size.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/get_bucket_size.js
@@ -42,14 +42,18 @@ const calculateBucketData = (timeInterval, capabilities) => {
   }
 
   // Check decimal
-  if (parsedInterval.value % 1 !== 0) {
+  if (parsedInterval && parsedInterval.value % 1 !== 0) {
     if (parsedInterval.unit !== 'ms') {
-      const { value, unit } = convertIntervalToUnit(
+      const converted = convertIntervalToUnit(
         intervalString,
         ASCENDING_UNIT_ORDER[ASCENDING_UNIT_ORDER.indexOf(parsedInterval.unit) - 1]
       );
 
-      intervalString = value + unit;
+      if (converted) {
+        intervalString = converted.value + converted.unit;
+      }
+
+      intervalString = undefined;
     } else {
       intervalString = '1ms';
     }