refactor(proto)!: use Connector enum for Apple Pay session token connector field - #2128
Open
Utkal059 wants to merge 1 commit into
Open
refactor(proto)!: use Connector enum for Apple Pay session token connector field#2128Utkal059 wants to merge 1 commit into
Utkal059 wants to merge 1 commit into
Conversation
`ApplepayClientAuthenticationResponse.connector` was a free-form string that every caller filled in with the connector's snake_case name, so an invalid value could only be caught on the far side of the wire. It is now the `Connector` enum the proto already defines, which is how `Currency`, `CountryAlpha2` and `SdkNextAction` are already carried on this message. Domain-side the field becomes `ConnectorEnum`, and braintree and trustpay -- the two connectors that build an Apple Pay session token -- pass the variant instead of a string literal. A new `ForeignTryFrom<ConnectorEnum>` for the proto `Connector` does the domain -> proto mapping through the SCREAMING_SNAKE_CASE name, mirroring the existing `CountryAlpha2` conversion, and errors rather than falling back to `CONNECTOR_UNSPECIFIED`. This changes the wire type of field 3, so `buf breaking` will flag it and the PR needs the `proto-breaking-approved` label. The consumer migration is juspay/hyperswitch#13481, which switches the same field on `ApplepaySessionTokenResponse` to hyperswitch's `Connector` enum and will bump the `unified-connector-service-client` tag once this lands.
14 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Apple Pay client-auth/session-token payload to carry the connector as a protobuf Connector enum instead of a free-form string, aligning the gRPC contract with existing enum usage and making connector mapping explicit at conversion boundaries.
Changes:
- Changed
ApplepayClientAuthenticationResponse.connectorinpayment.protofromstringtoConnector(enum). - Updated domain model + gRPC conversion to use
ConnectorEnumand added aForeignTryFrom<ConnectorEnum>mapping to the proto enum (with a unit test). - Updated Braintree and Trustpay Apple Pay session-token builders to pass
ConnectorEnum::{Braintree, Trustpay}.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/types-traits/grpc-api-types/proto/payment.proto | Wire-contract change: Apple Pay connector field is now a proto enum. |
| crates/types-traits/domain_types/src/types.rs | Converts ConnectorEnum to proto Connector during Apple Pay gRPC response building; adds test coverage. |
| crates/types-traits/domain_types/src/connector_types.rs | Domain Apple Pay response now stores connector as ConnectorEnum. |
| crates/integrations/connector-integration/src/connectors/trustpay/transformers.rs | Uses ConnectorEnum::Trustpay when constructing Apple Pay session token data. |
| crates/integrations/connector-integration/src/connectors/braintree/transformers.rs | Uses ConnectorEnum::Braintree when constructing Apple Pay session token data. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
5363
to
5366
| pub payment_request_data: Option<ApplePayPaymentRequest>, | ||
| /// The session token is w.r.t this connector | ||
| pub connector: String, | ||
| pub connector: ConnectorEnum, | ||
| /// Identifier for the delayed session response |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
ApplepayClientAuthenticationResponse.connectorwasstring, even though every callerfills it in with a connector name that already exists as a variant of the
Connectorenumin the same file. This makes it
Connector.Currency,CountryAlpha2andSdkNextActionare already carried as proto enums on thismessage, so the string was the odd one out.
Changes:
payment.proto—string connector = 3;becomesConnector connector = 3;onApplepayClientAuthenticationResponse.domain_types/connector_types.rs— the matching domain field becomesConnectorEnum.token now pass
ConnectorEnum::Braintree/ConnectorEnum::Trustpayinstead ofBRAINTREE_CONNECTOR_NAME.to_string()/"trustpay".to_string(). Same value at runtime;BRAINTREE_CONNECTOR_NAMEstays, since the Google Pay and PayPal responses on the samemessage still use it.
domain_types/types.rs— a newForeignTryFrom<ConnectorEnum>forgrpc_api_types::payments::Connector, used by both grpc conversion sites. It resolvesthrough the SCREAMING_SNAKE_CASE name exactly like the existing
CountryAlpha2conversion, and returns
UnexpectedResponseErrorrather than falling back toCONNECTOR_UNSPECIFIED, so a missing mapping surfaces as an error instead of anunusable session token.
I scoped this to the Apple Pay message to keep it symmetric with the hyperswitch PR.
GooglePaySessionResponse.connector,GooglePayThirdPartySdk.connectorandPaypalClientAuthenticationResponse.connectorare stillstringand have the sameproblem — happy to follow up on those separately if you want them moved too.
Motivation and Context
Follow-up to review feedback on juspay/hyperswitch#13481, which changes the same field
on hyperswitch's
ApplepaySessionTokenResponsefromStringto hyperswitch'sConnectorenum. Because UCS sends the field as a string, that PR currently has to parse it at the
boundary with
Connector::from_str. @hrithikesh026 pointed out that this coupleshyperswitch to UCS's serialization format, and @Nithin1506200 asked for the enum change to
be made here so the conversion becomes explicit on both sides.
Ordering: this lands and gets tagged first, then hyperswitch#13481 bumps
unified-connector-service-clientand reads the enum directly.Additional Changes
This is a wire-breaking change on field 3 (
stringis length-delimited, an enum is avarint), so
buf breakingwill fail under theFILEruleset. Per the guidance theproto-checks job prints, that means the PR needs the
proto-breaking-approvedlabel — Ican't add it myself.
Heads up on a conflict between two of your workflows, which cost me a red check here: the
Fail on unapproved breaking changestep inproto-checks.ymltells you to "useproto!:as the PR title type", but
Verify PR title follows conventional commit standardsrejectsit with
Commit type `proto` not allowed. That job runs onpull_request_targetwith nocheckout step (deliberately, per the comment at the top of the file), so
cog.toml— whereprotois registered as a commit type — is never on disk, and cocogitto falls back to itsbuilt-in types. Every merged proto change I looked at uses the scope form instead
(
feat(proto):,fix(proto):), so I've retitled this onerefactor(proto)!:. Either theproto-checks message or the title job's config probably wants a fix.
The one checklist item that isn't satisfied yet is "migration PR is already merged and
deployed": hyperswitch#13481 is open, and pins UCS by git tag, so it won't see this change
until it bumps the tag. That's the sequence @Nithin1506200 proposed on that PR ("we will
get it merged and you can bump it here later"), but if you'd rather not break the wire at
all, the alternative is the additive route the same job suggests — add
Connector connector_type = 9;, mark field 3[deprecated = true], and drop it in afollow-up once consumers have moved. Say the word and I'll rework it that way.
Also worth flagging, found while checking the mapping is total:
ConnectorEnum::RazorpayV2and
ConnectorEnum::Affirmhave no counterpart in the protoConnectorenum (there isRAZORPAYbut noRAZORPAY_V2, and noAFFIRMat all). Todaygrpc_connector_from_connector_enumquietly turns both intoCONNECTOR_UNSPECIFIED.Neither serves an Apple Pay session, so nothing here regresses, but it's a pre-existing gap
worth a separate look.
How did you test it?
Added
apple_pay_session_connectors_map_to_their_proto_variantsindomain_types/src/types.rs, covering the only two connectors that build an Apple Paysession token — it asserts
ConnectorEnum::BraintreeandConnectorEnum::Trustpayland onConnector::BraintreeandConnector::Trustpay, which is what breaks if either side isrenamed.
On the hyperswitch side, hyperswitch#13481 has a matching test pinning
"braintree"and"trustpay"as the exact strings UCS sends today.I wasn't able to run the full
cargo clippy/cargo nextestmatrix locally — thisworkspace needs more memory than my machine has — so I'm relying on CI for those and will
turn around anything that comes back red.
cargo +nightly fmt --checkis clean on everyfile I touched.