Visitar URL original
fix(firestore): Change asserts to throw argumentError (#17302) · firebase/flutterfire@ec1e6a5 · GitHub
Skip to content

Commit ec1e6a5

Browse files
MichaelVerdonMichaelVerdon
authored
fix(firestore): Change asserts to throw argumentError (#17302)
Assertions are stripped in release mode in Flutter preventing errors from throwing which resulted in crashes.
1 parent c38263e commit ec1e6a5

5 files changed

Lines changed: 65 additions & 73 deletions

File tree

‎packages/cloud_firestore/cloud_firestore/lib/src/collection_reference.dart‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,13 @@ class _JsonCollectionReference extends _JsonQuery
113113
@override
114114
DocumentReference<Map<String, dynamic>> doc([String? path]) {
115115
if (path != null) {
116-
assert(path.isNotEmpty, 'a document path must be a non-empty string');
117-
assert(!path.contains('//'), 'a document path must not contain "//"');
118-
assert(path != '/', 'a document path must point to a valid document');
116+
if (path.isEmpty) {
117+
throw ArgumentError('A document path must be a non-empty string');
118+
} else if (path.contains('//')) {
119+
throw ArgumentError('A document path must not contain "//"');
120+
} else if (path == '/') {
121+
throw ArgumentError('A document path must point to a valid document');
122+
}
119123
}
120124

121125
return _JsonDocumentReference(firestore, _delegate.doc(path));

‎packages/cloud_firestore/cloud_firestore/lib/src/document_reference.dart‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,18 +120,15 @@ class _JsonDocumentReference
120120

121121
@override
122122
CollectionReference<Map<String, dynamic>> collection(String collectionPath) {
123-
assert(
124-
collectionPath.isNotEmpty,
125-
'a collectionPath path must be a non-empty string',
126-
);
127-
assert(
128-
!collectionPath.contains('//'),
129-
'a collection path must not contain "//"',
130-
);
131-
assert(
132-
isValidCollectionPath(collectionPath),
133-
'a collection path must point to a valid collection.',
134-
);
123+
if (collectionPath.isEmpty) {
124+
throw ArgumentError('A collectionPath must be a non-empty string.');
125+
} else if (collectionPath.contains('//')) {
126+
throw ArgumentError('A collection path must not contain "//".');
127+
} else if (!isValidCollectionPath(collectionPath)) {
128+
throw ArgumentError(
129+
'A collection path must point to a valid collection.',
130+
);
131+
}
135132

136133
return _JsonCollectionReference(
137134
firestore,

‎packages/cloud_firestore/cloud_firestore/lib/src/firestore.dart‎

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,15 @@ class FirebaseFirestore extends FirebasePluginPlatform {
8888

8989
/// Gets a [CollectionReference] for the specified Firestore path.
9090
CollectionReference<Map<String, dynamic>> collection(String collectionPath) {
91-
assert(
92-
collectionPath.isNotEmpty,
93-
'a collectionPath path must be a non-empty string',
94-
);
95-
assert(
96-
!collectionPath.contains('//'),
97-
'a collection path must not contain "//"',
98-
);
99-
assert(
100-
isValidCollectionPath(collectionPath),
101-
'a collection path must point to a valid collection.',
102-
);
91+
if (collectionPath.isEmpty) {
92+
throw ArgumentError('A collection path must be a non-empty string.');
93+
} else if (collectionPath.contains('//')) {
94+
throw ArgumentError('A collection path must not contain "//".');
95+
} else if (!isValidCollectionPath(collectionPath)) {
96+
throw ArgumentError(
97+
'A collection path must point to a valid collection.',
98+
);
99+
}
103100

104101
return _JsonCollectionReference(this, _delegate.collection(collectionPath));
105102
}
@@ -214,14 +211,13 @@ class FirebaseFirestore extends FirebasePluginPlatform {
214211

215212
/// Gets a [Query] for the specified collection group.
216213
Query<Map<String, dynamic>> collectionGroup(String collectionPath) {
217-
assert(
218-
collectionPath.isNotEmpty,
219-
'a collection path must be a non-empty string',
220-
);
221-
assert(
222-
!collectionPath.contains('/'),
223-
'a collection path passed to collectionGroup() cannot contain "/"',
224-
);
214+
if (collectionPath.isEmpty) {
215+
throw ArgumentError('A collection path must be a non-empty string.');
216+
} else if (collectionPath.contains('/')) {
217+
throw ArgumentError(
218+
'A collection path passed to collectionGroup() cannot contain "/".',
219+
);
220+
}
225221

226222
return _JsonQuery(this, _delegate.collectionGroup(collectionPath));
227223
}
@@ -237,18 +233,13 @@ class FirebaseFirestore extends FirebasePluginPlatform {
237233

238234
/// Gets a [DocumentReference] for the specified Firestore path.
239235
DocumentReference<Map<String, dynamic>> doc(String documentPath) {
240-
assert(
241-
documentPath.isNotEmpty,
242-
'a document path must be a non-empty string',
243-
);
244-
assert(
245-
!documentPath.contains('//'),
246-
'a collection path must not contain "//"',
247-
);
248-
assert(
249-
isValidDocumentPath(documentPath),
250-
'a document path must point to a valid document.',
251-
);
236+
if (documentPath.isEmpty) {
237+
throw ArgumentError('A document path must be a non-empty string.');
238+
} else if (documentPath.contains('//')) {
239+
throw ArgumentError('A document path must not contain "//".');
240+
} else if (!isValidDocumentPath(documentPath)) {
241+
throw ArgumentError('A document path must point to a valid document.');
242+
}
252243

253244
return _JsonDocumentReference(this, _delegate.doc(documentPath));
254245
}

‎packages/cloud_firestore/cloud_firestore/test/cloud_firestore_test.dart‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,12 @@ void main() {
8181
});
8282

8383
test('does not expect an empty path', () {
84-
expect(() => firestore!.collection(''), throwsAssertionError);
84+
expect(() => firestore!.collection(''), throwsArgumentError);
8585
});
8686

8787
test('does accept an invalid path', () {
8888
// 'foo/bar' points to a document
89-
expect(() => firestore!.collection('foo/bar'), throwsAssertionError);
89+
expect(() => firestore!.collection('foo/bar'), throwsArgumentError);
9090
});
9191
});
9292

@@ -96,13 +96,13 @@ void main() {
9696
});
9797

9898
test('does not expect an empty path', () {
99-
expect(() => firestore!.collectionGroup(''), throwsAssertionError);
99+
expect(() => firestore!.collectionGroup(''), throwsArgumentError);
100100
});
101101

102102
test('does accept a path containing "/"', () {
103103
expect(
104104
() => firestore!.collectionGroup('foo/bar/baz'),
105-
throwsAssertionError,
105+
throwsArgumentError,
106106
);
107107
});
108108
});
@@ -113,12 +113,12 @@ void main() {
113113
});
114114

115115
test('does not expect an empty path', () {
116-
expect(() => firestore!.doc(''), throwsAssertionError);
116+
expect(() => firestore!.doc(''), throwsArgumentError);
117117
});
118118

119119
test('does accept an invalid path', () {
120120
// 'foo' points to a collection
121-
expect(() => firestore!.doc('bar'), throwsAssertionError);
121+
expect(() => firestore!.doc('bar'), throwsArgumentError);
122122
});
123123
});
124124
});

