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

Add YamlNamingStrategy #400

Merged
merged 20 commits into from
May 27, 2023

Conversation

russellbanks
Copy link
Contributor

@russellbanks russellbanks commented Mar 15, 2023

Adds YamlNamingStrategy from #398 with pre-defined snake_case, kebab-case, PascalCase, and camelCase.

The SnakeCase implementation has been taken and modified from JsonNamingStrategy.kt in KotlinX Serialization.

Usage

YamlConfiguration(
    yamlNamingStrategy = YamlNamingStrategy.PascalCase  // CamelCase, SnakeCase, KebabCase
),

Example data class:

@Serializable
data class Data(
   val propertyName: String,
   val otherName: String
)

Example Yaml:

PropertyName: "value"
OtherName: "value"

This will successfully serialize and deserialize without the need for @SerialName on every value in the data class.

Custom naming strategy

Converts to lowercase and capitalises first letter:

YamlConfiguration(
    yamlNamingStrategy = { it.lowercase().replaceFirstChar(Char::titlecase) }
),

Converts to uppercase

YamlConfiguration(
    yamlNamingStrategy = String::uppercase
),

Note

As @SerialName is resolved at compile time rather than runtime, using a YamlNamingStrategy will ignore the casing of all @SerialName annotations.

Closes #398

@charleskorn
Copy link
Owner

Thank you for the PR @russellbanks!

@kenyee
Copy link

kenyee commented Apr 1, 2023

any idea when this will be merged?
Could use this on a project I'm working on :-)

@charleskorn
Copy link
Owner

any idea when this will be merged? Could use this on a project I'm working on :-)

This is currently an incomplete draft - it'll be merged when it's ready :)

If you'd like to see it merged sooner, I'm sure @russellbanks would love some help.

@russellbanks
Copy link
Contributor Author

It's the linking up of the algorithms to the serial names that would be helpful to have some pointers for :)

@charleskorn
Copy link
Owner

It's the linking up of the algorithms to the serial names that would be helpful to have some pointers for :)

This is where property names are emitted - so that should be the place to perform the transformation.

@russellbanks russellbanks marked this pull request as ready for review April 26, 2023 18:00
@russellbanks
Copy link
Contributor Author

russellbanks commented Apr 26, 2023

@charleskorn This is ready for review I think!

One thing that I should note is that applying a naming strategy will ignore the casing of all @SerialName annotations as that is resolved at compile time rather than runtime, meaning there's no way to know if it was applied or not. Naming strategies should cover most naming cases in Yaml and in the rare case that there is inconsistent naming, @SerialName or a custom naming strategy can be used instead.

Copy link
Owner

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together @russellbanks! Overall it's looking good.

@russellbanks
Copy link
Contributor Author

@charleskorn I've made the requested changes! Let me know if there's anything else.

Copy link
Owner

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great, thanks again for this @russellbanks.

I've just noticed one more case that is missing a test: could you please add a test for the case where an unknown property is included in the YAML document being read? I want to make sure the exception includes the transformed names (from the naming strategy) rather than the property in the descriptor.

@russellbanks
Copy link
Contributor Author

russellbanks commented May 15, 2023

This is looking great, thanks again for this @russellbanks.

I've just noticed one more case that is missing a test: could you please add a test for the case where an unknown property is included in the YAML document being read? I want to make sure the exception includes the transformed names (from the naming strategy) rather than the property in the descriptor.

Well thought @charleskorn! I've been able to map that but this test case fails and I'm not sure how to map the YamlMap's entries.

context("given some input representing an object with an unknown key, using a naming strategy") {
    val input = "oneTwoThree: something".trimIndent()

    context("parsing that input") {
        it("throws an exception with the naming strategy applied") {
            val exception = shouldThrow<UnknownPropertyException> {
                Yaml(
                    configuration = YamlConfiguration(yamlNamingStrategy = YamlNamingStrategy.SnakeCase)
                ).decodeFromString(ComplexStructure.serializer(), input)
            }

            exception.asClue {
                it.message shouldBe "Unknown property 'one_two_three'. Known properties are: boolean, byte, char, double, enum, float, int, long, nullable, short, string"
                it.line shouldBe 1
                it.column shouldBe 1
                it.propertyName shouldBe "one_two_three"
                it.validPropertyNames shouldBe setOf("boolean", "byte", "char", "double", "enum", "float", "int", "long", "nullable", "short", "string")
                it.path shouldBe YamlPath.root.withMapElementKey("one_two_three", Location(1, 1))
            }
        }
    }
}

This is the test case that fails

it.path shouldBe YamlPath.root.withMapElementKey("one_two_three", Location(1, 1))
Expected :YamlPath(segments=[com.charleskorn.kaml.YamlPathSegment$Root@1ebcc98f, MapElementKey(key=one_two_three, location=Location(line=1, column=1))])
Actual   :YamlPath(segments=[com.charleskorn.kaml.YamlPathSegment$Root@1ebcc98f, MapElementKey(key=oneTwoThree, location=Location(line=1, column=1))])

@charleskorn
Copy link
Owner

I think the test is not quite right - the exception should contain the properties as they appear / would appear in YAML.

So this:

it.path shouldBe YamlPath.root.withMapElementKey("one_two_three", Location(1, 1))

should be:

it.path shouldBe YamlPath.root.withMapElementKey("oneTwoThree", Location(1, 1))

And the exception message should be something like Unknown property 'oneTwoThree'. Known properties are: boolean, byte, ...

@russellbanks
Copy link
Contributor Author

@charleskorn I've done that and I have also tested this PR by publishing to Maven Local and it works well in my project :)

@charleskorn charleskorn merged commit 4028606 into charleskorn:main May 27, 2023
@charleskorn
Copy link
Owner

Thanks again for the PR @russellbanks!

@russellbanks russellbanks deleted the add-yaml-naming-strategy branch May 27, 2023 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Yaml Naming Strategies
3 participants