npe when sorting on metadata property in results
Closed this issue · 7 comments
In corpus-frontend you can add metadata properties to the results like this (year, author, residence):
When sorting on such a property and it holds a null value an npe occurs:
java.lang.NullPointerException
at nl.inl.blacklab.resultproperty.HitPropertyDocumentStoredField.<init>(HitPropertyDocumentStoredField.java:46)
at nl.inl.blacklab.resultproperty.HitPropertyDocumentStoredField.copyWith(HitPropertyDocumentStoredField.java:63)
at nl.inl.blacklab.resultproperty.HitProperty.reverse(HitProperty.java:265)
at nl.inl.blacklab.resultproperty.HitProperty.deserialize(HitProperty.java:125)
at nl.inl.blacklab.server.requesthandlers.SearchParameters.hitsSortSettings(SearchParameters.java:514)
at nl.inl.blacklab.server.requesthandlers.SearchParameters.hitsSorted(SearchParameters.java:562)
Shall I do a PR for this? Or should this be discussed first...
Suggestion:
let return DocPropertyStoredField.fromArray(docPropStoredField.get(hits.doc(result))); return a PropertyValueString with an empty String when docPropStoredField is null.
A PR is always welcome! But I don't quite follow what's going on here.
The NPE seems to occur inside the constructor, called from copyWith, but the line numbers don't match up with the source code (at least if I look at dev or v3.0.1). Your solution refers to code around line 46, but that's in the get() method, not the constructor.
And how would docPropStoredField become null? Maybe we should guard against that (i.e. throw IllegalArgumentException in the constructor); I've been adding guards like that in the various Spans classes in a branch I'm working on and it has surfaced a few bugs already.
Maybe your PR will clear up my confusion. :-)
Been looking where docPropStoredField may become null, cannot find it yet, but it has to originate from QueryTool#sortHits(String sortBy, String annotationName) (dev branch).
I thought the error occurred in BLS called from the frontend? QueryTool is a commandline test tool, so cannot have caused that problem, I think.
Maybe we should just check for null (and throw) in the HitProperty constructor; that's a good idea anyway and the stack trace will reveal the culprit.
I am debugging now
closing this, because I saw the bug in:
blacklab-server-2.1.1.war
corpus-frontend-2.1.0.war
and I need to run higher versions in which this issue seems resolved
Alright, thanks for investigating. I will still make a mental note to add more guards against invalid parameter values in HitProperty and elsewhere.