Feedback about new API for CoAP Observe. about java-coap

[sbernard31]

Hi @sbernard31 , should we come back to this discussion after long break?

Just came back.

I still have a concert if using ObserverManager for composite observations is a right way forward for now

I understand.

Is the ObserverManager functionality the only issue right now? If so, could we merge it (PR: #36) to master and keep working on it separately?

Yep let's do that.

[szysas]

There is no nightly builds, only option would be to build locally.

[szysas]

Created a PR: #57

[sbernard31]

About naming, I take a look at RFC7641 to check wording used.

At client side, they often talks about "Observation", e.g :

A client that is no longer interested in receiving notifications for a resource can simply "forget" the observation.

So I guess ObservationRelationsStore is OK, if you want shorted I think Observation(s)Store is OK too.
For ObservationListener, not sure but maybe Notification(s)Listener / Notification(s)Receiver, is maybe clearer ?

At server side, they often talks about "Observer", e.g :

A GET request with an Observe Option set to 0 (register) requests the server not only to return a current representation of the target resource, but also to add the client to the list of observers of that resource.

So maybe for InboundSubscriptionManager (which I find not so clear), we could have something like Observer(s)Manager ?
Or if Observer(s)Manager doesn't fit well ? maybe ObservedResource(s)Manager (this is not really used in RFC but maybe clearer than InboundSubscriptionManager ?

Just to let you know, in Californium, naming are :

• ObservationStore containing Observation and a NotificationListener at client side,
• ObserveManager handling ObserveRelation at server side.

[sbernard31]

About InboundSubscriptionManager API, I understand that a java-coap user need to call InboundSubscriptionManager. sendObservation(String uriPath, CoapResponse obs). It seems that this class also hold "observe relation" internally.

So I understand that each time one resource of my CoAP resources tree is modified, I need to generate a CoapResponse to pass to subMgr.sendObservation(...); and this will be up to subMgr to know if this should be used to send Notification to Observers.

If I'm correct, I think there is a little issue with this design, ideally a CoapResponse should be created only if notification need to be send.

There is maybe another issue with following use case :
Imagine a resource /test and 2 different observer A and B. Both want to observe that resource but one uses a GET request with SenML-JSON content format and another with SenML-CBOR. I think current design will not work with this 🤔

[szysas]

Thanks @sbernard31,

You have really good points about naming, I will do changes:

• ObservationsStore (instead of ObservationRelationsStore)
• NotificationsReceiver (instead of ObservationListener )
• ObserversManager (instead of InboundSubscriptionManager)

If I'm correct, I think there is a little issue with this design, ideally a CoapResponse should be created only if notification need to be send.

The idea behind passing CoapResponse is to pass not only payload but also coap headers (like content-type, etag..etc)
Are you worried about performance with that approach?
What if, we replace CoapResponse with a function Supplier<CoapResponse> to lazily create CoapResponse only if there are any observers?

There is maybe another issue with following use case :
Imagine a resource /test and 2 different observer A and B. Both want to observe that resource but one uses a GET request with SenML-JSON content format and another with SenML-CBOR. I think current design will not work with this 🤔

I'm aware of this limitation, which was also before changes we are discussing. I would suggest to take it as separate issue.

[sbernard31]

The idea behind passing CoapResponse is to pass not only payload but also coap headers (like content-type, etag..etc)

Yep I understand.

Are you worried about performance with that approach?

Exactly, I can imagine use case where resource tree change a lot and if each modification means encoding in complex content format (like SenML-JSON or CBOR), this will not be so good. And I'm afraid this will not be compatible with other issue, I reported.

What if, we replace CoapResponse with a function Supplier to lazily create CoapResponse only if there are any observers?

Maybe we can even replace it by a Service<CoapRequest, CoapResponse> instead of Supplier<CoapResponse>
That means that you need to store the original CoapRequest used to register the observer.
That also means that if wanted you can plug the RouterService (or composed service based on it) to retrieve the CoapResponse.
(This will looks like a bit what I did but not in a really elegant way at : eclipse-leshan/leshan@10ab042)

This solution could also solve the other issue I reported

I'm aware of this limitation, which was also before changes we are discussing. I would suggest to take it as separate issue.

We can do that but I'm afraid that I mixed both problem 😁 just above.

[szysas]

Maybe we can even replace it by a Service<CoapRequest, CoapResponse> instead of Supplier
That means that you need to store the original CoapRequest used to register the observer.
That also means that if wanted you can plug the RouterService (or composed service based on it) to retrieve the CoapResponse.

That is a neat idea, I like it :)

[szysas]

Note, I updated branch with above changes (https://github.com/open-coap/java-coap/tree/observation-handlers#coap-client-complete-usage)

[sbernard31]

Still about ObserversManager.

But I ask myself if we should also add a way to have a custom way to select the observer instead of just select by URI.

I try to explain my use case :
In Leshan, we have a kind of abstraction of LWM2M resource tree.
This model raise event when 1 or several resources changed.
We can have 1 event for several resource changes at same time.
Currently we pass this event to CoAP library (Californium or java-coap) which will send the notification.

Now a concrete example :
Imagine an event which says that resources below are modified :

/3/0/1
/3/0/2
/3/1/3
/3/2/4

Now Imagine that I observe resources below :

/3    - > this means that you want to observe all the resouces under /3 path and if at least one changes you send the whole subtree.
/3/0/1

With current design, I need to translate :

/3/0/1
/3/0/2
/3/1/3
/3/2/4

in

/3
/3/0
/3/0/1
/3/0/2
/3/1
/3/1/3
/3/2
/3/2/4

And call sendNotification(String URI, Service<CoapRequest, CoapResponse> responseProvider) for each of them.

An idea would be to propose another method with custom Filter/Selector :

sendNotification(
    Predicate<CoapRequest /* I'm not sure if CoapRequest is the right input */> ObserverFilter,
    Service<CoapRequest, CoapResponse> responseProvider) 

Let me know if It is not clear enough.

[sbernard31]

Maybe instead of Predicate<CoapRequest> ObserverFilter, this is rather Predicate<String> ResourceFilter ?

[szysas]

Good point and good idea.
I will try first with Predicate<String> resourceFilter.

[szysas]

@sbernard31 let me know if you have any more suggestions, if not I would like to merge this #57. Maybe it would be easier for you to test a released version.

[sbernard31]

@szysas, I'm currently testing the client side and first tests sounds good (but guess what I'm a gradle noob 😅)
Then I plan to test the server side.
This should allow me to provide more feedback.
Then maybe do a more detailed review of your PR.

Is it OK for you to wait ? or that was a problem to not be able to merge now ?

[szysas]

That is perfectly fine to wait.

[sbernard31]

I adapted my code to new API. If you are curious about what it could looks like :
At Client side : eclipse-leshan/leshan@7210459
At Server side : eclipse-leshan/leshan@bb12430

This is clearly better.

At client side :
For now, I have 1 concern : How to handle the case where we try to add the Observation to the ObservationStore but there is already an Observation registered for this "identifier" ?
In Californium the code looks like that, this is done before we send the request. But I don't know if I like this way because this means that if something goes wrong we need to remove it from the store.

There was a long discussion about that long time ago at Californium repository : eclipse-californium/californium#500. (That was so long time ago that I didn't remember everything about this 😅 )

For now, I don't know what could be the best move. I share this just to let you know.
I think this is maybe too soon to address this. Knowing that for now, I didn't explore to use java-coap with a secure transport.

At Server side:
I'm not able to implement LWM2M Composite Observe feature. This feature allows to observe several resources with only 1 FETCH (not GET) request on "/" (resources to observe are define in the payload.
(If you ask yourself like me if this is possible to have several relation from same peer on "/", you could have a look at : OpenMobileAlliance/OMA_LwM2M_for_Developers#528)

Current code does not support that ☝️ , I think I will also need Predicate<CoapRequest> ObserverFilter because I need the CoapRequest payload which contains the resources to observe (or ideally the decoded payload which I could attached to CoapRequest ? I don't know if I will be able to do that ? )

Let me know if you need more clarification for this 2 points ☝️

[szysas]

How to handle the case where we try to add the Observation to the ObservationStore but there is already an Observation registered for this "identifier" ?

Overwrite?
Anyway, there should be enough identifiers in CoapRequest to create unique observation identity per device.

At Server side

I see your point but I am not really sure how changing into Predicate<CoapRequest> helps.

One way to handle composite observation would be to create Filter like:

        Filter.SimpleFilter<CoapRequest, CoapResponse> compositeSubscriber = (request, service) -> {
            if (request.getMethod() == Method.FETCH){
                // - parse payload
                // - create multiple observation requests
                // - call underlying service multiple times
                return completedFuture(CoapResponse.ok(":)"));
            } else {
                return service.apply(request);
            }
        };

Then compose it with ObserversManager:

compositeSubscriber.andThen(observersManager).then(rootResource)

[sbernard31]

Overwrite?
Anyway, there should be enough identifiers in CoapRequest to create unique observation identity per device.

Yep, ideally the observation should be identified by "device identity + token". But currently (for californium historical reason), Leshan are using token only.

Anyway, the point is more that It's hard to be sure that Token used for "observe request" will not be already used when ObservationsStore.add(CoapRequest)

Even if, I check if token is not already used (at cost of 1 more call to shared stored) when I generate it, I can not be sure it will not be used at "add" time. (e.g. because of race condition)

I just let you know the problem, just in case, you have an idea to handle it.
On my side, I will think more about it. And to be more clear, I think current design at client side is a good first step.

[szysas]

Stupid idea but what if you simply make a counter and increment each time subscription is send? With this approach you have extremely small chances that token is reused (hard to believe that there will be 281 474 976 710 655 subscriptions)

[sbernard31]

About compositeSubscriber

Maybe I don't get you but I think it will not work. 🤔
The CompositeObserve is a way to observe several resources at same time but when only 1 resource changes then all the observed resources are sent in notification. (This is like if you create a kind of new virtual resource)

CompositeObserve is a LWM2M feature and so must not be part of java-coap but using FETCH for observe is a CoAP one. (See https://www.rfc-editor.org/rfc/rfc8132.html#section-2.4) So ideally we should find an API which support FETCH for CoAP (which is very general feature) and that API should be usable for specific LWM2M Composite Observe feature.

About FETCH, the RFC8132§1.1. FETCH says :

The CoAP GET method [RFC7252] is used to obtain the representation of
a resource, where the resource is specified by a URI and additional
request parameters can also shape the representation.
...
As an alternative to using GET, many implementations make use of the
POST method to perform extended requests (even if they are
semantically idempotent, safe, and even cacheable) to be able to pass
along the input parameters within the request payload as opposed to
using the request URI.

As with POST, the input to the FETCH operation
is passed along within the payload of the request rather than as part
of the request URI. Unlike POST, however, the semantics of the FETCH
method are more specifically defined.

So I feel that using URI as main identifier in ObserversManager is not enough if you want to support FETCH, It seems that you need payload too. So maybe the right level is CoapRequest which hold URI + payload ?
or I missed something ?

I see your point but I am not really sure how changing into Predicate helps.

The idea is to implement something like this :
https://github.com/eclipse/leshan/blob/bb1243070c1773a63b162442eca218e6c34c3e74/leshan-tl-javacoap-client/src/main/java/org/eclipse/leshan/transport/javacoap/client/observe/NotificationHandler.java#L38-L48

But for Observe Composite it will looks like :

   /** Called when resources changed */
   public void resourceChanged(LwM2mPath... paths) {
        // send notification if there is observers for it
        observersManager.sendObservation((coapRequest) -> {
            List<LwM2mPath> listOfResourceObservedByCompositeObserve = getPaths(coapRequest);
            if (listOfResourceObservedByCompositeObserve != null) {
            for (LwM2mPath observePath : listOfResourceObservedByCompositeObserve) {
                for (LwM2mPath path : paths) {
                    if (path.startWith(observePath)) {
                        return true;
                    }
                }
            }
        }
        }, responseProvider);

   
    /** get LWM2M path to observe from coap request
    public List<LwM2mPath> getPaths(CoapRequest coapRequest) {
     // here I can decode payload which contains LWM2M path for each call
     // but ideally I would prefer to be able to decode it once and then attach it to the request. 
     }

[sbernard31]

Stupid idea but what if you simply make a counter and increment each time subscription is send? With this approach you have extremely small chances that token is reused (hard to believe that there will be 281 474 976 710 655 subscriptions)

Thx for the idea 🙏 , maybe this is way to explore 🤔
This counter should be in a shared store and should be called each time we need to send a new observe request. (Token used should not conflict with none observe request, so maybe we should have 2 exclusive subset of token : one in shared store for observe another in memory for none observe request)

[szysas]

About composite observe:
I agree that it should not be part of this coap library, that's why we should not add any special functionality into ObserversManager (I don't think it would help anything). In other words it should not be aware of any other way of subscribing (like FETCH request).
My suggestion is to use API of ObserversManager to add subscriptions (observation relations). We could have simpler API like:

observersManager. subscribe(token, uriPath, peerAddress)

Then, when doing usual:

observersManager.sendObservation(uriPath, ...)

Works the same as with "normal" subscriptions.
I created test use case that shows this: https://github.com/open-coap/java-coap/blob/observation-handlers/coap-core/src/test/java/com/mbed/coap/server/observe/ObserversManagerTest.java#L205

Another alternative is to implement own ObserversManager in Leshan, that is specific to LwM2M. Note that ObserversManager in java-coap is not coupled to anything so it can be easily replaced.
What do you think?

[sbernard31]

Looking at your previous comment and also code your share, it seems to me there is 2 points about Observe with FETCH we don't understand in a same way. I think this is important that we agree on this (or at least agree on disagreement 😅) before to enter in implementation discussion 🙂

So let's focus on CoAP only (ignoring LWM2M for now).
I try to clarify my understanding :

1. Observe with FETCH is part of CoAP via RFC8132. So ideally java-coap should support it by providing an API to help user to implement observe feature based on FETCH. So it makes sense to first try to find that API (but I agree that if we didn't find anything useful, a fallback could be to let user implement their own ObserversManager)

2. Observe with FETCH is not about creating several observe relation with one request, it is about creating one observe relation based on "URI + payload" instead of just the "URI". If you look at RFC8132§2.7. A Simple Example for FETCH, you will see another usage than the LWM2M Composite Operation one. In that case the FETCH is used to request subpart of the resource. With observe this will be about observing a subpart of the resource. To know what is observed you need the URI + the payload.

Let me know, if there is points you disagree with 🙂

[szysas]

1. Agree
2. Partially agree.

In RFC8132 there is very little about observe with FETCH:

The Observe option [RFC7641] can be used with a FETCH request as it
can be used with a GET request.

How I see it, is that there is no difference when subscribing with GET or FETCH. With GET, you can subscribe for different content type (accept) as well as with FETCH but in addition there is a payload. That payload will be used when creating notification same as accept header.
Going back to ObserversManager API, it already has a callback in sendObservation method, that helps with constructing proper notification.

[sbernard31]

2. Partially agree.

So let's try to clarify point 2. together.

With GET, you can subscribe for different content type (accept) as well as with FETCH but in addition there is a payload. That payload will be used when creating notification same as accept header.

I agree that accept + payload is needed to create the "notification".
But I think it is also needed to identify if a "change" of the observed resource need to trigger a notification or not.

Still with RFC8132§2.7. A Simple Example for FETCH, you can observe :

   FETCH CoAP://www.example.com/object
   Content-Format: NNN (application/example-map-keys+json)
   Accept: application/json
   [
     "foo"
   ]

and also :

   FETCH CoAP://www.example.com/object
   Content-Format: NNN (application/example-map-keys+json)
   Accept: application/json
   [
     "x-coord"
   ]

Both observation relation target CoAP://www.example.com/object URI but one should be triggered with x-coord change and the other when foo change.
(About both relation on same URI, I share again this post : OpenMobileAlliance/OMA_LwM2M_for_Developers#528 which is an OMA/LWM2M question but solve with a CoAP answer from IETF)

Going back to ObserversManager API, it already has a callback in sendObservation method, that helps with constructing proper notification.

I let aside implementation question for now.

[szysas]

But I think it is also needed to identify if a "change" of the observed resource need to trigger a notification or not

OK, I see your point, so in addition to different notifications content, we might have no notification at all.

Agree with you on this ;)

[sbernard31]

Agree with you on this ;)

Ok so I think we can go back to code discussion. 🙂

I understand the ObserversManager hold all "observe relation" at serve side. (It manages registering / unregistering)
And User of ObserversManager is responsible to :

1. know when something "change" in the system.
2. know if a given relation is concerned by a "change" from To be Defined ?
3. Generate the "notification"/"coap response" from CoAP request used to register the observation.

I think the main missing point is 2, that's why I proposed the Predicate<CoapRequest> ObserverFilter which should work ?
But my concern with that solution is that : coaprequest contains the payload which is needed to know "if relation is concerned by a change" but this information is in the payload and need to be decode each time. It would be better to have a way to attach the decoded data to something ? Maybe we need an ObserveRelation object, I don't know ?

[szysas]

Uff ;)

Adding Predicate<CoapRequest> observerFilter would cause that we need to iterate through every subscriber of any resource, I'm not very sure about it.

But my concern with that solution is that : coaprequest contains the payload which is needed to know "if relation is concerned by a change" but this information is in the payload and need to be decode each time. It would be better to have a way to attach the decoded data to something ? Maybe we need an ObserveRelation object, I don't know ?

And here we are entering area that is becoming more LwM2M specific. So maybe implementing ObserversManager in Leshan, with all optimisations related to composite observation could be an option?

[sbernard31]

Adding Predicate observerFilter would cause that we need to iterate through every subscriber of any resource, I'm not very sure about it.

Yep, I understand the concern. Note that this is also true for Predicate<String> uriPathFilter
At the same time, I'm ask myself if having a lot of observe relation at same time is a real use case, so maybe iterate is not really an issue 🤔

And here we are entering area that is becoming more LwM2M specific.

I disagree, I think often when you will use FETCH you will have information in the payload which need to be decoded.
Some examples:

• SenML : https://www.rfc-editor.org/rfc/rfc8790.html#section-3.1 which is not specific to LWM2M
• DNS over CoAP : https://www.ietf.org/archive/id/draft-ietf-core-dns-over-coap-02.html#section-4.2

So maybe implementing ObserversManager in Leshan, with all optimisations related to composite observation could be an option?

Yep this is clearly an option but I feel too bad that we don't find something at java-coap level because I feel there is some part of the logic which should be sharable.
If you agree, I will try to experiment this a little and maybe come back to you with some code. Of course, feel free to reject the codes/ideas if you think that this will not go in the right direction for java-coap. 🙂

Anyway thanks for this constructive discussion, this helps me to clarify my mind about all of this. 🙏

[szysas]

Yep, I understand the concern. Note that this is also true for Predicate uriPathFilter
At the same time, I'm ask myself if having a lot of observe relation at same time is a real use case, so maybe iterate is not really an issue

Well, there will less subscribed uri-paths, and comparing just Strings is a lot easier.. but that just optimisation.

If you agree, I will try to experiment this a little and maybe come back to you with some code. Of course, feel free to reject the codes/ideas if you think that this will not go in the right direction for java-coap. 🙂

👍

[sbernard31]

Here is my first try : eclipse-leshan/leshan@2739377

This is a draft.
ObserversManager, ObserverStore and HashMapObserversStore is pure CoAP class.
The rest is how I used it in Leshan.

The store allows :

• to define custom key for Observers (by default I use token + peeraddress)
• to attach UserData.
• to persist relation if needed (I don't use it but I know by the past some CoAP users wanted to do that)

We still have the "iterate over all observers issue" but I don't know if this is a real issue or if fixing it will be a premature optimization but If you want me to try to find a solution for it, I can try.

Let me know, what you think about that and if we should go deeper in that way.

[szysas]

@sbernard31
I will come back to this in August, after my summer vacation which just started ;).

[sbernard31]

No urgency on my side, I have a working solution to move forward.

🌴 Enjoy your vacation 😉

[szysas]

Hi @sbernard31 , should we come back to this discussion after long break?

I still have a concert if using ObserverManager for composite observations is a right way forward for now, but in general observation mechanism from CoAP and LwM2M is not the simplest and so I'm probably missing the point.

Is the ObserverManager functionality the only issue right now? If so, could we merge it (PR: #36) to master and keep working on it separately?