Skip to content

Commit

Permalink
Make GARE validation as simple as possible
Browse files Browse the repository at this point in the history
Rather than trying to build a framework, this is the "no framework"
solution, in which we define very concrete helpers for very concrete
purposes, including a dedicated one for the "ConsentRequired" Literal.

All of the GARE classes now inherit from an internal helper class,
Serializable, which defines from_dict, to_dict, and _supported_fields
in a generic way. The last of these *does* use some signature
inspection magic, but nothing too abstruse.

The basic transformation is to replace each combination of a
class-level annotation + a `SUPPORTED_FIELDS` entry with relevant
assignment in `__init__`.

Some tangentially related simplifications and minor improvements are
included:

- `extra_fields -> extra`

- Removal of unnecessary str splitting (after object is initialized)

- `isinstance` checking can also handle deserialization of dicts

- checking for non-null values will not accept `session_message`-only
  GARE data -- at least one of the semantic fields is required by the
  rewritten check

- GlobusAuthRequirementsError does not inherit from `GlobusError` or
  `Exception` -- it is not clear that this inheritance is useful or
  instructive to any user, since it mixes Exception hierarchies with
  data-representing hierarchies

- replace direct `ValueError` usage with a custom ValidationError
  class -- this avoids any messy scenarios in which a ValueError is
  accidentally introduced and incorrectly caught (e.g. from a stdlib
  call)

There are also some more significant improvements included. Most
notably:

- Annotations explicitly do not accept `None` unless it is a valid
  value (i.e. annotations align with validation requirements)

- to_dict can now look for a concrete type (`Serializable`) and
  therefore can automatically invoke `to_dict` down a tree of objects

Although brevity is a non-goal of this changeset -- more verbose but
clearer would be acceptable -- the result is almost 200 LOC lighter in
the `src/` tree. The primary ways in which things became shorter
appear to be:

- explicit version is often much shorter than the framework-ized
  version (e.g. `LegacyConsentRequiredAPError`), and even where the
  two are close, the explicit version is shorter

- `to_dict` and `from_dict` centralization
  • Loading branch information
sirosen committed Aug 6, 2023
1 parent dede384 commit 5dab4bf
Show file tree
Hide file tree
Showing 7 changed files with 271 additions and 463 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
from __future__ import annotations

import inspect
import typing as t

T = t.TypeVar("T", bound="Serializable")


class Serializable:
_EXLUDE_VARS: t.ClassVar[tuple[str, ...]] = ("self", "extra_fields", "extra")
extra: dict[str, t.Any]

@classmethod
def _supported_fields(cls) -> list[str]:
signature = inspect.signature(cls.__init__)
return [
name for name in signature.parameters.keys() if name not in cls._EXLUDE_VARS
]

@classmethod
def from_dict(cls: type[T], data: dict[str, t.Any]) -> T:
"""
Instantiate from a dictionary.
:param data: The dictionary to create the error from.
:type data: dict
"""

# Extract any extra fields
extras = {k: v for k, v in data.items() if k not in cls._supported_fields()}
kwargs: dict[str, t.Any] = {"extra": extras}
# Ensure required fields are supplied
for field_name in cls._supported_fields():
kwargs[field_name] = data.get(field_name)

return cls(**kwargs)

def to_dict(self, include_extra: bool = False) -> dict[str, t.Any]:
"""
Render to a dictionary.
:param include_extra: Whether to include stored extra (non-standard) fields in
the returned dictionary.
:type include_extra: bool
"""
result = {}

# Set any authorization parameters
for field in self._supported_fields():
value = getattr(self, field)
if value is not None:
if isinstance(value, Serializable):
value = value.to_dict(include_extra=include_extra)
result[field] = value

# Set any extra fields
if include_extra:
result.update(self.extra)

return result
77 changes: 77 additions & 0 deletions src/globus_sdk/experimental/auth_requirements_error/_validators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
from __future__ import annotations

import sys
import typing as t

from ._serializable import Serializable

if sys.version_info >= (3, 8):
from typing import Literal
else:
from typing_extensions import Literal

S = t.TypeVar("S", bound=Serializable)


class ValidationError(ValueError):
pass


def str_(name: str, value: t.Any) -> str:
if not isinstance(value, str):
raise ValidationError(f"'{name}' must be a string")
return value


def opt_str(name: str, value: t.Any) -> str | None:
if value is None:
return None
if not isinstance(value, str):
raise ValidationError(f"'{name}' must be a string")
return value


def consent_required_literal(name: str, value: t.Any) -> Literal["ConsentRequired"]:
if not isinstance(value, str) or value != "ConsentRequired":
raise ValidationError(f"'{name}' must be the string 'ConsentRequired'")
return t.cast(Literal["ConsentRequired"], value)


def opt_bool(name: str, value: t.Any) -> bool | None:
if value is None:
return None
if not isinstance(value, bool):
raise ValidationError(f"'{name}' must be a bool")
return value


def str_list(name: str, value: t.Any) -> list[str]:
if not (isinstance(value, list) and all(isinstance(s, str) for s in value)):
raise ValidationError(f"'{name}' must be a list of strings")
return value


def opt_str_list(name: str, value: t.Any) -> list[str] | None:
if value is None:
return None
if not (isinstance(value, list) and all(isinstance(s, str) for s in value)):
raise ValidationError(f"'{name}' must be a list of strings")
return value


def opt_str_list_or_commasep(name: str, value: t.Any) -> list[str] | None:
if value is None:
return None
if isinstance(value, str):
value = value.split(",")
if not (isinstance(value, list) and all(isinstance(s, str) for s in value)):
raise ValidationError(f"'{name}' must be a list of strings")
return value


def instance_or_dict(name: str, value: t.Any, cls: type[S]) -> S:
if isinstance(value, cls):
return value
if isinstance(value, dict):
return cls.from_dict(value)
raise ValidationError(f"'{name}' must be a '{cls.__name__}' object or a dictionary")
Loading

0 comments on commit 5dab4bf

Please sign in to comment.