search

plantbreeding / brapi-Java-TestServer

BrAPI Java Spring-Boot Test Server
https://brapi.org
MIT License
5 stars 7 forks source link

Scale doesn't update to null with PUT /variables #52

Closed ctucker3 closed 3 years ago

ctucker3 commented 3 years ago

Create a trait:

https://test-server.brapi.org/brapi/v2/variables

[{
    "additionalInfo": {},
    "commonCropName": "Tomatillo",
    "contextOfUse": [],
    "defaultValue": null,
    "documentationURL": null,
    "externalReferences": [
        {
            "referenceID": "3ad28fb5-e23f-4561-abb4-c554ee364d17",
            "referenceSource": "breeding-insight.org"
        }
    ],
    "growthStage": null,
    "institution": "Test 4 Updated",
    "language": "english",
    "method": {
        "additionalInfo": {},
        "bibliographicalReference": null,
        "description": "Test",
        "externalReferences": [
            {
                "referenceID": "72301e7c-0b92-476f-9b24-953ae84e9ce3",
                "referenceSource": "breeding-insight.org"
            }
        ],
        "formula": null,
        "methodClass": "Measurement",
        "methodName": "Test Trait Measurement",
        "ontologyReference": null
    },
    "ontologyReference": null,
    "scale": {
        "additionalInfo": {},
        "dataType": "Numerical",
        "decimalPlaces": 2,
        "externalReferences": [
            {
                "referenceID": "dfa62af0-1194-4c55-b04f-3032f61fc34e",
                "referenceSource": "breeding-insight.org"
            }
        ],
        "ontologyReference": null,
        "scaleName": "grams",
        "validValues": {
            "categories": [],
            "max": 10,
            "min": 1
        }
    },
    "scientist": "BI-DEV Admin",
    "status": "active",
    "submissionTimestamp": null,
    "synonyms": [],
    "trait": {
        "additionalInfo": {},
        "alternativeAbbreviations": [],
        "attribute": null,
        "entity": "Plant",
        "externalReferences": [
            {
                "referenceID": "3ad28fb5-e23f-4561-abb4-c554ee364d17",
                "referenceSource": "breeding-insight.org"
            }
        ],
        "mainAbbreviation": null,
        "ontologyReference": null,
        "status": "active",
        "synonyms": [],
        "traitClass": null,
        "traitDescription": "Test",
        "traitName": "Test Trait"
    },
    "observationVariableName": "Test Trait"
}]

Then try to update it (notice change in scale decimal places, valid value min, valid value max):

{
    "additionalInfo": {},
    "commonCropName": "Tomatillo",
    "contextOfUse": [],
    "defaultValue": null,
    "documentationURL": null,
    "externalReferences": [
        {
            "referenceID": "3ad28fb5-e23f-4561-abb4-c554ee364d17",
            "referenceSource": "breeding-insight.org"
        }
    ],
    "growthStage": null,
    "institution": "Test 4 Updated",
    "language": "english",
    "method": {
        "additionalInfo": {},
        "bibliographicalReference": null,
        "description": "Test",
        "externalReferences": [
            {
                "referenceID": "72301e7c-0b92-476f-9b24-953ae84e9ce3",
                "referenceSource": "breeding-insight.org"
            }
        ],
        "formula": null,
        "methodClass": "Measurement",
        "methodName": "Test Trait Measurement UPDATED",
        "ontologyReference": null,
        "methodDbId": "994fa424-63b3-42e0-8909-436ac1edce06"
    },
    "ontologyReference": null,
    "scale": {
        "additionalInfo": {},
        "dataType": "Numerical",
        "decimalPlaces": null,
        "externalReferences": [
            {
                "referenceID": "dfa62af0-1194-4c55-b04f-3032f61fc34e",
                "referenceSource": "breeding-insight.org"
            }
        ],
        "ontologyReference": null,
        "scaleName": "grams",
        "validValues": {
            "categories": [],
            "max": null,
            "min": null
        },
        "scaleDbId": "e940a5b3-6168-446b-9666-d823d1d3c57e"
    },
    "scientist": "BI-DEV Admin",
    "status": "active",
    "submissionTimestamp": null,
    "synonyms": [],
    "trait": {
        "additionalInfo": {},
        "alternativeAbbreviations": [],
        "attribute": null,
        "entity": "Plant",
        "externalReferences": [
            {
                "referenceID": "3ad28fb5-e23f-4561-abb4-c554ee364d17",
                "referenceSource": "breeding-insight.org"
            }
        ],
        "mainAbbreviation": null,
        "ontologyReference": null,
        "status": "active",
        "synonyms": [],
        "traitClass": null,
        "traitDescription": "Test",
        "traitName": "Test Trait UPDATED",
        "traitDbId": "2f6e8ff8-ce19-4ed5-9711-4f60922e716e"
    },
    "observationVariableDbId": "40deb32b-a416-410e-ae7e-11c42c98b996",
    "observationVariableName": "Test Trait"
}

Result:

The scale decimal places, valid value min, valid value max don't change to be null.

BrapiCoordinatorSelby commented 3 years ago

This is a tough issue to solve because of the JSON deserializer in Java. By default, Java can't tell the difference between an explicit null and a missing field. The server is currently configured to not update missing fields. I think this can be resolved with some custom deserializer code to detect the difference, but it will take some research to find a clever way to do that.

Example: This

...
"dataType": "Numerical",
"decimalPlaces": null,
"externalReferences": 
...

is read the same as this

...
"dataType": "Numerical",
"externalReferences": 
...
ctucker3 commented 3 years ago

Ah got ya, so is this an issue for all of the endpoints?

BrapiCoordinatorSelby commented 3 years ago

yea unfortunately With String fields you can kinda work around it by using an empty string instead of null to mean "present but no data", but for Integers its a little more difficult. decimalPlaces has to be positive, so a -1 value could be a workaround there ... but -1 might be a valid min value.

BrapiCoordinatorSelby commented 3 years ago

Update: The solution to this is to make the POJO's with every field as Optional<>. The fields default to null, then the setter methods are used to create a new Optional<> object when JSON is sent. If object.getField() == null then the field was not modified and the field was not present in the incoming JSON. If object.getField().isPresent() == false then the field was modified and explicitly set to null. If object.getField().isPresent() == true then the field has a new value to update.

This process is somewhat time consuming to implement, so not all the PUT endpoints are implemented yet. Per @ctucker3 's request the following endpoints have this extra structure. Other endpoints will be implemented by request. PUT /variables PUT /traits PUT /scales PUT /methods PUT /programs