FIX: MakeEscapedJsonString now null-checks inputs #2018
+47
−7
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Most device decriptor fields are null-checked in ComparePropertyToDeviceDescriptor, however the addition of MakeEscapedJsonString meant that 'capabilities' was not being checked anymore, and so could throw a null-ref exception if a device didn't fill this field.
Changes made
This change adds a null/empty string check to MakeEscapedJsonString to mirror the old behaviour.
Additionally this'll also now mean any devices with an empty non-null string can skip a minor StringBuilder allocation.
Testing
Local testing with gamepads to see that the resulting comparisons in ComparePropertyToDeviceDescriptor still work correctly. This was tested between null, empty, and valid json string inputs.
A new unit test has been added too to exercise the null-input path.
Unit tests were also ran.
Risk
Minor difference in construction of a default JsonString (with a non-null text field).
Checklist
Before review:
Changed
,Fixed
,Added
sections.Area_CanDoX
,Area_CanDoX_EvenIfYIsTheCase
,Area_WhenIDoX_AndYHappens_ThisIsTheResult
.During merge:
NEW: ___
.FIX: ___
.DOCS: ___
.CHANGE: ___
.RELEASE: 1.1.0-preview.3
.After merge: