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

Name Utils underscoreSeparate method adds extra underscores to the beginning of strings when starting with underscore #10140

Closed
trynow-david opened this issue Nov 21, 2023 · 3 comments · Fixed by #10516
Assignees
Labels
type: bug Something isn't working

Comments

@trynow-david
Copy link

Expected Behavior

underscoreSeparate("_nameOfThing") should result in "_name_of_thing" but actually results in "__name_of_thing"

Actual Behaviour

String prepended with an underscore result in double underscore prepended strings

Steps To Reproduce

  1. Call underscoreSeparate with a string like "_nameOfThing"
  2. Look at result, which is wrong.

Note I looked for unit tests for this method and there aren't any or I would have commented that you could add one to a test case. The private method beneath this method is the real problem specifically these lines

         private static String separateCamelCase(String name, boolean lowerCase, char separatorChar) {

...
                   if (c != separatorChar) {
                            if (last == separatorChar) {
                                newName.append(separatorChar);
                            }
                            newName.append(c);

Environment Information

No response

Example Application

No response

Version

4.2.0

@trynow-david
Copy link
Author

To note the further issue this causes is that when you name a column on a micronaut-data entity something like @column(name = "_my_column_name")
it will not select the right column in querying and causes issues, which you can't fix without changing the naming strategy

@trynow-david
Copy link
Author

For those who run into this issue, you can override the PhysicalNamingStrategy like this with the fix in it.

package com.trynow.micronaut

import io.micronaut.context.annotation.Primary
import io.micronaut.context.annotation.Replaces
import io.micronaut.data.hibernate.naming.DefaultPhysicalNamingStrategy
import io.micronaut.data.model.naming.NamingStrategy
import jakarta.inject.Singleton
import org.hibernate.boot.model.naming.Identifier
import org.hibernate.boot.model.naming.PhysicalNamingStrategy
import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment

/***
 * This is an almost exact copy of the DefaultPhysicalNamingStrategy class with a fix for a bug
 * that occurs when an identifier start with an underscore.
 */
@Singleton
@Primary
@Replaces(DefaultPhysicalNamingStrategy::class)
class PhysicalNamingStrategyCustom : PhysicalNamingStrategy {
    override fun toPhysicalCatalogName(name: Identifier?, jdbcEnvironment: JdbcEnvironment?): Identifier? {
        return getIdentifier(name)
    }

    override fun toPhysicalSchemaName(name: Identifier?, jdbcEnvironment: JdbcEnvironment?): Identifier? {
        return getIdentifier(name)
    }

    override fun toPhysicalTableName(name: Identifier?, jdbcEnvironment: JdbcEnvironment?): Identifier? {
        return getIdentifier(name)
    }

    override fun toPhysicalSequenceName(name: Identifier?, jdbcEnvironment: JdbcEnvironment?): Identifier? {
        return getIdentifier(name)
    }

    override fun toPhysicalColumnName(name: Identifier?, jdbcEnvironment: JdbcEnvironment?): Identifier? {
        name?.let {
            // In this case the getIdentifier code causes double underscore to be prepended to a string,
            // so we just return the original name, and we will just handle that column name more explicitly
            if (name.text.startsWith("_")) {
                return name
            }
        }
        return getIdentifier(name)
    }

    private fun getIdentifier(name: Identifier?): Identifier? {
        return if (name == null) {
            null
        } else {
            Identifier(
                NamingStrategy.DEFAULT.mappedName(name.text),
                name.isQuoted
            )
        }
    }
}

@wetted
Copy link
Contributor

wetted commented Feb 19, 2024

underscoreSeparate("_nameOfThing") should result in "_name_of_thing" but actually results in "__name_of_thing"

Actually, the expected result should be _name_Of_Thing because the NameUtils.underscoreSeparate does not perform lowercase. I will overload the call to support it with underscoreSeparate(String camelCase, boolean lowercase)

wetted added a commit that referenced this issue Feb 19, 2024
… is the separator char results in a double separator char prefix. Also overload the method to support a lowercase result.

fixes #10140
@wetted wetted added the type: bug Something isn't working label Feb 19, 2024
sdelamo pushed a commit that referenced this issue Feb 22, 2024
… is the separator char (#10516)

* Fix special case of NameUtils.underscoreSeparate where the first char is the separator char results in a double separator char prefix. Also overload the method to support a lowercase result.

fixes #10140
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
No open projects
Status: Done
2 participants