Fall back to using deprecated methods for accessing CoAP options

The new API for accessing CoAP options in Californium has a bug which prevents retrieving Options by OptionDefinition.
eclipse-hono · Aug 7, 2023 · cd0f43a · cd0f43a
1 parent 7f9d3d9
commit cd0f43a
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 54 deletions.
diff --git a/adapters/coap/src/main/java/org/eclipse/hono/adapter/coap/CoapContext.java b/adapters/coap/src/main/java/org/eclipse/hono/adapter/coap/CoapContext.java
@@ -424,8 +424,7 @@ private boolean shouldResponseIncludeTimeOption() {
     public ResponseCode respond(final Response response) {
         if (shouldResponseIncludeTimeOption()) {
             // Add a time option with the current time to the response
-            final TimeOption timeOption = new TimeOption();
-            response.getOptions().addOption(timeOption);
+            response.getOptions().addOption(new TimeOption());
         }
         acceptFlag.set(true);
         exchange.respond(response);

diff --git a/adapters/coap/src/main/java/org/eclipse/hono/adapter/coap/option/TimeOption.java b/adapters/coap/src/main/java/org/eclipse/hono/adapter/coap/option/TimeOption.java
@@ -14,7 +14,6 @@
 package org.eclipse.hono.adapter.coap.option;
 
 import org.eclipse.californium.core.coap.Option;
-import org.eclipse.californium.core.coap.option.IntegerOptionDefinition;
 
 /**
  * CoAP custom time option.
@@ -55,16 +54,11 @@ public final class TimeOption extends Option {
      */
     public static final String QUERY_PARAMETER_NAME = "hono-time";
 
-    /**
-     * This option's definition.
-     */
-    public static final IntegerOptionDefinition DEFINTION = new IntegerOptionDefinition(NUMBER, "ServerTime");
-
     /**
      * Create time option with current system time.
      */
     public TimeOption() {
-        super(DEFINTION, System.currentTimeMillis());
+        this(System.currentTimeMillis());
     }
 
     /**
@@ -73,15 +67,6 @@ public TimeOption() {
      * @param time time in system milliseconds.
      */
     public TimeOption(final long time) {
-        super(DEFINTION, time);
-    }
-
-    /**
-     * Create time option.
-     *
-     * @param value time in system milliseconds as byte array.
-     */
-    public TimeOption(final byte[] value) {
-        super(DEFINTION, value);
+        super(NUMBER, time);
     }
 }
diff --git a/adapters/coap/src/test/java/org/eclipse/hono/adapter/coap/CoapContextTest.java b/adapters/coap/src/test/java/org/eclipse/hono/adapter/coap/CoapContextTest.java
@@ -23,7 +23,6 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
-import org.eclipse.californium.core.coap.Option;
 import org.eclipse.californium.core.coap.OptionSet;
 import org.eclipse.californium.core.coap.Response;
 import org.eclipse.californium.core.server.resources.CoapExchange;
@@ -196,7 +195,7 @@ void testTimeOptionIsIncludedInResponseIfOptionPresentInRequest() {
         final long start = System.currentTimeMillis();
         final CoapExchange exchange = mock(CoapExchange.class);
         final OptionSet requestOptions = new OptionSet();
-        requestOptions.addOption(new Option(TimeOption.NUMBER, new byte[0]));
+        requestOptions.addOption(new TimeOption(0));
         when(exchange.getRequestOptions()).thenReturn(requestOptions);
         final Adapter coapConfig = new Adapter(Constants.PROTOCOL_ADAPTER_TYPE_COAP);
         final TenantObject tenant = TenantObject.from("tenant", true).addAdapter(coapConfig);
@@ -207,10 +206,10 @@ void testTimeOptionIsIncludedInResponseIfOptionPresentInRequest() {
         when(response.getOptions()).thenReturn(responseOptions);
         ctx.respond(response);
         verify(response).getOptions();
-        assertThat(responseOptions.hasOption(TimeOption.NUMBER)).isTrue();
-        final long serverTime = responseOptions.getOtherOption(TimeOption.NUMBER).getLongValue();
+        final var serverTime = responseOptions.getOtherOption(TimeOption.NUMBER);
+        assertThat(serverTime).isNotNull();
         final long end = System.currentTimeMillis();
-        assertThat(serverTime).isIn(Range.closed(start, end));
+        assertThat(serverTime.getLongValue()).isIn(Range.closed(start, end));
     }
 
     /**
@@ -230,9 +229,9 @@ void testTimeOptionIsIncludedInResponseIfParameterPresentInRequest() {
         when(response.getOptions()).thenReturn(responseOptions);
         ctx.respond(response);
         verify(response).getOptions();
-        assertThat(responseOptions.hasOption(TimeOption.NUMBER)).isTrue();
-        final long serverTime = responseOptions.getOtherOption(TimeOption.NUMBER).getLongValue();
+        final var serverTime = responseOptions.getOtherOption(TimeOption.NUMBER);
+        assertThat(serverTime).isNotNull();
         final long end = System.currentTimeMillis();
-        assertThat(serverTime).isIn(Range.closed(start, end));
+        assertThat(serverTime.getLongValue()).isIn(Range.closed(start, end));
     }
 }
diff --git a/tests/src/test/java/org/eclipse/hono/tests/coap/CoapTestBase.java b/tests/src/test/java/org/eclipse/hono/tests/coap/CoapTestBase.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019, 2022 Contributors to the Eclipse Foundation
+ * Copyright (c) 2019, 2023 Contributors to the Eclipse Foundation
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information regarding copyright ownership.
@@ -1314,18 +1314,17 @@ public void testUploadMessagesWithTtdThatReplyWithOneWayCommand(
     }
 
     /**
-     * Ensures that a timestamp is within 2 seconds of current system clock.
-     * This is necessary because the test containers may have a clock drift of a few milliseconds
-     * from the host clock.
+     * Ensures that a CoAP response contains a server time option.
      *
-     * @param ts the timestamp to check
+     * @param response The CoAP response.
      */
-//    private void assertTimestampWithinRangeOfCurrentTime(final long ts) {
-//        // Use a buffer time of 2 seconds
-//        final long buffer = 2000;
-//        final long currentTime = System.currentTimeMillis();
-//        assertThat(ts).isIn(Range.closed(currentTime - buffer, currentTime + buffer));
-//    }
+    private void assertResponseContainsServerTime(final CoapResponse response) {
+        final var serverTimeOption = response.getOptions().getOtherOption(TimeOption.NUMBER);
+        assertThat(serverTimeOption).isNotNull();
+        // Unit tests verify that the time value is correct but due to minute time differences in the
+        // containers running integration tests it suffices to verify that the value is non zero.
+        assertThat(serverTimeOption.getLongValue()).isGreaterThan(0L);
+    }
 
     /**
      * Verify that the CoAP adapter responds with a time option in the response when request includes
@@ -1352,18 +1351,12 @@ public void testServerTimeResponseWithRequestOption(final VertxTestContext ctx)
                     final CoapClient client = getCoapsClient(deviceId, tenantId, SECRET);
                     final Promise<CoapResponse> result = Promise.promise();
                     final var request = createCoapsRequest(Code.POST, getPostResource(), 0);
-                    request.getOptions().addOtherOption(new TimeOption(new byte[0]));
+                    request.getOptions().addOption(new TimeOption(0));
                     client.advanced(getHandler(result, ResponseCode.CHANGED), request);
                     return result.future();
                 })
                 .onSuccess(res -> {
-                    ctx.verify(() -> {
-                        assertThat(res.getOptions().hasOption(TimeOption.DEFINTION)).isTrue();
-                        final long serverTimeValue = res.getOptions().getOtherOption(TimeOption.DEFINTION).getLongValue();
-                        // Unit tests verify that the time value is correct but due to minute time differences in the
-                        // containers running integration tests it suffices to verify that the value is non zero.
-                        assertThat(serverTimeValue).isGreaterThan(0L);
-                    });
+                    ctx.verify(() -> assertResponseContainsServerTime(res));
                     checks.flag();
                 })
                 .onFailure(ctx::failNow);
@@ -1400,13 +1393,7 @@ public void testServerTimeResponseWithRequestParameter(final VertxTestContext ct
                     return result.future();
                 })
                 .onSuccess(res -> {
-                    ctx.verify(() -> {
-                        assertThat(res.getOptions().hasOption(TimeOption.DEFINTION)).isTrue();
-                        final long serverTimeValue = res.getOptions().getOtherOption(TimeOption.DEFINTION).getLongValue();
-                        // Unit tests verify that the time value is correct but due to minute time differences in the
-                        // containers running integration tests it suffices to verify that the value is non zero.
-                        assertThat(serverTimeValue).isGreaterThan(0L);
-                    });
+                    ctx.verify(() -> assertResponseContainsServerTime(res));
                     checks.flag();
                 })
                 .onFailure(ctx::failNow);
@@ -1445,7 +1432,7 @@ public void testNoServerTimeResponse(final VertxTestContext ctx) {
                 })
                 .onSuccess(res -> {
                     ctx.verify(() -> {
-                        assertThat(res.getOptions().hasOption(TimeOption.DEFINTION)).isFalse();
+                        assertThat(res.getOptions().hasOption(TimeOption.NUMBER)).isFalse();
                     });
                     checks.flag();
                 })