Configuration Json provider: adds empty objects to configuration

[alefranz]

Currently empty objects loaded via ConfigurationBuilder.AddJsonFile() are ignored.

This adds empty objects to the configuration, so that when you enumerate the children on the parent, the key will be listed.

This element is present in configuration as a null value without children.

Please note that the extension method Exists() will return false for this element, as "exists" is a misnomer. The actual semantic of that extension method is "not empty", as per description it is checking that the element has at least a value or children.
I believe that, given the actual semantic of Exists, it is correct for it to be false in this scenario.

Fixes: #40218

[ghost]

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

[alefranz]

I'm not convinced this is intentional, I've raised #40827

[maryamariyan]

LGTM thanks @alefranz.

[maryamariyan]

cc: @halter73 @HaoK as you were in the issue discussion and also have more background on this area.

[halter73]

We should probably return "" instead of null for empty objects for consistency.

[alefranz]

Thank you!

[ChadNedzlek]

This just broke our service rollout today, because a whole bunch of nulls started flowing through our configuration layers that didn't use to exist.

If anyone else ends up here because of a bunch of NullReferenceExceptions in config code when updating from 5.0.0 => 6.0.0 of Microsoft.Extensions.Configuration.Json, the answer is just to remove any empty objects from all your config files.

[ericstj]

I had marked this as "breaking-change" before realizing it was 2 years old so adding a breaking change notice for 5.0 is not really going to help much.

In the future we should be careful about changing the nullable behavior of our APIs. I would hope that such a change would appear as a change to the nullable annotations in the API which are now present as of 7.0 preview4, and thus it would be a signal to folks recompiling to handle null. I would still consider that "breaking" in the sense that we should document it, but allow it.

I still think we could do more here. I was wondering if a debug visualizer on the configuration section might have been a signal to you debugging if you saw that the element had a null value. I wasn't sure of the proximity between your usage of the Configuration API and the API which threw the NRE and if a debug visualizer might help.

[ChadNedzlek]

We only hit it because 3.1 is coming to the end of its life soon, so we had to migrate. I assume others will be in this boat relatively soon as well, so documentation might still be helpful. I don't know if we have any good numbers on how many people are still on 3.1.

I don't think the config section would have existed yet in the code that crashed, since it was in the middle of the configuration pipeline. It crashed inside "Load", which caught the ArgumentNullException and turned it into a InvalidDataException, so tracking it down took a fair bit of time.

This behavior is unexpected, so I wouldn't have even though to look at a visualizer. The idea that MyKey:{} would have resulted in a null string never crossed my mind. Instead we were digging into de-compiled source code and trying to set breakpoints there and binary searching the config file by deleting parts of it to try to figure out where the exception was coming from. It was... really unpleasant.