refactor(ads): move reward verification to coresdk#6895
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
Merge PR #6895 (refactor: move reward verification to coresdk) into the ADS-306 failure-reason work and re-apply the feature onto the relocated layout: - Poller / Outcome / RewardVerificationResult now live in Sources/Ads/RewardVerification; reward-verification log strings live in AdsStrings (core). The feature is re-applied there: failure taxonomy (backendRejected(reason:message:), exhaustedPending, exhaustedTransient, unexpectedResponse, terminalError, cancelled), last-observed-disposition exhaustion classification, reason+message threading with reason fallback, and the cause-specific diagnostics. - Purchases.pollRewardVerificationStatus threads failure_reason/message into the SPI poll status. - Adapter Dispatcher always delivers on cancellation (no silent drop); dead outcome_cancelled string removed. - Tests relocated to Tests/UnitTests/Ads/RewardVerification re-apply the new assertions; the cancellation tests are deterministic (cooperative hang). Refs ADS-306
e1cd1d8 to
5be234b
Compare
5be234b to
11b49ae
Compare
…mEntitlementComputation
ajpallares
left a comment
There was a problem hiding this comment.
Did another pass! We're getting there, I only have some smaller comments.
Also, the PR description still says pollRewardVerification(transactionId:), but the current state is shipped as pollRewardVerification(clientTransactionID:)
…RewardVerificationTests.swift Co-authored-by: Antonio Pallares <ajpallares@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 461fe0b. Configure here.
| public let appUserID: String | ||
|
|
||
| /// Creates a token for reward verification | ||
| @_spi(Experimental) public init( |
There was a problem hiding this comment.
Shouldn't this be @_spi(Internal) at most?
Probably internal even, since the SDK would create this to return it but it shouldn't get instantiated from outside
There was a problem hiding this comment.
Good catch! It can't be internal because we are using it from a different module for unit tests - but I switched it to @_spi(Internal) 👍
ajpallares
left a comment
There was a problem hiding this comment.
Great work! Just a suggestion to add some API tests, but probably better done in a follow-up PR
There was a problem hiding this comment.
Even if marked as @_spi(Experimental), I think the new reward-verification APIs (generateRewardVerificationToken, pollRewardVerification, RewardVerificationToken, RewardVerificationResult) should be covered in Tests/APITesters/.
SwiftAPITester/PurchasesAPI.swift already imports @_spi(Experimental), so there's an existing pattern to follow.
Even if we end up modifying these APIs, it will lock the signatures and make changes more intentional, while also making sure these are covered by API tests when we remove the Experimental mark.
Please, let's add these API tests in a follow-up PR so that this one doesn't grow bigger 🙏
|
@RCGitBot please test |
|
@RCGitBot please test |
|
@RCGitBot please test |
Motivation
Move reward-verification (SSV) logic from the AdMob adapter into core, and expose it as two primitives. The goal with this is to make it available outside the RC AdMob Adapter (for example hybrids, or later non-admob providers). The adapter and future bridges share one core implementation instead of each reimplementing it.
Description
Relocates the non-AdMob-specific SSV logic (the poll loop, outcome/result types) from
RevenueCatAdMobinto coreRevenueCat, and exposes two@_spi(Experimental)methods onPurchases:generateRewardVerificationToken(impressionId:)— mints the SSV token (transaction id + custom-data payload + appuser id) at ad load.
pollRewardVerification(clientTransactionID:)— runs the verification poll loop and invalidates the virtual-currenciescache on a verified VC reward.
RevenueCatAdMobbecomes a thin wrapper that calls these; its AdMob-specific glue (Setup/State/Dispatcher) stays in the adapter.Notes
@_spiconsumers: code inspecting the result now needs@_spi(Experimental) import RevenueCatin addition to the adapter import (the result types moved core-ward).@objcwireability is intentionally not in this PR (tracked separately) — these primitives are Swift-shaped for now; the AdMob adapter consumes them today.Note
Medium Risk
Refactors the rewarded-ad verification and virtual-currency cache path across core and the AdMob adapter; intended behavior parity but any wiring mistake could affect SSV or VC freshness.
Overview
Moves server-side reward verification (SSV) out of
RevenueCatAdMobinto coreRevenueCat, so adapters and future integrations share one implementation.Core adds
@_spi(Experimental)Purchases.generateRewardVerificationToken(impressionId:)(SSV JSON payload + transaction id) andPurchases.pollRewardVerification(clientTransactionID:), which runs the movedRewardVerification.Pollerloop, maps toRewardVerificationResult, and invalidates virtual currencies cache on verified virtual-currency rewards.RewardVerificationToken, internalOutcome, and poll logging move toAdsStringsin core.The AdMob adapter keeps AdMob-only glue (
Setup,State,Dispatcher, one-shot main-actor delivery). Load-time setup now goes throughTokenProvider→ core token minting instead of localapiKey/appUserIDJSON encoding; present-time polling delegates toPurchases.shared.pollRewardVerificationinstead of an in-adapter poller andmapOutcome. Adapter-side VC cache invalidation and poll log strings are removed.Tests follow the split: poller/outcome coverage lives in core unit tests; adapter tests mock tokens and injectable poll closures.
@_spi(Experimental) import RevenueCatis required whereRewardVerificationResult/ token types are used.Reviewed by Cursor Bugbot for commit 3433a87. Bugbot is set up for automated code reviews on this repo. Configure here.