Skip to content

Commit

Permalink
Fix NSNull Attributes crash (#359)
Browse files Browse the repository at this point in the history
* added test that fails if NSNull value is passed into attribution

* added NSDictionary extension to filter out NSNull values

* fixed a bug in the test

* fix regression

* removed unnecessary usage of removingNSNullValues, added tests to ensure that the right value is cached for postAttributionData
  • Loading branch information
aboedo committed Sep 30, 2020
1 parent 93ed171 commit 6238bfb
Show file tree
Hide file tree
Showing 9 changed files with 180 additions and 9 deletions.
10 changes: 5 additions & 5 deletions Examples/SwiftExample/Podfile.lock
@@ -1,7 +1,7 @@
PODS:
- Purchases (3.7.0-SNAPSHOT):
- PurchasesCoreSwift (= 3.7.0-SNAPSHOT)
- PurchasesCoreSwift (3.7.0-SNAPSHOT)
- Purchases (3.8.0-SNAPSHOT):
- PurchasesCoreSwift (= 3.8.0-SNAPSHOT)
- PurchasesCoreSwift (3.8.0-SNAPSHOT)

DEPENDENCIES:
- Purchases (from `../../`)
Expand All @@ -14,8 +14,8 @@ EXTERNAL SOURCES:
:path: "../../"

SPEC CHECKSUMS:
Purchases: 646df90239df1b8245395d7e15c5741ac1f15aed
PurchasesCoreSwift: 72df98f7f32ea1c646e8bdb25df3d71559cfeed2
Purchases: 883b1a2c57cd2783fe3e73f264c37ab897d34cd5
PurchasesCoreSwift: e2369fb313cba83adcd3fd033a2415c7cdf62638

PODFILE CHECKSUM: abf85a7b493b4247cdd1f63fcaee462b025a5eed

Expand Down
12 changes: 12 additions & 0 deletions Purchases.xcodeproj/project.pbxproj
Expand Up @@ -22,6 +22,9 @@
2D50332A2406EA61009CAE61 /* RCSubscriberAttributesManager.h in Headers */ = {isa = PBXBuildFile; fileRef = 2D5033272406EA61009CAE61 /* RCSubscriberAttributesManager.h */; settings = {ATTRIBUTES = (Private, ); }; };
2D50332B2406EA61009CAE61 /* RCSubscriberAttributesManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 2D5033282406EA61009CAE61 /* RCSubscriberAttributesManager.m */; };
2D8DB34B24072AAE00BE3D31 /* SubscriberAttributeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D8DB34A24072AAE00BE3D31 /* SubscriberAttributeTests.swift */; };
2D8E9D482523747D00AFEE11 /* NSDictionaryExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DD269162522A20A006AC4BC /* NSDictionaryExtensionsTests.swift */; };
2D8E9D54252374A600AFEE11 /* NSDictionary+RCExtensions.h in Headers */ = {isa = PBXBuildFile; fileRef = 2D8E9D52252374A600AFEE11 /* NSDictionary+RCExtensions.h */; settings = {ATTRIBUTES = (Private, ); }; };
2D8E9D55252374A600AFEE11 /* NSDictionary+RCExtensions.m in Sources */ = {isa = PBXBuildFile; fileRef = 2D8E9D53252374A600AFEE11 /* NSDictionary+RCExtensions.m */; };
2DC5621F24EC63430031F69B /* PurchasesCoreSwift.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 2DC5621624EC63420031F69B /* PurchasesCoreSwift.framework */; };
2DC5622624EC63430031F69B /* PurchasesCoreSwift.h in Headers */ = {isa = PBXBuildFile; fileRef = 2DC5621824EC63430031F69B /* PurchasesCoreSwift.h */; settings = {ATTRIBUTES = (Public, ); }; };
2DC5622E24EC636C0031F69B /* Transaction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E355CBB3F3A31A32687B14 /* Transaction.swift */; };
Expand Down Expand Up @@ -260,6 +263,8 @@
2D5BB46A24C8E8ED00E27537 /* ReceiptParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReceiptParser.swift; sourceTree = "<group>"; };
2D604CA224E5BF37004821DC /* RCTransaction.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = RCTransaction.h; sourceTree = "<group>"; };
2D8DB34A24072AAE00BE3D31 /* SubscriberAttributeTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SubscriberAttributeTests.swift; sourceTree = "<group>"; };
2D8E9D52252374A600AFEE11 /* NSDictionary+RCExtensions.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "NSDictionary+RCExtensions.h"; sourceTree = "<group>"; };
2D8E9D53252374A600AFEE11 /* NSDictionary+RCExtensions.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "NSDictionary+RCExtensions.m"; sourceTree = "<group>"; };
2D8F622224D30F9D00F993AA /* ReceiptParsingError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReceiptParsingError.swift; sourceTree = "<group>"; };
2D97458E24BDFCEF006245E9 /* IntroEligibilityCalculator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IntroEligibilityCalculator.swift; sourceTree = "<group>"; };
2DA0068E24E2E515002C59D3 /* MockIntroEligibilityCalculator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockIntroEligibilityCalculator.swift; sourceTree = "<group>"; };
Expand All @@ -269,6 +274,7 @@
2DC5621E24EC63430031F69B /* PurchasesCoreSwiftTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = PurchasesCoreSwiftTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
2DC5622524EC63430031F69B /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
2DD02D5624AD0B0500419CD9 /* RCIntroEligibilityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RCIntroEligibilityTests.swift; sourceTree = "<group>"; };
2DD269162522A20A006AC4BC /* NSDictionaryExtensionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSDictionaryExtensionsTests.swift; sourceTree = "<group>"; };
2DD448FD24088473002F5694 /* RCPurchases+SubscriberAttributes.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "RCPurchases+SubscriberAttributes.h"; path = "Purchases/SubscriberAttributes/RCPurchases+SubscriberAttributes.h"; sourceTree = SOURCE_ROOT; };
2DD448FE24088473002F5694 /* RCPurchases+SubscriberAttributes.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = "RCPurchases+SubscriberAttributes.m"; path = "Purchases/SubscriberAttributes/RCPurchases+SubscriberAttributes.m"; sourceTree = SOURCE_ROOT; };
2DD7BA4C24C63A830066B4C2 /* MockSystemInfo.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockSystemInfo.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1011,6 +1017,8 @@
37E35088AB7C4F7A242D3ED5 /* NSError+RCExtensions.m */,
37E3528315F4F007250F62B2 /* NSLocale+RCExtensions.h */,
37E35548F15DE7CFFCE3AA8A /* NSLocale+RCExtensions.m */,
2D8E9D52252374A600AFEE11 /* NSDictionary+RCExtensions.h */,
2D8E9D53252374A600AFEE11 /* NSDictionary+RCExtensions.m */,
);
path = FoundationExtensions;
sourceTree = "<group>";
Expand Down Expand Up @@ -1055,6 +1063,7 @@
isa = PBXGroup;
children = (
3589D15324C219BE00A65CBB /* AttributionFetcherTests.swift */,
2DD269162522A20A006AC4BC /* NSDictionaryExtensionsTests.swift */,
37E35B9AC7A350CA2437049D /* ISOPeriodFormatterTests.swift */,
37E35EEE7783629CDE41B70C /* SystemInfoTests.swift */,
);
Expand Down Expand Up @@ -1131,6 +1140,7 @@
3589D15624C21DBD00A65CBB /* RCAttributionFetcher+Protected.h in Headers */,
37E357301A5D15C0E90C9BD3 /* RCProductInfoExtractor.h in Headers */,
37E352039F075299CD4CF6B0 /* RCHTTPRequest.h in Headers */,
2D8E9D54252374A600AFEE11 /* NSDictionary+RCExtensions.h in Headers */,
37E35A328C4DC8A4D2DB4B06 /* RCAttributionNetwork.h in Headers */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down Expand Up @@ -1510,6 +1520,7 @@
2D50332B2406EA61009CAE61 /* RCSubscriberAttributesManager.m in Sources */,
354D50E022E7D014009B870C /* RCOfferings.m in Sources */,
35CE74FD20C38A7100CE09D8 /* RCOffering.m in Sources */,
2D8E9D55252374A600AFEE11 /* NSDictionary+RCExtensions.m in Sources */,
37E35C90AAEFEA77BB48DC0F /* NSData+RCExtensions.m in Sources */,
37E3556DD7FB5317A43086CC /* NSDate+RCExtensions.m in Sources */,
37E35B4BDE7D7B1EFB3EE800 /* NSError+RCExtensions.m in Sources */,
Expand Down Expand Up @@ -1563,6 +1574,7 @@
37E35F20FB949985BEEB4B58 /* MockRequestFetcher.swift in Sources */,
37E35AD0B0D9EF0CDA29DAC2 /* MockStoreKitWrapper.swift in Sources */,
37E35EBDFC5CD3068E1792A3 /* MockNotificationCenter.swift in Sources */,
2D8E9D482523747D00AFEE11 /* NSDictionaryExtensionsTests.swift in Sources */,
37E354E0A9A371481540B2B0 /* MockAttributionFetcher.swift in Sources */,
37E35EDC57C486AC2D66B4B8 /* MockOfferingsFactory.swift in Sources */,
2DD7BA4D24C63A830066B4C2 /* MockSystemInfo.swift in Sources */,
Expand Down
2 changes: 1 addition & 1 deletion Purchases/Caching/RCDeviceCache.m
Expand Up @@ -9,7 +9,7 @@
#import "RCDeviceCache.h"
#import "RCDeviceCache+Protected.h"
#import "RCLogUtils.h"

