Skip to content

Support for association target type detection for jMolecules. #2344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

odrotbohm
Copy link
Member

@odrotbohm odrotbohm commented Mar 31, 2021

We now detect the component type of jMolecules' Association as association target type in AbstractPersistentProperty. AnnotationBasedPersistentProperty keeps the lookup of the explicit @Reference around but falls back to the Association analysis for that.

@Reference is now only considered by AnnotationBasedPersistentProperty. It was previously also (erroneously) considered by AbstractPersistentProperty.

It was also changed to return null in case no association target type could be looked up instead of returning the sole property's type in case that is an entity. This was done to satisfy the behavior documented on the interface but also to avoid the call to isEntity() during the calculation which might use association information in turn and thus lead to a stack overflow.

@odrotbohm
Copy link
Member Author

Please do not merge yet. I'm about to push an update for this one.

We now detect the component type of jMolecules' Association as association target type in AbstractPersistentProperty. AnnotationBasedPersistentProperty keeps the lookup of the explicit @reference around but falls back to the Association analysis for that.

@reference is now only considered by AnnotationBasedPersistentProperty. It was previously also (erroneously) considered by AbstractPersistentProperty.

It was also changed to return `null` in case no association target type could be looked up instead of returning the sole property's type in case that is an entity. This was done to satisfy the behavior documented on the interface but also to avoid the call to isEntity() during the calculation which might use association information in turn and thus lead to a stack overflow.
@odrotbohm odrotbohm force-pushed the issue/association-target-type branch from f799a3f to 519ac95 Compare March 31, 2021 08:10
@odrotbohm
Copy link
Member Author

Updates done, ready for review.

@mp911de mp911de self-assigned this Mar 31, 2021
@mp911de mp911de closed this in 0ab31bb Mar 31, 2021
@mp911de mp911de added this to the 2.5 RC1 (2021.0.0) milestone Mar 31, 2021
@mp911de mp911de deleted the issue/association-target-type branch March 31, 2021 09:06
@mp911de mp911de added the type: enhancement A general enhancement label Mar 31, 2021
mp911de added a commit that referenced this pull request Mar 31, 2021
We now determine the association target type from the actual property type if the property reports that it is an association and the target type is an entity.
The previous change removed this behavior so we're reinstating it here.

See #2344.
odrotbohm added a commit that referenced this pull request Mar 31, 2021
We now detect the component type of jMolecules' Association as association target type in AbstractPersistentProperty. AnnotationBasedPersistentProperty keeps the lookup of the explicit @reference around but falls back to the Association analysis for that.

@reference is now only considered by AnnotationBasedPersistentProperty. It was previously also (erroneously) considered by AbstractPersistentProperty.

It was also changed to return `null` in case no association target type could be looked up instead of returning the sole property's type in case that is an entity. This was done to satisfy the behavior documented on the interface but also to avoid the call to isEntity() during the calculation which might use association information in turn and thus lead to a stack overflow. Reworded the contract of PersistentProperty.getAssociationTargetType() to emphasize the symmetry with ….isAssociation() in implementation classes.

Closes: #2344.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants