Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[File upload][Maps] NP migration for server & client #51045

Merged
merged 20 commits into from
Nov 25, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c5ff754
Use np savedObjectsClient in indexing service
Nov 11, 2019
ba8ac43
Export File Upload UI via start since it requires initialization
Nov 12, 2019
2169a73
Pass services through top level react component props
Nov 13, 2019
dcd9444
Handle basePath ref and 'kbn-version' for requests
Nov 13, 2019
340e611
Merge remote-tracking branch 'upstream/master' into np-migrate-file-u…
Nov 14, 2019
4cfbe91
Bulk of logic for removing hapi server dependencies for server app
Nov 19, 2019
7593926
Use request obj subset of original request
Nov 19, 2019
afcfe02
Move startup logic over to server plugin file and call from index.js
Nov 19, 2019
42d1f97
Merge remote-tracking branch 'upstream/master' into np-migrate-file-u…
Nov 19, 2019
562850a
Update server tests
Nov 19, 2019
65e517b
Clean up
Nov 20, 2019
a5d4f1e
Merge remote-tracking branch 'upstream/master' into np-migrate-file-u…
Nov 20, 2019
359fc5f
Remove old makeUsageCollector export statement
Nov 20, 2019
deeacd0
Merge remote-tracking branch 'upstream/master' into np-migrate-file-u…
Nov 20, 2019
8c64c06
Merge remote-tracking branch 'upstream/master' into np-migrate-file-u…
Nov 20, 2019
9bfc86d
initServicesAndConstants in the start method instead of in the react …
Nov 21, 2019
c202aa8
Merge remote-tracking branch 'upstream/master' into np-migrate-file-u…
Nov 21, 2019
06b59eb
Review feedback
Nov 22, 2019
8bdb72b
Merge remote-tracking branch 'upstream/master' into np-migrate-file-u…
Nov 22, 2019
baaa1b8
Merge branch 'master' into np-migrate-file-upload-client
elasticmachine Nov 25, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ export const ES_GEO_FIELD_TYPE = {
GEO_POINT: 'geo_point',
GEO_SHAPE: 'geo_shape',
};

export const DEFAULT_KBN_VERSION = 'kbnVersion';
30 changes: 22 additions & 8 deletions x-pack/legacy/plugins/file_upload/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { mirrorPluginStatus } from '../../server/lib/mirror_plugin_status';
import { fileUploadRoutes } from './server/routes/file_upload';
import { makeUsageCollector } from './server/telemetry/';
import mappings from './mappings';
import { FileUploadPlugin } from './server/plugin';
import { mappings } from './mappings';

export const fileUpload = kibana => {
return new kibana.Plugin({
Expand All @@ -23,11 +21,27 @@ export const fileUpload = kibana => {
},

init(server) {
const { xpack_main: xpackMainPlugin } = server.plugins;
const coreSetup = server.newPlatform.setup.core;
const pluginsSetup = {};

mirrorPluginStatus(xpackMainPlugin, this);
fileUploadRoutes(server);
makeUsageCollector(server);
// legacy dependencies
const __LEGACY = {
route: server.route.bind(server),
plugins: {
elasticsearch: server.plugins.elasticsearch,
},
savedObjects: {
getSavedObjectsRepository:
server.savedObjects.getSavedObjectsRepository
kindsun marked this conversation as resolved.
Show resolved Hide resolved
},
usage: {
collectorSet: {
makeUsageCollector: server.usage.collectorSet.makeUsageCollector
}
}
};

new FileUploadPlugin().setup(coreSetup, pluginsSetup, __LEGACY);
}
});
};
9 changes: 0 additions & 9 deletions x-pack/legacy/plugins/file_upload/mappings.json

This file was deleted.

15 changes: 15 additions & 0 deletions x-pack/legacy/plugins/file_upload/mappings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* 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.
*/

