onvif / specs

ONVIF Network Interface Specifications
Other
325 stars 87 forks source link

Remove redundant array definition for various types #429

Closed jflevesque-genetec closed 1 month ago

jflevesque-genetec commented 1 month ago

I've updated the OpenAPI document to remove some redundant array definition.

New model:

{
  "CamerasConfiguration": {
    "ShareToken": "string",
    "EndpointReferences": "string"
  },
  "JWTConfiguration": {
    "Audiences": [
      "string"
    ],
    "TrustedIssuers": [
      "string"
    ],
    "JWTCertificate": [
      "string"
    ],
    "JWTTrustAnchor": [
      "string"
    ]
  },
  "UplinkConfiguration": {
    "RemoteAddress": "string",
    "UserLevel": "Administrator",
    "UplinkTrustAnchor": [
      "string"
    ]
  }
}

Old model:

{
  "CamerasConfiguration": [
    {
      "ShareToken": [
        {
          "Value": "string"
        }
      ],
      "EndpointReferences": [
        {
          "Address": "string"
        }
      ]
    }
  ],
  "JWTConfiguration": [
    {
      "Audiences": [
        "string"
      ],
      "TrustedIssuers": [
        "string"
      ],
      "JWTCertificate": [
        {
          "Value": "string"
        }
      ],
      "JWTTrustAnchor": [
        {
          "Value": "string"
        }
      ]
    }
  ],
  "UplinkConfiguration": [
    {
      "RemoteAddress": "string",
      "UserLevel": "Administrator",
      "UplinkTrustAnchor": [
        {
          "Value": "string"
        }
      ]
    }
  ]
}
ocampana-videotec commented 1 month ago

@jflevesque-genetec CamerasConfiguration was changed to array yesterday, to accommodate to do a bulk request for more cameras at once. Within it, EndpointReferencesis an array if you want to indicate more then one entry point for the callback for rendundacy.

Configuring only one Uplink configuration is fine for me, if other members agree as well.

Finally, I think that your simplification is getting rid of the minimum checks for mandatory fields in the defined objects and their structure. If just we put a string it is less easy to understand what should be put there. I think that the specs should guide the developers.

jflevesque-genetec commented 1 month ago

@jflevesque-genetec CamerasConfiguration was changed to array yesterday, to accommodate to do a bulk request for more cameras at once. Within it, EndpointReferencesis an array if you want to indicate more then one entry point for the callback for rendundacy.

Configuring only one Uplink configuration is fine for me, if other members agree as well.

Finally, I think that your simplification is getting rid of the minimum checks for mandatory fields in the defined objects and their structure. If just we put a string it is less easy to understand what should be put there. I think that the specs should guide the developers.

If we want to do bulk enroll, I feel like the array definition should be top level, something like

[
  {
    "CamerasConfiguration": {
      "ShareToken": "string",
      "EndpointReferences": "string"
    },
    "JWTConfiguration": {
      "Audiences": [
        "string"
      ],
      "TrustedIssuers": [
        "string"
      ],
      "JWTCertificate": [
        "string"
      ],
      "JWTTrustAnchor": [
        "string"
      ]
    },
    "UplinkConfiguration": {
      "RemoteAddress": "string",
      "UserLevel": "Administrator",
      "UplinkTrustAnchor": [
        "string"
      ]
    }
  }
]

Having this type of model enables a client to modify any of the configurations per device instead of being forced to share some of them or make multiple calls if configs are different.