Allow for null and empty parameters in the MultiField annotation.#2960
Allow for null and empty parameters in the MultiField annotation.#2960sothawo merged 2 commits intospring-projects:mainfrom
null and empty parameters in the MultiField annotation.#2960Conversation
Signed-off-by: Youssef Aouichaoui <[email protected]>
spring-projects-issues
added
the
status: waiting-for-triage
|
I think it would be better to include these parameters in boolean storeNullValue() default false;
boolean storeEmptyValue() default true;to also support inner fields storeNullValue = isField ? getRequiredAnnotation(Field.class).storeNullValue()
: isMultiField && (getRequiredAnnotation(MultiField.class).mainField().storeNullValue()
|| Arrays.stream(getRequiredAnnotation(MultiField.class).otherFields())
.anyMatch(InnerField::storeNullValue));
storeEmptyValue = isField ? getRequiredAnnotation(Field.class).storeEmptyValue()
: !isMultiField || getRequiredAnnotation(MultiField.class).mainField().storeEmptyValue()
|| Arrays.stream(getRequiredAnnotation(MultiField.class).otherFields())
.anyMatch(InnerField::storeEmptyValue); |
sothawo
left a comment
sothawo
left a comment
There was a problem hiding this comment.
Thanks for taking that over.
As storeEmpty and nul values are written to the index mapping I think there should be a test in the (Reacive)MappingBuilderUnitTests as well.
sothawo
left a comment
sothawo
left a comment
There was a problem hiding this comment.
The inner field of a multi field can have a different null_value than the outer field. This should be reflected in the mapping builder test, I added the changes for that; it is already working but wasn't in the test.
I think this should be covered in the integration tests as well, using different null values for the outer and inner fields.
And no, I don't know why one would set different values, but it is possible so we need to make sure to support it.
A multi-field doesn’t inherit any mapping options from its parent field.
Signed-off-by: Youssef Aouichaoui <[email protected]>
|
I just tried to use this in a sample application, but it fails when trying to create the index with the Might be an error on our side, or an error in the Elasticsearch client. Won't have any more time today to check. We should have an integration test for this that creates an index with the mapping, stores a value and retrieves it. |
|
TIL: you cannot set a Although the documentation about null_value does not mention this. Will get back to your PR tomorrow. |
3 participants
Closes #2952