Skip to content

Commit fbd59e1

Browse files
DellaBittaDellaBitta
committed
Merge branch 'ddb-sync-bundle-parse' into ddb-no-client-origin-bundles
2 parents 8e1e51a + 04620bc commit fbd59e1

File tree

5 files changed

+38
-55
lines changed

5 files changed

+38
-55
lines changed

‎packages/firestore/src/api/snapshot.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ export function documentSnapshotFromJSON<
601601
>(
602602
db: Firestore,
603603
json: object,
604-
...args: unknown[]
604+
converter?: FirestoreDataConverter<AppModelType, DbModelType>
605605
): DocumentSnapshot<AppModelType, DbModelType> {
606606
if (validateJSON(json, DocumentSnapshot._jsonSchema)) {
607607
if (json.bundle === 'NOT SUPPORTED') {
@@ -637,12 +637,6 @@ export function documentSnapshotFromJSON<
637637
ResourcePath.fromString(json.bundleName)
638638
);
639639

640-
let converter: FirestoreDataConverter<AppModelType, DbModelType> | null =
641-
null;
642-
if (args[0]) {
643-
converter = args[0] as FirestoreDataConverter<AppModelType, DbModelType>;
644-
}
645-
646640
// Return the external facing DocumentSnapshot.
647641
return new DocumentSnapshot(
648642
db,
@@ -653,7 +647,7 @@ export function documentSnapshotFromJSON<
653647
/* hasPendingWrites= */ false,
654648
/* fromCache= */ false
655649
),
656-
converter
650+
converter ? converter : null
657651
);
658652
}
659653
throw new FirestoreError(
@@ -913,7 +907,7 @@ export function querySnapshotFromJSON<
913907
>(
914908
db: Firestore,
915909
json: object,
916-
...args: unknown[]
910+
converter?: FirestoreDataConverter<AppModelType, DbModelType>
917911
): QuerySnapshot<AppModelType, DbModelType> {
918912
if (validateJSON(json, QuerySnapshot._jsonSchema)) {
919913
if (json.bundle === 'NOT SUPPORTED') {
@@ -960,16 +954,10 @@ export function querySnapshotFromJSON<
960954
/* hasCachedResults= */ false
961955
);
962956

963-
let converter: FirestoreDataConverter<AppModelType, DbModelType> | null =
964-
null;
965-
if (args[0]) {
966-
converter = args[0] as FirestoreDataConverter<AppModelType, DbModelType>;
967-
}
968-
969957
// Create an external Query object, required to construct the QuerySnapshot.
970958
const externalQuery = new Query<AppModelType, DbModelType>(
971959
db,
972-
converter,
960+
converter ? converter : null,
973961
query
974962
);
975963

‎packages/firestore/src/lite-api/reference.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -334,22 +334,12 @@ export class DocumentReference<
334334
>(
335335
firestore: Firestore,
336336
json: object,
337-
...args: unknown[]
337+
converter?: FirestoreDataConverter<NewAppModelType, NewDbModelType>
338338
): DocumentReference<NewAppModelType, NewDbModelType> {
339339
if (validateJSON(json, DocumentReference._jsonSchema)) {
340-
let converter: FirestoreDataConverter<
341-
NewAppModelType,
342-
NewDbModelType
343-
> | null = null;
344-
if (args[0]) {
345-
converter = args[0] as FirestoreDataConverter<
346-
NewAppModelType,
347-
NewDbModelType
348-
>;
349-
}
350340
return new DocumentReference<NewAppModelType, NewDbModelType>(
351341
firestore,
352-
converter,
342+
converter ? converter : null,
353343
new DocumentKey(ResourcePath.fromString(json.referencePath))
354344
);
355345
}

‎packages/firestore/src/util/bundle_reader_sync_impl.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import { BundleMetadata } from '../protos/firestore_bundle_proto';
1919
import { JsonProtoSerializer } from '../remote/serializer';
20+
import { Code, FirestoreError } from '../util/error';
2021

2122
import { BundleReaderSync, SizedBundleElement } from './bundle_reader';
2223

@@ -84,7 +85,10 @@ export class BundleReaderSyncImpl implements BundleReaderSync {
8485
*/
8586
private readJsonString(length: number): string {
8687
if (this.cursor + length > this.bundleData.length) {
87-
throw new Error('Reached the end of bundle when more is expected.');
88+
throw new FirestoreError(
89+
Code.INTERNAL,
90+
'Reached the end of bundle when more is expected.'
91+
);
8892
}
8993
const result = this.bundleData.slice(this.cursor, (this.cursor += length));
9094
return result;

‎packages/firestore/src/util/json_validation.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export function validateJSON<S extends JsonSchema>(
112112
schema: S
113113
): json is Json<S> {
114114
if (!isPlainObject(json)) {
115-
throw new FirestoreError(Code.INVALID_ARGUMENT, 'json must be an object');
115+
throw new FirestoreError(Code.INVALID_ARGUMENT, 'JSON must be an object');
116116
}
117117
let error: string | undefined = undefined;
118118
for (const key in schema) {
@@ -121,12 +121,13 @@ export function validateJSON<S extends JsonSchema>(
121121
const value: { value: unknown } | undefined =
122122
'value' in schema[key] ? { value: schema[key].value } : undefined;
123123
if (!(key in json)) {
124-
error = `json missing required field: ${key}`;
124+
error = `JSON missing required field: '${key}'`;
125+
break;
125126
}
126127
// eslint-disable-next-line @typescript-eslint/no-explicit-any
127128
const fieldValue = (json as any)[key];
128129
if (typeString && typeof fieldValue !== typeString) {
129-
error = `json field '${key}' must be a ${typeString}.`;
130+
error = `JSON field '${key}' must be a ${typeString}.`;
130131
break;
131132
} else if (value !== undefined && fieldValue !== value.value) {
132133
error = `Expected '${key}' field to equal '${value.value}'`;

‎packages/firestore/test/unit/api/database.test.ts

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ describe('DocumentReference', () => {
9999
const db = newTestFirestore();
100100
expect(() => {
101101
DocumentReference.fromJSON(db, {});
102-
}).to.throw;
102+
}).to.throw("JSON missing required field: 'type'");
103103
});
104104

105105
it('fromJSON() throws with missing type data', () => {
@@ -110,7 +110,7 @@ describe('DocumentReference', () => {
110110
bundleName: 'test name',
111111
bundle: 'test bundle'
112112
});
113-
}).to.throw;
113+
}).to.throw("JSON missing required field: 'type'");
114114
});
115115

116116
it('fromJSON() throws with invalid type data', () => {
@@ -122,7 +122,7 @@ describe('DocumentReference', () => {
122122
bundleName: 'test name',
123123
bundle: 'test bundle'
124124
});
125-
}).to.throw;
125+
}).to.throw("JSON field 'type' must be a string");
126126
});
127127

