Apparently wrong type of CopyOptions.metadata

[anantakrishna]

It seems that the type of CopyOptions.metadata is wrong.

Prior to #2234 (landed in f48dcd2), CopyOptions.metadata was of type Metadata=any: https://github.com/googleapis/nodejs-storage/pull/2234/files#diff-bc9705d0f7a567399044dfc66ccc82d4d9aa1cff116842a0094d54e463c9ecbcL313, https://github.com/googleapis/nodejs-storage/pull/2234/files#diff-dd08f09f3839e8051415914bfa81750cc0f28ae2d9b1d6b95eb87bb0efff2577L48. This fact indicates that this property refers to the custom metadata. Moreover, cacheControl, contentType and other known metadata properties are declared directly on the CopyOptions interface.

After that PR was merged, CopyOptions.metadata got type FileMetadata, which itself contains cacheControl, contentType, as well as nested metadata, which is essentially a Record<string, string> and means “custom metadata”.

It seems to me that CopyOptions.metadata should be of type Record<string, string> directly. This is confirmed by the code below.

Steps to reproduce

file.copy(to,
  {
    contentType: 'outer', // A property from CopyOptions
    metadata: { // Typed as FileMetadata
      contentType: 'internal',
      custom1: 'outer', // Not a part of FileMetadata, but allowed by TypeScript
      metadata: { // Intended to hold custom metadata, but ignored
        custom2: 'internal'
      }
    },
  }
);

The copied file gets the following metadata:

• Content-Type: outer
• Custom metadata:
  • contentType: inner - note how a property of FileMetadata.contentType it became a custom metadata.
  • custom1: outer

Whereas custom2 is ignored.

---

Environment details

• OS: any
• Node.js version: any
• npm version: any
• @google-cloud/storage version: 7.7.0

[ddelgrosso1]

Thanks @anantakrishna we will take a look and see about getting it fixed up.

[anantakrishna]

Thanks. Just in case, here are related changes: #1406, #1426.

[vishwarajanand]

There are a couple of issues here:

1. I am not sure whether CopyOptions.metadata tyope should be reverted, I raised a PR but will await @ddelgrosso1 to reassert. I do think it should be changed though for neatness.

2. For the repro provided, I can validate that the nested metadata is indeed sent on request stream, but the backend seems to be ignoring (because it's nested?). There are a few approaches for end users here, but the library should not be doing either of these:

   • Json encode the nested metadata into first level of custom metadata.
   • Flatten all nested metadata down to first level

[ddelgrosso1]

1. We discussed offline but putting here for visibility as well. Yes, CopyOptions.metadata should be reverted. It should not be of type FileMetadata. CopyOptions was meant to represent editable metadata and I missed this during my previous changes.

2. This point should become moot if we change it back to the correct typing.