Enum property values not hyphenated anymore in the configuration metadata

[ppalaga]

Describe the bug

We have a config property in Quarkus CXF defined as follows:

    @WithDefault("Keep-Alive")
    @WithConverter(ConnectionTypeConverter.class)
    public ConnectionType connection();

Before the recent refactoring of quarkus-extension-processor, the metadata was produced with hyphenated enum values target/asciidoc/generated/config/all-configuration-roots-generated-doc/io.quarkiverse.cxf.CxfConfig:

  {
    "configDocKey": {
      "type": "org.apache.cxf.transports.http.configuration.ConnectionType",
      "key": "quarkus.cxf.client.\"client-name\".connection",
      "additionalKeys": [],
      "configDoc": "The connection disposition. If close the connection to the server is closed after each request/response dialog. If\nKeep-Alive the client requests the server to keep the connection open, and if the server honors the keep alive request,\nthe connection is reused. Many servers and proxies do not honor keep-alive requests.",
      "withinAMap": true,
      "defaultValue": "keep-alive",
      "javaDocSiteLink": "",
      "docMapKey": "connection",
      "configPhase": "RUN_TIME",
      "acceptedValues": [
        "`close`",
        "`keep-alive`"
      ],
      "optional": false,
      "list": false,
      "passThroughMap": false,
      "withinAConfigGroup": true,
      "topLevelGrouping": "quarkus.cxf",
      "since": "2.2.3",
      "environmentVariable": "QUARKUS_CXF_CLIENT__CLIENT_NAME__CONNECTION",
      "enum": true
    }
  },

After the refactoring, the enum values in target/quarkus-config-doc/quarkus-config-model.yaml are present verbatim:

  - !<io.quarkus.annotation.processor.documentation.config.model.ConfigProperty>
    enum: true
    sourceClass: "io.quarkiverse.cxf.CxfClientConfig"
    sourceName: "connection"
    path: "quarkus.cxf.client.\"client-name\".connection"
    type: "org.apache.cxf.transports.http.configuration.ConnectionType"
    phase: "RUN_TIME"
    environmentVariable: "QUARKUS_CXF_CLIENT__CLIENT_NAME__CONNECTION"
    typeDescription: "ConnectionType"
    withinMap: true
    converted: true
    enumAcceptedValues:
      qualifiedName: "org.apache.cxf.transports.http.configuration.ConnectionType"
      values:
        CLOSE:
          configValue: "CLOSE"
        KEEP_ALIVE:
          configValue: "KEEP_ALIVE"
    defaultValue: "Keep-Alive"

Expected behavior

The enum values should be produced hyphenated, same as before quarkus-extension-processor refactoring.

I am aware of the following note in ConfigResolver

quarkus/core/processor/src/main/java/io/quarkus/annotation/processor/documentation/config/resolver/ConfigResolver.java

Lines 155 to 156 in baf77db

	// if the property has a converter, we don't hyphenate the values (per historical rules, not exactly sure of the reason)
	boolean hyphenateEnumValues = !discoveryConfigProperty.isConverted();

However my example proves that the rule to decide about hyphenation must have been complexer than just "if the property has a converter"

[quarkus-bot]

/cc @radcortez (config)

[ppalaga]

cc @gsmet

[gsmet]

Yeah, I have ported this rule from the existing code base.
It kinda makes sense because when you have a converter, you have no idea how it will convert things and if the hyphenated value is a valid input.

Now what I don't understand is why you had the previous behavior. I'll have a look later today.

[gsmet]

https://github.com/quarkusio/quarkus/blob/3.13.2/core/processor/src/main/java/io/quarkus/annotation/processor/generate_doc/ConfigDocItemFinder.java#L232-L235

I think the only reason why it wasn't hyphenated is that when @ConfigMapping support was added, the @WithConvert annotation wasn't added at this place.

We are a lot more consistent now.

Not entirely sure what's best but really hyphenating when having a converter that is not the default is probably a bad idea.

Is there a reason why you have a converter here?

[ppalaga]

Is there a reason why you have a converter here?

Haha, yes, IIRC it's only because earlier it was not possible to persuade Quarkus not to hyphenate the enum values in the metadata so I had to introduce a way to map what users got in the docs to the real values.

Another detail that makes this case a bit harder is that the enum comes from CXF, so I cannot annotate it with @ConfigDocEnumValue.

It is only a single property on Quarkus CXF side having this issue (but I have not looked into Camel yet) and I actually agree not-hyphenating makes sense if there is @WithConverter.

It would be great to have a way to adjust the presented values somehow.

Maybe a new @ConfigDocEnumValues annotation like this?

    @WithDefault("Keep-Alive")
    @WithConverter(ConnectionTypeConverter.class)
    @ConfigDocEnumValues("KEEP_ALIVE:Keep-Alive,CLOSE:Close")
    public ConnectionType connection();

[gsmet]

Maybe an annotation to force the hyphenation would be a bit more sustainable from a maintenance POV?

[ppalaga]

Maybe an annotation to force the hyphenation would be a bit more sustainable from a maintenance POV?

Yes, that would solve the current issue, but a general way to document accepted values (even for non-enums) would come in handy, wouldn't it?

How about this?

    @WithDefault("Keep-Alive")
    @WithConverter(ConnectionTypeConverter.class)
    @ConfigDocAcceptedValues({"Keep-Alive", "Close"})
    public ConnectionType connection();

[ppalaga]

I mean @ConfigDocAcceptedValues would be useful also for IDE content assist.

[ppalaga]

Anyway, if all the above is too complicated, given that I need this in just a single case, I can also workaround it by wrapping the CXF enum in a local one and use @ConfigDocEnumValue.

[ppalaga]

I can also workaround it by wrapping the CXF enum in a local one and use @ConfigDocEnumValue.

I did that right now and I'd be fine with closing this issue unless you see some value in handling this in a general manner?

[gsmet]

@ConfigDocAcceptedValues({"Keep-Alive", "Close"}) is cute but I'm afraid it's going to be hard to maintain.

@ConfigDocEnumValues("KEEP_ALIVE:Keep-Alive,CLOSE:Close") is less cute but probably a bit more future-proof.
(whatever the name we give to it)

For now I think I would start with @ConfigDocEnum(hyphenateAcceptedValues = true) and see what we can come up with in the future.

[gsmet]

I'm working on a patch.