From c42389ebb4d2cd89470462f56fa57fa9c8cf5222 Mon Sep 17 00:00:00 2001 From: pgjones Date: Thu, 25 Aug 2022 11:40:24 +0100 Subject: [PATCH] Switch from Sentinel types to Enums The latter are much easier to work with when type hinting and can be used successfully with mypyc, whereas the former are sadly very difficult in both aspects. This loses the nice property of `type(NEED_DATA) is NEED_DATA` (as expanded on in the deleted docs section). However, I don't think this is widely used in practice. --- docs/source/api.rst | 24 +------- docs/source/changes.rst | 6 +- h11/_connection.py | 45 +++++++------- h11/_readers.py | 6 +- h11/_state.py | 110 ++++++++++++++++++----------------- h11/_util.py | 29 --------- h11/_writers.py | 6 +- h11/tests/helpers.py | 5 +- h11/tests/test_connection.py | 5 +- h11/tests/test_util.py | 20 ------- 10 files changed, 98 insertions(+), 158 deletions(-) diff --git a/docs/source/api.rst b/docs/source/api.rst index 4b6798e..d774102 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -365,29 +365,7 @@ from :meth:`Connection.next_event`: .. data:: NEED_DATA PAUSED -All of these behave the same, and their behavior is modeled after -:data:`None`: they're opaque singletons, their :meth:`__repr__` is -their name, and you compare them with ``is``. - -.. _sentinel-type-trickiness: - -Finally, h11's constants have a quirky feature that can sometimes be -useful: they are instances of themselves. - -.. ipython:: python - - type(h11.NEED_DATA) is h11.NEED_DATA - type(h11.PAUSED) is h11.PAUSED - -The main application of this is that when handling the return value -from :meth:`Connection.next_event`, which is sometimes an instance of -an event class and sometimes :data:`NEED_DATA` or :data:`PAUSED`, you -can always call ``type(event)`` to get something useful to dispatch -one, using e.g. a handler table, :func:`functools.singledispatch`, or -calling ``getattr(some_object, "handle_" + -type(event).__name__)``. Not that this kind of dispatch-based strategy -is always the best approach -- but the option is there if you want it. - +These special constants are part of a ``PseudoEvent`` enum. The Connection object --------------------- diff --git a/docs/source/changes.rst b/docs/source/changes.rst index aecdce5..9b4c453 100644 --- a/docs/source/changes.rst +++ b/docs/source/changes.rst @@ -181,9 +181,9 @@ v0.7.0 (2016-11-25) New features (backwards compatible): -* Made it so that sentinels are :ref:`instances of themselves - `, to enable certain dispatch tricks on - the return value of :func:`Connection.next_event` (see `issue #8 +* Made it so that sentinels are instances of themselves, to enable + certain dispatch tricks on the return value of + :func:`Connection.next_event` (see `issue #8 `__ for discussion). * Added :data:`Data.chunk_start` and :data:`Data.chunk_end` properties diff --git a/h11/_connection.py b/h11/_connection.py index d175270..d8838b1 100644 --- a/h11/_connection.py +++ b/h11/_connection.py @@ -1,5 +1,6 @@ # This contains the main Connection class. Everything in h11 revolves around # this. +from enum import auto, Enum from typing import Any, Callable, cast, Dict, List, Optional, Tuple, Type, Union from ._events import ( @@ -22,27 +23,29 @@ DONE, ERROR, MIGHT_SWITCH_PROTOCOL, + Role, SEND_BODY, SERVER, + State, SWITCHED_PROTOCOL, + SwitchState, + SwitchType, ) -from ._util import ( # Import the internal things we need - LocalProtocolError, - RemoteProtocolError, - Sentinel, -) +from ._util import LocalProtocolError # Import the internal things we need +from ._util import RemoteProtocolError from ._writers import WRITERS, WritersType # Everything in __all__ gets re-exported as part of the h11 public API. __all__ = ["Connection", "NEED_DATA", "PAUSED"] -class NEED_DATA(Sentinel, metaclass=Sentinel): - pass +class PseudoEvent(Enum): + NEED_DATA = auto() + PAUSED = auto() -class PAUSED(Sentinel, metaclass=Sentinel): - pass +NEED_DATA = PseudoEvent.NEED_DATA +PAUSED = PseudoEvent.PAUSED # If we ever have this much buffered without it making a complete parseable @@ -154,7 +157,7 @@ class Connection: def __init__( self, - our_role: Type[Sentinel], + our_role: Role, max_incomplete_event_size: int = DEFAULT_MAX_INCOMPLETE_EVENT_SIZE, ) -> None: self._max_incomplete_event_size = max_incomplete_event_size @@ -162,7 +165,7 @@ def __init__( if our_role not in (CLIENT, SERVER): raise ValueError("expected CLIENT or SERVER, not {!r}".format(our_role)) self.our_role = our_role - self.their_role: Type[Sentinel] + self.their_role: Role if our_role is CLIENT: self.their_role = SERVER else: @@ -192,7 +195,7 @@ def __init__( self.client_is_waiting_for_100_continue = False @property - def states(self) -> Dict[Type[Sentinel], Type[Sentinel]]: + def states(self) -> Dict[Role, Union[State, SwitchState]]: """A dictionary like:: {CLIENT: , SERVER: } @@ -203,14 +206,14 @@ def states(self) -> Dict[Type[Sentinel], Type[Sentinel]]: return dict(self._cstate.states) @property - def our_state(self) -> Type[Sentinel]: + def our_state(self) -> Union[State, SwitchState]: """The current state of whichever role we are playing. See :ref:`state-machine` for details. """ return self._cstate.states[self.our_role] @property - def their_state(self) -> Type[Sentinel]: + def their_state(self) -> Union[State, SwitchState]: """The current state of whichever role we are NOT playing. See :ref:`state-machine` for details. """ @@ -240,12 +243,12 @@ def start_next_cycle(self) -> None: assert not self.client_is_waiting_for_100_continue self._respond_to_state_changes(old_states) - def _process_error(self, role: Type[Sentinel]) -> None: + def _process_error(self, role: Role) -> None: old_states = dict(self._cstate.states) self._cstate.process_error(role) self._respond_to_state_changes(old_states) - def _server_switch_event(self, event: Event) -> Optional[Type[Sentinel]]: + def _server_switch_event(self, event: Event) -> Optional[SwitchType]: if type(event) is InformationalResponse and event.status_code == 101: return _SWITCH_UPGRADE if type(event) is Response: @@ -257,7 +260,7 @@ def _server_switch_event(self, event: Event) -> Optional[Type[Sentinel]]: return None # All events go through here - def _process_event(self, role: Type[Sentinel], event: Event) -> None: + def _process_event(self, role: Role, event: Event) -> None: # First, pass the event through the state machine to make sure it # succeeds. old_states = dict(self._cstate.states) @@ -307,7 +310,7 @@ def _process_event(self, role: Type[Sentinel], event: Event) -> None: def _get_io_object( self, - role: Type[Sentinel], + role: Role, event: Optional[Event], io_dict: Union[ReadersType, WritersType], ) -> Optional[Callable[..., Any]]: @@ -323,13 +326,13 @@ def _get_io_object( else: # General case: the io_dict just has the appropriate reader/writer # for this state - return io_dict.get((role, state)) # type: ignore[return-value] + return io_dict.get((role, state)) # type: ignore[arg-type, return-value] # This must be called after any action that might have caused # self._cstate.states to change. def _respond_to_state_changes( self, - old_states: Dict[Type[Sentinel], Type[Sentinel]], + old_states: Dict[Role, Union[State, SwitchState]], event: Optional[Event] = None, ) -> None: # Update reader/writer @@ -423,7 +426,7 @@ def _extract_next_receive_event( event = NEED_DATA return event # type: ignore[no-any-return] - def next_event(self) -> Union[Event, Type[NEED_DATA], Type[PAUSED]]: + def next_event(self) -> Union[Event, PseudoEvent]: """Parse the next event out of our receive buffer, update our internal state, and return it. diff --git a/h11/_readers.py b/h11/_readers.py index 08a9574..9f4fcef 100644 --- a/h11/_readers.py +++ b/h11/_readers.py @@ -28,11 +28,13 @@ DONE, IDLE, MUST_CLOSE, + Role, SEND_BODY, SEND_RESPONSE, SERVER, + State, ) -from ._util import LocalProtocolError, RemoteProtocolError, Sentinel, validate +from ._util import LocalProtocolError, RemoteProtocolError, validate __all__ = ["READERS"] @@ -225,7 +227,7 @@ def expect_nothing(buf: ReceiveBuffer) -> None: ReadersType = Dict[ - Union[Type[Sentinel], Tuple[Type[Sentinel], Type[Sentinel]]], + Union[State, Tuple[Role, State]], Union[Callable[..., Any], Dict[str, Callable[..., Any]]], ] diff --git a/h11/_state.py b/h11/_state.py index 3593430..cad3e27 100644 --- a/h11/_state.py +++ b/h11/_state.py @@ -110,10 +110,11 @@ # tables. But it can't automatically read the transitions that are written # directly in Python code. So if you touch those, you need to also update the # script to keep it in sync! +from enum import auto, Enum from typing import cast, Dict, Optional, Set, Tuple, Type, Union from ._events import * -from ._util import LocalProtocolError, Sentinel +from ._util import LocalProtocolError # Everything in __all__ gets re-exported as part of the h11 public API. __all__ = [ @@ -131,65 +132,64 @@ ] -class CLIENT(Sentinel, metaclass=Sentinel): - pass +class Role(Enum): + CLIENT = auto() + SERVER = auto() -class SERVER(Sentinel, metaclass=Sentinel): - pass +CLIENT = Role.CLIENT +SERVER = Role.SERVER -# States -class IDLE(Sentinel, metaclass=Sentinel): - pass - - -class SEND_RESPONSE(Sentinel, metaclass=Sentinel): - pass - - -class SEND_BODY(Sentinel, metaclass=Sentinel): - pass - +class State(Enum): + IDLE = auto() + SEND_RESPONSE = auto() + SEND_BODY = auto() + DONE = auto() + MUST_CLOSE = auto() + CLOSED = auto() + ERROR = auto() -class DONE(Sentinel, metaclass=Sentinel): - pass - -class MUST_CLOSE(Sentinel, metaclass=Sentinel): - pass - - -class CLOSED(Sentinel, metaclass=Sentinel): - pass - - -class ERROR(Sentinel, metaclass=Sentinel): - pass +# States +IDLE = State.IDLE +SEND_RESPONSE = State.SEND_RESPONSE +SEND_BODY = State.SEND_BODY +DONE = State.DONE +MUST_CLOSE = State.MUST_CLOSE +CLOSED = State.CLOSED +ERROR = State.ERROR -# Switch types -class MIGHT_SWITCH_PROTOCOL(Sentinel, metaclass=Sentinel): - pass +class SwitchState(Enum): + MIGHT_SWITCH_PROTOCOL = auto() + SWITCHED_PROTOCOL = auto() -class SWITCHED_PROTOCOL(Sentinel, metaclass=Sentinel): - pass +# Switch states +MIGHT_SWITCH_PROTOCOL = SwitchState.MIGHT_SWITCH_PROTOCOL +SWITCHED_PROTOCOL = SwitchState.SWITCHED_PROTOCOL -class _SWITCH_UPGRADE(Sentinel, metaclass=Sentinel): - pass +class SwitchType(Enum): + UPGRADE = auto() + CONNECT = auto() -class _SWITCH_CONNECT(Sentinel, metaclass=Sentinel): - pass +_SWITCH_UPGRADE = SwitchType.UPGRADE +_SWITCH_CONNECT = SwitchType.CONNECT EventTransitionType = Dict[ - Type[Sentinel], + Role, Dict[ - Type[Sentinel], - Dict[Union[Type[Event], Tuple[Type[Event], Type[Sentinel]]], Type[Sentinel]], + Union[State, SwitchState], + Dict[ + Union[ + Type[Event], Tuple[Type[Event], Role], Tuple[Type[Event], SwitchType] + ], + Union[State, SwitchState], + ], ], ] @@ -227,7 +227,8 @@ class _SWITCH_CONNECT(Sentinel, metaclass=Sentinel): } StateTransitionType = Dict[ - Tuple[Type[Sentinel], Type[Sentinel]], Dict[Type[Sentinel], Type[Sentinel]] + Tuple[Union[State, SwitchState], Union[State, SwitchState]], + Dict[Role, Union[State, SwitchState]], ] # NB: there are also some special-case state-triggered transitions hard-coded @@ -256,11 +257,14 @@ def __init__(self) -> None: # This is a subset of {UPGRADE, CONNECT}, containing the proposals # made by the client for switching protocols. - self.pending_switch_proposals: Set[Type[Sentinel]] = set() + self.pending_switch_proposals: Set[SwitchType] = set() - self.states: Dict[Type[Sentinel], Type[Sentinel]] = {CLIENT: IDLE, SERVER: IDLE} + self.states: Dict[Role, Union[State, SwitchState]] = { + CLIENT: IDLE, + SERVER: IDLE, + } - def process_error(self, role: Type[Sentinel]) -> None: + def process_error(self, role: Role) -> None: self.states[role] = ERROR self._fire_state_triggered_transitions() @@ -268,17 +272,17 @@ def process_keep_alive_disabled(self) -> None: self.keep_alive = False self._fire_state_triggered_transitions() - def process_client_switch_proposal(self, switch_event: Type[Sentinel]) -> None: + def process_client_switch_proposal(self, switch_event: SwitchType) -> None: self.pending_switch_proposals.add(switch_event) self._fire_state_triggered_transitions() def process_event( self, - role: Type[Sentinel], + role: Role, event_type: Type[Event], - server_switch_event: Optional[Type[Sentinel]] = None, + server_switch_event: Optional[SwitchType] = None, ) -> None: - _event_type: Union[Type[Event], Tuple[Type[Event], Type[Sentinel]]] = event_type + _event_type: Union[Type[Event], Tuple[Type[Event], SwitchType]] = event_type if server_switch_event is not None: assert role is SERVER if server_switch_event not in self.pending_switch_proposals: @@ -300,8 +304,10 @@ def process_event( def _fire_event_triggered_transitions( self, - role: Type[Sentinel], - event_type: Union[Type[Event], Tuple[Type[Event], Type[Sentinel]]], + role: Role, + event_type: Union[ + Type[Event], Tuple[Type[Event], SwitchType], Tuple[Type[Event], Role] + ], ) -> None: state = self.states[role] try: diff --git a/h11/_util.py b/h11/_util.py index 6718445..c72a6e4 100644 --- a/h11/_util.py +++ b/h11/_util.py @@ -92,35 +92,6 @@ def validate( return match.groupdict() -# Sentinel values -# -# - Inherit identity-based comparison and hashing from object -# - Have a nice repr -# - Have a *bonus property*: type(sentinel) is sentinel -# -# The bonus property is useful if you want to take the return value from -# next_event() and do some sort of dispatch based on type(event). - -_T_Sentinel = TypeVar("_T_Sentinel", bound="Sentinel") - - -class Sentinel(type): - def __new__( - cls: Type[_T_Sentinel], - name: str, - bases: Tuple[type, ...], - namespace: Dict[str, Any], - **kwds: Any - ) -> _T_Sentinel: - assert bases == (Sentinel,) - v = super().__new__(cls, name, bases, namespace, **kwds) - v.__class__ = v # type: ignore - return v - - def __repr__(self) -> str: - return self.__name__ - - # Used for methods, request targets, HTTP versions, header names, and header # values. Accepts ascii-strings, or bytes/bytearray/memoryview/..., and always # returns bytes. diff --git a/h11/_writers.py b/h11/_writers.py index 939cdb9..c4d803a 100644 --- a/h11/_writers.py +++ b/h11/_writers.py @@ -11,8 +11,8 @@ from ._events import Data, EndOfMessage, Event, InformationalResponse, Request, Response from ._headers import Headers -from ._state import CLIENT, IDLE, SEND_BODY, SEND_RESPONSE, SERVER -from ._util import LocalProtocolError, Sentinel +from ._state import CLIENT, IDLE, Role, SEND_BODY, SEND_RESPONSE, SERVER, State +from ._util import LocalProtocolError __all__ = ["WRITERS"] @@ -125,7 +125,7 @@ def send_eom(self, headers: Headers, write: Writer) -> None: WritersType = Dict[ - Union[Tuple[Type[Sentinel], Type[Sentinel]], Type[Sentinel]], + Union[Tuple[Role, State], State], Union[ Dict[str, Type[BodyWriter]], Callable[[Union[InformationalResponse, Response], Writer], None], diff --git a/h11/tests/helpers.py b/h11/tests/helpers.py index 571be44..d9dd60e 100644 --- a/h11/tests/helpers.py +++ b/h11/tests/helpers.py @@ -10,8 +10,7 @@ Request, Response, ) -from .._state import CLIENT, CLOSED, DONE, MUST_CLOSE, SERVER -from .._util import Sentinel +from .._state import CLIENT, CLOSED, DONE, MUST_CLOSE, Role, SERVER try: from typing import Literal @@ -71,7 +70,7 @@ def conns(self) -> ValuesView[Connection]: # expect="match" if expect=send_events; expect=[...] to say what expected def send( self, - role: Type[Sentinel], + role: Role, send_events: Union[List[Event], Event], expect: Union[List[Event], Event, Literal["match"]] = "match", ) -> bytes: diff --git a/h11/tests/test_connection.py b/h11/tests/test_connection.py index 73a27b9..f30bf8a 100644 --- a/h11/tests/test_connection.py +++ b/h11/tests/test_connection.py @@ -20,12 +20,13 @@ IDLE, MIGHT_SWITCH_PROTOCOL, MUST_CLOSE, + Role, SEND_BODY, SEND_RESPONSE, SERVER, SWITCHED_PROTOCOL, ) -from .._util import LocalProtocolError, RemoteProtocolError, Sentinel +from .._util import LocalProtocolError, RemoteProtocolError from .helpers import ConnectionPair, get_all_events, receive_and_get @@ -926,7 +927,7 @@ def test_errors() -> None: # After an error sending, you can no longer send # (This is especially important for things like content-length errors, # where there's complex internal state being modified) - def conn(role: Type[Sentinel]) -> Connection: + def conn(role: Role) -> Connection: c = Connection(our_role=role) if role is SERVER: # Put it into the state where it *could* send a response... diff --git a/h11/tests/test_util.py b/h11/tests/test_util.py index 79bc095..b730d30 100644 --- a/h11/tests/test_util.py +++ b/h11/tests/test_util.py @@ -10,7 +10,6 @@ LocalProtocolError, ProtocolError, RemoteProtocolError, - Sentinel, validate, ) @@ -81,25 +80,6 @@ def test_validate_formatting() -> None: assert "oops 10 xx" in str(excinfo.value) -def test_make_sentinel() -> None: - class S(Sentinel, metaclass=Sentinel): - pass - - assert repr(S) == "S" - assert S == S - assert type(S).__name__ == "S" - assert S in {S} - assert type(S) is S - - class S2(Sentinel, metaclass=Sentinel): - pass - - assert repr(S2) == "S2" - assert S != S2 - assert S not in {S2} - assert type(S) is not type(S2) - - def test_bytesify() -> None: assert bytesify(b"123") == b"123" assert bytesify(bytearray(b"123")) == b"123"