Fix error caused by undefined "trackedQuery" in QueryManager #8570

stephenarosaj · 2024-10-15T17:30:48Z

Add check for non-query refType in executeQuery that throws error on refType mismatch
Add unit test to check for properly thrown error in case that executeQuery receives non-query refType, and doesn't throw error when receiving query refType

…executeQuery-refType-mismatch

changeset-bot · 2024-10-15T17:30:52Z

🦋 Changeset detected

Latest commit: d4de37a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@firebase/data-connect	Patch
firebase	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-10-15T17:31:06Z

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_vertexai_responses.sh should be updated to clone the latest version of the responses: v4.0

google-oss-bot · 2024-10-15T17:46:49Z

Size Report ¹

Affected Products

@firebase/data-connect
Type Base (2e28041) Merge (3cfd8ab) Diff
browser 19.8 kB 19.9 kB +131 B (+0.7%)
esm5 22.5 kB 22.6 kB +131 B (+0.6%)
main 24.3 kB 24.4 kB +131 B (+0.5%)
module 19.8 kB 19.9 kB +131 B (+0.7%)
firebase
Type Base (2e28041) Merge (3cfd8ab) Diff
firebase-data-connect.js 16.5 kB 16.6 kB +101 B (+0.6%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/pS10EOtq0E.html

maneesht · 2024-10-15T17:51:17Z

packages/data-connect/test/unit/QueryManager.test.ts

+    );
+
+    expect(() => qm.executeQuery(mutationRef)).to.throw(error.message);
+    expect(() => qm.executeQuery(queryRef)).to.not.throw(error.message);


You should just be able to call executeQuery(dc, queryRef) here and similarly the line before, though I'd recommend renaming both of those variables to prevent conflicts with the queryRef and mutationRef functions

Accepted in commit a0ecb6f

maneesht · 2024-10-15T17:52:15Z

packages/data-connect/test/unit/QueryManager.test.ts

