Feature/refactor and add remaining queryspec - #168
Draft
abhinow-porwal wants to merge 8 commits into
Draft
Conversation
Add QuerySpec support for run and createOrUpdate operations across RelationalDao and MultiTenantRelationalDao. This enables more flexible query execution alongside existing DetachedCriteria-based methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses PR santanusinha#154 review feedback by adding complete test coverage: Unit Tests: - CreateOrUpdateByQuerySpecTest with 3 tests covering: - Entity creation path (when entity doesn't exist) - Entity update path (when entity exists) - Null entityGenerator handling Integration Tests: - MultiTenantRelationalDaoTest: 2 new tests - testCreateOrUpdateWithQuerySpec (creation and update paths) - testMultiShardRunWithQuerySpec (1000 entities across shards) - RelationalDaoTest: 2 new tests - testCreateOrUpdateWithQuerySpec (wrapper method) - testRunWithQuerySpec (multi-shard queries) Code Quality Fixes: - Fixed indentation in MultiTenantRelationalDao (lines 150-160) - Added JavaDoc to get(QuerySpec) method - Enhanced JavaDoc in CreateOrUpdateByQuerySpec documenting null behavior - Fixed OpType to use CREATE_OR_UPDATE_BY_QUERY_SPEC - Minor cleanup in OpContext Test Results: 298 tests passing, 0 failures, 0 regressions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This refactoring eliminates code duplication by making CreateOrUpdate generic over the criteria type parameter, removing the need for a separate CreateOrUpdateByQuerySpec class. 🎯 Problem Solved: CreateOrUpdate was tightly coupled to DetachedCriteria, forcing us to duplicate 99% of the logic in CreateOrUpdateByQuerySpec just to support QuerySpec. This violated the DRY principle and created maintenance burden. 💡 Solution: Added generic type parameter <C> to CreateOrUpdate, making it work with any criteria type (DetachedCriteria, QuerySpec, or future types). 📝 Changes: - Made CreateOrUpdate generic: CreateOrUpdate<T, C> - Updated visitor interface: visit(CreateOrUpdate<T, C>) - Removed CreateOrUpdateByQuerySpec.java (70 lines of duplicate code) - Removed CreateOrUpdateByQuerySpecTest.java (moved to CreateOrUpdateTest) - Removed CREATE_OR_UPDATE_BY_QUERY_SPEC from OpType enum - Updated all usages: * CreateOrUpdate.<T, DetachedCriteria> for legacy Hibernate API * CreateOrUpdate.<T, QuerySpec<T, T>> for modern JPA Criteria API - Updated BucketKeyPersistor visitor to handle generic CreateOrUpdate - Added comprehensive QuerySpec test coverage (3 new test methods) ✅ Results: - Single source of truth for create-or-update logic - Eliminated 203 lines of code (215 deleted, 12 added) - All 298 tests passing (up from 295) - Both DetachedCriteria and QuerySpec paths fully tested - Easy to add new criteria types (just use different type parameter) - Follows DRY and SOLID principles 🧪 Test Coverage: - testCreateOrUpdate_creation (DetachedCriteria) - testCreateOrUpdate_updation (DetachedCriteria) - testCreateOrUpdateWithQuerySpec_creation (QuerySpec) - testCreateOrUpdateWithQuerySpec_updation (QuerySpec) - testCreateOrUpdateWithQuerySpec_nullEntityGenerator (QuerySpec) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
abhinow-porwal
force-pushed
the
feature/refactor_and_add_remaining_queryspec
branch
from
May 7, 2026 08:48
08dfe3a to
beccd16
Compare
This refactoring makes RunWithCriteria generic over the criteria type
parameter, following the same pattern as CreateOrUpdate. This eliminates
the dual-path logic and wildcard types, improving type safety and code
maintainability.
🎯 Problem Solved:
RunWithCriteria used QuerySpec<?, ?> (wildcard types) which lost type
safety, and had complex dual-path logic with separate fields for
DetachedCriteria and QuerySpec paths. The class had to validate at
runtime which path was being used.
💡 Solution:
Added generic type parameter <C> to RunWithCriteria, making it work with
any criteria type in a type-safe manner.
📝 Changes:
- Made RunWithCriteria generic: RunWithCriteria<T, C>
- Replaced dual-path fields with single generic fields:
* Before: detachedCriteria, querySpec, handler, querySpecHandler
* After: criteria (type C), handler (Function<C, T>)
- Removed runtime validation in apply() method
- Removed QuerySpec<?, ?> wildcard usage (now type-safe)
- Updated visitor interface: visit(RunWithCriteria<T, C>)
- Updated all usages:
* RunWithCriteria.<T, DetachedCriteria> for legacy Hibernate API
* RunWithCriteria.<T, QuerySpec<T, T>> for modern JPA Criteria API
- Updated BucketKeyPersistor visitor to handle generic RunWithCriteria
✅ Results:
- Type-safe criteria handling (no more wildcards)
- Simplified logic (removed dual-path conditionals)
- Reduced code by 18 lines (39 deleted, 21 added)
- All 298 tests passing
- Consistent pattern with CreateOrUpdate
- Better maintainability and readability
🔍 Before vs After:
Before (complex dual-path):
```java
public class RunWithCriteria<T> {
private DetachedCriteria detachedCriteria;
private QuerySpec<?, ?> querySpec; // ❌ Wildcards lose type safety
private Function<DetachedCriteria, T> handler;
private Supplier<T> querySpecHandler;
public T apply(Session session) {
if (detachedCriteria != null && handler != null) {
return handler.apply(detachedCriteria);
}
if (querySpec != null && querySpecHandler != null) {
return querySpecHandler.get();
}
throw new IllegalStateException("...");
}
}
```
After (simple generic):
```java
public class RunWithCriteria<T, C> {
@nonnull private C criteria; // ✅ Type-safe
@nonnull private Function<C, T> handler;
public T apply(Session session) {
return handler.apply(criteria);
}
}
```
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace raw List types with parameterized List<T> types in QuerySpec-based run() methods, improving type safety and eliminating @SuppressWarnings annotations. Changes: - MultiTenantRelationalDao: * run(QuerySpec): Map<Integer, List> → Map<Integer, List<T>> * run(QuerySpec, translator): Function<Map<Integer, List>, U> → Function<Map<Integer, List<T>>, U> * RelationalDaoPriv.run(QuerySpec): List → List<T> * OpContext<List> → OpContext<List<T>> in RunWithCriteria builder - RelationalDao: * run(QuerySpec): Map<Integer, List> → Map<Integer, List<T>> * run(QuerySpec, translator): Function<Map<Integer, List>, U> → Function<Map<Integer, List<T>>, U> Benefits: - Better compile-time type checking - No @SuppressWarnings("rawtypes") needed - More explicit API contracts - Consistent with generic best practices All 298 tests passing ✅ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.