‎packages/cloud_firestore/cloud_firestore/test/collection_reference_test.dart‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -98,19 +98,19 @@ void main() {
9898

9999
test('path must be non-empty strings', () {
100100
DocumentReference docRef = firestore.doc('foo/bar');
101-
expect(() => firestore.collection(''), throwsAssertionError);
102-
expect(() => docRef.collection(''), throwsAssertionError);
101+
expect(() => firestore.collection(''), throwsArgumentError);
102+
expect(() => docRef.collection(''), throwsArgumentError);
103103
});
104104

105105
test('path must be odd length', () {
106106
DocumentReference docRef = firestore.doc('foo/bar');
107-
expect(() => firestore.collection('foo/bar'), throwsAssertionError);
107+
expect(() => firestore.collection('foo/bar'), throwsArgumentError);
108108
expect(
109109
() => firestore.collection('foo/bar/baz/quu'),
110-
throwsAssertionError,
110+
throwsArgumentError,
111111
);
112-
expect(() => docRef.collection('foo/bar'), throwsAssertionError);
113-
expect(() => docRef.collection('foo/bar/baz/quu'), throwsAssertionError);
112+
expect(() => docRef.collection('foo/bar'), throwsArgumentError);
113+
expect(() => docRef.collection('foo/bar/baz/quu'), throwsArgumentError);
114114
});
115115

116116
test('must not have empty segments', () {
@@ -124,31 +124,31 @@ void main() {
124124
DocumentReference docRef = colRef.doc('test-document');
125125

126126
for (final path in badPaths) {
127-
expect(() => firestore.collection(path), throwsAssertionError);
128-
expect(() => firestore.doc(path), throwsAssertionError);
129-
expect(() => colRef.doc(path), throwsAssertionError);
130-
expect(() => docRef.collection(path), throwsAssertionError);
127+
expect(() => firestore.collection(path), throwsArgumentError);
128+
expect(() => firestore.doc(path), throwsArgumentError);
129+
expect(() => colRef.doc(path), throwsArgumentError);
130+
expect(() => docRef.collection(path), throwsArgumentError);
131131
}
132132
});
133133

134134
group('validate', () {
135135
test('path must be non-empty strings', () {
136136
DocumentReference docRef = firestore.doc('foo/bar');
137-
expect(() => firestore.collection(''), throwsAssertionError);
138-
expect(() => docRef.collection(''), throwsAssertionError);
137+
expect(() => firestore.collection(''), throwsArgumentError);
138+
expect(() => docRef.collection(''), throwsArgumentError);
139139
});
140140

141141
test('path must be odd length', () {
142142
DocumentReference docRef = firestore.doc('foo/bar');
143-
expect(() => firestore.collection('foo/bar'), throwsAssertionError);
143+
expect(() => firestore.collection('foo/bar'), throwsArgumentError);
144144
expect(
145145
() => firestore.collection('foo/bar/baz/quu'),
146-
throwsAssertionError,
146+
throwsArgumentError,
147147
);
148-
expect(() => docRef.collection('foo/bar'), throwsAssertionError);
148+
expect(() => docRef.collection('foo/bar'), throwsArgumentError);
149149
expect(
150150
() => docRef.collection('foo/bar/baz/quu'),
151-
throwsAssertionError,
151+
throwsArgumentError,
152152
);
153153
});
154154

@@ -163,10 +163,10 @@ void main() {
163163
DocumentReference docRef = colRef.doc('test-document');
164164

165165
for (final String path in badPaths) {
166-
expect(() => firestore.collection(path), throwsAssertionError);
167-
expect(() => firestore.doc(path), throwsAssertionError);
168-
expect(() => colRef.doc(path), throwsAssertionError);
169-
expect(() => docRef.collection(path), throwsAssertionError);
166+
expect(() => firestore.collection(path), throwsArgumentError);
167+
expect(() => firestore.doc(path), throwsArgumentError);
168+
expect(() => colRef.doc(path), throwsArgumentError);
169+
expect(() => docRef.collection(path), throwsArgumentError);
170170
}
171171
});
172172
});

0 commit comments

Comments
 (0)