🐛Fixes rpc cancellation in webserver and extra tests on one-time-paym…

…ent workflow (ITISFoundation#4923)
sanderegg · Oct 27, 2023 · 5f4a44c · 5f4a44c
1 parent 6bccd73
commit 5f4a44c
Show file tree

Hide file tree

Showing 15 changed files with 183 additions and 40 deletions.
diff --git a/packages/models-library/src/models_library/api_schemas_webserver/wallets.py b/packages/models-library/src/models_library/api_schemas_webserver/wallets.py
@@ -49,7 +49,9 @@ class PutWalletBodyParams(OutputSchema):
 # Payments to top-up credits in wallets
 #
 
+# NOTE: that these can be UUIDs (or not)
 PaymentID: TypeAlias = IDStr
+PaymentMethodID: TypeAlias = IDStr
 
 
 class CreateWalletPayment(InputSchema):
@@ -80,9 +82,6 @@ class PaymentTransaction(OutputSchema):
     invoice_url: HttpUrl = FieldNotRequired()
 
 
-PaymentMethodID: TypeAlias = IDStr
-
-
 class PaymentMethodInit(OutputSchema):
     wallet_id: WalletID
     payment_method_id: PaymentMethodID

diff --git a/packages/models-library/src/models_library/basic_types.py b/packages/models-library/src/models_library/basic_types.py
@@ -72,13 +72,12 @@ class UUIDStr(ConstrainedStr):
     regex = re.compile(UUID_RE)
 
 
-# non-empty string identifier e.g. "123" or "name_id1" (avoids "" identifiers)
-class NonEmptyStr(ConstrainedStr):
+# non-empty bounded string used as identifier
+# e.g. "123" or "name_123" or "fa327c73-52d8-462a-9267-84eeaf0f90e3" but NOT ""
+class IDStr(ConstrainedStr):
     strip_whitespace = True
     min_length = 1
-
-
-IDStr: TypeAlias = NonEmptyStr
+    max_length = 50
 
 
 # auto-incremented primary-key IDs

diff --git a/packages/models-library/tests/test_basic_types.py b/packages/models-library/tests/test_basic_types.py
@@ -1,7 +1,14 @@
 from typing import NamedTuple
 
 import pytest
-from models_library.basic_types import EnvVarKey, MD5Str, SHA1Str, UUIDStr, VersionTag
+from models_library.basic_types import (
+    EnvVarKey,
+    IDStr,
+    MD5Str,
+    SHA1Str,
+    UUIDStr,
+    VersionTag,
+)
 from pydantic import ConstrainedStr, ValidationError
 from pydantic.tools import parse_obj_as
 
@@ -30,6 +37,11 @@ class _Example(NamedTuple):
         good="d2cbbd98-d0f8-4de1-864e-b390713194eb",
         bad="123456-is-not-an-uuid",
     ),
+    _Example(
+        constr=IDStr,
+        good="d2cbbd98-d0f8-4de1-864e-b390713194eb",  # as an uuid
+        bad="",  # empty string not allowed
+    ),
 ]
 
 
