Emulator fix - stop triggering metadata update on a file upload (#4213)

* stop triggering metadata update on a file upload * changelog entry * adding trigger tests * formatter
firebase · Feb 24, 2022 · 5564ad1 · 5564ad1
1 parent 6c9e4a8
commit 5564ad1
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,4 @@
 - Updates reserved environment variables for CF3 to include 'EVENTARC_CLOUD_EVENT_SOURCE' (#4196).
 - Fixes arg order for `firebase emulators:start --only storage` (#4195).
 - Fixes bug where environment variable for gen 2 functions weren't updated on deploy (#4209).
+- Fixes an issue in the storage emulator where a file upload would trigger functions with a metadata update handler (#4213).
diff --git a/scripts/integration-helpers/framework.ts b/scripts/integration-helpers/framework.ts
@@ -291,6 +291,14 @@ export class TriggerEndToEndTest {
     return this.invokeHttpFunction("writeToSpecificStorageBucket");
   }
 
+  updateMetadataDefaultStorage(): Promise<Response> {
+    return this.invokeHttpFunction("updateMetadataFromDefaultStorage");
+  }
+
+  updateMetadataSpecificStorageBucket(): Promise<Response> {
+    return this.invokeHttpFunction("updateMetadataFromSpecificStorageBucket");
+  }
+
   updateDeleteFromDefaultStorage(): Promise<Response> {
     return this.invokeHttpFunction("updateDeleteFromDefaultStorage");
   }

diff --git a/scripts/triggers-end-to-end-tests/functions/index.js b/scripts/triggers-end-to-end-tests/functions/index.js
@@ -126,6 +126,30 @@ exports.writeToSpecificStorageBucket = functions.https.onRequest(async (req, res
   res.json({ created: "ok" });
 });
 
+exports.updateMetadataFromDefaultStorage = functions.https.onRequest(async (req, res) => {
+  await admin.storage().bucket().file(STORAGE_FILE_NAME).save("hello metadata update!");
+  console.log("Wrote to Storage bucket");
+  await admin.storage().bucket().file(STORAGE_FILE_NAME).setMetadata({ somekey: "someval" });
+  console.log("Updated metadata of default Storage bucket");
+  res.json({ done: "ok" });
+});
+
+exports.updateMetadataFromSpecificStorageBucket = functions.https.onRequest(async (req, res) => {
+  await admin
+    .storage()
+    .bucket("test-bucket")
+    .file(STORAGE_FILE_NAME)
+    .save("hello metadata update!");
+  console.log("Wrote to a specific Storage bucket");
+  await admin
+    .storage()
+    .bucket("test-bucket")
+    .file(STORAGE_FILE_NAME)
+    .setMetadata({ somenewkey: "somenewval" });
+  console.log("Updated metadata of a specific Storage bucket");
+  res.json({ done: "ok" });
+});
+
 exports.updateDeleteFromDefaultStorage = functions.https.onRequest(async (req, res) => {
   await admin.storage().bucket().file(STORAGE_FILE_NAME).save("something new!");
   console.log("Wrote to Storage bucket");

diff --git a/scripts/triggers-end-to-end-tests/tests.ts b/scripts/triggers-end-to-end-tests/tests.ts
@@ -270,18 +270,18 @@ describe("storage emulator function triggers", () => {
   });
 
   it("should have triggered cloud functions", () => {
-    /* on object create two events fire (finalize & metadata update) */
+    /* on object create one event fires (finalize) */
     // default bucket
     expect(test.storageFinalizedTriggerCount).to.equal(1);
-    expect(test.storageMetadataTriggerCount).to.equal(1);
     expect(test.storageV2FinalizedTriggerCount).to.equal(1);
-    expect(test.storageV2MetadataTriggerCount).to.equal(1);
+    expect(test.storageMetadataTriggerCount).to.equal(0);
+    expect(test.storageV2MetadataTriggerCount).to.equal(0);
     expect(test.storageDeletedTriggerCount).to.equal(0);
     expect(test.storageV2DeletedTriggerCount).to.equal(0);
     // specific bucket
     expect(test.storageBucketFinalizedTriggerCount).to.equal(0);
-    expect(test.storageBucketMetadataTriggerCount).to.equal(0);
     expect(test.storageBucketV2FinalizedTriggerCount).to.equal(0);
+    expect(test.storageBucketMetadataTriggerCount).to.equal(0);
     expect(test.storageBucketV2MetadataTriggerCount).to.equal(0);
     expect(test.storageBucketDeletedTriggerCount).to.equal(0);
     expect(test.storageBucketV2DeletedTriggerCount).to.equal(0);
@@ -297,25 +297,81 @@ describe("storage emulator function triggers", () => {
   });
 
   it("should have triggered cloud functions", () => {
-    /* on object create two events fire (finalize & metadata update) */
+    /* on object create one event fires (finalize) */
     // default bucket
     expect(test.storageFinalizedTriggerCount).to.equal(0);
+    expect(test.storageV2FinalizedTriggerCount).to.equal(0);
     expect(test.storageMetadataTriggerCount).to.equal(0);
+    expect(test.storageV2MetadataTriggerCount).to.equal(0);
+    expect(test.storageDeletedTriggerCount).to.equal(0);
+    expect(test.storageV2DeletedTriggerCount).to.equal(0);
+    // specific bucket
+    expect(test.storageBucketFinalizedTriggerCount).to.equal(1);
+    expect(test.storageBucketV2FinalizedTriggerCount).to.equal(1);
+    expect(test.storageBucketMetadataTriggerCount).to.equal(0);
+    expect(test.storageBucketV2MetadataTriggerCount).to.equal(0);
+    expect(test.storageBucketDeletedTriggerCount).to.equal(0);
+    expect(test.storageBucketV2DeletedTriggerCount).to.equal(0);
+    test.resetCounts();
+  });
+
+  it("should write and update metadata from the default bucket of the storage emulator", async function (this) {
+    this.timeout(EMULATOR_TEST_TIMEOUT);
+
+    const response = await test.updateMetadataDefaultStorage();
+    expect(response.status).to.equal(200);
+    await new Promise((resolve) => setTimeout(resolve, EMULATORS_WRITE_DELAY_MS));
+  });
+
+  it("should have triggered cloud functions", () => {
+    /* on object create one event fires (finalize) */
+    /* on update one event fires (metadataUpdate) */
+    // default bucket
+    expect(test.storageFinalizedTriggerCount).to.equal(1);
+    expect(test.storageV2FinalizedTriggerCount).to.equal(1);
+    expect(test.storageMetadataTriggerCount).to.equal(1);
+    expect(test.storageV2MetadataTriggerCount).to.equal(1);
+    expect(test.storageDeletedTriggerCount).to.equal(0);
+    expect(test.storageV2DeletedTriggerCount).to.equal(0);
+    // specific bucket
+    expect(test.storageBucketFinalizedTriggerCount).to.equal(0);
+    expect(test.storageBucketV2FinalizedTriggerCount).to.equal(0);
+    expect(test.storageBucketMetadataTriggerCount).to.equal(0);
+    expect(test.storageBucketV2MetadataTriggerCount).to.equal(0);
+    expect(test.storageBucketDeletedTriggerCount).to.equal(0);
+    expect(test.storageBucketV2DeletedTriggerCount).to.equal(0);
+    test.resetCounts();
+  });
+
+  it("should write and update metadata from a specific bucket of the storage emulator", async function (this) {
+    this.timeout(EMULATOR_TEST_TIMEOUT);
+
+    const response = await test.updateMetadataSpecificStorageBucket();
+    expect(response.status).to.equal(200);
+    await new Promise((resolve) => setTimeout(resolve, EMULATORS_WRITE_DELAY_MS));
+  });
+
+  it("should have triggered cloud functions", () => {
+    /* on object create one event fires (finalize) */
+    /* on update one event fires (metadataUpdate) */
+    // default bucket
+    expect(test.storageFinalizedTriggerCount).to.equal(0);
     expect(test.storageV2FinalizedTriggerCount).to.equal(0);
+    expect(test.storageMetadataTriggerCount).to.equal(0);
     expect(test.storageV2MetadataTriggerCount).to.equal(0);
     expect(test.storageDeletedTriggerCount).to.equal(0);
     expect(test.storageV2DeletedTriggerCount).to.equal(0);
     // specific bucket
     expect(test.storageBucketFinalizedTriggerCount).to.equal(1);
-    expect(test.storageBucketMetadataTriggerCount).to.equal(1);
     expect(test.storageBucketV2FinalizedTriggerCount).to.equal(1);
+    expect(test.storageBucketMetadataTriggerCount).to.equal(1);
     expect(test.storageBucketV2MetadataTriggerCount).to.equal(1);
     expect(test.storageBucketDeletedTriggerCount).to.equal(0);
     expect(test.storageBucketV2DeletedTriggerCount).to.equal(0);
     test.resetCounts();
   });
 
-  it("should write, update, and delete from the default bucket of the storage emulator", async function (this) {
+  it("should write and delete from the default bucket of the storage emulator", async function (this) {
     this.timeout(EMULATOR_TEST_TIMEOUT);
 
     const response = await test.updateDeleteFromDefaultStorage();
@@ -324,26 +380,26 @@ describe("storage emulator function triggers", () => {
   });
 
   it("should have triggered cloud functions", () => {
-    /* on update two events fire (finalize & metadata update) */
+    /* on create one event fires (finalize) */
     /* on delete one event fires (delete) */
     // default bucket
     expect(test.storageFinalizedTriggerCount).to.equal(1);
-    expect(test.storageMetadataTriggerCount).to.equal(1);
     expect(test.storageV2FinalizedTriggerCount).to.equal(1);
-    expect(test.storageV2MetadataTriggerCount).to.equal(1);
+    expect(test.storageMetadataTriggerCount).to.equal(0);
+    expect(test.storageV2MetadataTriggerCount).to.equal(0);
     expect(test.storageDeletedTriggerCount).to.equal(1);
     expect(test.storageV2DeletedTriggerCount).to.equal(1);
     // specific bucket
     expect(test.storageBucketFinalizedTriggerCount).to.equal(0);
-    expect(test.storageBucketMetadataTriggerCount).to.equal(0);
     expect(test.storageBucketV2FinalizedTriggerCount).to.equal(0);
+    expect(test.storageBucketMetadataTriggerCount).to.equal(0);
     expect(test.storageBucketV2MetadataTriggerCount).to.equal(0);
     expect(test.storageBucketDeletedTriggerCount).to.equal(0);
     expect(test.storageBucketV2DeletedTriggerCount).to.equal(0);
     test.resetCounts();
   });
 
-  it("should write, update, and delete from a specific bucket of the storage emulator", async function (this) {
+  it("should write and delete from a specific bucket of the storage emulator", async function (this) {
     this.timeout(EMULATOR_TEST_TIMEOUT);
 
     const response = await test.updateDeleteFromSpecificStorageBucket();
@@ -352,20 +408,20 @@ describe("storage emulator function triggers", () => {
   });
 
   it("should have triggered cloud functions", () => {
-    /* on update two events fire (finalize & metadata update) */
+    /* on create one event fires (finalize) */
     /* on delete one event fires (delete) */
     // default bucket
     expect(test.storageFinalizedTriggerCount).to.equal(0);
-    expect(test.storageMetadataTriggerCount).to.equal(0);
     expect(test.storageV2FinalizedTriggerCount).to.equal(0);
+    expect(test.storageMetadataTriggerCount).to.equal(0);
     expect(test.storageV2MetadataTriggerCount).to.equal(0);
     expect(test.storageDeletedTriggerCount).to.equal(0);
     expect(test.storageV2DeletedTriggerCount).to.equal(0);
     // specific bucket
     expect(test.storageBucketFinalizedTriggerCount).to.equal(1);
-    expect(test.storageBucketMetadataTriggerCount).to.equal(1);
     expect(test.storageBucketV2FinalizedTriggerCount).to.equal(1);
-    expect(test.storageBucketV2MetadataTriggerCount).to.equal(1);
+    expect(test.storageBucketMetadataTriggerCount).to.equal(0);
+    expect(test.storageBucketV2MetadataTriggerCount).to.equal(0);
     expect(test.storageBucketDeletedTriggerCount).to.equal(1);
     expect(test.storageBucketV2DeletedTriggerCount).to.equal(1);
     test.resetCounts();

diff --git a/src/emulator/storage/files.ts b/src/emulator/storage/files.ts
@@ -286,8 +286,7 @@ export class StorageLayer {
         customMetadata: upload.metadata.metadata,
       },
       this._cloudFunctions,
-      bytes,
-      upload.metadata
+      bytes
     );
     const file = new StoredFile(finalMetadata, filePath);
     this._files.set(filePath, file);
@@ -320,8 +319,7 @@ export class StorageLayer {
         customMetadata: incomingMetadata.metadata,
       },
       this._cloudFunctions,
-      bytes,
-      incomingMetadata
+      bytes
     );
     const file = new StoredFile(md, this._persistence.getDiskPath(filePath));
     this._files.set(filePath, file);