Fix concurrency, entitlement and API defects; raise to OS 26 and add tests - #5
Merged
Merged
Conversation
Three related defects in how entitlement state was maintained. A refund arrived as a purchase. Transaction.updates delivers revocations — refunds, a family-sharing grant being withdrawn, an entitlement expiring — and the listener responded to every update by calling markPurchased, which only ever sets isPurchased to true. It also yielded a PurchasedProductEvent for the refund. The listener now rebuilds from currentEntitlements, which moves the flag in both directions. hasEntitlement(for:) returned a cached false without consulting StoreKit. The doc said the cache was a fallback, but the code checked it first and returned whatever it held. A user who bought on another device, or who called this before the first refresh, was told they had no entitlement. The cache is now trusted only when it says yes. refreshEntitlements cleared every flag and set them again, so on each refresh every entitled product looked newly purchased and the event stream repeated itself for products the subscriber already knew about. It now collects the entitled identifiers and applies the difference, so an entitlement that has genuinely gone away is cleared and events fire only on a real transition. Also: currentEntitlements results are checked for revocationDate in hasEntitlement, entitlementProductIDs and refreshEntitlements; and the default branch of purchase(productID:) no longer wraps PurchasesError.unknown inside another PurchasesError.unknown. Doc comments on hasEntitlement(for:) and refreshEntitlements described the old behaviour and have been rewritten to match. Not covered by tests: the package has no test target, and exercising these paths needs an SKTestSession plus a StoreKit configuration file. Verified by reading and by `swift build`.
Three independent defects, none of which change the public API surface. PurchasesProtocol's default implementation repeated the requirement's own signature and only added a default argument value. For any conformer that did not implement the requirement itself, that overload became the witness and called itself forever — a mock written against the protocol, which is the protocol's stated purpose, hung instead of returning. Replaced with a parameterless overload, so the same omission is now a compile-time conformance error. Both call forms are preserved. activeSubscriptions() and activeSubscription(inGroup:) read Transaction.currentEntitlements without filtering on revocationDate, while hasEntitlement(for:), entitlementProductIDs() and refreshEntitlements() all do. A refunded or withdrawn subscription therefore still counted as active in exactly the two places that answer "is this user subscribed". requestProducts(includingCache:) returned productsCache.values, which is keyed by identifier: the order changed on every launch and shuffled the paywall. It also leaked products cached opportunistically from entitlements that were never part of the configured identifiers. Both now go through configuredProducts, which projects the cache through identifiers and preserves the caller's order. Identifiers are deduplicated at init so that order cannot repeat a product. Also translates two stray Russian comments in the touched method.
…e listener cycle purchasedProducts handed out one shared AsyncStream. AsyncStream has exactly one consumer, so two `for await` loops did not each receive the sequence — they split it. Two screens observing purchases each saw roughly half the events and silently missed the rest, which for an entitlement stream means a paid feature staying locked. Each access now returns its own stream and EventBroadcaster fans every event out to all live subscribers. That stream also used the default unbounded buffering policy. The stream is optional to consume, the manager is a process-lifetime singleton, and nothing drained the buffer unless the host app happened to subscribe, so events accumulated for the lifetime of the process. Per-subscriber buffers are now bufferingNewest(32): for entitlement changes the newest state is the one worth keeping when a slow subscriber overflows. The transaction listener captured self strongly. Transaction.updates never ends, so the actor was retained forever and deinit was unreachable — meaning the cancel and finish deinit performs were dead code that could never run. The loop now lives in the task closure with a weak capture and re-acquires self per update, so the actor deallocates and deinit runs. Behaviour change worth noting: events are no longer replayed to a late subscriber. The previous replay was a side effect of the unbounded buffer, and only ever reached one subscriber anyway. Current state is available from hasEntitlement(for:) and requestProducts(includingCache:), and the property documentation now says so.
Platforms move from iOS 15 / watchOS 8 / tvOS 15 / macOS 12 / visionOS 1 to 26 across the board. This drops support for every earlier release, so it is a breaking change for adopters and wants a major version tag. Raising the floor makes Synchronization.Mutex available, so EventBroadcaster no longer guards its state with NSLock behind an @unchecked Sendable assertion. It is now checked-Sendable: the compiler verifies the isolation instead of taking the assertion on trust. The comment justifying NSLock by the old iOS 15 floor went with it. Adds the package's first test target, covering the defects fixed on this branch so they cannot come back silently: - every subscriber receives every event, rather than the two observers splitting the sequence between them - a subscriber that never drains keeps bufferingNewest(32), not everything - a terminated subscriber is unregistered instead of accumulating - the parameterless PurchasesProtocol overload forwards instead of recursing, guarded by a timeout so a regression fails rather than hangs - ProductType maps each StoreKit type, and pins the silent nonConsumable fallback for types this SDK has never seen Every suite carries a one-minute time limit. CI now runs swift test, and without that backstop a single blocked await would hold the pipeline until the job timeout rather than failing. CI moves off the pinned macos-14 image and Xcode 16.2, which cannot build an OS 26 deployment target, onto macos-latest with the latest stable Xcode.
Two defects in the public surface. The singleton was stored in a nonisolated(unsafe) static var, which opted it out of the compiler's checking without putting anything in its place. configure(identifiers:) wrote it while other threads read it through shared, which ThreadSanitizer reports as a data race on a single configure call racing ordinary reads — no double configuration needed. Under load the racing reference-count traffic also produced an object deallocated with a non-zero retain count, so this was reachable memory corruption, not only a theoretical race. The precondition guarding double configuration was itself a check-then-act on that unsynchronised memory, so two concurrent calls could both pass it. The value now lives in a Mutex and configure tests and sets under one lock. PurchasesProtocol is documented as existing so it can be mocked, but no conformer outside the module could produce a single return value: StoreProduct's initializer was internal, and it required a StoreKit.Product, which has no public initializer and cannot be constructed at all. PurchasedProductEvent's memberwise initializer was internal too, so a stand-in event stream was equally out of reach. StoreProduct.product is now optional and there is a public initializer taking the fields directly, PurchasedProductEvent has a public one, and setPurchasingFlag copies the value instead of rebuilding it from a backing product that a synthesized value does not have. Making product optional is source-breaking for anyone reading it, which suits the major release the platform bump already requires. Note that purchase(productID:) still returns a StoreKit.Transaction, which cannot be constructed either, so a stand-in can only throw from it. Closing that means wrapping Transaction in an SDK-level value, which is a design decision rather than a defect fix. Adds tests for both, and drops a DocC line pointing readers at setPurchasingFlag(_:), which is internal.
The .build directory was committed: 2319 files of compiler output — module caches, object files, dependency graphs, SwiftPM's build database and its lock. None of it belongs in version control. It is regenerated by any build, it churns on every compile, and it made `git add -A` sweep binary artifacts into unrelated commits. Removed from the index only, so the working copy is untouched and no rebuild is needed. Added .build/ to .gitignore so it stays out.
Two kinds of local noise were under version control. .swiftpm carried per-machine Xcode state: UserInterfaceState.xcuserstate and xcschememanagement.plist, both under paths named after one developer's account. That is window and scheme state belonging to a single checkout, and it conflicts on every pull once more than one person opens the project. The shared scheme beside it, xcshareddata/xcschemes/RKPurchaseKit.xcscheme, stays tracked: the DocC job builds with -scheme RKPurchaseKit and would break without it. Six .DS_Store files were tracked even though .gitignore already listed them — they were committed before the rule existed, and an ignore rule does not apply to a file already in the index. Removed from the index only; the working copy is untouched. .gitignore gains xcuserdata/ and drops the Sources/.DS_Store line, which the bare .DS_Store pattern already covers at every depth.
Purchasing re-fetched the product from StoreKit on every call, even when the cache already held it, spending a network round trip to learn something known. It now reuses the cached StoreKit.Product when there is one, which the newly optional StoreProduct.product makes reachable. The success branch then cached the product with isPurchased false and set the flag afterwards, so a purchased product passed through a moment of looking unentitled and the transition emitted a second time. It caches straight to true, and reads the previous flag after the await on transaction.finish() rather than before: the transaction listener can rebuild entitlements during that suspension and emit for this product already, and re-reading afterwards keeps one purchase to one event. Both force unwraps of productsCache are gone; cache(_:purchased:) returns the value it stored, which is what each site actually wanted. PurchasesError.notConfigured was declared and documented but never thrown — the only way to reach the singleton trapped instead. resolved() returns it and throws that case, leaving shared as the convenient trapping form. An unknown StoreKit product type mapped to nonConsumable, which reads as a permanent one-off purchase: a guess, reported as fact. It now maps to ProductType.unknown. Adding the case is source-breaking for exhaustive switches, which suits the major release already in progress. The purchase switch's matching hole no longer throws an NSError with a placeholder domain either; it throws PurchasesError.unhandledPurchaseResult. PurchasesError gains Equatable, written by hand because the wrapped Error is not Equatable, and LocalizedError so localizedDescription carries usable text. The strings are not localised: the package ships no resource bundle, and adding one is a larger decision than the conformance.
README still pointed adopters at 1.0.5, two tags behind even before this work, and said nothing about the breaking changes. It now asks for 2.0.0 and lists what changes for a 1.x adopter: the platform floor, the optional StoreProduct.product with its new public initializer, the added ProductType and PurchasesError cases that widen exhaustive switches, purchasedProducts no longer replaying to a late subscriber, and the reshaped requestProducts() overload. The workflow gains a toolchain check. It selects latest-stable rather than a pinned Xcode, which is resilient but not guaranteed to be new enough; when it is not, the failure lands deep inside compilation with errors that never mention the SDK. The check fails first with a message naming the real cause and the two ways out. Verified locally against Xcode 26.6: swift build, swift test, xcodebuild docbuild and docc transform-for-static-hosting all succeed, with no DocC warnings, and resolved(), ProductType.unknown and unhandledPurchaseResult all appear in the generated documentation.
Ramiz69
changed the title
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
1 participant
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.
Started as the entitlement-revocation fix and grew to cover an audit of the whole library. Every defect below was reproduced before being fixed, and the repository now has a test target that pins the behaviour.
Correctness and concurrency
purchasedProductslost events.AsyncStreamhas exactly one consumer, so twofor awaitloops split the sequence rather than each receiving it — two screens observing purchases each saw about half the events. Measured before the fix: one observer got[1,3,5,7,9], the other[2,4,6,8,10]. Each subscriber now gets its own stream andEventBroadcasterfans events out to all of them.bufferingNewest(32).selfstrongly andTransaction.updatesnever ends, so the actor was retained forever anddeinitwas unreachable — the cancel and finish it performs were dead code. The loop now lives in the task closure with a weak capture.nonisolated(unsafe)opted the static out of checking without replacing it. ThreadSanitizer reports a race on a singleconfigurecall racing ordinarysharedreads; under load the racing reference-count traffic also producedobject deallocated with non-zero retain count. The guardingpreconditionwas itself a check-then-act on that memory. It now lives in aMutexandconfiguretests and sets under one lock.PurchasesProtocol. The default implementation repeated the requirement's signature and only added a default argument, so for a conformer that did not implement it the overload became its own witness and looped forever. A mock written against the protocol hung. It is now a parameterless overload, which turns the same omission into a compile-time conformance error.activeSubscriptions()andactiveSubscription(inGroup:)readcurrentEntitlementswithout filteringrevocationDate, while the three other entitlement paths did — so a refund still read as subscribed in the two places that answer "is this user subscribed".requestProductsreturnedproductsCache.values, whose order changes every run, and leaked products cached from entitlements that were never inidentifiers. Both now project the cache throughidentifiers.isPurchasedfalse and set it afterwards, so the product briefly read as unentitled and the transition could emit twice. It caches straight to true and reads the previous flag after theawait, where the listener may already have emitted.API
PurchasesProtocolis documented as existing to be mocked, but no conformer outside the module could produce a return value:StoreProduct's initializer was internal and required aStoreKit.Product, which has no public initializer.StoreProduct.productis now optional with a public initializer taking the fields directly, andPurchasedProductEventhas a public one.nonConsumable— a guess reported as fact. It now maps toProductType.unknown.PurchasesError.notConfiguredwas declared and documented but never thrown;resolved()returns the singleton and throws it, leavingsharedas the trapping form. The error gainsEquatableandLocalizedError.productsCacheare gone, and purchasing reuses the cached product instead of re-fetching it every time.Platform and tooling
Deployment targets move to OS 26 across the board, which also makes
Synchronization.Mutexavailable, soEventBroadcasteris checked-Sendablerather than@unchecked.The package gains its first test target: 24 tests in 5 suites, each suite under a one-minute time limit so a blocked
awaitfails rather than holding CI to the job timeout. CI now runsswift test, moves off the pinned macos-14 / Xcode 16.2 image that cannot build an OS 26 target, and fails fast with a clear message when the toolchain is too old.Build artifacts (2319 files under
.build), per-machine Xcode state and.DS_Storefiles are no longer tracked. The shared scheme the DocC job depends on is kept.Breaking changes
Platform floor, optional
StoreProduct.product, the newProductTypeandPurchasesErrorcases, andpurchasedProductsno longer replaying to a late subscriber. README documents the migration and asks for2.0.0.Verified locally on Xcode 26.6
swift build,swift test,xcodebuild docbuildanddocc transform-for-static-hostingall succeed, with no DocC warnings.