fix(firestore-bigquery-export): Add Backward Compatibility and New Event Types for Firestore BigQuery Export #2244

Gustolandia · 2024-12-20T16:36:02Z

#1981

Description (issue 1981):
This PR introduces backward-compatible event types while adding new naming conventions for Firestore BigQuery Export. The changes ensure robust event handling across multiple versions of the extension. The following updates have been made:

Key Changes:
1. New Event Types:
• Introduced new events using the updated naming convention:
• firebase.extensions.firestore-bigquery-export.v1.onStart
• firebase.extensions.firestore-bigquery-export.v1.onSuccess
• firebase.extensions.firestore-bigquery-export.v1.onError
• firebase.extensions.firestore-bigquery-export.v1.onCompletion
2. Backward Compatibility:
• Maintains the existing events under the old naming convention:
• firebase.extensions.firestore-counter.v1.*
• Both old and new event types are published for each operation to ensure smooth migration.
3. Refactored Event Publishing:
• Updated event publishing logic to dynamically handle both old and new event types.
• Functions now publish events using Promise.all for better concurrency.
4. Unit Tests:
• Enhanced unit tests to verify:
• Events for both old and new naming conventions are correctly published.
• Proper handling of CREATE, DELETE, and UPDATE scenarios.
• Mocked Eventarc interactions for improved testing reliability.
5. Code Improvements:
• Refactored events.ts to dynamically generate both old and new event types for reuse.
• Added extensive documentation to clarify the event publishing logic and backward compatibility strategy.

Testing:
1.	Unit Tests:
•	Added test cases for CREATE, DELETE, and UPDATE scenarios.
•	Verified that both old (firestore-counter) and new (firestore-bigquery-export) event types are published.
2.	Local Testing:
•	Tested event publishing with mocked Eventarc to ensure consistency.
•	Verified performance and reliability with simulated Firestore data changes.

Checklist:
•	Code refactored for event compatibility.
•	Tests updated and passed successfully.
•	Documentation updated for event naming conventions.
•	Verified performance under high-load scenarios.

Notes:
•	This PR ensures compatibility while transitioning to the new naming convention.
•	Developers can gradually update their consumers to listen for new events.
•	Legacy systems will remain unaffected by these changes.

firestore-bigquery-export/extension.yaml

firestore-bigquery-export/functions/__tests__/functions.test.ts

cabljac · 2024-12-23T10:35:01Z

firestore-bigquery-export/functions/src/events.ts

+/**
+ * Generates both the OLD and NEW event types to maintain backward compatibility.
+ *
+ * Old Event Type: firebase.extensions.firestore-counter.v1.{eventName}


I'm not sure we should be publishing both, i think we probably want a breaking change here, @pr-Mais ?

probably yes, but should be documented well in the CHANGELOG

I thought we had agreed in not having breaking changes. Can you explain what is the issue @cabljac @pr-Mais ?

firestore-bigquery-export/extension.yaml

pr-Mais · 2024-12-30T13:51:31Z

firestore-bigquery-export/extension.yaml

+  - param: EXT_INSTANCE_ID
+    label: Extension Instance ID
+    description: >-
+      What is the unique identifier for this instance of the extension? This ID
+      ensures that multiple instances do not interfere with one another. It is
+      set automatically by Firebase but can also be customized if required.
+    required: true
+


Suggested change

- param: EXT_INSTANCE_ID

label: Extension Instance ID

description: >-

What is the unique identifier for this instance of the extension? This ID

ensures that multiple instances do not interfere with one another. It is

set automatically by Firebase but can also be customized if required.

required: true

Since other extensions don't use unique extension ID for the events, let's keep it consistent.

I added this based on your previous comments regarding several instances of the same extension.

Yeah, after second thought we agreed on keeping the behaviour consistent with other extensions

pr-Mais · 2024-12-30T13:54:08Z

firestore-bigquery-export/functions/src/events.ts

+ */
+const getEventTypes = (eventName: string) => [
+  `firebase.extensions.firestore-counter.v1.${eventName}`, // OLD Event Type for backward compatibility
+  `firebase.extensions.${EXTENSION_NAME}.v1.${eventName}`, // NEW Event Type following the updated convention


Suggested change

`firebase.extensions.${EXTENSION_NAME}.v1.${eventName}`, // NEW Event Type following the updated convention

`firebase.extensions.firestore-bigquery-export.v1.${eventName}`, // NEW Event Type following the updated convention

Same as before: I added this based on your previous comments regarding several instances of the same extension.

pr-Mais · 2024-12-30T13:54:25Z

firestore-bigquery-export/functions/src/events.ts

 export const recordStartEvent = async (data: string | object) => {
-  if (!eventChannel) return;
+  if (!eventChannel) return Promise.resolve(); // Explicitly return a resolved Promise


Suggested change

if (!eventChannel) return Promise.resolve(); // Explicitly return a resolved Promise

if (!eventChannel) return Promise.resolve();

But if there is no eventChannel?

What do you mean? I just removed the comment, not needed as the return is self-explanatory.

pr-Mais · 2024-12-30T13:54:38Z

firestore-bigquery-export/functions/src/events.ts

 export const recordErrorEvent = async (err: Error, subject?: string) => {
-  if (!eventChannel) return;
+  if (!eventChannel) return Promise.resolve(); // Ensure consistent return type


Suggested change

if (!eventChannel) return Promise.resolve(); // Ensure consistent return type

if (!eventChannel) return Promise.resolve();

But if there is no eventChannel?

Sorry my bad.

pr-Mais · 2024-12-30T13:54:53Z

firestore-bigquery-export/functions/src/events.ts

 export const recordSuccessEvent = async ({
  subject,
  data,
 }: {
  subject: string;
  data: string | object;
 }) => {
-  if (!eventChannel) return;
+  if (!eventChannel) return Promise.resolve(); // Explicitly return a resolved Promise


Suggested change

if (!eventChannel) return Promise.resolve(); // Explicitly return a resolved Promise

if (!eventChannel) return Promise.resolve();

But if there is no eventChannel?

Sorry my bad.

pr-Mais · 2024-12-30T13:55:03Z

firestore-bigquery-export/functions/src/events.ts

 export const recordCompletionEvent = async (data: string | object) => {
-  if (!eventChannel) return;
+  if (!eventChannel) return Promise.resolve(); // Ensure consistent return type


Suggested change

if (!eventChannel) return Promise.resolve(); // Ensure consistent return type

if (!eventChannel) return Promise.resolve();

But if there is no eventChannel?

This suggested change is just to remove the unnecessary code comment

fyi - in an async function, return; is the same as return Promise.resolve() .

It's implied by the async keyword.

ts might complain about some difference between Promise<void> and Promise<undefined>

Sorry my bad.

…tibility (#1981)

Gustolandia added type: bug Something isn't working extension: firestore-bigquery-export Related to firestore-bigquery-export extension labels Dec 20, 2024

Gustolandia requested review from dackers86, cabljac and pr-Mais December 20, 2024 16:36

Gustolandia self-assigned this Dec 20, 2024

Gustolandia force-pushed the @invertase/Extensions/firestore-bigquery-export/1981 branch from aa614f5 to 925668e Compare December 20, 2024 18:23

Gustolandia requested a review from CorieW December 20, 2024 18:34

Gustolandia mentioned this pull request Dec 20, 2024

🐛 [firestore-bigquery-export] Fix eventArc event types #1981

Open

Gustolandia linked an issue Dec 20, 2024 that may be closed by this pull request

🐛 [firestore-bigquery-export] Fix eventArc event types #1981

Open

cabljac changed the title ~~fix(firestore-bigquery-export): @invertase: Add Backward Compatibility and New Event Types for Firestore BigQuery Export~~ fix(firestore-bigquery-export): Add Backward Compatibility and New Event Types for Firestore BigQuery Export Dec 23, 2024

cabljac reviewed Dec 23, 2024

View reviewed changes

firestore-bigquery-export/extension.yaml Show resolved Hide resolved

cabljac reviewed Dec 23, 2024

View reviewed changes

firestore-bigquery-export/functions/__tests__/functions.test.ts Show resolved Hide resolved

cabljac reviewed Dec 23, 2024

View reviewed changes

pr-Mais reviewed Dec 23, 2024

View reviewed changes

firestore-bigquery-export/extension.yaml Show resolved Hide resolved

Gustolandia requested review from pr-Mais and cabljac December 27, 2024 20:02

Gustolandia force-pushed the @invertase/Extensions/firestore-bigquery-export/1981 branch 3 times, most recently from 9ddae40 to 3fcac8e Compare December 28, 2024 01:08

pr-Mais requested changes Dec 30, 2024

View reviewed changes

Gustolandia requested a review from pr-Mais December 30, 2024 18:17

cabljac force-pushed the next branch from 29da9a4 to ed34bd6 Compare January 6, 2025 11:21

cabljac requested a review from a team as a code owner January 6, 2025 11:21

cabljac force-pushed the @invertase/Extensions/firestore-bigquery-export/1981 branch from 3fcac8e to 15cc2b0 Compare January 6, 2025 12:26

fix(firestore-bigquery-export): update event types for Eventarc compa…

26951aa

…tibility (#1981)

cabljac force-pushed the @invertase/Extensions/firestore-bigquery-export/1981 branch from 15cc2b0 to 26951aa Compare January 6, 2025 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(firestore-bigquery-export): Add Backward Compatibility and New Event Types for Firestore BigQuery Export #2244

fix(firestore-bigquery-export): Add Backward Compatibility and New Event Types for Firestore BigQuery Export #2244

Gustolandia commented Dec 20, 2024

cabljac Dec 23, 2024

pr-Mais Dec 23, 2024

Gustolandia Dec 27, 2024

pr-Mais Dec 30, 2024

Gustolandia Dec 30, 2024

pr-Mais Jan 1, 2025

pr-Mais Dec 30, 2024

Gustolandia Dec 30, 2024

pr-Mais Dec 30, 2024

Gustolandia Dec 30, 2024

pr-Mais Jan 1, 2025

pr-Mais Dec 30, 2024

Gustolandia Dec 30, 2024

Gustolandia Jan 6, 2025

pr-Mais Dec 30, 2024

Gustolandia Dec 30, 2024

Gustolandia Jan 6, 2025

pr-Mais Dec 30, 2024

Gustolandia Dec 30, 2024

cabljac Jan 6, 2025

cabljac Jan 6, 2025

cabljac Jan 6, 2025

Gustolandia Jan 6, 2025

	`firebase.extensions.${EXTENSION_NAME}.v1.${eventName}`, // NEW Event Type following the updated convention
	`firebase.extensions.firestore-bigquery-export.v1.${eventName}`, // NEW Event Type following the updated convention

	if (!eventChannel) return Promise.resolve(); // Explicitly return a resolved Promise
	if (!eventChannel) return Promise.resolve();

fix(firestore-bigquery-export): Add Backward Compatibility and New Event Types for Firestore BigQuery Export #2244

Are you sure you want to change the base?

fix(firestore-bigquery-export): Add Backward Compatibility and New Event Types for Firestore BigQuery Export #2244

Conversation

Gustolandia commented Dec 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment