open-coap / java-coap

CoAP Java library
Apache License 2.0
18 stars 5 forks source link

New CoapRequest / CoapResponse Builder API feedback #73

Closed sbernard31 closed 5 months ago

sbernard31 commented 7 months ago

I updated Leshan code to use the new java-coap API.

I think the API is largely better now :+1:

Only issue I faced was when I wanted to modify a CoapRequest to add element in TransportContext.

I didn't find better way than :

TransportContext extendedContext = observeRequest.getTransContext() //
                            .with(LwM2mKeys.LESHAN_OBSERVED_PATHS, paths);

CoapRequest modifiedObservedRequest = new CoapRequest(observeRequest.getMethod(),
        observeRequest.getToken(), observeRequest.options(), observeRequest.getPayload(),
        observeRequest.getPeerAddress(), extendedContext);

super.add(modifiedObservedRequest);

A solution to avoid the verbose coapRequest creation could be https://github.com/open-coap/java-coap/pull/68#discussion_r1344358349 and so here this could look like :

TransportContext extendedContext = observeRequest.getTransContext() //
                            .with(LwM2mKeys.LESHAN_OBSERVED_PATHS, paths);

super.add(coapRequest.modify().context(extendedContext).build());

Maybe we could also add a method to modify transportContext in the builder ? something like :

super.add(coapRequest.modify()
                     .context(ctx -> ctx.with(LwM2mKeys.LESHAN_OBSERVED_PATHS, paths))
                     .build());

Note that:

(If you are curious the whole migration change are available at : https://github.com/eclipse-leshan/leshan/commit/c2f823244de882f4ad682c99928e0b78f57c2133)

sbernard31 commented 7 months ago

Not directly linked but I'm think that some part of the README was not updated with the new API :

-        // (optional) set custom observation relation store, for example one that will use external storage
+        // (optional) set custom observations store, for example one that will use external storage
-       .observationRelationsStore(new HashMapObservationRelations())
+       .observationsStore(new HashMapObservationsStore())
-// define subscription manager for observable resources
+// define observers manager for observable resources
-InboundSubscriptionManager subscriptionManager = new InboundSubscriptionManager();
+ObserversManager observersManager = new ObserversManager();

server = CoapServer.builder()
        // configure with plain text UDP transport, listening on port 5683
        .transport(new DatagramSocketTransport(5683))
        // define routing
        // (note that each resource function is a `Service` type and can be decorated/transformed with `Filter`)
        .route(RouterService.builder()
                .get("/.well-known/core", req ->
                        CoapResponse.ok("</sensors/temperature>", MediaTypes.CT_APPLICATION_LINK__FORMAT).toFuture()
                )
                .post("/actuators/switch", req -> {
                    // ...
                    return coapResponse(Code.C204_CHANGED).toFuture();
                })
                // observable resource
-               .get("/sensors/temperature", subscriptionManager.then(req ->
+               .get("/sensors/temperature", observersManager.then(req ->
-                        completedFuture(CoapResponse.ok("21C"))
+                        CoapResponse.ok("21C").toFuture())
                ))
        )
        .build();

observersManager.init(server);

I didn't test/execute the code, so probably deserve a double check :sweat_smile:

szysas commented 7 months ago

Thanks for feedback. I agree than modifying transport context is really needed. Thanks for spotting outdated readme. It is actually based on theses tests: https://github.com/open-coap/java-coap/blob/master/coap-core/src/test/java/protocolTests/UsageTest.java. Do you know any idea/tools how to verify if readme examples are up to date?

sbernard31 commented 7 months ago

Do you know any idea/tools how to verify if readme examples are up to date?

I had some ideas but not sure they really works :

So nothing great :grimacing:

szysas commented 7 months ago

I had some ideas but not sure they really works

Thanks