Implement backend data field filter for H2 & Postgresql (similar to MongoDB) #3516

gdimitropoulos-sotec · 2023-07-24T08:48:13Z

Implement backend filter for H2 & Postgresql :

field/value filter
value filter of type bool/number/string
wildcard filter for string values
execute the same tests for H2 & Postgres
add documentation

Also-by: Matthias Feurer [email protected]
Signed-off-by: g.dimitropoulos [email protected]

- field/value filter - value filter of type bool/number/string - enable filter wildcards for string value - add documentation Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

- run same unittests for all JDBC implementations Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

sonatype-lift · 2023-07-24T08:48:16Z

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

sophokles73 · 2023-07-24T17:53:53Z

site/documentation/content/user-guide/device-registry.md

@@ -60,7 +60,7 @@ The tenants in the registry can be managed using the Device Registry Management
 The JDBC based registry implementation does not support the following features:

 * Tenants can be retrieved using the [search tenants]({{< relref "/api/management#tenants/searchTenants" >}})
- operation defined by the Device Registry Management API, but the *filterJson* and *sortJson* query parameters are
+ operation defined by the Device Registry Management API, but the *sortJson* query parameters are


... query parameter is (currently) being ignored.

sophokles73 · 2023-07-24T17:54:33Z

site/documentation/content/api/management/device-registry-v1.yaml

@@ -1687,10 +1687,11 @@ components:
 filterJson:
 name: filterJson
 in: query
- description: |
+ description: | 


can you remove the trailing whitespaces?

sophokles73 · 2023-07-24T17:55:40Z

...ase-jdbc/src/main/resources/org/eclipse/hono/service/base/jdbc/store/device/base.h2.sql.yaml

+ REPLACE(data, '"') LIKE CONCAT('%%', :field, ':', REPLACE(:value, '"'))
+ ORDER BY device_id ASC
+ LIMIT :page_size
+ OFFSET :page_offset


can you please add an EOL?

sophokles73 · 2023-07-24T17:55:49Z

.../src/main/resources/org/eclipse/hono/service/base/jdbc/store/device/base.postgresql.sql.yaml

+ jsonb_extract_path(data,'ext')->>:field LIKE CAST(:value as varchar)
+ ORDER BY device_id ASC
+ LIMIT :page_size
+ OFFSET :page_offset


can you please add an EOL?

sophokles73 · 2023-07-24T17:56:54Z

...s/base-jdbc/src/main/resources/org/eclipse/hono/service/base/jdbc/store/device/base.sql.yaml

+ FROM %s 
+ WHERE 


can you remove the trailing whitespace?

sophokles73 · 2023-07-24T18:03:24Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param key  The key of the device.
+ * @param span  The span to contribute to.


can you please remove the tabs here and in the other locations?

sophokles73 · 2023-07-24T18:04:36Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param field the field of filter expression
+ * @param value the value of the filter expression


please start with a capital letter and end with a period (.)

sophokles73 · 2023-07-24T18:07:24Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ // check if we got back a result, if not this will abort early
+ .compose(result -> extractVersionForUpdate(result, resourceVersion))
+
+ // take the version and start processing on
+ .compose(version -> Future.succeededFuture()
+
+ .compose(x -> {
+ final Promise<CredentialsDto> result = Promise.promise();
+ final var updatedCredentialsDto = CredentialsDto.forUpdate(


is anything being changed here or did you only change the indentation?

only the indentation

sophokles73 · 2023-07-24T18:09:17Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param tenantId The tenantId to search devices.
+ * @param pageSize The page size.
+ * @param pageOffset The page offset.
+ * @param filters The list of filters.


FMPOV we should document that only the first item in the list will be considered.

I change the "The list of filters." to "The list of filters (currently only the first value of the list is used)."

sophokles73 · 2023-07-24T18:12:37Z

...org/eclipse/hono/deviceregistry/jdbc/impl/JdbcPostgresDeviceManagementSearchDevicesTest.java

+import java.nio.file.Path;
+
+
+class JdbcPostgresDeviceManagementSearchDevicesTest extends JdbcBasedDeviceManagementSearchDevicesTest {


what is this class needed for?

This class changes the DEFAULT_DATABASE_TYPE from H2 to POSTGRESQL and runs all the tests from JdbcBasedDeviceManagementSearchDevicesTest. We need this to run the tests against both types of JDBC DBs.

The type of database being used when running the tests is determined by the AbstractJdbcRegistryTest.databaseType system property. The tests are being run for both H2 and Postgresql by means of the two configurations of the Maven Surefire plugin in services/device-registry-jdbc/pom.xml. FMPOV there is no need for this class.

i remove the class

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

sophokles73 · 2023-07-26T11:52:04Z

...y-jdbc/src/test/java/org/eclipse/hono/deviceregistry/jdbc/impl/AbstractJdbcRegistryTest.java

+ protected static DatabaseType DEFAULT_DATABASE_TYPE = DatabaseType.H2;
+ protected static DatabaseType DATABASE_TYPE = DatabaseType.valueOf(System.getProperty(AbstractJdbcRegistryTest.class.getSimpleName()


looks like these changes can be reverted as well now, can't they?

yes, i change it back to private final

sophokles73 · 2023-07-26T11:53:07Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param key  The key of the device to create.
+ * @param device  The device data.
+ * @param tenant  The configuration of the tenant that the device belongs to.
 * @param globalDevicesPerTenantLimit The globally defined maximum number of devices per tenant. A value
 * &lt;= 0 will be interpreted as no limit being defined.
- * @param spanContext The span to contribute to.
+ * @param spanContext  The span to contribute to.


can you please remove this indentation?

sophokles73 · 2023-07-26T11:53:19Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param key  The key of the device to update.
+ * @param statement  The statement to use for the update.
+ * @param jsonValue  The value to set.
 * @param resourceVersion The optional resource version.
- * @param nextVersion The new version to set.
- * @param span The span to contribute to.
+ * @param nextVersion  The new version to set.
+ * @param span  The span to contribute to.


remove tabs

sophokles73 · 2023-07-26T11:53:29Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param key  The key of the device to update.
+ * @param device  The device data to store.
 * @param resourceVersion The optional resource version.
- * @param spanContext The span to contribute to.
+ * @param spanContext  The span to contribute to.


remove tabs

sophokles73 · 2023-07-26T11:53:42Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

@@ -477,7 +547,7 @@ public Future<Versioned<Void>> updateDevice(
 * If there is exactly one row, it will read the device registration information from the column
 * {@code data} and optionally current resource version from the column {@code version}.
 *
- * @param key The key of the device to read.
+ * @param key  The key of the device to read.


remove tabs

sophokles73 · 2023-07-26T11:53:48Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param key  The key of the device to delete.
 * @param resourceVersion An optional resource version.
- * @param spanContext The span to contribute to.
+ * @param spanContext  The span to contribute to.


remove tabs

sophokles73 · 2023-07-26T11:53:56Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param tenantId The tenant to count devices for.
+ * @param spanContext The span to contribute to.
+ * @param countStatement The count statement to use.
+ * @param field The field of filter expression.
+ * @param value The value of the filter expression.


remove tabs

sophokles73 · 2023-07-26T11:54:03Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param key  The key of the device to update.
+ * @param credentials  The credentials to set.
 * @param resourceVersion The optional resource version to update.
- * @param spanContext The span to contribute to.
+ * @param spanContext  The span to contribute to.


remove tabs

sophokles73 · 2023-07-26T11:54:16Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

 /**
 * Get all credentials for a device.
 * <p>
 * This gets the credentials of a device. If the device cannot be found, the
 * result must be empty. If no credentials could be found for an existing device,
 * the result must not be empty, but provide an empty {@link CredentialsReadResult}.
 *
- * @param key The key of the device.
+ * @param key  The key of the device.


remove tabs

sophokles73 · 2023-07-26T11:54:38Z

...jdbc/src/main/java/org/eclipse/hono/service/base/jdbc/store/device/TableManagementStore.java

+ * @param tenantId The tenantId to search devices.
+ * @param pageSize The page size.
+ * @param pageOffset The page offset.
+ * @param filters The list of filters (currently only the first value of the list is used).


remove tabs

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

sophokles73

LGTM

… management API Implement backend filter for H2 & Postgresql: - field/value filter - value filter of type bool/number/string - enable filter wildcards for string value - add documentation Also-by: Matthias Feurer <[email protected]> Signed-off-by: George Dimitropoulos <[email protected]>

gdimitropoulos-sotec added 2 commits July 6, 2023 15:54

Implement backend filter for H2 & Postgresql:

ff0854c

- field/value filter - value filter of type bool/number/string - enable filter wildcards for string value - add documentation Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

- remove AbstractJdbcPostgresBaseRegistryTest class

edf501a

- run same unittests for all JDBC implementations Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

gdimitropoulos-sotec requested review from kaniyan and sophokles73 as code owners July 24, 2023 08:48

sophokles73 added enhancement JDBC Device Registry The JDBC based Device Registry implementation labels Jul 24, 2023

sophokles73 reviewed Jul 24, 2023

View reviewed changes

gdimitropoulos-sotec added 5 commits July 25, 2023 10:56

make changes after PR review

07ff350

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

remove tab from java docs

db5ddf6

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

remove tabs from java docs

d4e0c22

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

remove tabs and fix java doc

fd152b9

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

remove JdbcPostgresDeviceManagementSearchDevicesTest

46edc02

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

sophokles73 reviewed Jul 26, 2023

View reviewed changes

gdimitropoulos-sotec added 2 commits July 27, 2023 09:20

make DEFAULT_DATABASE_TYPE and type private final

c4255a5

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

remove tabs from javadoc

a77cff4

Also-by: Matthias Feurer [email protected] Signed-off-by: g.dimitropoulos <[email protected]>

sophokles73 added this to the 2.4.0 milestone Jul 27, 2023

sophokles73 approved these changes Jul 27, 2023

View reviewed changes

sophokles73 merged commit 3f2eee3 into eclipse-hono:master Jul 27, 2023
5 checks passed

		* @param key The key of the device.
		* @param span The span to contribute to.

		* @param field the field of filter expression
		* @param value the value of the filter expression

		import java.nio.file.Path;


		class JdbcPostgresDeviceManagementSearchDevicesTest extends JdbcBasedDeviceManagementSearchDevicesTest {

		protected static DatabaseType DEFAULT_DATABASE_TYPE = DatabaseType.H2;
		protected static DatabaseType DATABASE_TYPE = DatabaseType.valueOf(System.getProperty(AbstractJdbcRegistryTest.class.getSimpleName()

Implement backend data field filter for H2 & Postgresql (similar to MongoDB) #3516

Implement backend data field filter for H2 & Postgresql (similar to MongoDB) #3516

Conversation

gdimitropoulos-sotec commented Jul 24, 2023

sonatype-lift bot commented Jul 24, 2023

Sonatype Lift is retiring

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophokles73 left a comment

Choose a reason for hiding this comment