Merge pull request braintree#291 from braintree/refactor-data-collector

Refactor BTDataCollector and PPDataCollector

Braintree.podspec

@@ -60,6 +60,7 @@ Pod::Spec.new do |s|
     s.source_files = "BraintreeVenmo/**/*.{h,m}"
     s.public_header_files = "BraintreeVenmo/Public/*.h"
     s.dependency "Braintree/Core"
+    s.dependency "Braintree/PayPalDataCollector"
   end
 
   s.subspec "UI" do |s|

Braintree.xcodeproj/project.pbxproj

@@ -14,6 +14,9 @@
 		031B94D01C3485ED00F136C8 /* EXPMatchers+haveKerning.m in Sources */ = {isa = PBXBuildFile; fileRef = 031B94C31C34822800F136C8 /* EXPMatchers+haveKerning.m */; };
 		031B94D11C34863200F136C8 /* BTUISpec.m in Sources */ = {isa = PBXBuildFile; fileRef = 031B94C01C34822800F136C8 /* BTUISpec.m */; };
 		0325AC241C289B5F00E90999 /* BraintreeThreeDSecure_UITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0325AC231C289B5F00E90999 /* BraintreeThreeDSecure_UITests.swift */; };
+		032D3A721C580B09000E5F6B /* PPDataCollectorTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 032D3A711C580B09000E5F6B /* PPDataCollectorTest.m */; };
+		032D3A731C580B09000E5F6B /* PPDataCollectorTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 032D3A711C580B09000E5F6B /* PPDataCollectorTest.m */; };
+		032D3A741C580B09000E5F6B /* PPDataCollectorTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 032D3A711C580B09000E5F6B /* PPDataCollectorTest.m */; };
 		033800101C248B9C00CFF1D9 /* BraintreeDropIn_UITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0338000F1C248B9C00CFF1D9 /* BraintreeDropIn_UITests.swift */; };
 		033F3ABB1C3490F2005383BF /* BTUI_UIColor.m in Sources */ = {isa = PBXBuildFile; fileRef = 031B94BB1C34822800F136C8 /* BTUI_UIColor.m */; };
 		033F3ABC1C3490F3005383BF /* BTUI_UIColor.m in Sources */ = {isa = PBXBuildFile; fileRef = 031B94BB1C34822800F136C8 /* BTUI_UIColor.m */; };
@@ -1172,6 +1175,7 @@
 		031B94C21C34822800F136C8 /* EXPMatchers+haveKerning.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "EXPMatchers+haveKerning.h"; sourceTree = "<group>"; };
 		031B94C31C34822800F136C8 /* EXPMatchers+haveKerning.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "EXPMatchers+haveKerning.m"; sourceTree = "<group>"; };
 		0325AC231C289B5F00E90999 /* BraintreeThreeDSecure_UITests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BraintreeThreeDSecure_UITests.swift; sourceTree = "<group>"; };
+		032D3A711C580B09000E5F6B /* PPDataCollectorTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = PPDataCollectorTest.m; sourceTree = "<group>"; };
 		0338000F1C248B9C00CFF1D9 /* BraintreeDropIn_UITests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BraintreeDropIn_UITests.swift; sourceTree = "<group>"; };
 		035945551BE96345002116BB /* BTUIVenmoMonogramCardView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BTUIVenmoMonogramCardView.h; sourceTree = "<group>"; };
 		035945561BE96345002116BB /* BTUIVenmoMonogramCardView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = BTUIVenmoMonogramCardView.m; sourceTree = "<group>"; };
@@ -3039,6 +3043,7 @@
 		A7B462431C3C90DB00048423 /* PayPalOneTouch */ = {
 			isa = PBXGroup;
 			children = (
+				032D3A711C580B09000E5F6B /* PPDataCollectorTest.m */,
 				A5D3A56E1C20975F003A25A1 /* PPFPTIDataTest.m */,
 				A5D3A56F1C20975F003A25A1 /* PPFPTITrackerTest.m */,
 				A7B462441C3C911200048423 /* PPOTAppSwitchResponseTest.m */,
@@ -4427,6 +4432,7 @@
 				A79AF9F31B56DB54007B1DF1 /* BTClientMetadataSpec.m in Sources */,
 				A7C823081B4DA9D7009D45D6 /* BTHTTPSpec.m in Sources */,
 				A71F7DE51B616BA0005DA1B0 /* BTVenmoAppSwitchURLSpec.m in Sources */,
+				032D3A721C580B09000E5F6B /* PPDataCollectorTest.m in Sources */,
 				A7A094F61B8276E500D732CC /* BTCardNonce_Tests.swift in Sources */,
 				A7B7989C1C233C57001327FA /* BTConfiguration_Tests.swift in Sources */,
 				A7B4624D1C3C911200048423 /* PPOTAppSwitchUtilTest.m in Sources */,
@@ -4672,6 +4678,7 @@
 				41BC33E21BF6948C00161106 /* BTAPIClient_SwiftTests.swift in Sources */,
 				41BC33E31BF6948C00161106 /* BTURLUtils_Tests.swift in Sources */,
 				41BC33E41BF6948C00161106 /* BTClientMetadataSpec.m in Sources */,
+				032D3A731C580B09000E5F6B /* PPDataCollectorTest.m in Sources */,
 				41BC33E51BF6948C00161106 /* BTHTTPSpec.m in Sources */,
 				41BC33E71BF6948C00161106 /* BTCardNonce_Tests.swift in Sources */,
 				A7B4624E1C3C911200048423 /* PPOTAppSwitchUtilTest.m in Sources */,
@@ -4730,6 +4737,7 @@
 				A775AFE41C1608B30003A6E6 /* BTJSON_Tests.swift in Sources */,
 				A775AFE51C1608B30003A6E6 /* BTPayPalDriver_Tests.swift in Sources */,
 				A775AFE61C1608B30003A6E6 /* BTThreeDSecureDriver_Tests.swift in Sources */,
+				032D3A741C580B09000E5F6B /* PPDataCollectorTest.m in Sources */,
 				A775AFE71C1608B30003A6E6 /* BTPaymentMethodNonceParser_Tests.swift in Sources */,
 				A775AFE81C1608B30003A6E6 /* BTCardNonce_Tests.swift in Sources */,
 				A7B4624F1C3C911200048423 /* PPOTAppSwitchUtilTest.m in Sources */,

BraintreeDataCollector/BTDataCollector.m

@@ -38,15 +38,7 @@ - (void)setUpKountWithDebugOn:(BOOL)debugLogging {
 #pragma mark - Public methods
 
 + (NSString *)payPalClientMetadataId {
-    Class paypalClass = NSClassFromString(@"PPDataCollector");
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wundeclared-selector"
-    if (paypalClass && [paypalClass respondsToSelector:@selector(clientMetadataID)]) {
-        return [paypalClass performSelector:@selector(clientMetadataID)];
-    }
-#pragma clang diagnostic pop
-
-    return nil;
+    return [BTDataCollector generatePayPalClientMetadataId];
 }
 
 /// At this time, this method only collects data with Kount. However, it is possible that in the future,
@@ -78,7 +70,7 @@ - (NSString *)collectFraudDataForCard:(BOOL)includeCard forPayPal:(BOOL)includeP
         [self.kount collect:deviceSessionId];
     }
     if (includePayPal) {
-        NSString *payPalClientMetadataId = [BTDataCollector payPalClientMetadataId];
+        NSString *payPalClientMetadataId = [BTDataCollector generatePayPalClientMetadataId];
         if (payPalClientMetadataId) {
             dataDictionary[@"correlation_id"] = payPalClientMetadataId;
         }
@@ -111,6 +103,18 @@ - (void)setFraudMerchantId:(NSString *)fraudMerchantId {
 
 #pragma mark - Private methods
 
++ (NSString *)generatePayPalClientMetadataId {
+    Class paypalClass = NSClassFromString(@"PPDataCollector");
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wundeclared-selector"
+    if (paypalClass && [paypalClass respondsToSelector:@selector(clientMetadataID)]) {
+        return [paypalClass performSelector:@selector(clientMetadataID)];
+    }
+#pragma clang diagnostic pop
+
+    return nil;
+}
+
 /// Generates a new session ID
 - (NSString *)sessionId {
     return [[[NSUUID UUID] UUIDString] stringByReplacingOccurrencesOfString:@"-" withString:@""];

BraintreeDataCollector/Public/BTDataCollector.h

@@ -30,7 +30,7 @@ extern NSString * const BTDataCollectorKountErrorDomain;
 /// Generates a new PayPal fraud ID if PayPal is integrated; otherwise returns `nil`.
 ///
 /// @return A PayPal fraud ID
-+ (nullable NSString *)payPalClientMetadataId;
++ (nullable NSString *)payPalClientMetadataId DEPRECATED_MSG_ATTRIBUTE("Integrate PayPalDataCollector and use [PPDataCollector collectPayPalDeviceData] instead.");
 
 /// Collects device data for Kount.
 ///
@@ -49,11 +49,11 @@ extern NSString * const BTDataCollectorKountErrorDomain;
 
 /// Collects device data for PayPal.
 ///
-/// This should be used when the user is paying with PayPal only.
+/// This should be used when the user is paying with PayPal or Venmo only.
 ///
 /// @return a deviceData string that should be passed into server-side calls, such as `Transaction.sale`,
 ///         for PayPal transactions. This JSON serialized string contains a PayPal fraud ID.
-- (NSString *)collectPayPalClientMetadataId;
+- (NSString *)collectPayPalClientMetadataId DEPRECATED_MSG_ATTRIBUTE("Integrate PayPalDataCollector and use [PPDataCollector collectPayPalDeviceData] instead.");
 
 /// Collects device data using Kount and PayPal.
 ///

BraintreePayPal/PayPalDataCollector/PPDataCollector.m

@@ -47,4 +47,20 @@ + (nonnull NSString *)clientMetadataID {
     return [[PPDataCollector clientMetadataIDProvider] clientMetadataID:nil];
 }
 
++ (nonnull NSString *)collectPayPalDeviceData {
+    NSMutableDictionary *dataDictionary = [NSMutableDictionary new];
+    NSString *payPalClientMetadataId = [PPDataCollector clientMetadataID];
+    if (payPalClientMetadataId) {
+        dataDictionary[@"correlation_id"] = payPalClientMetadataId;
+    }
+
+    NSError *error;
+    NSData *data = [NSJSONSerialization dataWithJSONObject:dataDictionary options:0 error:&error];
+    if (!data) {
+        NSLog(@"ERROR: Failed to create deviceData string, error = %@", error);
+    }
+
+    return [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
+}
+
 @end

BraintreePayPal/PayPalDataCollector/Public/PPDataCollector.h

@@ -11,13 +11,27 @@
 
 /// Returns a client metadata ID.
 ///
+/// @warning This returns a raw client metadata ID, which is not the correct format for device data
+/// when creating a transaction. Instead, it is recommended to use `collectPayPalDeviceData`.
+///
 /// @param pairingID a pairing ID to associate with this clientMetadataID must be 10-32 chars long or null
 /// @return a client metadata ID to send as a header
 + (nonnull NSString *)clientMetadataID:(nullable NSString *)pairingID;
 
 /// Returns a client metadata ID.
 ///
+/// @warning This returns a raw client metadata ID, which is not the correct format for device data
+/// when creating a transaction. Instead, it is recommended to use `collectPayPalDeviceData`.
+///
 /// @return a client metadata ID to send as a header
 + (nonnull NSString *)clientMetadataID;
 
+/// Collects device data for PayPal.
+///
+/// This should be used when the user is paying with PayPal or Venmo only.
+///
+/// @return a deviceData string that should be passed into server-side calls, such as `Transaction.sale`,
+///         for PayPal transactions. This JSON serialized string contains a PayPal fraud ID.
++ (nonnull NSString *)collectPayPalDeviceData;
+
 @end

Demo/Features/Fraud Protection - BTDataCollector/BraintreeDemoBTDataCollectorViewController.m

@@ -71,8 +71,11 @@ - (IBAction)tappedCollectKount {
 }
 
 - (IBAction)tappedCollectDyson {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
     self.dataLabel.text = [BTDataCollector payPalClientMetadataId];
     self.progressBlock(@"Collected PayPal clientMetadataID!");
+#pragma clang diagnostic pop
 }
 
 - (IBAction)tappedRequestLocationAuthorization:(__unused id)sender {

IntegrationTests/BraintreeDataCollector_IntegrationTests.m

@@ -54,8 +54,6 @@ - (void)testCollectCardFraudData_returnsFraudDataWithNoPayPalFraudData {
     [self waitForExpectationsWithTimeout:10 handler:nil];
 }
 
-#pragma clang diagnostic pop
-
 - (void)testCollectPayPalClientMetadataId_returnsClientMetadataId {
     id delegate = OCMProtocolMock(@protocol(BTDataCollectorDelegate));
     self.data.delegate = delegate;
@@ -69,5 +67,6 @@ - (void)testCollectPayPalClientMetadataId_returnsClientMetadataId {
     [self waitForExpectationsWithTimeout:5 handler:nil];
 }
 
+#pragma clang diagnostic pop
 
 @end

Podfile.lock

@@ -94,7 +94,7 @@ DEPENDENCIES:
 
 EXTERNAL SOURCES:
   Braintree:
-    :path: "."
+    :path: .
 
 SPEC CHECKSUMS:
   AFNetworking: cb8d14a848e831097108418f5d49217339d4eb60

UnitTests/BTDataCollector_Tests.swift

@@ -54,25 +54,6 @@ class BTDataCollector_Tests: XCTestCase {
         XCTAssertEqual(testDelegate!.error!.userInfo[NSLocalizedFailureReasonErrorKey] as? String, nil)
     }
 
-    func testPayPalClientMetadataId() {
-        let clientMetadataId = BTDataCollector.payPalClientMetadataId()
-        // Ensure clientMetadataId is not nil and has a length of at least 12.
-        // This is just a guess of a reasonable id length. In practice, the id
-        // should have a length of 32.
-        XCTAssert(clientMetadataId!.characters.count >= 12)
-    }
-
-    func testCollectPayPalClientMetadataId() {
-        let dataCollector = BTDataCollector(environment: .Sandbox)
-        let deviceData = dataCollector.collectPayPalClientMetadataId()
-        let data = deviceData.dataUsingEncoding(NSUTF8StringEncoding)
-        let dictionary = try! NSJSONSerialization.JSONObjectWithData(data!, options: NSJSONReadingOptions.MutableContainers) as! Dictionary<String, AnyObject>
-        // Ensure correlation_id (clientMetadataId) is not nil and has a length of at least 12.
-        // This is just a guess of a reasonable id length. In practice, the id
-        // typically has a length of 32.
-        XCTAssert((dictionary["correlation_id"] as! String).characters.count >= 32)
-    }
-
     /// Ensure that both Kount and PayPal data can be collected together
     func testCollectFraudData() {
         let dataCollector = BTDataCollector(environment: .Sandbox)

UnitTests/PayPalOneTouch/PPDataCollectorTest.m

@@ -0,0 +1,31 @@
+//
+//  PPDataCollectorTest.m
+//  PayPalOneTouch
+//
+//  Copyright © 2015 PayPal, Inc. All rights reserved.
+//
+
+#import <XCTest/XCTest.h>
+#import "PayPalDataCollector.h"
+
+@interface PPDataCollectorTest : XCTestCase
+
+@end
+
+@implementation PPDataCollectorTest
+
+- (void)testDeviceDataContainsCorrelationIdKey {
+    NSString *deviceData = [PPDataCollector collectPayPalDeviceData];
+    NSData *data = [deviceData dataUsingEncoding:NSUTF8StringEncoding];
+    NSDictionary *dictionary = [NSJSONSerialization JSONObjectWithData:data options:NSJSONReadingMutableContainers error:NULL];
+    NSString *cmid = [dictionary objectForKey:@"correlation_id"];
+    XCTAssert(cmid.length >= 32);
+}
+
+- (void)testClientMetadataDoesNotContainCorrelationIdKey {
+    NSString *cmid = [PPDataCollector clientMetadataID];
+    XCTAssertTrue([cmid rangeOfString:@"correlation_id"].location == NSNotFound);
+    XCTAssert(cmid.length >= 12);
+}
+
+@end