@@ -50,3 +62,14 @@ def test_constrained_str_succeeds(
 def test_constrained_str_fails(constraint_str_type: type[ConstrainedStr], sample: str):
     with pytest.raises(ValidationError):
         parse_obj_as(constraint_str_type, sample)
+
+
+def test_string_identifier_constraint_type():
+
+    # strip spaces
+    assert parse_obj_as(IDStr, "   123 trim spaces   ") == "123 trim spaces"
+
+    # limited to 50!
+    parse_obj_as(IDStr, "X" * 50)
+    with pytest.raises(ValidationError):
+        parse_obj_as(IDStr, "X" * 51)
diff --git a/services/payments/VERSION b/services/payments/VERSION
@@ -1 +1 @@
-1.3.0
+1.3.1
diff --git a/services/payments/openapi.json b/services/payments/openapi.json
@@ -3,7 +3,7 @@
   "info": {
     "title": "simcore-service-payments web API",
     "description": " Service that manages creation and validation of registration payments",
-    "version": "1.3.0"
+    "version": "1.3.1"
   },
   "paths": {
     "/": {
@@ -104,7 +104,8 @@
             "required": true,
             "schema": {
               "type": "string",
-              "format": "uuid",
+              "maxLength": 50,
+              "minLength": 1,
               "title": "Payment Id"
             },
             "name": "payment_id",
@@ -161,7 +162,8 @@
             "required": true,
             "schema": {
               "type": "string",
-              "format": "uuid",
+              "maxLength": 50,
+              "minLength": 1,
               "title": "Payment Method Id"
             },
             "name": "payment_method_id",
@@ -360,7 +362,8 @@
           },
           "payment_method_id": {
             "type": "string",
-            "format": "uuid",
+            "maxLength": 50,
+            "minLength": 1,
             "title": "Payment Method Id"
           }
         },

diff --git a/services/payments/setup.cfg b/services/payments/setup.cfg
@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 1.3.0
+current_version = 1.3.1
 commit = True
 message = services/payments version: {current_version} → {new_version}
 tag = False

diff --git a/services/payments/src/simcore_service_payments/api/rest/_acknowledgements.py b/services/payments/src/simcore_service_payments/api/rest/_acknowledgements.py
@@ -106,7 +106,7 @@ async def acknowledge_payment(
         )
 
     if transaction.state == PaymentTransactionState.SUCCESS:
-        assert payment_id == transaction.payment_id  # nosec
+        assert f"{payment_id}" == f"{transaction.payment_id}"  # nosec
         background_tasks.add_task(on_payment_completed, transaction, rut_api)
 
 

diff --git a/services/payments/src/simcore_service_payments/api/rpc/_payments.py b/services/payments/src/simcore_service_payments/api/rpc/_payments.py
@@ -3,7 +3,7 @@
 
 import arrow
 from fastapi import FastAPI
-from models_library.api_schemas_webserver.wallets import WalletPaymentCreated
+from models_library.api_schemas_webserver.wallets import PaymentID, WalletPaymentCreated
 from models_library.users import UserID
 from models_library.wallets import WalletID
 from pydantic import EmailStr
@@ -19,7 +19,7 @@
 
 from ..._constants import PAG, PGDB
 from ...db.payments_transactions_repo import PaymentsTransactionsRepo
-from ...models.payments_gateway import InitPayment, PaymentID, PaymentInitiated
+from ...models.payments_gateway import InitPayment, PaymentInitiated
 from ...services.payments_gateway import PaymentsGatewayApi
 
 _logger = logging.getLogger(__name__)
@@ -104,7 +104,6 @@ async def cancel_payment(
     user_id: UserID,
     wallet_id: WalletID,
 ) -> None:
-
     # validation
     repo = PaymentsTransactionsRepo(db_engine=app.state.engine)
     payment = await repo.get_payment_transaction(

diff --git a/services/payments/src/simcore_service_payments/db/payments_transactions_repo.py b/services/payments/src/simcore_service_payments/db/payments_transactions_repo.py
@@ -2,6 +2,7 @@
 from decimal import Decimal
 
 import sqlalchemy as sa
+from models_library.api_schemas_webserver.wallets import PaymentID
 from models_library.users import UserID
 from models_library.wallets import WalletID
 from pydantic import HttpUrl, PositiveInt, parse_obj_as
@@ -18,7 +19,6 @@
     PaymentNotFoundError,
 )
 from ..models.db import PaymentsTransactionsDB
-from ..models.payments_gateway import PaymentID
 
 
 class BaseRepository:

diff --git a/services/payments/src/simcore_service_payments/models/payments_gateway.py b/services/payments/src/simcore_service_payments/models/payments_gateway.py
@@ -1,9 +1,9 @@
 from datetime import datetime
 from pathlib import Path
-from typing import Literal, TypeAlias
-from uuid import UUID
+from typing import Literal
 
-from models_library.basic_types import AmountDecimal, NonEmptyStr
+from models_library.api_schemas_webserver.wallets import PaymentID, PaymentMethodID
+from models_library.basic_types import AmountDecimal, IDStr
 from pydantic import BaseModel, EmailStr, Extra, Field
 
 
@@ -19,17 +19,14 @@ class InitPayment(BaseModel):
     amount_dollars: AmountDecimal
     # metadata to store for billing or reference
     credits_: AmountDecimal = Field(..., alias="credits")
-    user_name: NonEmptyStr
+    user_name: IDStr
     user_email: EmailStr
-    wallet_name: NonEmptyStr
+    wallet_name: IDStr
 
     class Config:
         extra = Extra.forbid
 
 
-PaymentID: TypeAlias = UUID
-
-
 class PaymentInitiated(BaseModel):
     payment_id: PaymentID
 
@@ -38,15 +35,12 @@ class PaymentCancelled(BaseModel):
     message: str | None = None
 
 
-PaymentMethodID: TypeAlias = UUID
-
-
 class InitPaymentMethod(BaseModel):
     method: Literal["CC"] = "CC"
     # metadata to store for billing or reference
-    user_name: NonEmptyStr
+    user_name: IDStr
     user_email: EmailStr
-    wallet_name: NonEmptyStr
+    wallet_name: IDStr
 
     class Config:
         extra = Extra.forbid

diff --git a/services/payments/src/simcore_service_payments/models/schemas/acknowledgements.py b/services/payments/src/simcore_service_payments/models/schemas/acknowledgements.py
@@ -1,15 +1,19 @@
 from typing import Any, ClassVar
 
+from models_library.api_schemas_webserver.wallets import PaymentID, PaymentMethodID
 from pydantic import BaseModel, Field, HttpUrl, validator
 
-from ..payments_gateway import PaymentID, PaymentMethodID
-
 
 class _BaseAck(BaseModel):
     success: bool
     message: str = Field(default=None)
 
 
+#
+# ACK payment-methods
+#
+
+
 class AckPaymentMethod(_BaseAck):
     ...
 
@@ -49,6 +53,11 @@ class SavedPaymentMethod(AckPaymentMethod):
 ]
 
 
