Add comment support to json

[altro3]

Feature description

This is not a common case, but sometimes when you use json for the configuration format, you need to make some notes inside the json . I know the json standard doesn't support comments, but for example jackson has a feature to deserialize json with comments.

There is currently no way to configure objectMapper in serde to support comments. I also think that it is necessary to add the ability to create a custom objectMapper with non-typical settings, for example, support for comments :-).

[altro3]

@graemerocher @dstepanov @sdelamo Any news about this ?

You need to understand why this feature is extremely necessary:

Without it, it is impossible to use the json format for configuration normally. Of course, comments are required in large configurations. otherwise, it is impossible to work normally with such a configuration. If anything, jackson supports comments in json

[dstepanov]

You can configure Jackson read/write features see SerdeJacksonConfiguration

[sdelamo]

As you said, JSON does not support comments in the specification. I would not complicate the code to support a not standard feature.

Moreover, we are not recommending people to use JSON for configuration but formats such as properties, yaml, toml. Formats which support comments.

[altro3]

The decision on the choice of format does not always depend on you. Yes, I understand that the specification does not provide for comments in json. But all popular solutions for working with json support the inclusion of comments in the parser, I mean jackson and gson.

As I understand from the project, this is an attempt to speed up the work of these libraries at the expense of the micronaut code generator. If so, then it is quite logical to support all the functionality of these libraries so that it is really possible to connect micronaut-serde.

So, if I were you, I would not focus on specifications or other bureaucratic inventions, but rather look at what is actually used by real programmers

My opinion is that you should create not on the basis of rules invented by someone, but on the basis of logic, common sense and efficiency. And if the rules get in your way, then your task is to expand the rules.

Programs and computers work based on rules. So who are we - programs or real people?

[graemerocher]

@altro3 did you try @dstepanov's recommendation of setting the Jackson feature to allow comments ?https://fasterxml.github.io/jackson-core/javadoc/2.8/com/fasterxml/jackson/core/JsonParser.Feature.html#ALLOW_COMMENTS

[graemerocher]

See https://micronaut-projects.github.io/micronaut-serialization/latest/guide/configurationreference.html#io.micronaut.serde.jackson.SerdeJacksonConfiguration

[altro3]

I'll check it out in a few days. There's no time for that right now.

[graemerocher]

I see, would be good to know because if it works without changes I don't see why anything needs to be done. It doesn't make sense to enable by default parser options that add overhead or are used in niche cases. As long as they are configurable and can be enabled by the user I see no issue here.

[altro3]

@dstepanov @graemerocher I checked it out. It turned out to be an interesting situation. Yes, the comment support works as it should. Thanks!

but!

Now I can't turn off support for comments in any way and they are always parsed, ignoring the settings:

micronaut:
  serde:
    jackson:
      parser-features:
        ALLOW_COMMENTS: false
      read-features:
        ALLOW_JAVA_COMMENTS: false

You can reproduce it here: https://github.com/altro3/micronaut3-bug

Just send reqesut like this:

[altro3]

@dstepanov There's another problem: you've added the ability to clone a mapper - that's fine. but! It's funny, but now it's not possible to set the "allow comments" setting, and this setting is very important, because it's most convenient to create a default mapper with comments turned off, but for configs use a cloned mapper with comments enabled in JSON

[dstepanov]

It wasn’t me who added it :) Any idea why did that break it?

[altro3]

@graemerocher @dstepanov I just don't understand the logic of how this feature works in the mikronaut series. It says so in jackson:

Feature that determines whether parser will allow use of Java/C/C++ style comments (both '/'+'*' and '//' varieties) within 
parsed content or not.
Since JSON specification does not mention comments as legal construct, this is a non-standard feature; however, in the wild
 this is extensively used. As such, feature is disabled by default for parsers and must be explicitly enabled.

In general, it is not clear to me whether enabling or disabling this feature affects performance. But it is clear that this feature should be turned off for the mapper, which is used for data exchange
I also understand that it should be possible to enable this feature exclusively for local use or special cases.

And now it turns out that comments are ALWAYS allowed and there is no way to disable them.

I repeat: it is not clear to me how critical this is and how it affects performance. But from my point of view, it should be possible to disable support for comments and throw an error in this case if json with comments comes

[dstepanov]

@altro3 What do you think can be done here? The comment feature is part of the Jackson; according to the code, it should be disabled by default. Is there anything else we can do?

[altro3]

@dstepanov So that's the problem. This feature is ALWAYS on at the moment! there is no way to disable parsing of json's with comments inside.

It looks like a configuration mapping error. my serde settings:

micronaut:
  serde:
    jackson:
      parser-features:
        ALLOW_COMMENTS: false
      read-features:
        ALLOW_JAVA_COMMENTS: false

So, now comments ALLOWED always. And can't disable this feature and it enabled by default. This is a bug, I believe.

Sample app to reproduce thi buug: https://github.com/altro3/micronaut3-bug

[dstepanov]

Your example fails with:

{
  "_links": {
    "self": [
      {
        "href": "/test",
        "templated": false
      }
    ]
  },
  "_embedded": {
    "errors": [
      {
        "message": "Invalid JSON: Unexpected character ('/' (code 47)): maybe a (non-standard) comment? (not recognized as one since Feature 'ALLOW_COMMENTS' not enabled for parser)\n at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 2, column: 4]"
      }
    ]
  },
  "message": "Invalid JSON"
}

Also fails without any extra parser configuration.

[altro3]

Omg... I'm sorry, I didn't know this was a postman feature. It turns out that postman allows you to write comments and removes them himself before sending a request.

It turns out that one problem remains: when copying jsonMapper, it is not possible to enable/disable comment support.

Sample code

        var configMapper = objectMapper.cloneWithConfiguration(
                new DeserializationConfiguration()
                new SerializationConfiguration()
        );

Now DeserializationConfiguration is very poor:

How can I create normal objectMapper without comments support and copy it with comments support for different goals?

[dstepanov]

We should introduce interface JacksonObjectMapper extends ObjectMapper with some extra clone method that supports SerdeJacksonConfiguration.

[altro3]

Yes, this feature is clearly missing now :-(