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

JAVA-5774 Allow all property types for Filter.regex() #1619

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jenssuhr
Copy link

While it makes some sense to only allow the regex function on String properties, this also precludes using it with properties that wrap the String in a value class.

To allow for that use case, I think it would make sense to relax the property type and allow anything there.

@rozza
Copy link
Member

rozza commented Feb 4, 2025

I'm not sure I fully understand the usecase could you please add some test cases? That should help.

@jenssuhr jenssuhr force-pushed the MIRA-5774-regex-restrictions branch from 75bd37c to b9f235b Compare February 4, 2025 09:58
@jenssuhr
Copy link
Author

jenssuhr commented Feb 4, 2025

I'm not sure I fully understand the usecase could you please add some test cases? That should help.

Sure, I can do that

@jenssuhr jenssuhr closed this Feb 4, 2025
@jenssuhr jenssuhr deleted the MIRA-5774-regex-restrictions branch February 4, 2025 10:00
@jenssuhr jenssuhr restored the MIRA-5774-regex-restrictions branch February 4, 2025 10:01
@jenssuhr
Copy link
Author

jenssuhr commented Feb 4, 2025

Oops, forgot that renaming the source branch closes the PR

@jenssuhr jenssuhr reopened this Feb 4, 2025
…ter on a non-String (value class) property
@jenssuhr
Copy link
Author

jenssuhr commented Feb 4, 2025

I've added an example for filtering on a property with a value class type, but it would also be useful for data classes with a custom codec that serialize into a string.

@jyemin jyemin added the external label Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants