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

Empty array in config json file results in null class property #1341

Closed
stweedie opened this issue Apr 2, 2019 · 10 comments
Closed

Empty array in config json file results in null class property #1341

stweedie opened this issue Apr 2, 2019 · 10 comments
Milestone

Comments

@stweedie
Copy link

stweedie commented Apr 2, 2019

Describe the bug

Empty array in config json file results in null class property

To Reproduce

appsettings.json

{
  "nullValue": null,
  "anEmptyArray": [],
  "ridiculousWorkaround": ["this string has to not be empty"]
}

Startup.cs (ConfigureServices method)

var nullValue = Configuration.GetSection("nullValue").Get<IEnumerable<string>>();
var values = Configuration.GetSection("anEmptyArray").Get<IEnumerable<string>>();
var workaround = Configuration.GetSection("ridiculousWorkaround").Get<IEnumerable<string>>();

// nullValue is null
// values is null
// workaround is List<string> length 1

Expected behavior

in the above example, 'nullValue' would be null, 'values' should be an empty enumerable, and 'workaround' should be an enumerable with 1 value.

Additional context

There was an issue filed about this that was closed, but the issue was not fixed.
aspnet/Configuration#788

@Eilon Eilon transferred this issue from dotnet/aspnetcore Apr 2, 2019
@ajcvickers ajcvickers added this to the Backlog milestone Apr 18, 2019
@andre-ss6
Copy link

At least it should work with empty arrays. From what I read on the original issue, the problem is that you check for a key and if it doesn't exist, null is returned, thus you don't know when it's really null or when it simply doesn't exist.

However, I don't understand why empty arrays suffer from that problem. [] is not the same as null. If your code is treating those two as the same, I think that's a bug.

@analogrelay
Copy link

This is expected behavior, though perhaps a little counter-intuitive with JSON. The important thing to remember is that the config system is a flat collection key-value pairs. Any hierarchy is a convention-based model. The JSON provider emits key-value pairs representing the hierarchy and the binder recreates hierarchical objects from it.

So, given a slightly modified version of the JSON you provided:

{
  "nullValue": null,
  "anEmptyArray": [],
  "ridiculousWorkaround": ["foo", "bar"]
}

Arrays are serialized by taking each item in the array and adding an index to the prefix used by the key. Null values and empty arrays don't emit any key-value pairs by design. So, the resulting key-value pairs are:

ridiculousWorkaround:0 = "foo"
ridiculousWorkaround:1 = "bar"

Thus, when binding anEmptyArray, it is treated as the same as a null value. It's important to remember here that the config system is not a full-fidelity serialization model. It's a key-value pair system with multiple possible sources (JSON, XML, Environment Variables, etc.).

Closing as this is by-design behavior.

@andre-ss6
Copy link

This is unfortunate :/ That sounds more like an explanation of the limitations of how the code works currently than of a design intention. But well 🤷‍♂

@stweedie
Copy link
Author

stweedie commented Mar 12, 2020

I'm not sure I agree that this should be marked as 'resolved'. Maybe it should be converted to a feature request then. The motivating use case is to allow me to treat an empty array as an empty enumerable, not as null. I understand that this is by design.

Can you at least elaborate further on why empty arrays don't emit any key value pairs? Something has to be reading that variable and determining that it is an array. This seems like a bad case of 'parent with 0 children == no parent'.

@analogrelay
Copy link

Something has to be reading that variable and determining that it is an array.

Correct, the JSON config provider is reading this. The convention the JSON config provider applies is to use the current path as a prefix, then emit [prefix]:[index] items for each item in the array. In the config model, arrays do not exist, they are constructed from a convention over key names.

Can you at least elaborate further on why empty arrays don't emit any key value pairs?

What kind of value would you expect to be emitted here? Similar to arrays, object properties don't emit any pairs directly either. The JSON config provider only actually emits scalars, in order to match the behavior of all the other providers (Environment Variable and Command Line come to mind). For example, given the following JSON:

{
	"grandparent": {
		"parent": [
			{
				"child": "value"
			}
		]
	}
}

Only a single pair is emitted by this:

grandparent:parent:0:child=value

The grandparent, grandparent:parent and grandparent:parent:0 keys don't exist, they only exist as prefixes (you'll notice that Get("grandparent") will return no value).

The main reason the config system works like this is that it is built on common functionality across the various providers (as most abstractions do ;)). It's not designed to fully serialize a JSON document, rather the JSON is a serialization of the config data model, which doesn't provide a way to distinguish from missing values and values which are solely prefixes.

Maybe it should be converted to a feature request then.

It certainly would be a feature request to change this data model, and if you'd like to file it, feel free to do so. To be honest, I don't expect a change this deep in the data model to be something we'd prioritize, but it's a valid suggestion. The important aspect here would be a way to describe arrays in the key-value pairs model in a non-breaking way that can be expressed by the other config providers.

@stweedie
Copy link
Author

I think what's happening here is that you're speaking from the case of what exists in the code as it stands now, and I'm saying that the code and design as it stands now SHOULD distinguish between a missing key and a key that points to an empty array / object / dictionary, whatever. The problem is that the design does not allow for an empty array.

What kind of value would you expect to be emitted here?

Whatever kind of value indicates that the specified value does exist, even if it has no children. For instance, by this logic, why is an empty string distinguished from a null string or even a missing key?

{
  "SomeString": "",
  "NullString": null
}
public class ConfigObject
{
  public string SomeString { get; set; }
  public string NullString { get; set; }
  public string MissingString { get; set; }
}

...which doesn't provide a way to distinguish from missing values and values which are solely prefixes.

An empty array, object, or dictionary is NOT a 'prefix'. It is a value unto itself. Again, a 'key' with 0 children should be treated differently to a key that does not exist.

But, this design has its shortcomings (other than incorrectly binding an empty array / object).

Consider a use case (which is not too far from why I originally raised this issue in the first place), where I want to control the scopes for an api client. I want this client to have full access in every environment except for production:
appsettings.json

{
  "Scopes": [ "Read", "Write", "Update", "Delete" ]
}

appsettings.Production.json

{
  "Scopes": [ "Read" ]
}

What would the result be? What would any reasonable developer that doesn't know about this issue or design expect the result to be? More importantly, how can I accomplish my use case?

@analogrelay
Copy link

I think what's happening here is that you're speaking from the case of what exists in the code as it stands now, and I'm saying that the code and design as it stands now SHOULD distinguish between a missing key and a key that points to an empty array / object / dictionary, whatever.

I don't disagree with the premise, and yes I am clarifying the existing behavior. The config data model, as currently defined, does not support this kind of distinction. Changing that behavior is always an option, but a costly one that impacts the full ecosystem of config providers. That's not an excuse to not make changes, but it is a cost to consider when making changes.

What would the result be?

Yep, this is also a limitation in the current design that has been frequently brought up.

I'm not disagreeing that the config model has limitations, or that they are worth discussing. What I am saying is that they go beyond this one issue, are costly to fix, and as of yet have not been something we could prioritize changing. We have to make decisions based on the constraints and other components we have to maintain.

How about this, I'll happily open an issue to detail some of the limitations of the config model in general and point out issues like this (and the list merging one you mentioned). We can keep it around on our backlog until it is something we are able to prioritize. I just want to make sure our current priorities are clear.

@analogrelay
Copy link

It would also be useful to see a concrete example of a configuration scenario that needs to depend on the difference between an empty object/array and a null or missing value entirely. I'm sure those exist, but it would clarify things to see something beyond just constructed scenarios.

@analogrelay
Copy link

Filed #3083

@stweedie
Copy link
Author

Thanks for the information

@ghost ghost locked as resolved and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants