Fix RSQL filter for no target tag and OR (#1824)

* Fix RSQL filter for no target tag and OR * add test for such filter * Clean up the code keeps the legacy Rsql Visitor which could be used with hawkbit.rsql.legacyRsqlVisitor=true --------- Signed-off-by: Marinov Avgustin <Avgustin.Marinov@bosch.com>
2024-08-27 10:48:55 +03:00
parent 55cc600114
commit d9d4469a95
9 changed files with 592 additions and 21 deletions
--- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java
@@ -1203,6 +1203,16 @@ class TargetManagementTest extends AbstractJpaIntegrationTest {
        return target;
    }

+    private Target createTargetWithTargetTypeAndMetadata(final String controllerId, final long targetTypeId, final int count) {
+        final Target target = testdataFactory.createTarget(controllerId, controllerId, targetTypeId);
+
+        for (int index = 1; index <= count; index++) {
+            insertTargetMetadata("key" + index, controllerId + "-value" + index, target);
+        }
+
+        return target;
+    }
+
    @Test
    @WithUser(allSpPermissions = true)
    @Description("Checks that target type is not assigned to target if invalid.")
@@ -1278,6 +1288,24 @@ class TargetManagementTest extends AbstractJpaIntegrationTest {
        validateFoundTargetsByRsql(rsqlOrControllerIdNotEqualFilter, controllerId1, controllerId2);
    }

+    @Test
+    @Description("Test that RSQL filter finds targets with tag and metadata.")
+    void findTargetsByRsqlWithTypeAndMetadata() {
+        final String controllerId1 = "target1";
+        final String controllerId2 = "target2";
+        createTargetWithMetadata(controllerId1, 2);
+        final TargetType type = testdataFactory.createTargetType("type1", Collections.emptyList());
+        createTargetWithTargetTypeAndMetadata(controllerId2, type.getId(), 2);
+
+        assertThat(targetManagement.count()).as("Total targets").isEqualTo(2);
+
+        final String rsqlAndByBoth = "targettype.key==type1 or metadata.key1==target1-value1";
+        validateFoundTargetsByRsql(rsqlAndByBoth, controllerId1, controllerId2);
+
+        final String rsqlAndControllerIdFilter = "targettype.key==type1 and metadata.key1==target1-value1";
+        validateFoundTargetsByRsql(rsqlAndControllerIdFilter);
+    }
+
    @Test
    @Description("Target matches filter.")
    void matchesFilter() {
--- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetTagManagementTest.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetTagManagementTest.java
@@ -273,7 +273,7 @@ class TargetTagManagementTest extends AbstractJpaIntegrationTest {
    }

    @Test
-    @Description("Ensures that a tag cannot be created if one exists already with that name (ecpects EntityAlreadyExistsException).")
+    @Description("Ensures that a tag cannot be created if one exists already with that name (expects EntityAlreadyExistsException).")
    void failedDuplicateTargetTagNameException() {
        targetTagManagement.create(entityFactory.tag().create().name("A"));
        assertThatExceptionOfType(EntityAlreadyExistsException.class)
@@ -281,7 +281,7 @@ class TargetTagManagementTest extends AbstractJpaIntegrationTest {
    }

    @Test
-    @Description("Ensures that a tag cannot be updated to a name that already exists on another tag (ecpects EntityAlreadyExistsException).")
+    @Description("Ensures that a tag cannot be updated to a name that already exists on another tag (expects EntityAlreadyExistsException).")
    void failedDuplicateTargetTagNameExceptionAfterUpdate() {
        targetTagManagement.create(entityFactory.tag().create().name("A"));
        final TargetTag tag = targetTagManagement.create(entityFactory.tag().create().name("B"));
--- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldTest.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLTargetFieldTest.java
@@ -338,9 +338,9 @@ class RSQLTargetFieldTest extends AbstractJpaIntegrationTest {

    private void assertRSQLQuery(final String rsqlParam, final long expectedTargets) {
        final Slice<Target> findTargetPage = targetManagement.findByRsql(PAGE, rsqlParam);
-        final long countTargetsAll = targetManagement.countByRsql(rsqlParam);
        assertThat(findTargetPage).isNotNull();
-        assertThat(findTargetPage.getNumberOfElements()).isEqualTo(countTargetsAll).isEqualTo(expectedTargets);
+        assertThat(findTargetPage.getNumberOfElements()).isEqualTo(expectedTargets);
+        assertThat(targetManagement.countByRsql(rsqlParam)).isEqualTo(expectedTargets);
    }

    private void assertRSQLQueryThrowsException(final String rsqlParam) {
--- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java
@@ -34,6 +34,9 @@ import jakarta.persistence.criteria.Root;
 import jakarta.persistence.criteria.Subquery;
 import jakarta.persistence.metamodel.Attribute;

+import jakarta.persistence.metamodel.EntityType;
+import jakarta.persistence.metamodel.SingularAttribute;
+import jakarta.persistence.metamodel.Type;
 import org.eclipse.hawkbit.repository.DistributionSetFields;
 import org.eclipse.hawkbit.repository.FieldNameProvider;
 import org.eclipse.hawkbit.repository.SoftwareModuleFields;
@@ -51,6 +54,7 @@ import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer;
 import org.eclipse.hawkbit.repository.rsql.VirtualPropertyResolver;
 import org.eclipse.hawkbit.security.SystemSecurityContext;
 import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
@@ -126,6 +130,12 @@ public class RSQLUtilityTest {
    private static final TenantConfigurationValue<String> TEST_POLLING_OVERDUE_TIME_INTERVAL = TenantConfigurationValue
            .<String> builder().value("00:07:37").build();

+    @BeforeEach
+    public void beforeEach() {
+        setupRoot(baseSoftwareModuleRootMock);
+        setupRoot(subqueryRootMock);
+    }
+
    @Test
    @Description("Testing throwing exception in case of not allowed RSQL key")
    public void rsqlUnsupportedFieldExceptionTest() {
@@ -264,7 +274,7 @@ public class RSQLUtilityTest {

    @Test
    public <T> void correctRsqlBuildsPredicate() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "name==abc;version==1.2";
        when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.get("version")).thenReturn(baseSoftwareModuleRootMock);
@@ -285,7 +295,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlBuildsSimpleNotEqualPredicate() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "name!=abc";
        when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class);
@@ -309,7 +319,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlBuildsSimpleNotLikePredicate() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "name!=abc*";
        when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class);
@@ -333,7 +343,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlBuildsNotSimpleNotLikePredicate() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        // with this query a subquery has to be made, so it is no simple query
        final String correctRsql = "type!=abc";
        when(baseSoftwareModuleRootMock.get(anyString())).thenReturn(baseSoftwareModuleRootMock);
@@ -360,7 +370,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlBuildsEqualPredicateWithPercentage() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "name==a%";
        when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class);
@@ -381,7 +391,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlBuildsLikePredicateWithPercentage() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "name==a%*";
        when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class);
@@ -402,7 +412,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlBuildsLikePredicateWithPercentageSQLServer() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "name==a%*";
        when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class);
@@ -424,7 +434,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlBuildsLessThanPredicate() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "name=lt=abc";
        when(baseSoftwareModuleRootMock.get("name")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) SoftwareModule.class);
@@ -442,7 +452,7 @@ public class RSQLUtilityTest {

    @Test
    public void correctRsqlWithEnumValue() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "testfield==bumlux";
        when(baseSoftwareModuleRootMock.get("testfield")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) TestValueEnum.class);
@@ -459,7 +469,7 @@ public class RSQLUtilityTest {

    @Test
    public void wrongRsqlWithWrongEnumValue() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String correctRsql = "testfield==unknownValue";
        when(baseSoftwareModuleRootMock.get("testfield")).thenReturn(baseSoftwareModuleRootMock);
        when(baseSoftwareModuleRootMock.getJavaType()).thenReturn((Class) TestValueEnum.class);
@@ -478,7 +488,7 @@ public class RSQLUtilityTest {
    @Test
    @Description("Tests the resolution of overdue_ts placeholder in context of a RSQL expression.")
    public void correctRsqlWithOverdueMacro() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String overdueProp = "overdue_ts";
        final String overduePropPlaceholder = "${" + overdueProp + "}";
        final String correctRsql = "testfield=le=" + overduePropPlaceholder;
@@ -506,7 +516,7 @@ public class RSQLUtilityTest {
    @Test
    @Description("Tests RSQL expression with an unknown placeholder.")
    public void correctRsqlWithUnknownMacro() {
-        reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
+        reset0(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
        final String overdueProp = "unknown";
        final String overduePropPlaceholder = "${" + overdueProp + "}";
        final String correctRsql = "testfield=le=" + overduePropPlaceholder;
@@ -575,6 +585,28 @@ public class RSQLUtilityTest {
        RSQLUtility.validateRsqlFor(rsql, TestFieldEnum.class);
    }

+    private void reset0(final Object... mocks) {
+        reset(mocks);
+        if (Arrays.asList(mocks).contains(baseSoftwareModuleRootMock)) {
+            setupRoot(baseSoftwareModuleRootMock);
+        }
+        if (Arrays.asList(mocks).contains(subqueryRootMock)) {
+            setupRoot(subqueryRootMock);
+        }
+    }
+
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    private void setupRoot(final Root<?> root) {
+        final Type type = Mockito.mock(Type.class);
+        when(type.getPersistenceType()).thenReturn(Type.PersistenceType.BASIC);
+        final SingularAttribute singularAttribute = Mockito.mock(SingularAttribute.class);
+        when(singularAttribute.getType()).thenReturn(type);
+        final EntityType entityType = Mockito.mock(EntityType.class);
+        when(entityType.getAttribute(any())).thenReturn(singularAttribute);
+        when(entityType.getPersistenceType()).thenReturn(Type.PersistenceType.BASIC);
+        when(root.getModel()).thenReturn(entityType);
+    }
+
    private enum TestValueEnum {
        BUMLUX;
    }
--- a/hawkbit-repository/hawkbit-repository-jpa/src/test/resources/jpa-test.properties
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/resources/jpa-test.properties
@@ -10,7 +10,12 @@

 # Debug utility functions - START
 logging.level.org.eclipse.persistence=ERROR
+#incomment to see the debug of persistence, e.g. to see the generated SQLs
+#logging.level.org.eclipse.persistence=DEBUG
 spring.jpa.properties.eclipselink.logging.level=FINE
 spring.jpa.properties.eclipselink.logging.level.sql=FINE
 spring.jpa.properties.eclipselink.logging.parameters=true
 # Debug utility functions - END
+
+# enable / disable case sensitiveness of the DB when playing around
+#hawkbit.rsql.caseInsensitiveDB=true