Skip to content

[Saved Object Management] Address v1 HTTP API TODOs #150892

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

Merged
jloleysens merged 8 commits into from
Feb 16, 2023
Merged

[Saved Object Management] Address v1 HTTP API TODOs #150892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e16a64b
probably not necessary, we are going to recommend teams use the resol…
jloleysens Feb 10, 2023
f919661
minor code refactor, removed usage of core server type from public an…
jloleysens Feb 10, 2023
2720745
optimize usage of types slightly
jloleysens Feb 10, 2023
bf1c2b8
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Feb 10, 2023
f8a8fe6
Merge branch 'main' into address-som-api-versioning-todos
jloleysens Feb 13, 2023
9a876fd
lock down the accepted sort fields
jloleysens Feb 13, 2023
3854b37
Merge branch 'main' into address-som-api-versioning-todos
jloleysens Feb 14, 2023
37beda9
re-added explicit typing
jloleysens Feb 15, 2023
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
18 changes: 13 additions & 5 deletions src/plugins/saved_objects_management/common/types/v1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ export interface SavedObjectInvalidRelation {

export interface SavedObjectManagementTypeInfo {
name: string;
// TODO: Fix. We should not directly expose these values to public code.
namespaceType: SavedObjectsNamespaceType;
hidden: boolean;
displayName: string;
Expand Down Expand Up @@ -118,15 +117,24 @@ export interface FindQueryHTTP {
perPage?: number;
page?: number;
type: string | string[];
// TODO: Fix. this API allows writing an arbitrary query that is passed straight to our persistence layer, thus leaking SO attributes to the public...
/**
* SO's can register one or more "defaultSearchField"'s against which this query will run using the
* {@link SavedObjectsFindOptions['searchFields'] | search fields} option.
*
* Therefore we are not directly exposing SO attributes to the public here.
*/
search?: string;
defaultSearchOperator?: FindSearchOperatorHTTP;
// TODO: Fix. this API allows sorting by any field, thus leaking SO attributes to the public...
sortField?: string;
sortField?: keyof SavedObjectWithMetadata;
sortOrder?: FindSortOrderHTTP;
hasReference?: ReferenceHTTP | ReferenceHTTP[];
hasReferenceOperator?: FindSearchOperatorHTTP;
// TODO: Fix. This exposes attribute schemas to clients.
/**
* It is not clear who might be using this API option, the SOM UI only ever passes in "id" here.
*
* TODO: Determine use. If not in use we should remove this option. If in use we must deprecate and eventually
* remove.
*/
fields?: string | string[];
}

Expand Down
7 changes: 2 additions & 5 deletions src/plugins/saved_objects_management/public/lib/get_saved_object_counts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import type { HttpStart, SavedObjectsFindOptionsReference } from '@kbn/core/public';
import type { HttpStart } from '@kbn/core/public';
import type { v1 } from '../../common';

export async function getSavedObjectCounts({
Expand All @@ -16,10 +16,7 @@ export async function getSavedObjectCounts({
references,
}: {
http: HttpStart;
typesToInclude: string[];
searchString?: string;
references?: SavedObjectsFindOptionsReference[];
}): Promise<v1.ScrollCountResponseHTTP> {
} & v1.ScrollCountBodyHTTP): Promise<v1.ScrollCountResponseHTTP> {
return await http.post(`/api/kibana/management/saved_objects/scroll/counts`, {
body: JSON.stringify({ typesToInclude, searchString, references }),
});
Expand Down
12 changes: 5 additions & 7 deletions .../saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@ import { saveAs } from '@elastic/filesaver';
import { EuiSpacer, Query, CriteriaWithPagination } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { HttpStart, OverlayStart, NotificationsStart, ApplicationStart } from '@kbn/core/public';
import type { SavedObjectsFindOptions } from '@kbn/core-saved-objects-api-server';
import { RedirectAppLinks } from '@kbn/kibana-react-plugin/public';
import { SavedObjectsTaggingApi } from '@kbn/saved-objects-tagging-oss-plugin/public';
import { DataViewsContract } from '@kbn/data-views-plugin/public';
import { DataPublicPluginStart } from '@kbn/data-plugin/public';
import { CustomBrandingStart } from '@kbn/core-custom-branding-browser';
import { Subscription } from 'rxjs';
import type { SavedObjectManagementTypeInfo } from '../../../common/types';
import type { SavedObjectManagementTypeInfo, FindQueryHTTP } from '../../../common/types/latest';
import {
parseQuery,
getSavedObjectCounts,
Expand Down Expand Up @@ -233,17 +232,16 @@ export class SavedObjectsTable extends Component<SavedObjectsTableProps, SavedOb

// "searchFields" is missing from the "findOptions" but gets injected via the API.
// The API extracts the fields from each uiExports.savedObjectsManagement "defaultSearchField" attribute
const findOptions: SavedObjectsFindOptions = {
const findOptions: FindQueryHTTP = {
search: queryText ? `${queryText}*` : undefined,
perPage,
page: page + 1,
fields: ['id'],
type: searchTypes,
sortField: sort?.field,
sortOrder: sort?.direction,
hasReference: getTagFindReferences({ selectedTags, taggingApi }),
};
findOptions.sortField = sort?.field;
findOptions.sortOrder = sort?.direction;

findOptions.hasReference = getTagFindReferences({ selectedTags, taggingApi });

try {
const resp = await findObjects(http, findOptions);
Expand Down
9 changes: 7 additions & 2 deletions src/plugins/saved_objects_management/server/routes/find.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { schema } from '@kbn/config-schema';
import { type Type, schema } from '@kbn/config-schema';
import type { IRouter } from '@kbn/core/server';

import type { v1 } from '../../common';
Expand All @@ -24,6 +24,11 @@ export const registerFindRoute = (
const searchOperatorSchema = schema.oneOf([schema.literal('OR'), schema.literal('AND')], {
defaultValue: 'OR',
});
const sortFieldSchema: Type<keyof v1.SavedObjectWithMetadata> = schema.oneOf([
schema.literal('created_at'),
schema.literal('updated_at'),
schema.literal('type'),
]);
Comment on lines +27 to +31
Copy link
Copy Markdown
Contributor Author

@jloleysens jloleysens Feb 13, 2023

Choose a reason for hiding this comment

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

This is probably the riskiest change to introduce here. The sort columns we define for the front end are pretty much fixed to these values but it is possible for columns to be registered dynamically and attempt to sort by a nested attribute.* field. This seems very unlikely and it does not look like this feature is being used by any other Kibana code.

Copy link
Copy Markdown
Contributor

@TinaHeiligers TinaHeiligers Feb 15, 2023

Choose a reason for hiding this comment

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

Seems reasonable to me. In fact, I like the approach that we have pre-defined options for sorting. We do, however, have to consider the case where a type may not have these fields, since we don't have a default option to sort on here.

What do you think about defaulting to type if the other fields are missing? It's unlikely they can be though, so probably not needed.


router.get(
{
Expand All @@ -35,7 +40,7 @@ export const registerFindRoute = (
type: schema.oneOf([schema.string(), schema.arrayOf(schema.string())]),
search: schema.maybe(schema.string()),
defaultSearchOperator: searchOperatorSchema,
sortField: schema.maybe(schema.string()),
sortField: schema.maybe(sortFieldSchema),
sortOrder: schema.maybe(schema.oneOf([schema.literal('asc'), schema.literal('desc')])),
hasReference: schema.maybe(
schema.oneOf([referenceSchema, schema.arrayOf(referenceSchema)])
Expand Down
1 change: 0 additions & 1 deletion src/plugins/saved_objects_management/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
"@kbn/utility-types",
"@kbn/i18n-react",
"@kbn/test-jest-helpers",
"@kbn/core-saved-objects-api-server",
"@kbn/monaco",
"@kbn/config-schema",
"@kbn/core-custom-branding-browser-mocks",
Expand Down