From 35c44ef59ff7fe4049b3fbe2a68f552e77eb3de0 Mon Sep 17 00:00:00 2001 From: Josh Caponigro Date: Wed, 19 Aug 2026 22:45:52 -0500 Subject: [PATCH 1/3] Add ruff and mypy config, fix all lint and type findings --- .vscode/settings.json | 7 +++++ __init__.py | 0 apis/devices.py | 16 +++++----- apis/push.py | 6 ++-- database.py | 3 +- entities/__init__.py | 2 ++ entities/device_entity.py | 25 +++++++++------ models/__init__.py | 2 ++ push/__init__.py | 2 ++ push/apn_handler/__init__.py | 2 ++ push/apn_handler/client.py | 17 ++++------ push/apn_handler/credentials.py | 8 ++--- push/apn_handler/errors.py | 4 +-- push/apn_handler/payload.py | 4 +-- push/handler.py | 33 ++++++++++--------- pyproject.toml | 39 +++++++++++++++++++++++ services/__init__.py | 2 ++ services/device_service.py | 4 +-- services/push_service.py | 19 +++++------ tests/conftest.py | 4 +-- tests/test_apns_client.py | 56 +++++++++------------------------ tests/test_device_api.py | 12 ++----- tests/test_push_api.py | 8 ++--- tests/test_push_handler.py | 8 ++--- utils/__init__.py | 2 ++ 25 files changed, 149 insertions(+), 136 deletions(-) create mode 100644 .vscode/settings.json delete mode 100644 __init__.py create mode 100644 pyproject.toml diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..9b38853 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,7 @@ +{ + "python.testing.pytestArgs": [ + "tests" + ], + "python.testing.unittestEnabled": false, + "python.testing.pytestEnabled": true +} \ No newline at end of file diff --git a/__init__.py b/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/apis/devices.py b/apis/devices.py index 4dab17a..2e28868 100644 --- a/apis/devices.py +++ b/apis/devices.py @@ -11,42 +11,42 @@ @router.post("/register", response_model=Device) -def register_device(device: Device, deviceService: DeviceService = Depends()): +def register_device(device: Device, device_service: DeviceService = Depends()): """ Registers a new device with the push notification framework. Args: device (Device): The device to register. - deviceService (DeviceService): An instance of the DeviceService class. Injected by FastAPI. + device_service (DeviceService): An instance of the DeviceService class. Injected by FastAPI. Returns: Device: The registered device. """ - return deviceService.register_device(device) + return device_service.register_device(device) @router.get("/all", response_model=list[Device]) def get_registered_devices( - deviceService: DeviceService = Depends(), + device_service: DeviceService = Depends(), ): """ Retrieve a list of all registered devices. Args: - deviceService (DeviceService): An instance of the DeviceService class. Injected by FastAPI. + device_service (DeviceService): An instance of the DeviceService class. Injected by FastAPI. Returns: list[Device]: A list of Device objects representing all registered devices. """ - return deviceService.get_registered_devices() + return device_service.get_registered_devices() # FOR TESTING PURPOSES ONLY @router.get("/clear", response_model=None) def clear_registered_devices( - deviceService: DeviceService = Depends(), + device_service: DeviceService = Depends(), ): """ Clears all registered devices from the device service. """ - return deviceService.clear_registered_devices() + return device_service.clear_registered_devices() diff --git a/apis/push.py b/apis/push.py index 7101633..3da1bce 100644 --- a/apis/push.py +++ b/apis/push.py @@ -11,16 +11,16 @@ @router.post("/send", response_model=dict[str, str]) -def send_push(message: Message, pushService: PushService = Depends()): +def send_push(message: Message, push_service: PushService = Depends()): """ Sends a push notification to each recipient. Args: message (Message): The message to send. - pushService (PushService): The push service to use. Injected by FastAPI. + push_service (PushService): The push service to use. Injected by FastAPI. Returns: dict[str, str]: A mapping of each device token to "Success" or the APNs failure reason. """ - return pushService.send_push(message) + return push_service.send_push(message) diff --git a/database.py b/database.py index ef68dac..6a4e83d 100644 --- a/database.py +++ b/database.py @@ -11,7 +11,8 @@ def _engine_str(name: str = getenv("DB_NAME")) -> str: Helper function for reading settings from environment variables to produce connection string. Arguments: - name (str): The name of the database. Defaults to the value of the "DB_NAME" environment variable. + name (str): The name of the database. Defaults to the value of the + "DB_NAME" environment variable. Returns: str: The connection string for the database. diff --git a/entities/__init__.py b/entities/__init__.py index 09ca219..ac6b962 100644 --- a/entities/__init__.py +++ b/entities/__init__.py @@ -4,3 +4,5 @@ from .device_entity import DeviceEntity from .entity_base import EntityBase + +__all__ = ["DeviceEntity", "EntityBase"] diff --git a/entities/device_entity.py b/entities/device_entity.py index f5c1706..ca366fa 100644 --- a/entities/device_entity.py +++ b/entities/device_entity.py @@ -1,4 +1,7 @@ -from sqlalchemy import Column, DateTime, Integer, String, func +from datetime import datetime + +from sqlalchemy import DateTime, String, func +from sqlalchemy.orm import Mapped, mapped_column from models import Device @@ -23,15 +26,17 @@ class DeviceEntity(EntityBase): __tablename__ = "devices" - id = Column(Integer, primary_key=True, index=True) - token = Column(String(255), nullable=False, unique=True) - name = Column(String(255), nullable=True) - systemName = Column(String(255), nullable=True) - systemVersion = Column(String(255), nullable=True) - model = Column(String(255), nullable=True) - localizedModel = Column(String(255), nullable=True) - created_at = Column(DateTime, default=func.now()) - updated_at = Column(DateTime, default=func.now(), onupdate=func.now()) + id: Mapped[int] = mapped_column(primary_key=True, index=True) + token: Mapped[str] = mapped_column(String(255), unique=True) + name: Mapped[str | None] = mapped_column(String(255)) + systemName: Mapped[str | None] = mapped_column(String(255)) + systemVersion: Mapped[str | None] = mapped_column(String(255)) + model: Mapped[str | None] = mapped_column(String(255)) + localizedModel: Mapped[str | None] = mapped_column(String(255)) + created_at: Mapped[datetime | None] = mapped_column(DateTime, default=func.now()) + updated_at: Mapped[datetime | None] = mapped_column( + DateTime, default=func.now(), onupdate=func.now() + ) def to_model(self) -> Device: return Device( diff --git a/models/__init__.py b/models/__init__.py index ce83c28..5ea4f62 100644 --- a/models/__init__.py +++ b/models/__init__.py @@ -4,3 +4,5 @@ from .device import Device from .message import Message + +__all__ = ["Device", "Message"] diff --git a/push/__init__.py b/push/__init__.py index 8e8d36d..a5bf61c 100644 --- a/push/__init__.py +++ b/push/__init__.py @@ -3,3 +3,5 @@ """ from .handler import PushHandler, get_push_handler, shutdown_push_handler + +__all__ = ["PushHandler", "get_push_handler", "shutdown_push_handler"] diff --git a/push/apn_handler/__init__.py b/push/apn_handler/__init__.py index f2c4753..2fb04f6 100644 --- a/push/apn_handler/__init__.py +++ b/push/apn_handler/__init__.py @@ -1,3 +1,5 @@ from .client import APNsClient, Notification from .credentials import Credentials, TokenCredentials from .payload import Payload + +__all__ = ["APNsClient", "Credentials", "Notification", "Payload", "TokenCredentials"] diff --git a/push/apn_handler/client.py b/push/apn_handler/client.py index 95a7658..57c976a 100644 --- a/push/apn_handler/client.py +++ b/push/apn_handler/client.py @@ -51,14 +51,13 @@ def __init__( proxy_port: int | None = None, heartbeat_period: float | None = None, ) -> None: + self.__credentials: Credentials if isinstance(credentials, str): self.__credentials = CertificateCredentials(credentials, password) else: self.__credentials = credentials - self._init_connection( - use_sandbox, use_alternative_port, proto, proxy_host, proxy_port - ) + self._init_connection(use_sandbox, use_alternative_port, proto, proxy_host, proxy_port) if heartbeat_period: raise NotImplementedError("heartbeat not supported") @@ -68,9 +67,7 @@ def __init__( # APNs expects providers to keep connections open across requests; # opening one per notification is treated as abusive by Apple. ssl_context = self.__credentials.ssl_context - self.__http_client = httpx.Client( - http2=True, verify=ssl_context if ssl_context else True - ) + self.__http_client = httpx.Client(http2=True, verify=ssl_context if ssl_context else True) def _init_connection( self, @@ -81,9 +78,7 @@ def _init_connection( proxy_port: int | None, ) -> None: self.__server = self.SANDBOX_SERVER if use_sandbox else self.LIVE_SERVER - self.__port = ( - self.ALTERNATIVE_PORT if use_alternative_port else self.DEFAULT_PORT - ) + self.__port = self.ALTERNATIVE_PORT if use_alternative_port else self.DEFAULT_PORT def send_notification( self, @@ -128,7 +123,7 @@ def send_notification_sync( headers = {} - inferred_push_type = None # type: Optional[str] + inferred_push_type: str | None = None if topic is not None: headers["apns-topic"] = topic if topic.endswith(".voip"): @@ -158,7 +153,7 @@ def send_notification_sync( headers["apns-priority"] = priority.value if expiration is not None: - headers["apns-expiration"] = "%d" % expiration + headers["apns-expiration"] = str(expiration) if isinstance(self.__credentials, TokenCredentials): auth_header = self.__credentials.get_authorization_header(topic) diff --git a/push/apn_handler/credentials.py b/push/apn_handler/credentials.py index 78efa25..1c4e222 100644 --- a/push/apn_handler/credentials.py +++ b/push/apn_handler/credentials.py @@ -19,9 +19,7 @@ def get_authorization_header(self, topic: str | None) -> str | None: # Credentials subclass for certificate authentication class CertificateCredentials(Credentials): - def __init__( - self, cert_file: str | None = None, password: str | None = None - ) -> None: + def __init__(self, cert_file: str, password: str | None = None) -> None: ssl_context = ssl.create_default_context() ssl_context.load_cert_chain(cert_file, password=password) super().__init__(ssl_context) @@ -43,14 +41,14 @@ def __init__( self.__encryption_algorithm = encryption_algorithm self.__token_lifetime = token_lifetime - self.__jwt_token = None # type: Optional[Tuple[float, str]] + self.__jwt_token: tuple[float, str] | None = None # Use the default constructor because we don't have an SSL context super().__init__() def get_authorization_header(self, topic: str | None) -> str: token = self._get_or_create_topic_token() - return "bearer %s" % token + return f"bearer {token}" def _is_expired_token(self, issue_date: float) -> bool: return time.time() > issue_date + self.__token_lifetime diff --git a/push/apn_handler/errors.py b/push/apn_handler/errors.py index 70f7ee3..3889b05 100644 --- a/push/apn_handler/errors.py +++ b/push/apn_handler/errors.py @@ -92,8 +92,8 @@ class InvalidProviderToken(APNsException): class MissingProviderToken(APNsException): - """No provider certificate was used to connect to APNs and Authorization header was missing or no provider token - was specified.""" + """No provider certificate was used to connect to APNs and Authorization + header was missing or no provider token was specified.""" class BadPath(APNsException): diff --git a/push/apn_handler/payload.py b/push/apn_handler/payload.py index 07465b3..0fdf6d4 100644 --- a/push/apn_handler/payload.py +++ b/push/apn_handler/payload.py @@ -34,7 +34,7 @@ def __init__( self.launch_image = launch_image def dict(self) -> dict[str, Any]: - result = {} # type: Dict[str, Any] + result: dict[str, Any] = {} if self.title: result["title"] = self.title @@ -92,7 +92,7 @@ def __init__( self.thread_id = thread_id def dict(self) -> dict[str, Any]: - result = {"aps": {}} # type: Dict[str, Any] + result: dict[str, Any] = {"aps": {}} if self.alert is not None: if isinstance(self.alert, PayloadAlert): diff --git a/push/handler.py b/push/handler.py index a834879..e301ee8 100644 --- a/push/handler.py +++ b/push/handler.py @@ -9,14 +9,14 @@ class PushHandler: A wrapper class for sending push notifications using the Apple Push Notification service (APNs). Attributes: - token_credentials (TokenCredentials): A dictionary containing the token credentials required to connect to APNs. - connection (APNsClient): An instance of the APNsClient class used to establish a connection to APNs. + token_credentials (TokenCredentials): The token credentials required to + connect to APNs. + connection (APNsClient): An instance of the APNsClient class used to + establish a connection to APNs. Methods: - send_push(to_device_token: str, body: str, sound: str = "default", badge: int = 1) -> None: - Sends a push notification to a single device token. - send_multiple_push(to_device_tokens: list[str], body: str, sound: str = "default", badge: int = 1) -> None: - Sends a push notification to multiple device tokens. + send_push: Sends a push notification to a single device token. + send_multiple_push: Sends a push notification to multiple device tokens. """ def __init__(self): @@ -39,10 +39,13 @@ def send_push( Sends a push notification to a single device token. Args: - to_device_token (str): The device token of the device to send the push notification to. + to_device_token (str): The device token of the device to send the + push notification to. body (str): The message body of the push notification. - sound (str, optional): The name of the sound to play when the push notification is received. Defaults to "default". - badge (int, optional): The number to display as the badge of the app icon. Defaults to 1. + sound (str, optional): The name of the sound to play when the push + notification is received. Defaults to "default". + badge (int, optional): The number to display as the badge of the + app icon. Defaults to 1. """ payload: Payload = Payload(alert=body, sound=sound, badge=badge) self.connection.send_notification( @@ -62,10 +65,13 @@ def send_multiple_push( A failure for one token does not prevent delivery to the others. Args: - to_device_tokens (list[str]): A list of device tokens to send the push notification to. + to_device_tokens (list[str]): A list of device tokens to send the + push notification to. body (str): The message body of the push notification. - sound (str, optional): The name of the sound to play when the push notification is received. Defaults to "default". - badge (int, optional): The number to display as the badge of the app icon. Defaults to 1. + sound (str, optional): The name of the sound to play when the push + notification is received. Defaults to "default". + badge (int, optional): The number to display as the badge of the + app icon. Defaults to 1. Returns: dict[str, str]: A mapping of each device token to "Success" or the @@ -73,8 +79,7 @@ def send_multiple_push( """ payload: Payload = Payload(alert=body, sound=sound, badge=badge) notifications = [ - Notification(token=token, payload=payload) - for token in dict.fromkeys(to_device_tokens) + Notification(token=token, payload=payload) for token in dict.fromkeys(to_device_tokens) ] return self.connection.send_notification_batch( notifications, topic=PushConfig.get_apns_app_bundle_id() diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..8849a79 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,39 @@ +[tool.ruff] +target-version = "py311" +line-length = 100 + +[tool.ruff.lint] +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "N", # pep8-naming + "UP", # pyupgrade + "B", # flake8-bugbear + "SIM", # flake8-simplify + "C4", # flake8-comprehensions +] +# Model fields mirror Apple's UIDevice camelCase property names +ignore = ["N815"] + +[tool.ruff.lint.flake8-bugbear] +# Depends() in argument defaults is the FastAPI injection idiom +extend-immutable-calls = ["fastapi.Depends"] + +[tool.ruff.lint.per-file-ignores] +# Environment variables must be set before application modules are imported +"tests/conftest.py" = ["E402"] +# APNsException is the vendored apns2 package's public exception name +"push/apn_handler/errors.py" = ["N818"] + +[tool.mypy] +python_version = "3.11" +strict_optional = true +warn_unused_ignores = true +warn_redundant_casts = true +check_untyped_defs = true + +[[tool.mypy.overrides]] +module = "tests.*" +disable_error_code = ["method-assign"] diff --git a/services/__init__.py b/services/__init__.py index 7503601..4f55172 100644 --- a/services/__init__.py +++ b/services/__init__.py @@ -4,3 +4,5 @@ from .device_service import DeviceService from .push_service import PushService + +__all__ = ["DeviceService", "PushService"] diff --git a/services/device_service.py b/services/device_service.py index 58826ed..3a7bc9a 100644 --- a/services/device_service.py +++ b/services/device_service.py @@ -92,9 +92,7 @@ def remove_devices(self, tokens: list[str]) -> None: """ if not tokens: return - self._session.execute( - delete(DeviceEntity).where(DeviceEntity.token.in_(tokens)) - ) + self._session.execute(delete(DeviceEntity).where(DeviceEntity.token.in_(tokens))) self._session.commit() def clear_registered_devices(self) -> None: diff --git a/services/push_service.py b/services/push_service.py index a8eaeee..a9f2113 100644 --- a/services/push_service.py +++ b/services/push_service.py @@ -17,7 +17,8 @@ class PushService: Attributes: handler (PushHandler): The handler used to send push notifications. Injected by FastAPI. - deviceService (DeviceService): Used to remove devices APNs reports as unregistered. Injected by FastAPI. + device_service (DeviceService): Used to remove devices APNs reports as + unregistered. Injected by FastAPI. Methods: send_push(message: Message) -> dict[str, str]: Sends a push notification. @@ -26,17 +27,17 @@ class PushService: def __init__( self, handler: PushHandler = Depends(get_push_handler), - deviceService: DeviceService = Depends(), + device_service: DeviceService = Depends(), ): """ Initialize the PushService. Args: handler (PushHandler): The push notification handler to use. Injected by FastAPI. - deviceService (DeviceService): The device service to use. Injected by FastAPI. + device_service (DeviceService): The device service to use. Injected by FastAPI. """ self.handler = handler - self.deviceService = deviceService + self.device_service = device_service def send_push(self, message: Message) -> dict[str, str]: """ @@ -55,18 +56,14 @@ def send_push(self, message: Message) -> dict[str, str]: results = self.handler.send_multiple_push( to_device_tokens=message.recipients, body=message.body ) - stale_tokens = [ - token for token, result in results.items() if result == "Unregistered" - ] + stale_tokens = [token for token, result in results.items() if result == "Unregistered"] if stale_tokens: # Pruning is best-effort cleanup; the notifications are already # sent, so a database failure here must not turn the completed # push into an apparent failure (a client retry would re-send). try: - self.deviceService.remove_devices(stale_tokens) + self.device_service.remove_devices(stale_tokens) logger.info("Removed unregistered device tokens: %s", stale_tokens) except SQLAlchemyError: - logger.exception( - "Failed to remove unregistered device tokens: %s", stale_tokens - ) + logger.exception("Failed to remove unregistered device tokens: %s", stale_tokens) return results diff --git a/tests/conftest.py b/tests/conftest.py index 9ccff71..8366256 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -75,9 +75,7 @@ def route(request): transport = httpx.MockTransport(route) real_client = httpx.Client - monkeypatch.setattr( - httpx, "Client", lambda **kwargs: real_client(transport=transport) - ) + monkeypatch.setattr(httpx, "Client", lambda **kwargs: real_client(transport=transport)) return PushHandler() return factory diff --git a/tests/test_apns_client.py b/tests/test_apns_client.py index 9ebc837..ccd9859 100644 --- a/tests/test_apns_client.py +++ b/tests/test_apns_client.py @@ -21,9 +21,7 @@ def handler(request): transport = httpx.MockTransport(handler) real_client = httpx.Client - monkeypatch.setattr( - httpx, "Client", lambda **kwargs: real_client(transport=transport) - ) + monkeypatch.setattr(httpx, "Client", lambda **kwargs: real_client(transport=transport)) credentials = TokenCredentials( auth_key_path=KEY_PATH, auth_key_id="TESTKEY123", team_id="TESTTEAM12" @@ -35,9 +33,7 @@ def test_send_notification_succeeds_on_200(monkeypatch): requests = [] client = make_client(monkeypatch, 200, {}, requests) - client.send_notification( - "device-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") request = requests[0] assert request.url.path == "/3/device/device-token" @@ -49,29 +45,21 @@ def test_send_notification_raises_typed_exception_for_apns_reason(monkeypatch): client = make_client(monkeypatch, 400, {"reason": "BadDeviceToken"}) with pytest.raises(BadDeviceToken): - client.send_notification( - "bad-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("bad-token", Payload(alert="hello"), topic="com.example.test") def test_send_notification_raises_unregistered_for_gone_token(monkeypatch): - client = make_client( - monkeypatch, 410, {"reason": "Unregistered", "timestamp": "1700000000"} - ) + client = make_client(monkeypatch, 410, {"reason": "Unregistered", "timestamp": "1700000000"}) with pytest.raises(Unregistered): - client.send_notification( - "stale-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("stale-token", Payload(alert="hello"), topic="com.example.test") def test_send_notification_raises_base_exception_for_unknown_reason(monkeypatch): client = make_client(monkeypatch, 400, {"reason": "SomeFutureReason"}) with pytest.raises(APNsException): - client.send_notification( - "device-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") def test_http_client_is_reused_across_sends(monkeypatch): @@ -96,13 +84,9 @@ def counting_client(**kwargs): def make_raw_client(monkeypatch, status_code, text): - transport = httpx.MockTransport( - lambda request: httpx.Response(status_code, text=text) - ) + transport = httpx.MockTransport(lambda request: httpx.Response(status_code, text=text)) real_client = httpx.Client - monkeypatch.setattr( - httpx, "Client", lambda **kwargs: real_client(transport=transport) - ) + monkeypatch.setattr(httpx, "Client", lambda **kwargs: real_client(transport=transport)) credentials = TokenCredentials( auth_key_path=KEY_PATH, auth_key_id="TESTKEY123", team_id="TESTTEAM12" ) @@ -113,20 +97,14 @@ def test_410_without_json_body_still_reports_unregistered(monkeypatch): client = make_raw_client(monkeypatch, 410, "gone") with pytest.raises(Unregistered): - client.send_notification( - "stale-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("stale-token", Payload(alert="hello"), topic="com.example.test") def test_non_json_error_body_is_not_leaked_as_reason(monkeypatch): - client = make_raw_client( - monkeypatch, 502, "Bad Gateway from some proxy" - ) + client = make_raw_client(monkeypatch, 502, "Bad Gateway from some proxy") with pytest.raises(APNsException) as exc_info: - client.send_notification( - "device-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") assert "" not in str(exc_info.value) @@ -135,9 +113,7 @@ def test_raised_exception_message_carries_the_reason(monkeypatch): client = make_client(monkeypatch, 400, {"reason": "SomeFutureReason"}) with pytest.raises(APNsException) as exc_info: - client.send_notification( - "device-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") assert "SomeFutureReason" in str(exc_info.value) @@ -146,9 +122,7 @@ def test_json_error_body_without_reason_key_maps_to_status_marker(monkeypatch): client = make_client(monkeypatch, 400, {"timestamp": "1700000000"}) with pytest.raises(APNsException) as exc_info: - client.send_notification( - "device-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") assert "HTTPError400" in str(exc_info.value) @@ -157,8 +131,6 @@ def test_non_dict_json_error_body_maps_to_status_marker(monkeypatch): client = make_raw_client(monkeypatch, 503, '"Service Unavailable"') with pytest.raises(APNsException) as exc_info: - client.send_notification( - "device-token", Payload(alert="hello"), topic="com.example.test" - ) + client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") assert "HTTPError503" in str(exc_info.value) diff --git a/tests/test_device_api.py b/tests/test_device_api.py index 1642f74..54d67da 100644 --- a/tests/test_device_api.py +++ b/tests/test_device_api.py @@ -63,12 +63,8 @@ def test_clear_removes_all_registered_devices(client): def test_register_same_token_twice_updates_existing_device(client): - first = client.post( - "/devices/register", json={"token": "abc123", "systemVersion": "17.0"} - ) - second = client.post( - "/devices/register", json={"token": "abc123", "systemVersion": "18.1"} - ) + first = client.post("/devices/register", json={"token": "abc123", "systemVersion": "17.0"}) + second = client.post("/devices/register", json={"token": "abc123", "systemVersion": "18.1"}) assert second.status_code == 200 assert second.json()["id"] == first.json()["id"] @@ -93,9 +89,7 @@ def test_reregistering_with_only_token_preserves_stored_fields(client): def test_concurrent_registration_race_falls_back_to_update(test_engine): with Session(test_engine) as session: - DeviceService(session=session).register_device( - Device(token="abc123", name="winner") - ) + DeviceService(session=session).register_device(Device(token="abc123", name="winner")) with Session(test_engine) as session: service = DeviceService(session=session) diff --git a/tests/test_push_api.py b/tests/test_push_api.py index b102dac..355b15f 100644 --- a/tests/test_push_api.py +++ b/tests/test_push_api.py @@ -69,9 +69,7 @@ def test_send_push_removes_unregistered_devices(client, apns_handler_factory): def test_push_handler_is_shared_across_requests(monkeypatch): transport = httpx.MockTransport(lambda request: httpx.Response(200)) real_client = httpx.Client - monkeypatch.setattr( - httpx, "Client", lambda **kwargs: real_client(transport=transport) - ) + monkeypatch.setattr(httpx, "Client", lambda **kwargs: real_client(transport=transport)) monkeypatch.setattr(push.handler, "_shared_handler", None) first = get_push_handler() @@ -102,9 +100,7 @@ def tracking_client(**kwargs): assert push.handler._shared_handler is None -def test_send_push_with_no_recipients_returns_empty_results( - client, apns_handler_factory -): +def test_send_push_with_no_recipients_returns_empty_results(client, apns_handler_factory): override_handler(apns_handler_factory({})) response = client.post("/push/send", json={"recipients": [], "body": "hello"}) diff --git a/tests/test_push_handler.py b/tests/test_push_handler.py index 1dc34ad..b3c43aa 100644 --- a/tests/test_push_handler.py +++ b/tests/test_push_handler.py @@ -35,9 +35,7 @@ def route(request): transport = httpx.MockTransport(route) real_client = httpx.Client - monkeypatch.setattr( - httpx, "Client", lambda **kwargs: real_client(transport=transport) - ) + monkeypatch.setattr(httpx, "Client", lambda **kwargs: real_client(transport=transport)) return PushHandler(), requests @@ -68,9 +66,7 @@ def route(request): transport = httpx.MockTransport(route) real_client = httpx.Client - monkeypatch.setattr( - httpx, "Client", lambda **kwargs: real_client(transport=transport) - ) + monkeypatch.setattr(httpx, "Client", lambda **kwargs: real_client(transport=transport)) handler = PushHandler() results = handler.send_multiple_push( diff --git a/utils/__init__.py b/utils/__init__.py index 13ae390..534f24f 100644 --- a/utils/__init__.py +++ b/utils/__init__.py @@ -1 +1,3 @@ from .env import getenv + +__all__ = ["getenv"] From fdfe0ddcf7f687dfbd153e84562a469d1f0ec773 Mon Sep 17 00:00:00 2001 From: Josh Caponigro Date: Wed, 19 Aug 2026 22:47:20 -0500 Subject: [PATCH 2/3] Update APNs client to current spec: sandbox hostname, new reasons, new push types --- push/apn_handler/client.py | 24 ++++++++++++++++++++---- push/apn_handler/errors.py | 26 ++++++++++++++++++++++++++ services/push_service.py | 6 +++++- tests/test_apns_client.py | 22 ++++++++++++++++++++++ tests/test_push_api.py | 13 +++++++++++++ tests/test_push_handler.py | 4 ++-- 6 files changed, 88 insertions(+), 7 deletions(-) diff --git a/push/apn_handler/client.py b/push/apn_handler/client.py index 57c976a..33340f4 100644 --- a/push/apn_handler/client.py +++ b/push/apn_handler/client.py @@ -23,6 +23,11 @@ class NotificationType(Enum): Complication = "complication" FileProvider = "fileprovider" MDM = "mdm" + LiveActivity = "liveactivity" + Location = "location" + Widgets = "widgets" + Controls = "controls" + PushToTalk = "pushtotalk" Notification = collections.namedtuple("Notification", ["token", "payload"]) @@ -33,7 +38,7 @@ class NotificationType(Enum): class APNsClient: - SANDBOX_SERVER = "api.development.push.apple.com" + SANDBOX_SERVER = "api.sandbox.push.apple.com" LIVE_SERVER = "api.push.apple.com" DEFAULT_PORT = 443 @@ -126,12 +131,22 @@ def send_notification_sync( inferred_push_type: str | None = None if topic is not None: headers["apns-topic"] = topic - if topic.endswith(".voip"): + if topic.endswith(".voip-ptt"): + inferred_push_type = NotificationType.PushToTalk.value + elif topic.endswith(".voip"): inferred_push_type = NotificationType.VoIP.value elif topic.endswith(".complication"): inferred_push_type = NotificationType.Complication.value elif topic.endswith(".pushkit.fileprovider"): inferred_push_type = NotificationType.FileProvider.value + elif topic.endswith(".push-type.liveactivity"): + inferred_push_type = NotificationType.LiveActivity.value + elif topic.endswith(".location-query"): + inferred_push_type = NotificationType.Location.value + elif topic.endswith(".push-type.widgets"): + inferred_push_type = NotificationType.Widgets.value + elif topic.endswith(".push-type.controls"): + inferred_push_type = NotificationType.Controls.value elif any( [ notification.alert is not None, @@ -172,8 +187,9 @@ def _extract_reason(response: httpx.Response) -> str: """Extract the 'reason' field from an APNs error response body. Bodies without a parseable reason (e.g. from an intermediary proxy) are - never returned verbatim: a 410 is always Unregistered per the APNs spec, - and anything else is reduced to a generic status marker. + never returned verbatim: per the APNs spec a 410 always means the token + is gone, so it maps to Unregistered; anything else is reduced to a + generic status marker. """ if response.status_code == 200: return "" diff --git a/push/apn_handler/errors.py b/push/apn_handler/errors.py index 3889b05..93131ea 100644 --- a/push/apn_handler/errors.py +++ b/push/apn_handler/errors.py @@ -113,6 +113,28 @@ def __init__(self, *args, timestamp: str | None = None) -> None: self.timestamp = timestamp +class ExpiredToken(APNsException): + """The device token has expired.""" + + def __init__(self, *args, timestamp: str | None = None) -> None: + super().__init__(*args) + + self.timestamp = timestamp + + +class InvalidPushType(BadPayloadException): + """The apns-push-type value is invalid.""" + + +class BadEnvironmentKeyIdInToken(APNsException): + """The key ID in the provider token does not match the environment.""" + + +class UnrelatedKeyIdInToken(APNsException): + """The key ID in the provider token is not related to the key ID used in + the first push of this connection.""" + + class PayloadTooLarge(BadPayloadException): """The message payload was too large. The maximum payload size is 4096 bytes.""" @@ -166,6 +188,10 @@ def exception_class_for_reason(reason: str) -> type[APNsException]: "BadPath": BadPath, "MethodNotAllowed": MethodNotAllowed, "Unregistered": Unregistered, + "ExpiredToken": ExpiredToken, + "InvalidPushType": InvalidPushType, + "BadEnvironmentKeyIdInToken": BadEnvironmentKeyIdInToken, + "UnrelatedKeyIdInToken": UnrelatedKeyIdInToken, "PayloadTooLarge": PayloadTooLarge, "TooManyProviderTokenUpdates": TooManyProviderTokenUpdates, "TooManyRequests": TooManyRequests, diff --git a/services/push_service.py b/services/push_service.py index a9f2113..28ed041 100644 --- a/services/push_service.py +++ b/services/push_service.py @@ -56,7 +56,11 @@ def send_push(self, message: Message) -> dict[str, str]: results = self.handler.send_multiple_push( to_device_tokens=message.recipients, body=message.body ) - stale_tokens = [token for token, result in results.items() if result == "Unregistered"] + stale_tokens = [ + token + for token, result in results.items() + if result in ("Unregistered", "ExpiredToken") + ] if stale_tokens: # Pruning is best-effort cleanup; the notifications are already # sent, so a database failure here must not turn the completed diff --git a/tests/test_apns_client.py b/tests/test_apns_client.py index ccd9859..fea7e33 100644 --- a/tests/test_apns_client.py +++ b/tests/test_apns_client.py @@ -134,3 +134,25 @@ def test_non_dict_json_error_body_maps_to_status_marker(monkeypatch): client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") assert "HTTPError503" in str(exc_info.value) + + +def test_new_apns_reasons_map_to_typed_exceptions(monkeypatch): + from push.apn_handler.errors import InvalidPushType + + client = make_client(monkeypatch, 400, {"reason": "InvalidPushType"}) + + with pytest.raises(InvalidPushType): + client.send_notification("device-token", Payload(alert="hello"), topic="com.example.test") + + +def test_live_activity_topic_infers_liveactivity_push_type(monkeypatch): + requests = [] + client = make_client(monkeypatch, 200, {}, requests) + + client.send_notification( + "device-token", + Payload(alert="hello"), + topic="com.example.test.push-type.liveactivity", + ) + + assert requests[0].headers["apns-push-type"] == "liveactivity" diff --git a/tests/test_push_api.py b/tests/test_push_api.py index 355b15f..a66d7af 100644 --- a/tests/test_push_api.py +++ b/tests/test_push_api.py @@ -135,3 +135,16 @@ def remove_devices(self, tokens): assert response.status_code == 200 assert response.json() == {"good-token": "Success", "stale-token": "Unregistered"} + + +def test_send_push_removes_expired_token_devices(client, apns_handler_factory): + override_handler( + apns_handler_factory( + {"dead-token": (410, {"reason": "ExpiredToken", "timestamp": "1700000000"})} + ) + ) + client.post("/devices/register", json={"token": "dead-token"}) + + client.post("/push/send", json={"recipients": ["dead-token"], "body": "hello"}) + + assert client.get("/devices/all").json() == [] diff --git a/tests/test_push_handler.py b/tests/test_push_handler.py index b3c43aa..b096e4f 100644 --- a/tests/test_push_handler.py +++ b/tests/test_push_handler.py @@ -45,7 +45,7 @@ def test_sandbox_flag_targets_development_server(monkeypatch): handler.send_push("device-token", body="hello") - assert requests[0].url.host == "api.development.push.apple.com" + assert requests[0].url.host == "api.sandbox.push.apple.com" def test_production_server_is_default(monkeypatch): @@ -96,7 +96,7 @@ def test_sandbox_flag_tolerates_surrounding_whitespace(monkeypatch): handler.send_push("device-token", body="hello") - assert requests[0].url.host == "api.development.push.apple.com" + assert requests[0].url.host == "api.sandbox.push.apple.com" def test_unrecognized_sandbox_value_is_rejected(monkeypatch): From c9091e87c77e55fc1879097728b40df4274fa4bc Mon Sep 17 00:00:00 2001 From: Josh Caponigro Date: Wed, 19 Aug 2026 22:53:00 -0500 Subject: [PATCH 3/3] Run ruff and mypy in CI, update action versions --- .github/workflows/test.yml | 19 ++++++++++++------- services/push_service.py | 4 +--- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 67af6d0..9b5096c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,5 +1,4 @@ -# This workflow will install Python dependencies, run tests and lint with a single version of Python -# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python +# Runs lint, type checks, and tests on every push and pull request to main. name: Python application @@ -20,14 +19,20 @@ jobs: steps: - uses: actions/checkout@v4 - name: Set up Python 3.11 - uses: actions/setup-python@v3 + uses: actions/setup-python@v5 with: python-version: "3.11" + cache: "pip" - name: Install dependencies run: | python -m pip install --upgrade pip - pip install pytest - if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - - name: Test with pytest + pip install -r requirements.txt + pip install ruff mypy + - name: Lint with ruff run: | - pytest + ruff check . + ruff format --check . + - name: Type-check with mypy + run: mypy apis entities models push services utils main.py database.py + - name: Test with pytest + run: pytest diff --git a/services/push_service.py b/services/push_service.py index 28ed041..2dbcaf9 100644 --- a/services/push_service.py +++ b/services/push_service.py @@ -57,9 +57,7 @@ def send_push(self, message: Message) -> dict[str, str]: to_device_tokens=message.recipients, body=message.body ) stale_tokens = [ - token - for token, result in results.items() - if result in ("Unregistered", "ExpiredToken") + token for token, result in results.items() if result in ("Unregistered", "ExpiredToken") ] if stale_tokens: # Pruning is best-effort cleanup; the notifications are already