ChrisSamo632 commented on a change in pull request #4755:
URL: https://github.com/apache/nifi/pull/4755#discussion_r560819377
##########
File path:
nifi-nar-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/main/java/org/apache/nifi/elasticsearch/ElasticSearchClientServiceImpl.java
##########
@@ -90,6 +93,15 @@ public void onEnabled(final ConfigurationContext context)
throws InitializationE
try {
setupClient(context);
responseCharset =
Charset.forName(context.getProperty(CHARSET).getValue());
+
+ // re-create the ObjectMapper in case the SUPPRESS_NULLS property
has changed - the JsonInclude settings aren't dynamic
+ mapper = new ObjectMapper();
+ if
(ALWAYS_SUPPRESS.getValue().equals(context.getProperty(SUPPRESS_NULLS).getValue()))
{
+ mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
Review comment:
I couldn't re-produce the `null` scenario I'm sure I saw while trying to
use the PutElasticsearchRecord processor in 1.11.4.
However, I do still think there's something to change here, in that you
can't currently output `null`/empty values to Elasticsearch using this
processor (so kind of the reverse of the original ticket) due to the
`removeEmpty` method in the `PutElasticsearchRecord` processor. Looking at this
method, it could also result in empty nested objects/arrays being sent to
Elasticsearch currently, which wouldn't always be ideal.
So, I've changed the code to:
* Not `removeEmpty` within `PutElasticsearchRecord`
* Use the Jackson ObjectMapper to remove `null`/empty fields (including
empty nested arrays/objects) when `ALWAYS_SUPPRESS` is selected
* Leave the `null`/empty fields in the output to Elasticsearch when
`NEVER_SUPPRESS` is selected
I've added a Flow template/export to the Jira ticket (included converting
the data into Avro as that's what I use in the Flow where I believe I saw this
behaviour in 1.11.4 before switching to the `PutElasticsearchHttpRecord`
instead - although that didn't cause a re-production of the issue either).
Re-based the branch from `main` and updated both unit and integration tests
to cover these scenarios. Default behaviour of the `PutElasticsearchRecord`
processor remains one of removing `null`/empty fields (i.e. `ALWAYS_SUPPRESS`
in `ElasticSearchClientService`). I think my concerns with the integration
tests above are explained by the fact that testing the service directly wasn't
executing the `remoteEmpty` method of the processor.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
