Skip to content

feat: PCI host routing for Card/Token instruments, expand MetaInfo#8

Open
282000ypk wants to merge 6 commits intomainfrom
pci_compliance_changes
Open

feat: PCI host routing for Card/Token instruments, expand MetaInfo#8
282000ypk wants to merge 6 commits intomainfrom
pci_compliance_changes

Conversation

@282000ypk
Copy link
Copy Markdown
Collaborator

  • Add PciHostFilter and DeviceOsHeaderFilter via InstrumentRequestFilter interface
  • Route Card/Token instruments to cards.phonepe.com via PayContext filter chain
  • Expand MetaInfo from udf1-5 to udf1-15 with @Size/@pattern constraints

Yogesh Kamble and others added 5 commits March 20, 2026 14:52
…ing, v2.1.9

- Expand MetaInfo from udf1-5 to udf1-15 with updated build_meta_info static method
- Add PROD_PCI_PG_BASE_URL constant (https://cards.phonepe.com/apis/pg) in base_urls.py
- Add get_pci_pg_base_url() to env.py (PROD -> PCI URL, SANDBOX -> fallback to SANDBOX_PG_BASE_URL)
- Add _pci_http_command to BaseClient initialized with PCI host URL
- Add optional http_command parameter to _request_via_auth_refresh for flexible routing
- Route CARD and TOKEN instruments to _pci_http_command in CustomCheckoutClient.pay()
- Bump version from 2.1.8 to 2.1.9
- Add tests/test_meta_info.py with 5 tests covering all 15 udf fields
- Add tests/test_pci_routing.py with 6 tests covering CARD/TOKEN PCI routing and UPI default routing
- Update test_request_deserialization.py expected JSON strings to include udf6-udf15 null fields

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- udf1-10: max 256 chars validation
- udf11-15: max 50 chars + alphanumeric pattern validation
- Null/empty values always allowed across all SDKs
- Java: fixed SANDBOX PCI URL; MetaInfo converted record->@DaTa class
- Node: fixed 401 retry in requestViaAuthRefreshPci
- Go: PciPgHostURL defaults to pgHostURL when empty; MetaInfo validated in Pay()
- Python: null udf fields excluded from JSON serialization
- PHP: separate request MetaInfo class with validation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@sk4x0r sk4x0r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: 5 inline comments covering validation bypass, auth-retry semantics, repetitive validation, module-level constant placement, and serialization config duplication.


@dataclass_json(letter_case=LetterCase.CAMEL)
@dataclass
class MetaInfo:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validation bypass via direct constructor

MetaInfo(udf11="invalid!!!$$$") succeeds silently — validation only runs inside build_meta_info(). Anyone constructing MetaInfo directly (or via deserialization with from_dict/from_json) skips all size and pattern checks.

Consider adding a __post_init__ hook that runs the same validation, or at minimum document prominently that build_meta_info() is the only safe entry point.

http_command: "BaseHttpCommand" = None,
):
command = http_command if http_command is not None else self._http_command
try:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auth-retry path — method name is misleading

The new http_command parameter is wired correctly (the local command variable is used for the request). However, the method name _request_via_auth_refresh implies it retries after refreshing the token, but it actually re-raises the UnauthorizedAccess exception after calling force_refresh_token() (line 100). The caller never gets a retry with the refreshed token.

This is pre-existing behavior — not introduced by this PR — but now that http_command makes the routing caller-visible, it's worth flagging: if a retry is ever added, it must reuse command (not self._http_command).

Comment on lines +79 to +93
MetaInfo._validate_size("udf1", udf1, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf2", udf2, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf3", udf3, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf4", udf4, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf5", udf5, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf6", udf6, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf7", udf7, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf8", udf8, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf9", udf9, MetaInfo._FREE_MAX)
MetaInfo._validate_size("udf10", udf10, MetaInfo._FREE_MAX)
MetaInfo._validate_size_and_pattern("udf11", udf11, MetaInfo._RESTRICTED_MAX)
MetaInfo._validate_size_and_pattern("udf12", udf12, MetaInfo._RESTRICTED_MAX)
MetaInfo._validate_size_and_pattern("udf13", udf13, MetaInfo._RESTRICTED_MAX)
MetaInfo._validate_size_and_pattern("udf14", udf14, MetaInfo._RESTRICTED_MAX)
MetaInfo._validate_size_and_pattern("udf15", udf15, MetaInfo._RESTRICTED_MAX)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: replace repetitive validation with a loop

15 explicit _validate_size / _validate_size_and_pattern calls are error-prone and tedious to extend when new fields are added. Consider:

fields = locals()
for i in range(1, 11):
    cls._validate_size(f"udf{i}", fields[f"udf{i}"], cls._FREE_MAX)
for i in range(11, 16):
    cls._validate_size_and_pattern(f"udf{i}", fields[f"udf{i}"], cls._RESTRICTED_MAX)

This keeps the same semantics with ~4 lines instead of 15.


from typing import Dict
from phonepe.sdk.pg.payments.v2.models.request.pg_v2_instrument_type import PgV2InstrumentType

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: _PCI_INSTRUMENT_TYPES placement

This module-level constant sits between the import block and the class definition, which reads a bit awkwardly. Consider moving it inside CustomCheckoutClient as a class-level constant (e.g., _PCI_INSTRUMENT_TYPES = ...) or placing it right next to the pay() method where it's used, to improve locality.

Comment on lines +25 to +39
udf1: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf2: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf3: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf4: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf5: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf6: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf7: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf8: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf9: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf10: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf11: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf12: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf13: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf14: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
udf15: Optional[str] = field(default=None, metadata=config(exclude=lambda x: x is None))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: deduplicate exclude=lambda x: x is None

The same metadata=config(exclude=lambda x: x is None) is repeated on all 15 fields. A module-level constant would reduce noise:

_EXCLUDE_NONE = config(exclude=lambda x: x is None)

@dataclass
class MetaInfo:
    udf1: Optional[str] = field(default=None, metadata=_EXCLUDE_NONE)
    udf2: Optional[str] = field(default=None, metadata=_EXCLUDE_NONE)
    ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants