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.

Sign up for GitHub

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

Jump to bottom

Prevent incorrect merge of nested maps in metadata #936

Merged
merged 1 commit into from
Sep 17, 2020
Merged

Prevent incorrect merge of nested maps in metadata #936

merged 1 commit into from
Sep 17, 2020

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Sep 17, 2020 •
edited
Loading

Goal

Prevents the incorrect merging of nested maps in the Metadata class. This manifested as the same key-value pair being added to the incorrect location multiple times.

Changeset

The Metadata class now only attempts to merge values if they are both metadata maps. Additionally mergeMaps now takes the correct first parameter - previously the entire section was passed rather than the existing map value.

Testing

  • Converted existing unit tests for Metadata merging to Kotlin and consolidated in one location
  • Added new unit test cases which cover observed behaviour of incorrect nested map merging
  • Altered the expected JSON fixture for the MetadataSerializationTest which was incorrectly asserting the previous behaviour

@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Sep 17, 2020 •
edited
Loading

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1437.77 1358.79
arm64_v8a 369.25 287.33
armeabi 348.77 266.84
armeabi_v7a 328.29 250.47
x86 406.1 324.18
x86_64 389.72 307.8

Generated by 🚫 Danger

@fractalwrench fractalwrench marked this pull request as ready for review September 17, 2020 13:08
@fractalwrench fractalwrench merged commit 9152262 into next Sep 17, 2020
@fractalwrench fractalwrench deleted the PLAT-4999/metadata-merge-fix branch September 17, 2020 16:01
This was referenced Oct 1, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomlongridge tomlongridge tomlongridge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fractalwrench @bugsnagbot @tomlongridge