From 0dd9ccd38425e7f22a28b085033cbec3e7b80c74 Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Thu, 16 Aug 2018 09:29:09 -0400 Subject: [PATCH] [Spaces] - Reporting updates (#21457) @kobelb -- this PR in its current state is just enough to get the reporting API tests passing again. This doesn't account for space-awareness, and I'll need to pick your brain as to how to do that for reporting, but I figured that would be better addressed when we tackle space-aware advanced settings (it appears that reporting is only using the SOC via the UISettingsService below) --- .../csv/server/__tests__/execute_job.js | 26 ++++++++++- .../export_types/csv/server/create_job.js | 1 + .../export_types/csv/server/execute_job.js | 15 ++++++- .../printable_pdf/server/create_job/index.js | 3 +- .../server/execute_job/compatibility_shim.js | 22 ++++----- .../execute_job/compatibility_shim.test.js | 24 +++++++++- .../server/execute_job/get_absolute_url.js | 3 +- .../execute_job/get_absolute_url.test.js | 10 +++++ .../printable_pdf/server/execute_job/index.js | 6 +++ .../server/execute_job/index.test.js | 45 ++++++++++++++++--- 10 files changed, 130 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/reporting/export_types/csv/server/__tests__/execute_job.js b/x-pack/plugins/reporting/export_types/csv/server/__tests__/execute_job.js index 1f0504db3681d..1d409e45c092c 100644 --- a/x-pack/plugins/reporting/export_types/csv/server/__tests__/execute_job.js +++ b/x-pack/plugins/reporting/export_types/csv/server/__tests__/execute_job.js @@ -109,13 +109,35 @@ describe('CSV Execute Job', function () { mockServer.config().get.withArgs('xpack.reporting.csv.scroll').returns({}); }); - describe('savedObjects', function () { - it('calls getScopedSavedObjectsClient with request containing decrypted headers', async function () { + describe('calls getScopedSavedObjectsClient with request', function () { + it('containing decrypted headers', async function () { const executeJob = executeJobFactory(mockServer); await executeJob({ headers: encryptedHeaders, fields: [], searchRequest: { index: null, body: null } }, cancellationToken); expect(mockServer.savedObjects.getScopedSavedObjectsClient.calledOnce).to.be(true); expect(mockServer.savedObjects.getScopedSavedObjectsClient.firstCall.args[0].headers).to.be.eql(headers); }); + + it(`containing getBasePath() returning server's basePath if the job doesn't have one`, async function () { + const serverBasePath = '/foo-server/basePath/'; + mockServer.config().get.withArgs('server.basePath').returns(serverBasePath); + const executeJob = executeJobFactory(mockServer); + await executeJob({ headers: encryptedHeaders, fields: [], searchRequest: { index: null, body: null } }, cancellationToken); + expect(mockServer.savedObjects.getScopedSavedObjectsClient.calledOnce).to.be(true); + expect(mockServer.savedObjects.getScopedSavedObjectsClient.firstCall.args[0].getBasePath()).to.be.eql(serverBasePath); + }); + + it(`containing getBasePath() returning job's basePath if the job has one`, async function () { + const serverBasePath = '/foo-server/basePath/'; + mockServer.config().get.withArgs('server.basePath').returns(serverBasePath); + const executeJob = executeJobFactory(mockServer); + const jobBasePath = 'foo-job/basePath/'; + await executeJob( + { headers: encryptedHeaders, fields: [], searchRequest: { index: null, body: null }, basePath: jobBasePath }, + cancellationToken + ); + expect(mockServer.savedObjects.getScopedSavedObjectsClient.calledOnce).to.be(true); + expect(mockServer.savedObjects.getScopedSavedObjectsClient.firstCall.args[0].getBasePath()).to.be.eql(jobBasePath); + }); }); describe('uiSettings', function () { diff --git a/x-pack/plugins/reporting/export_types/csv/server/create_job.js b/x-pack/plugins/reporting/export_types/csv/server/create_job.js index 02d78a97ef9be..f116cbb763014 100644 --- a/x-pack/plugins/reporting/export_types/csv/server/create_job.js +++ b/x-pack/plugins/reporting/export_types/csv/server/create_job.js @@ -21,6 +21,7 @@ function createJobFn(server) { return { headers: serializedEncryptedHeaders, indexPatternSavedObject: indexPatternSavedObject, + basePath: request.getBasePath(), ...jobParams }; }; diff --git a/x-pack/plugins/reporting/export_types/csv/server/execute_job.js b/x-pack/plugins/reporting/export_types/csv/server/execute_job.js index a407cacc63fef..baa1cd458c8a0 100644 --- a/x-pack/plugins/reporting/export_types/csv/server/execute_job.js +++ b/x-pack/plugins/reporting/export_types/csv/server/execute_job.js @@ -16,9 +16,18 @@ function executeJobFn(server) { const config = server.config(); const logger = createTaggedLogger(server, ['reporting', 'csv', 'debug']); const generateCsv = createGenerateCsv(logger); + const serverBasePath = config.get('server.basePath'); return async function executeJob(job, cancellationToken) { - const { searchRequest, fields, indexPatternSavedObject, metaFields, conflictedTypesFields, headers: serializedEncryptedHeaders } = job; + const { + searchRequest, + fields, + indexPatternSavedObject, + metaFields, + conflictedTypesFields, + headers: serializedEncryptedHeaders, + basePath + } = job; let decryptedHeaders; try { @@ -31,6 +40,10 @@ function executeJobFn(server) { const fakeRequest = { headers: decryptedHeaders, + // This is used by the spaces SavedObjectClientWrapper to determine the existing space. + // We use the basePath from the saved job, which we'll have post spaces being implemented; + // or we use the server base path, which uses the default space + getBasePath: () => basePath || serverBasePath, }; const callEndpoint = (endpoint, clientParams = {}, options = {}) => { diff --git a/x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/index.js b/x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/index.js index bfd3cb6eaa9d5..bab11f1004552 100644 --- a/x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/index.js +++ b/x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/index.js @@ -18,7 +18,7 @@ function createJobFn(server) { relativeUrls, browserTimezone, layout - }, headers) { + }, headers, request) { const serializedEncryptedHeaders = await crypto.encrypt(headers); return { @@ -28,6 +28,7 @@ function createJobFn(server) { headers: serializedEncryptedHeaders, browserTimezone, layout, + basePath: request.getBasePath(), forceNow: new Date().toISOString(), }; }); diff --git a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.js b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.js index f2d5430f9e1fc..df55bb75d2621 100644 --- a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.js +++ b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.js @@ -10,28 +10,28 @@ import { getAbsoluteUrlFactory } from './get_absolute_url'; export function compatibilityShimFactory(server) { const getAbsoluteUrl = getAbsoluteUrlFactory(server); - const getSavedObjectAbsoluteUrl = (savedObj) => { - if (savedObj.urlHash) { - return getAbsoluteUrl({ hash: savedObj.urlHash }); + const getSavedObjectAbsoluteUrl = (job, savedObject) => { + if (savedObject.urlHash) { + return getAbsoluteUrl({ hash: savedObject.urlHash }); } - if (savedObj.relativeUrl) { - const { pathname: path, hash, search } = url.parse(savedObj.relativeUrl); - return getAbsoluteUrl({ path, hash, search }); + if (savedObject.relativeUrl) { + const { pathname: path, hash, search } = url.parse(savedObject.relativeUrl); + return getAbsoluteUrl({ basePath: job.basePath, path, hash, search }); } - if (savedObj.url.startsWith(getAbsoluteUrl())) { - return savedObj.url; + if (savedObject.url.startsWith(getAbsoluteUrl())) { + return savedObject.url; } - throw new Error(`Unable to generate report for url ${savedObj.url}, it's not a Kibana URL`); + throw new Error(`Unable to generate report for url ${savedObject.url}, it's not a Kibana URL`); }; return function (executeJob) { return async function (job, cancellationToken) { - const urls = job.objects.map(getSavedObjectAbsoluteUrl); + const urls = job.objects.map(savedObject => getSavedObjectAbsoluteUrl(job, savedObject)); return await executeJob({ ...job, urls }, cancellationToken); }; }; -} \ No newline at end of file +} diff --git a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.test.js b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.test.js index 7552ebc665cba..f60bc0d83e155 100644 --- a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.test.js +++ b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.test.js @@ -54,7 +54,7 @@ test(`it generates the absolute url if a urlHash is provided`, async () => { expect(mockCreateJob.mock.calls[0][0].urls[0]).toBe('http://localhost:5601/app/kibana#visualize'); }); -test(`it generates the absolute url if a relativeUrl is provided`, async () => { +test(`it generates the absolute url using server's basePath if a relativeUrl is provided`, async () => { const mockCreateJob = jest.fn(); const compatibilityShim = compatibilityShimFactory(createMockServer()); @@ -64,7 +64,17 @@ test(`it generates the absolute url if a relativeUrl is provided`, async () => { expect(mockCreateJob.mock.calls[0][0].urls[0]).toBe('http://localhost:5601/app/kibana#/visualize?'); }); -test(`it generates the absolute url if a relativeUrl with querystring is provided`, async () => { +test(`it generates the absolute url using job's basePath if a relativeUrl is provided`, async () => { + const mockCreateJob = jest.fn(); + const compatibilityShim = compatibilityShimFactory(createMockServer()); + + const relativeUrl = '/app/kibana#/visualize?'; + await compatibilityShim(mockCreateJob)({ basePath: '/s/marketing', objects: [ { relativeUrl } ] }); + expect(mockCreateJob.mock.calls.length).toBe(1); + expect(mockCreateJob.mock.calls[0][0].urls[0]).toBe('http://localhost:5601/s/marketing/app/kibana#/visualize?'); +}); + +test(`it generates the absolute url using server's basePath if a relativeUrl with querystring is provided`, async () => { const mockCreateJob = jest.fn(); const compatibilityShim = compatibilityShimFactory(createMockServer()); @@ -74,6 +84,16 @@ test(`it generates the absolute url if a relativeUrl with querystring is provide expect(mockCreateJob.mock.calls[0][0].urls[0]).toBe('http://localhost:5601/app/kibana?_t=123456789#/visualize?_g=()'); }); +test(`it generates the absolute url using job's basePath if a relativeUrl with querystring is provided`, async () => { + const mockCreateJob = jest.fn(); + const compatibilityShim = compatibilityShimFactory(createMockServer()); + + const relativeUrl = '/app/kibana?_t=123456789#/visualize?_g=()'; + await compatibilityShim(mockCreateJob)({ basePath: '/s/marketing', objects: [ { relativeUrl } ] }); + expect(mockCreateJob.mock.calls.length).toBe(1); + expect(mockCreateJob.mock.calls[0][0].urls[0]).toBe('http://localhost:5601/s/marketing/app/kibana?_t=123456789#/visualize?_g=()'); +}); + test(`it passes the provided browserTimezone through`, async () => { const mockCreateJob = jest.fn(); const compatibilityShim = compatibilityShimFactory(createMockServer()); diff --git a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.js b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.js index e2f594eec609f..b224d0835fa94 100644 --- a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.js +++ b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.js @@ -11,6 +11,7 @@ function getAbsoluteUrlFn(server) { const config = server.config(); return function getAbsoluteUrl({ + basePath = config.get('server.basePath'), hash, path = '/app/kibana', search @@ -19,7 +20,7 @@ function getAbsoluteUrlFn(server) { protocol: config.get('xpack.reporting.kibanaServer.protocol') || server.info.protocol, hostname: config.get('xpack.reporting.kibanaServer.hostname') || config.get('server.host'), port: config.get('xpack.reporting.kibanaServer.port') || config.get('server.port'), - pathname: config.get('server.basePath') + path, + pathname: basePath + path, hash: hash, search }); diff --git a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.test.js b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.test.js index 1391d4665cb50..39ca6fd52f51e 100644 --- a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.test.js +++ b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.test.js @@ -92,6 +92,14 @@ test(`uses the provided hash with queryString`, () => { expect(absoluteUrl).toBe(`http://something:8080/tst/app/kibana#${hash}`); }); +test(`uses the provided basePath`, () => { + const mockServer = createMockServer(); + + const getAbsoluteUrl = getAbsoluteUrlFactory(mockServer); + const absoluteUrl = getAbsoluteUrl({ basePath: '/s/marketing' }); + expect(absoluteUrl).toBe(`http://something:8080/s/marketing/app/kibana`); +}); + test(`uses the path`, () => { const mockServer = createMockServer(); @@ -109,3 +117,5 @@ test(`uses the search`, () => { const absoluteUrl = getAbsoluteUrl({ search }); expect(absoluteUrl).toBe(`http://something:8080/tst/app/kibana?${search}`); }); + + diff --git a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.js b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.js index 103e3b4c3293d..72969083d98e3 100644 --- a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.js +++ b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.js @@ -31,6 +31,8 @@ function executeJobFn(server) { const crypto = cryptoFactory(server); const compatibilityShim = compatibilityShimFactory(server); + const serverBasePath = server.config().get('server.basePath'); + const decryptJobHeaders = async (job) => { const decryptedHeaders = await crypto.decrypt(job.headers); return { job, decryptedHeaders }; @@ -44,6 +46,10 @@ function executeJobFn(server) { const getCustomLogo = async ({ job, filteredHeaders }) => { const fakeRequest = { headers: filteredHeaders, + // This is used by the spaces SavedObjectClientWrapper to determine the existing space. + // We use the basePath from the saved job, which we'll have post spaces being implemented; + // or we use the server base path, which uses the default space + getBasePath: () => job.basePath || serverBasePath }; const savedObjects = server.savedObjects; diff --git a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.test.js b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.test.js index 2b4ea67d5895c..10c68f508a736 100644 --- a/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.test.js +++ b/x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.test.js @@ -42,7 +42,7 @@ beforeEach(() => { 'xpack.reporting.kibanaServer.protocol': 'http', 'xpack.reporting.kibanaServer.hostname': 'localhost', 'xpack.reporting.kibanaServer.port': 5601, - 'server.basePath': '' + 'server.basePath': '/sbp' }[key]; }); @@ -106,6 +106,37 @@ test(`omits blacklisted headers`, async () => { expect(generatePdfObservable).toBeCalledWith(undefined, [], undefined, permittedHeaders, undefined, undefined); }); +test('uses basePath from job when creating saved object service', async () => { + const encryptedHeaders = await encryptHeaders({}); + + const logo = 'custom-logo'; + mockServer.uiSettingsServiceFactory().get.mockReturnValue(logo); + + const generatePdfObservable = generatePdfObservableFactory(); + generatePdfObservable.mockReturnValue(Rx.of(Buffer.from(''))); + + const executeJob = executeJobFactory(mockServer); + const jobBasePath = '/sbp/s/marketing'; + await executeJob({ objects: [], headers: encryptedHeaders, basePath: jobBasePath }, cancellationToken); + + expect(mockServer.savedObjects.getScopedSavedObjectsClient.mock.calls[0][0].getBasePath()).toBe(jobBasePath); +}); + +test(`uses basePath from server if job doesn't have a basePath when creating saved object service`, async () => { + const encryptedHeaders = await encryptHeaders({}); + + const logo = 'custom-logo'; + mockServer.uiSettingsServiceFactory().get.mockReturnValue(logo); + + const generatePdfObservable = generatePdfObservableFactory(); + generatePdfObservable.mockReturnValue(Rx.of(Buffer.from(''))); + + const executeJob = executeJobFactory(mockServer); + await executeJob({ objects: [], headers: encryptedHeaders }, cancellationToken); + + expect(mockServer.savedObjects.getScopedSavedObjectsClient.mock.calls[0][0].getBasePath()).toBe('/sbp'); +}); + test(`gets logo from uiSettings`, async () => { const encryptedHeaders = await encryptHeaders({}); @@ -145,9 +176,9 @@ test(`adds forceNow to hash's query, if it exists`, async () => { const executeJob = executeJobFactory(mockServer); const forceNow = '2000-01-01T00:00:00.000Z'; - await executeJob({ objects: [{ relativeUrl: 'app/kibana#/something' }], forceNow, headers: encryptedHeaders }, cancellationToken); + await executeJob({ objects: [{ relativeUrl: '/app/kibana#/something' }], forceNow, headers: encryptedHeaders }, cancellationToken); - expect(generatePdfObservable).toBeCalledWith(undefined, ['http://localhost:5601/app/kibana#/something?forceNow=2000-01-01T00%3A00%3A00.000Z'], undefined, {}, undefined, undefined); + expect(generatePdfObservable).toBeCalledWith(undefined, ['http://localhost:5601/sbp/app/kibana#/something?forceNow=2000-01-01T00%3A00%3A00.000Z'], undefined, {}, undefined, undefined); }); test(`appends forceNow to hash's query, if it exists`, async () => { @@ -160,12 +191,12 @@ test(`appends forceNow to hash's query, if it exists`, async () => { const forceNow = '2000-01-01T00:00:00.000Z'; await executeJob({ - objects: [{ relativeUrl: 'app/kibana#/something?_g=something' }], + objects: [{ relativeUrl: '/app/kibana#/something?_g=something' }], forceNow, headers: encryptedHeaders }, cancellationToken); - expect(generatePdfObservable).toBeCalledWith(undefined, ['http://localhost:5601/app/kibana#/something?_g=something&forceNow=2000-01-01T00%3A00%3A00.000Z'], undefined, {}, undefined, undefined); + expect(generatePdfObservable).toBeCalledWith(undefined, ['http://localhost:5601/sbp/app/kibana#/something?_g=something&forceNow=2000-01-01T00%3A00%3A00.000Z'], undefined, {}, undefined, undefined); }); test(`doesn't append forceNow query to url, if it doesn't exists`, async () => { @@ -176,9 +207,9 @@ test(`doesn't append forceNow query to url, if it doesn't exists`, async () => { const executeJob = executeJobFactory(mockServer); - await executeJob({ objects: [{ relativeUrl: 'app/kibana#/something' }], headers: encryptedHeaders }, cancellationToken); + await executeJob({ objects: [{ relativeUrl: '/app/kibana#/something' }], headers: encryptedHeaders }, cancellationToken); - expect(generatePdfObservable).toBeCalledWith(undefined, ['http://localhost:5601/app/kibana#/something'], undefined, {}, undefined, undefined); + expect(generatePdfObservable).toBeCalledWith(undefined, ['http://localhost:5601/sbp/app/kibana#/something'], undefined, {}, undefined, undefined); }); test(`returns content_type of application/pdf`, async () => {