Skip to content

Commit 12b5931

Browse files
dcbrandaodcbrandao
committed
Remove asserts
Asserts are bad for production code paths, replacing them with throwing ValidationErrors
1 parent 16c1978 commit 12b5931

8 files changed

Lines changed: 86 additions & 40 deletions

File tree

‎spanner_orm/condition.py‎

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ def _validate(self, model_class: Type[Any]) -> None:
9292
class ColumnsEqualCondition(Condition):
9393
"""Used to join records by matching column values."""
9494

95-
def __init__(self, origin_column: str,
96-
destination_model_class: Type[Any],
95+
def __init__(self, origin_column: str, destination_model_class: Type[Any],
9796
destination_column: str):
9897
super().__init__()
9998
self.column = origin_column
@@ -117,13 +116,19 @@ def _types(self) -> Dict[str, type_pb2.Type]:
117116
return {}
118117

119118
def _validate(self, model_class: Type[Any]) -> None:
120-
assert self.column in model_class.schema
119+
if self.column not in model_class.schema:
120+
raise error.ValidationError('{} is not a column on {}'.format(
121+
self.column, model_class.table))
121122
origin = model_class.schema[self.column]
122-
assert self.destination_column in self.destination_model_class.schema
123+
if self.destination_column not in self.destination_model_class.schema:
124+
raise error.ValidationError('{} is not a column on {}'.format(
125+
self.destination_column, self.destination_model_class.table))
123126
dest = self.destination_model_class.schema[self.destination_column]
124127

125-
assert (origin.field_type() == dest.field_type() and
126-
origin.nullable() == dest.nullable())
128+
if (origin.field_type() != dest.field_type() or
129+
origin.nullable() != dest.nullable()):
130+
raise error.ValidationError('Types of {} and {} do not match'.format(
131+
origin.name, dest.name))
127132

128133

129134
class ForceIndexCondition(Condition):
@@ -155,11 +160,15 @@ def _types(self) -> Dict[str, type_pb2.Type]:
155160
return {}
156161

157162
def _validate(self, model_class: Type[Any]) -> None:
158-
assert self.name in model_class.indexes
159-
if self.index:
160-
assert self.index == model_class.indexes[self.name]
163+
if self.name not in model_class.indexes:
164+
raise error.ValidationError('{} is not an index on {}'.format(
165+
self.name, model_class.table))
166+
if self.index and self.index != model_class.indexes[self.name]:
167+
raise error.ValidationError('{} does not belong to {}'.format(
168+
self.index.name, model_class.table))
161169

162-
assert not model_class.indexes[self.name].primary
170+
if model_class.indexes[self.name].primary:
171+
raise error.ValidationError('Cannot force query using primary index')
163172

164173

165174
class IncludesCondition(Condition):
@@ -228,9 +237,12 @@ def _types(self) -> Dict[str, type_pb2.Type]:
228237
return {}
229238

230239
def _validate(self, model_class: Type[Any]) -> None:
231-
assert self.name in model_class.relations
232-
if self.relation:
233-
assert self.relation == model_class.relations[self.name]
240+
if self.name not in model_class.relations:
241+
raise error.ValidationError('{} is not a relation on {}'.format(
242+
self.name, model_class.table))
243+
if self.relation and self.relation != model_class.relations[self.name]:
244+
raise error.ValidationError('{} does not belong to {}'.format(
245+
self.relation.name, model_class.table))
234246

235247
other_model_class = model_class.relations[self.name].destination
236248
for condition in self._conditions:
@@ -377,7 +389,9 @@ def _validate(self, model_class: Type[Any]) -> None:
377389
for (column, _) in self.orderings:
378390
if isinstance(column, field.Field):
379391
column = column.name
380-
assert column in model_class.schema
392+
if column not in model_class.schema:
393+
raise error.ValidationError('{} is not a column on {}'.format(
394+
column, model_class.table))
381395

382396

383397
class ComparisonCondition(Condition):
@@ -417,10 +431,15 @@ def _types(self) -> type_pb2.Type:
417431
return {self._column_key: self.model_class.schema[self.column].grpc_type()}
418432

419433
def _validate(self, model_class: Type[Any]) -> None:
420-
assert self.column in model_class.schema
421-
if self.field:
422-
assert self.field == model_class.schema[self.column]
423-
assert self.value is not None
434+
if self.column not in model_class.schema:
435+
raise error.ValidationError('{} is not a column on {}'.format(
436+
self.column, model_class.table))
437+
if self.field and self.field != model_class.schema[self.column]:
438+
raise error.ValidationError('{} does not belong to {}'.format(
439+
self.column, model_class.table))
440+
if self.value is None:
441+
raise error.ValidationError('{} does not support NULL'.format(
442+
self.__name__))
424443
model_class.schema[self.column].validate(self.value)
425444

426445

@@ -440,10 +459,14 @@ def _types(self) -> type_pb2.Type:
440459
return {self._column_key: list_type}
441460

442461
def _validate(self, model_class: Type[Any]) -> None:
443-
assert isinstance(self.value, list)
444-
assert self.column in model_class.schema
445-
if self.field:
446-
assert self.field == model_class.schema[self.column]
462+
if not isinstance(self.value, list):
463+
raise error.ValidationError('{} is not a list'.format(self.value))
464+
if self.column not in model_class.schema:
465+
raise error.ValidationError('{} is not a column on {}'.format(
466+
self.column, model_class.table))
467+
if self.field and self.field != model_class.schema[self.column]:
468+
raise error.ValidationError('{} does not belong to {}'.format(
469+
self.column, model_class.table))
447470
for value in self.value:
448471
model_class.schema[self.column].validate(value)
449472

@@ -478,9 +501,12 @@ def _types(self) -> type_pb2.Type:
478501
return super()._types()
479502

480503
def _validate(self, model_class: Type[Any]) -> None:
481-
assert self.column in model_class.schema
482-
if self.field:
483-
assert self.field == model_class.schema[self.column]
504+
if self.column not in model_class.schema:
505+
raise error.ValidationError('{} is not a column on {}'.format(
506+
self.column, model_class.table))
507+
if self.field and self.field != model_class.schema[self.column]:
508+
raise error.ValidationError('{} does not belong to {}'.format(
509+
self.column, model_class.table))
484510
model_class.schema[self.column].validate(self.value)
485511

486512

‎spanner_orm/error.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,7 @@
1717

1818
class SpannerError(Exception):
1919
pass
20+
21+
22+
class ValidationError(Exception):
23+
pass

‎spanner_orm/field.py‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
import datetime
2121
from typing import Any, Type
2222

23+
from spanner_orm import error
24+
2325
from google.cloud.spanner_v1.proto import type_pb2
2426

2527

@@ -54,7 +56,8 @@ def primary_key(self) -> bool:
5456

5557
def validate(self, value) -> None:
5658
if value is None:
57-
assert self._nullable, 'None set for non-nullable field'
59+
if not self._nullable:
60+
raise error.ValidationError('None set for non-nullable field')
5861
else:
5962
self._type.validate_type(value)
6063

@@ -91,7 +94,8 @@ def grpc_type() -> type_pb2.Type:
9194

9295
@staticmethod
9396
def validate_type(value: Any) -> None:
94-
assert isinstance(value, bool), '{} is not of type bool'.format(value)
97+
if not isinstance(value, bool):
98+
raise error.ValidationError('{} is not of type bool'.format(value))
9599

96100

97101
class Integer(FieldType):
@@ -107,7 +111,8 @@ def grpc_type() -> type_pb2.Type:
107111

108112
@staticmethod
109113
def validate_type(value: Any) -> None:
110-
assert isinstance(value, int), '{} is not of type int'.format(value)
114+
if not isinstance(value, int):
115+
raise error.ValidationError('{} is not of type int'.format(value))
111116

112117

113118
class String(FieldType):
@@ -123,7 +128,8 @@ def grpc_type() -> type_pb2.Type:
123128

124129
@staticmethod
125130
def validate_type(value) -> None:
126-
assert isinstance(value, str), '{} is not of type str'.format(value)
131+
if not isinstance(value, str):
132+
raise error.ValidationError('{} is not of type str'.format(value))
127133

128134

129135
class StringArray(FieldType):
@@ -139,9 +145,11 @@ def grpc_type() -> type_pb2.Type:
139145

140146
@staticmethod
141147
def validate_type(value: Any) -> None:
142-
assert isinstance(value, list), '{} is not of type list'.format(value)
148+
if not isinstance(value, list):
149+
raise error.ValidationError('{} is not of type list'.format(value))
143150
for item in value:
144-
assert isinstance(item, str), '{} is not of type str'.format(item)
151+
if not isinstance(item, str):
152+
raise error.ValidationError('{} is not of type str'.format(item))
145153

146154

147155
class Timestamp(FieldType):
@@ -157,7 +165,8 @@ def grpc_type() -> type_pb2.Type:
157165

158166
@staticmethod
159167
def validate_type(value: Any) -> None:
160-
assert isinstance(value, datetime.datetime)
168+
if not isinstance(value, datetime.datetime):
169+
raise error.ValidationError('{} is not of type datetime'.format(value))
161170

162171

163172
ALL_TYPES = [Boolean, Integer, String, StringArray, Timestamp]

‎spanner_orm/index.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
from typing import List, Optional
2020

21+
from spanner_orm import error
22+
2123

2224
class Index(object):
2325
"""Represents an index on a Model."""
@@ -29,7 +31,8 @@ def __init__(self,
2931
null_filtered: bool = False,
3032
unique: bool = False,
3133
storing_columns: Optional[List[str]] = None):
32-
assert columns, 'An index must have at least one column'
34+
if not columns:
35+
raise error.ValidationError('An index must have at least one column')
3336
self.columns = columns
3437
self.name = None
3538
self.parent = parent

‎spanner_orm/metadata.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
from typing import Any, Dict, Type, Optional
3434

35+
from spanner_orm import error
3536
from spanner_orm import field
3637
from spanner_orm import index
3738
from spanner_orm import registry
@@ -67,7 +68,8 @@ def finalize(self) -> None:
6768
relevant state has been added and the calculation of the final data should
6869
now happen.
6970
"""
70-
assert not self._finalized
71+
if self._finalized:
72+
raise error.SpannerError('Metadata was already finalized')
7173
sorted_fields = list(sorted(self.fields.values(), key=lambda f: f.position))
7274

7375
if index.Index.PRIMARY_INDEX not in self.indexes:

‎spanner_orm/model.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ def table(cls):
124124
def validate_value(cls, field_name, value, error_type=error.SpannerError):
125125
try:
126126
cls.schema[field_name].validate(value)
127-
except AssertionError as ex:
127+
except error.ValidationError as ex:
128128
raise error_type(*ex.args)
129129

130130

‎spanner_orm/relationship.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ def __init__(self,
6262

6363
@property
6464
def constraints(self) -> List[RelationshipConstraint]:
65-
assert self.origin, 'Origin must be set before constraints is called'
65+
if not self.origin:
66+
raise error.ValidationError(
67+
'Origin must be set before constraints is called')
6668
return self._parse_constraints()
6769

6870
@property
@@ -81,11 +83,11 @@ def _parse_constraints(self) -> List[RelationshipConstraint]:
8183
constraints = []
8284
for origin_column, destination_column in self._constraints.items():
8385
if origin_column not in self.origin.schema:
84-
raise error.SpannerError(
86+
raise error.ValidationError(
8587
'Origin column must be present in origin model')
8688

8789
if destination_column not in self.destination.schema:
88-
raise error.SpannerError(
90+
raise error.ValidationError(
8991
'Destination column must be present in destination model')
9092

9193
# TODO(dbrandao): remove when pytype #234 is fixed

‎spanner_orm/tests/query_test.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,13 +284,13 @@ def test_includes_error_on_multiple_results_for_single(self):
284284
_ = select_query.process_results(rows)
285285

286286
def test_includes_error_on_invalid_relation(self):
287-
with self.assertRaises(AssertionError):
287+
with self.assertRaises(error.ValidationError):
288288
self.includes('bad_relation')
289289

290290
@parameterized.parameters(('bad_column', 0), ('child_key', 'good value'),
291291
('key', ['bad value']))
292292
def test_includes_error_on_invalid_subconditions(self, column, value):
293-
with self.assertRaises(AssertionError):
293+
with self.assertRaises(error.ValidationError):
294294
self.includes('parent', condition.equal_to(column, value))
295295

296296
def test_or(self):

0 commit comments

Comments
 (0)