Fix NullPointerException in Monitor.getQuery when query is not present #12736
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The javadoc for Monitor.getQuery suggests that it will return null when the query is not present but instead it throws a NullPointerException.
This is because
bytesHolder[0]
will still be null after the call tosearch
if it does not find any query matching the id in the monitor's index. Then when theserializer.deserialize
call is made, any attempts to index the BytesRef will throw the NullPointerException. This will happen here when using the serializer/deserializer created withMonitorQuerySerializer.fromParser
.Alternative Approach
This could instead be fixed just for the case of
MonitorQuerySerializer.fromParser
by making the serializer/deserializer returned from it null-safe (i.e. it checks if the input is null and returns null). But I went with this approach since it would be robust against custom implementations of theMonitorQuerySerializer
interface that also do not check for null like thefromParser
implementation.Tests
I introduced
testGetQueryNotPresent
which fails on master with the NullPointerException, but passes on this branch. I also added a correspondingtestGetQueryPresent
which passes on both master and this branch. Since I needed a monitor with persistence for these two new tests I refactored creating a monitor with persistence into a helper functionnewMonitorWithPersistence
that gets reused throughout theTestMonitorPersistence
file.