HSEARCH-2433 AssertionFailure with providedId in embeddeds #1200
Conversation
| @@ -427,37 +430,57 @@ private Annotation getIdAnnotation(XProperty member, TypeMetadata.Builder typeMe | |||
| return idAnnotation; | |||
| } | |||
|
|
|||
| private void initializeProvidedIdMetadata(ProvidedId providedId, XClass clazz, TypeMetadata.Builder typeMetadataBuilder) { | |||
| private void initializeProvidedIdMetadata(String prefix, ProvidedId providedId, XClass clazz, TypeMetadata.Builder typeMetadataBuilder, | |||
| boolean isRoot, PathsContext pathsContext, ParseContext parseContext) { | |||
| PropertyMetadata propertyMetadata; | |||
There was a problem hiding this comment.
I know it's preexisting to your patch but we should probably move this declaration to where it is used. It makes no sense to have it here.
| typeMetadataBuilder.idProperty( propertyMetadata ); | ||
|
|
||
| if ( isRoot ) { | ||
| typeMetadataBuilder.idProperty( propertyMetadata ); |
There was a problem hiding this comment.
Mmmmh, did I miss something or if !isRoot, the whole work we did above was totally useless?
There was a problem hiding this comment.
You're right, there's something wrong in here. I'll see what I can do (probably just add a else)
|
@yrodiere you mention on the JIRA comments that this problem affects the Lucene backend as well. Worth moving the test to |
|
@Sanne Yes, I'll see if I can highlight the issue in an -engine test. |
|
I meant to suggest to move this one, not to add an additional one. We don't want to have too many redundant tests checking for the same things ;) |
|
@Sanne I'll move to -orm, then. -engine tests are not yet executed on Elasticsearch. Not even sure yet when they will be... |
yrodiere
force-pushed
the
HSEARCH-2433
branch
from
51973a5
to
af7d5f7
Compare
Right, forgot about that. Let's merge this then, the missing test for the embedded mode is an orthogonal issue. |
There were three issues: * The prefix was not used on the providedId field, potentially resulting in root type provided ID being erased by embeddeds' provided IDs * The providedId was being added unconditionally for embeddeds, not respecting settings like @IndexedEmbedded.includeEmbeddedObjectIds * The field bridge was not ignored as required when creating metadata for containedIn
yrodiere
force-pushed
the
HSEARCH-2433
branch
from
af7d5f7
to
386bf15
Compare
|
@Sanne I rebased on master: the build was failing because of https://hibernate.atlassian.net/browse/HSEARCH-2438 , which was merged to master a few minutes ago |
|
@gsmet I addressed your concerns. Thanks! |
|
merged, thanks all! |
https://hibernate.atlassian.net/browse/HSEARCH-2433
Details in the commit comments.