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

Feature request: automatically add nested defaults #178

Open
theahura opened this issue Aug 18, 2023 · 8 comments
Open

Feature request: automatically add nested defaults #178

theahura opened this issue Aug 18, 2023 · 8 comments

Comments

@theahura
Copy link

If I have a jsonschema like:

schema = {
    "type": "object",
    "properties": {
        "foo": {
            "type": "object",
            "properties": {
                "bar": {
                    "type": "string",
                    "default": "bar"
                }
            }
        },
    },
    "required": [],
    "additionalProperties": False
}

the output of validation will be

{}

instead of

{
  foo: {
    bar: "bar"
  }
}

is there a way to make it so that nested defaults are automatically populated?

@horejsek
Copy link
Owner

horejsek commented Sep 3, 2023

Not automatically, but you can always define default on the foo object as well, that is:

schema = {
    "type": "object",
    "properties": {
        "foo": {
            "type": "object",
            "default": {"bar": "bar"},
            "properties": {
                "bar": {
                    "type": "string",
                    "default": "bar"
                }
            }
        },
    },
    "required": [],
    "additionalProperties": False
}

Doing this automatically might be quite expensive call, and also surprising. Sometimes you might not want the default to be set automatically for foo, but once foo is defined, you want to just ensure it contains required properties.

Would this suggestion work for you?

If not, I might have an idea to provide some special value (fastjsonschema.NESTED_DEFAULT, for example), that could indicate that an algorithm should make an object and fill child defaults too. But it deviates from the specification, so would rather avoid it unless necessary.

@theahura
Copy link
Author

theahura commented Sep 6, 2023

I ended up doing what you suggested, but I don't love it as a solution. I think it violates DRY -- if we update the underlying schema for bar, we also need to update foo, and this may be easy to forget or mess up for more complicated schema types. I'm concerned that these sorts of errors are also uniquely tricky to track down, since defaults are rarely the first thing a dev may think to check

That said, I agree that the spec is not fleshed out here -- anything related to default behavior feels surprising and unexpected for some set of usecases

@horejsek
Copy link
Owner

horejsek commented Oct 1, 2023

I agree that it can violite DRY. But at the same time it is more explicit I would say.

What would you say on my suggestion above? It could look like this:

schema = {
    "type": "object",
    "properties": {
        "foo": {
            "type": "object",
            "default": {"bar": fastjsonschema.NESTED_DEFAULT},
            "properties": {
                "bar": {
                    "type": "string",
                    "default": "bar"
                }
            }
        },
    },
    "required": [],
    "additionalProperties": False
}

@eike-fokken
Copy link

While it would be very expensive, maybe it would be nice, if fastjsonschema provides an additional function add_defaults_recursively (as you can see I am just GREAT at naming things...), that recursively adds defaults but must be called explicitly by the user so they know what they are signing up for.

In case you, @horejsek are interested, I could cook up a PR, but probably not before next week.

@eike-fokken
Copy link

eike-fokken commented Mar 4, 2024

Also: I just realized that if I have an array of objects and the object contains properties with defaults, these ARE added.

Is the general rule the following?

EDIT: Oh, obviously not as the thread topic shows. But apparently in arrays nested defaults are set.

"The topmost layer of defaults is added"?

@horejsek
Copy link
Owner

@eike-fokken To be honest, I'm not following what you mean?

@eike-fokken
Copy link

sorry, I was forced to work on other things for the past month and am not so sure, what I meant. I also understood in the meantime that recursively setting defaults is a bit wonky, as lower level defaults may disagree with upper-level defaults. If I get my thoughts in order, I will post again.

Thanks for this great library, by the way!

@efokken-abb
Copy link

Now I worked again on this and now I feel I understand the behavior of the library and the behavior requested by this issue.

From my testing, fastjsonschema goes through all nesting from top to bottom and if at any one point a property is

listed AND has a default value AND is absent,

then the default will be inserted. This is independent of the nesting level of the json (above the defaulted value).

The OP wants the following: If an outer container does not have a default, but one of its properties does, than the outer container should automatically get a default that contains the default of the (inner) property.

Maybe this helps future readers. I can say that I don't need this functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants