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

[Reporting] Clean up types for internal APIs needed for UI #105508

Merged
merged 13 commits into from
Jul 15, 2021
Merged
65 changes: 25 additions & 40 deletions x-pack/plugins/reporting/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ export interface TaskRunResult {

export interface ReportSource {
jobtype: string;
kibana_name: string;
kibana_id: string;
created_by: string | false;
payload: {
headers: string; // encrypted headers
Expand All @@ -68,17 +66,20 @@ export interface ReportSource {
isDeprecated?: boolean;
};
meta: { objectType: string; layout?: string };
browser_type: string;
migration_version: string;
max_attempts: number;
timeout: number;

status: JobStatus;
attempts: number;
output: TaskRunResult | null;
tsullivan marked this conversation as resolved.
Show resolved Hide resolved
created_at: string;

// fields with undefined values exist in report jobs that have not been claimed
kibana_name?: string;
kibana_id?: string;
browser_type?: string;
timeout?: number;
max_attempts?: number;
started_at?: string;
completed_at?: string;
created_at: string;
process_expiration?: string | null; // must be set to null to clear the expiration
}

Expand Down Expand Up @@ -108,37 +109,20 @@ export interface JobContent {
content: string;
}

export interface ReportApiJSON {
/* Info API response: report query results do not need to include the
* payload.headers or output.content */
tsullivan marked this conversation as resolved.
Show resolved Hide resolved
type ReportSimple = Omit<ReportSource, 'payload' | 'output'> & {
payload: Omit<ReportSource['payload'], 'headers'>;
} & {
output?: Omit<Partial<TaskRunResult>, 'content'>; // is undefined for report jobs that are not completed
};
tsullivan marked this conversation as resolved.
Show resolved Hide resolved

/*
* The response format for all of the report job APIs
*/
export interface ReportApiJSON extends ReportSimple {
id: string;
index: string;
kibana_name: string;
kibana_id: string;
browser_type: string | undefined;
created_at: string;
jobtype: string;
created_by: string | false;
timeout?: number;
output?: {
content_type: string;
size: number;
warnings?: string[];
};
process_expiration?: string;
completed_at: string | undefined;
payload: {
layout?: LayoutParams;
title: string;
browserTimezone?: string;
isDeprecated?: boolean;
};
meta: {
layout?: string;
objectType: string;
};
max_attempts: number;
started_at: string | undefined;
attempts: number;
status: string;
Copy link
Member Author

Choose a reason for hiding this comment

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

This was all nearly duplicated from ReportSource

}

export interface LicenseCheckResults {
Expand All @@ -147,13 +131,14 @@ export interface LicenseCheckResults {
message: string;
}

/* Notifier Toasts */
export interface JobSummary {
id: JobId;
status: JobStatus;
title: string;
jobtype: string;
maxSizeReached?: boolean;
csvContainsFormulas?: boolean;
jobtype: ReportSource['jobtype'];
title: ReportSource['payload']['title'];
maxSizeReached: TaskRunResult['max_size_reached'];
csvContainsFormulas: TaskRunResult['csv_contains_formulas'];
}

export interface JobSummarySet {
Expand Down
74 changes: 74 additions & 0 deletions x-pack/plugins/reporting/public/lib/job.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { JobId, ReportApiJSON, ReportSource, TaskRunResult } from '../../common/types';

type ReportPayload = ReportSource['payload'];

/*
* This class represents a report job for the UI
* It can be instantiated with ReportApiJSON: the response data format for the report job APIs
*/
export class Job {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using a class here? I think we could accomplish the same thing with just an interface and a function that maps one object to another (if needed). Unless we intend to associate behaviours with this in way that is in keeping with how we use classes elsewhere in Reporting.

Copy link
Member Author

@tsullivan tsullivan Jul 14, 2021

Choose a reason for hiding this comment

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

Unless we intend to associate behaviours with this in way that is in keeping with how we use classes elsewhere in Reporting.

Yes, this is the case. In an earlier incarnation of the code, there are methods on this class for achieving consistency about warnings [1], and then further on, showing some info about deprecated export types. That got to be a lot of changes in the UI that just needed to be isolated to a separate PR.

[1] Why is the only warning shown in a table cell about when the max size was reached? Why don't we use the table to show that a CSV could contain formulas, but we do show it in the toast notification? Why doesn't the info side panel show "status" the same way it's shown in the table?

public id: JobId;
public index: string;

public objectType: ReportPayload['objectType'];
public title: ReportPayload['title'];
public isDeprecated: ReportPayload['isDeprecated'];
public browserTimezone?: ReportPayload['browserTimezone'];
public layout: ReportPayload['layout'];

public jobtype: ReportSource['jobtype'];
public created_by: ReportSource['created_by'];
public created_at: ReportSource['created_at'];
public started_at: ReportSource['started_at'];
public completed_at: ReportSource['completed_at'];
public status: ReportSource['status'];
public attempts: ReportSource['attempts'];
public max_attempts: ReportSource['max_attempts'];

public timeout: ReportSource['timeout'];
public kibana_name: ReportSource['kibana_name'];
public kibana_id: ReportSource['kibana_id'];
public browser_type: ReportSource['browser_type'];

public size?: TaskRunResult['size'];
public content_type?: TaskRunResult['content_type'];
public csv_contains_formulas?: TaskRunResult['csv_contains_formulas'];
public max_size_reached?: TaskRunResult['max_size_reached'];
public warnings?: TaskRunResult['warnings'];

constructor(report: ReportApiJSON) {
this.id = report.id;
this.index = report.index;

this.jobtype = report.jobtype;
this.objectType = report.payload.objectType;
this.title = report.payload.title;
this.layout = report.payload.layout;
this.created_by = report.created_by;
this.created_at = report.created_at;
this.started_at = report.started_at;
this.completed_at = report.completed_at;
this.status = report.status;
this.attempts = report.attempts;
this.max_attempts = report.max_attempts;

this.timeout = report.timeout;
this.kibana_name = report.kibana_name;
this.kibana_id = report.kibana_id;
this.browser_type = report.browser_type;
this.browserTimezone = report.payload.browserTimezone;
this.size = report.output?.size;

this.isDeprecated = report.payload.isDeprecated || false;
this.csv_contains_formulas = report.output?.csv_contains_formulas;
this.max_size_reached = report.output?.max_size_reached;
this.warnings = report.output?.warnings;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,45 @@ import {
API_MIGRATE_ILM_POLICY_URL,
REPORTING_MANAGEMENT_HOME,
} from '../../../common/constants';
import {
DownloadReportFn,
JobId,
ManagementLinkFn,
ReportApiJSON,
ReportDocument,
ReportSource,
} from '../../../common/types';
import { DownloadReportFn, JobId, ManagementLinkFn, ReportApiJSON } from '../../../common/types';
import { add } from '../../notifier/job_completion_notifications';

export interface JobQueueEntry {
_id: string;
_source: ReportSource;
}
import { Job } from '../job';

export interface JobContent {
content: string;
content_type: boolean;
}

interface JobParams {
[paramName: string]: any;
}

export interface DiagnoseResponse {
help: string[];
success: boolean;
logs: string;
}

export class ReportingAPIClient {
interface JobParams {
[paramName: string]: any;
}

interface IReportingAPI {
getReportURL(jobId: string): string;
downloadReport(jobId: string): void;
deleteReport(jobId: string): Promise<void>;
list(page: number, jobIds: string[]): Promise<Job[]>;
total(): Promise<number>;
getError(jobId: string): Promise<JobContent>;
getInfo(jobId: string): Promise<Job>;
findForJobIds(jobIds: string[]): Promise<Job[]>;
getReportingJobPath(exportType: string, jobParams: JobParams): string;
createReportingJob(exportType: string, jobParams: any): Promise<Job>;
getServerBasePath(): string;
verifyConfig(): Promise<DiagnoseResponse>;
verifyBrowser(): Promise<DiagnoseResponse>;
verifyScreenCapture(): Promise<DiagnoseResponse>;
getManagementLink: ManagementLinkFn;
getDownloadLink: DownloadReportFn;
}

export class ReportingAPIClient implements IReportingAPI {
private http: HttpSetup;

constructor(http: HttpSetup) {
Expand All @@ -71,68 +79,75 @@ export class ReportingAPIClient {
});
}

public list = (page = 0, jobIds: string[] = []): Promise<JobQueueEntry[]> => {
public async list(page = 0, jobIds: string[] = []) {
const query = { page } as any;
if (jobIds.length > 0) {
// Only getting the first 10, to prevent URL overflows
query.ids = jobIds.slice(0, 10).join(',');
}

return this.http.get(`${API_LIST_URL}/list`, {
const jobQueueEntries: ReportApiJSON[] = await this.http.get(`${API_LIST_URL}/list`, {
query,
asSystemRequest: true,
});
};

public total(): Promise<number> {
return this.http.get(`${API_LIST_URL}/count`, {
return jobQueueEntries.map((report) => new Job(report));
}

public async total() {
return await this.http.get(`${API_LIST_URL}/count`, {
asSystemRequest: true,
});
}

public getContent(jobId: string): Promise<JobContent> {
return this.http.get(`${API_LIST_URL}/output/${jobId}`, {
public async getError(jobId: string) {
return await this.http.get(`${API_LIST_URL}/output/${jobId}`, {
asSystemRequest: true,
});
}

public getInfo(jobId: string): Promise<ReportApiJSON> {
return this.http.get(`${API_LIST_URL}/info/${jobId}`, {
public async getInfo(jobId: string) {
const report: ReportApiJSON = await this.http.get(`${API_LIST_URL}/info/${jobId}`, {
asSystemRequest: true,
});
return new Job(report);
}

public findForJobIds = (jobIds: JobId[]): Promise<ReportDocument[]> => {
return this.http.fetch(`${API_LIST_URL}/list`, {
public async findForJobIds(jobIds: JobId[]) {
const reports: ReportApiJSON[] = await this.http.fetch(`${API_LIST_URL}/list`, {
query: { page: 0, ids: jobIds.join(',') },
method: 'GET',
});
};
return reports.map((report) => new Job(report));
}

/*
* Return a URL to queue a job, with the job params encoded in the query string of the URL. Used for copying POST URL
*/
tsullivan marked this conversation as resolved.
Show resolved Hide resolved
public getReportingJobPath = (exportType: string, jobParams: JobParams) => {
public getReportingJobPath(exportType: string, jobParams: JobParams) {
const params = stringify({ jobParams: rison.encode(jobParams) });
return `${this.http.basePath.prepend(API_BASE_GENERATE)}/${exportType}?${params}`;
};
}

/*
* Sends a request to queue a job, with the job params in the POST body
*/
public createReportingJob = async (exportType: string, jobParams: any) => {
public async createReportingJob(exportType: string, jobParams: any) {
const jobParamsRison = rison.encode(jobParams);
const resp = await this.http.post(`${API_BASE_GENERATE}/${exportType}`, {
method: 'POST',
body: JSON.stringify({
jobParams: jobParamsRison,
}),
});
const resp: { job: ReportApiJSON } = await this.http.post(
`${API_BASE_GENERATE}/${exportType}`,
{
method: 'POST',
body: JSON.stringify({
jobParams: jobParamsRison,
}),
}
);

add(resp.job.id);

return resp;
};
return new Job(resp.job);
}

public getManagementLink: ManagementLinkFn = () =>
this.http.basePath.prepend(REPORTING_MANAGEMENT_HOME);
Expand All @@ -148,28 +163,31 @@ export class ReportingAPIClient {
/*
* Diagnostic-related API calls
*/
public verifyConfig = (): Promise<DiagnoseResponse> =>
this.http.post(`${API_BASE_URL}/diagnose/config`, {
public async verifyConfig() {
return await this.http.post(`${API_BASE_URL}/diagnose/config`, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FWIW: these functions don't do anything with the call result so we don't need async await sugar. I think the return type gaurds have us covered.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's kind of been preached to me to use this way instead of "desugared promises." It makes stack traces more readable, and there are some optimizations in V8 that went into making async and await native. https://v8.dev/blog/fast-async

asSystemRequest: true,
});
}

/*
* Diagnostic-related API calls
*/
public verifyBrowser = (): Promise<DiagnoseResponse> =>
this.http.post(`${API_BASE_URL}/diagnose/browser`, {
public async verifyBrowser() {
return await this.http.post(`${API_BASE_URL}/diagnose/browser`, {
asSystemRequest: true,
});
}

/*
* Diagnostic-related API calls
*/
public verifyScreenCapture = (): Promise<DiagnoseResponse> =>
this.http.post(`${API_BASE_URL}/diagnose/screenshot`, {
public async verifyScreenCapture() {
return await this.http.post(`${API_BASE_URL}/diagnose/screenshot`, {
asSystemRequest: true,
});
}

public migrateReportingIndicesIlmPolicy = (): Promise<void> => {
return this.http.put(`${API_MIGRATE_ILM_POLICY_URL}`);
};
public async migrateReportingIndicesIlmPolicy() {
return await this.http.put(`${API_MIGRATE_ILM_POLICY_URL}`);
}
}
Loading