128128
it('fromJSON() throws with missing bundleSource', () => {
@@ -133,7 +133,7 @@ describe('DocumentReference', () => {
133133
bundleName: 'test name',
134134
bundle: 'test bundle'
135135
});
136-
}).to.throw;
136+
}).to.throw("JSON missing required field: 'bundleSource'");
137137
});
138138

139139
it('fromJSON() throws with invalid bundleSource type', () => {
@@ -145,7 +145,7 @@ describe('DocumentReference', () => {
145145
bundleName: 'test name',
146146
bundle: 'test bundle'
147147
});
148-
}).to.throw;
148+
}).to.throw("JSON field 'bundleSource' must be a string");
149149
});
150150

151151
it('fromJSON() throws with invalid bundleSource value', () => {
@@ -157,7 +157,7 @@ describe('DocumentReference', () => {
157157
bundleName: 'test name',
158158
bundle: 'test bundle'
159159
});
160-
}).to.throw;
160+
}).to.throw("Expected 'bundleSource' field to equal 'DocumentSnapshot'");
161161
});
162162

163163
it('fromJSON() throws with missing bundleName', () => {
@@ -168,7 +168,7 @@ describe('DocumentReference', () => {
168168
bundleSource: 'DocumentSnapshot',
169169
bundle: 'test bundle'
170170
});
171-
}).to.throw;
171+
}).to.throw("JSON missing required field: 'bundleName'");
172172
});
173173

174174
it('fromJSON() throws with invalid bundleName', () => {
@@ -180,7 +180,7 @@ describe('DocumentReference', () => {
180180
bundleName: 1,
181181
bundle: 'test bundle'
182182
});
183-
}).to.throw;
183+
}).to.throw("JSON field 'bundleName' must be a string");
184184
});
185185

186186
it('fromJSON() throws with missing bundle', () => {
@@ -191,7 +191,7 @@ describe('DocumentReference', () => {
191191
bundleSource: 'DocumentSnapshot',
192192
bundleName: 'test name'
193193
});
194-
}).to.throw;
194+
}).to.throw("JSON missing required field: 'bundle'");
195195
});
196196

