fix: tighten payment legacy fallback paths
This commit is contained in:
@@ -25,9 +25,9 @@ func (s *PaymentService) HandlePaymentNotification(ctx context.Context, n *payme
|
|||||||
// Look up order by out_trade_no (the external order ID we sent to the provider)
|
// Look up order by out_trade_no (the external order ID we sent to the provider)
|
||||||
order, err := s.entClient.PaymentOrder.Query().Where(paymentorder.OutTradeNo(n.OrderID)).Only(ctx)
|
order, err := s.entClient.PaymentOrder.Query().Where(paymentorder.OutTradeNo(n.OrderID)).Only(ctx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// Fallback: try legacy format (sub2_N where N is DB ID)
|
// Fallback only for true legacy "sub2_N" DB-ID payloads when the
|
||||||
trimmed := strings.TrimPrefix(n.OrderID, orderIDPrefix)
|
// current out_trade_no lookup genuinely did not find an order.
|
||||||
if oid, parseErr := strconv.ParseInt(trimmed, 10, 64); parseErr == nil {
|
if oid, ok := parseLegacyPaymentOrderID(n.OrderID, err); ok {
|
||||||
return s.confirmPayment(ctx, oid, n.TradeNo, n.Amount, pk, n.Metadata)
|
return s.confirmPayment(ctx, oid, n.TradeNo, n.Amount, pk, n.Metadata)
|
||||||
}
|
}
|
||||||
return fmt.Errorf("order not found for out_trade_no: %s", n.OrderID)
|
return fmt.Errorf("order not found for out_trade_no: %s", n.OrderID)
|
||||||
@@ -35,6 +35,25 @@ func (s *PaymentService) HandlePaymentNotification(ctx context.Context, n *payme
|
|||||||
return s.confirmPayment(ctx, order.ID, n.TradeNo, n.Amount, pk, n.Metadata)
|
return s.confirmPayment(ctx, order.ID, n.TradeNo, n.Amount, pk, n.Metadata)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func parseLegacyPaymentOrderID(orderID string, lookupErr error) (int64, bool) {
|
||||||
|
if !dbent.IsNotFound(lookupErr) {
|
||||||
|
return 0, false
|
||||||
|
}
|
||||||
|
orderID = strings.TrimSpace(orderID)
|
||||||
|
if !strings.HasPrefix(orderID, orderIDPrefix) {
|
||||||
|
return 0, false
|
||||||
|
}
|
||||||
|
trimmed := strings.TrimPrefix(orderID, orderIDPrefix)
|
||||||
|
if trimmed == "" || trimmed == orderID {
|
||||||
|
return 0, false
|
||||||
|
}
|
||||||
|
oid, err := strconv.ParseInt(trimmed, 10, 64)
|
||||||
|
if err != nil || oid <= 0 {
|
||||||
|
return 0, false
|
||||||
|
}
|
||||||
|
return oid, true
|
||||||
|
}
|
||||||
|
|
||||||
func (s *PaymentService) confirmPayment(ctx context.Context, oid int64, tradeNo string, paid float64, pk string, metadata map[string]string) error {
|
func (s *PaymentService) confirmPayment(ctx context.Context, oid int64, tradeNo string, paid float64, pk string, metadata map[string]string) error {
|
||||||
o, err := s.entClient.PaymentOrder.Get(ctx, oid)
|
o, err := s.entClient.PaymentOrder.Get(ctx, oid)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -307,3 +307,17 @@ func TestValidateProviderNotificationMetadataAllowsLegacyOrdersWithoutSnapshotFi
|
|||||||
})
|
})
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestParseLegacyPaymentOrderID(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
oid, ok := parseLegacyPaymentOrderID("sub2_42", &dbent.NotFoundError{})
|
||||||
|
assert.True(t, ok)
|
||||||
|
assert.EqualValues(t, 42, oid)
|
||||||
|
|
||||||
|
_, ok = parseLegacyPaymentOrderID("42", &dbent.NotFoundError{})
|
||||||
|
assert.False(t, ok)
|
||||||
|
|
||||||
|
_, ok = parseLegacyPaymentOrderID("sub2_42", errors.New("db down"))
|
||||||
|
assert.False(t, ok)
|
||||||
|
}
|
||||||
|
|||||||
@@ -292,10 +292,48 @@ func (s *PaymentService) getOrderProvider(ctx context.Context, o *dbent.PaymentO
|
|||||||
if inst != nil {
|
if inst != nil {
|
||||||
return s.createProviderFromInstance(ctx, inst)
|
return s.createProviderFromInstance(ctx, inst)
|
||||||
}
|
}
|
||||||
|
if !paymentOrderAllowsRegistryFallback(o) {
|
||||||
|
return nil, fmt.Errorf("order %d provider instance is unresolved", o.ID)
|
||||||
|
}
|
||||||
|
providerKey := paymentOrderFallbackProviderKey(s.registry, o)
|
||||||
|
if providerKey == "" {
|
||||||
|
return nil, fmt.Errorf("order %d provider fallback key is missing", o.ID)
|
||||||
|
}
|
||||||
|
if !s.webhookRegistryFallbackAllowed(ctx, providerKey) {
|
||||||
|
return nil, fmt.Errorf("order %d provider fallback is ambiguous for %s", o.ID, providerKey)
|
||||||
|
}
|
||||||
s.EnsureProviders(ctx)
|
s.EnsureProviders(ctx)
|
||||||
return s.registry.GetProvider(o.PaymentType)
|
return s.registry.GetProvider(o.PaymentType)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func paymentOrderAllowsRegistryFallback(order *dbent.PaymentOrder) bool {
|
||||||
|
if order == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if psOrderProviderSnapshot(order) != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(psStringValue(order.ProviderInstanceID)) != "" {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(psStringValue(order.ProviderKey)) != "" {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
func paymentOrderFallbackProviderKey(registry *payment.Registry, order *dbent.PaymentOrder) string {
|
||||||
|
if order == nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
if registry != nil {
|
||||||
|
if key := strings.TrimSpace(registry.GetProviderKey(payment.PaymentType(order.PaymentType))); key != "" {
|
||||||
|
return key
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return strings.TrimSpace(payment.GetBasePaymentType(strings.TrimSpace(order.PaymentType)))
|
||||||
|
}
|
||||||
|
|
||||||
func (s *PaymentService) createProviderFromInstance(ctx context.Context, inst *dbent.PaymentProviderInstance) (payment.Provider, error) {
|
func (s *PaymentService) createProviderFromInstance(ctx context.Context, inst *dbent.PaymentProviderInstance) (payment.Provider, error) {
|
||||||
if inst == nil {
|
if inst == nil {
|
||||||
return nil, fmt.Errorf("payment provider instance is missing")
|
return nil, fmt.Errorf("payment provider instance is missing")
|
||||||
|
|||||||
@@ -323,6 +323,43 @@ func TestVerifyOrderByOutTradeNoUsesOutTradeNoWhenPaymentTradeNoAlreadyExistsFor
|
|||||||
require.Equal(t, "upstream-trade-existing", got.PaymentTradeNo)
|
require.Equal(t, "upstream-trade-existing", got.PaymentTradeNo)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestPaymentOrderAllowsRegistryFallbackOnlyForLegacyOrdersWithoutPinnedProviderState(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
require.True(t, paymentOrderAllowsRegistryFallback(&dbent.PaymentOrder{
|
||||||
|
PaymentType: payment.TypeAlipay,
|
||||||
|
}))
|
||||||
|
|
||||||
|
instanceID := "12"
|
||||||
|
require.False(t, paymentOrderAllowsRegistryFallback(&dbent.PaymentOrder{
|
||||||
|
PaymentType: payment.TypeAlipay,
|
||||||
|
ProviderInstanceID: &instanceID,
|
||||||
|
}))
|
||||||
|
|
||||||
|
require.False(t, paymentOrderAllowsRegistryFallback(&dbent.PaymentOrder{
|
||||||
|
PaymentType: payment.TypeAlipay,
|
||||||
|
ProviderSnapshot: map[string]any{
|
||||||
|
"schema_version": 2,
|
||||||
|
"provider_instance_id": "12",
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestPaymentOrderQueryReferenceUsesOutTradeNoForOfficialProviders(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
order := &dbent.PaymentOrder{
|
||||||
|
PaymentType: payment.TypeWxpay,
|
||||||
|
OutTradeNo: "sub2_out_trade_no",
|
||||||
|
PaymentTradeNo: "wx-transaction-id",
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Equal(t, "sub2_out_trade_no", paymentOrderQueryReference(order, &paymentOrderLifecycleQueryProvider{}))
|
||||||
|
require.Equal(t, "sub2_out_trade_no", paymentOrderQueryReference(order, paymentFulfillmentTestProvider{
|
||||||
|
key: payment.TypeWxpay,
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
|
||||||
func newPaymentOrderLifecycleTestClient(t *testing.T) *dbent.Client {
|
func newPaymentOrderLifecycleTestClient(t *testing.T) *dbent.Client {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user