feat(loyalty): Google Wallet production readiness — 10 hardening items
Some checks failed
Some checks failed
- Fix rate limiter to extract real client IP and handle sync/async endpoints - Rate-limit public enrollment (10/min) and program info (30/min) endpoints - Add 409 Conflict to non-retryable status codes in retry decorator - Cache private key in get_save_url() to avoid re-reading JSON per call - Make update_class() return bool success status with error-level logging - Move Google Wallet config from core to loyalty module config - Document time.sleep() safety in retry decorator (threadpool execution) - Add per-card retry (1 retry, 2s delay) to wallet sync task - Add logo URL reachability check (HEAD request) to validate_config() - Add 26 comprehensive unit tests for GoogleWalletService Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -20,7 +20,7 @@ from typing import Any
|
||||
import requests
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.core.config import settings
|
||||
from app.modules.loyalty.config import config
|
||||
from app.modules.loyalty.exceptions import (
|
||||
GoogleWalletNotConfiguredException,
|
||||
WalletIntegrationException,
|
||||
@@ -51,7 +51,7 @@ def _retry_on_failure(func):
|
||||
# Don't retry client errors (400, 401, 403, 404, 409)
|
||||
exc_msg = str(exc)
|
||||
if any(f":{code}" in exc_msg or f" {code} " in exc_msg
|
||||
for code in ("400", "401", "403", "404")):
|
||||
for code in ("400", "401", "403", "404", "409")):
|
||||
logger.error("Google Wallet API client error (not retryable): %s", exc)
|
||||
break
|
||||
if attempt < MAX_RETRIES - 1:
|
||||
@@ -63,6 +63,9 @@ def _retry_on_failure(func):
|
||||
wait,
|
||||
exc,
|
||||
)
|
||||
# Safe: all loyalty routes are sync def, so FastAPI runs them
|
||||
# in a threadpool. time.sleep() only blocks the worker thread,
|
||||
# not the async event loop.
|
||||
time.sleep(wait)
|
||||
else:
|
||||
logger.error(
|
||||
@@ -83,13 +86,14 @@ class GoogleWalletService:
|
||||
self._credentials = None
|
||||
self._http_client = None
|
||||
self._signer = None
|
||||
self._private_key: str | None = None
|
||||
|
||||
@property
|
||||
def is_configured(self) -> bool:
|
||||
"""Check if Google Wallet is configured."""
|
||||
return bool(
|
||||
settings.loyalty_google_issuer_id
|
||||
and settings.loyalty_google_service_account_json
|
||||
config.google_issuer_id
|
||||
and config.google_service_account_json
|
||||
)
|
||||
|
||||
def validate_config(self) -> dict[str, Any]:
|
||||
@@ -103,8 +107,8 @@ class GoogleWalletService:
|
||||
|
||||
result: dict[str, Any] = {
|
||||
"configured": self.is_configured,
|
||||
"issuer_id": settings.loyalty_google_issuer_id,
|
||||
"service_account_path": settings.loyalty_google_service_account_json,
|
||||
"issuer_id": config.google_issuer_id,
|
||||
"service_account_path": config.google_service_account_json,
|
||||
"credentials_valid": False,
|
||||
"errors": [],
|
||||
}
|
||||
@@ -112,7 +116,7 @@ class GoogleWalletService:
|
||||
if not self.is_configured:
|
||||
return result
|
||||
|
||||
sa_path = settings.loyalty_google_service_account_json
|
||||
sa_path = config.google_service_account_json
|
||||
if not os.path.isfile(sa_path):
|
||||
result["errors"].append(f"Service account file not found: {sa_path}")
|
||||
return result
|
||||
@@ -145,6 +149,21 @@ class GoogleWalletService:
|
||||
except (OSError, ValueError) as exc:
|
||||
result["errors"].append(f"Failed to load credentials: {exc}")
|
||||
|
||||
# Check logo URL reachability (warning only, not a blocking error)
|
||||
logo_url = config.default_logo_url
|
||||
if logo_url:
|
||||
result["warnings"] = result.get("warnings", [])
|
||||
try:
|
||||
resp = requests.head(logo_url, timeout=5, allow_redirects=True)
|
||||
if resp.status_code >= 400:
|
||||
result["warnings"].append(
|
||||
f"Default logo URL returned HTTP {resp.status_code}: {logo_url}"
|
||||
)
|
||||
except requests.RequestException as exc:
|
||||
result["warnings"].append(
|
||||
f"Default logo URL unreachable: {logo_url} ({exc})"
|
||||
)
|
||||
|
||||
return result
|
||||
|
||||
def _get_credentials(self):
|
||||
@@ -152,7 +171,7 @@ class GoogleWalletService:
|
||||
if self._credentials:
|
||||
return self._credentials
|
||||
|
||||
if not settings.loyalty_google_service_account_json:
|
||||
if not config.google_service_account_json:
|
||||
raise GoogleWalletNotConfiguredException()
|
||||
|
||||
try:
|
||||
@@ -162,7 +181,7 @@ class GoogleWalletService:
|
||||
|
||||
self._credentials = (
|
||||
service_account.Credentials.from_service_account_file(
|
||||
settings.loyalty_google_service_account_json,
|
||||
config.google_service_account_json,
|
||||
scopes=scopes,
|
||||
)
|
||||
)
|
||||
@@ -180,13 +199,23 @@ class GoogleWalletService:
|
||||
from google.auth.crypt import RSASigner
|
||||
|
||||
self._signer = RSASigner.from_service_account_file(
|
||||
settings.loyalty_google_service_account_json,
|
||||
config.google_service_account_json,
|
||||
)
|
||||
return self._signer
|
||||
except (ValueError, OSError, KeyError) as exc:
|
||||
logger.error("Failed to create RSA signer: %s", exc)
|
||||
raise WalletIntegrationException("google", str(exc)) from exc
|
||||
|
||||
def _get_private_key(self) -> str:
|
||||
"""Get the private key from the service account JSON, with caching."""
|
||||
if self._private_key:
|
||||
return self._private_key
|
||||
|
||||
with open(config.google_service_account_json) as f:
|
||||
sa_data = json.load(f)
|
||||
self._private_key = sa_data["private_key"]
|
||||
return self._private_key
|
||||
|
||||
def _get_http_client(self):
|
||||
"""Get authenticated HTTP client."""
|
||||
if self._http_client:
|
||||
@@ -221,7 +250,7 @@ class GoogleWalletService:
|
||||
if not self.is_configured:
|
||||
raise GoogleWalletNotConfiguredException()
|
||||
|
||||
issuer_id = settings.loyalty_google_issuer_id
|
||||
issuer_id = config.google_issuer_id
|
||||
class_id = f"{issuer_id}.loyalty_program_{program.id}"
|
||||
|
||||
# issuerName is required by Google Wallet API
|
||||
@@ -246,7 +275,7 @@ class GoogleWalletService:
|
||||
# Google must be able to fetch the image, so it needs a public URL
|
||||
logo_url = program.logo_url
|
||||
if not logo_url:
|
||||
logo_url = settings.loyalty_default_logo_url
|
||||
logo_url = config.default_logo_url
|
||||
class_data["programLogo"] = {
|
||||
"sourceUri": {"uri": logo_url},
|
||||
}
|
||||
@@ -289,10 +318,14 @@ class GoogleWalletService:
|
||||
raise WalletIntegrationException("google", str(exc)) from exc
|
||||
|
||||
@_retry_on_failure
|
||||
def update_class(self, db: Session, program: LoyaltyProgram) -> None:
|
||||
"""Update a LoyaltyClass when program settings change."""
|
||||
def update_class(self, db: Session, program: LoyaltyProgram) -> bool:
|
||||
"""Update a LoyaltyClass when program settings change.
|
||||
|
||||
Returns:
|
||||
True if update succeeded, False otherwise.
|
||||
"""
|
||||
if not program.google_class_id:
|
||||
return
|
||||
return False
|
||||
|
||||
class_data = {
|
||||
"programName": program.display_name,
|
||||
@@ -312,14 +345,17 @@ class GoogleWalletService:
|
||||
json=class_data,
|
||||
)
|
||||
|
||||
if response.status_code not in (200, 201):
|
||||
logger.warning(
|
||||
"Failed to update Google Wallet class %s: %s",
|
||||
program.google_class_id,
|
||||
response.status_code,
|
||||
)
|
||||
if response.status_code in (200, 201):
|
||||
return True
|
||||
logger.error(
|
||||
"Failed to update Google Wallet class %s: %s",
|
||||
program.google_class_id,
|
||||
response.status_code,
|
||||
)
|
||||
return False
|
||||
except (requests.RequestException, ValueError, AttributeError) as exc:
|
||||
logger.error("Failed to update Google Wallet class: %s", exc)
|
||||
return False
|
||||
|
||||
# =========================================================================
|
||||
# LoyaltyObject Operations (Card-level)
|
||||
@@ -344,7 +380,7 @@ class GoogleWalletService:
|
||||
if not program.google_class_id:
|
||||
self.create_class(db, program)
|
||||
|
||||
issuer_id = settings.loyalty_google_issuer_id
|
||||
issuer_id = config.google_issuer_id
|
||||
object_id = f"{issuer_id}.loyalty_card_{card.id}"
|
||||
|
||||
object_data = self._build_object_data(card, object_id)
|
||||
@@ -490,9 +526,9 @@ class GoogleWalletService:
|
||||
credentials = self._get_credentials()
|
||||
|
||||
now = datetime.now(tz=UTC)
|
||||
origins = settings.loyalty_google_wallet_origins or []
|
||||
origins = config.google_wallet_origins or []
|
||||
|
||||
issuer_id = settings.loyalty_google_issuer_id
|
||||
issuer_id = config.google_issuer_id
|
||||
object_id = card.google_object_id or f"{issuer_id}.loyalty_card_{card.id}"
|
||||
|
||||
if card.google_object_id:
|
||||
@@ -517,11 +553,8 @@ class GoogleWalletService:
|
||||
"exp": now + timedelta(hours=1),
|
||||
}
|
||||
|
||||
# Load the private key directly from the service account file
|
||||
# (RSASigner doesn't expose .key; PyJWT needs the PEM string)
|
||||
with open(settings.loyalty_google_service_account_json) as f:
|
||||
sa_data = json.load(f)
|
||||
private_key = sa_data["private_key"]
|
||||
# PyJWT needs the PEM string; cached after first load
|
||||
private_key = self._get_private_key()
|
||||
|
||||
token = jwt.encode(
|
||||
claims,
|
||||
|
||||
Reference in New Issue
Block a user