#import "NSDictionary+RCExtensions.h"

@interface RCDeviceCache ()

Expand Down
21 changes: 21 additions & 0 deletions Purchases/FoundationExtensions/NSDictionary+RCExtensions.h
@@ -0,0 +1,21 @@
//
// NSDictionary+RCExtensions.h
// Purchases
//
// Created by Andrés Boedo on 9/29/20.
// Copyright © 2020 Purchases. All rights reserved.
//

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN


@interface NSDictionary (RCExtensions)

- (NSDictionary *)removingNSNullValues;

@end


NS_ASSUME_NONNULL_END
34 changes: 34 additions & 0 deletions Purchases/FoundationExtensions/NSDictionary+RCExtensions.m
@@ -0,0 +1,34 @@
//
// NSDictionary+RCExtensions.m
// Purchases
//
// Created by Andrés Boedo on 9/29/20.
// Copyright © 2020 Purchases. All rights reserved.
//

#import "NSDictionary+RCExtensions.h"

NS_ASSUME_NONNULL_BEGIN

@implementation NSDictionary (RCExtensions)

- (NSDictionary *)removingNSNullValues {
NSMutableDictionary *result = [[NSMutableDictionary alloc] init];

NSEnumerator *enumerator = [self keyEnumerator];
id key;

while ((key = enumerator.nextObject)) {
id value = self[key];
if (![value isKindOfClass:NSNull.class]) {
result[key] = value;
}
}

return result;
}

