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

in yaml, a ~ key should allow to assing a value to the parent key #506

Open
rmanibus opened this issue Nov 5, 2024 · 12 comments
Open

in yaml, a ~ key should allow to assing a value to the parent key #506

rmanibus opened this issue Nov 5, 2024 · 12 comments
Labels
yaml Issue related to YAML format backend

Comments

@rmanibus
Copy link

rmanibus commented Nov 5, 2024

In Yaml spec, if one property is the prefix of another, a value can be attributed to the parent one using ~ as a null key to represent any YAML property that is a prefix of another one.

The following test is failing:

    public void testTildeIsDeserializedAsNullKey() throws Exception
    {
        final String YAML =
                "quarkus:\n" +
                        "  log:\n" +
                        "    sentry:\n" +
                        "      ~: true\n" +
                        "      dsn: 'some string'";

        JsonNode node = MAPPER.readValue(YAML, JsonNode.class);
        assertTrue(node.get("quarkus").get("log").get("sentry").booleanValue());
    }

instead, the following statement is true:

assertTrue(node.get("quarkus").get("log").get("sentry").get("~").booleanValue())

quarkus.log.sentry."~" is true instead of quarkus.log.sentry

Relevant conversation:

https://quarkusio.zulipchat.com/#narrow/stream/187038-dev/topic/yaml.20property.20conflict

Relevant spec:

https://quarkus.io/guides/config-yaml#configuration-property-conflicts

https://yaml.org/spec/1.2.2/

@rmanibus
Copy link
Author

rmanibus commented Nov 5, 2024

To clarify, this is causing issues with the Spotless formatter, it is basically transforming any ~ key in the "~" string

@cowtowncoder
Copy link
Member

Quick question: how would you represent such document without ~ marker? Wouldn't there be problem with dsn key conflicting -- that is, there being both:

  • quarkus.log.sentry = true
  • quarkus.log.sentry.dsn = "some string"

which is impossible structurally?

Put another way; how would equivalent, non-tilde-using document look like?

@rmanibus
Copy link
Author

rmanibus commented Nov 5, 2024

I think you cannot, that's the whole point of the ~ key, it allows you to assign a value to a key that also has children

@cowtowncoder
Copy link
Member

cowtowncoder commented Nov 5, 2024

So could you not access quarkus.log.sentry.dsn in such case? Or is that simply dropped in such a case?

If there is no equivalent document, I don't think Jackson data model can represent this.
And if not, this is something that cannot really be supported -- it might actually be best to just throw an exception if so.

But I guess I am also not clear on point of using such marker.
I think I am missing something obvious.

EDIT: note, too, that link was to YAML 1.2 spec: Jackson 2.x uses SnakeYAML which supports YAML 1.1 only (https://bitbucket.org/snakeyaml/snakeyaml).
I don't know if tilde handling is part of 1.1 already.

@cowtowncoder
Copy link
Member

cowtowncoder commented Nov 5, 2024

Ohhh. Maybe it's effectively "empty" key? So something like JSON would have:

{ "quarkus": {
      "log": {
         "sentry": {
            "": true,
            "dsn": "some string"
         }
      }
   }
}

... although that would not be hidden by Jackson either; would need to be access with empty String as key.

EDIT: close, it is actually null key (similar to how "~" as value is same as null).

This is not something Jackson can represent since its data model does require all JsonToken.FIELD_NAME tokens to have String value. We could conceivably map it to empty String, becoming property with "" key. Not sure that'd be any better than leaving ~ as-is.

@cowtowncoder
Copy link
Member

Based on all of above, I am pretty sure we cannot, in general make this work:

assertTrue(node.get("quarkus").get("log").get("sentry").booleanValue());

since "sentry" must be an Object value (with 2 logical properties; one with String key, other with null). I guess I don't know how YAML libraries generally would expose it either.

So the question would be whether to map ~ key to something else; and if so, to what?

@cowtowncoder cowtowncoder added the yaml Issue related to YAML format backend label Nov 5, 2024
@rmanibus
Copy link
Author

rmanibus commented Nov 5, 2024

The point of using this marker is to allow the use of yaml in configuration systems that have not initially been thought of with yaml in mind

@cowtowncoder
Copy link
Member

Yeah the problem is that it does really map to JsonNode model (nor can I see an easy way to model it). In your case, either "null" key would need to replace value of sentry (so "sentry.dsn" would be lost), or we'd need a surrogate for "null" key.
For latter, empty String seems close enough -- but I am not sure TBH whether it'd be any more convenient to access than "~" key.

Put another way: what is being specifically asked is not doable. But I am open to ideas of alternative improvements.

@maxandersen
Copy link

@cowtowncoder are you effectively saying that jackson does'nt support true null key ? thats what ~ is, a special token to represent null/Null/NULL

@cowtowncoder
Copy link
Member

Correct: Jackson's streaming/token model is based on JSON and there's no null key, only null values. And JsonNode, similarly only String keys. YAML and other formats need to be expressed in these models.

Theoretically I suppose it would be possible to expose null key but I don't think it'd work through all processing.

And even if it was done, would require access by null as key like:

node.get("quarkus").get("log").get("sentry").get(null).booleanValue()

@maxandersen
Copy link

Yeah; but that (using null) for lookup would be expected. But yeah I can see how that could be problematic in places.

@cowtowncoder
Copy link
Member

I think I am still inclined to prefer empty String as surrogate (and possible make this YAMLParser.Feature whether to replace "~" key or not -- since there may well be existing code that relies on current handling).

I was first thinking of allowing specific "replacement key", that'd also allow null, but I think that there are too many places downstream that wouldn't stomach null that it's probably not worth it (more work to have non-boolean switches, but more importantly, trying to make databind pieces accept null for JsonToken.FIELD_NAME and so on).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
yaml Issue related to YAML format backend
Projects
None yet
Development

No branches or pull requests

3 participants