Add per-query ConversionService support#208
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for a per-query Spring ConversionService so conversions can be isolated per request/query and take precedence over the library-wide (static) converter registry.
Changes:
- Add
ConversionServiceplumbing toRSQLVisitorBaseand thread it through JPA + QueryDSL predicate/spec builders. - Extend
QuerySupportto carry a per-queryConversionService. - Add tests and README documentation describing precedence and usage.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rsql-querydsl/src/test/java/io/github/perplexhub/rsql/RSQLQueryDslSupportTest.java | Adds a test ensuring a per-query ConversionService is preferred for QueryDSL predicates. |
| rsql-querydsl/src/main/java/io/github/perplexhub/rsql/RSQLQueryDslSupport.java | Adds a new toPredicate overload that accepts a ConversionService. |
| rsql-querydsl/src/main/java/io/github/perplexhub/rsql/RSQLQueryDslPredicateConverter.java | Adds constructor overload to inject/set a per-query ConversionService. |
| rsql-jpa/src/test/java/io/github/perplexhub/rsql/RSQLJPASupportTest.java | Adds tests ensuring QuerySupport.conversionService takes precedence (including for custom predicates). |
| rsql-jpa/src/main/java/io/github/perplexhub/rsql/RSQLJPASupport.java | Passes QuerySupport’s ConversionService into the JPA predicate converter. |
| rsql-jpa/src/main/java/io/github/perplexhub/rsql/RSQLJPAPredicateConverter.java | Adds constructor overload to accept and set ConversionService. |
| rsql-jpa/src/main/java/io/github/perplexhub/rsql/QuerySupport.java | Adds conversionService field + includes it in toString(). |
| rsql-common/src/test/java/io/github/perplexhub/rsql/RSQLVisitorBaseTest.java | Adds tests for precedence/fallback between instance and global conversion services. |
| rsql-common/src/main/java/io/github/perplexhub/rsql/RSQLVisitorBase.java | Implements instance ConversionService precedence in convert(...). |
| README.md | Documents global vs per-query conversion behavior and usage examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @perplexhub, @nstdio, I opened this PR as a follow-up to #207 to add optional per-query The goal is to let applications isolate conversion rules per query/context without changing existing behavior for users who rely on I also included focused tests for the common conversion path, JPA default operators, JPA custom predicates, and QueryDSL. I would appreciate your review or guidance if you prefer a narrower scope or a different API shape. Thanks! |
|
@ggomarighetti, thanks for your contribution. I’m fine with the change. @ng-galien @nstdio, what are your thoughts? |
|
Just to add a bit more context: although I did not include many details in the discussion section of the issue, I ran into this scenario while integrating the library into a new project. To cover some specific needs we had, I started building a layer on top of The concrete case came up because we handle multi-tenancy through a We have now separated that code from our main application codebase and maintain it as an independent package. We are still developing and polishing it with the goal of eventually making it production-ready, and we may publish it later on. I also wanted to say thanks: the work you have done and continue to maintain in this library, together with |
ng-galien
left a comment
There was a problem hiding this comment.
Good PR. The ConversionService was probably the last missing piece needed to benefit from all customization features in a stateless way.
This fills a gap. We should see whether it is the last one needed to make all customization features stateless, and then provide an abstraction on top to handle per-request configuration. I had the same need for enterprise applications that require a per-request mechanism for security policies, permissions, and tenant isolation. |
|
@ng-galien Changes made, and a follow-up in the first conversation. |
Regarding customization, we have faced a very similar issue, and it eventually led us to remove some functionality from our library. Having to instantiate basic components like operators, converters, etc, at every functions are invoked pushed us to create a generic application-level method to centralize those calls. The feature you are proposing here would be really valuable. I will also follow up on the related issue, since it seems closely connected to this discussion. |
|
Hi @perplexhub, @ng-galien, @nstdio, Just wanted to kindly follow up on this PR. This change would be very useful for my current use case, since it would help me move forward without having to keep a local workaround on top of the library. Is there anything else you would like me to adjust before it can move forward? Thanks again for taking the time to review it! |
|
v7.0.2 |
Resume
This PR adds optional per-query
ConversionServicesupport for RSQL predicate conversion.Today, custom value converters are registered through
RSQLJPASupport.addConverter(...), which mutates the static conversion service owned byRSQLCommonSupport. That works for simple applications, but it makes conversion rules JVM-global. Applications that run multiple Spring contexts, execute tests in parallel, use application-specific value objects, or validate RSQL before delegating execution cannot guarantee that the conversion rules used during validation are the same rules used later bySpecification.toPredicate(...).The change keeps the current global converter registry as the backward-compatible fallback, while allowing callers to pass a query-specific
ConversionService:Conversion now follows this order:
ConversionService, when present and able to convert.defaultConversionService, preservingaddConverter(...)behavior.The same conversion path is used by default JPA operators and JPA custom predicates. QueryDSL also gets a new overload that accepts a per-query
ConversionService.Evidence
Reference
Closes #207