dataverse icon indicating copy to clipboard operation
dataverse copied to clipboard

CVOC : allow flexible params in retrievalUri (Ontoportal integration)

Open luddaniel opened this issue 1 year ago • 1 comments

@qqmyers Following agreements on next steps of #10145 here is a way to add flexibility in retrievalUri. We can now add named fields (child or siblings) of term-uri-field and capacity to encodeUri value when usefull. Example below :

cvoc_conf.json contains :

"field-name": "keyword",
"term-uri-field": "keywordTermURL",
"retrieval-uri": "https://data.agroportal.lirmm.fr/ontologies/{keywordVocabulary}/classes/{encodeUrl:keywordTermURL}",
"managed-fields": {
        "vocabularyName": "keywordVocabulary",
        "termName": "keywordValue",
        "vocabularyUri": "keywordVocabularyURI"
    },

Resulting in retrieval-uri = https://data.agroportal.lirmm.fr/ontologies/AGROVOC/classes/http%3A%2F%2Faims.fao.org%2Faos%2Fagrovoc%2Fc_331529

This code was tested with a classic skomos https://skosmos.loterre.fr/rest/v1/data?uri={0} and it's still working.

Refactoring has been made regarding the use of registerExternalTerm function around dataset imports / updates methods where it was used while parsing json fields meaning imcomplete related fields. Now registerExternalTerm is called after dataset metadata are ready to be commited and directly as step of Command of dataset create and update.

luddaniel avatar Mar 21 '24 14:03 luddaniel

Coverage Status

coverage: 20.602% (-0.001%) from 20.603% when pulling a72bb4c4daf7ccba0df3f3960b9f8307d3051fd4 on Recherche-Data-Gouv:9276-allow-flexible-params-in-retrievaluri-cvoc into a329f293f3e198074b71dce794444d3b3850cdf9 on IQSS:develop.

coveralls avatar Mar 21 '24 14:03 coveralls

Hello @qqmyers Thanks for the review,

  • Release note has been added for both PR here (#10404 + #10331) .
  • PR synchronized with develop
  • https://github.com/gdcc/dataverse-external-vocab-support/pull/20 updated

For testing / unit tests / integration tests, we decided to go for unit tests of methods of DatasetFieldServiceBean, coming in another PR. Other kind of testing looks complicated for now. I'll talk to you on slack for this topic.

luddaniel avatar Apr 15 '24 14:04 luddaniel

Decided with @qqmyers, the unit tests for all PR for Ontoportal / CVOC will be pushed after the merging of PR as it will require merged codes.

luddaniel avatar Apr 18 '24 14:04 luddaniel

@luddaniel - There's 1 test failing with this PR - a 500 error returned instead of a 200 response at edu.harvard.iq.dataverse.api.EditDDIIT.testUpdateVariableMetadata(EditDDIIT.java:153) which is caused ultimately by an attempt to save a DatasetVersion that doesn't have a createTime set. From the lengthy stack trace, it looks like that is due to the call to ctxt.dsField().registerExternalVocabValues(df). That calls em.flush() if an external val is found - perhaps that or something else in the call is causing the DatasetVersion itself to be flushed to the db. (I also see @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW) on registerExternalTerm().)

This all happens before the date gets set in UpdateDatasetVersionCommand line ~261 (setting the last update time also sets the createTime if it was null) so anything causing the new DatasetVersion to be written to the db would trigger the problem since createTime is defined as non-nullable.

If that's all correct, I think editing/perhaps having to add a CVOC value to a published version (causing a new version to be created) in the UI would trigger the issue. Assuming you can repeat the problem, it might be that we no longer need the flush(), or that setting the date can be moved earlier in the method, etc.

qqmyers avatar Apr 22 '24 20:04 qqmyers

@qqmyers your tips with https://github.com/IQSS/dataverse/pull/7334/commits/da87bd1cc5d2aacb8bd0c1c6a5c5750b8a2f9d10 were correct, I just needed to set the created time before registerExternalVocabValuesIfAny() function for whatever reason... Tested without @TransactionAttribute(TransactionAttributeType.REQUIRES_NEW) it seems to work just fine.

Suggested change on release note was added

luddaniel avatar May 03 '24 09:05 luddaniel