diff --git a/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionClient.java b/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionClient.java index 01bb3042c06..020a1bdf091 100644 --- a/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionClient.java +++ b/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionClient.java @@ -147,12 +147,14 @@ public Future setCommandHandlingAdapterInstance(final String deviceId, fin } @Override - public Future removeCommandHandlingAdapterInstance(final String deviceId, final String adapterInstanceId, + public Future removeCommandHandlingAdapterInstance(final String deviceId, final String adapterInstanceId, final SpanContext context) { final Span span = newSpan(context, SPAN_NAME_REMOVE_CMD_HANDLING_ADAPTER_INSTANCE); TracingHelper.setDeviceTags(span, tenantId, deviceId); span.setTag(MessageHelper.APP_PROPERTY_ADAPTER_INSTANCE_ID, adapterInstanceId); - return finishSpan(cache.removeCommandHandlingAdapterInstance(tenantId, deviceId, adapterInstanceId, span), span); + // skip tracing PRECON_FAILED response status as error (may have been trying to remove an expired entry) + return finishSpan(cache.removeCommandHandlingAdapterInstance(tenantId, deviceId, adapterInstanceId, span), span, + HttpURLConnection.HTTP_PRECON_FAILED); } @Override @@ -170,13 +172,20 @@ private Span newSpan(final SpanContext parent, final String operationName) { } private Future finishSpan(final Future result, final Span span) { + return finishSpan(result, span, null); + } + + private Future finishSpan(final Future result, final Span span, final Integer statusToSkipErrorTraceFor) { return result.recover(t -> { - Tags.HTTP_STATUS.set(span, ServiceInvocationException.extractStatusCode(t)); - TracingHelper.logError(span, t); + final int statusCode = ServiceInvocationException.extractStatusCode(t); + Tags.HTTP_STATUS.set(span, statusCode); + if (statusToSkipErrorTraceFor == null || statusCode != statusToSkipErrorTraceFor) { + TracingHelper.logError(span, t); + } span.finish(); return Future.failedFuture(t); }).map(resultValue -> { - Tags.HTTP_STATUS.set(span, resultValue != null ? HttpURLConnection.HTTP_OK : HttpURLConnection.HTTP_ACCEPTED); + Tags.HTTP_STATUS.set(span, resultValue != null ? HttpURLConnection.HTTP_OK : HttpURLConnection.HTTP_NO_CONTENT); span.finish(); return resultValue; }); diff --git a/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfo.java b/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfo.java index 112f668e47d..400c361b49a 100644 --- a/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfo.java +++ b/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfo.java @@ -166,7 +166,7 @@ public Future setCommandHandlingAdapterInstance(final String tenantId, fin } @Override - public Future removeCommandHandlingAdapterInstance(final String tenantId, final String deviceId, + public Future removeCommandHandlingAdapterInstance(final String tenantId, final String deviceId, final String adapterInstanceId, final Span span) { Objects.requireNonNull(tenantId); Objects.requireNonNull(deviceId); @@ -182,15 +182,16 @@ public Future removeCommandHandlingAdapterInstance(final String tenantI tenantId, deviceId, adapterInstanceId, t); return Future.failedFuture(new ServerErrorException(HttpURLConnection.HTTP_INTERNAL_ERROR, t)); }) - .map(removed -> { + .compose(removed -> { if (!removed) { LOG.debug("command handling adapter instance was not removed, key not mapped or value didn't match [tenant: {}, device-id: {}, adapter-instance: {}]", tenantId, deviceId, adapterInstanceId); + return Future.failedFuture(new ClientErrorException(HttpURLConnection.HTTP_PRECON_FAILED)); } else { LOG.debug("removed command handling adapter instance [tenant: {}, device-id: {}, adapter-instance: {}]", tenantId, deviceId, adapterInstanceId); + return Future.succeededFuture(); } - return removed; }); } diff --git a/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/DeviceConnectionInfo.java b/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/DeviceConnectionInfo.java index fb2760edb96..43911f7481e 100644 --- a/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/DeviceConnectionInfo.java +++ b/client-device-connection-infinispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/DeviceConnectionInfo.java @@ -99,14 +99,13 @@ Future setCommandHandlingAdapterInstance(String tenantId, String deviceId, * @param span The active OpenTracing span for this operation. It is not to be closed in this method! * An implementation should log (error) events on this span and it may set tags and use this span as the * parent for any spans created in this method. - * @return A future indicating the outcome of the operation, with its value indicating whether the protocol - * adapter instance value was removed or not. + * @return A future indicating the outcome of the operation. *

- * The future will be failed with a {@link org.eclipse.hono.client.ServiceInvocationException} if there - * was an error removing the value. - * @throws NullPointerException if any of the parameters is {@code null}. + * The future will be succeeded if the entry was successfully removed. + * Otherwise the future will be failed with a {@link org.eclipse.hono.client.ServiceInvocationException}. + * @throws NullPointerException if any of the parameters except context is {@code null}. */ - Future removeCommandHandlingAdapterInstance(String tenantId, String deviceId, String adapterInstanceId, Span span); + Future removeCommandHandlingAdapterInstance(String tenantId, String deviceId, String adapterInstanceId, Span span); /** * Gets information about the adapter instances that can handle a command for the given device. diff --git a/client-device-connection-infinispan/src/test/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfoTest.java b/client-device-connection-infinispan/src/test/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfoTest.java index ab030281f3d..78fd3baec1b 100644 --- a/client-device-connection-infinispan/src/test/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfoTest.java +++ b/client-device-connection-infinispan/src/test/java/org/eclipse/hono/deviceconnection/infinispan/client/CacheBasedDeviceConnectionInfoTest.java @@ -256,14 +256,11 @@ public void testRemoveCommandHandlingAdapterInstanceSucceeds(final VertxTestCont info.setCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, deviceId, adapterInstance, null, span) .compose(v -> info.removeCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, deviceId, adapterInstance, span)) - .onComplete(ctx.succeeding(result -> ctx.verify(() -> { - assertThat(result).isTrue(); - ctx.completeNow(); - }))); + .onComplete(ctx.completing()); } /** - * Verifies that the removeCommandHandlingAdapterInstance operation result is false if + * Verifies that the removeCommandHandlingAdapterInstance operation fails with a PRECON_FAILED status if * no entry was registered for the device. Only an adapter instance for another device of the tenant was * registered. * @@ -276,15 +273,16 @@ public void testRemoveCommandHandlingAdapterInstanceFailsForOtherDevice(final Ve info.setCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, deviceId, adapterInstance, null, span) .compose(v -> { return info.removeCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, "otherDevice", adapterInstance, span); - }).onComplete(ctx.succeeding(result -> ctx.verify(() -> { - assertThat(result).isFalse(); + }).onComplete(ctx.failing(t -> ctx.verify(() -> { + assertThat(t).isInstanceOf(ServiceInvocationException.class); + assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_PRECON_FAILED); ctx.completeNow(); }))); } /** - * Verifies that the removeCommandHandlingAdapterInstance operation result is false if - * the given adapter instance parameter doesn't match the one of the entry registered for the given device. + * Verifies that the removeCommandHandlingAdapterInstance operation fails with a PRECON_FAILED status + * if the given adapter instance parameter doesn't match the one of the entry registered for the given device. * * @param ctx The vert.x context. */ @@ -295,8 +293,9 @@ public void testRemoveCommandHandlingAdapterInstanceFailsForOtherAdapterInstance info.setCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, deviceId, adapterInstance, null, span) .compose(v -> { return info.removeCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, deviceId, "otherAdapterInstance", span); - }).onComplete(ctx.succeeding(result -> ctx.verify(() -> { - assertThat(result).isFalse(); + }).onComplete(ctx.failing(t -> ctx.verify(() -> { + assertThat(t).isInstanceOf(ServiceInvocationException.class); + assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_PRECON_FAILED); ctx.completeNow(); }))); } diff --git a/client/src/main/java/org/eclipse/hono/client/DeviceConnectionClient.java b/client/src/main/java/org/eclipse/hono/client/DeviceConnectionClient.java index c0e5b638d41..ab5e4a91ee8 100644 --- a/client/src/main/java/org/eclipse/hono/client/DeviceConnectionClient.java +++ b/client/src/main/java/org/eclipse/hono/client/DeviceConnectionClient.java @@ -95,18 +95,13 @@ Future setCommandHandlingAdapterInstance(String deviceId, String adapterIn * @param context The currently active OpenTracing span context or {@code null} if no span is currently active. * An implementation should use this as the parent for any span it creates for tracing * the execution of this operation. - * @return A future indicating the outcome of the operation, with its value indicating whether the protocol - * adapter instance value was removed or not. - *

- * NOTE: this method maps an outcome with status 404 or 412 as defined in the - * Device Connection API - * specification to a succeeded future with value {@code false} here. + * @return A future indicating the outcome of the operation. *

- * The future will be failed with a {@link org.eclipse.hono.client.ServiceInvocationException} if there - * was an error removing the value. + * The future will be succeeded if the entry was successfully removed. + * Otherwise the future will be failed with a {@link org.eclipse.hono.client.ServiceInvocationException}. * @throws NullPointerException if device id or adapter instance id is {@code null}. */ - Future removeCommandHandlingAdapterInstance(String deviceId, String adapterInstanceId, SpanContext context); + Future removeCommandHandlingAdapterInstance(String deviceId, String adapterInstanceId, SpanContext context); /** * Gets information about the adapter instances that can handle a command for the given device. diff --git a/client/src/main/java/org/eclipse/hono/client/impl/DeviceConnectionClientImpl.java b/client/src/main/java/org/eclipse/hono/client/impl/DeviceConnectionClientImpl.java index 272a666c0f2..d273be3f416 100644 --- a/client/src/main/java/org/eclipse/hono/client/impl/DeviceConnectionClientImpl.java +++ b/client/src/main/java/org/eclipse/hono/client/impl/DeviceConnectionClientImpl.java @@ -25,6 +25,7 @@ import org.eclipse.hono.client.DeviceConnectionClient; import org.eclipse.hono.client.HonoConnection; import org.eclipse.hono.client.SendMessageSampler; +import org.eclipse.hono.client.ServiceInvocationException; import org.eclipse.hono.client.StatusCodeMapper; import org.eclipse.hono.tracing.TracingHelper; import org.eclipse.hono.util.CacheDirective; @@ -37,6 +38,7 @@ import io.opentracing.Span; import io.opentracing.SpanContext; +import io.opentracing.tag.Tags; import io.vertx.core.Future; import io.vertx.core.Handler; import io.vertx.core.Promise; @@ -242,7 +244,7 @@ public Future getLastKnownGatewayForDevice(final String deviceId, fi } @Override - public Future removeCommandHandlingAdapterInstance(final String deviceId, final String adapterInstanceId, final SpanContext context) { + public Future removeCommandHandlingAdapterInstance(final String deviceId, final String adapterInstanceId, final SpanContext context) { Objects.requireNonNull(deviceId); Objects.requireNonNull(adapterInstanceId); @@ -261,17 +263,26 @@ public Future removeCommandHandlingAdapterInstance(final String deviceI resultTracker, null, currentSpan); - return mapResultAndFinishSpan(resultTracker.future(), result -> { - switch (result.getStatus()) { - case HttpURLConnection.HTTP_NO_CONTENT: - return Boolean.TRUE; - case HttpURLConnection.HTTP_NOT_FOUND: - case HttpURLConnection.HTTP_PRECON_FAILED: - return Boolean.FALSE; - default: - throw StatusCodeMapper.from(result); + // not using mapResultAndFinishSpan() in order to skip logging PRECON_FAILED result as error (may have been trying to remove an expired entry) + return resultTracker.future().recover(t -> { + Tags.HTTP_STATUS.set(currentSpan, ServiceInvocationException.extractStatusCode(t)); + TracingHelper.logError(currentSpan, t); + currentSpan.finish(); + return Future.failedFuture(t); + }).map(resultValue -> { + Tags.HTTP_STATUS.set(currentSpan, resultValue.getStatus()); + if (resultValue.isError() && resultValue.getStatus() != HttpURLConnection.HTTP_PRECON_FAILED) { + Tags.ERROR.set(currentSpan, Boolean.TRUE); } - }, currentSpan); + currentSpan.finish(); + + switch (resultValue.getStatus()) { + case HttpURLConnection.HTTP_NO_CONTENT: + return null; + default: + throw StatusCodeMapper.from(resultValue); + } + }); } @Override diff --git a/client/src/main/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImpl.java b/client/src/main/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImpl.java index c1c8cd6cf52..41c3ad63e84 100644 --- a/client/src/main/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImpl.java +++ b/client/src/main/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImpl.java @@ -34,6 +34,7 @@ import org.eclipse.hono.client.ProtocolAdapterCommandConsumerFactory; import org.eclipse.hono.client.SendMessageSampler; import org.eclipse.hono.client.ServerErrorException; +import org.eclipse.hono.client.ServiceInvocationException; import org.eclipse.hono.util.AddressHelper; import org.eclipse.hono.util.CommandConstants; import org.eclipse.hono.util.Constants; @@ -228,26 +229,30 @@ private Future removeCommandConsumer(final CommandHandlerWrapper commandHa .compose(client -> client.removeCommandHandlingAdapterInstance(deviceId, adapterInstanceId, onCloseSpanContext)) .recover(thr -> { - log.warn("error removing command handling adapter instance [tenant: {}, device: {}]", tenantId, - deviceId, thr); - return Future.failedFuture(thr); - }) - .compose(removed -> { - final boolean entryNotExpired = lifespan.isNegative() || Instant.now().isBefore(lifespanStart.plus(lifespan)); - if (!removed && entryNotExpired) { - // entry wasn't actually removed and entry hasn't expired (yet); - // This case happens when 2 consecutive command subscription requests from the same device - // (with no intermittent disconnect/unsubscribe - possibly because of a broken connection in between) - // have reached *different* protocol adapter instances/verticles. Now calling 'removeCommandHandlingAdapterInstance' - // on the 1st subscription fails because of the non-matching adapterInstanceId parameter. - // Throwing an explicit exception here will enable the protocol adapter to detect this case - // and skip sending an (incorrect) "disconnectedTtd" event message. - log.debug("command handling adapter instance not removed - not matched or already removed [tenant: {}, device: {}]", - tenantId, deviceId); - return Future.failedFuture(new ClientErrorException(HttpURLConnection.HTTP_PRECON_FAILED, - "no matching command consumer mapping found to be removed")); + if (ServiceInvocationException.extractStatusCode(thr) == HttpURLConnection.HTTP_PRECON_FAILED) { + final boolean entryMayHaveExpired = !lifespan.isNegative() && Instant.now().isAfter(lifespanStart.plus(lifespan)); + if (entryMayHaveExpired) { + log.trace("ignoring 412 error when removing command handling adapter instance; entry may have already expired [tenant: {}, device: {}]", + tenantId, deviceId); + return Future.succeededFuture(); + } else { + // entry wasn't actually removed and entry hasn't expired (yet); + // This case happens when 2 consecutive command subscription requests from the same device + // (with no intermittent disconnect/unsubscribe - possibly because of a broken connection in between) + // have reached *different* protocol adapter instances/verticles. Now calling 'removeCommandHandlingAdapterInstance' + // on the 1st subscription fails because of the non-matching adapterInstanceId parameter. + // Throwing an explicit exception here will enable the protocol adapter to detect this case + // and skip sending an (incorrect) "disconnectedTtd" event message. + log.debug("command handling adapter instance not removed - not matched or already removed [tenant: {}, device: {}]", + tenantId, deviceId); + return Future.failedFuture(new ClientErrorException(HttpURLConnection.HTTP_PRECON_FAILED, + "no matching command consumer mapping found to be removed")); + } + } else { + log.info("error removing command handling adapter instance [tenant: {}, device: {}]", tenantId, + deviceId, thr); + return Future.failedFuture(thr); } - return Future.succeededFuture((Void) null); }); } diff --git a/client/src/test/java/org/eclipse/hono/client/impl/DeviceConnectionClientImplTest.java b/client/src/test/java/org/eclipse/hono/client/impl/DeviceConnectionClientImplTest.java index ba54a5d6973..d3df7643822 100644 --- a/client/src/test/java/org/eclipse/hono/client/impl/DeviceConnectionClientImplTest.java +++ b/client/src/test/java/org/eclipse/hono/client/impl/DeviceConnectionClientImplTest.java @@ -187,10 +187,9 @@ public void testRemoveCommandHandlingAdapterInstance(final VertxTestContext ctx) // WHEN removing the command handling adapter instance client.removeCommandHandlingAdapterInstance("deviceId", "adapterInstanceId", span.context()) - .onComplete(ctx.succeeding(result -> { + .onComplete(ctx.succeeding(r -> { ctx.verify(() -> { // THEN the response has been handled and the span is finished - assertThat(result).isTrue(); verify(span).finish(); }); ctx.completeNow(); @@ -342,7 +341,7 @@ public void testRemoveCommandHandlingAdapterInstanceFailsWithSendError(final Ver /** * Verifies that a client invocation of the remove-cmd-handling-adapter-instance operation - * returns a Boolean.FALSE value if a NOT_FOUND response was returned. + * returns a Boolean.FALSE value if a PRECON_FAILED response was returned. * * @param ctx The vert.x test context. */ @@ -351,10 +350,10 @@ public void testRemoveCommandHandlingAdapterInstanceForNotFoundEntry(final Vertx // WHEN removing the command handling adapter instance client.removeCommandHandlingAdapterInstance("deviceId", "gatewayId", span.context()) - .onComplete(ctx.succeeding(removed -> { + .onComplete(ctx.failing(t -> { ctx.verify(() -> { // THEN the response has been handled and the span is finished - assertThat(removed).isFalse(); + assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_PRECON_FAILED); verify(span).finish(); }); ctx.completeNow(); @@ -362,7 +361,7 @@ public void testRemoveCommandHandlingAdapterInstanceForNotFoundEntry(final Vertx final Message sentMessage = verifySenderSend(); final Message response = ProtonHelper.message(); - MessageHelper.addProperty(response, MessageHelper.APP_PROPERTY_STATUS, HttpURLConnection.HTTP_NOT_FOUND); + MessageHelper.addProperty(response, MessageHelper.APP_PROPERTY_STATUS, HttpURLConnection.HTTP_PRECON_FAILED); MessageHelper.addCacheDirective(response, CacheDirective.maxAgeDirective(60)); response.setCorrelationId(sentMessage.getMessageId()); client.handleResponse(mock(ProtonDelivery.class), response); @@ -415,8 +414,8 @@ public void testGetLastKnownGatewayForDeviceFailsWithRejectedRequest(final Vertx // WHEN getting last known gateway information client.getLastKnownGatewayForDevice("deviceId", span.context()) .onComplete(ctx.failing(t -> { - assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); ctx.verify(() -> { + assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); // THEN the invocation fails and the span is marked as erroneous verify(span).setTag(eq(Tags.ERROR.getKey()), eq(Boolean.TRUE)); // and the span is finished @@ -481,8 +480,8 @@ public void testSetCommandHandlingAdapterInstanceFailsWithRejectedRequest(final // WHEN setting the command handling adapter instance client.setCommandHandlingAdapterInstance("deviceId", "adapterInstanceId", null, span.context()) .onComplete(ctx.failing(t -> { - assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); ctx.verify(() -> { + assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); // THEN the invocation fails and the span is marked as erroneous verify(span).setTag(eq(Tags.ERROR.getKey()), eq(Boolean.TRUE)); // and the span is finished @@ -514,8 +513,8 @@ public void testRemoveCommandHandlingAdapterInstanceFailsWithRejectedRequest(fin // WHEN removing the command handling adapter instance client.removeCommandHandlingAdapterInstance("deviceId", "adapterInstanceId", span.context()) .onComplete(ctx.failing(t -> { - assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); ctx.verify(() -> { + assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); // THEN the invocation fails and the span is marked as erroneous verify(span).setTag(eq(Tags.ERROR.getKey()), eq(Boolean.TRUE)); // and the span is finished @@ -547,8 +546,8 @@ public void testGetCommandHandlingAdapterInstancesFailsWithRejectedRequest(final // WHEN getting the command handling adapter instances client.getCommandHandlingAdapterInstances("deviceId", Collections.emptyList(), span.context()) .onComplete(ctx.failing(t -> { - assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); ctx.verify(() -> { + assertThat(((ServiceInvocationException) t).getErrorCode()).isEqualTo(HttpURLConnection.HTTP_BAD_REQUEST); // THEN the invocation fails and the span is marked as erroneous verify(span).setTag(eq(Tags.ERROR.getKey()), eq(Boolean.TRUE)); // and the span is finished diff --git a/client/src/test/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImplTest.java b/client/src/test/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImplTest.java index fb78da178ca..348a686d241 100644 --- a/client/src/test/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImplTest.java +++ b/client/src/test/java/org/eclipse/hono/client/impl/ProtocolAdapterCommandConsumerFactoryImplTest.java @@ -137,7 +137,7 @@ public void setUp() { when(devConClient.setCommandHandlingAdapterInstance(anyString(), anyString(), any(), any())) .thenReturn(Future.succeededFuture()); when(devConClient.removeCommandHandlingAdapterInstance(anyString(), anyString(), any())) - .thenReturn(Future.succeededFuture(Boolean.TRUE)); + .thenReturn(Future.succeededFuture()); commandConsumerFactory = new ProtocolAdapterCommandConsumerFactoryImpl(connection, SendMessageSampler.Factory.noop()); commandConsumerFactory.initialize(commandTargetMapper, deviceConnectionClientFactory); diff --git a/services/device-connection/src/main/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionService.java b/services/device-connection/src/main/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionService.java index 12a3352e4a3..448db2c694c 100644 --- a/services/device-connection/src/main/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionService.java +++ b/services/device-connection/src/main/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionService.java @@ -90,8 +90,7 @@ public Future setCommandHandlingAdapterInstance(final St public Future removeCommandHandlingAdapterInstance(final String tenantId, final String deviceId, final String adapterInstanceId, final Span span) { return cache.removeCommandHandlingAdapterInstance(tenantId, deviceId, adapterInstanceId, span) - .map(removed -> removed ? DeviceConnectionResult.from(HttpURLConnection.HTTP_NO_CONTENT) - : DeviceConnectionResult.from(HttpURLConnection.HTTP_PRECON_FAILED)) + .map(v -> DeviceConnectionResult.from(HttpURLConnection.HTTP_NO_CONTENT)) .otherwise(t -> DeviceConnectionResult.from(ServiceInvocationException.extractStatusCode(t))); } diff --git a/services/device-connection/src/test/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionServiceTest.java b/services/device-connection/src/test/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionServiceTest.java index f7e1d69ec9a..9f19f8bbbb1 100644 --- a/services/device-connection/src/test/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionServiceTest.java +++ b/services/device-connection/src/test/java/org/eclipse/hono/deviceconnection/infinispan/CacheBasedDeviceConnectionServiceTest.java @@ -184,7 +184,7 @@ public void testRemoveCommandHandlingAdapterInstanceReturningTrue(final VertxTes final String deviceId = "testDevice"; final String adapterInstanceId = "adapterInstanceId"; when(cache.removeCommandHandlingAdapterInstance(anyString(), anyString(), anyString(), any(Span.class))) - .thenReturn(Future.succeededFuture(true)); + .thenReturn(Future.succeededFuture()); givenAStartedService() .compose(ok -> svc.removeCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, deviceId, adapterInstanceId, span)) @@ -210,7 +210,7 @@ public void testRemoveCommandHandlingAdapterInstanceReturningFalse(final VertxTe final String deviceId = "testDevice"; final String adapterInstanceId = "adapterInstanceId"; when(cache.removeCommandHandlingAdapterInstance(anyString(), anyString(), anyString(), any(Span.class))) - .thenReturn(Future.succeededFuture(false)); + .thenReturn(Future.failedFuture(new ClientErrorException(HttpURLConnection.HTTP_PRECON_FAILED))); givenAStartedService() .compose(ok -> svc.removeCommandHandlingAdapterInstance(Constants.DEFAULT_TENANT, deviceId, adapterInstanceId, span)) diff --git a/tests/src/test/java/org/eclipse/hono/tests/jms/JmsBasedDeviceConnectionClient.java b/tests/src/test/java/org/eclipse/hono/tests/jms/JmsBasedDeviceConnectionClient.java index 2dcbfdb1ceb..e8e2d9cef8f 100644 --- a/tests/src/test/java/org/eclipse/hono/tests/jms/JmsBasedDeviceConnectionClient.java +++ b/tests/src/test/java/org/eclipse/hono/tests/jms/JmsBasedDeviceConnectionClient.java @@ -30,7 +30,6 @@ import org.eclipse.hono.util.CacheDirective; import org.eclipse.hono.util.DeviceConnectionConstants; import org.eclipse.hono.util.DeviceConnectionConstants.DeviceConnectionAction; -import org.eclipse.hono.util.DeviceConnectionResult; import org.eclipse.hono.util.MessageHelper; import org.eclipse.hono.util.RequestResponseResult; @@ -152,40 +151,17 @@ public Future getCommandHandlingAdapterInstances( * {@inheritDoc} */ @Override - public Future removeCommandHandlingAdapterInstance( + public Future removeCommandHandlingAdapterInstance( final String deviceId, final String adapterInstanceId, final SpanContext context) { - return createRequestMessage( + return sendRequest( DeviceConnectionAction.REMOVE_CMD_HANDLING_ADAPTER_INSTANCE.getSubject(), Map.of(MessageHelper.APP_PROPERTY_DEVICE_ID, deviceId, MessageHelper.APP_PROPERTY_ADAPTER_INSTANCE_ID, adapterInstanceId), null) - .compose(this::send) - .recover(thr -> { - final int errorCode = ServiceInvocationException.extractStatusCode(thr); - if (errorCode == HttpURLConnection.HTTP_NOT_FOUND - || errorCode == HttpURLConnection.HTTP_PRECON_FAILED) { - return Future.succeededFuture(DeviceConnectionResult.from(errorCode)); - } - return Future.failedFuture(thr); - }).compose(devConResult -> { - final Promise result = Promise.promise(); - switch (devConResult.getStatus()) { - case HttpURLConnection.HTTP_NO_CONTENT: - result.complete(Boolean.TRUE); - break; - case HttpURLConnection.HTTP_NOT_FOUND: - case HttpURLConnection.HTTP_PRECON_FAILED: - result.complete(Boolean.FALSE); - break; - default: - result.fail(new ServerErrorException(HttpURLConnection.HTTP_INTERNAL_ERROR, - "unsupported response status: " + devConResult.getStatus())); - } - return result.future(); - }); + .mapEmpty(); } /** diff --git a/tests/src/test/java/org/eclipse/hono/tests/registry/DeviceConnectionApiTests.java b/tests/src/test/java/org/eclipse/hono/tests/registry/DeviceConnectionApiTests.java index 2d9431444f2..62f5e5b7923 100644 --- a/tests/src/test/java/org/eclipse/hono/tests/registry/DeviceConnectionApiTests.java +++ b/tests/src/test/java/org/eclipse/hono/tests/registry/DeviceConnectionApiTests.java @@ -201,15 +201,12 @@ public void testRemoveCommandHandlingAdapterInstanceSucceeds(final VertxTestCont .map(client)) // then remove it .compose(client -> client.removeCommandHandlingAdapterInstance(deviceId, adapterInstance, null)) - .onComplete(ctx.succeeding(result -> { - ctx.verify(() -> assertThat(result).isTrue()); - ctx.completeNow(); - })); + .onComplete(ctx.completing()); } /** - * Verifies that a request to remove the command-handling adapter instance for a device succeeds with - * a false value if no adapter is registered for the device. + * Verifies that a request to remove the command-handling adapter instance for a device fails if no + * adapter is registered for the device. * * @param ctx The vert.x test context. */ @@ -219,8 +216,8 @@ public void testRemoveCommandHandlingAdapterInstanceFailsForNonExistingEntry(fin getClient(randomId()) .compose(client -> client.removeCommandHandlingAdapterInstance("non-existing-device", "adapterOne", null)) - .onComplete(ctx.succeeding(result -> { - ctx.verify(() -> assertThat(result).isFalse()); + .onComplete(ctx.failing(t -> { + ctx.verify(() -> assertErrorCode(t, HttpURLConnection.HTTP_PRECON_FAILED)); ctx.completeNow(); })); } @@ -240,8 +237,8 @@ public void testRemoveCommandHandlingAdapterInstanceFailsForNonMatchingAdapterIn getClient(randomId()) .compose(client -> client.setCommandHandlingAdapterInstance(deviceId, "adapterOne", Duration.ofMinutes(5), null).map(client)) .compose(client -> client.removeCommandHandlingAdapterInstance(deviceId, "notAdapterOne", null)) - .onComplete(ctx.succeeding(result -> { - ctx.verify(() -> assertThat(result).isFalse()); + .onComplete(ctx.failing(t -> { + ctx.verify(() -> assertErrorCode(t, HttpURLConnection.HTTP_PRECON_FAILED)); ctx.completeNow(); })); }