From f0051cedcb17fbeb90dcbfa09e7496d3e834b00f Mon Sep 17 00:00:00 2001 From: Jan Christoph Bernack Date: Sat, 7 Mar 2026 03:05:49 +0100 Subject: [PATCH 1/2] fix piggybacked CoAP response --- .../client/CoapClientIntegrationTest.java | 19 ++++++++++++++++--- .../callback/AbstractSyncSessionCallback.java | 1 - .../coap/callback/CoapResponseCallback.java | 5 ----- .../callback/CoapResponseCodeCallback.java | 8 +------- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java index 12b2b677d4..75f49804e2 100644 --- a/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java +++ b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java @@ -100,7 +100,7 @@ public class CoapClientIntegrationTest extends AbstractCoapIntegrationTest { client = createClientForFeatureWithConfirmableParameter(FeatureType.ATTRIBUTES, confirmable); CoapResponse coapResponse = client.postMethod(PAYLOAD_VALUES_STR.getBytes()); assertEquals(CoAP.ResponseCode.CREATED, coapResponse.getCode()); - assertEquals("CoAP response type is wrong!", client.getType(), coapResponse.advanced().getType()); + assertValidResponseType(client.getType(), coapResponse.advanced().getType()); DeviceId deviceId = savedDevice.getId(); List actualKeys = getActualKeysList(deviceId); @@ -184,7 +184,7 @@ public class CoapClientIntegrationTest extends AbstractCoapIntegrationTest { String featureTokenUrl = CoapTestClient.getFeatureTokenUrl(accessToken, FeatureType.ATTRIBUTES) + "?clientKeys=" + keysParam + "&sharedKeys=" + keysParam; client.setURI(featureTokenUrl); CoapResponse response = client.getMethod(); - assertEquals("CoAP response type is wrong!", client.getType(), response.advanced().getType()); + assertValidResponseType(client.getType(), response.advanced().getType()); } @SuppressWarnings({"unchecked", "rawtypes"}) @@ -260,7 +260,7 @@ public class CoapClientIntegrationTest extends AbstractCoapIntegrationTest { payloadBytes = response.getPayload(); responseCode = response.getCode(); observe = response.getOptions().getObserve(); - wasSuccessful = client.getType().equals(response.advanced().getType()); + wasSuccessful = isValidResponseType(client.getType(), response.advanced().getType()); if (observe != null) { if (observe > 0) { processOnLoadResponse(response, client); @@ -304,4 +304,17 @@ public class CoapClientIntegrationTest extends AbstractCoapIntegrationTest { private List getEntityKeys(EntityKeyType scope) { return CoapClientIntegrationTest.EXPECTED_KEYS.stream().map(key -> new EntityKey(scope, key)).collect(Collectors.toList()); } + + private static void assertValidResponseType(CoAP.Type requestType, CoAP.Type responseType) { + assertTrue("Unexpected CoAP response type " + responseType + " for request type " + requestType, + isValidResponseType(requestType, responseType)); + } + + private static boolean isValidResponseType(CoAP.Type requestType, CoAP.Type responseType) { + return switch (requestType) { + case CON -> CoAP.Type.ACK.equals(responseType) || CoAP.Type.CON.equals(responseType) || CoAP.Type.NON.equals(responseType); + case NON -> CoAP.Type.NON.equals(responseType) || CoAP.Type.CON.equals(responseType); + default -> false; + }; + } } diff --git a/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/AbstractSyncSessionCallback.java b/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/AbstractSyncSessionCallback.java index bf23ea6766..6fa56dbd47 100644 --- a/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/AbstractSyncSessionCallback.java +++ b/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/AbstractSyncSessionCallback.java @@ -87,7 +87,6 @@ public abstract class AbstractSyncSessionCallback implements SessionMsgListener protected void respond(Response response) { response.getOptions().setContentFormat(TbCoapContentFormatUtil.getContentFormat(exchange.getRequestOptions().getContentFormat(), state.getContentFormat())); - response.setConfirmable(exchange.advanced().getRequest().isConfirmable()); exchange.respond(response); } diff --git a/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCallback.java b/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCallback.java index c3f70ce0d7..2502f40bfc 100644 --- a/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCallback.java +++ b/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCallback.java @@ -36,7 +36,6 @@ public class CoapResponseCallback implements TransportServiceCallback { */ @Override public void onSuccess(Void msg) { - this.onSuccessResponse.setConfirmable(isConRequest()); exchange.respond(this.onSuccessResponse); } @@ -47,8 +46,4 @@ public class CoapResponseCallback implements TransportServiceCallback { public void onError(Throwable e) { exchange.respond(onFailureResponse); } - - protected boolean isConRequest() { - return exchange.advanced().getRequest().isConfirmable(); - } } diff --git a/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCodeCallback.java b/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCodeCallback.java index 2bfa172bc7..b114ddec2d 100644 --- a/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCodeCallback.java +++ b/common/transport/coap/src/main/java/org/thingsboard/server/transport/coap/callback/CoapResponseCodeCallback.java @@ -34,17 +34,11 @@ public class CoapResponseCodeCallback implements TransportServiceCallback @Override public void onSuccess(Void msg) { - Response response = new Response(onSuccessResponse); - response.setConfirmable(isConRequest()); - exchange.respond(response); + exchange.respond(new Response(onSuccessResponse)); } @Override public void onError(Throwable e) { exchange.respond(onFailureResponse); } - - protected boolean isConRequest() { - return exchange.advanced().getRequest().isConfirmable(); - } } From 38d59e0fce3716d73e1b8c48ccab1569995edda2 Mon Sep 17 00:00:00 2001 From: Jan Christoph Bernack Date: Sun, 8 Mar 2026 00:15:37 +0100 Subject: [PATCH 2/2] add tests for piggybacked CoAP responses --- .../coap/AbstractCoapIntegrationTest.java | 11 ++++ ...tCoapClientPiggybackedIntegrationTest.java | 52 +++++++++++++++++++ .../client/CoapClientIntegrationTest.java | 15 ++---- .../CoapClientNoPiggybackIntegrationTest.java | 40 ++++++++++++++ .../CoapClientPiggybackedIntegrationTest.java | 40 ++++++++++++++ 5 files changed, 146 insertions(+), 12 deletions(-) create mode 100644 application/src/test/java/org/thingsboard/server/transport/coap/client/AbstractCoapClientPiggybackedIntegrationTest.java create mode 100644 application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientNoPiggybackIntegrationTest.java create mode 100644 application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientPiggybackedIntegrationTest.java diff --git a/application/src/test/java/org/thingsboard/server/transport/coap/AbstractCoapIntegrationTest.java b/application/src/test/java/org/thingsboard/server/transport/coap/AbstractCoapIntegrationTest.java index 69d40f5409..1a2f4f1d02 100644 --- a/application/src/test/java/org/thingsboard/server/transport/coap/AbstractCoapIntegrationTest.java +++ b/application/src/test/java/org/thingsboard/server/transport/coap/AbstractCoapIntegrationTest.java @@ -42,6 +42,7 @@ import org.thingsboard.server.common.data.device.profile.JsonTransportPayloadCon import org.thingsboard.server.common.data.device.profile.ProtoTransportPayloadConfiguration; import org.thingsboard.server.common.data.device.profile.TransportPayloadTypeConfiguration; import org.thingsboard.server.common.data.security.DeviceCredentials; +import org.thingsboard.server.common.msg.session.FeatureType; import org.thingsboard.server.transport.AbstractTransportIntegrationTest; import org.thingsboard.server.utils.PortFinder; @@ -176,4 +177,14 @@ public abstract class AbstractCoapIntegrationTest extends AbstractTransportInteg device.setType(type); return doPost("/api/device", device, Device.class); } + + protected CoapTestClient createClientForFeatureWithConfirmableParameter(FeatureType featureType, boolean confirmable) { + CoapTestClient coapTestClient = new CoapTestClient(accessToken, featureType); + if (confirmable) { + coapTestClient.useCONs(); + } else { + coapTestClient.useNONs(); + } + return coapTestClient; + } } diff --git a/application/src/test/java/org/thingsboard/server/transport/coap/client/AbstractCoapClientPiggybackedIntegrationTest.java b/application/src/test/java/org/thingsboard/server/transport/coap/client/AbstractCoapClientPiggybackedIntegrationTest.java new file mode 100644 index 0000000000..32b7e9dc80 --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/transport/coap/client/AbstractCoapClientPiggybackedIntegrationTest.java @@ -0,0 +1,52 @@ +/** + * Copyright © 2016-2026 The Thingsboard Authors + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.transport.coap.client; + +import lombok.extern.slf4j.Slf4j; +import org.eclipse.californium.core.CoapResponse; +import org.eclipse.californium.core.coap.CoAP; +import org.junit.After; +import org.junit.Before; +import org.thingsboard.server.common.msg.session.FeatureType; +import org.thingsboard.server.transport.coap.AbstractCoapIntegrationTest; +import org.thingsboard.server.transport.coap.CoapTestConfigProperties; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +@Slf4j +public abstract class AbstractCoapClientPiggybackedIntegrationTest extends AbstractCoapIntegrationTest { + + @Before + public void beforeTest() throws Exception { + CoapTestConfigProperties configProperties = CoapTestConfigProperties.builder() + .deviceName("CoAP Message Flow Piggyback Device") + .build(); + processBeforeTest(configProperties); + } + + @After + public void afterTest() throws Exception { + processAfterTest(); + } + + protected void testCase(boolean confirmable, CoAP.Type expectedResponseType) throws Exception { + client = createClientForFeatureWithConfirmableParameter(FeatureType.ATTRIBUTES, confirmable); + CoapResponse response = client.postMethod(PAYLOAD_VALUES_STR); + assertNotNull(response); + assertEquals(expectedResponseType, response.advanced().getType()); + } +} diff --git a/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java index 75f49804e2..a128710078 100644 --- a/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java +++ b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientIntegrationTest.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -291,16 +291,6 @@ public class CoapClientIntegrationTest extends AbstractCoapIntegrationTest { }, DEVICE_RESPONSE, MediaTypeRegistry.APPLICATION_JSON); } - private CoapTestClient createClientForFeatureWithConfirmableParameter(FeatureType featureType, boolean confirmable) { - CoapTestClient coapTestClient = new CoapTestClient(accessToken, featureType); - if (confirmable) { - coapTestClient.useCONs(); - } else { - coapTestClient.useNONs(); - } - return coapTestClient; - } - private List getEntityKeys(EntityKeyType scope) { return CoapClientIntegrationTest.EXPECTED_KEYS.stream().map(key -> new EntityKey(scope, key)).collect(Collectors.toList()); } @@ -312,7 +302,8 @@ public class CoapClientIntegrationTest extends AbstractCoapIntegrationTest { private static boolean isValidResponseType(CoAP.Type requestType, CoAP.Type responseType) { return switch (requestType) { - case CON -> CoAP.Type.ACK.equals(responseType) || CoAP.Type.CON.equals(responseType) || CoAP.Type.NON.equals(responseType); + case CON -> + CoAP.Type.ACK.equals(responseType) || CoAP.Type.CON.equals(responseType) || CoAP.Type.NON.equals(responseType); case NON -> CoAP.Type.NON.equals(responseType) || CoAP.Type.CON.equals(responseType); default -> false; }; diff --git a/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientNoPiggybackIntegrationTest.java b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientNoPiggybackIntegrationTest.java new file mode 100644 index 0000000000..53939f84e3 --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientNoPiggybackIntegrationTest.java @@ -0,0 +1,40 @@ +/** + * Copyright © 2016-2026 The Thingsboard Authors + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.transport.coap.client; + +import lombok.extern.slf4j.Slf4j; +import org.eclipse.californium.core.coap.CoAP; +import org.junit.Test; +import org.springframework.test.context.TestPropertySource; +import org.thingsboard.server.dao.service.DaoSqlTest; + +@Slf4j +@DaoSqlTest +@TestPropertySource(properties = { + "transport.coap.piggyback_timeout=0" +}) +public class CoapClientNoPiggybackIntegrationTest extends AbstractCoapClientPiggybackedIntegrationTest { + @Test + public void testConfirmable() throws Exception { + // response should be sent via seperate CON transaction (not piggybacked) + testCase(true, CoAP.Type.CON); + } + + @Test + public void testNonConfirmable() throws Exception { + testCase(false, CoAP.Type.NON); + } +} diff --git a/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientPiggybackedIntegrationTest.java b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientPiggybackedIntegrationTest.java new file mode 100644 index 0000000000..68f28ce4db --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/transport/coap/client/CoapClientPiggybackedIntegrationTest.java @@ -0,0 +1,40 @@ +/** + * Copyright © 2016-2026 The Thingsboard Authors + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.transport.coap.client; + +import lombok.extern.slf4j.Slf4j; +import org.eclipse.californium.core.coap.CoAP; +import org.junit.Test; +import org.springframework.test.context.TestPropertySource; +import org.thingsboard.server.dao.service.DaoSqlTest; + +@Slf4j +@DaoSqlTest +@TestPropertySource(properties = { + "transport.coap.piggyback_timeout=2000" +}) +public class CoapClientPiggybackedIntegrationTest extends AbstractCoapClientPiggybackedIntegrationTest { + @Test + public void testConfirmable() throws Exception { + // response should be included in the ACK packet (piggybacked) + testCase(true, CoAP.Type.ACK); + } + + @Test + public void testNonConfirmable() throws Exception { + testCase(false, CoAP.Type.NON); + } +}