-
-
Notifications
You must be signed in to change notification settings - Fork 3
Domain desgin refactor #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Implement PurchaseState, FeatureRegistry, MarketingRegistry, and AccessControl as pure value types - Introduce Store and StoreKitProvider protocols with real and mockable implementations - Refactor InAppKit to delegate to domain models and abstracted store - Add thorough domain and infrastructure tests for all new components
- Implement PurchaseSetup builder for purchase initialization - Add ProductDefinition, DiscountRule, and related APIs - Introduce PaywallContext for paywall UI configuration - Extend View with .withPurchases() modifiers for setup - Provide type-erased configs and marketing fluent API - Ensure backwards compatibility with legacy aliases
- Add detailed DDD, layering, and component breakdown - Document domain/testing strategies and configuration patterns - Clarify feature mapping and dependency injection usage
- Rename ProductConfig to ProductDefinition throughout docs - Replace StoreKitConfiguration with PurchaseSetup - Update marketing "savings" to "promoText" in code examples - Add architecture and domain model documentation - Clarify backwards compatibility type aliases in notes
- Eliminate legacy typealiases such as ProductConfig, StoreKitConfiguration, and RelativeDiscountConfig - Rename relativeDiscountConfig/baseProductId to discountRule/comparedTo across core, UI, and tests - Update API docs and tests to match new naming conventions
- Remove InternalProductConfig and AnyProductDefinition - Make ProductDefinition non-generic with AnyHashable features - Update Product() and modifier APIs for simplified use - Refactor tests and registry to use new ProductDefinition structure
📝 WalkthroughWalkthroughThis pull request refactors InAppKit's architecture from a monolithic design to Domain-Driven Design with distinct layers: Domain Layer (pure models like PurchaseState, FeatureRegistry), Infrastructure Layer (Store protocol, AppStore), and Core/UI layers. It introduces Mockable dependency, renames key types (ProductConfig→ProductDefinition, RelativeDiscountConfig→DiscountRule, StoreKitConfiguration→PurchaseSetup), removes configuration files, and adds comprehensive domain model tests. Changes
Sequence DiagramssequenceDiagram
participant App as Application
participant InAppKit as InAppKit<br/>(Facade)
participant Store as Store<br/>(Infrastructure)
participant Registry as Registries<br/>(Domain)
participant Access as AccessControl<br/>(Domain)
App->>InAppKit: initialize(with products)
InAppKit->>Registry: Build FeatureRegistry<br/>& MarketingRegistry
InAppKit->>Store: loadProducts(ids)
Store-->>InAppKit: [Products]
rect rgb(100, 180, 100)
Note over InAppKit,Access: Domain-Driven Pattern
App->>InAppKit: hasAccess(to feature)
InAppKit->>Access: Query feature access
Access->>Registry: Get required products<br/>for feature
Access->>InAppKit: Check purchaseState
Access-->>InAppKit: access: Bool
InAppKit-->>App: Bool
end
sequenceDiagram
participant UI as SwiftUI View
participant Setup as PurchaseSetup
participant InAppKit as InAppKit
participant Domain as Domain Models<br/>(PurchaseState,<br/>FeatureRegistry)
participant Store as Store
UI->>Setup: .withPurchases(products)
Setup->>Setup: Configure builders
rect rgb(180, 150, 100)
Note over Setup,Store: Setup Lifecycle
Setup->>InAppKit: initialize(with products)
InAppKit->>Domain: Wire registries
InAppKit->>Store: loadProducts(ids)
Store-->>InAppKit: [Products]
InAppKit->>Domain: Update state
end
UI->>InAppKit: `@State` purchasedProductIDs
InAppKit-->>Domain: Delegate to<br/>purchaseState
Domain-->>UI: [productIds]
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes The refactoring is substantial and heterogeneous: multi-layer architectural changes, extensive API surface redesign (ProductConfig→ProductDefinition, StoreKitConfiguration→PurchaseSetup), new domain model patterns (immutable registries, access control logic), infrastructure abstraction layer, removed code paths, and interconnected changes across domain, infrastructure, core, and UI layers. Reviewers must understand the new DDD patterns, validate immutability guarantees, verify registry logic, and confirm test coverage aligns with the new architecture. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Fix all issues with AI Agents 🤖
In @docs/api-reference.md:
- Around line 467-470: The doc signature for MarketingRegistry still references
the removed type InternalProductConfig; update the declaration of
withMarketing(from config: InternalProductConfig) to use ProductDefinition
instead (i.e., withMarketing(from config: ProductDefinition)) and ensure any
related references or imports in the same doc block match the new
ProductDefinition type; keep the function name MarketingRegistry and
withMarketing unchanged.
- Around line 54-61: The docs show generic Product overloads and
ProductDefinition<T>, but the implementation uses a non-generic public struct
ProductDefinition with features: [AnyHashable]; update the API reference
signatures (lines referenced) to remove generics: change the three Product
declarations to non-generic forms returning ProductDefinition and accept
features as [AnyHashable] (e.g. public func Product(_ id: String, features:
[AnyHashable]) -> ProductDefinition) and for the CaseIterable variant accept an
array of AnyHashable (e.g. public func Product(_ id: String, allCases:
[AnyHashable]) -> ProductDefinition); also replace any occurrences of
ProductDefinition<T> with ProductDefinition in the later section (lines 329-349)
to match the non-generic implementation.
In @Sources/InAppKit/Core/Domain/ProductDefinition.swift:
- Line 40: ProductDefinition is incorrectly marked @unchecked Sendable while it
stores non-Sendable types; remove the @unchecked Sendable conformance and
replace non-Sendable fields: change badgeColor (and any Color usage inside
discountRule) from Color to a Sendable representation (e.g., a struct like RGBA
or hex String), update DiscountRule to use the same Sendable color
representation (or mark DiscountRule @unchecked Sendable with justification),
and replace the [AnyHashable] property with a concrete Sendable collection
(e.g., [String] or [AnyHashable] only if you validate/limit wrapped types to be
Sendable) so all stored properties of ProductDefinition are fully Sendable
without bypassing compiler checks.
In @Sources/InAppKit/Core/Domain/Store.swift:
- Around line 14-29: The Store protocol currently depends on StoreKit types:
remove import StoreKit from the domain and replace usages of StoreKit.Product in
the Store protocol with a domain-level product abstraction (e.g., DomainProduct
or ProductDefinition) so the protocol (Store) is infrastructure-agnostic; then
update adapters like DefaultStoreKitProvider.fetchProducts and AppStore.purchase
to convert between StoreKit.Product and the new domain product type (map
StoreKit.Product -> DomainProduct when returning products and map DomainProduct
-> StoreKit.Product or an identifier when calling product.purchase()), and
ensure Mockable and tests use the domain product type rather than
StoreKit.Product.
In @Sources/InAppKit/Core/InAppKit.swift:
- Around line 239-246: In refreshPurchases() the catch only logs via
Logger.statistics.error but doesn't set purchaseError like loadProducts and
restorePurchases do; update the catch block to assign the caught error to
purchaseError (e.g., purchaseError = error) in addition to logging so the
component state mirrors other failure paths and diagnostic flows for
store.purchases() failures are consistent with loadProducts/restorePurchases.
🧹 Nitpick comments (15)
CLAUDE.md (1)
128-139: Consider adding language specifier to fenced code block.The fenced code block at line 128 is missing a language identifier, which improves syntax highlighting and tooling support.
🔎 Proposed fix
-``` +```plaintext Tests/InAppKitTests/ ├── Domain/ ← Pure domain model testsBased on static analysis hints.
Tests/InAppKitTests/Infrastructure/AppStoreTests.swift (1)
131-138: Redundant nil check on non-optional type.
DefaultStoreKitProvider()returns a non-optional struct instance, so#expect(provider != nil)will always pass. If the intent is to verify instantiation doesn't crash, the current test body already achieves that by simply calling the initializer.🔎 Proposed simplification
@Test func `DefaultStoreKitProvider can be instantiated`() { // Given/When let provider = DefaultStoreKitProvider() - // Then - no crash, provider exists - #expect(provider != nil) + // Then - no crash, provider exists + // Instantiation success is sufficient validation + _ = provider }Sources/InAppKit/Core/Domain/PurchaseState.swift (1)
34-38: Consider simplifyingwithPurchasefor consistency.The
withPurchasemethod could use the same pattern aswithPurchasesfor consistency:🔎 Optional simplification
public func withPurchase(_ productId: String) -> PurchaseState { - var newIDs = purchasedProductIDs - newIDs.insert(productId) - return PurchaseState(purchasedProductIDs: newIDs) + PurchaseState(purchasedProductIDs: purchasedProductIDs.union([productId])) }Sources/InAppKit/Infrastructure/AppStore.swift (1)
86-93: Extract duplicated helper to a shared location.The
checkVerifiedhelper is duplicated inSources/InAppKit/Core/InAppKit.swift(lines 261-268). Consider extracting this to a shared utility or extension to maintain DRY principles.🔎 Suggested approach
Create a shared extension or utility file:
// Sources/InAppKit/Infrastructure/VerificationHelper.swift import StoreKit extension VerificationResult { func checkVerified() throws -> T { switch self { case .unverified: throw StoreError.failedVerification case .verified(let safe): return safe } } }Then use it as:
-let transaction = try checkVerified(verification) +let transaction = try verification.checkVerified()Sources/InAppKit/Core/Domain/PaywallContext.swift (1)
36-71: Consider dependency injection over singleton access.All marketing helper methods directly access
InAppKit.shared, creating tight coupling to global state. This makesPaywallContextharder to test in isolation and couples a domain model to a specific infrastructure singleton.Consider passing a
MarketingRegistry(or a protocol for marketing data access) as a constructor parameter or to individual methods, enabling better testability and cleaner domain boundaries.🔎 Suggested approach
public struct PaywallContext { // ... existing properties ... // Option 1: Add marketing registry to context private let marketingRegistry: MarketingRegistry? public init( triggeredBy: String? = nil, availableProducts: [Product] = [], recommendedProduct: Product? = nil, marketingRegistry: MarketingRegistry? = nil ) { self.triggeredBy = triggeredBy self.availableProducts = availableProducts self.recommendedProduct = recommendedProduct ?? availableProducts.first self.marketingRegistry = marketingRegistry } public func badge(for product: Product) -> String? { marketingRegistry?.badge(for: product.id) } // ... update other methods similarly ... }Then callers can inject the registry or fall back to the shared instance.
Tests/InAppKitTests/Infrastructure/StoreTests.swift (2)
77-94: Flaky test due to timing-based synchronization.Using
Task.sleep(for: .milliseconds(100))to wait for purchase state refresh is fragile. The test may fail intermittently on slower machines or CI environments, or may waste time on fast ones.Consider exposing a mechanism to await initialization completion, or verifying the mock was called (which confirms the async path executed) rather than relying on sleep timing.
🔎 Alternative approach using verification
@Test @MainActor func `hasAccess returns true after store returns purchases`() async { // Given let mockStore = MockStore() given(mockStore) .purchases() .willReturn(Set(["com.app.pro"])) let inAppKit = InAppKit.configure(with: mockStore) inAppKit.registerFeature("premium", productIds: ["com.app.pro"]) - // Wait for initial purchase refresh - try? await Task.sleep(for: .milliseconds(100)) + // Verify purchases() was called, indicating async refresh completed + await verify(mockStore) + .purchases() + .called(.atLeastOnce) // Then #expect(inAppKit.hasAccess(to: "premium")) }
96-113: Same flaky timing issue as above.This test has the identical
Task.sleepproblem for synchronization.Sources/InAppKit/Core/Domain/MarketingRegistry.swift (1)
96-99: Non-deterministic ordering inproductsWithBadges.The
filteron a dictionary produces results in arbitrary order. If consumers rely on consistent ordering (e.g., for UI lists or tests), consider returning aSet<String>or sorting the result.🔎 Optional: Return sorted or as Set
/// Get all products with badges - public var productsWithBadges: [String] { - marketingInfo.filter { $0.value.hasBadge }.map { $0.key } + public var productsWithBadges: Set<String> { + Set(marketingInfo.filter { $0.value.hasBadge }.map { $0.key }) }Tests/InAppKitTests/InAppKitTests.swift (1)
344-356: Potential test pollution from shared state.Tests using
InAppKit.sharedmay interfere with each other if run in parallel or in certain orders. The feature registrations accumulate across tests.Consider calling
clearFeatures()in test setup/teardown, or use a freshInAppKit.configure(with:)instance per test for isolation.🔎 Add cleanup to prevent test pollution
struct InAppKitFeatureTests { @Test @MainActor func `register feature maps feature to products`() { let manager = InAppKit.shared + defer { manager.clearFeatures() } manager.registerFeature(TestFeature.sync, productIds: ["com.test.pro"]) #expect(manager.isFeatureRegistered(TestFeature.sync)) }Sources/InAppKit/Core/Domain/FeatureRegistry.swift (1)
51-65: Additive behavior inwithFeatureis intentional but may surprise users.Calling
withFeaturefor the same feature multiple times will accumulate product IDs viaunion. This is useful for incremental registration but differs from a "set/replace" semantic. Consider documenting this behavior in the method's doc comment.🔎 Suggested doc enhancement
- /// Register a feature with product IDs + /// Register a feature with product IDs. + /// If the feature already exists, the new product IDs are merged (not replaced). public func withFeature(_ feature: AnyHashable, productIds: [String]) -> FeatureRegistry {Sources/InAppKit/Modifiers/PurchaseSetup.swift (1)
239-243: Inconsistent implementation bypasses fluent method.The variadic
withPurchasesextension directly mutatesconfig.productsinstead of callingconfig.withPurchases(products:). This works but is inconsistent with other patterns and harder to maintain.🔎 Use existing method for consistency
func withPurchases(_ productIds: String...) -> PurchaseEnabledView<Self> { - let config = PurchaseSetup() - config.products.append(contentsOf: productIds.map { ProductDefinition($0) }) - return PurchaseEnabledView(content: self, config: config) + PurchaseEnabledView(content: self, config: PurchaseSetup().withPurchases(products: productIds.map { ProductDefinition($0) })) }Sources/InAppKit/Core/InAppKit.swift (2)
74-79:configure(with:)returns new instance rather than modifying shared.The method creates a new
InAppKitinstance, butInAppKit.sharedremains unchanged. Callers must use the returned instance. This is correct for testing but the comment says "Reset shared instance" which is misleading.🔎 Fix misleading comment
#if DEBUG - /// Reset shared instance with a mock store (for testing) + /// Create a new InAppKit instance with a custom store (for testing) public static func configure(with store: any Store) -> InAppKit { return InAppKit(store: store) } #endif
262-269: Extract the duplicatecheckVerifiedmethod to a shared utility.The method is duplicated identically in both
AppStore.swift(line 86) and this file (line 262). Consolidate to a shared extension onVerificationResultor a utility function to reduce code duplication.docs/api-reference.md (1)
21-39: Add language specifier to code block.The directory structure code block should have a language identifier for consistent rendering and to satisfy linting.
🔎 Add language specifier
-``` +```text Sources/InAppKit/ ├── Core/Sources/InAppKit/Core/Domain/Store.swift (1)
14-29: Consider domain-specific error types for better error encapsulation.All
Storeprotocol methods usethrowswithout defining domain-specific error types. This means infrastructure errors (from StoreKit) will propagate directly to domain consumers, coupling error handling to infrastructure details.Consider introducing a domain error type like:
public enum StoreError: Error, Sendable { case productNotFound case purchaseFailed(reason: String) case networkError case verificationFailed // Map infrastructure errors to domain errors in AppStore }This would encapsulate infrastructure concerns and give consumers a stable error contract.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
CLAUDE.mdPackage.swiftSources/InAppKit/Configuration/ProductConfiguration.swiftSources/InAppKit/Configuration/StoreKitConfiguration.swiftSources/InAppKit/Core/Domain/AccessControl.swiftSources/InAppKit/Core/Domain/FeatureRegistry.swiftSources/InAppKit/Core/Domain/MarketingRegistry.swiftSources/InAppKit/Core/Domain/PaywallContext.swiftSources/InAppKit/Core/Domain/ProductDefinition.swiftSources/InAppKit/Core/Domain/PurchaseState.swiftSources/InAppKit/Core/Domain/Store.swiftSources/InAppKit/Core/InAppKit.swiftSources/InAppKit/Infrastructure/AppStore.swiftSources/InAppKit/Infrastructure/StoreKitProvider.swiftSources/InAppKit/Modifiers/PurchaseSetup.swiftSources/InAppKit/UI/Components/PaywallComponents.swiftSources/InAppKit/UI/Components/PurchaseOptionCard.swiftTests/InAppKitTests/Domain/AccessControlTests.swiftTests/InAppKitTests/Domain/FeatureRegistryTests.swiftTests/InAppKitTests/Domain/MarketingRegistryTests.swiftTests/InAppKitTests/Domain/PurchaseStateTests.swiftTests/InAppKitTests/InAppKitTests.swiftTests/InAppKitTests/Infrastructure/AppStoreTests.swiftTests/InAppKitTests/Infrastructure/StoreTests.swiftdocs/api-reference.mddocs/customization.md
💤 Files with no reviewable changes (2)
- Sources/InAppKit/Configuration/StoreKitConfiguration.swift
- Sources/InAppKit/Configuration/ProductConfiguration.swift
🧰 Additional context used
🧬 Code graph analysis (15)
Tests/InAppKitTests/Domain/PurchaseStateTests.swift (1)
Sources/InAppKit/Core/Domain/PurchaseState.swift (5)
isPurchased(27-29)withPurchase(34-38)withPurchases(41-43)withoutPurchase(46-50)cleared(53-55)
Tests/InAppKitTests/Domain/FeatureRegistryTests.swift (1)
Sources/InAppKit/Core/Domain/FeatureRegistry.swift (8)
withFeature(51-65)withFeature(81-83)productIds(29-31)productIds(91-93)isRegistered(24-26)isRegistered(86-88)features(34-36)withFeatures(68-74)
Sources/InAppKit/UI/Components/PaywallComponents.swift (2)
Sources/InAppKit/Modifiers/PurchaseSetup.swift (2)
withPaywallFeatures(59-62)withPaywallFeatures(202-204)Sources/InAppKit/Core/Domain/MarketingRegistry.swift (1)
features(77-79)
Sources/InAppKit/Infrastructure/StoreKitProvider.swift (2)
Sources/InAppKit/Core/InAppKit.swift (1)
products(172-179)Sources/InAppKit/Infrastructure/AppStore.swift (1)
products(31-35)
Sources/InAppKit/Core/Domain/Store.swift (2)
Sources/InAppKit/Core/InAppKit.swift (2)
products(172-179)purchase(124-140)Sources/InAppKit/Infrastructure/AppStore.swift (4)
products(31-35)purchase(37-56)purchases(58-77)restore(79-82)
Tests/InAppKitTests/Infrastructure/StoreTests.swift (5)
Sources/InAppKit/Infrastructure/AppStore.swift (3)
purchases(58-77)products(31-35)restore(79-82)Sources/InAppKit/Core/InAppKit.swift (8)
configure(76-78)products(172-179)loadProducts(114-122)restorePurchases(142-150)isPurchased(154-156)registerFeature(183-185)hasAccess(160-162)hasAccess(164-170)Sources/InAppKit/Core/Domain/FeatureRegistry.swift (2)
productIds(29-31)productIds(91-93)Sources/InAppKit/Core/Domain/PurchaseState.swift (1)
isPurchased(27-29)Sources/InAppKit/Core/Domain/AccessControl.swift (2)
hasAccess(20-36)hasAccess(39-49)
Tests/InAppKitTests/Domain/AccessControlTests.swift (3)
Sources/InAppKit/Core/Domain/FeatureRegistry.swift (4)
withFeature(51-65)withFeature(81-83)productIds(29-31)productIds(91-93)Sources/InAppKit/Core/InAppKit.swift (2)
hasAccess(160-162)hasAccess(164-170)Sources/InAppKit/Core/Domain/AccessControl.swift (2)
hasAccess(20-36)hasAccess(39-49)
Sources/InAppKit/Core/Domain/MarketingRegistry.swift (3)
Sources/InAppKit/Core/InAppKit.swift (5)
badge(197-199)badgeColor(201-203)promoText(209-211)discountRule(213-215)marketingFeatures(205-207)Sources/InAppKit/Core/Domain/FeatureRegistry.swift (1)
features(34-36)Sources/InAppKit/Core/Domain/PaywallContext.swift (1)
marketingInfo(55-62)
Sources/InAppKit/Core/Domain/FeatureRegistry.swift (1)
Sources/InAppKit/Core/Domain/MarketingRegistry.swift (1)
features(77-79)
Sources/InAppKit/Infrastructure/AppStore.swift (2)
Sources/InAppKit/Core/InAppKit.swift (3)
products(172-179)purchase(124-140)checkVerified(262-269)Sources/InAppKit/Infrastructure/StoreKitProvider.swift (3)
fetchProducts(32-34)currentEntitlements(36-42)sync(44-46)
Tests/InAppKitTests/Infrastructure/AppStoreTests.swift (3)
Sources/InAppKit/Infrastructure/StoreKitProvider.swift (3)
fetchProducts(32-34)currentEntitlements(36-42)sync(44-46)Sources/InAppKit/Core/InAppKit.swift (1)
products(172-179)Sources/InAppKit/Infrastructure/AppStore.swift (3)
products(31-35)purchases(58-77)restore(79-82)
Sources/InAppKit/Core/Domain/ProductDefinition.swift (3)
Sources/InAppKit/Core/Domain/MarketingRegistry.swift (5)
features(77-79)badge(67-69)badgeColor(72-74)promoText(82-84)discountRule(87-89)Sources/InAppKit/Core/InAppKit.swift (5)
badge(197-199)badgeColor(201-203)marketingFeatures(205-207)promoText(209-211)discountRule(213-215)Sources/InAppKit/Core/Domain/PaywallContext.swift (3)
badge(37-40)marketingFeatures(43-46)promoText(49-52)
Sources/InAppKit/Modifiers/PurchaseSetup.swift (2)
Sources/InAppKit/Core/InAppKit.swift (2)
products(172-179)initialize(87-110)Sources/InAppKit/UI/Components/PaywallComponents.swift (4)
withPaywallHeader(328-346)withPaywallHeader(349-367)withPaywallHeader(370-388)withPaywallFeatures(391-400)
Sources/InAppKit/UI/Components/PurchaseOptionCard.swift (2)
Sources/InAppKit/Core/Domain/MarketingRegistry.swift (2)
promoText(82-84)discountRule(87-89)Sources/InAppKit/Core/InAppKit.swift (2)
promoText(209-211)discountRule(213-215)
Sources/InAppKit/Core/Domain/PaywallContext.swift (1)
Sources/InAppKit/Core/InAppKit.swift (3)
badge(197-199)marketingFeatures(205-207)promoText(209-211)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
128-128: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/api-reference.md
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 SwiftLint (0.57.0)
Tests/InAppKitTests/Domain/FeatureRegistryTests.swift
[Warning] 139-139: Type name '' should be between 3 and 40 characters long
(type_name)
Tests/InAppKitTests/Domain/AccessControlTests.swift
[Warning] 129-129: Type name '' should be between 3 and 40 characters long
(type_name)
Tests/InAppKitTests/InAppKitTests.swift
[Error] 25-25: Type name 'feature' should start with an uppercase character
(type_name)
[Error] 34-34: Type name 'features' should start with an uppercase character
(type_name)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test
- GitHub Check: Unit Tests
🔇 Additional comments (42)
Sources/InAppKit/UI/Components/PurchaseOptionCard.swift (1)
77-172: LGTM! Consistent refactoring fromrelativeDiscountConfigtodiscountRule.The renaming is systematic and complete:
relativeDiscountConfig→discountRuleconfig.baseProductId→rule.comparedToconfig.style→rule.styleThe discount calculation logic is preserved correctly, and all references are updated consistently throughout the file.
CLAUDE.md (2)
19-98: Excellent documentation of the DDD architecture.The documentation clearly explains the layered architecture with:
- Domain Layer: Pure business logic with no StoreKit dependencies
- Infrastructure Layer: StoreKit integration
- Core Layer: Coordination
- UI Layer: SwiftUI components
The examples and test organization guidance are comprehensive and helpful.
11-11: Swift 6.2+ requirement is appropriate.Swift 6.2 is publicly available (Swift 6.3 was released January 3, 2026), so the minimum version requirement in the documentation is valid and achievable.
Sources/InAppKit/Core/Domain/ProductDefinition.swift (3)
18-34: Well-designedDiscountRuledomain model.The struct is properly marked as
Sendableand provides a clean API for defining discount calculations. The initializer parameter naming (comparedTo baseProductId) maintains backward compatibility while using the cleaner property namecomparedTo.
71-83: Excellent type-safe factory functions.The three
Product()overloads provide:
- No features variant
- Type-safe features with explicit array
- Automatic
allCasesmapping forCaseIterableenumsThis API design eliminates type erasure at the call site while maintaining flexibility.
87-132: Well-implemented fluent API with immutable updates.Each method returns a new
ProductDefinitioninstance, maintaining immutability. The debug logging inwithRelativeDiscount(lines 124-126) is appropriately guarded with#if DEBUG.Tests/InAppKitTests/Domain/PurchaseStateTests.swift (1)
1-141: Comprehensive test coverage forPurchaseStatedomain model.The test suite validates all aspects of the immutable
PurchaseState:
- Creation scenarios (empty and initialized)
- Purchase checks (
isPurchased,hasAnyPurchase)- Immutable transformations (
withPurchase,withPurchases)- Removal operations (
withoutPurchase,cleared)- Equality semantics
The Given-When-Then structure is consistently applied, making tests easy to understand. Line 72 correctly validates that the original state remains unchanged after
withPurchase, confirming immutability.Tests/InAppKitTests/Domain/AccessControlTests.swift (1)
1-233: Excellent comprehensive test coverage forAccessControldomain logic.The test suite thoroughly validates:
- Basic access control with correct/wrong purchases
- Multiple products per feature (any-of semantics)
- Fallback behavior for unregistered features
- Type-safe
AppFeatureenum support- Batch operations (
accessStatus,accessibleFeatures,missingFeatures)Lines 93-124 correctly test the fallback behavior: unregistered features fall back to
hasAnyPurchase, which is a sensible default. The tests are well-organized and clearly document expected behavior.Note: The SwiftLint warning at line 129 is a false positive—it's misinterpreting the context.
Tests/InAppKitTests/Domain/FeatureRegistryTests.swift (1)
1-199: LGTM! Comprehensive test coverage for FeatureRegistry.The test suite thoroughly covers:
- Empty registry behavior
- Feature registration (single/multiple products, multiple features)
- Querying (unknown features/products return empty)
- AppFeature convenience overloads
- Immutability guarantees
- Bulk registration via
withFeaturesThe static analysis warning about type name on line 139 is a false positive caused by backtick-quoted test method names, which is valid Swift Testing syntax.
Tests/InAppKitTests/Infrastructure/AppStoreTests.swift (1)
11-103: LGTM! Well-structured infrastructure tests.The AppStore tests effectively:
- Verify provider delegation for product fetching
- Test error propagation from provider
- Validate the restore flow (sync → currentEntitlements sequence)
- Use proper async/await patterns with Mockable's verification API
Tests/InAppKitTests/Domain/MarketingRegistryTests.swift (1)
1-259: LGTM! Thorough test coverage for MarketingRegistry and ProductMarketing.Excellent test coverage including:
- Registry creation and querying
- Marketing registration with badges, features, promo text, and discount rules
- ProductMarketing domain predicates (
hasMarketing,hasBadge,hasDiscountRule)- Removal operations (
withoutMarketing,cleared)- Immutability guarantees
- Bulk registration from
ProductDefinitionconfigurationsThe tests follow consistent Given/When/Then structure and cover both positive and negative cases.
Sources/InAppKit/Core/Domain/PurchaseState.swift (1)
11-56: LGTM! Clean pure domain model design.
PurchaseStatefollows excellent domain-driven design principles:
- Pure value type with no infrastructure dependencies
- Immutable API with copy-on-write semantics
Equatablefor testing and comparisonSendablefor safe concurrent access- Clear separation of queries and commands
docs/customization.md (1)
312-332: LGTM! Documentation properly updated for new API.The documentation correctly reflects the API migration:
StoreKitConfiguration→PurchaseSetupProductConfig→ProductDefinitionsavings→promoTextThese changes align with the domain-driven design refactor described in the PR objectives.
Package.swift (2)
24-31: The suggested refactoring to move Mockable to test-only is not feasible.The
@Mockablemacros are applied to protocols in the main library source code (StoreKitProvider.swiftandStore.swiftinSources/InAppKit/), not just in tests. Since@Mockableis a compile-time macro, the Mockable dependency is required by the mainInAppKittarget for successful compilation. Moving it to the test target only would break the build.If binary size is a concern, verify whether Mockable adds significant size to release builds despite the
MOCKINGflag being debug-only, and evaluate whether this library design (exposing mock generation to consumers) aligns with project goals.Likely an incorrect or invalid review comment.
20-20: Mockable version 0.5.0 is the latest release and is compatible with Swift 6.2. No action required.Sources/InAppKit/UI/Components/PaywallComponents.swift (1)
334-400: The review comment's concern about configuration loss is incorrect.When chaining
ContentView().withPaywallHeader().withPaywallFeatures(), the configuration is properly preserved. The second call onPurchaseEnabledView<ContentView>resolves to the more-specificPurchaseEnabledView<Content>.withPaywallFeaturesmethod (not the generic View extension), which preserves the existing config by callingconfig.withPaywallFeatures(builder)on the config from the first call rather than creating a new one.Likely an incorrect or invalid review comment.
Sources/InAppKit/Infrastructure/AppStore.swift (2)
37-56: LGTM!The purchase flow correctly handles verification, transaction finalization, and all outcome cases. The
@unknown defaultcase provides good forward compatibility for future StoreKit purchase result types.
15-15: @unchecked Sendable conformance is safe and correct.
AppStoreholds only an immutable reference toStoreKitProvider, which explicitly requiresSendableconformance.DefaultStoreKitProviderhas no mutable state—it's a stateless wrapper around StoreKit's async APIs (Product.products,Transaction.currentEntitlements,AppStore.sync). Thread-safety is guaranteed by the immutable property and async method access patterns.Sources/InAppKit/Core/Domain/AccessControl.swift (2)
38-94: LGTM!The overload and utility methods correctly delegate to the core
hasAccesslogic. The filtering and mapping implementations are clean and straightforward.
27-30: The fallback behavior is intentional and documented.The code grants access based on
hasAnyPurchasefor unregistered features by design. This is explicitly tested inAccessControlTests.swiftwith dedicated tests:
unregistered feature falls back to hasAnyPurchase when user has no purchasesunregistered feature falls back to hasAnyPurchase when user has purchasesThe
PurchaseModifiers.swiftfile also includes this pattern with a comment: "In release, fall back to any purchase check to avoid breaking user experience." This defensive fallback prevents breaking the app if a feature isn't properly registered. Ensure all features are registered at initialization to avoid triggering this fallback.Likely an incorrect or invalid review comment.
Sources/InAppKit/Core/Domain/PaywallContext.swift (1)
14-32: LGTM!Clean domain model with sensible defaults. The
recommendedProductfallback to the first available product is a good user experience consideration.Sources/InAppKit/Infrastructure/StoreKitProvider.swift (2)
13-25: LGTM!Excellent infrastructure abstraction. The
StoreKitProviderprotocol cleanly wraps StoreKit static APIs, making the infrastructure layer fully testable via@Mockable.
27-47: LGTM!The
DefaultStoreKitProviderimplementation correctly delegates to StoreKit APIs. The@unchecked Sendableconformance is safe since the class has no stored state and only calls thread-safe StoreKit static methods. ThecurrentEntitlements()method properly accumulates results from the async sequence.Tests/InAppKitTests/Infrastructure/StoreTests.swift (2)
121-143: Good coverage of PurchaseOutcome equatability.The tests thoroughly verify equality semantics for all
PurchaseOutcomecases, ensuringsuccesscompares byproductIdwhilecancelledandpendingcompare by case identity.
11-50: Well-structured infrastructure tests with mocks.The tests properly configure the
MockStoreusing Mockable'sgiven/verifyAPI and follow the Given/When/Then pattern. The verification ofproducts(for:)with the exact expected value is a good practice.Sources/InAppKit/Core/Domain/MarketingRegistry.swift (3)
35-48: Consider consistency inhasMarketingpredicate.
hasMarketingchecksbadge,features, andpromoText, but excludesbadgeColoranddiscountRule. This seems intentional (badgeColor requires a badge, discountRule is separate concern), but verify this is the desired behavior—a product with only adiscountRuleconfigured would returnhasMarketing == false.
11-49: Clean value-type design for ProductMarketing.The struct is
Sendable, uses optional fields with sensible defaults, and provides clear domain predicates. Good separation of concerns.
101-141: Immutable command pattern well implemented.The
withMarketing,withoutMarketing, andclearedmethods correctly return new instances without mutating the original, following functional domain modeling best practices.Tests/InAppKitTests/InAppKitTests.swift (3)
15-63: Good coverage of ProductDefinition creation patterns.Tests thoroughly verify product creation with no features, single features, multiple features,
allCases, and string features. The type assertions confirm the unifiedProductDefinitionreturn type.
116-161: Comprehensive DiscountRule tests.The tests cover all discount styles (percentage, amount, freeTime), custom colors, and verify that discount configuration preserves other properties. Good coverage of the fluent API.
563-603: StoreError localization tests are valuable.These tests ensure error messages contain expected keywords, which helps catch regressions in user-facing error text.
Sources/InAppKit/Core/Domain/FeatureRegistry.swift (2)
79-93: AppFeature convenience methods are well-designed.The extension correctly wraps
feature.rawValueinAnyHashablefor consistency with the base API. This provides a clean, type-safe interface for enum-based features.
11-46: Clean bidirectional registry implementation.The dual maps (
featureToProductsandproductToFeatures) enable efficient lookups in both directions. TheEquatableconformance allows easy comparison in tests.Sources/InAppKit/Modifiers/PurchaseSetup.swift (3)
95-121: Appropriate use ofnonisolated(unsafe)for environment keys.The
nonisolated(unsafe)attribute is necessary for closure-typedEnvironmentKeydefaults in Swift 6 strict concurrency. The URL keys correctly omit it sinceURL?isSendable.
209-225: PurchaseSetupModifier correctly wires environment and triggers initialization.The modifier applies all environment values and uses
.taskto callconfig.setup()asynchronously when the view appears. This is the appropriate pattern for SwiftUI lifecycle integration.
164-205: PurchaseEnabledView provides clean chaining API.The wrapper view correctly forwards configuration calls to the underlying
PurchaseSetup, enabling the fluent.withPurchases(...).withPaywall(...).withTerms(...)pattern.Sources/InAppKit/Core/InAppKit.swift (2)
17-52: Clean domain model delegation.The public accessors correctly delegate to the respective domain models (
purchaseState,featureRegistry,marketingRegistry). The deprecatedisPremiumproperty provides a migration path.
87-110: Well-structured initialization with domain models.The
initializemethod properly wires products to bothfeatureRegistryandmarketingRegistryusing immutable updates. The DEBUG logging for discount rules aids development.docs/api-reference.md (2)
493-551: Excellent testing documentation.The testing section clearly distinguishes between domain tests (pure, no mocks) and infrastructure tests (with Mockable), with practical code examples. The debug helpers section is useful for development workflows.
17-45: Clear architecture overview.The DDD principles and directory structure provide good orientation for developers. The key principles section succinctly explains the separation of concerns.
Sources/InAppKit/Core/Domain/Store.swift (2)
31-36: Clean domain model for purchase outcomes.The
PurchaseOutcomeenum is well-designed:
- Properly conforms to
EquatableandSendablefor testing and concurrency- Represents purchase outcomes in pure domain terms without infrastructure coupling
- Covers all necessary cases (success, cancelled, pending)
The
productIdin thesuccesscase provides useful context for logging and tracking, even though callers typically know which product they're purchasing.
11-12: Mockable 0.5.0 supports async throws methods in protocols, so the import and@Mockableannotation are compatible with theStoreprotocol's requirements.
| public func Product(_ id: String) -> ProductDefinition<String> | ||
|
|
||
| // With features array | ||
| public func Product<T: Hashable & Sendable>(_ id: String, features: [T]) -> ProductConfig<T> | ||
| public func Product<T: Hashable>(_ id: String, features: [T]) -> ProductDefinition<T> | ||
|
|
||
| // With allCases (for CaseIterable enums) | ||
| public func Product<T: CaseIterable & Hashable & Sendable>(_ id: String, features: T.AllCases) -> ProductConfig<T> | ||
| public func Product<T: CaseIterable & Hashable>(_ id: String, features: T.AllCases) -> ProductDefinition<T> | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ProductDefinition signature
ast-grep --pattern 'public struct ProductDefinition$_'
rg -n "struct ProductDefinition" --type swift -A 5Repository: tddworks/InAppKit
Length of output: 2098
🏁 Script executed:
#!/bin/bash
# Check the actual documentation to verify the discrepancy
head -100 docs/api-reference.md | tail -50
# Lines 54-61
sed -n '50,65p' docs/api-reference.mdRepository: tddworks/InAppKit
Length of output: 1965
🏁 Script executed:
#!/bin/bash
# Check lines 329-349 where the review also applies
sed -n '325,355p' docs/api-reference.mdRepository: tddworks/InAppKit
Length of output: 754
Update API reference documentation to match non-generic implementation.
The documentation in docs/api-reference.md shows ProductDefinition<T> and ProductDefinition<Feature: Hashable> with generic type parameters, but the actual implementation at Sources/InAppKit/Core/Domain/ProductDefinition.swift line 40 is non-generic: public struct ProductDefinition with features: [AnyHashable]. Update lines 54-61 and 329-349 to reflect the actual non-generic struct definition.
🤖 Prompt for AI Agents
In @docs/api-reference.md around lines 54-61, The docs show generic Product
overloads and ProductDefinition<T>, but the implementation uses a non-generic
public struct ProductDefinition with features: [AnyHashable]; update the API
reference signatures (lines referenced) to remove generics: change the three
Product declarations to non-generic forms returning ProductDefinition and accept
features as [AnyHashable] (e.g. public func Product(_ id: String, features:
[AnyHashable]) -> ProductDefinition) and for the CaseIterable variant accept an
array of AnyHashable (e.g. public func Product(_ id: String, allCases:
[AnyHashable]) -> ProductDefinition); also replace any occurrences of
ProductDefinition<T> with ProductDefinition in the later section (lines 329-349)
to match the non-generic implementation.
| // Immutable updates | ||
| public func withMarketing(_ productId: String, marketing: ProductMarketing) -> MarketingRegistry | ||
| public func withMarketing(from config: InternalProductConfig) -> MarketingRegistry | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
References removed type InternalProductConfig.
Line 469 references withMarketing(from config: InternalProductConfig) but InternalProductConfig was removed in this PR. This should reference ProductDefinition instead.
🔎 Fix outdated reference
// Immutable updates
public func withMarketing(_ productId: String, marketing: ProductMarketing) -> MarketingRegistry
- public func withMarketing(from config: InternalProductConfig) -> MarketingRegistry
+ public func withMarketing(from product: ProductDefinition) -> MarketingRegistry
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Immutable updates | |
| public func withMarketing(_ productId: String, marketing: ProductMarketing) -> MarketingRegistry | |
| public func withMarketing(from config: InternalProductConfig) -> MarketingRegistry | |
| } | |
| // Immutable updates | |
| public func withMarketing(_ productId: String, marketing: ProductMarketing) -> MarketingRegistry | |
| public func withMarketing(from product: ProductDefinition) -> MarketingRegistry | |
| } |
🤖 Prompt for AI Agents
In @docs/api-reference.md around lines 467-470, The doc signature for
MarketingRegistry still references the removed type InternalProductConfig;
update the declaration of withMarketing(from config: InternalProductConfig) to
use ProductDefinition instead (i.e., withMarketing(from config:
ProductDefinition)) and ensure any related references or imports in the same doc
block match the new ProductDefinition type; keep the function name
MarketingRegistry and withMarketing unchanged.
|
|
||
| /// Defines a product I want to sell. | ||
| /// "This is a product with these features and this marketing info" | ||
| public struct ProductDefinition: @unchecked Sendable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the ProductDefinition.swift file
find . -name "ProductDefinition.swift" -type fRepository: tddworks/InAppKit
Length of output: 115
🏁 Script executed:
# Check the file size and read its contents
wc -l Sources/InAppKit/Core/Domain/ProductDefinition.swift
head -120 Sources/InAppKit/Core/Domain/ProductDefinition.swiftRepository: tddworks/InAppKit
Length of output: 4004
🌐 Web query:
SwiftUI Color Sendable conformance Swift
💡 Result:
Short answer: SwiftUI's Color is not Sendable. It’s treated like a view/presentation type (and can wrap platform color objects), so the compiler warns when you try to use it in Sendable types or cross actor/task boundaries. [1][2]
Recommended approaches:
- Store/send a platform color or serializable representation instead (UIColor/NSColor/CGColor, RGBA/hex, Codable wrapper) and recreate Color where you need it. [3][2]
- If the Color is truly immutable and you accept the risk, mark your wrapper as @unchecked Sendable — but only if you guarantee thread-safety. [1]
- Alternatively, keep UI-related values isolated to the main actor (e.g., @mainactor) or avoid passing Color across concurrency domains. [1][2]
Sources:
- Swift Forums discussion showing Color treated as non-sendable and example patterns. [1]
- StackOverflow discussion: why SwiftUI types (Color, Views) are non-sendable and recommended use of platform color/serialization. [2]
- SwiftUI public interface (Color internals / platform ties). [3]
🏁 Script executed:
# Check DiscountRule more carefully and see if there are any compiler directives
grep -n "DiscountRule\|@unchecked\|Sendable" Sources/InAppKit/Core/Domain/ProductDefinition.swiftRepository: tddworks/InAppKit
Length of output: 532
🏁 Script executed:
# Check if there are any imports or special handling that might affect Sendable conformance
head -15 Sources/InAppKit/Core/Domain/ProductDefinition.swiftRepository: tddworks/InAppKit
Length of output: 306
🏁 Script executed:
# Search for any test files or compilation documentation
find . -name "*.swift" -type f | xargs grep -l "ProductDefinition\|DiscountRule" | head -10Repository: tddworks/InAppKit
Length of output: 416
🏁 Script executed:
# Check test files to understand how this code is used
cat -n ./Tests/InAppKitTests/Domain/MarketingRegistryTests.swift | head -50Repository: tddworks/InAppKit
Length of output: 1787
🏁 Script executed:
# Search for any Swift version requirements or build configuration
find . -name "Package.swift" -o -name ".swift-version" -o -name "*.yml" | head -5
cat Package.swift | head -50Repository: tddworks/InAppKit
Length of output: 1305
🏁 Script executed:
# Check if there are any conditional compilation flags or special handling
grep -r "os(iOS)\|os(macOS)\|#if" Sources/InAppKit/Core/Domain/ProductDefinition.swiftRepository: tddworks/InAppKit
Length of output: 78
🌐 Web query:
Swift 6.2 SwiftUI Color Sendable conformance
💡 Result:
Short answer: As of Swift 6-era guidance, SwiftUI's Color is treated as non-Sendable (it does not have a safe, public Sendable conformance). Do not rely on Color being Sendable — instead store a sendable representation (RGBA, hex, UIColor/CGColor on-platform, or your own value-type) or use @unchecked Sendable only after carefully verifying safety. [1][2][3]
References
- Discussion showing Color used as an example non‑sendable SwiftUI type and how it triggers Sendable warnings. [Swift Forums]. [1]
- StackOverflow explanation why many SwiftUI types (Color, Views) are non‑sendable and not intended to be passed between threads. [2]
- Swift 6.x release/discussion pages (concurrency changes; treat SwiftUI UI types cautiously). [3]
If you want, tell me which platform(s) and use case (actor, Sendable struct, SwiftData, etc.) and I’ll suggest exact patterns or code to store/transfer colors safely.
🌐 Web query:
Swift 5.10 Color Sendable protocol support
💡 Result:
Short answer: Swift 5.10 tightens Sendable checks and SwiftUI's Color is not declared Sendable, so using Color inside types required to be Sendable will trigger warnings/errors under strict concurrency. Options: (a) avoid putting Color in Sendable types and instead store plain color data (RGBA/hex) in a Sendable value type, (b) isolate the value with a global actor (e.g. @mainactor) or use nonisolated(unsafe) for specific storage, or (c) use @unchecked Sendable when you can guarantee thread-safety yourself.
Sources
- Swift 5.10 release notes (full data-isolation / Sendable behavior). [1]
- Swift Forums discussion showing Color used as example non-Sendable type and approaches like @unchecked Sendable / nonisolated(unsafe). [2]
- Community Q&A explaining why many SwiftUI types (Color/Text) are non-Sendable and recommending storing raw color data instead. [3]
Review @unchecked Sendable conformance—Color is not Sendable in Swift 6.2.
ProductDefinition is marked as @unchecked Sendable which bypasses compiler verification. This creates a safety concern:
Color(fieldsbadgeColorat line 44, and used indiscountRulefield at line 47 which is aDiscountRule) is not Sendable in Swift 6.2. Storing or passing Color across concurrency boundaries is unsafe.[AnyHashable](line 42) is onlySendableif all wrapped values areSendable, butAnyHashablecan wrap non-Sendabletypes at runtime.
Note: DiscountRule (line 18) is also marked as plain Sendable and contains a Color? field—this is inconsistent and may also need @unchecked or refactoring.
Consider either:
- Removing
@uncheckedand storing a serializable color representation (RGBA, hex, or platform-specific) instead ofColor - Documenting why this is safe if there's a specific concurrency guarantee
- Aligning
DiscountRuleto use the same pattern
🤖 Prompt for AI Agents
In @Sources/InAppKit/Core/Domain/ProductDefinition.swift around line 40,
ProductDefinition is incorrectly marked @unchecked Sendable while it stores
non-Sendable types; remove the @unchecked Sendable conformance and replace
non-Sendable fields: change badgeColor (and any Color usage inside discountRule)
from Color to a Sendable representation (e.g., a struct like RGBA or hex
String), update DiscountRule to use the same Sendable color representation (or
mark DiscountRule @unchecked Sendable with justification), and replace the
[AnyHashable] property with a concrete Sendable collection (e.g., [String] or
[AnyHashable] only if you validate/limit wrapped types to be Sendable) so all
stored properties of ProductDefinition are fully Sendable without bypassing
compiler checks.
| private func refreshPurchases() async { | ||
| do { | ||
| let purchased = try await store.purchases() | ||
| purchaseState = PurchaseState(purchasedProductIDs: purchased) | ||
| } catch { | ||
| Logger.statistics.error("Failed to refresh purchases: \(error.localizedDescription)") | ||
| } | ||
|
|
||
| self.purchasedProductIDs = purchasedProductIDs | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent failure in refreshPurchases may hide issues.
When store.purchases() fails, the error is logged but purchaseError is not set, unlike in loadProducts and restorePurchases. This inconsistency may make debugging harder for users.
🔎 Suggested fix for consistency
private func refreshPurchases() async {
do {
let purchased = try await store.purchases()
purchaseState = PurchaseState(purchasedProductIDs: purchased)
} catch {
Logger.statistics.error("Failed to refresh purchases: \(error.localizedDescription)")
+ purchaseError = error
}
}🤖 Prompt for AI Agents
In @Sources/InAppKit/Core/InAppKit.swift around lines 239-246, In
refreshPurchases() the catch only logs via Logger.statistics.error but doesn't
set purchaseError like loadProducts and restorePurchases do; update the catch
block to assign the caught error to purchaseError (e.g., purchaseError = error)
in addition to logging so the component state mirrors other failure paths and
diagnostic flows for store.purchases() failures are consistent with
loadProducts/restorePurchases.
- A middle-ground option: Move Store.swift from Domain/ to Core/ to signal it's a "boundary" protocol, not pure domain. The truly pure domain models (PurchaseState, FeatureRegistry, AccessControl) remain in Domain/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @Sources/InAppKit/Core/Store.swift:
- Around line 11-12: The protocol in Store.swift still exposes StoreKit.Product
in its method signatures, keeping an infrastructure dependency; remove the
direct StoreKit dependency by deleting the StoreKit import and introducing a
platform-agnostic product representation (e.g., ProductDTO or ProductProtocol)
used in the protocol signatures (referenced where StoreKit.Product appears
around the protocol methods), then update concrete implementations to map
between StoreKit.Product and this new lightweight Product representation at the
infrastructure boundary (convert StoreKit.Product <-> ProductDTO inside the
adapter/implementation), keeping tests and domain code free of StoreKit types.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Sources/InAppKit/Core/Store.swift
🧰 Additional context used
🧬 Code graph analysis (1)
Sources/InAppKit/Core/Store.swift (1)
Sources/InAppKit/Infrastructure/AppStore.swift (2)
purchases(58-77)restore(79-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Unit Tests
- GitHub Check: Build and Test
🔇 Additional comments (1)
Sources/InAppKit/Core/Store.swift (1)
31-36: Well-designed pure domain model.
PurchaseOutcomeis properly designed with no infrastructure dependencies—a good example of domain-level abstraction using only Swift primitives. TheEquatableandSendableconformances are appropriate for Swift 6.
| import StoreKit | ||
| import Mockable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Domain/infrastructure coupling persists despite file move.
The architectural issue flagged in the previous review remains unresolved: importing StoreKit and using StoreKit.Product in the protocol signature (lines 19, 22) tightly couples this abstraction to Apple's framework. Moving the file from Domain/ to Core/ acknowledges it's a boundary protocol, but doesn't eliminate the infrastructure dependency that prevents true domain independence and testability without StoreKit types.
🤖 Prompt for AI Agents
In @Sources/InAppKit/Core/Store.swift around lines 11-12, The protocol in
Store.swift still exposes StoreKit.Product in its method signatures, keeping an
infrastructure dependency; remove the direct StoreKit dependency by deleting the
StoreKit import and introducing a platform-agnostic product representation
(e.g., ProductDTO or ProductProtocol) used in the protocol signatures
(referenced where StoreKit.Product appears around the protocol methods), then
update concrete implementations to map between StoreKit.Product and this new
lightweight Product representation at the infrastructure boundary (convert
StoreKit.Product <-> ProductDTO inside the adapter/implementation), keeping
tests and domain code free of StoreKit types.
This pull request introduces significant improvements to the architecture, configuration, and testing of the
InAppKitSwift package. The most notable changes are the adoption of Domain-Driven Design, the integration of theMockablelibrary for dependency injection and mocking, and a major simplification and reorganization of the product configuration system. Additionally, the documentation has been updated to reflect these architectural changes and provide clearer guidance for development and testing.Architecture and Design Improvements:
Mockablelibrary, enabling easy mocking of store operations for testing and improving testability across the codebase. [1] [2]Configuration and API Simplification:
ProductConfiguration.swiftfile and replaced the previous product configuration types and fluent API with a new, simpler approach. The new documentation describes how to define products, features, and marketing information directly using the updated API. [1] [2]Testing Enhancements:
Mockableas a package dependency inPackage.swift, and configured both the main and test targets to support mocking in debug builds. This enables comprehensive domain and infrastructure testing, as outlined in the updated documentation.Documentation Updates:
CLAUDE.mdto provide a detailed architecture overview, usage examples, and testing strategies, ensuring the documentation matches the new code structure and patterns.Summary of Most Important Changes:
1. Architecture and Design
Mockablelibrary, improving testability and flexibility. [1] [2]2. Configuration and API
ProductConfiguration.swiftand replaced the product configuration system with a simpler, more maintainable approach as described in the new documentation. [1] [2]3. Testing
Mockableas a dependency and configured targets for mocking, enabling robust domain and infrastructure tests.Summary by CodeRabbit
New Features
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.