[1/2] Spec for filters argument should be driven by field types (#2254)

This commit breaks azul.openapi.schema.make_type and everything that depends on it. (@hannes-ucsc)

Author: amarjandu

lambdas/service/app.py

@@ -61,6 +61,7 @@
     HealthController,
 )
 from azul.indexer.document import (
+    FieldType,
     Nested,
 )
 from azul.logging import (
@@ -1087,28 +1088,41 @@ def list_catalogs():
     termFacets=generic_object_spec
 )
 
+
+def generate_operator_spec(field_type: FieldType) -> Sequence[JSON]:
+    array_schema = schema.array({},
+                                items=schema.array(field_type.api_type,
+                                                   minItems=2,
+                                                   maxItems=2),
+                                minItems=1,
+                                maxItems=16)
+    operator_spec = []
+    for operator in field_type.operators:
+        if operator == 'is':
+            operator_spec.append(
+                schema.object(is_=schema.array(field_type.api_type))
+            )
+        else:
+            operator_spec.append(
+                schema.object_type(additionalProperties=False,
+                                   properties={operator: array_schema})
+            )
+    return operator_spec
+
+
 filters_param_spec = params.query(
     'filters',
     schema.optional(application_json(schema.object_type(
         default='{}',
         example={'cellCount': {'within': [[10000, 1000000000]]}},
         properties={
             field: {
-                'oneOf': [
-                    schema.object(is_=schema.array({})),
-                    *(
-                        schema.object_type({
-                            op: schema.array({}, minItems=2, maxItems=2)
-                        })
-                        for op in ['contains', 'within', 'intersects']
-                    )
-                ]
+                'oneOf': generate_operator_spec(field_type)
             }
-            for field in app.fields
+            for field, field_type
+            in app.repository_controller.field_types(app.catalog).items()
         }
     ))),
-    # FIXME: Spec for `filters` argument should be driven by field types
-    #        https://github.com/DataBiosphere/azul/issues/2254
     description=format_description('''
         Criteria to filter entities from the search results.
 

src/azul/indexer/document.py

@@ -4,7 +4,6 @@
 )
 from collections.abc import (
     Mapping,
-    Sequence,
 )
 from datetime import (
     datetime,
@@ -18,8 +17,12 @@
     ClassVar,
     Generic,
     Optional,
+    Sequence,
+    Tuple,
+    Type,
     TypeVar,
     Union,
+    get_args,
 )
 
 import attr
@@ -40,6 +43,9 @@
     SimpleSourceSpec,
     SourceRef,
 )
+from azul.openapi import (
+    schema,
+)
 from azul.time import (
     format_dcp2_datetime,
     parse_dcp2_datetime,
@@ -228,15 +234,24 @@ def __str__(self) -> str:
         return f'aggregate for {self.entity}'
 
 
+# The native type of the field in documents as they are being created by a
+# transformer or processed by an aggregator.
 N = TypeVar('N')
 
+# The type of the field in a document just before it's being written to the
+# index. Think "translated type".
 T = TypeVar('T', bound=AnyJSON)
 
 
 class FieldType(Generic[N, T], metaclass=ABCMeta):
     shadowed: bool = False
     es_sort_mode: str = 'min'
     allow_sorting_by_empty_lists: bool = True
+    operators: Tuple[str, ...] = ('is',)
+
+    def __init__(self, native_type: Type[N], translated_type: Type[T]):
+        self.native_type = native_type
+        self.translated_type = translated_type
 
     @property
     @abstractmethod
@@ -254,12 +269,16 @@ def from_index(self, value: T) -> N:
     def to_tsv(self, value: N) -> str:
         return '' if value is None else str(value)
 
+    @property
+    def api_type(self) -> JSON:
+        return schema.make_type(self.native_type)
+
 
 class PassThrough(Generic[T], FieldType[T, T]):
     allow_sorting_by_empty_lists = False
 
-    def __init__(self, *, es_type: Optional[str]):
-        super().__init__()
+    def __init__(self, translated_type, *, es_type: Optional[str]):
+        super().__init__(translated_type, translated_type)
         self._es_type = es_type
 
     @property
@@ -272,21 +291,78 @@ def to_index(self, value: T) -> T:
     def from_index(self, value: T) -> T:
         return value
 
+    @property
+    def operators(self) -> Tuple[str, ...]:
+        if self.native_type == int:
+            return 'is', 'within'
+        else:
+            return 'is',
+
 
-pass_thru_str: PassThrough[str] = PassThrough(es_type='keyword')
-pass_thru_int: PassThrough[int] = PassThrough(es_type='long')
-pass_thru_bool: PassThrough[bool] = PassThrough(es_type='boolean')
+pass_thru_str: PassThrough[str] = PassThrough(str, es_type='keyword')
+pass_thru_int: PassThrough[int] = PassThrough(int, es_type='long')
+pass_thru_bool: PassThrough[bool] = PassThrough(bool, es_type='boolean')
 # FIXME: change the es_type for JSON to `nested`
 #        https://github.com/DataBiosphere/azul/issues/2621
-pass_thru_json: PassThrough[JSON] = PassThrough(es_type=None)
+pass_thru_json: PassThrough[JSON] = PassThrough(JSON, es_type=None)
+
+
+class ClosedRange(PassThrough[JSON]):
+    operators = ('is', 'within', 'contains', 'intersects')
+    valid_keys = {'gte', 'lte'}
+
+    def __init__(self, translated_type):
+        super().__init__(translated_type, es_type=None)
+
+    def to_index(self, value: T) -> T:
+        assert self.valid_keys == value.keys(), value
+        return super().to_index(value)
+
+    def from_index(self, value: T) -> T:
+        assert self.valid_keys == value.keys(), value
+        return super().from_index(value)
+
+    @property
+    def api_type(self) -> JSON:
+        return schema.make_type(int)
+
+
+closed_range = ClosedRange(JSON)
+
+
+class Nullable(FieldType[Optional[N], T]):
 
+    def __init__(self, native_type_: Type[N], translated_type: Type[T]) -> None:
+        super().__init__(Optional[native_type_], translated_type)
+        self.native_type_ = native_type_
 
-class NullableString(FieldType[Optional[str], str]):
+    @property
+    @abstractmethod
+    def es_type(self) -> Optional[str]:
+        raise NotImplementedError
+
+    @abstractmethod
+    def to_index(self, value: N) -> T:
+        raise NotImplementedError
+
+    @abstractmethod
+    def from_index(self, value: T) -> N:
+        raise NotImplementedError
+
+    @property
+    def api_type(self) -> JSON:
+        return schema.make_type(self.native_type_)
+
+
+class NullableString(Nullable[str, str]):
     # Note that the replacement values for `None` used for each data type
     # ensure that `None` values are placed at the end of a sorted list.
     null_string = '~null'
     es_type = 'keyword'
 
+    def __init__(self):
+        super().__init__(str, str)
+
     def to_index(self, value: Optional[str]) -> str:
         return self.null_string if value is None else value
 
@@ -298,16 +374,25 @@ def from_index(self, value: str) -> Optional[str]:
 
 Number = Union[float, int]
 
-N_ = TypeVar('N_', bound=Number)
+# `N_` is the same as `N`, except for numeric types. We would specify a bound
+# for this type variable if it weren't for a limitation of the PyCharm type
+# checker: with the bound set, PyCharm does not emit a warning when passing an
+# int to a method of NullableNumber(float).
+N_ = TypeVar('N_')
 
 
-class NullableNumber(Generic[N_], FieldType[Optional[N_], Number]):
+class NullableNumber(Generic[N_], Nullable[N_, Number]):
     shadowed = True
     # Maximum int that can be represented as a 64-bit int and double IEEE
     # floating point number. This prevents loss when converting between the two.
     null_int = sys.maxsize - 1023
     assert null_int == int(float(null_int))
     es_type = 'long'
+    operators = ('is', 'within')
+
+    def __init__(self, native_type_: Type[N_]) -> None:
+        assert issubclass(native_type_, get_args(Number))
+        super().__init__(native_type_, Number)
 
     def to_index(self, value: Optional[N_]) -> Number:
         return self.null_int if value is None else value
@@ -316,14 +401,18 @@ def from_index(self, value: Number) -> Optional[N_]:
         return None if value == self.null_int else value
 
 
-null_int: NullableNumber[int] = NullableNumber()
+null_int = NullableNumber(int)
 
-null_float: NullableNumber[float] = NullableNumber()
+null_float = NullableNumber(float)
 
 
 class NullableBool(NullableNumber[bool]):
     shadowed = False
     es_type = 'boolean'
+    operators = ('is',)
+
+    def __init__(self):
+        super().__init__(bool)
 
     def to_index(self, value: Optional[bool]) -> Number:
         value = {False: 0, True: 1, None: None}[value]
@@ -334,10 +423,10 @@ def from_index(self, value: Number) -> Optional[bool]:
         return {0: False, 1: True, None: None}[value]
 
 
-null_bool: NullableBool = NullableBool()
+null_bool = NullableBool()
 
 
-class NullableDateTime(FieldType[Optional[str], str]):
+class NullableDateTime(Nullable[str, str]):
     es_type = 'date'
     null = format_dcp2_datetime(datetime(9999, 1, 1, tzinfo=timezone.utc))
 
@@ -355,16 +444,26 @@ def from_index(self, value: str) -> Optional[str]:
             return value
 
 
-null_datetime: NullableDateTime = NullableDateTime()
+null_datetime: NullableDateTime = NullableDateTime(str, str)
 
 
 class Nested(PassThrough[JSON]):
     properties: Mapping[str, FieldType]
 
     def __init__(self, **properties):
-        super().__init__(es_type='nested')
+        super().__init__(JSON, es_type='nested')
         self.properties = properties
 
+    @property
+    def api_type(self) -> JSON:
+        properties = dict()
+        for field, field_type in self.properties.items():
+            if isinstance(field_type, Nullable):
+                properties[field] = schema.optional(field_type.native_type_)
+            else:
+                properties[field] = field_type.native_type
+        return schema.object(**properties)
+
 
 FieldTypes4 = Union[Mapping[str, FieldType], Sequence[FieldType], FieldType]
 FieldTypes3 = Union[Mapping[str, FieldTypes4], Sequence[FieldType], FieldType]

src/azul/openapi/schema.py

@@ -409,6 +409,9 @@ def make_type(t: TYPE) -> JSON:
     >>> make_type(str)
     {'type': 'string'}
 
+    >>> make_type(JSON)
+    {'type': 'object'}
+
     A JSON schema type name may be used instead:
 
     >>> make_type('string')
@@ -427,8 +430,8 @@ def make_type(t: TYPE) -> JSON:
     """
     if t is None or isinstance(t, type):
         return _primitive_types[t]
-    # We can't use `JSON` directly because it is generic and parameterized
-    # but __origin__ yields the unparameterized generic type.
+    elif t == JSON:
+        return {'type': 'object'}
     elif isinstance(t, str):
         return {'type': t}
     elif isinstance(t, get_origin(JSON)):