+#
+# ACK one-time payments
+#
+
+
 class AckPayment(_BaseAck):
     invoice_url: HttpUrl | None = Field(
         default=None, description="Link to invoice is required when success=true"

diff --git a/services/payments/src/simcore_service_payments/services/payments_gateway.py b/services/payments/src/simcore_service_payments/services/payments_gateway.py
@@ -12,16 +12,15 @@
 from fastapi import FastAPI
 from fastapi.encoders import jsonable_encoder
 from httpx import URL
+from models_library.api_schemas_webserver.wallets import PaymentID, PaymentMethodID
 
 from ..core.settings import ApplicationSettings
 from ..models.payments_gateway import (
     GetPaymentMethod,
     InitPayment,
     InitPaymentMethod,
     PaymentCancelled,
-    PaymentID,
     PaymentInitiated,
-    PaymentMethodID,
     PaymentMethodInitiated,
 )
 from ..utils.http_client import AppStateMixin, BaseHttpApi

diff --git a/services/payments/tests/unit/api/test__one_time_payment_workflows.py b/services/payments/tests/unit/api/test__one_time_payment_workflows.py
@@ -0,0 +1,114 @@
+# pylint: disable=redefined-outer-name
+# pylint: disable=unused-argument
+# pylint: disable=unused-variable
+# pylint: disable=too-many-arguments
+
+
+from collections.abc import Awaitable, Callable
+from typing import Any
+
+import httpx
+import pytest
+from faker import Faker
+from fastapi import FastAPI, status
+from models_library.api_schemas_webserver.wallets import WalletPaymentCreated
+from pydantic import parse_obj_as
+from pytest_mock import MockerFixture
+from pytest_simcore.helpers.typing_env import EnvVarsDict
+from pytest_simcore.helpers.utils_envs import setenvs_from_dict
+from respx import MockRouter
+from servicelib.rabbitmq import RabbitMQRPCClient, RPCMethodName
+from simcore_service_payments.api.rpc.routes import PAYMENTS_RPC_NAMESPACE
+from simcore_service_payments.models.schemas.acknowledgements import AckPayment
+
+pytest_simcore_core_services_selection = [
+    "postgres",
+    "rabbit",
+]
+pytest_simcore_ops_services_selection = [
+    "adminer",
+]
+
+
+@pytest.fixture
+def app_environment(
+    monkeypatch: pytest.MonkeyPatch,
+    app_environment: EnvVarsDict,
+    rabbit_env_vars_dict: EnvVarsDict,  # rabbitMQ settings from 'rabbit' service
+    postgres_env_vars_dict: EnvVarsDict,
+    wait_for_postgres_ready_and_db_migrated: None,
+):
+    # set environs
+    monkeypatch.delenv("PAYMENTS_RABBITMQ", raising=False)
+    monkeypatch.delenv("PAYMENTS_POSTGRES", raising=False)
+
+    return setenvs_from_dict(
+        monkeypatch,
+        {
+            **app_environment,
+            **rabbit_env_vars_dict,
+            **postgres_env_vars_dict,
+            "POSTGRES_CLIENT_NAME": "payments-service-pg-client",
+        },
+    )
+
+
+@pytest.fixture
+def init_payment_kwargs(faker: Faker) -> dict[str, Any]:
+    return {
+        "amount_dollars": 1000,
+        "target_credits": 10000,
+        "product_name": "osparc",
+        "wallet_id": 1,
+        "wallet_name": "wallet-name",
+        "user_id": 1,
+        "user_name": "user",
+        "user_email": "[email protected]",
+    }
+
+
+@pytest.mark.acceptance_test(
+    "https://github.com/ITISFoundation/osparc-simcore/pull/4715"
+)
+async def test_successful_one_time_payment_workflow(
+    app: FastAPI,
+    client: httpx.AsyncClient,
+    faker: Faker,
+    rabbitmq_rpc_client: Callable[[str], Awaitable[RabbitMQRPCClient]],
+    mock_payments_gateway_service_or_none: MockRouter | None,
+    init_payment_kwargs: dict[str, Any],
+    auth_headers: dict[str, str],
+    payments_clean_db: None,
+    mocker: MockerFixture,
+):
+    assert (
+        mock_payments_gateway_service_or_none
+    ), "cannot run against external because we ACK here"
+
+    mock_on_payment_completed = mocker.patch(
+        "simcore_service_payments.api.rest._acknowledgements.on_payment_completed",
+        autospec=True,
+    )
+
+    rpc_client = await rabbitmq_rpc_client("web-server-client")
+
+    # INIT
+    result = await rpc_client.request(
+        PAYMENTS_RPC_NAMESPACE,
+        parse_obj_as(RPCMethodName, "init_payment"),
+        **init_payment_kwargs,
+        timeout_s=None,  # for debug
+    )
+    assert isinstance(result, WalletPaymentCreated)
+
+    assert mock_payments_gateway_service_or_none.routes["init_payment"].called
+
+    # ACK
+    response = await client.post(
+        f"/v1/payments/{result.payment_id}:ack",
+        json=AckPayment(success=True, invoice_url=faker.url()).dict(),
+        headers=auth_headers,
+    )
+
+    assert response.status_code == status.HTTP_200_OK
+    assert mock_on_payment_completed.called