197197
it('fromJSON() throws with invalid bundle', () => {
@@ -203,7 +203,7 @@ describe('DocumentReference', () => {
203203
bundleName: 'test name',
204204
bundle: 1
205205
});
206-
}).to.throw;
206+
}).to.throw("JSON field 'bundle' must be a string");
207207
});
208208

209209
it('fromJSON() does not throw', () => {
@@ -564,7 +564,7 @@ describe('QuerySnapshot', () => {
564564
const db = newTestFirestore();
565565
expect(() => {
566566
querySnapshotFromJSON(db, {});
567-
}).to.throw;
567+
}).to.throw("JSON missing required field: 'type'");
568568
});
569569

570570
it('fromJSON() throws with missing type data', () => {
@@ -575,7 +575,7 @@ describe('QuerySnapshot', () => {
575575
bundleName: 'test name',
576576
bundle: 'test bundle'
577577
});
578-
}).to.throw;
578+
}).to.throw("JSON missing required field: 'type'");
579579
});
580580

581581
it('fromJSON() throws with invalid type data', () => {
@@ -587,18 +587,18 @@ describe('QuerySnapshot', () => {
587587
bundleName: 'test name',
588588
bundle: 'test bundle'
589589
});
590-
}).to.throw;
590+
}).to.throw("JSON field 'type' must be a string");
591591
});
592592

593-
it('fromJSON() throws with invalid type data', () => {
593+
it('fromJSON() throws with missing bundle source data', () => {
594594
const db = newTestFirestore();
595595
expect(() => {
596596
querySnapshotFromJSON(db, {
597597
type: QuerySnapshot._jsonSchemaVersion,
598598
bundleName: 'test name',
599599
bundle: 'test bundle'
600600
});
601-
}).to.throw;
601+
}).to.throw("JSON missing required field: 'bundleSource'");
602602
});
603603

604604
it('fromJSON() throws with invalid bundleSource type', () => {
@@ -610,7 +610,7 @@ describe('QuerySnapshot', () => {
610610
bundleName: 'test name',
611611
bundle: 'test bundle'
612612
});
613-
}).to.throw;
613+
}).to.throw("JSON field 'bundleSource' must be a string");
614614
});
615615

616616
it('fromJSON() throws with invalid bundleSource value', () => {
@@ -622,7 +622,7 @@ describe('QuerySnapshot', () => {
622622
bundleName: 'test name',
623623
bundle: 'test bundle'
624624
});
625-
}).to.throw;
625+
}).to.throw("Expected 'bundleSource' field to equal 'QuerySnapshot'");
626626
});
627627

628628
it('fromJSON() throws with missing bundleName', () => {
@@ -633,7 +633,7 @@ describe('QuerySnapshot', () => {
633633
bundleSource: 'QuerySnapshot',
634634
bundle: 'test bundle'
635635
});
636-
}).to.throw;
636+
}).to.throw("JSON missing required field: 'bundleName'");
637637
});
638638

639639
it('fromJSON() throws with invalid bundleName', () => {
@@ -645,21 +645,21 @@ describe('QuerySnapshot', () => {
645645
bundleName: 1,
646646
bundle: 'test bundle'
647647
});
648-
}).to.throw;
648+
}).to.throw("JSON field 'bundleName' must be a string");
649649
});
650650

651-
it('fromJSON() throws with missing bundle data', () => {
651+
it('fromJSON() throws with missing bundle field', () => {
652652
const db = newTestFirestore();
653653
expect(() => {
654654
querySnapshotFromJSON(db, {
655655
type: QuerySnapshot._jsonSchemaVersion,
656656
bundleSource: 'QuerySnapshot',
657657
bundleName: 'test name'
658658
});
659-
}).to.throw;
659+
}).to.throw("JSON missing required field: 'bundle'");
660660
});
661661

662-
it('fromJSON() throws with invalid bundle data', () => {
662+
it('fromJSON() throws with invalid bundle field', () => {
663663
const db = newTestFirestore();
664664
expect(() => {
665665
querySnapshotFromJSON(db, {
@@ -668,7 +668,7 @@ describe('QuerySnapshot', () => {
668668
bundleName: 'test name',
669669
bundle: 1
670670
});
671-
}).to.throw;
671+
}).to.throw("JSON field 'bundle' must be a string");
672672
});
673673

674674
it('fromJSON does not throw', () => {

0 commit comments

Comments
 (0)