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

GUACAMOLE-1239: Correct regressions introduced by case sensitive checks for usernames. #1026

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

necouchman
Copy link
Contributor

@necouchman necouchman commented Oct 11, 2024

This corrects the regressions introduced by #902 in implementing case-sensitive usernames, correcting the @Parameter annotations in a couple of places and moving the caseSensitive parameter up to the base mapper class instead of overriding the UserMapper class. It also moves the warning for the case sensitivity check for MySQL and SQL Server to the Guice module class so that it is only shown once when the module is loaded and not repeatedly for each query (as discussed on the Jira issue).

Note that I left the documentation for the caseSensitive parameter generic in the base class, and fairly ambiguous - if someone thinks I should put more verbage there around the fact that it currently only applies to usernames, I'm happy to do that, just trying to leave it as generic as possible sense it is in the base class.

@necouchman necouchman force-pushed the working/case-insensitive branch 2 times, most recently from 638484f to 1739efd Compare October 11, 2024 16:39
@necouchman necouchman marked this pull request as draft October 11, 2024 16:46
Copy link
Contributor

@mike-jumper mike-jumper left a comment

Choose a reason for hiding this comment

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

Everything else looks good and I like the generic rewording.

@necouchman
Copy link
Contributor Author

Everything else looks good and I like the generic rewording.

I need to take one more look at the ObjectRelationMapper and related interfaces to see if I need to move those changes down to the base class and do something similar there. Then it should be ready for review/merge.

@necouchman necouchman marked this pull request as ready for review October 11, 2024 18:12
@necouchman
Copy link
Contributor Author

@mike-jumper Okay, ready for final review, I believe...

@mike-jumper mike-jumper merged commit 3fcc59a into apache:staging/1.6.0 Oct 11, 2024
1 check passed
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.

2 participants