HHH-16383 - NaturalIdClass #11301
HHH-16383 - NaturalIdClass #11301
Conversation
|
Remaining work mostly comess down to implementing branch in See
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /** | ||
| * Used in creating dynamic annotation instances (e.g. from XML) | ||
| */ | ||
| public NaturalIdClassAnnotation(ModelsContext modelContext) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
| /** | ||
| * Used in creating annotation instances from JDK variant | ||
| */ | ||
| public NaturalIdClassAnnotation(NaturalIdClass annotation, ModelsContext modelContext) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
| /** | ||
| * Used in creating annotation instances from Jandex variant | ||
| */ | ||
| public NaturalIdClassAnnotation(Map<String, Object> attributeValues, ModelsContext modelContext) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
| /// Find an entity by [natural-id][org.hibernate.annotations.NaturalId]. | ||
| /// | ||
| /// @param entityType The type of entity to load. | ||
| /// @param naturalId The natural-id value. | ||
| /// @param options The options to apply to the find operation. | ||
| /// | ||
| /// @apiNote For non-aggregated composite natural-ids, consider leveraging a [org.hibernate.annotations.NaturalIdClass]. | ||
| <T> T findByNaturalId(Class<T> entityType, Object naturalId, FindOption... options); | ||
|
|
||
| /// Find an entity by [natural-id][org.hibernate.annotations.NaturalId]. | ||
| /// | ||
| /// @param entityName The name of the entity type to load. | ||
| /// @param naturalId The natural-id value. | ||
| /// @param options The options to apply to the find operation. | ||
| /// | ||
| /// @apiNote For non-aggregated composite natural-ids, consider leveraging a [org.hibernate.annotations.NaturalIdClass]. | ||
| Object findByNaturalId(String entityName, Object naturalId, FindOption... options); | ||
|
|
||
| /// Find multiple entities by [natural-id][org.hibernate.annotations.NaturalId]. | ||
| /// | ||
| /// @param entityType The type of entity to load. | ||
| /// @param naturalIds The natural-id values. | ||
| /// @param options The options to apply to the find operation. May contain [FindMultipleOption] values. | ||
| /// | ||
| /// @apiNote For non-aggregated composite natural-ids, consider leveraging a [org.hibernate.annotations.NaturalIdClass]. | ||
| <T> List<T> findMultipleByNaturalId(Class<T> entityType, List<Object> naturalIds, FindOption... options); | ||
|
|
||
| /// Find multiple entities by [natural-id][org.hibernate.annotations.NaturalId]. | ||
| /// | ||
| /// @param entityName The name of the entity type to load. | ||
| /// @param naturalIds The natural-id values. | ||
| /// @param options The options to apply to the find operation. May contain [FindMultipleOption] values. | ||
| /// | ||
| /// @apiNote For non-aggregated composite natural-ids, consider leveraging a [org.hibernate.annotations.NaturalIdClass]. | ||
| List<Object> findMultipleByNaturalId(String entityName, List<Object> naturalIds, FindOption... options); | ||
|
|
There was a problem hiding this comment.
@sebersole I'm not sure I would add these methods. I think it would be OK to use find() for lookup by natural id.
In the .... much less common, I hypothesize ... case where the @Id type and the @NaturalId type are identical (i.e. both Long or both String), it's easy enough for the user to create a @NaturalIdClass with a single field in order to allow them to be distinguished.
Alternatively, it would be possible to allow something like:
var person = session.find(Person.class, new NaturalIdValue(personId));or even:
var person = session.find(Person.class, personId, Id.NATURAL);where Id.NATURAL is a FindMultipleOption.
Actually this last possibility is growing on me, since it also works with EntityManager.find().
There was a problem hiding this comment.
I did consider this. Definitely pros and cons. The big one you also mentioned which is the case of the types matching. I'm not sure I would cateforize that "uncommon" - I know that's not the exact phrase you used, but I think you might be minimizing it. And I am not sure requiring the use of an (Natural)IdClass to simply to denote the difference for loading is great.
Now, your last suggestion - I assume you think Id.NATURAL is a FindOption, which I guess I can get behind. At that point there is a lot of complexity behiond the scenes to handle both of these routing through a single API method, but it is internal so not a blocker.
There was a problem hiding this comment.
Like you, having a FindOption here grew on me. I added support for that. At the moment I have:
enum FindBy { ID, NATURAL_ID }Though thinking
enum KeyType { ID, NATURAL }Going to let this naming percolate a bit.
hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java
Show resolved
Hide resolved
# Conflicts: # hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java
sebersole
force-pushed
the
naturalidclass
branch
from
63883b8 to
159be18
Compare
|
A few things I still want to clean up (so please don't merge it), but this is mostly ready for review. |
| if ( factoryHint instanceof Integer number ) { | ||
| return Timeout.milliseconds( number ); | ||
| } | ||
| return Timeout.milliseconds( Integer.parseInt( factoryHint.toString() ) ); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
2 participants
HHH-16383 - NaturalIdClass
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-16383