+      refType: MUTATION_STR as 'query'
+    };
+
+    const queryRef: QueryRef<string, string> = {


I'd recommend creating a queryRef the same way that most users do: by calling queryRef(dc, queryName);

Accepted in commit a0ecb6f

maneesht · 2024-10-15T17:52:32Z

packages/data-connect/test/unit/QueryManager.test.ts

+    });
+
+    const mutationRef: QueryRef<string, string> = {
+      name: 'm',


Call mutationRef here as well

Accepted in commit a0ecb6f

maneesht · 2024-10-15T17:54:45Z

packages/data-connect/test/unit/QueryManager.test.ts

+    const rt = new RESTTransport(options, undefined, undefined, authProvider);
+    const qm = new QueryManager(rt);
+    const app = initializeApp({ projectId: 'p' });
+    const dc = getDataConnect(app, {


Can you clean these up in an afterEach, like here:

firebase-js-sdk/packages/data-connect/test/unit/gmpid.test.ts

Lines 43 to 50 in a214691

beforeEach(() => {

initializeFetch(fakeFetchImpl);

app = initializeApp({ projectId: 'p', appId: APPID }, 'fdsasdf'); // TODO(mtewani): Replace with util function

dc = getDataConnect(app, { connector: 'c', location: 'l', service: 's' });

});

afterEach(async () => {

await dc._delete();

await deleteApp(app);

Accepted in commit a0ecb6f

google-oss-bot · 2024-10-15T17:59:00Z

Size Analysis Report ¹

Affected Products

@firebase/data-connect
- DataConnect
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.5 kB 11.6 kB +98 B (+0.9%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- MUTATION_STR
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.5 kB 11.6 kB +98 B (+0.8%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- MutationManager
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.5 kB 11.6 kB +98 B (+0.8%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- QUERY_STR
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.5 kB 11.6 kB +98 B (+0.9%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- SOURCE_CACHE
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.5 kB 11.6 kB +98 B (+0.9%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- SOURCE_SERVER
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.5 kB 11.6 kB +98 B (+0.8%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- connectDataConnectEmulator
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.6 kB 11.7 kB +98 B (+0.8%)
  size-with-ext-deps 29.7 kB 29.8 kB +101 B (+0.3%)
- executeMutation
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.6 kB 11.7 kB +98 B (+0.8%)
  size-with-ext-deps 29.7 kB 29.8 kB +101 B (+0.3%)
- executeQuery
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.6 kB 11.7 kB +98 B (+0.8%)
  size-with-ext-deps 29.7 kB 29.8 kB +101 B (+0.3%)
- getDataConnect
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 12.2 kB 12.3 kB +98 B (+0.8%)
  size-with-ext-deps 37.4 kB 37.5 kB +101 B (+0.3%)
- mutationRef
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.6 kB 11.7 kB +98 B (+0.8%)
  size-with-ext-deps 29.7 kB 29.8 kB +101 B (+0.3%)
- parseOptions
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.5 kB 11.6 kB +98 B (+0.9%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- queryRef
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.6 kB 11.7 kB +98 B (+0.8%)
  size-with-ext-deps 29.7 kB 29.8 kB +101 B (+0.3%)
- setLogLevel
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.6 kB 11.7 kB +98 B (+0.8%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- subscribe
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 12.8 kB 12.9 kB +98 B (+0.8%)
  size-with-ext-deps 37.9 kB 38.0 kB +101 B (+0.3%)
- terminate
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.6 kB 11.7 kB +98 B (+0.8%)
  size-with-ext-deps 29.6 kB 29.7 kB +101 B (+0.3%)
- toQueryRef
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 12.4 kB 12.5 kB +98 B (+0.8%)
  size-with-ext-deps 37.6 kB 37.7 kB +101 B (+0.3%)
- validateArgs
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 12.4 kB 12.5 kB +98 B (+0.8%)
  size-with-ext-deps 37.5 kB 37.6 kB +101 B (+0.3%)
- validateDCOptions
  Size
  Type Base (2e28041) Merge (3cfd8ab) Diff
  size 11.8 kB 11.9 kB +98 B (+0.8%)
  size-with-ext-deps 29.8 kB 29.9 kB +101 B (+0.3%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/KOmOl4B5fR.html

…executeQuery-refType-mismatch

…test.ts' test cases

stephenarosaj

Accepted changes to new unit test proposed by @maneesht in review

stephenarosaj · 2024-10-15T18:56:57Z

packages/data-connect/test/unit/QueryManager.test.ts

+    const rt = new RESTTransport(options, undefined, undefined, authProvider);
+    const qm = new QueryManager(rt);
+    const app = initializeApp({ projectId: 'p' });
+    const dc = getDataConnect(app, {


Accepted in commit a0ecb6f

stephenarosaj · 2024-10-15T18:57:13Z

packages/data-connect/test/unit/QueryManager.test.ts

+    });
+
+    const mutationRef: QueryRef<string, string> = {
+      name: 'm',


Accepted in commit a0ecb6f

stephenarosaj · 2024-10-15T18:57:18Z

packages/data-connect/test/unit/QueryManager.test.ts

+      refType: MUTATION_STR as 'query'
+    };
+
+    const queryRef: QueryRef<string, string> = {


Accepted in commit a0ecb6f

stephenarosaj · 2024-10-15T18:57:24Z

packages/data-connect/test/unit/QueryManager.test.ts

+    );
+
+    expect(() => qm.executeQuery(mutationRef)).to.throw(error.message);
+    expect(() => qm.executeQuery(queryRef)).to.not.throw(error.message);


Accepted in commit a0ecb6f

maneesht · 2024-10-15T19:53:14Z

.changeset/beige-roses-cross.md

+---
+
+- Add check for non-query refType in `executeQuery` that throws error on refType mismatch
+- Add unit test to check that error is thrown when `executeQuery` receives non-query refType, and not thrown otherwise


Changelogs get added to release notes, and the unit tests aren't really relevant. Can we say something like:
Throw error when calling executeQuery with mutations

Accepted in commit d4de37a

stephenarosaj added 3 commits October 14, 2024 18:45

Add check for non-query refType in executeQuery argument

387bf8c

Add unit test checking for non-query refType in executeQuery argument

59d2e4f

Merge remote-tracking branch 'origin/main' into stephenarosaj/handle-…

6187271

…executeQuery-refType-mismatch

stephenarosaj requested a review from a team as a code owner October 15, 2024 17:30

add changeset

e0a32a1

stephenarosaj requested review from a team as code owners October 15, 2024 17:42

maneesht requested changes Oct 15, 2024

View reviewed changes

stephenarosaj added 3 commits October 15, 2024 11:31

implement requested changes to new test

9bc852c

Merge remote-tracking branch 'origin/main' into stephenarosaj/handle-…

ed76645

…executeQuery-refType-mismatch

change app initialization parameters to prevent clobbering in 'utils.…

a0ecb6f

…test.ts' test cases

stephenarosaj requested a review from aashishpatil-g as a code owner October 15, 2024 18:51

stephenarosaj commented Oct 15, 2024

View reviewed changes

maneesht reviewed Oct 15, 2024

View reviewed changes

maneesht approved these changes Oct 15, 2024

View reviewed changes

update changelog

d4de37a

maneesht approved these changes Oct 15, 2024

View reviewed changes

hsubox76 approved these changes Oct 15, 2024

View reviewed changes

stephenarosaj merged commit cf988b0 into main Oct 15, 2024
39 checks passed

stephenarosaj deleted the stephenarosaj/handle-executeQuery-refType-mismatch branch October 15, 2024 21:49

google-oss-bot mentioned this pull request Oct 17, 2024

Version Packages #8581

Merged

Tech-FestivalDoJapao mentioned this pull request Nov 12, 2024

[Snyk] Upgrade firebase from 10.14.1 to 11.0.0 Tech-FestivalDoJapao/Gerenciamento#35

Merged

firebase locked and limited conversation to collaborators Nov 15, 2024

	beforeEach(() => {
	initializeFetch(fakeFetchImpl);
	app = initializeApp({ projectId: 'p', appId: APPID }, 'fdsasdf'); // TODO(mtewani): Replace with util function
	dc = getDataConnect(app, { connector: 'c', location: 'l', service: 's' });
	});
	afterEach(async () => {
	await dc._delete();
	await deleteApp(app);

Fix error caused by undefined "trackedQuery" in QueryManager #8570

Fix error caused by undefined "trackedQuery" in QueryManager #8570

Conversation

stephenarosaj commented Oct 15, 2024

changeset-bot bot commented Oct 15, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Oct 15, 2024

Vertex AI Mock Responses Check ⚠️

google-oss-bot commented Oct 15, 2024 • edited Loading

Size Report 1

Affected Products

@firebase/data-connect

firebase

Test Logs

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

google-oss-bot commented Oct 15, 2024 • edited Loading

Size Analysis Report 1

Affected Products

@firebase/data-connect

DataConnect

MUTATION_STR

MutationManager

QUERY_STR

SOURCE_CACHE

SOURCE_SERVER

connectDataConnectEmulator

executeMutation

executeQuery

getDataConnect

mutationRef

parseOptions

queryRef

setLogLevel

subscribe

terminate

toQueryRef

validateArgs

validateDCOptions

Test Logs

stephenarosaj left a 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

changeset-bot bot commented Oct 15, 2024 •

edited

Loading

google-oss-bot commented Oct 15, 2024 •

edited

Loading

Size Report ¹

`@firebase/data-connect`

`firebase`

google-oss-bot commented Oct 15, 2024 •

edited

Loading

Size Analysis Report ¹

`@firebase/data-connect`

`DataConnect`

`MUTATION_STR`

`MutationManager`

`QUERY_STR`

`SOURCE_CACHE`

`SOURCE_SERVER`

`connectDataConnectEmulator`

`executeMutation`

`executeQuery`

`getDataConnect`

`mutationRef`

`parseOptions`

`queryRef`

`setLogLevel`

`subscribe`

`terminate`

`toQueryRef`

`validateArgs`

`validateDCOptions`