add post body fields too

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
2022-12-21 15:10:31 +01:00
parent 7307d4db39
commit e23f13a573
9 changed files with 79 additions and 47 deletions
@@ -0,0 +1,6 @@
+---
+'@backstage/catalog-client': patch
+'@backstage/plugin-catalog-backend': patch
+---
+
+Enable the `by-refs` endpoint to receive `fields` through the POST body as well as through query parameters.
@@ -256,20 +256,20 @@ The request body is JSON, on the form

 ```json
 {
-  "entityRefs": ["component:default/foo", "api:default/bar"]
+  "entityRefs": ["component:default/foo", "api:default/bar"],
+  "fields": ["kind", "metadata.name"]
 }
 ```

-where each entry is an entity ref that you want to fetch.
+where each `entityRefs` entry is an entity ref that you want to fetch. The
+`fields` array is optional and works the same way as the `GET /entities` fields
+above, e.g. it's used to fetch only certain slices of each entity.

 The return type is JSON, on the form

 ```json
 {
-  "items": [
-    { "apiVersion": "backstage.io/v1alpha1", "kind": "Component", ... },
-    null
-  ]
+  "items": [{ "kind": "Component", "metadata": { "name": "foo" } }, null]
 }
 ```

@@ -277,11 +277,6 @@ where the `items` array has _the same length_ and _the same order_ as the input
 `entityRefs` array. Each element contains the corresponding entity data, or
 `null` if no entity existed in the catalog with that ref.

-You can also add `fields` query parameters in the exact same way as `GET
-/entities` above, to fetch only certain slices of each entity. At this point you
-can only specify these as query parameters on the URL, not in the request body.
-Providing them in the body may be added in the future.
-
 ## Locations

 TODO
@@ -232,9 +232,9 @@ describe('CatalogClient', () => {
      };
      server.use(
        rest.post(`${mockBaseUrl}/entities/by-refs`, async (req, res, ctx) => {
-          expect(req.url.searchParams.get('fields')).toBe('a,b');
          await expect(req.json()).resolves.toEqual({
            entityRefs: ['k:n/a', 'k:n/b'],
+            fields: ['a', 'b'],
          });
          return res(ctx.json({ items: [entity, null] }));
        }),
@@ -197,14 +197,13 @@ export class CatalogClient implements CatalogApi {
    request: GetEntitiesByRefsRequest,
    options?: CatalogRequestOptions,
  ): Promise<GetEntitiesByRefsResponse> {
-    const params: string[] = [];
+    const body: any = { entityRefs: request.entityRefs };
    if (request.fields?.length) {
-      params.push(`fields=${request.fields.map(encodeURIComponent).join(',')}`);
+      body.fields = request.fields;
    }

    const baseUrl = await this.discoveryApi.getBaseUrl('catalog');
-    const query = params.length ? `?${params.join('&')}` : '';
-    const url = `${baseUrl}/entities/by-refs${query}`;
+    const url = `${baseUrl}/entities/by-refs`;

    const response = await this.fetchApi.fetch(url, {
      headers: {
@@ -212,7 +211,7 @@ export class CatalogClient implements CatalogApi {
        ...(options?.token && { Authorization: `Bearer ${options?.token}` }),
      },
      method: 'POST',
-      body: JSON.stringify({ entityRefs: request.entityRefs }),
+      body: JSON.stringify(body),
    });

    if (!response.ok) {
@@ -268,6 +268,8 @@ describe('createRouter readonly disabled', () => {
      '{"unknown":7}',
      '{"entityRefs":7}',
      '{"entityRefs":[7]}',
+      '{"entityRefs":[7],"fields":7}',
+      '{"entityRefs":[7],"fields":[7]}',
    ])('properly rejects malformed request body, %p', async p => {
      await expect(
        request(app)
@@ -283,8 +285,12 @@ describe('createRouter readonly disabled', () => {
      const response = await request(app)
        .post('/entities/by-refs')
        .set('Content-Type', 'application/json')
-        .send('{"entityRefs":["a"]}');
+        .send('{"entityRefs":["a"],"fields":["b"]}');
      expect(entitiesCatalog.entitiesBatch).toHaveBeenCalledTimes(1);
+      expect(entitiesCatalog.entitiesBatch).toHaveBeenCalledWith({
+        entityRefs: ['a'],
+        fields: expect.any(Function),
+      });
      expect(response.status).toEqual(200);
      expect(response.body).toEqual({ items: [entity] });
    });
@@ -181,7 +181,7 @@ export async function createRouter(
        const token = getBearerToken(req.header('authorization'));
        const response = await entitiesCatalog.entitiesBatch({
          entityRefs: request.entityRefs,
-          fields: parseEntityTransformParams(req.query),
+          fields: parseEntityTransformParams(req.query, request.fields),
          authorizationToken: token,
        });
        res.status(200).json(response);
@@ -20,9 +20,10 @@ import { z } from 'zod';

 const schema = z.object({
  entityRefs: z.array(z.string()),
+  fields: z.array(z.string()).optional(),
 });

-export function entitiesBatchRequest(req: Request) {
+export function entitiesBatchRequest(req: Request): z.infer<typeof schema> {
  try {
    return schema.parse(req.body);
  } catch (error) {
@@ -18,22 +18,26 @@ import { Entity } from '@backstage/catalog-model';
 import { parseEntityTransformParams } from './parseEntityTransformParams';

 describe('parseEntityTransformParams', () => {
-  const entity: Entity = {
-    apiVersion: 'av',
-    kind: 'k',
-    metadata: {
-      name: 'n',
-      tags: ['t1', 't2'],
-      annotations: {
-        'example.test/url-like-key': 'ul1',
-        'example.com/other-url-like-key': 'ul2',
-        'other-example.test/next-url-like-key': 'ul3',
+  let entity: Entity;
+
+  beforeEach(() => {
+    entity = {
+      apiVersion: 'av',
+      kind: 'k',
+      metadata: {
+        name: 'n',
+        tags: ['t1', 't2'],
+        annotations: {
+          'example.test/url-like-key': 'ul1',
+          'example.com/other-url-like-key': 'ul2',
+          'other-example.test/next-url-like-key': 'ul3',
+        },
      },
-    },
-    spec: {
-      type: 't',
-    },
-  };
+      spec: {
+        type: 't',
+      },
+    };
+  });

  it('returns undefined when no fields given', () => {
    expect(parseEntityTransformParams({})).toBeUndefined();
@@ -46,7 +50,9 @@ describe('parseEntityTransformParams', () => {
  it('rejects attempts at array filtering', () => {
    expect(() =>
      parseEntityTransformParams({ fields: 'metadata.tags[0]' })!(entity),
-    ).toThrow(/invalid fields, array type fields are not supported/i);
+    ).toThrow(
+      'Invalid field "metadata.tags[0]", array type fields are not supported',
+    );
  });

  it('accepts both strings and arrays of strings as input', () => {
@@ -177,4 +183,18 @@ describe('parseEntityTransformParams', () => {
      ),
    ).toEqual({ kind: 'k' });
  });
+
+  it('handles both query params and extras, dealing with overlaps', () => {
+    expect(
+      parseEntityTransformParams({ fields: 'kind' }, [
+        'kind',
+        'metadata.name',
+      ])!(entity),
+    ).toEqual({
+      kind: 'k',
+      metadata: {
+        name: 'n',
+      },
+    });
+  });
 });
@@ -38,24 +38,29 @@ function getPathArrayAndValue(input: Entity, field: string) {

 export function parseEntityTransformParams(
  params: Record<string, unknown>,
+  extra?: string[],
 ): ((entity: Entity) => Entity) | undefined {
-  const fieldsStrings = parseStringsParam(params.fields, 'fields');
-  if (!fieldsStrings) {
-    return undefined;
-  }
+  const queryFields = parseStringsParam(params.fields, 'fields');

-  const fields = fieldsStrings
-    .map(s => s.split(','))
-    .flat()
-    .map(s => s.trim())
-    .filter(Boolean);
+  const fields = Array.from(
+    new Set(
+      [...(extra ?? []), ...(queryFields ?? [])]
+        .map(s => s.split(','))
+        .flat()
+        .map(s => s.trim())
+        .filter(Boolean),
+    ),
+  );

  if (!fields.length) {
    return undefined;
  }

-  if (fields.some(f => f.includes('['))) {
-    throw new InputError('invalid fields, array type fields are not supported');
+  const arrayTypeField = fields.find(f => f.includes('['));
+  if (arrayTypeField) {
+    throw new InputError(
+      `Invalid field "${arrayTypeField}", array type fields are not supported`,
+    );
  }

  return input => {