meaningless entryTypeDefinition.aCollectionOf property defiition

[redmitry]

Hi all,

There is invalid entryTypeDefinition.aCollectionOf property definition in the spec.

"aCollectionOf": {
    "description": "If the entry type is a collection of other entry types, (e.g. a Dataset is a collection of Records), then this attribute must list the entry types that could be included. One collection type could be defined as included more than one entry type (e.g. a Dataset could include Individuals or Genomic Variants), in such cases the entries are alternative, meaning that a given instance of this entry type could be of only one of the types (e.g. a given Dataset contains Individuals, while another Dataset could contain Genomic Variants, but not both at once).",
    "includedConcepts": {
        "$ref": "../common/basicElement.json",
        "type": "array"
    }
},

"includedConcepts" here is something to be ignored by schemas, so basically equivalent

"aCollectionOf": {}

Best,

Dmitry

[datsirul]

@jrambla This issue by @redmitry was also found by our work on the code generator. The file /entryTypeDefinition.yaml curently has:

  aCollectionOf:
    description: If the entry type...
    includedConcepts:
      type: array
      $ref: ../common/basicElement.yaml

I'm not sure what is the reason for the includedConcepts key, but for it to be a valid OpenAPI object it should be removed, like so:

  aCollectionOf:
    description: If the entry type...
    type: array
    $ref: ../common/basicElement.yaml

Note:
After fixing the .yaml files, the .json should be fixed as well by running the schema conversion script located in the bin folder

[jrambla]

This one is trickier , IMO, as it will break the current implementations.
The reason for includedConcepts was to make clear what the aCollectionOf attributes mean.
It could be redudant, but could not be removed now w/o a proper deprecation.
Could the alternative be considering aCollectionOf as an object that has only one property and ammend the spec like that w/o breaking changes?

[jrambla]

@redmitry @datsirul any answer to my question above?

[redmitry]

@redmitry @datsirul any answer to my question above?

The point is that the current schema is wrong.
Should be either:

"aCollectionOf":  {
    "description": "If the entry...",
    "type": "object",
    "properties":  {
        "includedConcepts": {
            "type": "array",
            "items": {
                "$ref": "../common/basicElement.json"
            }
        }
    }
}

OR (IMO what the intention was):

"aCollectionOf": {
    "description": "If the entry...",
    "type": "array",
     "items": {
         "$ref": "../common/basicElement.json"
     }
}

Cheers,

D.

[jrambla]

IMU, the second one will be a breaking change while the first one is not (THIS is the important thing to me).
Am I right?

[redmitry]

IMU, the second one will be a breaking change while the first one is not (THIS is the important thing to me). Am I right?

No. The current schema is just invalid. So we can't break something that is already broken.

"aCollectionOf": {
    "description": "If the entry...",
    "includedConcepts": {

here the "includedConcepts" should be ignored by schema parser. If the intention was to define a property, e.g. for json like:

{
  "aCollectionOf": {
      "includedConcepts": 
  }
}

this is 1.

Actually, there are two errors in the schema.

"includedConcepts": {
    "$ref": "../common/basicElement.json",
    "type": "array"
}

"includedConcepts" is "type": "array" AND "type": "object" (basicElement.json) in the same time.
I suppose that "$ref" refers to the array items :

"type": "array",
"items": {
    "$ref": "../common/basicElement.json"
 }

[jrambla]

@redmitry can you do a PR for the option:

"aCollectionOf": {
    "description": "If the entry...",
    "type": "array",
     "items": {
         "$ref": "../common/basicElement.json"
     }
}

Thanks!