CVOC: Fix NPE

[luddaniel]

What this PR does / why we need it:

Fix NPE in complex case of CVOC : free text + partial data using flexible params in retrievalUri

• Original and merged PR : CVOC : allow flexible params in retrievalUri (Ontoportal integration) #10404

Special notes for your reviewer:

Code is tested and will fix already merged code to develop.

I'm fixing my own code here

[qqmyers]

It seems like line 501 above is the only thing that could cause the NPE? And in that case the paramName can't be null but the value could be? I wonder if checking on line 571 to see if retrievalUri.contains(paramName) would be a better check - if the param name is in the Uri and it isn't replaced, that's a problem you wouldn't want to silently ignore? Or do you in the free text case? Seems like the code shouldn't just call the remote service with a broken URL, so maybe the NPE should be caught and used to avoid the call to the service instead?

[luddaniel]

CVOC is sooo complexe...

• you are right the only case where there is NPE can happen is : L501 retrievalUri = replaceRetrievalUriParam(retrievalUri, f.getDatasetFieldType().getName(), f.getValue()); when value is null which can happen with termUri filled but when a parameter set in retrievalUri has no value.
• retrievalUri.contains(paramName) would be a better check - if the param name is in the Uri and it isn't replaced, that's a problem you wouldn't want to silently ignore if param is null, empty or wrong I just do nothing, either it cannot be done, data are wrong or json setting is wrong. But that being said, a "if paramName does not exist in retrievalUri then logger.warning" can be done.
• the code shouldn't just call the remote service with a broken URL, so maybe the NPE should be caught and used to avoid the call to the service instead? hum, you are right, for a wrong parameter we could simply return; instead of performing the call.

[luddaniel]

@qqmyers 79b8d89 modifications have been made, if you can review / approve :)

[qqmyers]

Looks good - thanks for the update.

[qqmyers]

@luddaniel - I ran across another NPE when trying to use the latest dev branch with a skomos and ror script. I was getting a case where the value of a datasetfieldvalue was null. Either of the two changes in a520799 would fix it, but I was surprised to see that a DatasetFieldValue would return a null value (versus there not being a DatasetFieldValue at all).

I guess two questions - any insight into why there's a null value? Assuming no ideas about a deeper cause, could you confirm that this fix (one of the two checks in the commit) makes sense and perhaps add it to your PR so it can all get tested together?

Also - FWIW: I think the fix for the one failing test is now in develop so if you update your branch, you should get all tests passing.

[luddaniel]

@qqmyers I don't know how this null value came along with Dataverse releases or different works on CVOC. I did came across the same issue one day as I fixed this issue here : https://github.com/IQSS/dataverse/pull/10505/files#diff-171c256be8744ca0742a9570ed33a3e335eae0544d08be04545ef31fdd5328d1R485 de facto I approve your modification ^^

[coveralls]

coverage: 20.57% (-0.003%) from 20.573%
when pulling af88123 on Recherche-Data-Gouv:10404-fix-NPE
into 3c55c3f on IQSS:develop.