Add Stripe webhook verification and read models

[lohanidamodar]

Summary

Adapter and Pay methods now return typed, read-only models instead of raw Stripe arrays. The PR also adds webhook verification.

Breaking change: ship as 0.15.0. Callers that read the returned arrays have to switch to getters. Inputs (address arrays and so on) are unchanged.

Return types

Model	Methods
Payment	purchase, authorize, capture, cancelAuthorization, updatePayment, retryPurchase, getPayment
Refund	refund
PaymentMethod	createPaymentMethod, getPaymentMethod, updatePaymentMethod, updatePaymentMethodBillingDetails; listPaymentMethods returns PaymentMethod[]
Customer	createCustomer, getCustomer, updateCustomer; listCustomers returns Customer[]
SetupIntent	createFuturePayment, getFuturePayment, updateFuturePayment; listFuturePayments / Pay::listFuturePayment return SetupIntent[]
Mandate	getMandate
Dispute	listDisputes returns Dispute[]
WebhookEvent	constructWebhookEvent

deletePaymentMethod and deleteCustomer still return bool. listPaymentMethods and listCustomers now return the items directly instead of Stripe's {object: list, data: [...]} envelope.

Models

All models extend a read-only Model base:

• It holds the payload and has no setters.
• Getters return null when a field is absent or has an unexpected type, so callers keep their own fallbacks instead of inheriting invented defaults.
• Fields that can come back either as an ID or as an expanded object resolve to the ID.
• getRaw() is the escape hatch for fields without a getter.

Getters cover every field Appwrite Cloud reads:

• Payment: id, amount, amount received, currency, status, customer / payment method / latest charge IDs, client secret, last_payment_error code, decline code and message, next_action (free-form array), metadata, created. It also has status helpers and getCharges(), which returns Charge[] from the legacy charges list.
• Charge: id, amount, amount refunded, currency, status, isRefunded().
• PaymentMethod:
  • id, type, customer, metadata, created;
  • card brand, last4, expiry month and year, funding and country;
  • billing name, email, phone, city, country, line1, line2, postal code and state;
  • isCard(), isExpired().
• SetupIntent: id, status, customer, payment method, client secret, mandate, payment_method_options (free-form array).
• Mandate: id, status, payment method, isActive().
• Dispute: id, amount, currency, reason, status, charge and payment intent IDs, metadata, evidence due date, created, and getPaymentIntentMetadata() for an expanded payment_intent.
• Customer: id, name, email, phone, default payment method, metadata, created, isDeleted().
• Refund: id, amount, currency, status, payment intent, charge, reason, failure reason, metadata, created.
• WebhookEvent: id, type, data.object and its object type, plus TYPE_* constants for the payment intent, setup intent, mandate.updated, charge.refunded and charge.dispute.* events.

Webhook verification

• constructWebhookEvent($payload, $signatureHeader, $secret, $tolerance = 300): verifies the Stripe-Signature header with the existing Validator\Stripe\Webhook (HMAC-SHA256, hash_equals) and returns a WebhookEvent.
  • null tolerance skips the timestamp check.
  • On failure it throws Exception::SIGNATURE_VERIFICATION_FAILED (400).
  • It is a concrete Adapter method that throws by default.
• The validator no longer warns on header items without =. Its tests sign payloads locally.

Other

• Exception: constants for common Stripe error and decline codes.
• Stripe handleError(): a non-JSON error body is now the message instead of the type.
• Address::fromArray(): the inverse of asArray().

Tests

• Unit tests: every model's fromArray has unit tests with realistic Stripe payloads, covering expanded and missing fields. There are also webhook verification tests. The 82 key-free tests pass locally.
• StripeTest: now asserts on model getters. CI runs it against Stripe test mode with 110 tests, all passing.
• composer lint: passes.
• PHPStan 2.x level 8: no errors in the files this PR adds or changes. The remaining errors are already on main. The pinned PHPStan 1.9 cannot parse main's PHP 8.4+ syntax.

Consumer

• appwrite-labs/cloud#6067 migrates every Cloud call site to these models. It pins this PR through the feat/stripe-webhook-read-models branch until 0.15.0 is tagged.

[coderabbitai]

Walkthrough

