diff --git a/CHANGELOG.md b/CHANGELOG.md index b8ff04f..8e939f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,49 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.7.1] - 2026-09-09 + +Corrections found by checking the SDK against the current TapPay documentation. +Two of these are defects introduced in 0.7.0. 0.7.0 was tagged but never +published to PyPI, so no released version ever exposed them. + +### Fixed +- **`Models.Currencies` listed 11 currencies TapPay does not accept.** 0.7.0 + shipped JPY, GBP, AUD, EUR, CNY, KRW, SGD, THB, PHP, IDR and VND, none of + which are documented. They autocompleted in an IDE and looked sanctioned. + TapPay documents exactly four, each tied to an acquirer: TWD (most acquirers), + HKD (Bank of China), MYR (RAZER PAY) and USD (Global Payments). The removed + members are gone rather than deprecated, since keeping them would keep + suggesting currencies that fail at the API. Plain currency strings are still + accepted, so a currency TapPay adds later remains usable. +- **`raise_on_error=True` raised on a normal end-of-pagination.** The Record API + answers `status: 2` for "end of list, no more records under the given filter", + which is how paging terminates. 0.7.0 treated every non-zero status as a + failure, so paging to the end of any query raised `TapPayError`. Benign + statuses are now scoped per endpoint via `BENIGN_STATUSES`; `2` is benign for + `/tpc/transaction/query` only and still raises everywhere else. +- **Full refunds were unreachable.** `refund()` required `amount`, but TapPay + documents it as needed "only for partial refund" — omitting it refunds the + whole transaction. `amount` is now optional and dropped from the request when + `None`. Passing `0` still sends `0`, since a zero-value partial refund is not + the same as a full one. +- **The default read timeout was below TapPay's documented minimum.** It was + 27.0s, chosen from a generic `requests` convention; TapPay documents 30s + because banks take longer at peak. A short timeout reports failure for a + transaction that may have succeeded. `DEFAULT_TIMEOUT` is now `(3.05, 30.0)`. + +### Notes +- Verified against the TapPay Backend, Advanced Features and Reference pages as + of 2026-09-09. TapPay publishes no API changelog, so these are differences + observed against the current documentation, not a dated diff. +- Still outstanding and deferred to 0.8.0: `pay_by_token` accepts no cardholder + (whose `phone_number` and `email` are required for 3D-secure token payments); + `CardHolderData` is missing `name_en`, `phone_number_country_code`, + `member_id` and `bank_member_id`, and validates none of the documented field + lengths; `filters` and `details` are required by this SDK but optional in the + API; and nine documented endpoints remain unimplemented, including + reconciliation, card metadata and update-cardholder. + ## [0.7.0] - 2026-08-18 ### Added diff --git a/README.md b/README.md index bb44e07..f54bd36 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,13 @@ response = client.pay_by_token( ### Refunds +Omit `amount` to refund the transaction in full; pass it for a partial refund: + ```python +# Full refund +response = client.refund(rec_trade_id="rec_trade_id") + +# Partial refund response = client.refund( rec_trade_id="rec_trade_id", amount=100, @@ -126,8 +132,19 @@ response = client.refund( ### Currencies -Payments settle in TWD by default. Pass `currency` to override it, using either a -`Models.Currencies` member or a plain currency string: +Payments settle in TWD by default. TapPay documents four currencies, each tied to +the acquirer configured for your merchant account: + +| Currency | Acquirer | +| --- | --- | +| `TWD` | Most acquirers | +| `HKD` | Bank of China | +| `MYR` | RAZER PAY | +| `USD` | Global Payments | + +Pass `currency` to override the default, using either a `Models.Currencies` +member or a plain currency string (strings are accepted so a currency TapPay +adds before this list is updated remains usable): ```python response = client.pay_by_prime( @@ -162,6 +179,10 @@ except tappay.TapPayError as exc: print(exc.status, exc.msg, exc.response["rec_trade_id"]) ``` +`get_records` is exempt from one case: the Record API answers `status: 2` for +"no more records under this filter", which is how pagination terminates. That is +treated as a normal result, not an error, even with `raise_on_error=True`. + The exception hierarchy is: | Exception | Raised when | diff --git a/pyproject.toml b/pyproject.toml index 2c3633c..6671094 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "tappay" -version = "0.7.0" +version = "0.7.1" authors = [ { name="Shih Wei Chris Lo", email="shihwei@gmail.com" }, ] diff --git a/tappay/client.py b/tappay/client.py index dbe6f5f..d543b85 100644 --- a/tappay/client.py +++ b/tappay/client.py @@ -1,7 +1,7 @@ import logging import os from platform import python_version -from typing import Any, Dict, Optional, Tuple, Union, cast +from typing import Any, Dict, FrozenSet, Optional, Tuple, Union, cast import requests from requests.adapters import HTTPAdapter @@ -18,8 +18,10 @@ #: A ``(connect, read)`` pair in seconds. The connect value sits just above a #: multiple of the common 3 second TCP retransmission window, as recommended by -#: the ``requests`` documentation. -DEFAULT_TIMEOUT: Tuple[float, float] = (3.05, 27.0) +#: the ``requests`` documentation. The read value is the 30 seconds TapPay +#: documents: banks can take longer at peak, and timing out early reports a +#: failure for a transaction that may well have succeeded. +DEFAULT_TIMEOUT: Tuple[float, float] = (3.05, 30.0) #: Anything ``requests`` accepts for its ``timeout`` argument. ``None`` means #: "block indefinitely" and is strongly discouraged for server-side use. @@ -44,6 +46,15 @@ #: reported with an HTTP 200, which is why it needs explicit handling. SUCCESS_STATUS = 0 +#: Non-zero ``status`` values that are documented as normal outcomes rather +#: than failures, keyed by endpoint. The Record API answers ``2`` for "end of +#: list, no more records under the given filter", which is how pagination +#: terminates -- treating it as an error would make paging with +#: ``raise_on_error=True`` raise on the last page of every query. +BENIGN_STATUSES: Dict[str, FrozenSet[int]] = { + "/tpc/transaction/query": frozenset({2}), +} + class _Unset: """Sentinel distinguishing "argument omitted" from an explicit ``None``.""" @@ -298,7 +309,7 @@ def pay_by_token( def refund( self, rec_trade_id: str, - amount: int, + amount: Optional[int] = None, *, timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, @@ -306,12 +317,19 @@ def refund( """ Refund a payment Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#refund-api + + :param amount: Amount to refund. TapPay documents this as required only + for a partial refund; omit it to refund the transaction in full. + Sending ``0`` is a zero-value partial refund, not a full one, so the + field is dropped from the request when ``amount`` is ``None``. """ - params = { + params: Dict[str, Any] = { "rec_trade_id": rec_trade_id, - "amount": amount, } + if amount is not None: + params["amount"] = amount + if kwargs: params.update(**kwargs) @@ -508,9 +526,11 @@ def __post( response = self.session.post( uri, json=params, headers=self.headers, timeout=effective_timeout ) - return self.__parse(response) + return self.__parse(response, request_uri) - def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: + def __parse( + self, response: requests.Response, request_uri: str = "" + ) -> Optional[Dict[str, Any]]: if logger.isEnabledFor(logging.DEBUG): logger.debug("response status: %s", response.status_code) logger.debug("response content: %s", _redacted_body(response)) @@ -522,7 +542,7 @@ def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: return None elif 200 <= response.status_code < 300: data = self.__decode_json(response) - self.__raise_for_body_status(data) + self.__raise_for_body_status(data, request_uri) return data elif 400 <= response.status_code < 500: message = f"{response.status_code} response from {self.api_host}" @@ -559,12 +579,14 @@ def __decode_json(self, response: requests.Response) -> Optional[Dict[str, Any]] ) raise Exceptions.InvalidResponseError(message) from exc - def __raise_for_body_status(self, data: Any) -> None: + def __raise_for_body_status(self, data: Any, request_uri: str = "") -> None: """Raise if TapPay reported a failure inside a 2xx response body. No-op unless the client was built with ``raise_on_error=True``. A body without a ``status`` field is left alone, so responses that do not follow - the documented envelope are passed through to the caller untouched. + the documented envelope are passed through to the caller untouched, and + statuses listed in :data:`BENIGN_STATUSES` for the endpoint are treated + as normal outcomes rather than failures. """ if not self.raise_on_error or not isinstance(data, dict): return @@ -572,5 +594,7 @@ def __raise_for_body_status(self, data: Any) -> None: status = data.get("status") if status is None or status == SUCCESS_STATUS: return + if status in BENIGN_STATUSES.get(request_uri, frozenset()): + return raise Exceptions.TapPayError(status, data.get("msg"), data) diff --git a/tappay/models.py b/tappay/models.py index a0296a5..057143f 100644 --- a/tappay/models.py +++ b/tappay/models.py @@ -8,7 +8,13 @@ class Models: """Namespace for TapPay models.""" class Currencies(str, Enum): - """Currencies accepted by the TapPay APIs. + """Currencies documented as accepted by the TapPay APIs. + + Only these four are documented, and each depends on the acquirer + configured for your merchant account: TWD is supported by most + acquirers, HKD by Bank of China, MYR by RAZER PAY, and USD by Global + Payments. Confirm your own acquirer supports a currency before using + it, since an unsupported one is rejected at the API rather than here. A ``str``-backed enum, so members compare equal to their plain-string form and serialize as ``"TWD"`` through ``json.dumps``. Note that @@ -16,24 +22,13 @@ class Currencies(str, Enum): 3.11+, so never interpolate a member into a request payload. Methods accepting a currency also accept a plain string, so a currency - that TapPay adds before this list is updated remains usable. + TapPay adds before this list is updated remains usable. """ TWD = "TWD" - USD = "USD" - JPY = "JPY" HKD = "HKD" - GBP = "GBP" - AUD = "AUD" - EUR = "EUR" - CNY = "CNY" - KRW = "KRW" - SGD = "SGD" MYR = "MYR" - THB = "THB" - PHP = "PHP" - IDR = "IDR" - VND = "VND" + USD = "USD" class CardHolderData(BaseModel): """Card holder data model using Pydantic v2. diff --git a/tests/test_client.py b/tests/test_client.py index 852c05e..c5e8834 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -9,6 +9,7 @@ import tappay from tappay.client import ( + BENIGN_STATUSES, DEFAULT_MAX_RETRIES, DEFAULT_TIMEOUT, RETRYABLE_PATHS, @@ -268,9 +269,9 @@ def test_currency_applies_to_token_payments_and_card_binding( sandbox_client, card_holder, post ): sandbox_client.pay_by_token( - card_key="ck", card_token="ct", amount=1, details="d", currency="JPY" + card_key="ck", card_token="ct", amount=1, details="d", currency="MYR" ) - assert post.call_args.kwargs["json"]["currency"] == "JPY" + assert post.call_args.kwargs["json"]["currency"] == "MYR" sandbox_client.bind_card( prime="p", card_holder_data=card_holder, currency=Models.Currencies.HKD @@ -800,3 +801,121 @@ def test_malformed_body_is_reported_for_every_endpoint(sandbox_client, card_hold with mock_bad_json(sandbox_client): with pytest.raises(InvalidResponseError): sandbox_client.get_records({"time": {}}) + + +# --- Documented currency set (0.7.1) ------------------------------------- + + +def test_currencies_match_what_tappay_documents(): + """0.7.0 shipped 11 currencies TapPay does not accept. Only these four are + documented: TWD (most acquirers), HKD (Bank of China), MYR (RAZER PAY), + USD (Global Payments).""" + assert {c.value for c in Models.Currencies} == {"TWD", "HKD", "MYR", "USD"} + + +@pytest.mark.parametrize("code", ["JPY", "EUR", "GBP", "KRW", "CNY", "SGD", "VND"]) +def test_undocumented_currencies_are_not_offered_as_members(code): + """These autocompleted in an IDE and looked sanctioned; they were not.""" + assert not hasattr(Models.Currencies, code) + + +def test_unlisted_currency_strings_still_pass_through( + sandbox_client, card_holder, post +): + """The enum narrows what we *suggest*, not what the caller may send.""" + sandbox_client.pay_by_prime( + prime="p", amount=1, details="d", card_holder_data=card_holder, currency="JPY" + ) + + assert post.call_args.kwargs["json"]["currency"] == "JPY" + + +# --- Timeout matches TapPay's guidance (0.7.1) --------------------------- + + +def test_read_timeout_matches_tappays_documented_30_seconds(): + """TapPay documents a 30s timeout because banks are slow at peak; a shorter + one reports failure for a transaction that may have succeeded.""" + connect, read = DEFAULT_TIMEOUT + + assert read == 30.0 + assert connect > 3.0 + + +# --- Benign statuses are per-endpoint (0.7.1) ---------------------------- + + +def test_record_api_end_of_list_is_not_an_error(strict_client): + """status 2 from the Record API means "no more records", which is how + pagination terminates -- it must not raise under raise_on_error.""" + payload = {"status": 2, "msg": "Success", "number_of_transactions": 0} + with mock_post(strict_client, payload=payload): + response = strict_client.get_records({"time": {}}, page=99) + + assert response["status"] == 2 + + +def test_same_status_still_raises_on_a_payment_endpoint(strict_client, card_holder): + """2 is benign only for the Record API; it is not whitelisted globally.""" + with mock_post(strict_client, payload={"status": 2, "msg": "nope"}): + with pytest.raises(TapPayError) as exc_info: + strict_client.pay_by_prime( + prime="p", amount=1, details="d", card_holder_data=card_holder + ) + + assert exc_info.value.status == 2 + + +def test_other_non_zero_statuses_still_raise_on_the_record_api(strict_client): + with mock_post(strict_client, payload={"status": 3, "msg": "Card declined"}): + with pytest.raises(TapPayError): + strict_client.get_records({"time": {}}) + + +def test_benign_statuses_are_scoped_to_known_endpoints(): + assert BENIGN_STATUSES == {"/tpc/transaction/query": frozenset({2})} + + +def test_end_of_list_is_returned_unchanged_without_strict_mode(sandbox_client): + with mock_post(sandbox_client, payload={"status": 2}): + assert sandbox_client.get_records({"time": {}})["status"] == 2 + + +# --- Full refunds (0.7.1) ------------------------------------------------- + + +def test_full_refund_omits_the_amount_field(sandbox_client, post): + """TapPay refunds the whole transaction when `amount` is absent; sending + an amount makes it a partial refund instead.""" + sandbox_client.refund("rec") + + body = post.call_args.kwargs["json"] + assert body == {"rec_trade_id": "rec", "partner_key": "partner_key"} + assert "amount" not in body + + +def test_partial_refund_still_sends_the_amount(sandbox_client, post): + sandbox_client.refund("rec", 100) + + assert post.call_args.kwargs["json"]["amount"] == 100 + + +def test_zero_amount_refund_is_sent_rather_than_treated_as_absent(sandbox_client, post): + """0 is falsy but means a zero-value partial refund, not a full one.""" + sandbox_client.refund("rec", 0) + + assert post.call_args.kwargs["json"]["amount"] == 0 + + +def test_explicit_none_amount_means_a_full_refund(sandbox_client, post): + sandbox_client.refund("rec", amount=None) + + assert "amount" not in post.call_args.kwargs["json"] + + +def test_full_refund_still_accepts_extra_fields(sandbox_client, post): + sandbox_client.refund("rec", bank_refund_id="BR1") + + body = post.call_args.kwargs["json"] + assert body["bank_refund_id"] == "BR1" + assert "amount" not in body