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

InteractionOutput - no schema/schema.type and the expectations of the value() function #1243

Open
danielpeintner opened this issue Feb 16, 2024 · 11 comments
Labels
core Issues with the core library

Comments

@danielpeintner
Copy link
Member

PR #1230 reports/fixes/adjusts the case for schema == null (no expected value) for value() function reporting undefined .

Anyhow, there is a much broader issue:

  • what is a valid schema?
  • schema.type == null since we use oneof
  • optional return value
  • ...

Relates also to:

@egekorkan
Copy link
Member

what is a valid schema?

That is a big question, maybe, but anything that validates the JSON Schema meta schema is valid: https://github.com/json-schema-org/json-schema-spec/blob/draft-handrews-json-schema-validation-00/schema.json

My bigger question is why we are looking inside a data schema, it should be just "passed over". If it is validation, the whole property is a schema, with action input or output etc.

@relu91
Copy link
Member

relu91 commented Feb 17, 2024

My bigger question is why we are looking inside a data schema, it should be just "passed over". If it is validation, the whole property is a schema, with action input or output etc.

Because we are not using the schema just to validate the payload but also to guide the decoding of the payload. Basically, having the schema allows us to convert non-JSON content types to Javascript objects that can be later validated with the schema.

what is a valid schema?

Given what is said above I think that for us a valid schema (to be used in the value function) can be defined as (more or less):

function validSchema(schema) {
   return schema.type != null || schema.const != null || schema.enum != null || schema.oneOf?.reduce((acc, val)=> acc || validSchema(val), true)
}

with const or enum we can infer the type (number, string, object, or array) and the oneOf is just recursive. I should have included all the possibilities looking at https://www.w3.org/TR/wot-thing-description11/#dataschema . Let me know if I missed something.

P.s. I'd probably prefer that const and enum should always explicitly state the type but it could be just a warning.

@relu91 relu91 added the core Issues with the core library label Feb 24, 2024
@VigneshVSV
Copy link

VigneshVSV commented May 3, 2024

Hi, I have following use case:

  1. nullable properties of fixed type -

For example, I have the following property definition

"state": {
      "title": "state",
      "description": "current state machine state if state machine present",
      "const": false,
      "readOnly": true,
      "writeOnly": false,
      "type": "string",
      "oneOf": [
        {
          "type": "string"
        },
        {
          "type": "null"
        }
      ],
      "forms": [
        {
          "href": https://laptop-f60cu35d:8083/spectrometer/ocean-optics/USB2000-plus/state,
          "op": "readproperty",
          "htv:methodName": "GET",
          "contentType": "application/json"
        }
      ],
      "observable": false
    },

state is a string property, but it can be optionally None (python) in certain user defined cases. So I would like to claim that its one of string or null. However node-wot client throws the following error

wot-bundle.min.js:69468 Uncaught (in promise) Error: Invalid value according to DataSchema
    at InteractionOutput.<anonymous> (wot-bundle.min.js:69468:23)
    at Generator.next (<anonymous>)
    at fulfilled (wot-bundle.min.js:69369:58)

Since oneOf claims that the type can be more than one, if I skip the type field altogether,

"state": {
      "title": "state",
      "description": "current state machine state if state machine present",
      "const": false,
      "readOnly": true,
      "writeOnly": false,
      "oneOf": [
        {
          "type": "string"
        },
        {
          "type": "null"
        }
      ],
      "forms": [
        {
          "href": https://laptop-f60cu35d:8083/spectrometer/ocean-optics/USB2000-plus/state,
          "op": "readproperty",
          "htv:methodName": "GET",
          "contentType": "application/json"
        }
      ],
      "observable": false
    }

I get another error:

wot-bundle.min.js:69453 Uncaught (in promise) Error: No schema type defined
    at InteractionOutput.<anonymous> (wot-bundle.min.js:69453:23)
    at Generator.next (<anonymous>)
    at wot-bundle.min.js:69372:71
    at new Promise (<anonymous>)
    at __awaiter (wot-bundle.min.js:69368:12)
    at InteractionOutput.value (wot-bundle.min.js:69438:16)
    at updateState (App.svelte:44:89)

So I am unable to use oneOf at all.

  1. Since already there is discussion about enum, I would (also) like to have allowing of multiple types either by self inference by node-wot or the possibility to use oneOf. Following is an example of a property:
background_correction": {
      "title": "background_correction",
      "description": "set \"AUTO\" for Seabreeze internal black level correction, \"CUSTOM\" to load your own background, or None for no background correction",
      "const": false,
      "default": null,
      "readOnly": false,
      "writeOnly": false,
      "oneOf": [
        {
          "type": "string"
        },
        {
          "type": "null"
        }
      ],
      "enum": [
        "AUTO",
        "CUSTOM",
        null
      ],
      "forms": [
        {
          "href": "https://LAPTOP-F60CU35D:8083/spectrometer/ocean-optics/USB2000-plus/background-correction",
          "op": "readproperty",
          "htv:methodName": "GET",
          "contentType": "application/json"
        },
        {
          "href": "https://LAPTOP-F60CU35D:8083/spectrometer/ocean-optics/USB2000-plus/background-correction",
          "op": "writeproperty",
          "htv:methodName": "PUT",
          "contentType": "application/json"
        }
      ],
      "observable": false

Since my allowed values are "AUTO", "CUSTOM" and None, the type should be one of string or null. In this case, the type field and enum field contradict one-another for me.

For both the above examples, the python side of my code is sensible in a pythonic fashion, I generate the above from following code:

class MyObject(Thing):

  state = String(default=None, allow_None=True, URL_path='/state', readonly=True,
                  fget= lambda self :  self.state_machine.current_state  if hasattr(self, 'state_machine') else None,  
                  doc='current state machine state if state machine present') #type: type.Optional[str]
  
  background_correction = Selector(objects=['AUTO', 'CUSTOM', None], default=None, allow_None=True, 
                          URL_path='/background-correction',
                          doc="set 'AUTO' for Seabreeze internal black level correction, 'CUSTOM' to load your own background, or None for no background correction") #type: typing.Union[str, None]

The String and Selector objects are to be taken as meaningful python objects. Since it makes "pythonic sense", I really cannot change it and would like to account for 'allow_None' in the TD that I generate instead.

@danielpeintner
Copy link
Member Author

I did not look into it more closely but I think you assume that "type": "null" means a value can be null/undefined/optional.
This is not the case. In JSON schema validation this means that the value MUST be "null"

Maybe w3c/wot-thing-description#1234 is relevant.

@VigneshVSV
Copy link

VigneshVSV commented May 3, 2024

That would make sense why it does not validate correctly. However what if I want null instead of "null"

The NullSchema is written as follows in TD:

"Metadata describing data of type null. This subclass is indicated by the value null assigned to type in DataSchema instances. This Subclass describes only one acceptable value, namely null. It is important to note that null does not mean the absence of a value. It is analogous to null in JavaScript, None in Python, null in Java and nil in Ruby programming languages. It can be used as part of a oneOf declaration, where it is used to indicate, that the data can also be null."

What I really do is serializing None type of python which turns out to be null and it is exactly the suggested use case as per doc above for my python code you can see in previous comment.

I still think I am missing something conceptually and if someone can explain that would help.

Thanks in advance.

@VigneshVSV
Copy link

Also serializing None to "null" instead of null does not make sense.

@egekorkan
Copy link
Member

This issue got diverted a bit but @VigneshVSV 's problem is not about null. null should not be represented as "null", which is a string. It is the same issue above that node-wot always expects a type keyword

@VigneshVSV
Copy link

VigneshVSV commented May 3, 2024

a plain null also raises an issue:

"state": {
      "title": "state",
      "description": "current state machine state if state machine present",
      "const": false,
      "readOnly": true,
      "writeOnly": false,
      "oneOf": [
        {
          "type": "string"
        },
        {
          "type": null
        }
      ],
      "forms": [
        {
          "href": "https://LAPTOP-F60CU35D:8083/spectrometer/ocean-optics/USB2000-plus/state",
          "op": "readproperty",
          "htv:methodName": "GET",
          "contentType": "application/json"
        }
      ],
      "observable": false
    }

the TD playground says:

data/properties/state/oneOf/1/type must be string

both with and without the type field

@egekorkan
Copy link
Member

Yes the value of type must be a string, I meant the payloads which are delivered. Those have to be null. Once #1279 is merged, you can disable validation and your issue should disappear for a while. In the long term, node-wot should not do schema validation in this restrictive way. You can find me in Discord and I can further explain.

@egekorkan
Copy link
Member

egekorkan commented May 4, 2024

By the way, this issue shows up even for our test thing: http://plugfest.thingweb.io:8083/testthing -> The void actions simply do not work

@danielpeintner
Copy link
Member Author

danielpeintner commented May 6, 2024

By the way, this issue shows up even for our test thing: http://plugfest.thingweb.io:8083/testthing -> The void actions simply do not work

I see, POST http://plugfest.thingweb.io:8083/testthing/actions/void-void reports No schema defined
Anyhow, this has been fixed and the node-wot version on http://plugfest.thingweb.io seems to be out-dated.

I tried it locally and it works fine

EDIT: I pulled the node-wot updates to plugfest.thingweb.io.
@egekorkan please have a look and check whether it still behaves wrongly. I think it is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues with the core library
Projects
None yet
Development

No branches or pull requests

4 participants