This PR adds a large set of domain value objects and utilities (Currency, Customer, Payment, PaymentMethod, Refund, Dispute, SetupIntent, WebhookEvent, IdempotencyKey, Cursor, PaginatedResult, StripeWebhookEvents) and extends Address with toArray/fromArray/isComplete/isEmpty. Exception was expanded with detailed error constants and helpers. Adapter, Pay and Stripe implementations/signatures were tightened to return these domain objects (and accept Address/IdempotencyKey where appropriate); handleError now declares void and normalizes non-array responses. Comprehensive PHPUnit tests were added for Address, Currency, Customer, Payment, PaymentMethod, Refund and adapter flows.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 81.15% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title clearly summarizes the two primary changes: Stripe webhook verification and typed read models.
Description check	✅ Passed	The description is directly related to the changeset and provides detailed coverage of typed models, webhook verification, API changes, tests, and breaking-change impact.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch claude/improve-utopia-library-EdGjh

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/Pay/Payment/Payment.php`:
- Around line 571-580: The getAmountDecimal method incorrectly assumes 2-decimal
currencies by always dividing $this->amount by 100; update it to convert using
the Currency utility instead (e.g., call Currency::fromSmallestUnit or the
provided Currency::fromSmallestUnit($amount, $currency) helper) so it uses the
currency's actual minor unit scale; replace the hardcoded division in
getAmountDecimal(int $decimals = 2): float to use the Currency conversion for
$this->amount and $this->currency, then round the resulting decimal to $decimals
before returning.

In `@src/Pay/Refund/Refund.php`:
- Around line 376-385: getAmountDecimal currently always divides $this->amount
by 100 which breaks zero- and three-decimal currencies; update
Refund::getAmountDecimal to determine the currency's decimal places via the
Currency utility (e.g., Currency::decimalPlaces or similar API) using
$this->currency, compute the divider as pow(10, $currencyDecimalPlaces) and
divide $this->amount by that value, then round to the requested $decimals for
display; reference the method name getAmountDecimal and the Currency class when
making this change.

🧹 Nitpick comments (4)

src/Pay/Currency.php (2)

236-246: Unused $locale parameter should be removed or documented as reserved.

The $locale parameter is declared but never used. Either remove it or add a note that locale-based formatting is planned for a future implementation.

♻️ Suggested fix

-    public static function format(int $amount, string $currency, ?string $locale = null): string
+    public static function format(int $amount, string $currency): string
     {
         $decimalAmount = self::fromSmallestUnit($amount, $currency);
         $decimals = self::getDecimalPlaces($currency);
         $currency = strtoupper($currency);

         // Simple formatting without locale support for portability
         $formatted = number_format($decimalAmount, $decimals, '.', ',');

         return $currency.' '.$formatted;
     }

---

298-307: meetsMinimum may produce unexpected results for three-decimal currencies.

For three-decimal currencies (BHD, JOD, KWD, OMR, TND), the $minimumCents parameter represents cents (1/100), but these currencies use fils/millimes (1/1000). A $minimumCents = 50 would mean 0.50 in standard currencies but would still be compared against the smallest unit (fils) without adjustment.

Consider documenting this behavior or adding scaling logic for three-decimal currencies.

src/Pay/Customer/Customer.php (1)

35-38: Return type documentation inconsistency.

The constructor always sets createdAt to time() when null is passed (Line 37), so getCreatedAt() will never actually return null after construction. However, the PHPDoc on Line 204 states @return int|null. Consider updating the return type to just int for accuracy, or documenting that null is only possible if explicitly set via setCreatedAt(null).

src/Pay/PaymentMethod/PaymentMethod.php (1)

438-455: Verify 2-digit year handling in isExpired().

The PHPDoc specifies expYear should be 4 digits, but some payment processors may return 2-digit years (e.g., 25 instead of 2025). The current implementation with DateTime::createFromFormat('Y-n', ...) expects a 4-digit year and would produce incorrect results for 2-digit years.

Consider adding validation or normalization in fromArray() or documenting this requirement explicitly.

🛡️ Optional: Normalize 2-digit years in fromArray

-            expYear: isset($cardData['exp_year']) ? (int) $cardData['exp_year'] : ($data['expYear'] ?? null),
+            expYear: isset($cardData['exp_year']) 
+                ? ((int) $cardData['exp_year'] < 100 ? 2000 + (int) $cardData['exp_year'] : (int) $cardData['exp_year']) 
+                : ($data['expYear'] ?? null),

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Pay/Adapter/Stripe.php (1)

259-275: ⚠️ Potential issue | 🟠 Major

Update tests to pass Address objects instead of arrays.

The tests at tests/Pay/Adapter/StripeTest.php:33, 479, and 529 are passing arrays directly to createCustomer(), but the method signature now requires an Address object or null. These calls need to be updated to instantiate Address objects with the array data or pass null if no address is needed.

This is a breaking change and causes type errors in strict PHP environments.

🧹 Nitpick comments (6)

src/Pay/Adapter/Stripe.php (4)

81-96: Consider using strict null comparison.

Lines 85 and 89 use loose comparison (!= null). While functionally equivalent for null checks, strict comparison (!== null) is more explicit and avoids potential issues with falsy values.

Suggested improvement

-        if ($amount != null) {
+        if ($amount !== null) {
             $requestBody['amount'] = $amount;
         }

-        if ($reason != null) {
+        if ($reason !== null) {
             $requestBody['reason'] = $reason;
         }

---

219-227: Replace deprecated asArray() with toArray().

According to the PR summary, asArray() is deprecated in favor of toArray(). This should be updated for consistency.

Suggested fix

         if (! is_null($address)) {
-            $requestBody['billing_details']['address'] = $address->asArray();
+            $requestBody['billing_details']['address'] = $address->toArray();
         }

---

269-271: Replace deprecated asArray() with toArray().

Same as the billing details case - use the non-deprecated method.

Suggested fix

         if (! is_null($address)) {
-            $requestBody['address'] = $address->asArray();
+            $requestBody['address'] = $address->toArray();
         }

---

308-325: Replace deprecated asArray() with toArray() in updateCustomer.

For consistency with the deprecation, update this usage as well.

Suggested fix

         if (! is_null($address)) {
-            $requestBody['address'] = $address->asArray();
+            $requestBody['address'] = $address->toArray();
         }

src/Pay/Adapter.php (2)

110-118: Consider using explicit nullable type hints for consistency.

The parameters use implicit nullable syntax (int $amount = null) rather than explicit nullable (?int $amount = null). While both work, mixing styles reduces consistency. The explicit ? syntax is preferred in modern PHP.

Suggested improvement

-    abstract public function refund(string $paymentId, int $amount = null, string $reason = null): Refund;
+    abstract public function refund(string $paymentId, ?int $amount = null, ?string $reason = null): Refund;

---

202-212: Inconsistent nullable type hint for $address parameter.

Line 185 uses ?Address $address = null but line 212 uses Address $address = null. For consistency, use the explicit nullable syntax throughout.

Suggested fix

-    abstract public function updateCustomer(string $customerId, string $name, string $email, Address $address = null, string $paymentMethod = null): Customer;
+    abstract public function updateCustomer(string $customerId, string $name, string $email, ?Address $address = null, ?string $paymentMethod = null): Customer;

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@src/Pay/Dispute/Dispute.php`:
- Around line 541-545: getAmountDecimal currently always divides by 100 which is
wrong for currencies with 0 or 3 decimal places; update getAmountDecimal to
consult the new Currency utilities (e.g., Currency class) to determine the
correct number of decimal places for $this->currency when $decimals is not
explicitly provided, compute the divisor as pow(10, $currencyDecimals), divide
$this->amount by that divisor, then round to the resolved number of decimals;
refer to the getAmountDecimal method and the Currency class (use its method that
returns decimal places or subunit divisor) to implement this logic.

