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

Convert event.device from NSDictionary to a structured class #526

Merged
merged 2 commits into from
Apr 8, 2020

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Apr 3, 2020

Goal

Converts event.device from an NSDictionary to a structured class where each field is represented as a property.

BugsnagDevice is used for sessions, and contains stateless properties that do not change over the lifecycle of an application. BugsnagDeviceWithState extends this class and is used for events, and contains stateful properties that change for each error - e.g. freeDisk.

Changeset

  • Added BugsnagDevice and BugsnagDeviceWithState classes with properties for each field in the spec
  • Altered BugsnagEvent to use readonly BugsnagDeviceWithState for event.device
  • Refactored BSGParseDevice and BSGParseDeviceWithState methods into new BugsnagDevice classes, so that they can parse a KSCrash report and populate their fields
  • Extracted JSON serialization into BugsnagDevice/WithState from BugsnagEvent
  • Moved surplus device metadata fields into event.metadata

Tests

  • Added unit test to verify that BugsnagDevice and BugsnagDeviceWithState populate correctly, and that they serialize JSON correctly
  • Updated existing test coverage
  • Ran example app and verified that device data is sent to the dashboard for an event

@fractalwrench fractalwrench changed the title feat: add structured class for event.device Convert event.device from NSDictionary to a structured class Apr 3, 2020
@fractalwrench fractalwrench force-pushed the v6-structured-device branch 4 times, most recently from 197449a to 8e72aea Compare April 6, 2020 13:27
@fractalwrench fractalwrench marked this pull request as ready for review April 6, 2020 15:02
@fractalwrench fractalwrench force-pushed the v6-structured-device branch from 8e72aea to bd246e3 Compare April 7, 2020 09:50
Source/BugsnagDevice.m Outdated Show resolved Hide resolved
Copy link
Contributor

@robinmacharg robinmacharg left a comment

Choose a reason for hiding this comment

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

Looks good in the main. A couple of simplifying refactorings that seem to make sense in the context of the move to structured device info have been suggested; probably worth scanning all the comments once over. One outstanding question I have is over what looks like the removal of the wordSize field from session device payloads. Understanding that decision would be useful.

Copy link
Contributor

@robinmacharg robinmacharg left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, looks good!

@fractalwrench fractalwrench merged commit 504f572 into v6 Apr 8, 2020
@fractalwrench fractalwrench deleted the v6-structured-device branch April 8, 2020 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants