eclipse-thingweb / node-wot

A fast and extensible framework to connect any device with your browser and backend applications
https://thingweb.io
Other
166 stars 80 forks source link

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

Open danielpeintner opened 8 months ago

danielpeintner commented 8 months ago

PR https://github.com/eclipse-thingweb/node-wot/pull/1230 reports/fixes/adjusts the case for schema == null (no expected value) for value() function reporting undefined .

Anyhow, there is a much broader issue:

Relates also to:

egekorkan commented 8 months ago

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 commented 8 months ago

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.

VigneshVSV commented 6 months ago

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.

2) 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 commented 6 months ago

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 https://github.com/w3c/wot-thing-description/issues/1234 is relevant.

VigneshVSV commented 6 months ago

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 commented 6 months ago

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

egekorkan commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.