MLE-26460 Refactor: Cleaning up usage of QueryConfig #1875
Conversation
|
Copyright Validation Results ✅ Valid Files
✅ All files have valid copyright headers! |
There was a problem hiding this comment.
Pull request overview
This PR refactors the QueryConfig class from a mutable inner class to an immutable record, improving code clarity and maintainability. The refactoring consolidates multiple constructor parameters into a single QueryConfig object.
Key changes:
- Converted
QueryConfigfrom an inner class with public fields to a top-level immutable record - Simplified
QueryBatcherImplconstructor by accepting a singleQueryConfigparameter instead of 6 individual parameters - Restructured the
initConfigmethod to constructQueryConfigusing local variables before instantiation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| QueryConfig.java | New record definition with 6 fields replacing the previous inner class |
| QueryBatcherImpl.java | Updated constructor to accept QueryConfig object and extract values from it |
| DataMovementServices.java | Refactored initConfig to build QueryConfig record; removed inner class definition |
| DataMovementManagerImpl.java | Updated to pass QueryConfig object to QueryBatcherImpl constructor; removed unused import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.queryMethod = "POST"; | ||
| this.query = queryMgr.newRawCtsQueryDefinition(new StringHandle(serializedCtsQuery).withFormat(Format.JSON)); | ||
| this.originalQuery = originalQuery; | ||
| this.filtered = queryConfig.filtered(); |
There was a problem hiding this comment.
The filtered field is being assigned the value from queryConfig.filtered() which can be null (Boolean object), but the previous code had a null check. If filtered is null, this could cause unexpected behavior. Consider preserving the null check: if (queryConfig.filtered() != null) { this.filtered = queryConfig.filtered(); }
| this.filtered = queryConfig.filtered(); | |
| if (queryConfig.filtered() != null) { | |
| this.filtered = queryConfig.filtered(); | |
| } |
Made QueryConfig into a record so it's immutable, making it easier to understand. And passing it to the constructor of QueryBatcherImpl instead of having 6 separate args.
rjrudin
force-pushed
the
feature/1872-github
branch
from
2460b7a to
831ecdc
Compare
Made QueryConfig into a record so it's immutable, making it easier to understand. And passing it to the constructor of QueryBatcherImpl instead of having 6 separate args.