[APM] Prevent imports of public in server code (#75979)

cauemarcondes · web-flow · commit 712c4bdde79e · 2020-08-28T12:43:49.000+02:00
* renaming test file

* removing eslint rule

* restructuring observability types

* refactoring fetchOptions type
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -296,10 +296,7 @@ module.exports = {
                 errorMessage: `Plugins may only import from src/core/server and src/core/public.`,
               },
               {
-                target: [
-                  '(src|x-pack)/plugins/*/server/**/*',
-                  '!x-pack/plugins/apm/**/*', // https://github.com/elastic/kibana/issues/67210
-                ],
+                target: ['(src|x-pack)/plugins/*/server/**/*'],
                 from: ['(src|x-pack)/plugins/*/public/**/*'],
                 errorMessage: `Server code can not import from public, use a common directory.`,
               },
diff --git a/x-pack/plugins/apm/common/fetch_options.ts b/x-pack/plugins/apm/common/fetch_options.ts
@@ -0,0 +1,14 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { HttpFetchOptions } from 'kibana/public';
+
+export type FetchOptions = Omit<HttpFetchOptions, 'body'> & {
+  pathname: string;
+  isCachable?: boolean;
+  method?: string;
+  body?: any;
+};
diff --git a/x-pack/plugins/apm/public/hooks/useCallApi.ts b/x-pack/plugins/apm/public/hooks/useCallApi.ts
@@ -5,8 +5,9 @@
  */
 
 import { useMemo } from 'react';
-import { callApi, FetchOptions } from '../services/rest/callApi';
+import { callApi } from '../services/rest/callApi';
 import { useApmPluginContext } from './useApmPluginContext';
+import { FetchOptions } from '../../common/fetch_options';
 
 export function useCallApi() {
   const { http } = useApmPluginContext().core;
diff --git a/x-pack/plugins/apm/public/services/rest/callApi.ts b/x-pack/plugins/apm/public/services/rest/callApi.ts
@@ -4,17 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { HttpSetup } from 'kibana/public';
 import { isString, startsWith } from 'lodash';
 import LRU from 'lru-cache';
 import hash from 'object-hash';
-import { HttpSetup, HttpFetchOptions } from 'kibana/public';
-
-export type FetchOptions = Omit<HttpFetchOptions, 'body'> & {
-  pathname: string;
-  isCachable?: boolean;
-  method?: string;
-  body?: any;
-};
+import { FetchOptions } from '../../../common/fetch_options';
 
 function fetchOptionsWithDebug(fetchOptions: FetchOptions) {
   const debugEnabled =
diff --git a/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts b/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts
@@ -4,7 +4,8 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { HttpSetup } from 'kibana/public';
-import { callApi, FetchOptions } from './callApi';
+import { FetchOptions } from '../../../common/fetch_options';
+import { callApi } from './callApi';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { APMAPI } from '../../../server/routes/create_apm_api';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
diff --git a/x-pack/plugins/apm/server/lib/environments/get_all_environments.test.ts b/x-pack/plugins/apm/server/lib/environments/get_all_environments.test.ts
@@ -8,7 +8,7 @@ import { getAllEnvironments } from './get_all_environments';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 
 describe('getAllEnvironments', () => {
   let mock: SearchParamsMock;
diff --git a/x-pack/plugins/apm/server/lib/errors/distribution/queries.test.ts b/x-pack/plugins/apm/server/lib/errors/distribution/queries.test.ts
@@ -8,7 +8,7 @@ import { getErrorDistribution } from './get_distribution';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../../public/utils/testHelpers';
+} from '../../../utils/test_helpers';
 
 describe('error distribution queries', () => {
   let mock: SearchParamsMock;
diff --git a/x-pack/plugins/apm/server/lib/errors/queries.test.ts b/x-pack/plugins/apm/server/lib/errors/queries.test.ts
@@ -9,7 +9,7 @@ import { getErrorGroups } from './get_error_groups';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 
 describe('error queries', () => {
   let mock: SearchParamsMock;
diff --git a/x-pack/plugins/apm/server/lib/metrics/queries.test.ts b/x-pack/plugins/apm/server/lib/metrics/queries.test.ts
@@ -12,7 +12,7 @@ import { getThreadCountChart } from './by_agent/java/thread_count';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 import { SERVICE_NODE_NAME_MISSING } from '../../../common/service_nodes';
 
 describe('metrics queries', () => {
diff --git a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts b/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts
@@ -9,7 +9,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { rangeFilter } from '../../../common/utils/range_filter';
-import { Coordinates } from '../../../../observability/public';
+import { Coordinates } from '../../../../observability/typings/common';
 import { Setup, SetupTimeRange } from '../helpers/setup_request';
 import { ProcessorEvent } from '../../../common/processor_event';
 
diff --git a/x-pack/plugins/apm/server/lib/rum_client/queries.test.ts b/x-pack/plugins/apm/server/lib/rum_client/queries.test.ts
@@ -7,7 +7,7 @@
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 import { getClientMetrics } from './get_client_metrics';
 import { getPageViewTrends } from './get_page_view_trends';
 import { getPageLoadDistribution } from './get_page_load_distribution';
diff --git a/x-pack/plugins/apm/server/lib/service_nodes/queries.test.ts b/x-pack/plugins/apm/server/lib/service_nodes/queries.test.ts
@@ -13,7 +13,7 @@ import { getServiceNodes } from './';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 import { getServiceNodeMetadata } from '../services/get_service_node_metadata';
 import { SERVICE_NODE_NAME_MISSING } from '../../../common/service_nodes';
 
diff --git a/x-pack/plugins/apm/server/lib/services/annotations/index.test.ts b/x-pack/plugins/apm/server/lib/services/annotations/index.test.ts
@@ -7,7 +7,7 @@ import { getDerivedServiceAnnotations } from './get_derived_service_annotations'
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../../public/utils/testHelpers';
+} from '../../../utils/test_helpers';
 import noVersions from './__fixtures__/no_versions.json';
 import oneVersion from './__fixtures__/one_version.json';
 import multipleVersions from './__fixtures__/multiple_versions.json';
diff --git a/x-pack/plugins/apm/server/lib/services/queries.test.ts b/x-pack/plugins/apm/server/lib/services/queries.test.ts
@@ -12,7 +12,7 @@ import { hasHistoricalAgentData } from './get_services/has_historical_agent_data
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 
 describe('services queries', () => {
   let mock: SearchParamsMock;
diff --git a/x-pack/plugins/apm/server/lib/settings/agent_configuration/queries.test.ts b/x-pack/plugins/apm/server/lib/settings/agent_configuration/queries.test.ts
@@ -11,7 +11,7 @@ import { searchConfigurations } from './search_configurations';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../../public/utils/testHelpers';
+} from '../../../utils/test_helpers';
 import { findExactConfiguration } from './find_exact_configuration';
 
 describe('agent configuration queries', () => {
diff --git a/x-pack/plugins/apm/server/lib/settings/custom_link/create_or_update_custom_link.test.ts b/x-pack/plugins/apm/server/lib/settings/custom_link/create_or_update_custom_link.test.ts
@@ -5,7 +5,7 @@
  */
 
 import { Setup } from '../../helpers/setup_request';
-import { mockNow } from '../../../../public/utils/testHelpers';
+import { mockNow } from '../../../utils/test_helpers';
 import { CustomLink } from '../../../../common/custom_link/custom_link_types';
 import { createOrUpdateCustomLink } from './create_or_update_custom_link';
 
diff --git a/x-pack/plugins/apm/server/lib/settings/custom_link/get_transaction.test.ts b/x-pack/plugins/apm/server/lib/settings/custom_link/get_transaction.test.ts
@@ -6,7 +6,7 @@
 import {
   inspectSearchParams,
   SearchParamsMock,
-} from '../../../../public/utils/testHelpers';
+} from '../../../utils/test_helpers';
 import { getTransaction } from './get_transaction';
 import { Setup } from '../../helpers/setup_request';
 import {
diff --git a/x-pack/plugins/apm/server/lib/settings/custom_link/list_custom_links.test.ts b/x-pack/plugins/apm/server/lib/settings/custom_link/list_custom_links.test.ts
@@ -8,7 +8,7 @@ import { listCustomLinks } from './list_custom_links';
 import {
   inspectSearchParams,
   SearchParamsMock,
-} from '../../../../public/utils/testHelpers';
+} from '../../../utils/test_helpers';
 import { Setup } from '../../helpers/setup_request';
 import {
   SERVICE_NAME,
diff --git a/x-pack/plugins/apm/server/lib/traces/queries.test.ts b/x-pack/plugins/apm/server/lib/traces/queries.test.ts
@@ -8,7 +8,7 @@ import { getTraceItems } from './get_trace_items';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 
 describe('trace queries', () => {
   let mock: SearchParamsMock;
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts b/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts
@@ -8,7 +8,7 @@ import { transactionGroupsFetcher } from './fetcher';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 
 describe('transaction group queries', () => {
   let mock: SearchParamsMock;
diff --git a/x-pack/plugins/apm/server/lib/transactions/queries.test.ts b/x-pack/plugins/apm/server/lib/transactions/queries.test.ts
@@ -11,7 +11,7 @@ import { getTransaction } from './get_transaction';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { loggerMock } from '../../../../../../src/core/server/logging/logger.mock';
 
diff --git a/x-pack/plugins/apm/server/lib/ui_filters/local_ui_filters/queries.test.ts b/x-pack/plugins/apm/server/lib/ui_filters/local_ui_filters/queries.test.ts
@@ -8,7 +8,7 @@ import { getLocalUIFilters } from './';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../../public/utils/testHelpers';
+} from '../../../utils/test_helpers';
 import { getServicesProjection } from '../../../projections/services';
 
 describe('local ui filter queries', () => {
diff --git a/x-pack/plugins/apm/server/lib/ui_filters/queries.test.ts b/x-pack/plugins/apm/server/lib/ui_filters/queries.test.ts
@@ -8,7 +8,7 @@ import { getEnvironments } from './get_environments';
 import {
   SearchParamsMock,
   inspectSearchParams,
-} from '../../../public/utils/testHelpers';
+} from '../../utils/test_helpers';
 
 describe('ui filter queries', () => {
   let mock: SearchParamsMock;
diff --git a/x-pack/plugins/apm/server/routes/typings.ts b/x-pack/plugins/apm/server/routes/typings.ts
@@ -15,10 +15,9 @@ import { PickByValue, Optional } from 'utility-types';
 import { Observable } from 'rxjs';
 import { Server } from 'hapi';
 import { ObservabilityPluginSetup } from '../../../observability/server';
-// eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { FetchOptions } from '../../public/services/rest/callApi';
 import { SecurityPluginSetup } from '../../../security/server';
 import { MlPluginSetup } from '../../../ml/server';
+import { FetchOptions } from '../../common/fetch_options';
 import { APMConfig } from '..';
 
 export interface Params {
diff --git a/x-pack/plugins/apm/server/utils/test_helpers.tsx b/x-pack/plugins/apm/server/utils/test_helpers.tsx
@@ -0,0 +1,117 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+import {
+  ESFilter,
+  ESSearchResponse,
+  ESSearchRequest,
+} from '../../typings/elasticsearch';
+import { PromiseReturnType } from '../../typings/common';
+import { APMConfig } from '..';
+
+interface Options {
+  mockResponse?: (
+    request: ESSearchRequest
+  ) => ESSearchResponse<unknown, ESSearchRequest>;
+}
+
+interface MockSetup {
+  start: number;
+  end: number;
+  apmEventClient: any;
+  internalClient: any;
+  config: APMConfig;
+  uiFiltersES: ESFilter[];
+  indices: {
+    /* eslint-disable @typescript-eslint/naming-convention */
+    'apm_oss.sourcemapIndices': string;
+    'apm_oss.errorIndices': string;
+    'apm_oss.onboardingIndices': string;
+    'apm_oss.spanIndices': string;
+    'apm_oss.transactionIndices': string;
+    'apm_oss.metricsIndices': string;
+    /* eslint-enable @typescript-eslint/naming-convention */
+    apmAgentConfigurationIndex: string;
+    apmCustomLinkIndex: string;
+  };
+}
+
+export async function inspectSearchParams(
+  fn: (mockSetup: MockSetup) => Promise<any>,
+  options: Options = {}
+) {
+  const spy = jest.fn().mockImplementation(async (request) => {
+    return options.mockResponse
+      ? options.mockResponse(request)
+      : {
+          hits: {
+            hits: {
+              total: {
+                value: 0,
+              },
+            },
+          },
+        };
+  });
+
+  let response;
+  let error;
+
+  const mockSetup = {
+    start: 1528113600000,
+    end: 1528977600000,
+    apmEventClient: { search: spy } as any,
+    internalClient: { search: spy } as any,
+    config: new Proxy(
+      {},
+      {
+        get: (_, key) => {
+          switch (key) {
+            default:
+              return 'myIndex';
+
+            case 'xpack.apm.metricsInterval':
+              return 30;
+          }
+        },
+      }
+    ) as APMConfig,
+    uiFiltersES: [{ term: { 'my.custom.ui.filter': 'foo-bar' } }],
+    indices: {
+      /* eslint-disable @typescript-eslint/naming-convention */
+      'apm_oss.sourcemapIndices': 'myIndex',
+      'apm_oss.errorIndices': 'myIndex',
+      'apm_oss.onboardingIndices': 'myIndex',
+      'apm_oss.spanIndices': 'myIndex',
+      'apm_oss.transactionIndices': 'myIndex',
+      'apm_oss.metricsIndices': 'myIndex',
+      /* eslint-enable @typescript-eslint/naming-convention */
+      apmAgentConfigurationIndex: 'myIndex',
+      apmCustomLinkIndex: 'myIndex',
+    },
+    dynamicIndexPattern: null as any,
+  };
+  try {
+    response = await fn(mockSetup);
+  } catch (err) {
+    error = err;
+    // we're only extracting the search params
+  }
+
+  return {
+    params: spy.mock.calls[0][0],
+    response,
+    error,
+    spy,
+    teardown: () => spy.mockClear(),
+  };
+}
+
+export type SearchParamsMock = PromiseReturnType<typeof inspectSearchParams>;
+
+export function mockNow(date: string | number | Date) {
+  const fakeNow = new Date(date).getTime();
+  return jest.spyOn(Date, 'now').mockReturnValue(fakeNow);
+}
diff --git a/x-pack/plugins/observability/typings/common.ts b/x-pack/plugins/observability/typings/common.ts
@@ -9,3 +9,5 @@ export type ObservabilityApp = 'infra_metrics' | 'infra_logs' | 'apm' | 'uptime'
 export type PromiseReturnType<Func> = Func extends (...args: any[]) => Promise<infer Value>
   ? Value
   : Func;
+
+export { Coordinates } from '../public/typings/fetch_overview_data/';
