feat: add morphium-jakarta-data as optional module - #16
Conversation
Extend release.sh to handle morphium-jakarta-data alongside morphium-core and poppydb in the Sonatype Central bundle. Replaces the previous per-module copy-paste blocks with a small module registry (MODULE_DIRS/MODULE_ARTIFACT_IDS/MODULE_EXTRA_CLASSIFIERS parallel arrays, bash 3.2 compatible) and a shared add_module_to_bundle() helper, since the blocks were structurally identical and copy-paste would not scale to the further modules coming in M4/M5. Version-sync checks, backup cleanup, rollback, structure verification, bundle assembly and the artifact verification loop all now iterate over the registry instead of naming morphium-core/poppydb explicitly. Verified: bash -n passes, shellcheck shows no new findings (all 3 remaining findings are on pre-existing lines), and 'mvn -pl morphium-jakarta-data package source:jar javadoc:jar' produces the expected jar/sources.jar/javadoc.jar triple.
📝 WalkthroughWalkthroughThe PR adds the optional ChangesJakarta Data extension
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Repository as Jakarta Data repository
participant Bridge as QueryMethodBridge
participant Parser as MethodNameParser
participant Executor as QueryExecutor
participant Morphium as Morphium
Repository->>Bridge: invoke derived query
Bridge->>Parser: parse method name
Parser-->>Bridge: QueryDescriptor
Bridge->>Executor: execute descriptor
Executor->>Morphium: build and run query
Morphium-->>Executor: query result
Executor-->>Repository: return declared result shape
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces morphium-jakarta-data as a new optional Morphium reactor module implementing a framework-agnostic Jakarta Data 1.0 repository runtime, and updates build/release/documentation to integrate it without impacting morphium-core consumers.
Changes:
- Adds the new
morphium-jakarta-datamodule (runtime bridges, query derivation, JDQL parsing, paging/sorting) plus unit tests. - Registers the module behind a default-on Maven profile (
extensions) and updatesrelease.shto bundle modules via a registry-driven loop. - Extends docs and changelogs to document the new optional extension module.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| release.sh | Refactors bundling/signing to a module registry to include new module artifacts. |
| pom.xml | Adds default-on extensions profile and manages jakarta.data-api version/dependency. |
| mkdocs.yml | Adds navigation entry for the new Jakarta Data docs page. |
| docs/index.md | Adds an “Extensions (Optional Modules)” section referencing Jakarta Data. |
| docs/jakarta-data.md | Adds comprehensive user documentation for the new Jakarta Data runtime module. |
| CHANGELOG.md | Adds release notes entry announcing the new optional Jakarta Data module. |
| morphium-jakarta-data/pom.xml | New module POM with Morphium + Jakarta Data API dependencies and test setup. |
| morphium-jakarta-data/README.md | New module README describing purpose, scope, and usage patterns. |
| morphium-jakarta-data/CHANGELOG.md | New module changelog documenting integration into the reactor/lockstep versioning. |
| morphium-jakarta-data/src/main/java/de/caluga/morphium/data/*.java | New runtime implementation (repositories, bridges, parsers, pagination/sorting helpers). |
| morphium-jakarta-data/src/test/java/de/caluga/morphium/data/*.java | New unit tests covering method-name parsing, JDQL parsing, and alias-aware query building. |
Suppressed comments (1)
morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java:272
- splitOnCombinator uses the same lowercase-before heuristic as containsCombinator, which prevents splitting for acronym/digit-ending field names and makes derived query parsing incorrect for such methods.
char before = text.charAt(idx - 1);
char after = text.charAt(idx + combinator.length());
if (Character.isLowerCase(before) && Character.isUpperCase(after)) {
result.add(text.substring(start, idx));
start = idx + combinator.length();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case LIKE -> { | ||
| String pattern = value.toString() | ||
| .replace("%", ".*") | ||
| .replace("_", "."); | ||
| if (cond.negated()) { | ||
| // NOT LIKE → $not with $regex | ||
| field.not(); | ||
| field.matches(Pattern.compile(pattern)); | ||
| } else { | ||
| field.matches(Pattern.compile(pattern)); | ||
| } | ||
| } |
| char before = text.charAt(idx - 1); | ||
| char after = text.charAt(idx + combinator.length()); | ||
| if (Character.isLowerCase(before) && Character.isUpperCase(after)) { | ||
| return true; | ||
| } |
| // Split on Asc/Desc boundaries while keeping the direction | ||
| // e.g. "PriceDescNameAsc" -> [Price,Desc], [Name,Asc] | ||
| // Pattern: field name followed by optional Asc/Desc | ||
| Pattern p = Pattern.compile("([A-Z][a-z0-9]*(?:[A-Z][a-z0-9]*)*?)(Asc|Desc)?(?=(?:[A-Z])|$)"); |
| MODULE_DIRS=(morphium-core poppydb morphium-jakarta-data) | ||
| MODULE_ARTIFACT_IDS=(morphium poppydb morphium-jakarta-data) | ||
| MODULE_EXTRA_CLASSIFIERS=("" "cli" "") | ||
|
|
| private static boolean containsTopLevelOr(String wherePart) { | ||
| String upper = wherePart.toUpperCase(Locale.ROOT); | ||
| int depth = 0; | ||
| for (int i = 0; i < upper.length(); i++) { | ||
| char c = upper.charAt(i); | ||
| if (c == '(') depth++; | ||
| else if (c == ')') depth--; | ||
| else if (depth == 0 && i + 4 <= upper.length() | ||
| && upper.startsWith(" OR ", i)) { | ||
| return true; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdc468ce6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public Object doUpdate(Object entity) { | ||
| morphium.store(entity); | ||
| return entity; |
There was a problem hiding this comment.
When callers invoke CrudRepository.update(...) for an entity id that is not already present, this path calls morphium.store(entity). In Morphium, store ultimately uses the store command with upsert=true, so an update-only repository method silently inserts a new document instead of reporting the missing/optimistic-locking failure that callers expect from update (and the same issue applies to doUpdateAll below). Consider checking that each id exists or using a non-upsert update path for these methods.
Useful? React with 👍 / 👎.
| // Split ORDER BY from WHERE (case-insensitive) | ||
| private static final Pattern ORDER_BY_SPLIT = Pattern.compile( |
There was a problem hiding this comment.
This splitter only matches when some text precedes ORDER BY, so a valid all-rows sorted query such as @Query("ORDER BY name ASC") or @Query("SELECT name FROM Product ORDER BY name ASC") falls through to condition parsing and fails with Cannot parse JDQL condition: ORDER BY .... Since the documented grammar makes WHERE optional before ORDER BY, handle a leading ORDER BY clause instead of requiring a non-empty prefix.
Useful? React with 👍 / 👎.
| String orderBySpec, JdqlQuery jdqlQuery, | ||
| Map<String, Object> paramValues, | ||
| Morphium morphium, Class entityClass) { | ||
| List<CursorHelper.SortSpec> sortSpecs = CursorHelper.parseSortSpecs(orderBySpec); |
There was a problem hiding this comment.
For an @Query method returning CursoredPage where the sort is expressed in the JDQL string (... ORDER BY createdAt), orderBySpec is only the separate @OrderBy annotation spec, so sortSpecs is empty here. The following CursorHelper.applySort then replaces the already-applied JDQL sort with an empty sort, and cursor extraction/next-page predicates have no key fields, causing unordered or repeated cursor pages. Build the cursor sort specs from jdqlQuery.orderBy() (merged with any annotation order) before applying cursor logic.
Useful? React with 👍 / 👎.
| String conditionsSpec, String orderBySpec, | ||
| Morphium morphium, Class entityClass, | ||
| Object[] args) { | ||
| List<CursorHelper.SortSpec> sortSpecs = CursorHelper.parseSortSpecs(orderBySpec); |
There was a problem hiding this comment.
When a @Find method returns CursoredPage and receives a runtime Sort or Order parameter, executeFind applies that dynamic sort before entering this branch, but this method rebuilds cursor keys only from the static orderBySpec and then calls CursorHelper.applySort, which replaces the query sort. In that scenario the requested sort is ignored and cursor values/predicates are based on an empty or stale keyset, so next/previous pages can repeat or skip rows; pass the dynamic sort specs into the cursored branch or reject unsupported dynamic cursor sorting.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (15)
morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlMethodBridge.java (2)
639-663: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist the aggregate pattern into a static constant.
Both methods compile the same literal pattern on every call, once per HAVING condition and once per ORDER BY spec. The identical pattern already exists as
AGGREGATE_PATTERNinmorphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.java(lines 66-67). Declare one package-visible constant and reuse it in both classes.♻️ Proposed refactor
public final class JdqlMethodBridge { private static final ConcurrentHashMap<String, JdqlQuery> CACHE = new ConcurrentHashMap<>(); + + private static final Pattern AGG_REF = Pattern.compile( + "(?i)(COUNT|SUM|AVG|MIN|MAX)\\s*\\(\\s*([a-zA-Z_][a-zA-Z0-9_.]*|this)\\s*\\)");private static String resolveAggFieldForHaving(String aggFuncStr, JdqlQuery query) { - Matcher aggMatcher = Pattern.compile( - "(?i)(COUNT|SUM|AVG|MIN|MAX)\\s*\\(\\s*([a-zA-Z_][a-zA-Z0-9_.]*|this)\\s*\\)") - .matcher(aggFuncStr); + Matcher aggMatcher = AGG_REF.matcher(aggFuncStr);private static String resolveAggSortKey(String orderField, JdqlQuery query, Morphium morphium, Class entityClass) { // Check if it's an aggregate function reference like "COUNT(this)" - Matcher aggMatcher = Pattern.compile( - "(?i)(COUNT|SUM|AVG|MIN|MAX)\\s*\\(\\s*([a-zA-Z_][a-zA-Z0-9_.]*|this)\\s*\\)") - .matcher(orderField); + Matcher aggMatcher = AGG_REF.matcher(orderField);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlMethodBridge.java` around lines 639 - 663, Hoist the shared aggregate regex into a single package-visible constant and reuse it instead of recompiling the same literal in JdqlMethodBridge. Update resolveAggFieldForHaving and resolveAggSortKey to reference the existing AGGREGATE_PATTERN symbol from JdqlParser (or an equivalent shared constant), keeping the matching behavior unchanged while removing the duplicated Pattern.compile calls.
574-595: 🚀 Performance & Scalability | 🔵 TrivialConsider server-side paging for grouped results.
agg.aggregateMap()materializes every group, then lines 587-591 slice the list in the JVM. Memory and latency grow with group cardinality, not with page size. For high-cardinality GROUP BY fields, push$skipand$limitinto the pipeline after$sort, and obtain the total with a separate$countfacet only whenpageRequest.requestTotal()is true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlMethodBridge.java` around lines 574 - 595, Update the grouped-results path in JdqlMethodBridge around agg.aggregateMap() to apply pageRequest paging in the aggregation pipeline, placing $skip and $limit after the grouped-result sort instead of slicing allMapped in memory. When requestTotal() is true, add a separate $count facet or equivalent total-count pipeline; otherwise avoid counting. Preserve existing mapping and MorphiumPage behavior while preventing full group materialization for paged requests.morphium-jakarta-data/src/test/java/de/caluga/morphium/data/JdqlParserTest.java (1)
416-462: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for HAVING without GROUP BY.
The error-message tests cover a malformed condition and a malformed HAVING body inside a GROUP BY query. They do not cover
SELECT COUNT(this) FROM Entity HAVING COUNT(this) > 1, which currently parses into a condition on the field nameHAVING COUNT(this)instead of failing. See the issue raised onmorphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.javalines 226-245.Other uncovered clauses: SELECT aggregate classification,
IN/NOT IN,LIKE, and ORDER BY direction parsing.Do you want me to add these test cases?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/test/java/de/caluga/morphium/data/JdqlParserTest.java` around lines 416 - 462, Add a parser test in JdqlParserTest that exercises SELECT COUNT(this) FROM Entity HAVING COUNT(this) > 1 without a GROUP BY clause and asserts JdqlParser.parse throws IllegalArgumentException with the expected JDQL parse error. Keep the test in the existing ErrorMessageTests nested class, and target the parser behavior around HAVING handling rather than the malformed-body cases already covered.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.java (1)
532-595: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that string literals must not contain JDQL keywords.
containsTopLevelOrandsplitTopLeveltrack parenthesis depth only. They do not skip quoted literals.WHERE name = 'BOB AND SON'therefore splits inside the literal, andparseConditionreceives fragments that parse into a different condition. The same applies toORandINinside literals.Full tokenization is out of scope for this PR. State the restriction next to the "Not supported" line in the class javadoc so callers know to bind such values with parameters.
♻️ Proposed documentation change
- * Not supported: JOINs, subqueries. + * Not supported: JOINs, subqueries. + * <p> + * Restriction: quoted string literals must not contain the keywords {`@code` AND}, {`@code` OR}, + * {`@code` IN}, {`@code` LIKE} or {`@code` BETWEEN}. Clause splitting is parenthesis-aware but not + * literal-aware. Bind such values with named parameters instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.java` around lines 532 - 595, Update the JdqlParser class javadoc at the existing “Not supported” note to explicitly state that quoted string literals must not contain JDQL keywords such as AND, OR, or IN. Keep the change documentation-only, and place the restriction alongside the current unsupported-feature guidance so callers know to use parameters instead of embedding such values directly in wherePart.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.java (2)
146-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the count-query construction.
Lines 149-155 and Lines 226-232 build the same count query from
conditionsSpecandargs. Extract one private helper and call it from both branches, so the condition decoding stays in one place.♻️ Proposed helper
`@SuppressWarnings`({"unchecked", "rawtypes"}) private static Query buildCountQuery(String conditionsSpec, Object[] args, Morphium morphium, Class entityClass) { Query countQuery = morphium.createQueryFor(entityClass); if (!conditionsSpec.isEmpty()) { for (String p : conditionsSpec.split(",")) { String[] fieldAndIdx = p.split(":"); String mongoField = resolveMongoField(morphium, entityClass, fieldAndIdx[0]); countQuery.f(mongoField).eq(args[Integer.parseInt(fieldAndIdx[1])]); } } return countQuery; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.java` around lines 146 - 157, Extract the duplicated count-query construction from the requestTotal branch and its counterpart around the second count-query block into one private buildCountQuery helper. Have the helper create the query, decode conditionsSpec, resolve fields, and apply args, then replace both inline implementations with calls to it while preserving existing behavior and generic suppression requirements.
273-277: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid loading the full result set for an annotated delete.
query.asList()loads every matching entity into memory, then the loop issues one delete per entity. For a large match set this causes high memory use and N round trips.QueryExecutoruses bulkquery.delete()for deriveddeleteBy*methods, so the two delete paths behave differently.Choose one behavior and document it. If lifecycle callbacks are not required, use
query.delete(). If callbacks are required, iterate in batches instead of materializing the whole result set. The specification also allows@Deletemethods to returnintorlong. For methods annotated with@Delete, the return type must be one of void, int, or long. Avoid-only bridge cannot serve the counting variants.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.java` around lines 273 - 277, Update the annotated-delete handling in FindMethodBridge so `@Delete` methods accept only void, int, or long return types and support the declared count for int/long results. Avoid query.asList() materialization: use query.delete() when lifecycle callbacks are not required, or process matching entities in batches when callbacks must run, preserving the appropriate deletion count and documenting the chosen behavior.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/SortMapper.java (1)
22-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
SortMapperfrom the other call sites, or remove it.The javadoc states that
FindMethodBridge,JdqlMethodBridge, andAbstractMorphiumRepositoryinline the same mapping instead of calling this class. That leavesapplywith no production caller, andresolveMongoFieldis now duplicated inSortMapper,CursorHelper,FindMethodBridge, andAbstractMorphiumRepository.Make
SortMapperthe single owner of Jakarta DataOrder/Sortmapping and of field-name resolution. Add a publicresolveMongoField(Morphium, Class<?>, String)plus anapply(Query, Sort, ...)overload, and call them from the bridges and the repository base class. If you prefer to keep the inlined code, delete this class to avoid an unused parallel implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/SortMapper.java` around lines 22 - 53, Make SortMapper the single implementation for Jakarta Data sort mapping: expose resolveMongoField(Morphium, Class<?>, String), add an apply(Query, Sort, Morphium, Class<?>) overload, and update FindMethodBridge, JdqlMethodBridge, CursorHelper, and AbstractMorphiumRepository to delegate their field resolution and order/sort application to it. Remove duplicated inline implementations while preserving existing sort behavior; alternatively delete SortMapper if all call sites remain inline.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java (2)
126-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the abandoned regex and the stale comment.
Line 127 compiles a
Patternthat the method never uses. The comment on line 129 describes it as a superseded approach. Delete both so the token-based implementation is the only documented path.♻️ Proposed cleanup
// Split on Asc/Desc boundaries while keeping the direction // e.g. "PriceDescNameAsc" -> [Price,Desc], [Name,Asc] - // Pattern: field name followed by optional Asc/Desc - Pattern p = Pattern.compile("([A-Z][a-z0-9]*(?:[A-Z][a-z0-9]*)*?)(Asc|Desc)?(?=(?:[A-Z])|$)"); - - // Simpler approach: split tokens List<String> tokens = splitCamelCase(orderPart);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java` around lines 126 - 130, Remove the unused Pattern declaration and its stale regex-approach comment from the method containing splitCamelCase(orderPart), leaving the token-based implementation as the only path.
130-148: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winApply
resolveFieldNameto OrderBy fields too.
parseConditionresolves each condition field throughresolveFieldName, which performs an exact then case-insensitive lookup againstentityFields.parseOrderBybuilds the field name from raw tokens and skips that step.The two paths therefore disagree. A field that a condition resolves case-insensitively does not resolve in an
OrderBysuffix. Acronym fields are also affected, becausesplitCamelCasesplits before every uppercase character and the rebuilt name differs from the declared field.Pass
entityFieldsintoparseOrderByand resolve each field name the same way.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java` around lines 130 - 148, Update parseOrderBy to accept entityFields and apply resolveFieldName to each assembled OrderSpec field before adding it, matching parseCondition’s exact and case-insensitive resolution. Update the caller and any related signatures to pass entityFields, while preserving the existing direction parsing for Asc and Desc.pom.xml (1)
406-412: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
-DskipExtensions=falsealso disables the extensions.Maven property activation with
!skipExtensionstests only whether the property is defined. If a user passes-DskipExtensions=false, the profile deactivates, and the extension modules are skipped. Document this in the comment block, or activate on an explicit value pair to make the intent unambiguous.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pom.xml` around lines 406 - 412, The extensions profile activation using !skipExtensions incorrectly treats -DskipExtensions=false as disabling the profile; update the activation in the extensions profile to explicitly recognize the intended property value, or document this behavior in the surrounding comment block.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java (3)
168-172: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLog the field-resolution fallback.
resolveMongoFieldswallows the exception and returns the Java field name.resolveAliaseslogs the same class of failure at trace level on line 252.The fallback here is the more consequential one. If the entity has no such field, the query is built against an unmapped name and matches nothing, with no diagnostic in the logs. Add a trace or debug log with the field name, the entity class, and the cause.
♻️ Proposed fix
try { return morphium.getARHelper().getMongoFieldName(entityClass, javaFieldName); } catch (Exception e) { + log.debug("Could not resolve mongo field name for '{}' on {}; using the java name", + javaFieldName, entityClass.getSimpleName(), e); return javaFieldName; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java` around lines 168 - 172, Update resolveMongoField’s exception fallback to emit a trace or debug log before returning javaFieldName, including the field name, entity class, and caught exception as the cause; preserve the existing fallback return behavior.
189-226: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a
defaultbranch so a new operator cannot silently widen the filter.The switch covers all 22 current
Operatorconstants, so the returned map is never empty today. The switch is a statement switch with nodefault, so it is not exhaustiveness-checked.If a later change adds an operator and misses this switch,
buildRawConditionreturns an empty map.addRawConditionToQuerythen adds no filter, and the query matches every document. A silently widened filter is difficult to detect.Throw on an unhandled operator.
♻️ Proposed fix
case SIZE -> result.put(fieldName, Map.of("$size", ((Number) args[cond.paramIndex()]).intValue())); + default -> throw new IllegalStateException( + "Unhandled operator in buildRawCondition: " + cond.operator()); } return result;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java` around lines 189 - 226, Add a default branch to the operator switch in buildRawCondition that throws an appropriate exception for any unhandled operator. Preserve all existing operator mappings while ensuring future Operator additions cannot return an empty condition and widen the query.
67-67: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a limited single-document probe for exists checks.
query.countAll() > 0reads every matching document for a boolean result.Query.get()andlimit(int)are available; limit the query before checking existence so this branch does not needlessly scan matching documents.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java` at line 67, Update the EXISTS branch in QueryExecutor to apply limit(1) to the query and then use Query.get() to determine whether a matching document exists, replacing the full countAll() scan while preserving the boolean result.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MorphiumPage.java (1)
35-47: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDefensively copy or validate
contentinMorphiumPage.
MorphiumPagestores the supplied list directly, socontent(),hasContent(),numberOfElements(), anditerator()can observe mutations from caller-provided content. Guard againstnullbeforenumberOfElements()/hasNext()and either copy the list on construction or reject it explicitly when immutable/pagination state must be guaranteed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MorphiumPage.java` around lines 35 - 47, Update the MorphiumPage constructor to reject null content and defensively copy the supplied list before assigning the content field, ensuring content(), hasContent(), numberOfElements(), and iterator() observe stable page data while preserving the existing pagination state.morphium-jakarta-data/src/test/java/de/caluga/morphium/data/QueryExecutorAliasTest.java (1)
56-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplicitly enable camelCase conversion in
setUp().
ObjectMappingSettingsenables camelCase-to-snake_case conversion by default, and this test assertsota_update_id,campaign_number, andota_upload_date. Addcfg.objectMappingSettings().enableCamelCaseConversion();so the setup records the naming-strategy assumption.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/test/java/de/caluga/morphium/data/QueryExecutorAliasTest.java` around lines 56 - 63, Update the QueryExecutorAliasTest setUp method to explicitly call enableCamelCaseConversion on cfg.objectMappingSettings() before constructing Morphium, preserving the naming strategy required by the asserted snake_case field names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/jakarta-data.md`:
- Around line 28-38: Update the three documented integration mentions so they no
longer present quarkus-morphium and spring-boot-morphium as already available
setup paths: in docs/jakarta-data.md at the intro note, mark those integrations
as planned/future until their artifacts ship; in docs/jakarta-data.md at the
wiring section, present the DI/container details as future integration guidance
rather than current instructions; and in morphium-jakarta-data/README.md, remove
or qualify the recommendation to use those modules now by stating their
availability is conditional on the corresponding artifacts being released.
- Around line 56-66: The dependency examples in docs/jakarta-data.md lines 56-66
and morphium-jakarta-data/README.md lines 38-46 must use a published Morphium
version or documented project property instead of ${project.version}. Update
morphium-jakarta-data/CHANGELOG.md lines 11-12 to match the root pom.xml
version, 6.3.0-SNAPSHOT, and remove the stale archived standalone repository
wording there.
In `@morphium-jakarta-data/README.md`:
- Around line 112-115: Update the PersonRepositoryImpl findAll() example to
return Stream<Person> instead of List<Person>, preserving the doFindAll() result
flow with the appropriate stream return. Add the omitted java.util.stream.Stream
import so the method correctly overrides the Jakarta Data contract.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/AbstractMorphiumRepository.java`:
- Around line 216-225: Add offset pagination to both
AbstractMorphiumRepository.doFindAllCursored and
FindMethodBridge.executeCursoredFind: when PageRequest.Mode is OFFSET, apply
skip((page - 1) * size) before query.limit(requestedSize + 1); retain
cursor-condition handling for non-OFFSET modes and existing sorting/limit
behavior.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/CursorHelper.java`:
- Around line 111-141: Require applyCursorCondition(...) in CursorHelper to
validate that sortSpecs is non-empty and that the cursor contains exactly one
value per sort specification before calling query.or(...), rejecting invalid
input. Apply this shared validation at CursorHelper.java:111-141;
AbstractMorphiumRepository.java:203-212 and FindMethodBridge.java:193-201
require no direct changes because they should rely on the shared validation.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.java`:
- Around line 124-128: Reject methods that provide both a non-null Limit and a
PageRequest instead of applying both pagination paths. Update the logic around
limitParamIndex and pageRequestParamIndex in FindMethodBridge to throw
UnsupportedOperationException before calling query.skip or query.limit,
preserving existing behavior when only one pagination parameter is present.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlMethodBridge.java`:
- Around line 693-698: Update the class-loading logic in JdqlMethodBridge to
handle a null Thread.currentThread().getContextClassLoader() by falling back to
the bridge class loader before loading resultRecordClass. Preserve the existing
IllegalArgumentException for ClassNotFoundException.
- Around line 774-783: Update toNumber so JdqlQuery.AggregateType.AVG always
converts numeric results with doubleValue(), including Integer and Long inputs,
while preserving COUNT’s long conversion and existing handling for other
aggregate types.
- Around line 235-244: Update the cursor-page flag calculation in the method
containing isFirstPage, isLastPage, and the CursoredPageRecord constructors to
account for traversal direction via isForward. Map hasMore to the forward
boundary when traversing forward and to the opposite boundary when traversing
backward, while preserving OFFSET behavior and the empty-content return path so
nextPageRequest and previousPageRequest receive cursors at the correct limits.
- Around line 354-364: Update the LIKE handling in JdqlMethodBridge to convert
parameters through a dedicated likeToRegex helper: anchor the result with ^ and
$, translate only % and _ as wildcards, and escape every other regex
metacharacter. Handle null parameters without calling value.toString() directly,
while preserving the existing negated and non-negated field matching behavior.
In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.java`:
- Around line 513-528: Update parseOrderBy to validate the optional direction
token: accept only ASC or DESC (case-insensitively), preserve ascending as the
default when omitted, and reject any other token instead of silently treating it
as ascending. Ensure the existing OrderSpec construction receives the validated
direction.
- Around line 226-245: Update the no-GROUP-BY branch in JdqlParser so it detects
a HAVING clause and throws the same parse error before HAVING text can remain in
wherePart; remove the unreachable groupByFields null/empty check from the
groupByFields != null branch. Preserve the existing aggregate/projection
validation and use the parser’s existing HAVING representation or detection
logic.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java`:
- Around line 230-245: Make entityFields authoritative throughout
MethodNameParser: update resolveFieldName to throw IllegalArgumentException when
supplied fields have no exact or case-insensitive match; in parseCondition,
validate the decapitalized whole part before OPERATOR_MATCHES so fields named
size, like, in, or not are preserved; pass entityFields through parseOrderBy and
resolve sort fields consistently. In QueryMethodBridge, supply the entity field
names instead of null when calling parse so cached descriptors validate method
names early. Affected sites: MethodNameParser.java lines 230-245, 157-185, and
130-148 require the described parser changes; QueryMethodBridge also requires
the entity-field input change.
- Around line 97-115: Update the combinator handling in MethodNameParser so
method names containing both “And” and “Or” are detected before splitting and
rejected with a clear parse-time error. Preserve the existing behavior for
expressions using only one combinator, and ensure the failure occurs before
parseCondition processes any parts.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java`:
- Around line 210-211: Update the CONTAINS and NOT_CONTAINS handling in
QueryExecutor so String fields use substring-matching regular expressions, while
preserving MongoDB’s existing element semantics for collection fields. Resolve
the field type before constructing the filter and emit the corresponding
positive or negated $regex; alternatively, explicitly reject String usage and
document collection-only behavior as requested.
- Around line 68-76: Update the DELETE branch in QueryExecutor’s derived-delete
handling to return the count produced by query.delete() rather than calling
query.countAll() beforehand. Use the delete operation’s returned value as the
long result, preserving the existing bulk-delete behavior and lifecycle-callback
semantics.
- Around line 115-128: Update the alias-handling branch in QueryExecutor to
combine conditions with $and for negating operators (including NotEquals, NIN,
IS_NOT_NULL, NOT_CONTAINS, and IS_NOT_EMPTY), while retaining $or for positive
operators. Extend QueryExecutorAliasTest with coverage for a negating operator
on an aliased field, preserving the expected logical-field behavior when the
alternate name is missing.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryMethodBridge.java`:
- Around line 76-90: Update QueryMethodBridge’s static-order handling around
CACHE.computeIfAbsent and parseOrderBySpec so Jakarta Data `@OrderBy` attributes
are preserved, including descending and repeatable declarations, and reject any
ignoreCase usage if Morphium cannot support it. Also make the callsite
generation carry the full static order specification instead of only
field[:ASC|DESC], and add validation in the QueryDescriptor merge path to reject
mixed static-order declarations when both `@OrderBy` and a name-derived OrderBy*
suffix are present.
---
Nitpick comments:
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.java`:
- Around line 146-157: Extract the duplicated count-query construction from the
requestTotal branch and its counterpart around the second count-query block into
one private buildCountQuery helper. Have the helper create the query, decode
conditionsSpec, resolve fields, and apply args, then replace both inline
implementations with calls to it while preserving existing behavior and generic
suppression requirements.
- Around line 273-277: Update the annotated-delete handling in FindMethodBridge
so `@Delete` methods accept only void, int, or long return types and support the
declared count for int/long results. Avoid query.asList() materialization: use
query.delete() when lifecycle callbacks are not required, or process matching
entities in batches when callbacks must run, preserving the appropriate deletion
count and documenting the chosen behavior.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlMethodBridge.java`:
- Around line 639-663: Hoist the shared aggregate regex into a single
package-visible constant and reuse it instead of recompiling the same literal in
JdqlMethodBridge. Update resolveAggFieldForHaving and resolveAggSortKey to
reference the existing AGGREGATE_PATTERN symbol from JdqlParser (or an
equivalent shared constant), keeping the matching behavior unchanged while
removing the duplicated Pattern.compile calls.
- Around line 574-595: Update the grouped-results path in JdqlMethodBridge
around agg.aggregateMap() to apply pageRequest paging in the aggregation
pipeline, placing $skip and $limit after the grouped-result sort instead of
slicing allMapped in memory. When requestTotal() is true, add a separate $count
facet or equivalent total-count pipeline; otherwise avoid counting. Preserve
existing mapping and MorphiumPage behavior while preventing full group
materialization for paged requests.
In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.java`:
- Around line 532-595: Update the JdqlParser class javadoc at the existing “Not
supported” note to explicitly state that quoted string literals must not contain
JDQL keywords such as AND, OR, or IN. Keep the change documentation-only, and
place the restriction alongside the current unsupported-feature guidance so
callers know to use parameters instead of embedding such values directly in
wherePart.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java`:
- Around line 126-130: Remove the unused Pattern declaration and its stale
regex-approach comment from the method containing splitCamelCase(orderPart),
leaving the token-based implementation as the only path.
- Around line 130-148: Update parseOrderBy to accept entityFields and apply
resolveFieldName to each assembled OrderSpec field before adding it, matching
parseCondition’s exact and case-insensitive resolution. Update the caller and
any related signatures to pass entityFields, while preserving the existing
direction parsing for Asc and Desc.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MorphiumPage.java`:
- Around line 35-47: Update the MorphiumPage constructor to reject null content
and defensively copy the supplied list before assigning the content field,
ensuring content(), hasContent(), numberOfElements(), and iterator() observe
stable page data while preserving the existing pagination state.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java`:
- Around line 168-172: Update resolveMongoField’s exception fallback to emit a
trace or debug log before returning javaFieldName, including the field name,
entity class, and caught exception as the cause; preserve the existing fallback
return behavior.
- Around line 189-226: Add a default branch to the operator switch in
buildRawCondition that throws an appropriate exception for any unhandled
operator. Preserve all existing operator mappings while ensuring future Operator
additions cannot return an empty condition and widen the query.
- Line 67: Update the EXISTS branch in QueryExecutor to apply limit(1) to the
query and then use Query.get() to determine whether a matching document exists,
replacing the full countAll() scan while preserving the boolean result.
In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/SortMapper.java`:
- Around line 22-53: Make SortMapper the single implementation for Jakarta Data
sort mapping: expose resolveMongoField(Morphium, Class<?>, String), add an
apply(Query, Sort, Morphium, Class<?>) overload, and update FindMethodBridge,
JdqlMethodBridge, CursorHelper, and AbstractMorphiumRepository to delegate their
field resolution and order/sort application to it. Remove duplicated inline
implementations while preserving existing sort behavior; alternatively delete
SortMapper if all call sites remain inline.
In
`@morphium-jakarta-data/src/test/java/de/caluga/morphium/data/JdqlParserTest.java`:
- Around line 416-462: Add a parser test in JdqlParserTest that exercises SELECT
COUNT(this) FROM Entity HAVING COUNT(this) > 1 without a GROUP BY clause and
asserts JdqlParser.parse throws IllegalArgumentException with the expected JDQL
parse error. Keep the test in the existing ErrorMessageTests nested class, and
target the parser behavior around HAVING handling rather than the malformed-body
cases already covered.
In
`@morphium-jakarta-data/src/test/java/de/caluga/morphium/data/QueryExecutorAliasTest.java`:
- Around line 56-63: Update the QueryExecutorAliasTest setUp method to
explicitly call enableCamelCaseConversion on cfg.objectMappingSettings() before
constructing Morphium, preserving the naming strategy required by the asserted
snake_case field names.
In `@pom.xml`:
- Around line 406-412: The extensions profile activation using !skipExtensions
incorrectly treats -DskipExtensions=false as disabling the profile; update the
activation in the extensions profile to explicitly recognize the intended
property value, or document this behavior in the surrounding comment block.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a3427599-2cbf-4bc9-a2b3-c15994171ae0
📒 Files selected for processing (27)
CHANGELOG.mddocs/index.mddocs/jakarta-data.mdmkdocs.ymlmorphium-jakarta-data/CHANGELOG.mdmorphium-jakarta-data/README.mdmorphium-jakarta-data/pom.xmlmorphium-jakarta-data/src/main/java/de/caluga/morphium/data/AbstractMorphiumRepository.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/CursorHelper.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlMethodBridge.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlQuery.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/MorphiumPage.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/MorphiumRepository.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryDescriptor.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryMethodBridge.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryResultHelper.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/RepositoryMetadata.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/SortMapper.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/JdqlParserTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/MethodNameParserTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/QueryExecutorAliasTest.javapom.xmlrelease.sh
| !!! note "Applications typically don't add this module directly" | ||
| Applications normally consume Jakarta Data through a full framework integration: | ||
| **quarkus-morphium** (Gizmo bytecode generation at build time) or | ||
| **spring-boot-morphium** (JDK dynamic proxies at runtime). Those modules pull in | ||
| `morphium-jakarta-data` transitively and wire the generated/proxied repositories | ||
| into their respective dependency-injection containers. | ||
|
|
||
| `morphium-jakarta-data` is directly relevant to you if you are **building your own | ||
| framework integration** — for a DI container or framework not already covered by | ||
| the two integrations above. See [Building your own framework integration](#building-your-own-framework-integration) | ||
| below. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not document unshipped framework integrations as available.
CHANGELOG.md says the Quarkus and Spring Boot integrations will follow in later PRs. The two module documents currently present those integrations as available setup paths.
docs/jakarta-data.md#L28-L38: Mark the integrations as planned until their artifacts ship.docs/jakarta-data.md#L479-L487: Mark the wiring details as future integration guidance.morphium-jakarta-data/README.md#L9-L18: Remove the instruction to use those modules now, or state their availability condition.
📍 Affects 2 files
docs/jakarta-data.md#L28-L38(this comment)docs/jakarta-data.md#L479-L487morphium-jakarta-data/README.md#L9-L18
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/jakarta-data.md` around lines 28 - 38, Update the three documented
integration mentions so they no longer present quarkus-morphium and
spring-boot-morphium as already available setup paths: in docs/jakarta-data.md
at the intro note, mark those integrations as planned/future until their
artifacts ship; in docs/jakarta-data.md at the wiring section, present the
DI/container details as future integration guidance rather than current
instructions; and in morphium-jakarta-data/README.md, remove or qualify the
recommendation to use those modules now by stating their availability is
conditional on the corresponding artifacts being released.
| ```xml | ||
| <dependency> | ||
| <groupId>de.caluga</groupId> | ||
| <artifactId>morphium-jakarta-data</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> | ||
| ``` | ||
|
|
||
| In the Morphium reactor, `${project.version}` currently resolves to `6.3.0-SNAPSHOT`. | ||
| This module follows Morphium's regular release versioning; there is no separate version | ||
| line to track. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C2 \
'6\.2\.6-SNAPSHOT|6\.3\.0-SNAPSHOT|<version>|<revision>|project\.version' \
pom.xml \
morphium-jakarta-data/pom.xml \
docs/jakarta-data.md \
morphium-jakarta-data/README.md \
morphium-jakarta-data/CHANGELOG.mdRepository: Bardioc1977/morphium
Length of output: 12059
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- root pom relevant properties/modules ---\n'
sed -n '1,80p' pom.xml
printf '\n--- jakarta-data examples and notes ---\n'
sed -n '50,70p' docs/jakarta-data.md
sed -n '34,47p' morphium-jakarta-data/README.md
sed -n '9,14p' morphium-jakarta-data/CHANGELOG.md
printf '\n--- Maven placeholder resolution probe ---\n'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/consumer/pom.xml" <<'XML'
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>example.consumer</groupId>
<artifactId>consumer</artifactId>
<version>7.0.0</version>
<packaging>pom</packaging>
<properties>
<maven.compiler.source>17</maven.compiler.source>
</properties>
</project>
XML
if command -v mvn >/dev/null 2>&1; then
set -x
tmpmod="$(mktemp -d)"
cd "$tmpmod"
mkdir consumer consumer/deps
sed "s#<version>7.0.0</version>#<version>${tmpdir}/consumer.pom:7.0.0</version>#" "$tmpdir/consumer/pom.xml" > consumer/deps/m2e.pom || true
mvn help:evaluate -Dexpression=project.version -DforceStdout -f "$tmpdir/consumer/pom.xml" | cat -n
cd - >/dev/null
else
echo 'MAVEN_UNAVAILABLE'
fiRepository: Bardioc1977/morphium
Length of output: 249
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- root pom relevant properties/modules ---'
sed -n '1,80p' pom.xml
printf '%s\n' ''
printf '%s\n' '--- jakarta-data examples and notes ---'
sed -n '50,70p' docs/jakarta-data.md
sed -n '34,47p' morphium-jakarta-data/README.md
sed -n '9,14p' morphium-jakarta-data/CHANGELOG.md
printf '%s\n' ''
printf '%s\n' '--- Maven placeholder resolution probe ---'
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
cat > "$tmpdir/consumer/pom.xml" <<'XML'
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>example.consumer</groupId>
<artifactId>consumer</artifactId>
<version>7.0.0</version>
<packaging>pom</packaging>
<properties>
<maven.compiler.source>17</maven.compiler.source>
</properties>
</project>
XML
if command -v mvn >/dev/null 2>&1; then
tmpmod="$(mktemp -d)"
mkdir "$tmpmod/consumer"
sed "s#<version>7.0.0</version>#<version>${tmpdir}/consumer.pom:7.0.0</version>#" "$tmpdir/consumer/pom.xml" > "$tmpmod/consumer/deps/m2e.pom" || true
mvn help:evaluate -Dexpression=project.version -DforceStdout -f "$tmpdir/consumer/pom.xml" | cat -n
else
echo 'MAVEN_UNAVAILABLE'
fiRepository: Bardioc1977/morphium
Length of output: 6011
🌐 Web query:
Maven property ${project.version} consumer POM project version dependency resolution documentation
💡 Result:
In Maven, the ${project.version} property is a standard built-in variable that resolves to the version defined in the current project's POM [1][2]. When a project is built and deployed, Maven distinguishes between the Build POM (the original POM used to build the artifact) and the Consumer POM (a simplified version generated for downstream users) [3][4]. Dependency Resolution and Consumer POMs: 1. Property Transformation: When generating a Consumer POM, Maven can transform property-based versions, such as ${project.version}, into their exact, literal values [3]. This ensures that downstream consumers do not need to resolve properties that were only relevant to the original project's build environment (such as those defined in parent POMs or external property files) [3]. 2. Dependency Consistency: Using ${project.version} for dependencies within a multi-module reactor is a common best practice [5][4]. When the project is deployed, the generated Consumer POM will contain the resolved version, ensuring that the dependency is correctly identified by external tools and other Maven versions that may not have access to the original build context [3][4]. 3. Maven 4 Evolution: Starting with Maven 4, the Consumer POM feature is central to how artifacts are published [4]. It enables backward compatibility, allowing modern features (like the new BOM packaging or flattened dependency management) to be translated into standard POM structures that older Maven versions (e.g., 3.x) can consume reliably [6][4][7]. 4. Best Practices: Always use ${project.version} rather than the deprecated ${version} alias [5][2]. While Maven 3 allowed the ${version} shorthand, Maven 4 strictly rejects it, requiring the explicit ${project.version} property for correct interpolation [5]. In summary, ${project.version} is resolved during the build and deployment process. The Consumer POM acts as an interface that provides a stable, resolved version of the project's metadata to external consumers, shielding them from the complexities of the original build-time property definitions [3].
Citations:
- 1: https://maven.apache.org/pom.html
- 2: https://maven.apache.org/guides/introduction/introduction-to-the-pom
- 3: https://cwiki.apache.org/confluence/display/MAVEN/Build+vs+Consumer+POM?focusedCommentId=406622572
- 4: https://maven.apache.org/guides/mini/guide-migration-to-mvn4.html
- 5: Maven 4 rejects missing dependency version in reactor that Maven 3 accepted apache/maven#12304
- 6: https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html
- 7: https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism
🌐 Web query:
Maven generated consumer POM dependencies versions use ${project.parent.version} ${project.version} property
💡 Result:
In Maven, the distinction between the "build POM" (the one used for development) and the "consumer POM" (the one published to repositories) is designed to handle property interpolation and structural simplification automatically [1][2]. When you use properties like ${project.version} or ${project.parent.version} in your build POM, Maven's consumer POM generation process (or the Flatten Maven Plugin) resolves these placeholders into their concrete, evaluated values before the POM is deployed [1][2][3]. Key aspects of this process include: 1. Automatic Resolution: Maven generates a consumer POM during the build process [2]. A primary goal of this generated POM is to transform internal, property-based versions into absolute values [1][4]. This ensures that consumers of your artifact do not need to possess the same parent POM or property definitions to resolve dependency versions [2][3]. 2. Flatten Maven Plugin: Before Maven's native support for this was refined in later versions, the Flatten Maven Plugin was the standard tool for this task [1]. It allows configuration of how elements are interpolated [5]. Using modes like interpolate or extended_interpolate ensures that variables are replaced with their effective values [5]. 3. Maven 4+ Consumer POMs: Modern Maven (4+) generates a "stripped down" consumer POM by default [2]. This process includes flattening managed dependencies and resolving variables, effectively replacing placeholders like ${project.version} with the project's actual version [2]. You can control this behavior with the user property maven.consumer.pom.flatten [2][6]. 4. Best Practices: If you are encountering issues where consumers cannot resolve these properties, it is standard practice to ensure the artifact you deploy to the repository is the flattened version [3]. Relying on ${project.parent.version} in dependencies (as opposed to defining concrete versions in dependencyManagement) is a common source of resolution errors; using dependencyManagement in a parent POM allows children to inherit versions without needing to explicitly define them using potentially brittle variables [7]. In summary, the generated consumer POM is intended to be self-contained [1][2]. If your published POM still contains ${project.version} instead of the literal version string, it suggests that the consumer POM generation (or flattening process) is not being invoked or is incorrectly configured to preserve those properties [2][3].
Citations:
- 1: https://cwiki.apache.org/confluence/display/MAVEN/Build+vs+Consumer+POM
- 2: https://maven.apache.org/whatsnewinmaven4.html
- 3: https://www.mojohaus.org/flatten-maven-plugin/index.html
- 4: https://cwiki.apache.org/confluence/display/MAVEN/Build+vs+Consumer+POM?focusedCommentId=406622572
- 5: https://www.mojohaus.org/flatten-maven-plugin/apidocs/org/codehaus/mojo/flatten/ElementHandling.html
- 6: Disable consumer POM flattening by default and add an opt-in feature apache/maven#11347
- 7: https://stackoverflow.com/questions/40979198/maven-how-to-read-the-parent-pom-version
🌐 Web query:
"${project.version}" Maven dependency in documentation means resolve to consumer version
💡 Result:
In Maven, the ${project.version} property is a reference to the version element defined in the current project's POM [1][2]. It does not mean "resolve to the consumer version" in a general sense; rather, it instructs Maven to use the version of the current project when resolving that specific dependency [2][3]. Key points regarding its use: 1. Self-Referencing and Siblings: It is most commonly used in multi-module builds to define dependencies on other modules within the same reactor (sibling projects) that share the same version [4][2][3]. By using ${project.version}, you ensure that the dependency version stays synchronized with the project's own version [2][5]. 2. Project Interpolation: Maven processes these variables during "interpolation," where it replaces the placeholder with the actual value defined in the project's model [5][6]. Because this happens during the build lifecycle, it allows for portable POM files where you do not need to hardcode version strings repeatedly [2][5]. 3. Not for "Consumer" Resolution: The property resolves to the version of the project currently being built, not to any external "consumer." If an external project consumes your artifact, it will typically see the resolved version string (e.g., 1.0.0) in the deployed POM, as the variable is interpolated before the artifact is installed or deployed [2][5]. In summary, ${project.version} is a convenience variable that promotes "Don't Repeat Yourself" (DRY) principles by binding a dependency's version to the current project's version [5][7].
Citations:
- 1: https://maven.apache.org/pom.html
- 2: https://www.sonatype.com/maven-complete-reference/properties-and-resource-filtering
- 3: https://cwiki.apache.org/confluence/display/MAVEN/Maven%20Properties%20Guide
- 4: https://maven.apache.org/guides/mini/guide-maven-ci-friendly.html
- 5: https://maven.apache.org/guides/introduction/introduction-to-the-pom
- 6: https://maven.apache.org/ref/3.2.5/maven-model-builder/index.html
- 7: https://maven.apache.org/guides/introduction/introduction-to-the-pom.html
Use a stable Morphium version in the examples.
${project.version} is only appropriate in the module POM’s build context. Use a published Morphium version or a documented Morphium project property in docs/jakarta-data.md and morphium-jakarta-data/README.md; the current examples can mislead consumers into pinning the wrong value. Align morphium-jakarta-data/CHANGELOG.md to the root pom.xml version (6.3.0-SNAPSHOT) instead of 6.2.6-SNAPSHOT, and remove the stale archived standalone repository wording.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 56-56: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
📍 Affects 3 files
docs/jakarta-data.md#L56-L66(this comment)morphium-jakarta-data/README.md#L38-L46morphium-jakarta-data/CHANGELOG.md#L11-L12
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/jakarta-data.md` around lines 56 - 66, The dependency examples in
docs/jakarta-data.md lines 56-66 and morphium-jakarta-data/README.md lines 38-46
must use a published Morphium version or documented project property instead of
${project.version}. Update morphium-jakarta-data/CHANGELOG.md lines 11-12 to
match the root pom.xml version, 6.3.0-SNAPSHOT, and remove the stale archived
standalone repository wording there.
| @Override | ||
| public List<Person> findAll() { | ||
| return doFindAll().toList(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -P -A8 -B4 '\b(findAll|doFindAll)\s*\(' \
morphium-jakarta-data/src/main/java \
docs/jakarta-data.md \
morphium-jakarta-data/README.mdRepository: Bardioc1977/morphium
Length of output: 6539
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== README relevant section =="
sed -n '1,140p' morphium-jakarta-data/README.md | cat -n
echo
echo "== docs jakarta-data relevant sections =="
sed -n '430,475p' docs/jakarta-data.md | cat -n
sed -n '1,80p' docs/jakarta-data.md | cat -n
echo
echo "== AbstractMorphiumRepository relevant methods =="
sed -n '130,165p' morphium-jakarta-data/src/main/java/de/caluga/morphium/data/AbstractMorphiumRepository.java | cat -n
sed -n '400,430p' morphium-jakarta-data/src/main/java/de/caluga/morphium/data/AbstractMorphiumRepository.java | cat -nRepository: Bardioc1977/morphium
Length of output: 17693
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== docs repository interface definition =="
sed -n '72,86p' docs/jakarta-data.md | cat -n
echo
echo "== docs product repository implementation =="
sed -n '365,420p' docs/jakarta-data.md | cat -n
echo
echo "== AbstractMorphiumRepository class declaration/imports =="
sed -n '1,80p' morphium-jakarta-data/src/main/java/de/caluga/morphium/data/AbstractMorphiumRepository.java | cat -n
echo
echo "== PersonRepository declaration nearby == "
sed -n '92,118p' morphium-jakarta-data/README.md | cat -nRepository: Bardioc1977/morphium
Length of output: 8399
🌐 Web query:
Jakarta Data 1.0 BasicRepository findAll Stream<T> signature CrudRepository
💡 Result:
In Jakarta Data 1.0, the BasicRepository<T, K> interface defines the findAll() method with the following signature [1]: @Find Stream findAll; This method retrieves all persistent entities of the specified type from the database and returns a Stream<T> [1][2]. It is important to note that this method will never return null, though it may throw an UnsupportedOperationException if the underlying database (such as certain Key-Value or Wide-Column stores) is not capable of performing the findAll operation [1][3]. The CrudRepository<T, K> interface extends BasicRepository<T, K> [4][5]. Consequently, any repository interface that extends CrudRepository inherits this findAll() method signature [4].
Citations:
- 1: https://github.com/jakartaee/data/blob/main/api/src/main/java/jakarta/data/repository/BasicRepository.java
- 2: https://apidia.net/mvn/jakarta.platform/jakartaee/11.0.0/jakarta.data.repository.BasicRepository.html
- 3: https://javadoc.io/static/jakarta.platform/jakarta.jakartaee-web-api/11.0.0-M1/jakarta/data/repository/BasicRepository.html
- 4: https://jakarta.ee/specifications/data/1.0/apidocs/jakarta.data/jakarta/data/repository/crudrepository
- 5: https://jakarta.ee/specifications/data/1.0/apidocs/jakarta.data/jakarta/data/repository/package-summary
Return Stream<Person> for findAll() in the example.
PersonRepositoryImpl claims @Override, but @Find Stream<T> findAll() is the Jakarta Data contract. A List<Person> method is an overload, not an override, so any generated/proxied findAll() call is left unimplemented.
Proposed contract-aligned example
- public List<Person> findAll() {
- return doFindAll().toList();
+ public Stream<Person> findAll() {
+ return doFindAll();Add java.util.stream.Stream to the omitted imports.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public List<Person> findAll() { | |
| return doFindAll().toList(); | |
| } | |
| `@Override` | |
| public Stream<Person> findAll() { | |
| return doFindAll(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@morphium-jakarta-data/README.md` around lines 112 - 115, Update the
PersonRepositoryImpl findAll() example to return Stream<Person> instead of
List<Person>, preserving the doFindAll() result flow with the appropriate stream
return. Add the omitted java.util.stream.Stream import so the method correctly
overrides the Jakarta Data contract.
| private static String resolveFieldName(String part, java.util.Set<String> entityFields) { | ||
| String camelCase = decapitalize(part); | ||
| if (entityFields != null && !entityFields.isEmpty()) { | ||
| // Try exact match first | ||
| if (entityFields.contains(camelCase)) { | ||
| return camelCase; | ||
| } | ||
| // Try case-insensitive match | ||
| for (String f : entityFields) { | ||
| if (f.equalsIgnoreCase(camelCase)) { | ||
| return f; | ||
| } | ||
| } | ||
| } | ||
| return camelCase; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
entityFields is accepted but never authoritative, so bad method names fail silently. parse takes an entityFields set that the Javadoc describes as validation input. No path uses it to reject or disambiguate a name, and QueryMethodBridge.executeQuery calls MethodNameParser.parse(methodName, null), so the set is always absent on the repository path. Every consequence below is a query built against a field that does not exist, which returns empty results and reports no error.
morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java#L230-L245: throwIllegalArgumentExceptionwhenentityFieldsis supplied and contains no exact or case-insensitive match, instead of returning the derived camelCase name.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java#L157-L185: inparseCondition, test the decapitalized whole part againstentityFieldsbefore theOPERATOR_MATCHESloop, so a field namedsize,like,in, ornotis not reinterpreted as an operator.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java#L130-L148: passentityFieldsintoparseOrderByand resolve each sort field throughresolveFieldName, matching the condition path.
Supply the entity field names from QueryMethodBridge so all three checks take effect. The descriptor is cached per method name there, so validation runs once and surfaces a clear failure early rather than an empty result at runtime.
📍 Affects 1 file
morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java#L230-L245(this comment)morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java#L157-L185morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java#L130-L148
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java`
around lines 230 - 245, Make entityFields authoritative throughout
MethodNameParser: update resolveFieldName to throw IllegalArgumentException when
supplied fields have no exact or case-insensitive match; in parseCondition,
validate the decapitalized whole part before OPERATOR_MATCHES so fields named
size, like, in, or not are preserved; pass entityFields through parseOrderBy and
resolve sort fields consistently. In QueryMethodBridge, supply the entity field
names instead of null when calling parse so cached descriptors validate method
names early. Affected sites: MethodNameParser.java lines 230-245, 157-185, and
130-148 require the described parser changes; QueryMethodBridge also requires
the entity-field input change.
| QueryDescriptor descriptor = CACHE.computeIfAbsent(cacheKey, k -> { | ||
| QueryDescriptor parsed = MethodNameParser.parse(methodName, null); | ||
|
|
||
| // Merge method-name-derived OrderBy with @OrderBy annotation specs | ||
| if (!orderBySpec.isEmpty()) { | ||
| var mergedOrderBy = new ArrayList<>(parsed.orderBy()); | ||
| mergedOrderBy.addAll(parseOrderBySpec(orderBySpec)); | ||
| return new QueryDescriptor( | ||
| parsed.prefix(), | ||
| parsed.conditions(), | ||
| parsed.combinator(), | ||
| mergedOrderBy, | ||
| parsed.returnType()); | ||
| } | ||
| return parsed; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect annotation-processing and generated-call paths for `@OrderBy` handling.
fd -e java . morphium-jakarta-data/src/main/java \
-x rg -n -C 5 'import jakarta\.data\.repository\.OrderBy|`@OrderBy`|orderBySpec|executeQuery(?:Async)?\(|ignoreCase|descending' {}Repository: Bardioc1977/morphium
Length of output: 22710
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files | rg 'morphium-jakarta-data/src/main/java|gradle|pom|build' | sed -n '1,200p'
echo "== Locate annotation processor / method name parser / descriptor =="
fd -e java . morphium-jakarta-data/src/main/java \
-x sh -c 'echo "--- $1"; ast-grep outline "$1"' sh {}
echo "== Search for OrderBy annotation / generator calls =="
rg -n -C 4 'OrderBy\(|`@OrderBy`|generate|Generator|invokeStatic|queryMethodBridge|executeQuery|methodName\+|#|conditionsSpec|orderBySpec' morphium-jakarta-data/src/main/java || true
echo "== Search for Jakarta Data dependencies =="
rg -n 'jakarta\.data|data-api|data-server|Data|VERSION|Jakarta|Gradle|Gradle' -S . --glob '!**/build/**' --glob '!**/.gradle/**' --glob '!**/out/**' | sed -n '1,240p'Repository: Bardioc1977/morphium
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pom files containing Jakarta Data dependency =="
rg -n -C 4 'jakarta\.data|jakarta-data|data-api|Repository|Gizmo|processor|annotation|generate' morphium-jakarta-data/pom.xml pom.xml morphium-commons/pom.xml 2>/dev/null || true
echo "== Git diff stat and relevant QueryMethodBridge area =="
git diff --stat || true
git diff -- morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryMethodBridge.java 2>/dev/null | sed -n '1,220p' || true
echo "== QueryMethodBridge full relevant methods =="
wc -l morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryMethodBridge.java
sed -n '60,235p' morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryMethodBridge.java
echo "== QueryDescriptor relevant data =="
sed -n '1,140p' morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryDescriptor.java
echo "== MethodNameParser parseOrderBy and query descriptor usage =="
sed -n '1,180p' morphium-jakedown/src/main/java/de/caluga/morphium/data/MethodNameParser.java 2>/dev/null || sed -n '1,180p' morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java
echo "== Behavioral probe: parseOrderBySpec accepts only field/dir =="
python3 - <<'PY'
def parse(spec):
out=[]
for part in spec.split(','):
trimmed=part.strip()
if part=='' and trimmed=='':
raise Exception("Empty")
if trimmed=='':
continue
fa=trimmed.split(':',-1)
f=fa[0].strip()
if len(fa)>2: raise Exception("many")
d=True
if len(fa)>1:
dir=fa[1].strip()
if dir=='DESC':
d=False
elif dir=='ASC':
pass
else:
raise Exception(bad)
out.append((f,d))
return out
cases = [
"createdAt",
"createdAt:ASC",
"createdAt:DESC",
"createdAt,updatedAt",
"createdAt:ASC,updatedAt:DESC",
"createdAt:DESC,updatedAt:ASC",
"createdAt:ignoredCase",
"createdAt:[descending:true]",
"",
]
for c in cases:
try:
print(f"In {c!r} -> {parse(c)}")
except Exception as e:
print(f"In {c!r} -> EX {type(e).__name__}: {e_args_str(e) if e_args_str(e) else e}")
PYRepository: Bardioc1977/morphium
Length of output: 23687
🌐 Web query:
Jakarta Data 1.0 OrderBy annotation repeatable descending ignoreCase forbids method name OrderBy API documentation
💡 Result:
In Jakarta Data 1.0, the @OrderBy annotation behaves as follows regarding the points you raised: Repeatable: Yes, @OrderBy is a repeatable annotation [1]. It is defined with @Repeatable(OrderBy.List.class), allowing you to specify multiple @OrderBy annotations on a single repository method to define a complex sort order [2][3][1]. Descending and IgnoreCase: The @OrderBy annotation includes boolean elements to control sort behavior [2][1]. - descending: Indicates whether to use descending order; the default value is false (ascending) [2][1]. - ignoreCase: Indicates whether to request case-insensitive ordering from a database with case-sensitive collation; the default value is false [2][1]. Forbidden Method Name: Yes, the use of @OrderBy on a repository method imposes restrictions on the method name [2][4]. A repository method annotated with @OrderBy must not contain the "OrderBy" keyword as part of its Query by Method Name declaration [2][5][4]. If a method violates this rule, or if it also contains a @Query annotation specifying a query with an "ORDER BY" clause, a Jakarta Data provider is permitted to reject the declaration at compile time or cause the method to throw an UnsupportedOperationException [2][1][4]. These restrictions exist because @OrderBy is an alternative mechanism for defining sort order that should not be combined with other conflicting sorting mechanisms, such as Query by Method Name or explicit JPQL/JDQL ORDER BY clauses [2][4][6].
Citations:
- 1: https://github.com/jakartaee/data/blob/main/api/src/main/java/jakarta/data/repository/OrderBy.java
- 2: https://jakarta.ee/specifications/data/1.0/apidocs/jakarta.data/jakarta/data/repository/orderby
- 3: https://jakarta.ee/specifications/data/1.0/apidocs/jakarta.data/jakarta/data/repository/orderby.list
- 4: https://jakarta.ee/specifications/platform/11/apidocs/jakarta/data/repository/orderby
- 5: https://jakarta.ee/specifications/data/1.0/jakarta-data-addendum-1.0.html
- 6: https://jakarta.ee/specifications/data/1.0/jakarta-data-1.0.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for all occurrences of executeQuery with orderBySpec argument =="
rg -n -C 4 'executeQuery\([^)]+\)\s*\{|executeQuery\(.*, orderBySpec|executeQuery\([^,]+,\s*[^,]+,\s*[^,]+,\s*[^,]+,\s*[^,]+,\s*"?"\)|"OrderBy"|OrderBy\(' morphium-jakarta-data/src/main/java . --glob '!**/.git/**' --glob '!**/build/**' --glob '!**/.gradle/**' | sed -n '1,220p'
echo "== Full locate for repository code in repo =="
fd -e java . . | rg 'jakarta|Repository|Data|Processor|Generator|Gizmo|Annotated' | sed -n '1,220p'Repository: Bardioc1977/morphium
Length of output: 19100
Preserve Jakarta Data static-order semantics.
parseOrderBySpec(...) only handles field[:ASC|DESC], but Jakarta Data 1.0 @OrderBy supports descending and ignoreCase and is repeatable. Add encoding for these attributes; reject ignoreCase if Morphium cannot support it; extend the generated callsite with the full static order spec; and reject mixed static-order declarations such as @OrderBy plus a name-derived OrderBy* suffix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryMethodBridge.java`
around lines 76 - 90, Update QueryMethodBridge’s static-order handling around
CACHE.computeIfAbsent and parseOrderBySpec so Jakarta Data `@OrderBy` attributes
are preserved, including descending and repeatable declarations, and reject any
ignoreCase usage if Morphium cannot support it. Also make the callsite
generation carry the full static order specification instead of only
field[:ASC|DESC], and add validation in the QueryDescriptor merge path to reject
mixed static-order declarations when both `@OrderBy` and a name-derived OrderBy*
suffix are present.
…and negated alias conditions CONTAINS previously built an exact-equality condition instead of a substring match; it now builds an unanchored, literal-escaped regex. Query.delete() for derived deleteBy* methods returned the pre-delete countAll() instead of the actual number of deleted documents; it now reads the "n" field from the driver's delete-result map, falling back to the pre-count only if that field is absent or non-numeric. Negating operators (NE, NIN, NOT_CONTAINS, IS_NOT_NULL, IS_NOT_EMPTY) on a field with @Aliases combined alias branches with $or, which is almost always trivially true for a negation (a document lacking the alias field entirely would match "alias != X"). Alias branches for negating operators are now combined with $and instead. Reported by CodeRabbit/Codex review on PR #16.
…mode skip Three related bugs across the cursor-pagination paths in AbstractMorphiumRepository.doFindAllCursored, JdqlMethodBridge.executeCursoredJdql, and FindMethodBridge.executeCursoredFind: - A @query method's own JDQL ORDER BY clause was silently dropped for CursoredPage results: the cursor keyset was built only from the separate @orderby annotation spec, which CursorHelper.applySort then used to overwrite the sort already applied for the JDQL ORDER BY, leaving the cursor without any sort key. The JDQL ORDER BY now takes precedence when present. - A dynamic Sort/Order method parameter on a @find method returning CursoredPage was applied to the query but then overwritten by CursorHelper.applySort with the (empty) static @orderby keyset. The dynamic sort is now threaded through into the cursor keyset. - PageRequest.Mode.OFFSET requests for a CursoredPage applied neither a cursor condition nor a skip, so any page beyond the first returned the same records regardless of the requested page number. All three methods now skip to the requested page in that mode, mirroring the existing offset-page logic in doFindAllPaged(). Also in JdqlMethodBridge.toNumber(): AVG aggregate results are now always returned as double, since some drivers/the in-memory aggregator return an Integer/Long when every averaged value happens to be a whole number, which previously surfaced as a long instead of the double Jakarta Data callers expect. Reported by CodeRabbit/Codex review on PR #16.
…OUP-BY detection, direction validation, and string-literal-aware AND/OR splitting
…nd fix combinator detection for acronym-ending field segments
CursorHelper.applyCursorCondition silently produced an empty $or condition when sortSpecs was empty, causing cursor-based pagination to lose its keyset filter entirely without any indication of the problem. Cursor pagination without a sort keyset is conceptually undefined (there is no unique 'continue after here' criterion), so this now fails fast with an IllegalArgumentException instead of degrading into a silently broken/no-op paging condition. Adds CursorHelperTest covering the null/empty sortSpecs guard as well as a regression check that a non-empty keyset still produces the expected $or condition.
… BY record class Thread.currentThread().getContextClassLoader().loadClass(...) can fail to find the result record class in modular/OSGi/framework environments where the context classloader differs from the one that loaded this bridge class (or can be null in some embedded contexts). Fall back to JdqlMethodBridge's own classloader before giving up. Reported by CodeRabbit review on PR #16.
…xact inequality
NOT_CONTAINS mapped to $ne (exact not-equal) instead of negating the
substring match that CONTAINS uses, so findByFieldNotContaining(x) filtered
on "field != x" instead of "field does not contain x" -- wrong results
whenever the field wasn't an exact match to the argument. Now generates
{$not: {$regex: ...}}, mirroring CONTAINS's $regex construction.
Adds regression tests exercising both the generated MongoDB query shape and
real InMemDriver data, following the existing CONTAINS test pattern in this
file.
Found in code review on PR sboesebeck#266 (sboesebeck/morphium).
…ved-query LIKE path JDQL LIKE/NOT LIKE built its regex from the raw literal without escaping regex metacharacters or anchoring the pattern, unlike the derived-query LIKE path (likeToRegex() in QueryExecutor), which already used Pattern.quote() and ^...$ anchoring correctly. WHERE code LIKE 'A.1' matched "AX1" (the literal dot was interpreted as a regex wildcard), and a wildcard-free pattern like WHERE name LIKE 'Widget' matched any value merely containing "Widget" instead of requiring an exact match. Now delegates to the already-correct QueryExecutor.likeToRegex(), which escapes literal segments with Pattern.quote() while still translating % and _ SQL wildcards, and anchors the whole pattern with ^...$. Adds JdqlMethodBridgeTest (no prior direct unit test existed for this class), exercising exact-match semantics, metacharacter escaping, and that SQL wildcards still work after the fix. Found in code review on PR sboesebeck#266 (sboesebeck/morphium).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java (1)
258-263: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFail-fast rejects valid method names whose field name equals an operator suffix.
parseConditiontests the operator suffixes before it checks the part againstentityFields. For an entity field namedsize,findBySizematches theSizesuffix, sofieldPartbecomes the empty string.resolveFieldName("")now reaches the new throw and reportsUnknown field ''. The same happens forfindByLike,findByIn, andfindByNoton entities that declare such fields. Before this change these names produced a wrong query silently; now they fail with a misleading message.Fix the precedence in
parseCondition: if the decapitalized whole part is a known entity field, treat it as an implicit equality; and never callresolveFieldNamewith an emptyfieldPart.🐛 Proposed fix in
parseCondition(lines 172-200)private static ParsedCondition parseCondition(String part, int paramIndex, java.util.Set<String> entityFields) { + // A whole part that names an entity field wins over an operator suffix + // (e.g. field "size" in findBySize must not match the "Size" operator). + if (entityFields != null && !entityFields.isEmpty()) { + String whole = decapitalize(part); + for (String f : entityFields) { + if (f.equalsIgnoreCase(whole)) { + return new ParsedCondition(new Condition(f, Operator.EQ, paramIndex), + paramIndex + 1); + } + } + } // Try to match operators from longest to shortest for (OperatorMatch om : OPERATOR_MATCHES) { - if (part.endsWith(om.suffix)) { + if (part.endsWith(om.suffix) && part.length() > om.suffix.length()) { String fieldPart = part.substring(0, part.length() - om.suffix.length());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java` around lines 258 - 263, Update parseCondition so it first checks whether the decapitalized whole method part matches an entity field and treats that name as an implicit equality, including fields named size, like, in, or not. Only parse operator suffixes when the whole part is not a known field, and guard the suffix path so resolveFieldName is never called with an empty fieldPart.
🧹 Nitpick comments (2)
morphium-jakarta-data/src/test/java/de/caluga/morphium/data/MethodNameParserTest.java (1)
129-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for a field name that equals an operator suffix.
ENTITY_FIELDScontains no name that collides with an operator suffix, so the tests do not coverfindBySize,findByLike, orfindByIn. Those names currently fail withUnknown field ''. See the comment onMethodNameParser.resolveFieldName. Add the entity fieldsizeto a fixture and assert thatfindBySizeparses to an equality condition onsize.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/test/java/de/caluga/morphium/data/MethodNameParserTest.java` around lines 129 - 137, Extend the test fixture used by MethodNameParserTest with a size field, then add a test for MethodNameParser.parse("findBySize", ENTITY_FIELDS) that verifies it produces an equality condition targeting size. Ensure the assertion covers both the resolved field name and equality operator, exercising the operator-suffix collision handled by MethodNameParser.resolveFieldName.morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java (1)
81-86: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCompute the fallback count only when the driver result lacks
n.
query.countAll()runs on every derived delete, including the normal path wheredeleteResultcontains a numericn. That doubles the round trips for eachdeleteBy*call. Readnfirst and count only in the fallback branch.♻️ Proposed change
case DELETE -> { - long preCount = query.countAll(); Map<String, Object> deleteResult = query.delete(); Object n = deleteResult == null ? null : deleteResult.get("n"); - yield (n instanceof Number) ? ((Number) n).longValue() : preCount; + // Fallback only: the driver did not report a numeric "n". + yield (n instanceof Number) ? ((Number) n).longValue() : query.countAll(); }Note that the fallback then counts the remaining matches after the delete, so it returns 0 in most cases. If the pre-delete count is the intended fallback, keep it but compute it only when the driver result is known to omit
n, for example by checking the driver once instead of on every call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java` around lines 81 - 86, Update the DELETE branch in QueryExecutor to call query.delete() and inspect its "n" value before invoking query.countAll(). Return the numeric driver count directly; only compute the fallback count when the result is null or lacks a numeric "n", preserving the intended pre-delete fallback semantics without an unconditional extra query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.java`:
- Around line 258-263: Update parseCondition so it first checks whether the
decapitalized whole method part matches an entity field and treats that name as
an implicit equality, including fields named size, like, in, or not. Only parse
operator suffixes when the whole part is not a known field, and guard the suffix
path so resolveFieldName is never called with an empty fieldPart.
---
Nitpick comments:
In
`@morphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.java`:
- Around line 81-86: Update the DELETE branch in QueryExecutor to call
query.delete() and inspect its "n" value before invoking query.countAll().
Return the numeric driver count directly; only compute the fallback count when
the result is null or lacks a numeric "n", preserving the intended pre-delete
fallback semantics without an unconditional extra query.
In
`@morphium-jakarta-data/src/test/java/de/caluga/morphium/data/MethodNameParserTest.java`:
- Around line 129-137: Extend the test fixture used by MethodNameParserTest with
a size field, then add a test for MethodNameParser.parse("findBySize",
ENTITY_FIELDS) that verifies it produces an equality condition targeting size.
Ensure the assertion covers both the resolved field name and equality operator,
exercising the operator-suffix collision handled by
MethodNameParser.resolveFieldName.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 20899b44-124f-4a0e-9d28-59b9af58dbb1
📒 Files selected for processing (14)
morphium-jakarta-data/src/main/java/de/caluga/morphium/data/AbstractMorphiumRepository.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/CursorHelper.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlMethodBridge.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/MethodNameParser.javamorphium-jakarta-data/src/main/java/de/caluga/morphium/data/QueryExecutor.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/AbstractMorphiumRepositoryUpdateTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/CursorHelperTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/JdqlMethodBridgeTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/JdqlParserTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/MethodNameParserTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/QueryExecutorAliasTest.javamorphium-jakarta-data/src/test/java/de/caluga/morphium/data/QueryExecutorTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- morphium-jakarta-data/src/main/java/de/caluga/morphium/data/CursorHelper.java
- morphium-jakarta-data/src/main/java/de/caluga/morphium/data/FindMethodBridge.java
- morphium-jakarta-data/src/main/java/de/caluga/morphium/data/JdqlParser.java
- morphium-jakarta-data/src/main/java/de/caluga/morphium/data/AbstractMorphiumRepository.java
* feat: add morphium-jakarta-data as optional module * build: register morphium-jakarta-data in extensions profile * docs: add jakarta data module documentation * docs: add changelog entry for morphium-jakarta-data module * build: include morphium-jakarta-data in release bundle Extend release.sh to handle morphium-jakarta-data alongside morphium-core and poppydb in the Sonatype Central bundle. Replaces the previous per-module copy-paste blocks with a small module registry (MODULE_DIRS/MODULE_ARTIFACT_IDS/MODULE_EXTRA_CLASSIFIERS parallel arrays, bash 3.2 compatible) and a shared add_module_to_bundle() helper, since the blocks were structurally identical and copy-paste would not scale to the further modules coming in M4/M5. Version-sync checks, backup cleanup, rollback, structure verification, bundle assembly and the artifact verification loop all now iterate over the registry instead of naming morphium-core/poppydb explicitly. Verified: bash -n passes, shellcheck shows no new findings (all 3 remaining findings are on pre-existing lines), and 'mvn -pl morphium-jakarta-data package source:jar javadoc:jar' produces the expected jar/sources.jar/javadoc.jar triple. * fix(jakarta-data): reject mixed And/Or method names and non-upserting update() * fix(jakarta-data): correct CONTAINS substring match, delete() count, and negated alias conditions CONTAINS previously built an exact-equality condition instead of a substring match; it now builds an unanchored, literal-escaped regex. Query.delete() for derived deleteBy* methods returned the pre-delete countAll() instead of the actual number of deleted documents; it now reads the "n" field from the driver's delete-result map, falling back to the pre-count only if that field is absent or non-numeric. Negating operators (NE, NIN, NOT_CONTAINS, IS_NOT_NULL, IS_NOT_EMPTY) on a field with @Aliases combined alias branches with $or, which is almost always trivially true for a negation (a document lacking the alias field entirely would match "alias != X"). Alias branches for negating operators are now combined with $and instead. Reported by CodeRabbit/Codex review on PR #16. * fix(jakarta-data): correct cursor pagination sort sources and OFFSET-mode skip Three related bugs across the cursor-pagination paths in AbstractMorphiumRepository.doFindAllCursored, JdqlMethodBridge.executeCursoredJdql, and FindMethodBridge.executeCursoredFind: - A @query method's own JDQL ORDER BY clause was silently dropped for CursoredPage results: the cursor keyset was built only from the separate @orderby annotation spec, which CursorHelper.applySort then used to overwrite the sort already applied for the JDQL ORDER BY, leaving the cursor without any sort key. The JDQL ORDER BY now takes precedence when present. - A dynamic Sort/Order method parameter on a @find method returning CursoredPage was applied to the query but then overwritten by CursorHelper.applySort with the (empty) static @orderby keyset. The dynamic sort is now threaded through into the cursor keyset. - PageRequest.Mode.OFFSET requests for a CursoredPage applied neither a cursor condition nor a skip, so any page beyond the first returned the same records regardless of the requested page number. All three methods now skip to the requested page in that mode, mirroring the existing offset-page logic in doFindAllPaged(). Also in JdqlMethodBridge.toNumber(): AVG aggregate results are now always returned as double, since some drivers/the in-memory aggregator return an Integer/Long when every averaged value happens to be a whole number, which previously surfaced as a long instead of the double Jakarta Data callers expect. Reported by CodeRabbit/Codex review on PR #16. * fix(jakarta-data): fix JDQL ORDER BY without WHERE, HAVING-without-GROUP-BY detection, direction validation, and string-literal-aware AND/OR splitting * fix(jakarta-data): validate unknown fields in derived query methods and fix combinator detection for acronym-ending field segments * fix(jakarta-data): require non-empty sort keyset for cursor pagination CursorHelper.applyCursorCondition silently produced an empty $or condition when sortSpecs was empty, causing cursor-based pagination to lose its keyset filter entirely without any indication of the problem. Cursor pagination without a sort keyset is conceptually undefined (there is no unique 'continue after here' criterion), so this now fails fast with an IllegalArgumentException instead of degrading into a silently broken/no-op paging condition. Adds CursorHelperTest covering the null/empty sortSpecs guard as well as a regression check that a non-empty keyset still produces the expected $or condition. * fix(jakarta-data): fall back to bridge classloader when loading GROUP BY record class Thread.currentThread().getContextClassLoader().loadClass(...) can fail to find the result record class in modular/OSGi/framework environments where the context classloader differs from the one that loaded this bridge class (or can be null in some embedded contexts). Fall back to JdqlMethodBridge's own classloader before giving up. Reported by CodeRabbit review on PR #16. * fix(jakarta-data): make NOT_CONTAINS a negated substring match, not exact inequality NOT_CONTAINS mapped to $ne (exact not-equal) instead of negating the substring match that CONTAINS uses, so findByFieldNotContaining(x) filtered on "field != x" instead of "field does not contain x" -- wrong results whenever the field wasn't an exact match to the argument. Now generates {$not: {$regex: ...}}, mirroring CONTAINS's $regex construction. Adds regression tests exercising both the generated MongoDB query shape and real InMemDriver data, following the existing CONTAINS test pattern in this file. Found in code review on PR sboesebeck#266 (sboesebeck/morphium). * fix(jakarta-data): escape and anchor JDQL LIKE patterns like the derived-query LIKE path JDQL LIKE/NOT LIKE built its regex from the raw literal without escaping regex metacharacters or anchoring the pattern, unlike the derived-query LIKE path (likeToRegex() in QueryExecutor), which already used Pattern.quote() and ^...$ anchoring correctly. WHERE code LIKE 'A.1' matched "AX1" (the literal dot was interpreted as a regex wildcard), and a wildcard-free pattern like WHERE name LIKE 'Widget' matched any value merely containing "Widget" instead of requiring an exact match. Now delegates to the already-correct QueryExecutor.likeToRegex(), which escapes literal segments with Pattern.quote() while still translating % and _ SQL wildcards, and anchors the whole pattern with ^...$. Adds JdqlMethodBridgeTest (no prior direct unit test existed for this class), exercising exact-match semantics, metacharacter escaping, and that SQL wildcards still work after the fix. Found in code review on PR sboesebeck#266 (sboesebeck/morphium). --------- Co-authored-by: Heiko Kopp <extern.heiko.kopp1@porsche.de>
Hi Stephan,
first PR of the module-integration series we discussed:
morphium-jakarta-data, the framework-agnostic Jakarta Data 1.0 runtime that the Quarkus and Spring Boot integrations build on. Quarkus and Spring Boot follow as separate PRs.What this PR does
morphium-jakarta-data/as a new module:CrudRepository/MorphiumRepositoryinterfaces, query derivation, JDQL via@Query,@Find/@Delete, pagination, sorting — framework-agnostic, zero dependencies beyond Morphium core andjakarta.data:jakarta.data-api.extensions, active by default, deactivatable with-DskipExtensions.docs/jakarta-data.md, adocs/index.mdpointer, aCHANGELOG.mdentry.release.sh(module registry instead of a third copy-paste block — scales to the Quarkus/Spring Boot modules coming next; behavior formorphium-core/poppydbunchanged).Optionality — verifiable, not just claimed
git diff --stat origin/develop -- morphium-core poppydbis empty,morphium-core/pom.xmlhas zero references to the extension, its dependency tree has nojakarta.data/io.quarkus/org.springframework, and-DskipExtensionsbuilds only Parent+Morphium+PoppyDB.Versioning
Lockstep with Morphium (inherits
morphium-parent's version, likepoppydb). Reversible with one POM line if you'd prefer an independent line.Verification
Full reactor build, 45 module tests green, core-only build confirmed extension-free, full core suite (2040 tests) shows only pre-existing failures reproduced identically against a clean
origin/developcheckout (Byte Buddy/JDK 25, a Mongo-dependent perf test, known messaging timing flakiness) — no regression.javadoc:jar/source:jarclean,shellcheckclean.Open questions
release.shmodule-registry refactor, or prefer copy-paste consistency?Summary by CodeRabbit