Updated parseEntityTransformParams to handle keys with '.' in them

[webark]

Signed-off-by: Mark David Avery mark@webark.cc

Hey, I just made a Pull Request!

When writing queries against the entity list, this will enable using keys with . in them. This includes annotations such as backstage.io/source-location and the like.

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)
• All your commits have a Signed-off-by line in the message. (more info)

[github-actions]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/plugin-catalog-backend	plugins/catalog-backend	patch	v1.1.2-next.1

[webark]

Applicable thread.

https://discord.com/channels/687207715902193673/968415863487225866

[webark]

edited: tweaked some of the implementations from my original posting (tweaked it again to be a reduce)

@freben so the two implementations that I settled on where

function getPathArrayAndValue(input: Entity, field: string) {
  return field.split('.').reduce(
    ([pathArray, inputSubset], pathPart, index, fieldParts) => {
      if (Object.hasOwn(inputSubset, pathPart)) {
        return [pathArray.concat(pathPart), inputSubset[pathPart]];
      } else if (fieldParts[index + 1] !== undefined) {
        fieldParts[index + 1] = `${pathPart}.${fieldParts[index + 1]}`;
        return [pathArray, inputSubset];
      }

      return [pathArray, undefined];
    },
    [[] as string[], input as any],
  );
}

and

function getPathArray(input: Entity, field: string) {
  return field.split('.').reduce((pathArray, pathPart, index, fieldParts) => {
    if (lodash.has(input, pathArray.concat(pathPart))) {
      pathArray.push(pathPart);
    } else if (fieldParts[index + 1] !== undefined) {
      fieldParts[index + 1] = `${pathPart}.${fieldParts[index + 1]}`;
    }

    return pathArray;
  }, [] as string[]);
}

My one concern is that we arent using lodash to retrieve or check (with _.has) the key, but we will be using it to set it later on, and if that could be an area for odd bugs to arise.

I'm open to your call and feedback @freben . thanks for taking the time! 😊

[webark]

I ended up just pushing the getPathArrayAndValue option since that was what you were suggesting. If we want to move back to the lodash.has approach, I'm fine with that and can update it as such. Thanks!

[webark]

Oh, hasOwn was introduced in 16, and we're still supporting 14. Makes sense. I'll update thatt.

[github-actions]

Thanks for the contribution!
All commits need to be DCO signed before merging. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

[webark]

OK, I fixed that commit that wasn't signed.

Let me know if you want me to squash the commits, or if that's something that you will do when you merge/rebase in the PR.

[freben]

I don't think you can do this in a reduce easily - before even going into the if statements you need to check basically that typeof inputSubset === 'object' or similar, and if it isn't, bail out early and return undefined - because there is no point in trying to descend into a null or a number or so. So this should probably be a for..of loop instead over the parts, or similar

[freben]

You could write a test that tries to fetch spec.type, and then a test that tries to fetch spec.type.doesnotexist

[webark]

Ohh sure. I was using a "hasOwn" before, but that was not supported in node 14. 😞

I'll add that test.

I had a for of loop originally, but it felt to cumbersome. I'll switch it back if that's preferred.

[webark]

@freben so

You could write a test that tries to fetch spec.type, and then a test that tries to fetch spec.type.doesnotexist

hasOwnProperty exists on all javascript types except null and undefined. So I added some tests before I thought of the null and undefined scenarios, and will update to handle those later.

[webark]

Ok. I switched it to loash's hasIn which seems to be a polyfill for 'hasOwn`. I like this, cause it serves the need of using lodah to check the key, as well as set the key, so there's less likely to be an odd bug between the two.

I kept it as a reduce for now. I could go back to a for of, that was a little like

function getPathArray(input: Entity, field: string) {
  const pathArray = [];
  let currentPathPart = '';

  for (const pathPart of field.split('.')) {
    currentPathPart += pathPart;

    if (lodash.has(input, pathArray.concat(currentPathPart))) {
      pathArray.push(currentPathPart);
      currentPathPart = '';
    } else {
      currentPathPart += '.';
    }
  }

  return pathArray;
}

Let me know if you want to be to go back to that approach, but with stepping through the object each cycle to get the value.

[suuus]

Thank you for contributing during Hacktoberfest 🍂! You can claim the Backstage Hacktoberfest Holopin at https://bck.st/hacktoberfest-holopin 🙌🏻