-
-
Notifications
You must be signed in to change notification settings - Fork 68
allow to customize json search function #188
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
base: master
Are you sure you want to change the base?
Conversation
rsql-jpa/src/main/java/io/github/perplexhub/rsql/jsonb/JsonbSupport.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for bearing with me addressing previous comments. We are almost there
rsql-jpa/src/main/java/io/github/perplexhub/rsql/jsonb/JsonbExtractor.java
Outdated
Show resolved
Hide resolved
@Builder | ||
@Accessors(fluent = true) | ||
@Getter | ||
public class JsonbExtractorSupport implements JsonbExtractor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe final
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, can we use record
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with record
im unable to use initilizing expressions with default values for lombok builder as it is now:
@Builder.Default
private final String pathExists = JsonbConfiguration.DEFAULT.pathExists()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this would address that https://stackoverflow.com/a/76107286
rsql-jpa/src/main/java/io/github/perplexhub/rsql/jsonb/JsonbExpressionBuilder.java
Outdated
Show resolved
Hide resolved
/** | ||
* jsonb expression configuration | ||
*/ | ||
public interface JsonbConfiguration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need it be an interface?
At the first glance I'm unable to see what benefit, flexibility or extensibility we gain by enabling client code to implement this interface. The configuration part is done via JsonbConfigurationSupport
and that's that. Perhaps we should rename JsonbConfigurationSupport
to JsonbConfiguration
and remove the interface? wdyt?
Resolves #187