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

observeallproperties and unobserveallproperties methods are missing #312

Open
egekorkan opened this issue Mar 29, 2021 · 11 comments
Open

observeallproperties and unobserveallproperties methods are missing #312

egekorkan opened this issue Mar 29, 2021 · 11 comments
Labels
API-improvement Suggestions for changing the API enhancement Thoughts and ideas about possible improvements for next iteration Planned or postponed topics for the future priority: high Issues that will be tackled in 2024 wait-for-td

Comments

@egekorkan
Copy link
Contributor

The TD spec has the new operations of observeallproperties and unobserveallproperties. However, these are not available in the Scripting API.

See also eclipse-thingweb/node-wot#405

@relu91
Copy link
Member

relu91 commented Mar 29, 2021

btw the spec does not even have writeallproperties, if I remember correctly it was found a little problematic when implemented.

@zolkis
Copy link
Contributor

zolkis commented Mar 30, 2021

We will wait until these operations are properly defined in the TD spec.
I am particularly interested how a server should implement this?

  • After the one operation for observeall a client is allowed to only request unobserveall, or could it also unobserve properties one by one, if the unobserve operation is available to [some of the] properties? I think if this is allowed, it should be done only for convenience, but then we could also say that observeall is a convenience feature that can be emulated by the implementation if it cannot be done in one operation. But that doesn't vibe well with TD semantics, so we need a decision there.
  • When one property changes, I think it makes sense to return only what has changed since the last notification.
    Reasons:
    • returning all properties when something changed also makes sense as a use case. But it's equivalent to a full readallproperties, so it is redundant. Therefore this should not be what observeall would do.
    • Also, it is eclipsing the information what has actually changed. Now if clients should keep track of that themselves, that obsoleted the whole idea of observing.

The same arguments are valid for subscribeallevents.

@danielpeintner
Copy link
Contributor

I think <name>all operations are meant for all interactions. If someone is interested in "one" it can be done so already.
Having said that, this is nothing the scripting API is in charge of. The TD specification needs to describe the proper behavior and scripting just follows the rules...

@zolkis
Copy link
Contributor

zolkis commented Mar 31, 2021

Also, we need to get experience with how this is implemented in various bindings, and what kind of states are kept around for tracking. Like are these scenarios valid:

  • observeall, then unobserveall (this is valid pairing),
  • observeall, then unobserve(property1) (is it valid or invalid?)
  • observe(property1), observe(property2), unobserveall (is this valid?)
  • observeall, then unobserve(property1), unobserve(property2), ... unobserve(lastProperty) (is this valid? if yes, is it equivalent to unobserveall?)

So far, based on what @danielpeintner said, only the first scenario should be valid, is that correct? And it means the underlying bindings can actually do this in a single operation, with no looping over all properties?

Now the question that is more relevant for Scripting: should Scripting allow emulation (by implementation, e.g. node-wot) of observeall/unobserveall even though they are not exposed by a TD as single operations, but still available as scripting convenience? Based on what @danielpeintner said, this should not be the case.

The problem with this is that an API contract published by a W3C spec has different expectations than an API contract exposed by a TD. In other words, if a developer calls thing.observeAllProperties(), it's kind of unintuitive to explain that it will fail if the TD does not have a single op to cover this, and why the implementation denies doing it by a loop, so now the developer will have to explicitly loop over all properties to subscribe to them.

On the other hand, if we implement the internal looping for convenience, then the implementation might have to keep an internal state of how exactly subscriptions happened, so that it could consistently unwind/manage the subscriptions.

In both cases we would deserve developer criticism, either from one viewpoint, or the other :). However, in W3C the advice is to favor the developers over the implementers. That would turn things towards the convenience side.

Of course, we could include an option to observeall that if it's not doable by one op, should the implementation emulate it. But now even the API got more complex. It would be better if there was a simple policy that worked with a simple API.

A lot to discuss, I guess.

@egekorkan
Copy link
Contributor Author

Something quite similar at w3c/wot-thing-description#1082

There @benfrancis said that unsubscribeall should unsubscribe from all previous subscriptions. Thus, all above cases mentioned by @zolkis would be valid.

Regarding convenience or not, I would say that it can be done later. For now, only if there is an observeall operation can allow the function observeaAll to be called.

@zolkis
Copy link
Contributor

zolkis commented Apr 1, 2021

OK, there the use case seems to be not only a convenience, but playing better with transactional interactions.

However, as @relu91 mentioned, Scripting doesn't support writeall at the moment, since it has a lot of problems around semantics (like synchronization in general). For instance, if a script provides fewer properties/values to write than there is in the Thing, does it mean some properties have to be removed? Or should the operation fail? But the latter is complicated by the fact that the TD/ConsumedThing could only see a view on the original Thing (for privacy and security reasons). Is it possible that a writeall method overrides the structure of the remote end? That could be considered a security vector.

Here with subscribeall and observeall, things are simpler and indeed we could play along the convenience argument.
IMHO it doesn't make sense to expose observeall that would work only if the op is there. This doesn't help developers, since now they need to call observeall, and if it fails, they have to loop over all observing properties anyway. Then we rather not expose this method at all, and let the implementation figure out if it can be done with one op.

This whole problem actually exposes the fragility of mapping request-response APIs to functions...
In fact, if we had an API that would make observe requests, and we'd have an option for specifying all, that would make it semantically coherent why it fails: "the thing you (explicitly) asked for is not supported".
But if an observeall() function fails if an obscure (hidden) op is present or not remotely, that is not a good API.

If we go with a function, then it has to work, either by one op, or by looping over multiple ops. The reason for this is that the developer has expressed intent to observe all properties. Therefore the implementation should do that, regardless if done by one (if possible) op or many: it's good if it can be optimized, but becomes an implementation detail, rather than a feature.

We could also include an option to that function that it should fail if the single op is not available, but needs to be explicitly stated in the code, so the default value would meant to emulate (loop over all properties).

Now the same argument applies to readall as well, and in principle to writeall as well (even though that is not supported, either).

@egekorkan
Copy link
Contributor Author

Even though I agree with most of the comments above, for me Scripting API works in a way that the developer has to read the TD properly beforehand. A developer can ignore looking at the forms in the interaction level but the forms on the Thing level, where all the Xallproperties operations are, has to be meticuluosly read by the developer. Thus, a developer should not write a script that has myThing.observeAllProperties() if there is no corresponding operation.

@zolkis
Copy link
Contributor

zolkis commented Apr 6, 2021

IMHO the whole point of the Scripting API is to provide easier ways to access remote Things than looking into forms and all associated quirks (at which point the Fetch API is probably the best to use).

Thus, a developer should not write a script that has myThing.observeAllProperties() if there is no corresponding operation.

Why not, if the intent was anyway to observe all, and it can be trivially done (saving a lot of code lines for scripts), albeit by multiple requests? Either the developer or the implementation needs to do it. From a developer's perspective, if I have a function to observe all properties, I usually don't care if it had a TD op behind, i.e. the developer use case is "observe all properties: if you can do it with one request, good, otherwise take all it needs to do it".
There should be a way to respect that use case, and we could make that with an option (strictly-one-op or loop-over).

Then we have the option to expose a request-response API, which can list all available ops that can be used in a request and then tailor each request bound by generic constraints and particular ones.

Then we have the option for an object-API, one @draggett proposed, which also makes the available ops enumerable, and callable like functions/ accessible like properties. Multiplicity would need specific functions, but so does our API. There if the op was supported, the function would be available, otherwise not (in the functional API, a function would always be there and fail if not supported, but see above).

So my point is that we should respect developers' needs first, then implementers, then spec makers. We are not in the position to tell developers how to work around quirks, it's the other way around :). Having an observeall() method, it would be awkward to explain why does it fail, when observing individual properties doesn't fail. That would not be developer-friendly. But could be done, of course, in the name of perfect TD-ness. :)

@egekorkan
Copy link
Contributor Author

, in the name of perfect TD-ness. :)

After thinking for a while, I think this is what I was always thinking of but I like your way of thinking more. I think it should be just made clear that both would be possible. This would apply to read/write all/multi properties too :)

@danielpeintner
Copy link
Contributor

Scripting Call 2021-07-19
Various ideas were discussed

  • provide convenience function to provide observe/readall for bindings that do not support it
  • do not support such a convenience function at all
  • have an option to indicate whether such a strict function may be used or the convenience function kicks in in a WoT runtime

The decision was made to go with having strict always, meaning

  • IF there is no readallproperties in the binding the readAllProperties() function will fail
  • IF there is interest from developers (through feedback etc) we still might create the option to decide

@relu91 relu91 added enhancement Thoughts and ideas about possible improvements wait-for-td labels Dec 11, 2023
@relu91 relu91 added API-improvement Suggestions for changing the API priority: high Issues that will be tackled in 2024 for next iteration Planned or postponed topics for the future labels Jan 22, 2024
@danielpeintner
Copy link
Contributor

A similar TD issue about events has been closed by now (see w3c/wot-thing-description#1082).

I opened w3c/wot-thing-description#1976

Please let me know whether it is good enough and conveys the information we require.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-improvement Suggestions for changing the API enhancement Thoughts and ideas about possible improvements for next iteration Planned or postponed topics for the future priority: high Issues that will be tackled in 2024 wait-for-td
Projects
None yet
Development

No branches or pull requests

4 participants