ebi-ait / checklist

Template repository for checklists
Apache License 2.0
1 stars 0 forks source link

optional attributes schema are not working as expected #32

Closed theisuru closed 2 weeks ago

theisuru commented 4 weeks ago

ENA checklist is an array of attributes:

{
...
 "attributes":[
       {
            "tag": "collection date",
            "value": "2021-03-09"
        },
        {
            "tag": "geographic location (country and/or sea)",
            "value": "Saudi Arabia"
        },
...
]}

Some of the tags are mandatory, some are optional and others are recommended which is another form of optional.

We used contains and minContains keywords to validate fields. contains expects the whole object within it to match with schema. Therefore, recommended and optional fields are ignored and not working as expected. minContains enabled optional attributes.

Example: This should fail validation because of "unit", but schema ignores the whole field.

sub-schema (ERC000052):

{
        "contains" : {
          "type" : "object",
          "description" : "Length of time from the beginning of the trial until the end of the trial",
          "properties" : {
            "tag" : {
              "enum" : [ "trial length" ]
            },
            "value" : {
              "type" : "string",
              "pattern" : "(0|((0\\.)|([1-9][0-9]*\\.?))[0-9]*)([Ee][+-]?[0-9]+)?"
            },
            "unit" : {
              "type" : "string",
              "enum" : [ "days", "hours", "minutes", "weeks", "years" ]
            }
          },
          "required" : [ "tag", "value" ]
        },
        "minContains" : 0,
        "maxContains" : 1,
        "recommended" : true
}

data fragment (SAMEA7025236):

{
    "tag": "trial timepoint",
    "value": "35",
    "unit": "day"
}

definition of done

amnonkhen commented 4 weeks ago

A solution is the attributes property will be defined in the following way:

  1. an a array of objects, with tag, value, units properties
  2. tag is en enum that can accept all synonyms of all attribute tags
    "tag": {
        "type": "string",
        "enum": [
          "tag1", "Tag1", "tag_one",
          "tag2", "Tag2", "tag_two",
          ...
          ]
      }
  3. tag and value properties are required, units is optional
  4. value is conditional using oneOf according to the value of `taga:
    
    "value": {
        "oneOf": [
          {
            "if": {
              "properties": {
                "tag": {
                  "enum": ["tag1", "Tag1", "tag_one"]
                }
              }
            },
            "then": {
              "type": "string",
              "pattern": "^[A-Za-z]+$"  // Example: Only alphabets
            }
          },
          {
            "if": {
              "properties": {
                "tag": {
                  "enum": ["tag2", "Tag2", "tag_two"]
                }
              }
            },
            "then": {
              "type": "integer",
              "minimum": 0  // Example: Only non-negative integers
            }
          },
    ...
    }
  5. mandatory elements will be ensured by an allOf condition for attrbitues:
    "allOf": [
    {
      "contains": {
        "properties": {
          "tag": {
            "enum": ["tag1", "Tag1", "tag_one"]
          }
        }
      }
    },
    {
      "contains": {
        "properties": {
          "tag": {
            "enum": ["tag2", "Tag2", "tag_two"]
          }
        }
      }
    },
amnonkhen commented 3 weeks ago

@theisuru please continue this ticket. The unit tests are failing. The invalid document gets accepted while the invalid ones get rejected. I tried validating using a separate json schema validator and got different errors. I suggest trying this as well (I used an online validator). The error s were related to multiple matches in the oneOf part of the value property.

theisuru commented 3 weeks ago

Seems current schema is not correct. The behaviour of schema is different from what we expect from if then else block in programming. JSON Schema is very permissive and accept unexpected if conditions. It is documented here. I am looking for a solution to this.

theisuru commented 2 weeks ago

After the recommendation from the stackoverlow, this seems to be working as expected. All the local test cases are working fine.

theisuru commented 2 weeks ago

merged this directly with main since there are too much deviation from the branch