Harmonize the exposing process

[relu91]

Currently, the exposing process is composed of two phases: produce and expose. The two phases are handled by the produce function and the expose method of the ExposedThing interface. The problem that my colleagues and I saw is in the mixed nature of the ExposedThing interface.

First of all, semantically, it sounds strange that a produce method creates an ExposedThing object that is actually not exposed yet until one calls the expose method on it. Secondly, technically, in an ExposedThing the handlers can be changed after the exposition, causing possible problems and side effects if the object is passed in different parts of the program. (what if one library function adds its own handler, or it expects a no-exposed thing).

Discussion

Define a strong split between the two phases and let be ExposedThing an immutable object. In concrete something like the following (typescript notation):

function produce(init: ExposedThingInit): Promise<ProducedThing>;

 export interface ProducedThing {
        setPropertyReadHandler(name: string, handler: PropertyReadHandler): ProducedThing;
        setPropertyWriteHandler(name: string, handler: PropertyWriteHandler): ProducedThing;
        setPropertyObserveHandler(name: string, handler: PropertyReadHandler): ProducedThing;
        setPropertyUnobserveHandler(name: string, handler: PropertyReadHandler): ProducedThing;
        setActionHandler(name: string, handler: ActionHandler): ProducedThing;
        setEventSubscribeHandler(name: string, handler: EventSubscriptionHandler): ProducedThing;
        setEventUnsubscribeHandler(name: string, handler: EventSubscriptionHandler): ProducedThing;
        setEventHandler(name: string, handler: EventListenerHandler): ProducedThing;
        getPartialThingDescription(): Partial<ThingDescription>;
       
        async expose(): Promise<ExposedThing> 
    }
 
 export interface ExposedThing {
     destroy(): Promise<void>;
     getThingDescription(): ThingDescription;
     emitEvent(name: string, data: InteractionInput): void;
     emitPropertyChange(name: string): void;
}

This way we can detect silly errors like emitEvent before exposing or setting a handler in an exposed thing at compile time.

[danielpeintner]

I agree, this proposal makes a lot of sense and clarifies any potential side-effect w.r.t. changing handlers of already exposed things.
@zolkis do you agree?

I do have one question about getPartialThingDescription() in ProducedThing. Is the expectation to get the initial ExposedThingInit from the produce(...) call or something else?

[danielpeintner]

Call 2022-07-11

• @relu91 proposed to implement it in node-wot first to get more experience
  • @ashimura strongly supports to proof the new API before changing the document/spec
• change return type for expose() to ExposedThingInit rather than Partial<ThingDescription>
• should wait for further feedback/comments

[egekorkan]

I support this change. However:

• This probably implies breaking changes to some libraries that use node-wot.
• Not that I have this exact use case but can there be use cases where the handlers need to be changed during runtime? Like a proxy that changes it based on the availability of the Thing it is proxying. I would implement such a functionality in the same handler with conditions but it just came to mind.

[danielpeintner]

FYI: @egekorkan made a good proposal in a personal conversation. We might want to collect such proposal in a "defer to next major version" list and add them all at once to the spec. This means implementations need to adapt only once jumping to the next version.

[danielpeintner]

Call 2022-07-18

• @ashimura should think about WoT versioning in general (synchronization between other specs)
• Discuss it properly in new charter period

[zolkis]

I think we need to check the produce and expose algorithms to figure out the best way.

I don't see much usable value in ProducedThing as an immutable ExposedThing. We need to talk about the exact use cases.

Depending on those use cases, I agree ExposedThing could be made immutable, and we could also rename expose() to start() on ExposedThing. Also, instead of destroy(), we should use stop(), as in other specs.

Making ExposedThing immutable would simplify its interface to a dictionary input, instead of the setting methods.
A big change - we need to be sure about the use cases.

[zolkis]

Thinking it over, explicit functions are more clear than init dict members.

The keyword produce() plays well with ProducedThing and the expose() plays well with ExposedThing.

I don't like the destroy() name. A stop() would be better, aligned with other specs as well, but it would beg for a start() method. It might make sense to add start() to even the proposed new ExposedThing and leave the expose() method on ProducedThing.

That way I could say I like the proposal above.

[relu91]

Call 05/12/2022:

• We found out that we should review this issue and try to solve it during this charter
• It involves API design and how we treat the object
• What we need right now is to settle the naming discussion and then create a PR.

[zolkis]

I am not sure yet about the intermediary object ProducedThing (actually a kind of Thing model or stub).

But we could agree faster on ExposedThing.

Question 1: should ExposedThing extend ProducedThing, or should the latter be an internal slot to the former (composition)?
My take: an internal slot would be more convenient and maybe more clear as well (no confusions about fake inheritance).

Question 2: Should expose() also start servicing requests right away, or should it just be a factory method to initialize the service for the exposed Thing, and the script would also have additional control on when to actually start the service, i.e. ExposedThing would have a start() method, therefore likely a stop() method, too. If a script also wants to release the resources as well, we could provide a shutdown() (or destroy()) method.

[SecureContext, Exposed=(Window,Worker)]
interface ExposedThing {
  //  Promise<undefined> start();  // optional
  //  Promise<undefined> stop();  // optional
  Promise<undefined> shutdown();  // former destroy()

  ThingDescription getThingDescription();  // bound to a binding?
  Promise<undefined> emitPropertyChange(DOMString name);
  Promise<undefined> emitEvent(DOMString name, InteractionInput data);
};

As a consequence of splitting out a functional ExposedThing, we may want to revisit how Things are produced.
Clearly, the ProducedThing is a stub where protocol bindings, security schemes etc. have been selected, but it's missing behavior.
Behaviour is defined by the handlers added to ProducedThing.

Question 3: could we add handlers as dictionary attributes (callbacks) to ProducedThing? Or keep the current methods for adding callbacks?
My take: currently the algorithms for the handlers-adding methods are quite trivial, and they mainly add the callbacks as internal slots, so they could be replaced by attributes as well. However, the algorithms are more flexible than defining getters/setters. So I suggest we keep the methods for now.

Question 4: do we really need to get the partial TD out of ProducedThing? It's kind of confusing at this point. The script already knows all information that would contain. Testing could be done on internal slots.
My take: we don't need it. We can add it when we realize it's needed. :)

Question 5: should we have options to the expose() method? If yes, what?
My take: we can add it when we realize what is needed. I included it in the Web IDL below, but needs not be specified.

[SecureContext, Exposed=(Window,Worker)]
interface ProducedThing {
  ExposedThing setPropertyReadHandler(DOMString name,
          PropertyReadHandler handler);
  ExposedThing setPropertyWriteHandler(DOMString name,
          PropertyWriteHandler handler);
  ExposedThing setPropertyObserveHandler(DOMString name,
          PropertyReadHandler handler);
  ExposedThing setPropertyUnobserveHandler(DOMString name,
          PropertyReadHandler handler);

  ExposedThing setActionHandler(DOMString name, ActionHandler action);

  ExposedThing setEventSubscribeHandler(DOMString name,
          EventSubscriptionHandler handler);
  ExposedThing setEventUnsubscribeHandler(DOMString name,
          EventSubscriptionHandler handler);

  Promise<ExposedThing> expose(optional ExposeOptions options);
};

callback PropertyReadHandler = Promise<InteractionInput>(
        optional InteractionOptions options = null);

callback PropertyWriteHandler = Promise<undefined>(
        InteractionOutput value,
        optional InteractionOptions options = null);

callback ActionHandler = Promise<InteractionInput>(
        InteractionOutput params,
        optional InteractionOptions options = null);

callback EventSubscriptionHandler = Promise<undefined>(
        optional InteractionOptions options = null);

dictionary ExposeOptions {
    USVString? protocolBinding;  // force a given binding
    // ... (other options)
};

[danielpeintner]

My view on the points raised (see inline)

Question 1: should ExposedThing extend ProducedThing, or should the latter be an internal slot to the former (composition)?

It should definitely not be an extension. An internal slot is "okay" to me but for me it is best to leave it apart.. it is somewhat aligned with the builder pattern. Separating the construction/set-up of a complex object from its representation.

Question 2: Should expose() also start servicing requests right away

I would keep it simple and say "yes". It should start right away. Having dedicated start() method MAY make sense but I would not do that.

Question 3: could we add handlers as dictionary attributes (callbacks) to ProducedThing? Or keep the current methods for adding callbacks?

I agree with your thinking that we should keep the methods we have now.

Question 4: do we really need to get the partial TD out of ProducedThing?

Out of my head I also do not have a good use-case for getPartialThingDescription().
As you said, we can introduce this method if we see the need also later..

Question 5: should we have options to the expose() method? If yes, what?

Agree, for the time being I would go without options.

[zolkis]

Looks like we have a common understanding where to start.

The start() method would allow to prepare (expose) in advance the things needed for a full solution, and start them at once, avoiding race conditions when only a part of the "services" needed for the solution are available. To some extent, controlling expose() should have a similar effect, if all delays caused by the background operations are of similar magnitude.

The stop() method would be useful if we wanted to allow to modify the behavior or options of the ExposedThing and then restart it. I think we don't want that, and would like to avoid it because of confusions. So an ExposedThing will always be immutable. If the script wants to modify its behaviour, should do that with the ProducedThing which made it.

With that, it's clear that expose() is a second-phase factory method and it needs to keep the phase-1 properties around in an immutable form, hence the internal slot in ExposedThing.

I'd like us to think more about ProducedThing. The produce() method got its name aligned with the rest of WoT specs, and it's logical but totally uninformative that it returns a "produced" thing :), actually a half-baked thing.
What it produces in this API is an intermediary object, instantiated with the runtime/bindings based on an init dictionary, but behavior is missing and more input & work is needed on it until it can be exposed.

So it's not a Thing model any more, it's already instantiated. It's a kind of a builder object that operates on an its own data and methods (not a very good design, usually builders operate on separate objects).

I considered why not make a constructible builder object to start with, without the produce() method. However, since it does more than just copy the init dict to internal slots, we need to keep the factory method (produce()).

To entertain a few options, produce() may return:

• a ProducedThing like now;
• the same object but named ConfigurableThing, with the current signature (adding behaviour);
• a ThingBuilder object that has the current signature, plus eventually the methods we previously discarded for adding/removing interactions (Properties, Actions, Events) to/from it. Currently we do that via the init dictionary supplied to produce(), which we can keep, but in addition we can also re-add the methods, with the expense of needing to specify a lot of algorithms that depend on Protocol Bindings. We don't need to do that now, just name it ThingBuilder, which currently only adds behavior, but in the future might be able to remove/add Interactions.

If we don't want to re-add later the interactions building methods, I'd vote for the ConfigurableThing name or something along similar lines. If we maintain the possibility to re-add builder methods, then we could use the ThingBuilder name.

[danielpeintner]

Honestly, I am fine with either of the names and I would mostly keep the signature.

Another possible name could be ExposedThingBuilder with maybe changing
expose(): Promise<ExposedThing> with
build(): Promise<ExposedThing>

Anyhow, I let others chime in...