In `@src/Pay/Pagination/PaginatedResult.php`:
- Around line 89-104: getPreviousCursor() currently falls back to returning
$this->endingBefore when it can't extract an ID from the first item, which
mirrors the same misleading behavior noted for getNextCursor(); update
getPreviousCursor() (and references to $this->data, reset($this->data),
$firstItem, method getId) to return null instead of $this->endingBefore when no
ID can be determined (i.e., after the object/array checks fail), so the method
only returns a valid cursor or null to accurately represent pagination state.
- Around line 64-79: The getNextCursor() method currently returns
$this->startingAfter as a fallback when the last item has no extractable ID;
change this to return null instead because startingAfter represents the current
request cursor, not the next page. Update the logic in getNextCursor()
(referencing the method name) so after checking is_object/$lastItem->getId and
is_array/$lastItem['id'] fail, the method returns null rather than
$this->startingAfter; ensure the hasMore/empty($this->data) early-exit remains
unchanged.

In `@src/Pay/Webhook/WebhookEvent.php`:
- Around line 414-427: In WebhookEvent::fromArray ensure requestId is always a
string or null by replacing the current fallback chain with logic that first
checks if $data['request'] is an array and contains a string 'id', then uses
that; else if $data['request'] is a string use it; otherwise use null — i.e.
extract requestId from $data['request']['id'] only when it exists and is a
string, or from $data['request'] only when it is a string, preventing an array
from being assigned to requestId.

