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

Prevent unnecessary receipt posts #323

Merged
merged 6 commits into from
Aug 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Purchases.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@
37E35F20B49BCE1B6D76B084 /* RCStoreKitRequestFetcher.h in Headers */ = {isa = PBXBuildFile; fileRef = 37E35804C14F5E6CEAF3909C /* RCStoreKitRequestFetcher.h */; settings = {ATTRIBUTES = (Private, ); }; };
37E35F20FB949985BEEB4B58 /* MockRequestFetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E35609E46E869675A466C1 /* MockRequestFetcher.swift */; };
37E35F549AEB655AB6DA83B3 /* MockSKDiscount.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E35EABF6D7AFE367718784 /* MockSKDiscount.swift */; };
37E35F67255A87BD86B39D43 /* MockReceiptParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E3558F697A939D2BBD7FEC /* MockReceiptParser.swift */; };
/* End PBXBuildFile section */

/* Begin PBXContainerItemProxy section */
Expand Down Expand Up @@ -374,6 +375,7 @@
37E35548F15DE7CFFCE3AA8A /* NSLocale+RCExtensions.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "NSLocale+RCExtensions.m"; sourceTree = "<group>"; };
37E3555B4BE0A4F7222E7B00 /* MockOfferingsFactory.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockOfferingsFactory.swift; sourceTree = "<group>"; };
37E355744D64075AA91342DE /* MockInAppPurchaseBuilder.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockInAppPurchaseBuilder.swift; sourceTree = "<group>"; };
37E3558F697A939D2BBD7FEC /* MockReceiptParser.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockReceiptParser.swift; sourceTree = "<group>"; };
37E355AE6CB674484555D1AC /* NSDate+RCExtensions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "NSDate+RCExtensions.h"; sourceTree = "<group>"; };
37E355CBB3F3A31A32687B14 /* Transaction.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Transaction.swift; sourceTree = "<group>"; };
37E35609E46E869675A466C1 /* MockRequestFetcher.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockRequestFetcher.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -784,6 +786,7 @@
2DD7BA4C24C63A830066B4C2 /* MockSystemInfo.swift */,
37E35659EB530A5109AFAB50 /* MockOperationDispatcher.swift */,
2D4D6AF224F7172900B656BE /* MockProductsRequest.swift */,
37E3558F697A939D2BBD7FEC /* MockReceiptParser.swift */,
);
path = Mocks;
sourceTree = "<group>";
Expand Down Expand Up @@ -1428,6 +1431,7 @@
37E35F0387D0ADE014186924 /* ProductInfoExtractorTests.swift in Sources */,
37E351505CB4764821451E27 /* ProductInfoExtensions.swift in Sources */,
37E3599326581376E0142EEC /* SystemInfoTests.swift in Sources */,
37E35F67255A87BD86B39D43 /* MockReceiptParser.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
6 changes: 4 additions & 2 deletions Purchases/ProtectedExtensions/RCPurchases+Protected.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
RCSubscriberAttributesManager,
RCSystemInfo,
RCOperationDispatcher,
RCIntroEligibilityCalculator;
RCIntroEligibilityCalculator,
RCReceiptParser;

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -40,7 +41,8 @@ NS_ASSUME_NONNULL_BEGIN
identityManager:(RCIdentityManager *)identityManager
subscriberAttributesManager:(RCSubscriberAttributesManager *)subscriberAttributesManager
operationDispatcher:(RCOperationDispatcher *)operationDispatcher
introEligibilityCalculator:(RCIntroEligibilityCalculator *)introEligibilityCalculator;
introEligibilityCalculator:(RCIntroEligibilityCalculator *)introEligibilityCalculator
receiptParser:(RCReceiptParser *)receiptParser;

+ (void)setDefaultInstance:(nullable RCPurchases *)instance;

Expand Down
18 changes: 16 additions & 2 deletions Purchases/Public/RCPurchases.m
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ @interface RCPurchases () <RCStoreKitWrapperDelegate> {
@property (nonatomic) RCSystemInfo *systemInfo;
@property (nonatomic) RCOperationDispatcher *operationDispatcher;
@property (nonatomic) RCIntroEligibilityCalculator *introEligibilityCalculator;
@property (nonatomic) RCReceiptParser *receiptParser;

@end

Expand Down Expand Up @@ -225,6 +226,7 @@ - (instancetype)initWithAPIKey:(NSString *)APIKey
deviceCache:deviceCache];
RCOperationDispatcher *operationDispatcher = [[RCOperationDispatcher alloc] init];
RCIntroEligibilityCalculator *introCalculator = [[RCIntroEligibilityCalculator alloc] init];
RCReceiptParser *receiptParser = [[RCReceiptParser alloc] init];

return [self initWithAppUserID:appUserID
requestFetcher:fetcher
Expand All @@ -240,7 +242,8 @@ - (instancetype)initWithAPIKey:(NSString *)APIKey
identityManager:identityManager
subscriberAttributesManager:subscriberAttributesManager
operationDispatcher:operationDispatcher
introEligibilityCalculator:introCalculator];
introEligibilityCalculator:introCalculator
receiptParser:receiptParser];
}

- (instancetype)initWithAppUserID:(nullable NSString *)appUserID
Expand All @@ -257,7 +260,8 @@ - (instancetype)initWithAppUserID:(nullable NSString *)appUserID
identityManager:(RCIdentityManager *)identityManager
subscriberAttributesManager:(RCSubscriberAttributesManager *)subscriberAttributesManager
operationDispatcher:(RCOperationDispatcher *)operationDispatcher
introEligibilityCalculator:(RCIntroEligibilityCalculator *)introEligibilityCalculator {
introEligibilityCalculator:(RCIntroEligibilityCalculator *)introEligibilityCalculator
receiptParser:(RCReceiptParser *)receiptParser {
if (self = [super init]) {
RCDebugLog(@"Debug logging enabled.");
RCDebugLog(@"SDK Version - %@", self.class.frameworkVersion);
Expand All @@ -283,6 +287,7 @@ - (instancetype)initWithAppUserID:(nullable NSString *)appUserID
self.subscriberAttributesManager = subscriberAttributesManager;
self.operationDispatcher = operationDispatcher;
self.introEligibilityCalculator = introEligibilityCalculator;
self.receiptParser = receiptParser;

RCReceivePurchaserInfoBlock callDelegate = ^void(RCPurchaserInfo *info, NSError *error) {
if (info) {
Expand Down Expand Up @@ -625,6 +630,15 @@ - (void)restoreTransactionsWithCompletionBlock:(nullable RCReceivePurchaserInfoB
CALL_IF_SET_ON_MAIN_THREAD(completion, nil, [RCPurchasesErrorUtils missingReceiptFileError]);
return;
}

RCPurchaserInfo * _Nullable cachedPurchaserInfo = [self readPurchaserInfoFromCache];
Copy link
Member Author

Choose a reason for hiding this comment

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

we'll move a lot of stuff out of this class eventually, but it's out-of-scope for this PR

BOOL hasOriginalPurchaseDate = cachedPurchaserInfo != nil && cachedPurchaserInfo.originalPurchaseDate != nil;
BOOL receiptHasTransactions = [self.receiptParser receiptHasTransactionsWithReceiptData:data];
if (!receiptHasTransactions && hasOriginalPurchaseDate) {
CALL_IF_SET_ON_MAIN_THREAD(completion, cachedPurchaserInfo, nil);
return;
}

RCSubscriberAttributeDict subscriberAttributes = self.unsyncedAttributesByKey;
[self.backend postReceiptData:data
appUserID:self.appUserID
Expand Down
29 changes: 22 additions & 7 deletions PurchasesCoreSwift/LocalReceiptParsing/ReceiptParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,32 @@

import Foundation

class ReceiptParser {
private let objectIdentifierParser: ASN1ObjectIdentifierBuilder
@objc(RCReceiptParser) public class ReceiptParser: NSObject {
private let objectIdentifierBuilder: ASN1ObjectIdentifierBuilder
private let containerBuilder: ASN1ContainerBuilder
private let receiptBuilder: AppleReceiptBuilder

init(objectIdentifierParser: ASN1ObjectIdentifierBuilder = ASN1ObjectIdentifierBuilder(),
containerBuilder: ASN1ContainerBuilder = ASN1ContainerBuilder(),
receiptBuilder: AppleReceiptBuilder = AppleReceiptBuilder()) {
self.objectIdentifierParser = objectIdentifierParser
@objc public convenience override init() {
self.init(objectIdentifierBuilder: ASN1ObjectIdentifierBuilder(),
containerBuilder: ASN1ContainerBuilder(),
receiptBuilder: AppleReceiptBuilder())
}

init(objectIdentifierBuilder: ASN1ObjectIdentifierBuilder,
containerBuilder: ASN1ContainerBuilder,
receiptBuilder: AppleReceiptBuilder) {
self.objectIdentifierBuilder = objectIdentifierBuilder
self.containerBuilder = containerBuilder
self.receiptBuilder = receiptBuilder
super.init()
}

@objc public func receiptHasTransactions(receiptData: Data) -> Bool {
if let receipt = try? parse(from: receiptData) {
return receipt.inAppPurchases.count > 0
}
// if the receipt can't be parsed, conservatively return true
return true
Comment on lines +31 to +36
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not a huge fan of returning a Bool here, since there are technically three possible cases:

  • true
  • false
  • unknown (receipt can't be parsed).

However, this isn't trivial to do in swift-objc interoperability:

  • you can either use a throws, which forces you to replace the Bool with NSNumber or an inout param, and the caller has to pass in an error, and then unwrap the NSNumber or pass in the inout param
  • encode the return into an object that basically wraps a Bool, then use that to encode the value.

They're not bad, but they did feel too complicated for what we're doing here.

More info: https://stackoverflow.com/a/36673027/5074358

Copy link
Member Author

Choose a reason for hiding this comment

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

if you're curious about what this would look like, I have a stash that I can upload into a branch for perspective

}

func parse(from receiptData: Data) throws -> AppleReceipt {
Expand All @@ -40,7 +55,7 @@ private extension ReceiptParser {
if container.encodingType == .constructed {
for (index, internalContainer) in container.internalContainers.enumerated() {
if internalContainer.containerIdentifier == .objectIdentifier {
let objectIdentifier = objectIdentifierParser.build(fromPayload: internalContainer.internalPayload)
let objectIdentifier = objectIdentifierBuilder.build(fromPayload: internalContainer.internalPayload)
Comment on lines -43 to +58
Copy link
Member Author

Choose a reason for hiding this comment

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

unrelated naming cleanup

if objectIdentifier == objectId && index < container.internalContainers.count - 1 {
// the container that holds the data comes right after the one with the object identifier
return container.internalContainers[index + 1]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ class ReceiptParserTests: XCTestCase {
super.setUp()
mockAppleReceiptBuilder = MockAppleReceiptBuilder()
mockASN1ContainerBuilder = MockASN1ContainerBuilder()
receiptParser = ReceiptParser(containerBuilder: mockASN1ContainerBuilder,
receiptParser = ReceiptParser(objectIdentifierBuilder: ASN1ObjectIdentifierBuilder(),
containerBuilder: mockASN1ContainerBuilder,
receiptBuilder: mockAppleReceiptBuilder)
}

Expand All @@ -27,7 +28,7 @@ class ReceiptParserTests: XCTestCase {
])

mockASN1ContainerBuilder.stubbedBuildResult = constructedContainer
let expectedReceipt = mockAppleReceipt()
let expectedReceipt = mockAppleReceiptWithoutPurchases()
mockAppleReceiptBuilder.stubbedBuildResult = expectedReceipt

let receivedReceipt = try! self.receiptParser.parse(from: Data())
Expand Down Expand Up @@ -63,7 +64,7 @@ class ReceiptParserTests: XCTestCase {
])

mockASN1ContainerBuilder.stubbedBuildResult = complexContainer
let expectedReceipt = mockAppleReceipt()
let expectedReceipt = mockAppleReceiptWithoutPurchases()
mockAppleReceiptBuilder.stubbedBuildResult = expectedReceipt

let receivedReceipt = try! self.receiptParser.parse(from: Data())
Expand Down Expand Up @@ -105,6 +106,23 @@ class ReceiptParserTests: XCTestCase {
expect { try self.receiptParser.parse(from: Data()) }
.to(throwError(ReceiptReadingError.dataObjectIdentifierMissing))
}

func testReceiptHasTransactionsTrueIfReceiptHasTransactions() {
mockASN1ContainerBuilder.stubbedBuildResult = containerWithDataObjectIdentifier()
mockAppleReceiptBuilder.stubbedBuildResult = mockAppleReceiptWithPurchases()
expect(self.receiptParser.receiptHasTransactions(receiptData: Data())) == true
}

func testReceiptHasTransactionsFalseIfNoIAPsInReceipt() {
mockASN1ContainerBuilder.stubbedBuildResult = containerWithDataObjectIdentifier()
mockAppleReceiptBuilder.stubbedBuildResult = mockAppleReceiptWithoutPurchases()
expect(self.receiptParser.receiptHasTransactions(receiptData: Data())) == false
}

func testReceiptHasTransactionsTrueIfReceiptCantBeParsed() {
mockASN1ContainerBuilder.stubbedBuildError = ReceiptReadingError.receiptParsingError
expect(self.receiptParser.receiptHasTransactions(receiptData: Data())) == true
}
}

private extension ReceiptParserTests {
Expand All @@ -118,7 +136,7 @@ private extension ReceiptParserTests {
return constructedContainer
}

func mockAppleReceipt() -> AppleReceipt {
func mockAppleReceiptWithoutPurchases() -> AppleReceipt {
return AppleReceipt(bundleId: "com.revenuecat.testapp",
applicationVersion: "3.2.3",
originalApplicationVersion: "3.1.1",
Expand All @@ -128,4 +146,42 @@ private extension ReceiptParserTests {
expirationDate: nil,
inAppPurchases: [])
}

func mockAppleReceiptWithPurchases() -> AppleReceipt {
return AppleReceipt(bundleId: "com.revenuecat.testapp",
applicationVersion: "3.2.3",
originalApplicationVersion: "3.1.1",
opaqueValue: Data(),
sha1Hash: Data(),
creationDate: Date(),
expirationDate: nil,
inAppPurchases: [
InAppPurchase(quantity: 1,
productId: "com.revenuecat.test",
transactionId: "892398531",
originalTransactionId: "892398531",
productType: .autoRenewableSubscription,
purchaseDate: Date(),
originalPurchaseDate: Date(),
expiresDate: nil,
cancellationDate: Date(),
isInTrialPeriod: false,
isInIntroOfferPeriod: false,
webOrderLineItemId: 79238531,
promotionalOfferIdentifier: nil),
InAppPurchase(quantity: 1,
productId: "com.revenuecat.test",
transactionId: "892398532",
originalTransactionId: "892398531",
productType: .autoRenewableSubscription,
purchaseDate: Date(),
originalPurchaseDate: Date(),
expiresDate: nil,
cancellationDate: Date(),
isInTrialPeriod: false,
isInIntroOfferPeriod: false,
webOrderLineItemId: 79238532,
promotionalOfferIdentifier: nil)
])
}
}
6 changes: 6 additions & 0 deletions PurchasesCoreSwiftTests/Mocks/MockReceiptParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ class MockReceiptParser: ReceiptParser {
expirationDate: nil,
inAppPurchases: [])

convenience init() {
self.init(objectIdentifierBuilder: ASN1ObjectIdentifierBuilder(),
containerBuilder: MockASN1ContainerBuilder(),
receiptBuilder: MockAppleReceiptBuilder())
}

override func parse(from receiptData: Data) throws -> AppleReceipt {
invokedParse = true
invokedParseCount += 1
Expand Down
30 changes: 30 additions & 0 deletions PurchasesTests/Mocks/MockReceiptParser.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//
// Created by Andrés Boedo on 8/27/20.
// Copyright (c) 2020 Purchases. All rights reserved.
//

import Foundation
@testable import PurchasesCoreSwift

class MockReceiptParser: ReceiptParser {

init() {
super.init(objectIdentifierBuilder: ASN1ObjectIdentifierBuilder(),
containerBuilder: ASN1ContainerBuilder(),
receiptBuilder: AppleReceiptBuilder())
}

var invokedReceiptHasTransactions = false
var invokedReceiptHasTransactionsCount = 0
var invokedReceiptHasTransactionsParameters: (receiptData: Data, Void)?
var invokedReceiptHasTransactionsParametersList = [(receiptData: Data, Void)]()
var stubbedReceiptHasTransactionsResult: Bool! = false

override func receiptHasTransactions(receiptData: Data) -> Bool {
invokedReceiptHasTransactions = true
invokedReceiptHasTransactionsCount += 1
invokedReceiptHasTransactionsParameters = (receiptData, ())
invokedReceiptHasTransactionsParametersList.append((receiptData, ()))
return stubbedReceiptHasTransactionsResult
}
}
Loading