Skip to content
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

Created PostedTransactionCache #2911

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Created PostedTransactionCache #2911

merged 2 commits into from
Aug 7, 2023

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented Jul 27, 2023

This will be used to skip posting the same transaction multiple times, as a way to work around StoreKit reporting SK1 notifications from SK2 APIs.

@NachoSoto NachoSoto added the perf A code change that improves performance label Jul 27, 2023
@NachoSoto NachoSoto requested a review from a team July 27, 2023 15:55
@NachoSoto NachoSoto marked this pull request as draft July 27, 2023 15:55
// TODO: tests
// TODO: integrate

final class PostedTransactionCache: PostedTransactionCacheType {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses DeviceCache underneath (composition, not inheritance). It also doesn't make DeviceCache have any more domain-specific caching code.

This is how I want to refactor DeviceCache so it's split in smaller classes.

@NachoSoto NachoSoto changed the title [WIP] Created PostedTransactionCache Created PostedTransactionCache Jul 28, 2023
@NachoSoto NachoSoto added refactor A code change that neither fixes a bug nor adds a feature and removed perf A code change that improves performance labels Jul 28, 2023
Comment on lines +29 to +32
self.userDefaults = try XCTUnwrap(.init(suiteName: UUID().uuidString))
self.deviceCache = .init(sandboxEnvironmentDetector: MockSandboxEnvironmentDetector(isSandbox: true),
userDefaults: self.userDefaults)
self.cache = .init(deviceCache: self.deviceCache)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to test with with real DeviceCache and UserDefaults which is a lot more accurate and less painful.

@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #2911 (a6777aa) into main (f4fa4d2) will increase coverage by 0.02%.
The diff coverage is 97.77%.

@@            Coverage Diff             @@
##             main    #2911      +/-   ##
==========================================
+ Coverage   86.60%   86.63%   +0.02%     
==========================================
  Files         217      218       +1     
  Lines       15517    15545      +28     
==========================================
+ Hits        13439    13467      +28     
  Misses       2078     2078              
Files Changed Coverage Δ
Sources/Caching/DeviceCache.swift 93.04% <96.87%> (+0.47%) ⬆️
Sources/Caching/PostedTransactionCache.swift 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

}

