Skip to content

JAVA-5736: Add bsonNamingStrategy option to support snake_case naming strategy #1589

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Feb 6, 2025

Conversation

leesungbin
Copy link
Contributor

@leesungbin leesungbin commented Dec 30, 2024

Ticket

JAVA-5736

Description

This PR adds bsonNamingStrategy property to BsonConfiguration to support snake_case naming strategy.

Testing

ran ./gradlew check

@leesungbin leesungbin marked this pull request as ready for review December 30, 2024 06:59
@jyemin jyemin requested a review from nhachicha January 2, 2025 14:56
@jyemin jyemin changed the title Add bsonNamingStrategy option to support snake_case naming strategy JAVA-5736: Add bsonNamingStrategy option to support snake_case naming strategy Jan 6, 2025
Copy link
Contributor

@nhachicha nhachicha left a comment

Choose a reason for hiding this comment

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

Hi @leesungbin
Thank you for the PR! We really appreciate your contribution.

However, the current Regexp-based approach doesn’t fully account for all use cases. According to the Kotlin language specification, variable names can include Unicode characters, such as Cyrillic characters. For example, a variable name like имяПеременной would not be handled correctly.

Additionally, backtick names (e.g., `my Class`) won’t be transformed as expected.

A possible refactor could leverage the existing implementation of SnakeCase in kotlinx.serialization. You can refer to this implementation:
SnakeCase in kotlinx.serialization.

Also, please ensure the tests align with the transformation rules and acronyms defined in:
JsonNamingStrategy Transformation Rules.

Looking forward to your updates! Let me know if you need any clarification.

@leesungbin
Copy link
Contributor Author

Hello @nhachicha, thanks for providing such detailed comments.

I have copied the convertCamelCase logic from the kotlinx.serialization library and used it to convert camelCase to snake_case.

Additionally, I introduced an error condition for scenarios where decoding snake_case to camelCase is ambiguous (for instance, my_http_auth could be interpreted as myHttpAuth or myHTTPAuth).

Please give it a look. Thanks!

@leesungbin leesungbin requested a review from nhachicha January 17, 2025 08:49
@nhachicha
Copy link
Contributor

Thanks for iterating on this @leesungbin can you please rebase against JAVA-5736 branch?

@leesungbin
Copy link
Contributor Author

Sure, I've rebased on to JAVA-5736.

@nhachicha nhachicha changed the base branch from main to JAVA-5736 January 28, 2025 10:27
@jyemin jyemin added the external label Feb 4, 2025
@nhachicha nhachicha merged commit f535254 into mongodb:JAVA-5736 Feb 6, 2025
2 of 3 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants