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

[firebase_storage] Updating storage metadata replaces all existing metadata #2307

Closed
puf opened this issue Apr 6, 2020 · 7 comments
Closed
Labels
impact: customer A bug with low impact (e.g. affecting only a few customers or has a workaround). (P3) plugin: storage type: bug Something isn't working

Comments

@puf
Copy link
Contributor

puf commented Apr 6, 2020

Describe the bug
According to the Firebase documentation you can update storage metadata via the updateMetadata API.

Only the properties specified in the metadata are updated, all others are left unmodified.

The FlutterFire documentation for the same API mimics this by:

Writable metadata properties can be deleted by passing the empty string.

This is not how it works in practice though: calling updateMetadata replaces all metadata with the information provided in the call.

To Reproduce

See https://stackoverflow.com/q/61051148

Steps to reproduce the behavior:

  1. Create a StorageReference to a file that has metadata, for example: a content type.
  2. Call storageReference.updateMetadata(StorageMetadata(customMetadata: {'receiver': 'ID'}));
  3. All existing metadata is removed, and only the custom metadata with a single property remains.

Expected behavior

Above call should leave the existing metadata unmodified.

Additional context

As far as I can see the problem comes from _buildMetadataUploadMap, which doesn't seem to handle missing properties correctly. Calling print(_buildMetadataUploadMap(StorageMetadata(customMetadata: {'receiver': 'ID2'}))) prints:

{cacheControl: null, contentDisposition: null, contentLanguage: null, contentType: null, contentEncoding: null, customMetadata: {receiver: ID2}}

I'm pretty sure this map should only contain customMetadata in this case.

@puf puf added the type: bug Something isn't working label Apr 6, 2020
@puf
Copy link
Contributor Author

puf commented Apr 6, 2020

A quick fix could be:

Map<String, dynamic> _buildMetadataUploadMap(StorageMetadata metadata) {
  var map = <String, dynamic>{
    'cacheControl': metadata.cacheControl,
    'contentDisposition': metadata.contentDisposition,
    'contentLanguage': metadata.contentLanguage,
    'contentType': metadata.contentType,
    'contentEncoding': metadata.contentEncoding,
    'customMetadata': metadata.customMetadata,
  };
  map.removeWhere((key, value) => value == null);
  return map;
}

@TahaTesser
Copy link

Hi @puf
can you please provide your flutter doctor -v and flutter run --verbose?
Also, to better address the issue, would be helpful if you could post a minimal code sample to reproduce the problem
Thank you

@TahaTesser TahaTesser added the blocked: customer-response Waiting for customer response, e.g. more information was requested. label Apr 6, 2020
@puf
Copy link
Contributor Author

puf commented Apr 6, 2020

Hey Taha,

The problem seems to exist in the latest code (linked above). The minimal code is in the steps-to-reproduce above, and in the Stack Overflow issue I linked.

puf

@Ehesp Ehesp added impact: customer A bug with low impact (e.g. affecting only a few customers or has a workaround). (P3) plugin: storage labels Apr 23, 2020
@Ehesp
Copy link
Member

Ehesp commented Apr 23, 2020

Will get this checked out & tested.

@Salakar Salakar removed the blocked: customer-response Waiting for customer response, e.g. more information was requested. label Apr 23, 2020
@greghesp
Copy link
Contributor

@puf

I've tested this on the latest roadmap version and confirm that the updateMetadata method now updates rather than replacing

First Call

Reference ref = FirebaseStorage.instance
  .ref()
   .child('/coding.jpg');

ref.updateMetadata(SettableMetadata(
  customMetadata: <String, String>{
    'message': 'hello world',
  },
));

Other metadata:
message: morning

Second Call

Reference ref = FirebaseStorage.instance
  .ref()
   .child('/coding.jpg');

ref.updateMetadata(SettableMetadata(
  customMetadata: <String, String>{
    'userId': 'ABC123',
  },
));

Other metadata:
message: morning
userId: ABC123

@puf
Copy link
Contributor Author

puf commented Sep 22, 2020

Thanks for the update Greg. Let me know once this makes it into a regular release, so I can update my answer on Stack Overflow.

@Salakar
Copy link
Member

Salakar commented Dec 6, 2020

Took a while to get back to this issue sorry but this has been in stable for over a month now so this is ok to be closed now.

Thanks again for the report @puf 🎉

@Salakar Salakar closed this as completed Dec 6, 2020
@firebase firebase locked and limited conversation to collaborators Jan 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
impact: customer A bug with low impact (e.g. affecting only a few customers or has a workaround). (P3) plugin: storage type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants