feat: Add Bugsnag.getMetadata(_ section:)

Also amend lower-level equivalent functionality to match (i.e. return nil on non-existence)
Added a OOM metadata test

CHANGELOG.md

@@ -33,6 +33,13 @@ Bugsnag Notifiers on other platforms.
      (Swift: `Bugsnag.clearMetadata(_ section)`)
      [#457](https://github.com/bugsnag/bugsnag-cocoa/pull/457)
 
+* Added `Bugsnag.getMetadata(_ section)`.  The behaviour is: calling with a valid section
+  name will return the metadata for that section if it exists, or `nil` if it does not exist.  Other,
+  similar functionality (e.g. `BugsnagConfiguration.getTab()` has been renamed and
+  had usage aligned with this change.
+  [#459](https://github.com/bugsnag/bugsnag-cocoa/pull/459)
+
+
 ## Bug fixes
 
 * Fix possible report corruption when using `notify()` from multiple threads

Source/Bugsnag.h

@@ -266,6 +266,16 @@ static NSString *_Nonnull const BugsnagSeverityInfo = @"info";
  */
 + (BOOL)resumeSession;
 
+/**
+* Return the metadata for a specific named section
+*
+* @param section The name of the section
+* @returns The mutable dictionary representing the metaadata section, if it
+*          exists, or nil if not.
+*/
++ (NSMutableDictionary *_Nullable)getMetadata:(NSString *_Nonnull)section
+    NS_SWIFT_NAME(getMetadata(_:));
+
 /**
  * Set the maximum number of breadcrumbs to keep and sent to Bugsnag.
  * By default, we'll keep and send the 20 most recent breadcrumb log

Source/Bugsnag.m

@@ -304,6 +304,10 @@ + (void)setWriteBinaryImagesForUserReported:
     }
 }
 
++ (NSMutableDictionary *)getMetadata:(NSString *)section {
+    return [[[self configuration] metadata] getMetadata:section];
+}
+
 @end
 
 //

Source/BugsnagMetadata.h

@@ -32,7 +32,15 @@
 
 - (instancetype _Nonnull)initWithDictionary:(NSMutableDictionary *_Nonnull)dict;
 
-- (NSMutableDictionary *_Nonnull)getTab:(NSString *_Nonnull)tabName;
+/**
+ * Get a named metadata section
+ *
+ * @param sectionName The name of the section
+ * @returns The mutable dictionary representing the metadata section, if it
+ *          exists, or nil if not.
+ */
+- (NSMutableDictionary *_Nullable)getMetadata:(NSString *_Nonnull)sectionName
+    NS_SWIFT_NAME(getMetadata(_:));
 
 - (void)clearMetadataInSection:(NSString *_Nonnull)section
     NS_SWIFT_NAME(clearMetadata(_:));

Source/BugsnagMetadata.m

@@ -54,14 +54,9 @@ - (id)mutableCopyWithZone:(NSZone *)zone {
     }
 }
 
-- (NSMutableDictionary *)getTab:(NSString *)tabName {
+- (NSMutableDictionary *)getMetadata:(NSString *)sectionName {
     @synchronized(self) {
-        NSMutableDictionary *tab = self.dictionary[tabName];
-        if (!tab) {
-            tab = [NSMutableDictionary dictionary];
-            self.dictionary[tabName] = tab;
-        }
-        return tab;
+        return self.dictionary[sectionName];
     }
 }
 
@@ -86,15 +81,22 @@ - (void)addAttribute:(NSString *)attributeName
         if (value) {
             id cleanedValue = BSGSanitizeObject(value);
             if (cleanedValue) {
-                [self getTab:tabName][attributeName] = cleanedValue;
-            } else {
+                NSDictionary *section = [self getMetadata:tabName];
+                if (!section) {
+                    [[self dictionary] setObject:[NSMutableDictionary new] forKey:tabName];
+                    section = [self getMetadata:tabName];
+                }
+                [section setValue:cleanedValue forKey:attributeName];
+            }
+            else {
                 Class klass = [value class];
                 bsg_log_err(@"Failed to add metadata: Value of class %@ is not "
                             @"JSON serializable",
                             klass);
             }
-        } else {
-            [[self getTab:tabName] removeObjectForKey:attributeName];
+        }
+        else {
+            [[self getMetadata:tabName] removeObjectForKey:attributeName];
         }
     }
     [self.delegate metadataChanged:self];

Source/BugsnagNotifier.m

@@ -725,7 +725,7 @@ - (void)metadataChanged:(BugsnagMetadata *)metadata {
                 [self.metadataLock unlock];
             }
         } else if (metadata == self.configuration.config) {
-            BSSerializeJSONDictionary([metadata getTab:BSGKeyConfig],
+            BSSerializeJSONDictionary([metadata getMetadata:BSGKeyConfig],
                                       &bsg_g_bugsnag_data.configJSON);
         } else if (metadata == self.state) {
             BSSerializeJSONDictionary([metadata toDictionary],

Tests/BugsnagEventTests.m

@@ -376,6 +376,44 @@ - (void)testHandledReportMetaData {
     XCTAssertEqualObjects(report.metadata[@"Custom"][@"Foo"], @"Bar");
 }
 
+/**
+ * Test report metadata handling in OOM situations
+ */
+- (void)testHandledReportMetaDataOOM {
+    BugsnagHandledState *state = [BugsnagHandledState handledStateWithSeverityReason:UnhandledException];
+    BugsnagMetadata *metadata = [BugsnagMetadata new];
+    [metadata addAttribute:@"Foo" withValue:@"Bar" toTabWithName:@"Custom"];
+    NSDictionary *dict = @{
+        @"user.state.didOOM" : @YES,
+        @"user.handledState": [state toJson],
+        @"user.metaData": [metadata toDictionary]
+    };
+
+    BugsnagEvent *report1 = [[BugsnagEvent alloc] initWithKSReport:dict];
+    XCTAssertNotNil(report1.metadata);
+    XCTAssertEqual(report1.metadata.count, 0);
+
+    // OOM metadata is set from the session user data.
+    [metadata addAttribute:@"id" withValue:@"OOMuser" toTabWithName:@"user"];
+    [metadata addAttribute:@"email" withValue:@"OOMemail" toTabWithName:@"user"];
+    [metadata addAttribute:@"name" withValue:@"OOMname" toTabWithName:@"user"];
+
+    // Try it again with more fully formed session data
+    dict = @{
+        @"user.state.didOOM" : @YES,
+        @"user.handledState": [state toJson],
+        @"user.state.oom.session" : [metadata toDictionary]
+    };
+
+    BugsnagEvent *report2 = [[BugsnagEvent alloc] initWithKSReport:dict];
+
+    XCTAssertNotNil(report2.metadata);
+    XCTAssertEqual(report2.metadata.count, 1);
+    XCTAssertEqual(report2.metadata[@"user"][@"id"], @"OOMuser");
+    XCTAssertEqual(report2.metadata[@"user"][@"name"], @"OOMname");
+    XCTAssertEqual(report2.metadata[@"user"][@"email"], @"OOMemail");
+}
+
 - (void)testUnhandledReportMetaData {
     BugsnagMetadata *metadata = [BugsnagMetadata new];
     [metadata addAttribute:@"Foo" withValue:@"Bar" toTabWithName:@"Custom"];

Tests/BugsnagTests.m

@@ -61,4 +61,20 @@ - (void)testBugsnagMetadataAddition {
     }];
 }
 
+/**
+ * Test that the global Bugsnag metadata retrieval performs as expected:
+ * return a section when there is one, or nil otherwise.
+ */
+- (void)testGetMetadata {
+    NSError *error;
+    BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1 error:&error];
+    [Bugsnag startBugsnagWithConfiguration:configuration];
+    XCTAssertNil([Bugsnag getMetadata:@"dummySection"]);
+    [Bugsnag addMetadataToSection:@"dummySection" key:@"aKey1" value:@"aValue1"];
+    NSMutableDictionary *section = [Bugsnag getMetadata:@"dummySection"];
+    XCTAssertNotNil(section);
+    XCTAssertEqual(section[@"aKey1"], @"aValue1");
+    XCTAssertNil([Bugsnag getMetadata:@"anotherSection"]);
+}
+
 @end

UPGRADING.md

@@ -50,13 +50,17 @@ ObjC:
 - [Bugsnag clearTabWithName:]
 + [Bugsnag clearMetadataInSection:]
 
++ [Bugsnag getSection:]
+
 Swift:
 
 - Bugsnag.addAttribute(attributeName:withValue:toTabWithName:)
 + Bugsnag.addMetadata(_:key:value:)
 
 - Bugsnag.clearTab(name:)
 + Bugsnag.clearMetadata(_ section)
+
++ Bugsnag.getSection(_ section)
 ```
 
 ### `BugsnagMetadata` class
@@ -68,9 +72,17 @@ ObjC:
 - [BugsnagMetadata clearTabWithName:]
 + [BugsnagMetadata clearMetadataInSection:]
 
+- [BugsnagMetadata getTab:]
++ [BugsnagMetadata getSection:]
+
 Swift:
 
 - BugsnagMetadata.clearTab(name:)
 + BugsnagMetadata.clearMetadata(_ section)
 
+- BugsnagMetadata.getTab(name:)
++ BugsnagMetadata.getSection(_ section)
 ```
+
+Note that `BugsnagMetadata.getTab()` previously would create a metadata section if it
+did not exist; the new behaviour is to return `nil`. 

iOS/Bugsnag.xcodeproj/project.pbxproj

@@ -13,6 +13,7 @@
 		000E6EA423D8AC8C009D8194 /* README.md in Resources */ = {isa = PBXBuildFile; fileRef = 000E6EA023D8AC8C009D8194 /* README.md */; };
 		000E6EA523D8AC8C009D8194 /* VERSION in Resources */ = {isa = PBXBuildFile; fileRef = 000E6EA123D8AC8C009D8194 /* VERSION */; };
 		000E6EA623D8AC8C009D8194 /* CHANGELOG.md in Resources */ = {isa = PBXBuildFile; fileRef = 000E6EA223D8AC8C009D8194 /* CHANGELOG.md */; };
+		004753D423F1A4E2009884EB /* BugsnagMetadataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 004753D323F1A4E2009884EB /* BugsnagMetadataTests.swift */; };
 		00D7ACAD23E9C63000FBE4A7 /* BugsnagTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 00D7ACAC23E9C63000FBE4A7 /* BugsnagTests.m */; };
 		00D7ACAF23EABBC800FBE4A7 /* BugsnagSwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 00D7ACAE23EABBC800FBE4A7 /* BugsnagSwiftTests.swift */; };
 		4B47970A22A9AE1F00FF9C2E /* BugsnagEventFromKSCrashReportTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 4B47970922A9AE1F00FF9C2E /* BugsnagEventFromKSCrashReportTest.m */; };
@@ -413,6 +414,7 @@
 		000E6EA023D8AC8C009D8194 /* README.md */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = net.daringfireball.markdown; name = README.md; path = ../README.md; sourceTree = "<group>"; };
 		000E6EA123D8AC8C009D8194 /* VERSION */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; name = VERSION; path = ../VERSION; sourceTree = "<group>"; };
 		000E6EA223D8AC8C009D8194 /* CHANGELOG.md */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = net.daringfireball.markdown; name = CHANGELOG.md; path = ../CHANGELOG.md; sourceTree = "<group>"; };
+		004753D323F1A4E2009884EB /* BugsnagMetadataTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BugsnagMetadataTests.swift; sourceTree = "<group>"; };
 		00D7ACAC23E9C63000FBE4A7 /* BugsnagTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = BugsnagTests.m; path = ../../Tests/BugsnagTests.m; sourceTree = "<group>"; };
 		00D7ACAE23EABBC800FBE4A7 /* BugsnagSwiftTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BugsnagSwiftTests.swift; sourceTree = "<group>"; };
 		4B3B193422CA7B0900475354 /* BugsnagCollectionsBSGDictSetSafeObjectTest.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = BugsnagCollectionsBSGDictSetSafeObjectTest.m; path = ../../Tests/BugsnagCollectionsBSGDictSetSafeObjectTest.m; sourceTree = "<group>"; };
@@ -642,6 +644,7 @@
 			children = (
 				000E6E9D23D8690F009D8194 /* BugsnagSwiftConfigurationTests.swift */,
 				00D7ACAE23EABBC800FBE4A7 /* BugsnagSwiftTests.swift */,
+				004753D323F1A4E2009884EB /* BugsnagMetadataTests.swift */,
 			);
 			path = "Swift Tests";
 			sourceTree = "<group>";
@@ -1268,6 +1271,7 @@
 				F4295995C3259BF7D9730BC4 /* BugsnagKSCrashSysInfoParserTest.m in Sources */,
 				E70E52152216E41C00A590AB /* BugsnagSessionTrackerStopTest.m in Sources */,
 				F4295F017754324FD52CCE46 /* RegisterErrorDataTest.m in Sources */,
+				004753D423F1A4E2009884EB /* BugsnagMetadataTests.swift in Sources */,
 				00D7ACAF23EABBC800FBE4A7 /* BugsnagSwiftTests.swift in Sources */,
 				F42952D83435C02F8D891C40 /* BugsnagThreadTest.m in Sources */,
 				4B47970A22A9AE1F00FF9C2E /* BugsnagEventFromKSCrashReportTest.m in Sources */,

iOS/BugsnagTests/Swift Tests/BugsnagMetadataTests.swift

@@ -0,0 +1,23 @@
+//
+//  BugsnagMetadataTests.swift
+//  Tests
+//
+//  Created by Robin Macharg on 10/02/2020.
+//  Copyright © 2020 Bugsnag. All rights reserved.
+//
+
+import XCTest
+
+class BugsnagMetadataTests: XCTestCase {
+
+    /**
+     * Test that getMetadata() is exposed to Swift correctly
+     */
+    func testgetMetadataIsExposedCorrectly() {
+        let metadata = BugsnagMetadata(dictionary:NSMutableDictionary())
+        XCTAssertNil(metadata.getMetadata("dummySection"))
+
+        metadata.addAttribute("aName", withValue: "aValue", toTabWithName: "dummySection")
+        XCTAssertNotNil(metadata.getMetadata("dummySection"))
+    }
+}

iOS/BugsnagTests/Swift Tests/BugsnagSwiftTests.swift

@@ -24,13 +24,13 @@ class BugsnagSwiftTests: XCTestCase {
 
                 Bugsnag.notify(exception1) { (event) in
                     // Arbitrary test, replicating the ObjC one
-                    let value = (event.metadata["mySection1"] as! [String : Any])["myKey1"] as! String
+                    let value = (event.metadata["mySection1"] as? [String : Any])?["myKey1"] as? String
                     XCTAssertEqual(value, "myValue1")
                 }
             }
         }
         catch let e as NSError {
-            print(e)
+            XCTFail("Failed to check method Swift exposure: \(e.debugDescription)")
         }
     }