Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a Redis backed version of the command-router service #3621

Closed
wants to merge 2 commits into from
Closed

Implement a Redis backed version of the command-router service #3621

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a772395
Split services/command-router module into three modules (base, infini…
StFS Apr 2, 2024
b3485f4
Remove commented out dependencies
StFS Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion bom/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,14 +427,34 @@ quarkus.vertx.max-event-loop-execute-time=${max.event-loop.execute-time:20000}
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-service-command-router</artifactId>
<artifactId>hono-service-command-router-base</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-service-command-router-infinispan</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-service-command-router-redis</artifactId>
<version>${project.version}</version>
</dependency>
Comment on lines +430 to +442
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created two "concrete" modules here that will produce docker images: hono-service-command-router-infinispan and hono-service-command-router-redis.

The problem with that is that we'd be changing the legacy name of the command router service (hono-service-command-router). So the question is whether I should instead use the legacy name for the Infinispan implementation, in order to maintain some sort of a backwards compatibility? It feels "cleaner" to me to explicitly name the docker images with the technology that they support and that's the way it's done in the device-registry service (one is explicitly named -mongodb and the other one is -jdbc). But that could potentially cause some trouble where the old image name suddenly stops being produced.

Another solution to this could be to have the pipeline that builds and publishes the images tag the -infinispan image additionally with the old name as well?

<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>client-device-connection</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>client-device-connection-infinispan</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>client-device-connection-redis</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-client-application</artifactId>
Expand Down
15 changes: 14 additions & 1 deletion client-device-connection-infinispan/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>

<dependency>
<groupId>org.infinispan</groupId>
<artifactId>infinispan-core-jakarta</artifactId>
Expand All @@ -59,18 +60,30 @@
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-core</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>client-device-connection</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-client-common</artifactId>
</dependency>

<dependency>
<groupId>io.vertx</groupId>
<artifactId>vertx-web</artifactId>
<artifactId>vertx-core</artifactId>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed a dependency on vertx-web and replaced it with vertx-core which does not seem to have had any adverse effects. So my feeling is that vertx-web was not needed and vertx-core is sufficient.

</dependency>

<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-core</artifactId>
Expand Down
1 change: 1 addition & 0 deletions ...inispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/BasicCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.function.Function;

import org.eclipse.hono.client.ServerErrorException;
import org.eclipse.hono.deviceconnection.common.Cache;
import org.eclipse.hono.util.Futures;
import org.eclipse.hono.util.Lifecycle;
import org.infinispan.commons.api.BasicCacheContainer;
Expand Down
2 changes: 1 addition & 1 deletion ...nispan/src/main/java/org/eclipse/hono/deviceconnection/infinispan/client/HotrodCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;

import org.eclipse.hono.deviceconnection.common.CommonCacheConfig;
import org.infinispan.client.hotrod.RemoteCache;
import org.infinispan.client.hotrod.RemoteCacheContainer;
import org.infinispan.client.hotrod.RemoteCacheManager;
Expand Down Expand Up @@ -174,7 +175,6 @@ protected <T> void postCacheAccess(final AsyncResult<T> cacheOperationResult) {
*/
@Override
public Future<JsonObject> checkForCacheAvailability() {

if (isStarted()) {
final ConnectionCheckResult lastResult = lastConnectionCheckResult;
if (lastResult != null && !lastResult.isOlderThan(CACHED_CONNECTION_CHECK_RESULT_MAX_AGE)) {
Expand Down
18 changes: 2 additions & 16 deletions ...an/client/QuarkusPropertyBindingTest.java → ...emoteCacheQuarkusPropertyBindingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,10 @@
import org.junit.jupiter.api.Test;

/**
* Tests verifying binding of configuration properties to {@link CommonCacheConfig} and
* {@link InfinispanRemoteConfigurationProperties}.
* Tests verifying binding of configuration properties to {@link InfinispanRemoteConfigurationProperties}.
*
*/
public class QuarkusPropertyBindingTest {

@Test
void testCommonCacheConfigurationPropertiesArePickedUp() {

final var commonCacheConfig = new CommonCacheConfig(
ConfigMappingSupport.getConfigMapping(
CommonCacheOptions.class,
this.getClass().getResource("/common-cache-options.yaml")));

assertThat(commonCacheConfig.getCacheName()).isEqualTo("the-cache");
assertThat(commonCacheConfig.getCheckKey()).isEqualTo("the-key");
assertThat(commonCacheConfig.getCheckValue()).isEqualTo("the-value");
}
public class RemoteCacheQuarkusPropertyBindingTest {

@SuppressWarnings("deprecation")
@Test
Expand Down
122 changes: 122 additions & 0 deletions client-device-connection-redis/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
Copyright (c) 2023 Contributors to the Eclipse Foundation

See the NOTICE file(s) distributed with this work for additional
information regarding copyright ownership.

This program and the accompanying materials are made available under the
terms of the Eclipse Public License 2.0 which is available at
http://www.eclipse.org/legal/epl-2.0

SPDX-License-Identifier: EPL-2.0
-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-bom</artifactId>
<version>2.6.0-SNAPSHOT</version>
<relativePath>../bom</relativePath>
</parent>
<artifactId>client-device-connection-redis</artifactId>

<name>Redis Device Connection client</name>
<description>A Redis based client for accessing device connection information in a Redis cluster.</description>

<dependencies>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>client-device-connection</artifactId>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-redis-client</artifactId>
</dependency>

<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-legal</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>hono-core</artifactId>
</dependency>

<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>

<!--
<dependency>
<groupId>io.vertx</groupId>
<artifactId>vertx-core</artifactId>
</dependency>
<dependency>
<groupId>io.vertx</groupId>
<artifactId>vertx-redis-client</artifactId>
</dependency>
-->

<!-- testing -->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently there are no unit tests in this module. If none are added, these dependencies should be removed. I need to dive into this a bit and see if I can figure out some way to set up mock tests for the redis cache.

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.truth</groupId>
<artifactId>truth</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.vertx</groupId>
<artifactId>vertx-junit5</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.hono</groupId>
<artifactId>core-test-utils</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
</plugin>
<plugin>
<groupId>org.jboss.jandex</groupId>
<artifactId>jandex-maven-plugin</artifactId>
</plugin>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
</plugin>
</plugins>
</build>
</project>
Loading
Loading