NS_ASSUME_NONNULL_END


@end
2 changes: 1 addition & 1 deletion Purchases/Purchasing/RCAttributionFetcher.m
Expand Up @@ -155,7 +155,7 @@ - (void)postAttributionData:(NSDictionary *)data
forAppUserID:appUserID
completion:^(NSError *_Nullable error) {
if (error == nil) {
[self.deviceCache setLatestNetworkAndAdvertisingIdsSent:newData
[self.deviceCache setLatestNetworkAndAdvertisingIdsSent:newDictToCache
forAppUserID:appUserID];
}
}];
Expand Down
73 changes: 71 additions & 2 deletions PurchasesTests/Misc/AttributionFetcherTests.swift
Expand Up @@ -17,14 +17,24 @@ class AttributionFetcherTests: XCTestCase {
var deviceCache: MockDeviceCache!
var identityManager: MockIdentityManager!

let userDefaultsSuiteName = "testUserDefaults"

override func setUp() {
super.setUp()
deviceCache = MockDeviceCache()
identityManager = MockIdentityManager(mockAppUserID: "userID")
deviceCache = MockDeviceCache(UserDefaults(suiteName: userDefaultsSuiteName)!)
let userID = "userID"
deviceCache.cacheAppUserID(userID)

identityManager = MockIdentityManager(mockAppUserID: userID)
attributionFetcher = RCAttributionFetcher(deviceCache: deviceCache,
identityManager: identityManager,
backend: MockBackend())
}

override func tearDown() {
UserDefaults.standard.removePersistentDomain(forName: userDefaultsSuiteName)
UserDefaults.standard.synchronize()
}

func testCanRotateASIdentifierManager() {
let expected = "ASIdentifierManager"
Expand Down Expand Up @@ -55,5 +65,64 @@ class AttributionFetcherTests: XCTestCase {

expect { self.attributionFetcher.rot13(randomized) } .to(equal(expected))
}

func testPostAttributionDataSkipsIfAlreadySent() {
let userID = "userID"
let backend = MockBackend()
backend.stubbedPostAttributionDataCompletionResult = (nil, ())

attributionFetcher = RCAttributionFetcher(deviceCache: deviceCache,
identityManager: identityManager,
backend: backend)
attributionFetcher.postAttributionData(["something": "here"],
from: .adjust,
forNetworkUserId: userID)
expect(backend.invokedPostAttributionDataCount) == 1

attributionFetcher.postAttributionData(["something": "else"],
from: .adjust,
forNetworkUserId: userID)

expect(backend.invokedPostAttributionDataCount) == 1

}

func testPostAttributionDataDoesntSkipIfNetworkUserIdChanged() {
let userID = "userID"
let backend = MockBackend()
backend.stubbedPostAttributionDataCompletionResult = (nil, ())

attributionFetcher = RCAttributionFetcher(deviceCache: deviceCache,
identityManager: identityManager,
backend: backend)
attributionFetcher.postAttributionData(["something": "here"],
from: .adjust,
forNetworkUserId: userID)
expect(backend.invokedPostAttributionDataCount) == 1

attributionFetcher.postAttributionData(["something": "else"],
from: .facebook,
forNetworkUserId: userID)

expect(backend.invokedPostAttributionDataCount) == 2
}

func testPostAttributionDataDoesntSkipIfSameUserIdButDifferentNetwork() {
let backend = MockBackend()
backend.stubbedPostAttributionDataCompletionResult = (nil, ())

attributionFetcher = RCAttributionFetcher(deviceCache: deviceCache,
identityManager: identityManager,
backend: backend)
attributionFetcher.postAttributionData(["something": "here"],
from: .adjust,
forNetworkUserId: "attributionUser1")
expect(backend.invokedPostAttributionDataCount) == 1

attributionFetcher.postAttributionData(["something": "else"],
from: .facebook,
forNetworkUserId: "attributionUser2")

expect(backend.invokedPostAttributionDataCount) == 2
}
}
34 changes: 34 additions & 0 deletions PurchasesTests/Misc/NSDictionaryExtensionsTests.swift
@@ -0,0 +1,34 @@
//
// NSDictionaryExtensionsTests.swift
// PurchasesCoreSwiftTests
//
// Created by Andrés Boedo on 9/28/20.
// Copyright © 2020 Purchases. All rights reserved.
//

import XCTest
import Nimble

@testable import Purchases

class NSDictionaryExtensionsTests: XCTestCase {
func testRemovingNSNullValuesFiltersCorrectly() {
let testValues: NSDictionary = [
"instrument": "guitar",
"type": 1,
"volume": NSNull()
]
let expectedValues: NSDictionary = [
"instrument": "guitar",
"type": 1
]

expect(testValues.removingNSNullValues() as NSDictionary) == expectedValues
}

func testRemovingNSNullValuesReturnsEmptyIfOriginalIsEmpty() {
let testValues = NSDictionary()

expect(testValues.removingNSNullValues() as NSDictionary) == testValues
}
}
1 change: 1 addition & 0 deletions PurchasesTests/PurchasesTests-Bridging-Header.h
Expand Up @@ -42,3 +42,4 @@
#include <Purchases/RCISOPeriodFormatter.h>
#include <Purchases/RCProductInfo.h>
#include <Purchases/RCProductInfoExtractor.h>
#include <Purchases/NSDictionary+RCExtensions.h>

0 comments on commit 6238bfb

Please sign in to comment.