fix(storage): do not set metadata property unless it has a value (#12805

)
firebase · May 22, 2024 · 978a87d · 978a87d
1 parent 7d4c200
commit 978a87d
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 55 deletions.
diff --git a/...droid/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStoragePlugin.java b/...droid/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStoragePlugin.java
@@ -391,13 +391,23 @@ public void referenceListAll(
 
   StorageMetadata getMetaDataFromPigeon(
       GeneratedAndroidFirebaseStorage.PigeonSettableMetadata pigeonSettableMetatdata) {
-    StorageMetadata.Builder androidMetaDataBuilder =
-        new StorageMetadata.Builder()
-            .setCacheControl(pigeonSettableMetatdata.getCacheControl())
-            .setContentDisposition(pigeonSettableMetatdata.getContentDisposition())
-            .setContentEncoding(pigeonSettableMetatdata.getContentEncoding())
-            .setContentLanguage(pigeonSettableMetatdata.getContentLanguage())
-            .setContentType(pigeonSettableMetatdata.getContentType());
+    StorageMetadata.Builder androidMetaDataBuilder = new StorageMetadata.Builder();
+
+    if (pigeonSettableMetatdata.getContentType() != null) {
+      androidMetaDataBuilder.setContentType(pigeonSettableMetatdata.getContentType());
+    }
+    if (pigeonSettableMetatdata.getCacheControl() != null) {
+      androidMetaDataBuilder.setCacheControl(pigeonSettableMetatdata.getCacheControl());
+    }
+    if (pigeonSettableMetatdata.getContentDisposition() != null) {
+      androidMetaDataBuilder.setContentDisposition(pigeonSettableMetatdata.getContentDisposition());
+    }
+    if (pigeonSettableMetatdata.getContentEncoding() != null) {
+      androidMetaDataBuilder.setContentEncoding(pigeonSettableMetatdata.getContentEncoding());
+    }
+    if (pigeonSettableMetatdata.getContentLanguage() != null) {
+      androidMetaDataBuilder.setContentLanguage(pigeonSettableMetatdata.getContentLanguage());
+    }
 
     Map<String, String> pigeonCustomMetadata = pigeonSettableMetatdata.getCustomMetadata();
     if (pigeonCustomMetadata != null) {

diff --git a/packages/firebase_storage/firebase_storage/ios/Classes/FLTFirebaseStoragePlugin.m b/packages/firebase_storage/firebase_storage/ios/Classes/FLTFirebaseStoragePlugin.m
@@ -175,13 +175,28 @@ - (FIRStorage *_Nullable)getFIRStorageFromAppNameFromPigeon:(PigeonStorageFireba
 
 - (FIRStorageMetadata *)getFIRStorageMetadataFromPigeon:(PigeonSettableMetadata *)pigeonMetadata {
   FIRStorageMetadata *metadata = [[FIRStorageMetadata alloc] init];
-  metadata.cacheControl = pigeonMetadata.cacheControl;
-  metadata.contentDisposition = pigeonMetadata.contentDisposition;
-  metadata.contentEncoding = pigeonMetadata.contentEncoding;
-  metadata.contentLanguage = pigeonMetadata.contentLanguage;
-  metadata.contentType = pigeonMetadata.contentType;
 
-  metadata.customMetadata = pigeonMetadata.customMetadata;
+  if (pigeonMetadata.cacheControl != nil) {
+    metadata.cacheControl = pigeonMetadata.cacheControl;
+  }
+  if (pigeonMetadata.contentType != nil) {
+    metadata.contentType = pigeonMetadata.contentType;
+  }
+  if (pigeonMetadata.contentDisposition != nil) {
+    metadata.contentDisposition = pigeonMetadata.contentDisposition;
+  }
+
+  if (pigeonMetadata.contentEncoding != nil) {
+    metadata.contentEncoding = pigeonMetadata.contentEncoding;
+  }
+
+  if (pigeonMetadata.contentLanguage != nil) {
+    metadata.contentLanguage = pigeonMetadata.contentLanguage;
+  }
+
+  if (pigeonMetadata.customMetadata != nil) {
+    metadata.customMetadata = pigeonMetadata.customMetadata;
+  }
 
   return metadata;
 }

diff --git a/packages/firebase_storage/firebase_storage_web/lib/src/interop/storage.dart b/packages/firebase_storage/firebase_storage_web/lib/src/interop/storage.dart
@@ -251,25 +251,38 @@ class UploadMetadata
     extends _UploadMetadataBase<storage_interop.UploadMetadataJsImpl> {
   /// Creates a new UploadMetadata with optional metadata parameters.
   factory UploadMetadata(
-          {String? md5Hash,
-          String? cacheControl,
-          String? contentDisposition,
-          String? contentEncoding,
-          String? contentLanguage,
-          String? contentType,
-          Map<String, String>? customMetadata}) =>
-      UploadMetadata.fromJsObject(
-        storage_interop.UploadMetadataJsImpl(
-          md5Hash: md5Hash?.toJS,
-          cacheControl: cacheControl?.toJS,
-          contentDisposition: contentDisposition?.toJS,
-          contentEncoding: contentEncoding?.toJS,
-          contentLanguage: contentLanguage?.toJS,
-          contentType: contentType?.toJS,
-          customMetadata:
-              (customMetadata != null) ? customMetadata.jsify() : null,
-        ),
-      );
+      {String? md5Hash,
+      String? cacheControl,
+      String? contentDisposition,
+      String? contentEncoding,
+      String? contentLanguage,
+      String? contentType,
+      Map<String, String>? customMetadata}) {
+    final metadata = storage_interop.UploadMetadataJsImpl();
+
+    if (md5Hash != null) {
+      metadata.md5Hash = md5Hash.toJS;
+    }
+    if (cacheControl != null) {
+      metadata.cacheControl = cacheControl.toJS;
+    }
+    if (contentDisposition != null) {
+      metadata.contentDisposition = contentDisposition.toJS;
+    }
+    if (contentEncoding != null) {
+      metadata.contentEncoding = contentEncoding.toJS;
+    }
+    if (contentLanguage != null) {
+      metadata.contentLanguage = contentLanguage.toJS;
+    }
+    if (contentType != null) {
+      metadata.contentType = contentType.toJS;
+    }
+    if (customMetadata != null) {
+      metadata.customMetadata = customMetadata.jsify();
+    }
+    return UploadMetadata.fromJsObject(metadata);
+  }
 
   /// Creates a new UploadMetadata from a [jsObject].
   UploadMetadata.fromJsObject(storage_interop.UploadMetadataJsImpl jsObject)
@@ -437,20 +450,34 @@ class SettableMetadata
     extends _SettableMetadataBase<storage_interop.SettableMetadataJsImpl> {
   /// Creates a new SettableMetadata with optional metadata parameters.
   factory SettableMetadata(
-          {String? cacheControl,
-          String? contentDisposition,
-          String? contentEncoding,
-          String? contentLanguage,
-          String? contentType,
-          Map? customMetadata}) =>
-      SettableMetadata.fromJsObject(storage_interop.SettableMetadataJsImpl(
-          cacheControl: cacheControl?.toJS,
-          contentDisposition: contentDisposition?.toJS,
-          contentEncoding: contentEncoding?.toJS,
-          contentLanguage: contentLanguage?.toJS,
-          contentType: contentType?.toJS,
-          customMetadata:
-              (customMetadata != null) ? customMetadata.jsify() : null));
+      {String? cacheControl,
+      String? contentDisposition,
+      String? contentEncoding,
+      String? contentLanguage,
+      String? contentType,
+      Map? customMetadata}) {
+    final metadata = storage_interop.SettableMetadataJsImpl();
+
+    if (cacheControl != null) {
+      metadata.cacheControl = cacheControl.toJS;
+    }
+    if (contentDisposition != null) {
+      metadata.contentDisposition = contentDisposition.toJS;
+    }
+    if (contentEncoding != null) {
+      metadata.contentEncoding = contentEncoding.toJS;
+    }
+    if (contentLanguage != null) {
+      metadata.contentLanguage = contentLanguage.toJS;
+    }
+    if (contentType != null) {
+      metadata.contentType = contentType.toJS;
+    }
+    if (customMetadata != null) {
+      metadata.customMetadata = customMetadata.jsify();
+    }
+    return SettableMetadata.fromJsObject(metadata);
+  }
 
   /// Creates a new SettableMetadata from a [jsObject].
   SettableMetadata.fromJsObject(storage_interop.SettableMetadataJsImpl jsObject)

diff --git a/tests/android/app/build.gradle b/tests/android/app/build.gradle
@@ -29,7 +29,7 @@ apply plugin: 'kotlin-android'
 apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle"
 
 android {
-    compileSdkVersion 33
+    compileSdkVersion 34
 
     compileOptions {
         sourceCompatibility JavaVersion.VERSION_1_8
@@ -47,7 +47,7 @@ android {
     defaultConfig {
         // TODO: Specify your own unique Application ID (https://developer.android.com/studio/build/application-id.html).
         applicationId "io.flutter.plugins.firebase.tests"
-        minSdkVersion 21
+        minSdkVersion 24
         targetSdkVersion 33
         versionCode flutterVersionCode.toInteger()
         versionName flutterVersionName

diff --git a/tests/integration_test/firebase_storage/reference_e2e.dart b/tests/integration_test/firebase_storage/reference_e2e.dart
@@ -258,8 +258,7 @@ void setupReferenceTests() {
           );
 
           expect(complete.metadata?.size, text.length);
-          // Metadata isn't saved on objects when using the emulator which fails test
-          // expect(complete.metadata?.contentLanguage, 'en');
+          expect(complete.metadata?.contentLanguage, 'en');
 
           // Download the file from Firebase Storage
           final downloadedData = await ref.getData();
@@ -332,22 +331,23 @@ void setupReferenceTests() {
               file,
               SettableMetadata(
                 contentLanguage: 'en',
+                contentType: 'text/plain',
                 customMetadata: <String, String>{'activity': 'test'},
               ),
             );
-
+            // metadata.contentType appears as application/octet-stream if not set. contentType is not inferred on emulator
             expect(complete.metadata?.size, kTestString.length);
-            // Metadata isn't saved on objects when using the emulator which fails test
-            // expect(complete.metadata?.contentLanguage, 'en');
-            // expect(complete.metadata?.customMetadata!['activity'], 'test');
-
+            expect(complete.metadata?.contentLanguage, 'en');
+            expect(complete.metadata?.customMetadata!['activity'], 'test');
+            expect(complete.metadata?.contentType, 'text/plain');
             // Check without SettableMetadata
             final Reference ref2 =
                 storage.ref('flutter-tests').child('flt-ok-2.txt');
             final TaskSnapshot complete2 = await ref2.putFile(
               file,
             );
             expect(complete2.metadata?.size, kTestString.length);
+            expect(complete2.metadata?.customMetadata, isA<Map>());
           },
           // putFile is not supported on the web platform.
           skip: kIsWeb,