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

[Cases] Split attachment types for versioned http apis #155440

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
47 changes: 47 additions & 0 deletions x-pack/plugins/cases/server/common/types/attachments.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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 type { User } from './user';

interface AttachmentCommonPersistedAttributes {
created_at: string;
created_by: User;
owner: string;
pushed_at: string | null;
pushed_by: User | null;
updated_at: string | null;
updated_by: User | null;
}

export interface AttachmentRequestAttributes {
type: string;
alertId?: string | string[];
index?: string | string[];
rule?: {
id: string | null;
name: string | null;
};
comment?: string;
actions?: {
targets: Array<{
hostname: string;
endpointId: string;
}>;
type: string;
};
externalReferenceMetadata?: Record<string, unknown> | null;
externalReferenceAttachmentTypeId?: string;
externalReferenceStorage?: {
type: string;
soType?: string;
};
persistableStateAttachmentState?: Record<string, unknown>;
Copy link
Member

Choose a reason for hiding this comment

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

I think the persistableStateAttachmentTypeId is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, yeah I was getting confused between that and the externalId 👍 I'll add it back.

persistableStateAttachmentTypeId?: string;
}

export type AttachmentPersistedAttributes = AttachmentRequestAttributes &
AttachmentCommonPersistedAttributes;
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/server/common/types/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export interface User {
email: string | null | undefined;
full_name: string | null | undefined;
username: string | null | undefined;
profile_uid?: string | undefined;
profile_uid?: string;
}

export interface UserProfile {
Expand Down
14 changes: 0 additions & 14 deletions x-pack/plugins/cases/server/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import type {
CommentRequest,
CommentRequestActionsType,
CommentRequestAlertType,
CommentRequestExternalReferenceSOType,
CommentRequestUserType,
CommentResponse,
CommentsResponse,
Expand All @@ -45,7 +44,6 @@ import {
CaseStatuses,
CommentType,
ConnectorTypes,
ExternalReferenceStorageType,
ExternalReferenceSORt,
FileAttachmentMetadataRt,
} from '../../common/api';
Expand Down Expand Up @@ -254,18 +252,6 @@ export const isCommentRequestTypeAlert = (
return context.type === CommentType.alert;
};

/**
* A type narrowing function for external reference so attachments.
*/
export const isCommentRequestTypeExternalReferenceSO = (
context: Partial<CommentRequest>
): context is CommentRequestExternalReferenceSOType => {
return (
context.type === CommentType.externalReference &&
context.externalReferenceStorage?.type === ExternalReferenceStorageType.savedObject
);
};

/**
* A type narrowing function for file attachments.
*/
Expand Down
78 changes: 19 additions & 59 deletions x-pack/plugins/cases/server/services/attachments/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,14 @@

import type {
SavedObject,
SavedObjectReference,
SavedObjectsBulkResponse,
SavedObjectsBulkUpdateResponse,
SavedObjectsFindResponse,
SavedObjectsUpdateOptions,
SavedObjectsUpdateResponse,
} from '@kbn/core/server';

import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type {
CommentAttributes as AttachmentAttributes,
CommentAttributesWithoutRefs as AttachmentAttributesWithoutRefs,
CommentPatchAttributes as AttachmentPatchAttributes,
} from '../../../common/api';
import type { CommentAttributes as AttachmentAttributes } from '../../../common/api';
import { CommentType } from '../../../common/api';
import { CASE_COMMENT_SAVED_OBJECT, CASE_SAVED_OBJECT } from '../../../common/constants';
import { buildFilter, combineFilters } from '../../client/utils';
Expand All @@ -32,50 +26,19 @@ import {
injectAttachmentSOAttributesFromRefsForPatch,
} from '../so_references';
import type { SavedObjectFindOptionsKueryNode } from '../../common/types';
import type { IndexRefresh } from '../types';
import type { AttachedToCaseArgs, ServiceContext } from './types';
import type {
AlertsAttachedToCaseArgs,
AttachmentsAttachedToCaseArgs,
BulkCreateAttachments,
BulkUpdateAttachmentArgs,
CountActionsAttachedToCaseArgs,
CreateAttachmentArgs,
DeleteAttachmentArgs,
ServiceContext,
UpdateAttachmentArgs,
} from './types';
import { AttachmentGetter } from './operations/get';

type AlertsAttachedToCaseArgs = AttachedToCaseArgs;

interface AttachmentsAttachedToCaseArgs extends AttachedToCaseArgs {
attachmentType: CommentType;
aggregations: Record<string, estypes.AggregationsAggregationContainer>;
}

interface CountActionsAttachedToCaseArgs extends AttachedToCaseArgs {
aggregations: Record<string, estypes.AggregationsAggregationContainer>;
}

interface DeleteAttachmentArgs extends IndexRefresh {
attachmentIds: string[];
}

interface CreateAttachmentArgs extends IndexRefresh {
attributes: AttachmentAttributes;
references: SavedObjectReference[];
id: string;
}

interface BulkCreateAttachments extends IndexRefresh {
attachments: Array<{
attributes: AttachmentAttributes;
references: SavedObjectReference[];
id: string;
}>;
}

interface UpdateArgs {
attachmentId: string;
updatedAttributes: AttachmentPatchAttributes;
options?: Omit<SavedObjectsUpdateOptions<AttachmentAttributes>, 'upsert'>;
}

export type UpdateAttachmentArgs = UpdateArgs;

interface BulkUpdateAttachmentArgs extends IndexRefresh {
comments: UpdateArgs[];
}
import type { AttachmentPersistedAttributes } from '../../common/types/attachments';

export class AttachmentService {
private readonly _getter: AttachmentGetter;
Expand Down Expand Up @@ -136,10 +99,7 @@ export class AttachmentService {

const combinedFilter = combineFilters([attachmentFilter, filter]);

const response = await this.context.unsecuredSavedObjectsClient.find<
AttachmentAttributes,
Agg
>({
const response = await this.context.unsecuredSavedObjectsClient.find<unknown, Agg>({
type: CASE_COMMENT_SAVED_OBJECT,
hasReference: { type: CASE_SAVED_OBJECT, id: caseId },
page: 1,
Expand Down Expand Up @@ -207,7 +167,7 @@ export class AttachmentService {
);

const attachment =
await this.context.unsecuredSavedObjectsClient.create<AttachmentAttributesWithoutRefs>(
await this.context.unsecuredSavedObjectsClient.create<AttachmentPersistedAttributes>(
Copy link
Member

Choose a reason for hiding this comment

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

What about bulkDelete, update, and bulkUpdate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️ don't know why I missed all that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like bulkDelete doesn't take a type. I'll update the others though.

CASE_COMMENT_SAVED_OBJECT,
extractedAttributes,
{
Expand All @@ -234,7 +194,7 @@ export class AttachmentService {
try {
this.context.log.debug(`Attempting to bulk create attachments`);
const res =
await this.context.unsecuredSavedObjectsClient.bulkCreate<AttachmentAttributesWithoutRefs>(
await this.context.unsecuredSavedObjectsClient.bulkCreate<AttachmentPersistedAttributes>(
attachments.map((attachment) => {
const { attributes: extractedAttributes, references: extractedReferences } =
extractAttachmentSORefsFromAttributes(
Expand Down Expand Up @@ -288,7 +248,7 @@ export class AttachmentService {
const shouldUpdateRefs = extractedReferences.length > 0 || didDeleteOperation;

const res =
await this.context.unsecuredSavedObjectsClient.update<AttachmentAttributesWithoutRefs>(
await this.context.unsecuredSavedObjectsClient.update<AttachmentPersistedAttributes>(
CASE_COMMENT_SAVED_OBJECT,
attachmentId,
extractedAttributes,
Expand Down Expand Up @@ -325,7 +285,7 @@ export class AttachmentService {
);

const res =
await this.context.unsecuredSavedObjectsClient.bulkUpdate<AttachmentAttributesWithoutRefs>(
await this.context.unsecuredSavedObjectsClient.bulkUpdate<AttachmentPersistedAttributes>(
comments.map((c) => {
const {
attributes: extractedAttributes,
Expand Down Expand Up @@ -380,7 +340,7 @@ export class AttachmentService {
try {
this.context.log.debug(`Attempting to find comments`);
const res =
await this.context.unsecuredSavedObjectsClient.find<AttachmentAttributesWithoutRefs>({
await this.context.unsecuredSavedObjectsClient.find<AttachmentPersistedAttributes>({
sortField: defaultSortField,
...options,
type: CASE_COMMENT_SAVED_OBJECT,
Expand Down
56 changes: 27 additions & 29 deletions x-pack/plugins/cases/server/services/attachments/operations/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import type { SavedObject } from '@kbn/core/server';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { FILE_SO_TYPE } from '@kbn/files-plugin/common';
import type { AttachmentPersistedAttributes } from '../../../common/types/attachments';
import {
CASE_COMMENT_SAVED_OBJECT,
CASE_SAVED_OBJECT,
Expand All @@ -19,13 +20,13 @@ import type {
AttachmentTotals,
AttributesTypeAlerts,
CommentAttributes as AttachmentAttributes,
CommentAttributesWithoutRefs as AttachmentAttributesWithoutRefs,
CommentAttributes,
} from '../../../../common/api';
import { CommentType } from '../../../../common/api';
import type {
AttachedToCaseArgs,
AlertIdsAggsResult,
BulkOptionalAttributes,
GetAllAlertsAttachToCaseArgs,
GetAttachmentArgs,
ServiceContext,
} from '../types';
Expand All @@ -37,16 +38,6 @@ import { partitionByCaseAssociation } from '../../../common/partitioning';
import type { AttachmentSavedObject } from '../../../common/types';
import { getCaseReferenceId } from '../../../common/references';

type GetAllAlertsAttachToCaseArgs = AttachedToCaseArgs;

interface AlertIdsAggsResult {
alertIds: {
buckets: Array<{
key: string;
}>;
};
}

export class AttachmentGetter {
constructor(private readonly context: ServiceContext) {}

Expand All @@ -59,7 +50,7 @@ export class AttachmentGetter {
);

const response =
await this.context.unsecuredSavedObjectsClient.bulkGet<AttachmentAttributesWithoutRefs>(
await this.context.unsecuredSavedObjectsClient.bulkGet<AttachmentPersistedAttributes>(
Copy link
Member

Choose a reason for hiding this comment

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

Same here, I think we should add the new type to all SO methods of the AttachmentGetter class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the ones I didn't add it to here are intentionally wanting unknown. Let me know if I missed any though.

attachmentIds.map((id) => ({ id, type: CASE_COMMENT_SAVED_OBJECT }))
);

Expand All @@ -85,7 +76,9 @@ export class AttachmentGetter {
`Attempting to retrieve attachments associated with cases: [${caseIds}]`
);

const finder = this.context.unsecuredSavedObjectsClient.createPointInTimeFinder({
// We are intentionally not adding the type here because we only want to interact with the id and this function
// should not use the attributes
const finder = this.context.unsecuredSavedObjectsClient.createPointInTimeFinder<unknown>({
type: CASE_COMMENT_SAVED_OBJECT,
hasReference: caseIds.map((id) => ({ id, type: CASE_SAVED_OBJECT })),
sortField: 'created_at',
Expand Down Expand Up @@ -133,18 +126,24 @@ export class AttachmentGetter {
const combinedFilter = combineFilters([alertsFilter, filter]);

const finder =
this.context.unsecuredSavedObjectsClient.createPointInTimeFinder<AttributesTypeAlerts>({
type: CASE_COMMENT_SAVED_OBJECT,
hasReference: { type: CASE_SAVED_OBJECT, id: caseId },
sortField: 'created_at',
sortOrder: 'asc',
filter: combinedFilter,
perPage: MAX_DOCS_PER_PAGE,
});
this.context.unsecuredSavedObjectsClient.createPointInTimeFinder<AttachmentPersistedAttributes>(
{
type: CASE_COMMENT_SAVED_OBJECT,
hasReference: { type: CASE_SAVED_OBJECT, id: caseId },
sortField: 'created_at',
sortOrder: 'asc',
filter: combinedFilter,
perPage: MAX_DOCS_PER_PAGE,
}
);

let result: Array<SavedObject<AttributesTypeAlerts>> = [];
for await (const userActionSavedObject of finder.find()) {
result = result.concat(userActionSavedObject.saved_objects);
result = result.concat(
// We need a cast here because to limited attachment type conflicts with the expected result even though they
// should be the same
userActionSavedObject.saved_objects as unknown as Array<SavedObject<AttributesTypeAlerts>>
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 calling this out. Do you see a better way around this? I believe we skip transforming this because alerts don't have fields that need to be migrated (the rule.id should have been but we had that bugfix).

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I cannot think of another way of doing it.

);
}

return result;
Expand Down Expand Up @@ -197,11 +196,10 @@ export class AttachmentGetter {
}: GetAttachmentArgs): Promise<SavedObject<AttachmentAttributes>> {
try {
this.context.log.debug(`Attempting to GET attachment ${attachmentId}`);
const res =
await this.context.unsecuredSavedObjectsClient.get<AttachmentAttributesWithoutRefs>(
CASE_COMMENT_SAVED_OBJECT,
attachmentId
);
const res = await this.context.unsecuredSavedObjectsClient.get<AttachmentPersistedAttributes>(
CASE_COMMENT_SAVED_OBJECT,
attachmentId
);

return injectAttachmentSOAttributesFromRefs(
res,
Expand Down Expand Up @@ -324,7 +322,7 @@ export class AttachmentGetter {
* to retrieve them all.
*/
const finder =
this.context.unsecuredSavedObjectsClient.createPointInTimeFinder<AttachmentAttributesWithoutRefs>(
this.context.unsecuredSavedObjectsClient.createPointInTimeFinder<AttachmentPersistedAttributes>(
{
type: CASE_COMMENT_SAVED_OBJECT,
hasReference: references,
Expand Down
Loading