export const mappings = {
'file-upload-telemetry': {
properties: {
filesUploadedTotalCount: {
type: 'long',
},
},
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import React, { Fragment, Component } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiCodeBlock, EuiSpacer, EuiText, EuiTitle, EuiProgress, EuiCallOut } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import chrome from 'ui/chrome';
import { basePath } from '../kibana_services';

export class JsonImportProgress extends Component {

Expand Down Expand Up @@ -114,7 +114,7 @@ export class JsonImportProgress extends Component {
<a
data-test-subj="indexManagementNewIndexLink"
target="_blank"
href={`${chrome.getBasePath()}/app/kibana#/
href={`${basePath}/app/kibana#/
management/elasticsearch/index_management/indices/
filter/${indexName}`.replace(/\s/g, '')}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { JsonUploadAndParse } from './components/json_upload_and_parse';
import { FileUploadPlugin as Plugin } from './plugin';

export function plugin() {
return new Plugin();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to alias, just do new FileUploadPlugin

Even beyond that, what is this file adding beyond indirection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's about all it's adding here since we're not sharing any static code. I threw it in there to give it the same "shape" as other plugins. Other examples I saw were pretty minimal and didn't add much either. Agreed that aliasing is superfluous here.

}
13 changes: 13 additions & 0 deletions x-pack/legacy/plugins/file_upload/public/kibana_services.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,18 @@
*/

import { start as data } from '../../../../../src/legacy/core_plugins/data/public/legacy';
import { DEFAULT_KBN_VERSION } from '../common/constants/file_import';

export const indexPatternService = data.indexPatterns.indexPatterns;

export let savedObjectsClient;
export let basePath;
export let apiBasePath;
export let kbnVersion;

export const initServicesAndConstants = ({ savedObjects, http, injectedMetadata }) => {
savedObjectsClient = savedObjects.client;
basePath = http.basePath.basePath;
apiBasePath = http.basePath.prepend('/api');
kbnVersion = injectedMetadata.getKibanaVersion(DEFAULT_KBN_VERSION);
};
12 changes: 12 additions & 0 deletions x-pack/legacy/plugins/file_upload/public/legacy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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 { npStart } from 'ui/new_platform';
import { plugin } from '.';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that looks weird

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled from the example on that one and found a few others who used the same syntax. No strong feelings either way personally so I just stuck with the example syntax


const pluginInstance = plugin();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this module hopping? Is this something from the platform?

import { FileUploadPlugin } from './plugin';

const pluginInstance = new FileUploadPlugin();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again just sticking with the expected shape for consistency with other plugins at this stage. Ultimately this file will go away when moved though.


export const start = pluginInstance.start(npStart.core);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the rename when it is being renamed yet again when it is imported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the previous comment on naming it "start". Here's the relevant section in the migration guide.

33 changes: 33 additions & 0 deletions x-pack/legacy/plugins/file_upload/public/plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* 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 { Plugin, CoreStart } from 'src/core/public';
// @ts-ignore
import { initResources } from './util/indexing_service';
// @ts-ignore
import { JsonUploadAndParse } from './components/json_upload_and_parse';
// @ts-ignore
import { initServicesAndConstants } from './kibana_services';

/**
* These are the interfaces with your public contracts. You should export these
* for other plugins to use in _their_ `SetupDeps`/`StartDeps` interfaces.
* @public
*/
export type FileUploadPluginSetup = ReturnType<FileUploadPlugin['setup']>;
export type FileUploadPluginStart = ReturnType<FileUploadPlugin['start']>;

/** @internal */
export class FileUploadPlugin implements Plugin<FileUploadPluginSetup, FileUploadPluginStart> {
public setup() {}

public start(core: CoreStart) {
initServicesAndConstants(core);
return {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason this is wrapped in an object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just another convention they used in the guide and I saw used around.

JsonUploadAndParse,
};
}
}
4 changes: 2 additions & 2 deletions x-pack/legacy/plugins/file_upload/public/util/http_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

// service for interacting with the server

import chrome from 'ui/chrome';
import { addSystemApiHeader } from 'ui/system_api';
import { i18n } from '@kbn/i18n';
import { kbnVersion } from '../kibana_services';

export async function http(options) {
if(!(options && options.url)) {
Expand All @@ -20,7 +20,7 @@ export async function http(options) {
const url = options.url || '';
const headers = addSystemApiHeader({
'Content-Type': 'application/json',
'kbn-version': chrome.getXsrfToken(),
'kbn-version': kbnVersion,
...options.headers
});

Expand Down
24 changes: 11 additions & 13 deletions x-pack/legacy/plugins/file_upload/public/util/indexing_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,23 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { http } from './http_service';
import chrome from 'ui/chrome';
import { i18n } from '@kbn/i18n';
import { indexPatternService } from '../kibana_services';
import { http as httpService } from './http_service';
import {
indexPatternService,
apiBasePath,
savedObjectsClient
} from '../kibana_services';
import { getGeoJsonIndexingDetails } from './geo_processing';
import { sizeLimitedChunking } from './size_limited_chunking';
import { i18n } from '@kbn/i18n';

const basePath = chrome.addBasePath('/api/fileupload');
const fileType = 'json';

export async function indexData(parsedFile, transformDetails, indexName, dataType, appName) {
if (!parsedFile) {
throw(i18n.translate('xpack.fileUpload.indexingService.noFileImported', {
defaultMessage: 'No file imported.'
}));
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

// Perform any processing required on file prior to indexing
Expand Down Expand Up @@ -129,8 +130,8 @@ async function writeToIndex(indexingDetails) {
ingestPipeline
} = indexingDetails;

return await http({
url: `${basePath}/import${paramString}`,
return await httpService({
url: `${apiBasePath}/fileupload/import${paramString}`,
method: 'POST',
data: {
index,
Expand Down Expand Up @@ -223,7 +224,6 @@ export async function createIndexPattern(indexPatternName) {
}

async function getIndexPatternId(name) {
const savedObjectsClient = chrome.getSavedObjectsClient();
const savedObjectSearch =
await savedObjectsClient.find({ type: 'index-pattern', perPage: 1000 });
const indexPatternSavedObjects = savedObjectSearch.savedObjects;
Expand All @@ -237,9 +237,8 @@ async function getIndexPatternId(name) {
}

export const getExistingIndexNames = async () => {
const basePath = chrome.addBasePath('/api');
const indexes = await http({
url: `${basePath}/index_management/indices`,
const indexes = await httpService({
url: `${apiBasePath}/index_management/indices`,
method: 'GET',
});
return indexes
Expand All @@ -248,7 +247,6 @@ export const getExistingIndexNames = async () => {
};

export const getExistingIndexPatternNames = async () => {
const savedObjectsClient = chrome.getSavedObjectsClient();
const indexPatterns = await savedObjectsClient.find({
type: 'index-pattern',
fields: ['id', 'title', 'type', 'fields'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,4 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Server } from 'hapi';

export function callWithInternalUserFactory(server: Server): any;
export function callWithInternalUserFactory(elasticsearchPlugin: any): any;
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@
*/



import { once } from 'lodash';

const _callWithInternalUser = once((server) => {
const { callWithInternalUser } = server.plugins.elasticsearch.getCluster('admin');
const _callWithInternalUser = once(elasticsearchPlugin => {
const { callWithInternalUser } = elasticsearchPlugin.getCluster('admin');
return callWithInternalUser;
});

export const callWithInternalUserFactory = (server) => {
export const callWithInternalUserFactory = elasticsearchPlugin => {
return (...args) => {
return _callWithInternalUser(server)(...args);
return _callWithInternalUser(elasticsearchPlugin)(...args);
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,15 @@ import { callWithInternalUserFactory } from './call_with_internal_user_factory';

describe('call_with_internal_user_factory', () => {
describe('callWithInternalUserFactory', () => {
let server: any;
let callWithInternalUser: any;

beforeEach(() => {
callWithInternalUser = jest.fn();
server = {
plugins: {
elasticsearch: {
getCluster: jest.fn(() => ({ callWithInternalUser })),
},
},
};
});

it('should use internal user "admin"', () => {
const callWithInternalUserInstance = callWithInternalUserFactory(server);
const callWithInternalUser: any = jest.fn();
const elasticsearchPlugin: any = {
getCluster: jest.fn(() => ({ callWithInternalUser })),
};
const callWithInternalUserInstance = callWithInternalUserFactory(elasticsearchPlugin);
callWithInternalUserInstance();

expect(server.plugins.elasticsearch.getCluster).toHaveBeenCalledWith('admin');
expect(elasticsearchPlugin.getCluster).toHaveBeenCalledWith('admin');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@

import { once } from 'lodash';

const callWithRequest = once((server) => {
const cluster = server.plugins.elasticsearch.getCluster('data');
const callWithRequest = once(elasticsearchPlugin => {
const cluster = elasticsearchPlugin.getCluster('data');
return cluster.callWithRequest;
});

export const callWithRequestFactory = (server, request) => {
export const callWithRequestFactory = (elasticsearchPlugin, request) => {
return (...args) => {
return callWithRequest(server)(request, ...args);
return callWithRequest(elasticsearchPlugin)(request, ...args);
};
};
37 changes: 37 additions & 0 deletions x-pack/legacy/plugins/file_upload/server/plugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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 { getImportRouteHandler } from './routes/file_upload';
import { getTelemetry, initTelemetry } from './telemetry/telemetry';
import { MAX_BYTES } from '../common/constants/file_import';

const TELEMETRY_TYPE = 'fileUploadTelemetry';

export class FileUploadPlugin {
setup(core, plugins, __LEGACY) {
const elasticsearchPlugin = __LEGACY.plugins.elasticsearch;
const getSavedObjectsRepository = __LEGACY.savedObjects.getSavedObjectsRepository;
const makeUsageCollector = __LEGACY.usage.collectorSet.makeUsageCollector;

// Set up route
__LEGACY.route({
method: 'POST',
path: '/api/fileupload/import',
handler: getImportRouteHandler(elasticsearchPlugin, getSavedObjectsRepository),
config: {
payload: { maxBytes: MAX_BYTES },
}
});

// Make usage collector
makeUsageCollector({
type: TELEMETRY_TYPE,
isReady: () => true,
fetch: async () =>
(await getTelemetry(elasticsearchPlugin, getSavedObjectsRepository)) || initTelemetry()
kindsun marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
Loading