func savePostedTransaction(_ transaction: StoreTransactionType) {
self.deviceCache.update(key: CacheKey.transactions,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: do a quick test on device with N x 1000 transactions.

Copy link
Contributor

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small question but not show stopping. Looks good 👍

@@ -552,47 +569,56 @@ private extension DeviceCache {
_ userDefaults: UserDefaults,
productEntitlementMapping mapping: ProductEntitlementMapping
) {
guard let data = try? JSONEncoder.default.encode(value: mapping, logErrors: true) else {
return
if userDefaults.set(codable: mapping,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any sort of error that we should be logging if this fails?

/// - Returns: whether the value could be saved
@discardableResult
func set<T: Codable>(codable: T, forKey key: DeviceCacheKeyType) -> Bool {
guard let data = try? JSONEncoder.default.encode(value: codable, logErrors: true) else {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshdholtz this logErrors: true answers your question.

@NachoSoto NachoSoto merged commit 2b87c30 into main Aug 7, 2023
14 checks passed
@NachoSoto NachoSoto deleted the cache-posted-transactions branch August 7, 2023 22:41
NachoSoto added a commit that referenced this pull request Aug 9, 2023
_This release is compatible with Xcode 15 beta 6 and visionOS beta 3_

### Bugfixes
* `visionOS`: support for `Xcode 15 beta 6` (#2989) via NachoSoto
(@NachoSoto)
* `CachingProductsManager`: avoid crash when caching different products
with same identifier (#2979) via NachoSoto (@NachoSoto)
* `PurchasesOrchestrator`: disambiguate transactions from the queue
(#2890) via NachoSoto (@NachoSoto)
### Performance Improvements
* `StoreKit2TransactionListener`: handle transactions asynchronously
(#2910) via NachoSoto (@NachoSoto)
### Other Changes
* `Atomic`: avoid race conditions modifying dictionaries (#2981) via
NachoSoto (@NachoSoto)
* `Logging`: avoid logging "updating offerings" when request is cached
(#2904) via NachoSoto (@NachoSoto)
* `StoreKit2TransactionListener`: converted into an `actor` (#2909) via
NachoSoto (@NachoSoto)
* `Integration Tests`: added more observer mode tests (#2905) via
NachoSoto (@NachoSoto)
* Created `PostedTransactionCache` (#2911) via NachoSoto (@NachoSoto)
* `IntroEligibility`: changed products to `Set<String>` (#2976) via
NachoSoto (@NachoSoto)
* `AdServices`: Rename `postAdServicesTokenIfNeeded()` to
`postAdServicesTokenOncePerInstallIfNeeded()` (#2968) via Josh Holtz
(@joshdholtz)
* `SK1StoreProduct`: changed `productType` warning to debug (#2957) via
NachoSoto (@NachoSoto)
* `PrivacyInfo.xcprivacy`: added `UserDefaults` to access API types
(#2913) via NachoSoto (@NachoSoto)
* `Integration Tests`: new test to verify that SK1 purchases don't leave
SK2 unfinished transactions (#2906) via NachoSoto (@NachoSoto)
* `Logging`: log entire cache key with verbose logs (#2888) via
NachoSoto (@NachoSoto)
* `StoreProduct`: added test covering warning log (#2897) via NachoSoto
(@NachoSoto)
* `CustomEntitlementComputation`: use custom API key (#2879) via Toni
Rico (@tonidero)
* `CachingProductsManager`: removed duplicate log and added tests
(#2898) via NachoSoto (@NachoSoto)
* `Xcode 15 beta 5`: fixed test compilation (#2885) via NachoSoto
(@NachoSoto)

---------

Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
MarkVillacampa pushed a commit that referenced this pull request Sep 6, 2023
This will be used to skip posting the same transaction multiple times,
as a way to work around StoreKit reporting SK1 notifications from SK2
APIs.
MarkVillacampa pushed a commit that referenced this pull request Sep 6, 2023
_This release is compatible with Xcode 15 beta 6 and visionOS beta 3_

### Bugfixes
* `visionOS`: support for `Xcode 15 beta 6` (#2989) via NachoSoto
(@NachoSoto)
* `CachingProductsManager`: avoid crash when caching different products
with same identifier (#2979) via NachoSoto (@NachoSoto)
* `PurchasesOrchestrator`: disambiguate transactions from the queue
(#2890) via NachoSoto (@NachoSoto)
### Performance Improvements
* `StoreKit2TransactionListener`: handle transactions asynchronously
(#2910) via NachoSoto (@NachoSoto)
### Other Changes
* `Atomic`: avoid race conditions modifying dictionaries (#2981) via
NachoSoto (@NachoSoto)
* `Logging`: avoid logging "updating offerings" when request is cached
(#2904) via NachoSoto (@NachoSoto)
* `StoreKit2TransactionListener`: converted into an `actor` (#2909) via
NachoSoto (@NachoSoto)
* `Integration Tests`: added more observer mode tests (#2905) via
NachoSoto (@NachoSoto)
* Created `PostedTransactionCache` (#2911) via NachoSoto (@NachoSoto)
* `IntroEligibility`: changed products to `Set<String>` (#2976) via
NachoSoto (@NachoSoto)
* `AdServices`: Rename `postAdServicesTokenIfNeeded()` to
`postAdServicesTokenOncePerInstallIfNeeded()` (#2968) via Josh Holtz
(@joshdholtz)
* `SK1StoreProduct`: changed `productType` warning to debug (#2957) via
NachoSoto (@NachoSoto)
* `PrivacyInfo.xcprivacy`: added `UserDefaults` to access API types
(#2913) via NachoSoto (@NachoSoto)
* `Integration Tests`: new test to verify that SK1 purchases don't leave
SK2 unfinished transactions (#2906) via NachoSoto (@NachoSoto)
* `Logging`: log entire cache key with verbose logs (#2888) via
NachoSoto (@NachoSoto)
* `StoreProduct`: added test covering warning log (#2897) via NachoSoto
(@NachoSoto)
* `CustomEntitlementComputation`: use custom API key (#2879) via Toni
Rico (@tonidero)
* `CachingProductsManager`: removed duplicate log and added tests
(#2898) via NachoSoto (@NachoSoto)
* `Xcode 15 beta 5`: fixed test compilation (#2885) via NachoSoto
(@NachoSoto)

---------

Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PostReceipt optimizations refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants