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.

Sign up for GitHub

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

[Spaces] - Reporting updates #21457

Merged
merged 2 commits into from
Aug 16, 2018
Merged

[Spaces] - Reporting updates #21457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af24c73
add 'request.getBasePath()' to fake requests
legrego Jul 30, 2018
5dbeb3a
Fix reporting tests spaces (#7)
kobelb Jul 31, 2018
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
26 changes: 24 additions & 2 deletions x-pack/plugins/reporting/export_types/csv/server/__tests__/execute_job.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/reporting/export_types/csv/server/create_job.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ function createJobFn(server) {
return {
headers: serializedEncryptedHeaders,
indexPatternSavedObject: indexPatternSavedObject,
basePath: request.getBasePath(),
...jobParams
};
};
Expand Down
15 changes: 14 additions & 1 deletion x-pack/plugins/reporting/export_types/csv/server/execute_job.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 = {}) => {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function createJobFn(server) {
relativeUrls,
browserTimezone,
layout
}, headers) {
}, headers, request) {
const serializedEncryptedHeaders = await crypto.encrypt(headers);

return {
Expand All @@ -28,6 +28,7 @@ function createJobFn(server) {
headers: serializedEncryptedHeaders,
browserTimezone,
layout,
basePath: request.getBasePath(),
forceNow: new Date().toISOString(),
};
});
Expand Down
22 changes: 11 additions & 11 deletions x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, now that I see this... I guess reporting is supposed to work when storeStateInSessionStorage is turned on? I have this bug: #20037 because there are issues. But maybe it only fails when the url is too long, otherwise it can work with the setting. Either way, we have no test coverage for this path. I'd have to test manually to see if the new basePath/spaces stuff works with the setting turned on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reporting should be expanding the hashes client-side before creating the job. The urlHash is an old version of the relativeUrl which doesn't include the /app/kibana prefix that we used to be able to rely upon until Canvas came around and needed to be able to target different "kibana applications".

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);
};
};
}
}
24 changes: 22 additions & 2 deletions ...lugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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());

Expand All @@ -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());
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ function getAbsoluteUrlFn(server) {
const config = server.config();

return function getAbsoluteUrl({
basePath = config.get('server.basePath'),
hash,
path = '/app/kibana',
search
Expand All @@ -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
});
Expand Down
10 changes: 10 additions & 0 deletions .../plugins/reporting/export_types/printable_pdf/server/execute_job/get_absolute_url.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -109,3 +117,5 @@ test(`uses the search`, () => {
const absoluteUrl = getAbsoluteUrl({ search });
expect(absoluteUrl).toBe(`http://something:8080/tst/app/kibana?${search}`);
});


6 changes: 6 additions & 0 deletions x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -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;
Expand Down
45 changes: 38 additions & 7 deletions x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
});

Expand Down Expand Up @@ -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({});

Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

These relativeUrls had to be changed because switching to using 'server.basePath': '/sbp' exacerbated an issue with the existing tests.


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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down