[OPENJPA-2968] Do not swallow failures around IdClass primary key values - #167
Open
rzo1 wants to merge 1 commit into
Open
[OPENJPA-2968] Do not swallow failures around IdClass primary key values#167rzo1 wants to merge 1 commit into
rzo1 wants to merge 1 commit into
Conversation
The @IdClass extraction and reconstruction paths added for the JPA 2.4.1.3 example 2b case caught every exception and continued: on write the primary key columns were filled with nulls, on read the field was left null, so a corrupt identity could be persisted or loaded without any trace. Both paths now raise a StoreException naming the field, the id class and the mapping, with the original failure chained, and a null id class value on the write path is logged as a warning. The columns are also no longer paired with the id class fields by Class.getDeclaredFields() order, which the JVM does not guarantee. Where every column names its target through referencedColumnName the fields are matched by that name; the declaration order remains only as the fallback for mappings that provide no name. TestDerivedIdEx2c covers the ordering: its id class declares the fields in the order opposite to the join columns, so a find() misses on the previous code and succeeds now.
solomax
approved these changes
Aug 31, 2026
| for (int c = 0; c < mic.size(); c++) { | ||
| if (cols.length == 1) rvals.add(null); | ||
| else ((Object[]) rvals.get(0))[idx++] = null; | ||
| List<java.lang.reflect.Field> df = |
Contributor
There was a problem hiding this comment.
weird enough java.lang.reflect.Field is not moved to import's
| } | ||
| } else { | ||
| Log log = fms[i].getRepository().getLog(); | ||
| if (log.isWarnEnabled()) |
|
|
||
| private static java.lang.reflect.Field findField( | ||
| List<java.lang.reflect.Field> fields, String name) { | ||
| for (java.lang.reflect.Field f : fields) |
Contributor
There was a problem hiding this comment.
I would add {} in these for/if's :))
Is the stream API will be slower here? :)
like: fields.stream().filter(f -> f.getName().equals(name)).findFirst().orElse(null)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
@IdClassextraction and reconstruction paths added for the JPA 2.4.1.3 example 2b case caught every exception and continued: on write the primary key columns were filled with nulls, on read the field was left null, so a corrupt identity could be persisted or loaded without a trace.Both paths now raise a
StoreExceptionnaming the field, the id class and the mapping, with the original failure chained. A null id class value on the write path is legitimate, so it still writes nulls, but logs a warning.The columns are also no longer paired with the id class fields by
Class.getDeclaredFields()order, which the JVM does not guarantee. Where every column names its target throughreferencedColumnNamethe fields are matched by that name; declaration order remains only as the fallback for mappings that provide no name, since that is all such a mapping offers.TestDerivedIdEx2ccovers the ordering: its id class declares the fields in the order opposite to the join columns, so afind()misses on the previous code and succeeds now.