In `@tests/Pay/Adapter/StripeTest.php`:
- Around line 112-114: Update the assertions in the StripeTest.php test to call
the actual PaymentMethod accessor names: replace calls to getExpiryYear() and
getExpiryMonth() on $pm with getExpYear() and getExpMonth() respectively so the
test uses the PaymentMethod methods getExpYear() and getExpMonth(); keep the
expected values (2030 and 8) and the last4() assertion unchanged.

🧹 Nitpick comments (6)

src/Pay/Customer/Customer.php (1)

36-39: Return type inconsistency: getCreatedAt() can never return null.

Since the constructor always sets $this->createdAt = $createdAt ?? time() (Line 39), the value is never null after construction. However, getCreatedAt() returns ?int. Consider either:

1. Changing the return type to int for getCreatedAt()
2. Or keeping ?int for semantic consistency with other domain models

This is a minor inconsistency that doesn't affect functionality.

src/Pay/Webhook/WebhookEvent.php (1)

354-385: Category detection order may cause unexpected results.

The getCategory() method checks predicates in order, but some event types match multiple categories. For example:

• customer.subscription.created contains both "customer" and "subscription"
• isSubscriptionEvent() is checked after isPaymentEvent(), so subscription events are correctly categorized
• However, isCustomerEvent() is checked last, which means customer.subscription.* events will be categorized as CATEGORY_SUBSCRIPTION, not CATEGORY_CUSTOMER

This is likely intentional (more specific categorization wins), but consider documenting this priority behavior.

src/Pay/SetupIntent/SetupIntent.php (1)

487-498: Consider whether isComplete() should include STATUS_CANCELED.

The isComplete() method returns true for both SUCCEEDED and CANCELED statuses. While technically both are terminal states, semantically "complete" might imply success. Consider renaming to isTerminal() or documenting that "complete" means "no further action possible."

src/Pay/Idempotency/IdempotencyKey.php (2)

151-176: Consider documenting hour-boundary behavior for deterministic keys.

The hour-level timestamp granularity (date('Y-m-d-H')) means a retry crossing an hour boundary will generate a different idempotency key. This is a reasonable trade-off for automatic key generation, but callers relying on deterministic keys for retries should be aware of this limitation.

Consider adding a note in the PHPDoc explaining this behavior, or offering an overload that accepts a custom timestamp for retry scenarios.

---

195-199: Minor doc inconsistency: comment mentions underscores but regex also allows hyphens.

The comment says "alphanumeric with optional underscores" but the regex /^[a-zA-Z0-9_-]{8,64}$/ also allows hyphens. Consider updating the comment to accurately reflect the allowed characters.

📝 Proposed fix

-        // Key should be alphanumeric with optional underscores, 8-64 characters
+        // Key should be alphanumeric with optional underscores and hyphens, 8-64 characters
         return (bool) preg_match('/^[a-zA-Z0-9_-]{8,64}$/', $key);

src/Pay/Adapter.php (1)

219-219: Minor inconsistency: nullable type hint style.

Line 192 (createCustomer) uses ?Address $address = null but line 219 (updateCustomer) uses Address $address = null. While both work, the explicit ?Address is preferred for clarity and consistency.

📝 Proposed fix

-    abstract public function updateCustomer(string $customerId, string $name, string $email, Address $address = null, string $paymentMethod = null): Customer;
+    abstract public function updateCustomer(string $customerId, string $name, string $email, ?Address $address = null, ?string $paymentMethod = null): Customer;

[greptile-apps]

Confidence Score: 4/5

The PR is not safe to merge until the unintended public return-contract break is removed or deliberately introduced and migrated as a breaking release.

Findings

1.  Typed returns break compatibility ▶
2.  Tests mirror implementation details ▶

Fix with agent prompt

### Issue 1
src/Pay/Adapter.php:72
Changing the abstract adapter operations from `array` to concrete model return types breaks both sides of the existing public contract. Third-party adapters that still implement the documented `: array` signatures will fail PHP's method compatibility check when their classes load. Existing consumers that follow the README and access results through `$customer['id']` or `$authorization['id']` will instead receive model objects that do not implement `ArrayAccess`, causing runtime failures. The related conversions in `Stripe` also reduce `listCustomers()` and `listPaymentMethods()` to their `data` items, so callers can no longer recover list metadata such as `has_more`.

### Issue 2
tests/Pay/Adapter/StripeTest.php:undefined-28
This test invokes the protected error handler through reflection instead of exercising the public request boundary. The same implementation coupling appears in `tests/Pay/AddressTest.php:12`, where the serializer supplies its own round-trip expectation, and `tests/Pay/Payment/PaymentTest.php:82-94`, where the status matrix duplicates the source mapping. This violates the repository directive to test observable behavior rather than mirror source implementation. These tests can pass while public behavior is broken and fail during harmless refactors, so this repository requirement must be satisfied before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR adds Stripe webhook verification and read models, but the latest revision also changes the established adapter and Pay return contract from provider arrays to concrete model objects rather than keeping models opt-in.

• Adds signature validation and typed webhook event decoding.
• Adds read-only models for payments, payment methods, customers, refunds, setup intents, mandates, disputes, charges, and webhook events.
• Converts Stripe operation and list results to those models across the public API.
• Fixes handling of non-JSON Stripe error responses.
• Leaves the existing implementation-coupled testing finding outstanding.

_{Reviews (5) · Last reviewed commit: "Return typed models from Pay and adapter..."}

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Adds opt-in typed payment and payment-method read models while preserving existing array-based adapter APIs.

Changes:

• Added Payment and PaymentMethod models with parsing and status helpers.
• Added Address::fromArray() and Stripe error constants.
• Improved non-JSON Stripe error handling and added focused tests.

File	Description
tests/​Pay/​PaymentMethod/​PaymentMethodTest.php	Payment method model tests
tests/​Pay/​Payment/​PaymentTest.php	Payment model tests
tests/​Pay/​AddressTest.php	Address deserialization tests
tests/​Pay/​Adapter/​StripeTest.php	Error handling tests
src/​Pay/​PaymentMethod/​PaymentMethod.php	Payment method read model
src/​Pay/​Payment/​Payment.php	Payment read model
src/​Pay/​Exception.php	Stripe error constants
src/​Pay/​Address.php	Address deserialization
src/​Pay/​Adapter/​Stripe.php	Non-JSON error handling

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[greptile-apps]

Tests mirror implementation details

This test invokes the protected error handler through reflection instead of exercising the public request boundary. The same implementation coupling appears in tests/Pay/AddressTest.php:12, where the serializer supplies its own round-trip expectation, and tests/Pay/Payment/PaymentTest.php:82-94, where the status matrix duplicates the source mapping. This violates the repository directive to test observable behavior rather than mirror source implementation. These tests can pass while public behavior is broken and fail during harmless refactors, so this repository requirement must be satisfied before merging.

Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/Pay/Adapter/StripeTest.php
Line: 28

Comment:
**Tests mirror implementation details**

This test invokes the protected error handler through reflection instead of exercising the public request boundary. The same implementation coupling appears in `tests/Pay/AddressTest.php:12`, where the serializer supplies its own round-trip expectation, and `tests/Pay/Payment/PaymentTest.php:82-94`, where the status matrix duplicates the source mapping. This violates the repository directive to test observable behavior rather than mirror source implementation. These tests can pass while public behavior is broken and fail during harmless refactors, so this repository requirement must be satisfied before merging.

**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Typed returns break compatibility

Changing the abstract adapter operations from array to concrete model return types breaks both sides of the existing public contract. Third-party adapters that still implement the documented : array signatures will fail PHP's method compatibility check when their classes load. Existing consumers that follow the README and access results through $customer['id'] or $authorization['id'] will instead receive model objects that do not implement ArrayAccess, causing runtime failures. The related conversions in Stripe also reduce listCustomers() and listPaymentMethods() to their data items, so callers can no longer recover list metadata such as has_more.

Knowledge Base Used:

• Payment adapter contract
• Payment client orchestration

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/Pay/Adapter.php
Line: 72

Comment:
**Typed returns break compatibility**

Changing the abstract adapter operations from `array` to concrete model return types breaks both sides of the existing public contract. Third-party adapters that still implement the documented `: array` signatures will fail PHP's method compatibility check when their classes load. Existing consumers that follow the README and access results through `$customer['id']` or `$authorization['id']` will instead receive model objects that do not implement `ArrayAccess`, causing runtime failures. The related conversions in `Stripe` also reduce `listCustomers()` and `listPaymentMethods()` to their `data` items, so callers can no longer recover list metadata such as `has_more`.

**Knowledge Base Used:**
- [Payment adapter contract](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/utopia-php/pay/-/docs/payment-adapter-contract.md)
- [Payment client orchestration](https://app.greptile.com/appwrite/-/custom-context/knowledge-base/utopia-php/pay/-/docs/payment-client-orchestration.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[Copilot]

Copilot review overview

🟡 Changes recommended

Update the published adapter tutorial and README examples for the new typed return values.

Review effort: Lite
Findings: 1

Open (1)

• Update README examples for model return values · New