[BUG] NullPointerException in KafkaOpenMetadataTopicConnector if no "group.id" or "local.server.id" are supplied in config.
Closed this issue · 16 comments
Is there an existing issue for this?
- I have searched the existing issues
Current Behavior
NullPointerException in KafkaOpenMetadataTopicConnector if no "group.id" supplied in config.
Expected Behavior
If group.id is a required field then if it is not specified then it should error.
Ifgroup.id is option then start should complete successfully.
Steps To Reproduce
conbfigure a Kafka connector without a group.id and without a local.server.id
Environment
- Egeria:
- OS:
- Java:
- Browser (for UI issues):
- Additional connectors and integration:Any Further Information?
In KafkaOpenMetadataTopicConnector line 171 the code does this :
if (StringUtils.isEmpty((String)consumerProperties.get("group.id"))) {
consumerProperties.put("group.id", serverId);
the issue here is if there is no serverId then putting a null value in the map null pointers
I marked this as a good first issue, as I hope one of the supplied configurations in the Dojo or labs could be amended to remove the group.id and have no local.server.id to reproduce this issue.
https://egeria-project.org/release-notes/3-11/?h=local.server.id may have introduced this. fyi @planetf1
I did change this code in 3.11 as mentioned
Did you get an NPE or is this by inspection?
According to https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#isEmpty-java.lang.CharSequence, calling StringUtils.isEmpty(null) should be fine (and is why that class was used)
in doc
[isEmpty](https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#isEmpty-java.lang.CharSequence-)([CharSequence](https://docs.oracle.com/javase/7/docs/api/java/lang/CharSequence.html?is-external=true) cs)
Checks if a CharSequence is empty ("") or null.
@planetf1 it is the put of a null value that is causing the null pointer - I have amended the words
Ok got it, I believe in my testing I never saw a scenario where serverId was null at that point in the code (cue discussion on more checks ;-) )
But I wonder, should we ever get here with serverId null? Are you saying group.id, and serverid are both null? In which case what would the group.id be set to? Is this a failure to start the connector (if we do find serverId is null)?
serverId is the identity of the server, so if this is NPEing it makes me think there's also a bug elsewhere - either in how the connector is being used here or elsewhere in the code when setting up the server?
I did not know what value to put in local.server.id, so had removed it from my config, it sounds like the code should check the config and ensure we have either a group.id or the local.server.id , if we have neither then the start should fail (gracefully with logging not npe).
Agreed, but I think there may be other problems besides the connector if no server.id (I may be wrong). The server.id always gets set when creating a new server, so I think the path that prompted this was the manual removal from the config.
So it does make sense to add the check here.. but also look at where else serverid gets used.
I can't see it gets used that much. If so then doing the check here, and ensuring the server fails initializing if it can't find a group.id to set makes sense
ConnectorConfigurationFactory.java also has a similar code fragment
so to be safe would also add a check here, again ensuring the init fails
Also would check the code in DataEngineProxyOperationalServices. The constructor gets passed the serverId, and it gets used within this module - including for similar checks. I've not tested it but suspect if the id were removed from the config, it could also NPE.
Removed good first issue since ths is difficult to test.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 20 days if no further activity occurs. Thank you for your contributions.