Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better validate "oneOf" choices #955

Closed
mmccool opened this issue Aug 17, 2020 · 7 comments
Closed

Better validate "oneOf" choices #955

mmccool opened this issue Aug 17, 2020 · 7 comments
Assignees
Labels
Not Clear when the issue is not clear and further input is needed by its author V1.1 should be resolved in v1.1

Comments

@mmccool
Copy link
Contributor

mmccool commented Aug 17, 2020

In the proposed "combination" security scheme, there are two choices, exactly one of which should be selected. The table, however, just says that both are "optional". There is an assertion in the text that specifies the "one of" constraint, but maybe there is a better way to specify this in the table. (eg "optional, but only one of ... can be chosen")

@mmccool mmccool self-assigned this Aug 17, 2020
@mmccool
Copy link
Contributor Author

mmccool commented Aug 17, 2020

I don't know if this applies to anything else in the spec, it's worth thinking about.

@relu91
Copy link
Member

relu91 commented Aug 17, 2020

I think this also relates to w3c/wot-scripting-api#238.

@relu91
Copy link
Member

relu91 commented Sep 2, 2020

I did some tests. This piece of JSONSchema seems to work quite well:

{
  "$schema": "https://json-schema.org/draft/2019-09/schema",
  "type": "object",
  "properties": {
    "scheme": {
      "type": "string",
      "enum": [
        "combination"
      ]
    },
    "oneOf": {
      "type": "array",
      "minItems": 2,
      "items": {
        "type": "string"
      }
    },
    "allOf": {
      "type": "array",
      "minItems": 2,
      "items": {
        "type": "string"
      }
    }
  },
  "oneOf":[
    {"required": [
                "oneOf"
            ]},
    {"required": [
                "allOf"
            ]}
  ],
  "required": [
    "scheme"
  ]
}

For example, it rejects these objects:

{
  "scheme":"combination"
}
{
  "scheme":"combination",
  "oneOf":["a","b"],
  "allOf":["a","b"]
}

On the other hand, it accepts correctly this one:

{
  "scheme":"combination",
  "oneOf":["a","b"]
}

I still miss how to check that the content of oneOf or allOf should be present as one of the SecuritySchema keys.

Edit: this stackoverflow question suggests that is not possible using just JSON Schema.

@mmccool
Copy link
Contributor Author

mmccool commented Sep 2, 2020

To be clear, my PR already handles this constraint in the JSON schema. What I was actually wondering about was whether we needed a constraint in the TTL shapes as well.

@mmccool
Copy link
Contributor Author

mmccool commented Sep 2, 2020

There is still an issue with matching names in securityDefinitions to usages in oneOf or allOf (and "security"!) but that's a different issue (which we probably should log).

@mmccool
Copy link
Contributor Author

mmccool commented Sep 3, 2020

Anyhow, I realized that my original point was also about how to express this constraint in the table. Calling each of "oneOf" and "allOf" optional is not quite correct. EXACTLY ONE of them is in fact mandatory, and so if one is used the other should not be. The problem of how to express the constraint formally and validate it is solved I think (using JSON Schema; the PR includes a rule that handles this). What I would like to do is figure out how to express it more clearly in the table... maybe something like "exclusive"?

@egekorkan egekorkan added V1.1 should be resolved in v1.1 Not Clear when the issue is not clear and further input is needed by its author labels Oct 26, 2021
@egekorkan
Copy link
Contributor

This will be hopefully reworked in the TD 2.0 and it is clear enough in TD 1.1. So closing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not Clear when the issue is not clear and further input is needed by its author V1.1 should be resolved in v1.1
Projects
None yet
Development

No branches or pull requests

3 participants