Skip to content

Configuration metadata - Improve camelCaseToDash for nicer configuration names #17273

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

Closed
davsclaus opened this issue Jun 20, 2019 · 6 comments
Closed
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@davsclaus
Copy link

The spring boot auto-configuration via metadata (eg application.properties) can be improved to have a better camelCaseToDash algorithm.

For example at Apache Camel we have a JMS component that has many options, one of them is:

 private Boolean includeAllJMSXProperties = false;

https://github.com/apache/camel/blob/master/platforms/spring-boot/components-starter/camel-jms-starter/src/main/java/org/apache/camel/component/jms/springboot/JmsComponentConfiguration.java#L506

Which gets generated as camel-jms-starter Spring Boot JAR with this option name:

We then grab all these SB option names and include them in the online documentation at:
https://github.com/apache/camel/blob/master/components/camel-jms/src/main/docs/jms-component.adoc#spring-boot-auto-configuration

Where you can see the option in the table listed as

camel.component.jms.configuration.include-all-j-m-s-x-properties

Notice how JMSX gets transformed to -j-m-s-x- which is a bit ugly, instead it would be nicer if it was -jmsx-

So it would be nicer if the algorithm was improved to output the name as:

camel.component.jms.configuration.include-all-jmsx-properties

Which we have done in our Camel Main (standalone Camel without Spring Boot)
https://github.com/apache/camel/blob/master/examples/camel-example-main-artemis/src/main/resources/META-INF/spring-configuration-metadata.json#L950

We are using this algorithm to do this:
https://github.com/apache/camel/blob/master/core/camel-util/src/main/java/org/apache/camel/util/StringHelper.java#L825

Which works with this unit test:

    @Test
    public void testCamelCashToDash() throws Exception {
        assertEquals("hello-world", camelCaseToDash("HelloWorld"));
        assertEquals("hello-big-world", camelCaseToDash("HelloBigWorld"));
        assertEquals("hello-big-world", camelCaseToDash("Hello-bigWorld"));
        assertEquals("my-id", camelCaseToDash("MyId"));
        assertEquals("my-id", camelCaseToDash("MyID"));
        assertEquals("my-url", camelCaseToDash("MyUrl"));
        assertEquals("my-url", camelCaseToDash("MyURL"));
        assertEquals("my-big-id", camelCaseToDash("MyBigId"));
        assertEquals("my-big-id", camelCaseToDash("MyBigID"));
        assertEquals("my-big-url", camelCaseToDash("MyBigUrl"));
        assertEquals("my-big-url", camelCaseToDash("MyBigURL"));
        assertEquals("my-big-id-again", camelCaseToDash("MyBigIdAgain"));
        assertEquals("my-big-id-again", camelCaseToDash("MyBigIDAgain"));
        assertEquals("my-big-url-again", camelCaseToDash("MyBigUrlAgain"));
        assertEquals("my-big-url-again", camelCaseToDash("MyBigURLAgain"));

        assertEquals("use-mdc-logging", camelCaseToDash("UseMDCLogging"));
    }

https://github.com/apache/camel/blob/master/core/camel-util/src/test/java/org/apache/camel/util/StringHelperTest.java

PS: I am not sure if there is any logic that would need to be changed as well to support mapping with this new algorithm.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 20, 2019
@mbhave
Copy link
Contributor

mbhave commented Jun 20, 2019

Related issue and commit which contains history about why this was done this way.

@philwebb
Copy link
Member

We might be able to revisit #5330 in 2.x since we've rewritten the binder.

@philwebb philwebb added the for: team-attention An issue we'd like other members of the team to review label Jun 20, 2019
@mbhave mbhave changed the title Configuration metadata - Improve camelCashToDash for nicer configuration names Configuration metadata - Improve camelCaseToDash for nicer configuration names Jun 24, 2019
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jun 26, 2019
@philwebb philwebb self-assigned this Jun 26, 2019
@bclozel bclozel added the for: team-attention An issue we'd like other members of the team to review label Feb 20, 2020
@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Mar 5, 2020
@mbhave
Copy link
Contributor

mbhave commented Mar 9, 2020

The binder supports this so we should do something so that the metadata generates include-all-jmsx-properties

@mbhave mbhave added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 9, 2020
@mbhave mbhave added this to the 2.x milestone Mar 9, 2020
@mbhave mbhave removed their assignment Mar 9, 2020
@wilkinsona
Copy link
Member

wilkinsona commented Mar 10, 2020

Excellent. Thanks, @mbhave.

I wonder what effect a change in the metadata will have on backwards compatibility. For example, if someone has include-all-j-m-s-x-properties in their .properties or .yml file how will their IDE behave when they upgrade to a version with the new metadata. I think it'd be nice if it didn't start complaining that they were using a non-existent property. It'd be nicer still if it could guide them towards using include-all-jmsx-properties instead.

@snicoll
Copy link
Member

snicoll commented Mar 10, 2020

We can imagine generating an additional deprecated entry with a replacement that points to the new format. This can be removed in a future release.

@philwebb
Copy link
Member

We're cleaning out the issue tracker and closing issues that we've not seen much demand to fix. Feel free to comment with additional justifications if you feel that this one should not have been closed.

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2022
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed type: enhancement A general enhancement labels Aug 19, 2022
@philwebb philwebb removed this from the 2.x milestone Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

7 participants