Fix CustomBaseRepositoryBean - not initialized in some cases (#2241)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>

hawkbit-repository/hawkbit-repository-core/pom.xml

@@ -29,7 +29,7 @@
         </dependency>
         <dependency>
             <groupId>org.apache.commons</groupId>
-            <artifactId>commons-lang3</artifactId>
+            <artifactId>commons-text</artifactId>
         </dependency>
         <dependency>
             <groupId>com.github.ben-manes.caffeine</groupId>

hawkbit-repository/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/rsql/VirtualPropertyResolver.java

@@ -11,8 +11,8 @@ package org.eclipse.hawkbit.repository.rsql;
 
 import java.io.Serial;
 
-import org.apache.commons.lang3.text.StrLookup;
+import org.apache.commons.text.StringSubstitutor;
-import org.apache.commons.lang3.text.StrSubstitutor;
+import org.apache.commons.text.lookup.StringLookupFactory;
 import org.eclipse.hawkbit.repository.TimestampCalculator;
 
 /**
@@ -41,31 +41,30 @@ import org.eclipse.hawkbit.repository.TimestampCalculator;
  * configuration.</li>
  * </ul>
  */
-public class VirtualPropertyResolver extends StrLookup<String> implements VirtualPropertyReplacer {
+public class VirtualPropertyResolver implements VirtualPropertyReplacer {
 
     @Serial
     private static final long serialVersionUID = 1L;
 
-    private transient StrSubstitutor substitutor;
+    private transient StringSubstitutor substitutor;
-
-    @Override
-    public String lookup(final String rhs) {
-        String resolved = null;
-
-        if ("now_ts".equalsIgnoreCase(rhs)) {
-            resolved = String.valueOf(System.currentTimeMillis());
-        } else if ("overdue_ts".equalsIgnoreCase(rhs)) {
-            resolved = String.valueOf(TimestampCalculator.calculateOverdueTimestamp());
-        }
-        return resolved;
-    }
 
     @Override
     public String replace(final String input) {
         if (substitutor == null) {
-            substitutor = new StrSubstitutor(this, StrSubstitutor.DEFAULT_PREFIX, StrSubstitutor.DEFAULT_SUFFIX,
+            substitutor = new StringSubstitutor(
-                    StrSubstitutor.DEFAULT_ESCAPE);
+                    StringLookupFactory.builder().get().functionStringLookup(this::lookup),
+                    StringSubstitutor.DEFAULT_PREFIX, StringSubstitutor.DEFAULT_SUFFIX, StringSubstitutor.DEFAULT_ESCAPE);
         }
         return substitutor.replace(input);
     }
+
+    private String lookup(final String rhs) {
+        if ("now_ts".equalsIgnoreCase(rhs)) {
+            return String.valueOf(System.currentTimeMillis());
+        } else if ("overdue_ts".equalsIgnoreCase(rhs)) {
+            return String.valueOf(TimestampCalculator.calculateOverdueTimestamp());
+        } else {
+            return null;
+        }
+    }
 }

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java

@@ -12,9 +12,11 @@ package org.eclipse.hawkbit.repository.jpa;
 import jakarta.persistence.EntityManager;
 
 import org.eclipse.hawkbit.repository.BaseRepositoryTypeProvider;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.jpa.repository.support.JpaRepositoryFactoryBean;
 import org.springframework.data.repository.Repository;
 import org.springframework.data.repository.core.support.RepositoryFactorySupport;
+import org.springframework.lang.NonNull;
 
 /**
  * A {@link JpaRepositoryFactoryBean} extension that allow injection of custom repository factories by using a
@@ -23,20 +25,24 @@ import org.springframework.data.repository.core.support.RepositoryFactorySupport
 @SuppressWarnings("java:S119") // java:S119 - ID is inherited from JpaRepositoryFactoryBean
 public class CustomBaseRepositoryFactoryBean<T extends Repository<S, ID>, S, ID> extends JpaRepositoryFactoryBean<T, S, ID> {
 
-    private final BaseRepositoryTypeProvider baseRepoProvider;
+    private BaseRepositoryTypeProvider baseRepoProvider;
 
     /**
      * Creates a new {@link JpaRepositoryFactoryBean} for the given repository interface.
      *
      * @param repositoryInterface must not be {@literal null}.
      */
-    public CustomBaseRepositoryFactoryBean(final Class<? extends T> repositoryInterface, final BaseRepositoryTypeProvider baseRepoProvider) {
+    public CustomBaseRepositoryFactoryBean(final Class<? extends T> repositoryInterface) {
         super(repositoryInterface);
+    }
+
+    @Autowired // if it is a constructor injection sometimes doesn't work - base repo provider is not available at construct time
+    public void setBaseRepoProvider(final BaseRepositoryTypeProvider baseRepoProvider) {
         this.baseRepoProvider = baseRepoProvider;
     }
 
     @Override
-    protected RepositoryFactorySupport createRepositoryFactory(final EntityManager entityManager) {
+    protected RepositoryFactorySupport createRepositoryFactory(@NonNull final EntityManager entityManager) {
         final RepositoryFactorySupport rfs = super.createRepositoryFactory(entityManager);
         rfs.setRepositoryBaseClass(baseRepoProvider.getBaseRepositoryType(getObjectType()));
         return rfs;

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java

@@ -187,7 +187,6 @@ import org.springframework.data.jpa.repository.config.EnableJpaAuditing;
 import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
 import org.springframework.integration.support.locks.LockRegistry;
 import org.springframework.lang.NonNull;
-import org.springframework.orm.jpa.vendor.Database;
 import org.springframework.retry.annotation.EnableRetry;
 import org.springframework.scheduling.annotation.EnableScheduling;
 import org.springframework.transaction.PlatformTransactionManager;
@@ -459,8 +458,7 @@ public class RepositoryApplicationConfiguration {
     }
 
     /**
-     * @return the singleton instance of the
+     * @return the singleton instance of the {@link AfterTransactionCommitExecutorHolder}
-     *         {@link AfterTransactionCommitExecutorHolder}
      */
     @Bean
     AfterTransactionCommitExecutorHolder afterTransactionCommitExecutorHolder() {
@@ -475,6 +473,17 @@ public class RepositoryApplicationConfiguration {
         return new ExceptionMappingAspectHandler();
     }
 
+    /**
+     * Default {@link BaseRepositoryTypeProvider} bean always provides the NoCountBaseRepository
+     *
+     * @return a {@link BaseRepositoryTypeProvider} bean
+     */
+    @Bean
+    @ConditionalOnMissingBean
+    BaseRepositoryTypeProvider baseRepositoryTypeProvider() {
+        return new HawkbitBaseRepository.RepositoryTypeProvider();
+    }
+
     /**
      * {@link JpaSystemManagement} bean.
      *
@@ -1027,18 +1036,6 @@ public class RepositoryApplicationConfiguration {
                 tenantAware, lockRegistry, systemSecurityContext);
     }
 
-    /**
-     * Default {@link BaseRepositoryTypeProvider} bean always provides the
-     * NoCountBaseRepository
-     *
-     * @return a {@link BaseRepositoryTypeProvider} bean
-     */
-    @Bean
-    @ConditionalOnMissingBean
-    BaseRepositoryTypeProvider baseRepositoryTypeProvider() {
-        return new HawkbitBaseRepository.RepositoryTypeProvider();
-    }
-
     /**
      * Default artifact encryption service bean that internally uses
      * {@link ArtifactEncryption} and {@link ArtifactEncryptionSecretsStore} beans

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/RSQLUtilityTest.java

@@ -437,10 +437,7 @@ class RSQLUtilityTest {
                 .toPredicate(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
 
         // verification
-        verify(macroResolver).lookup(overdueProp);
+        // the macro is already replaced when passed to #lessThanOrEqualTo -> the method is never invoked with the placeholder:
-        // the macro is already replaced when passed to #lessThanOrEqualTo ->
-        // the method is never invoked with the
-        // placeholder:
         verify(criteriaBuilderMock, never()).lessThanOrEqualTo(pathOfString(baseSoftwareModuleRootMock), overduePropPlaceholder);
     }
 
@@ -462,9 +459,7 @@ class RSQLUtilityTest {
                 .toPredicate(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
 
         // verification
-        verify(macroResolver).lookup(overdueProp);
+        // the macro is unknown and hence never replaced -> #lessThanOrEqualTo is invoked with the placeholder:
-        // the macro is unknown and hence never replaced -> #lessThanOrEqualTo
-        // is invoked with the placeholder:
         verify(criteriaBuilderMock).lessThanOrEqualTo(pathOfString(baseSoftwareModuleRootMock), overduePropPlaceholder);
     }
 

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/rsql/VirtualPropertyResolverTest.java

@@ -17,7 +17,7 @@ import java.util.concurrent.Callable;
 import io.qameta.allure.Description;
 import io.qameta.allure.Feature;
 import io.qameta.allure.Story;
-import org.apache.commons.lang3.text.StrSubstitutor;
+import org.apache.commons.text.StringSubstitutor;
 import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
 import org.eclipse.hawkbit.repository.model.TenantConfigurationValue;
 import org.eclipse.hawkbit.repository.model.helper.SystemSecurityContextHolder;
@@ -31,7 +31,6 @@ import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mockito;
-import org.mockito.Spy;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.test.context.bean.override.mockito.MockitoBean;
@@ -47,13 +46,12 @@ class VirtualPropertyResolverTest {
     private static final TenantConfigurationValue<String> TEST_POLLING_OVERDUE_TIME_INTERVAL =
             TenantConfigurationValue.<String> builder().value("00:07:37").build();
 
-    @Spy
-    private final VirtualPropertyResolver resolverUnderTest = new VirtualPropertyResolver();
     @MockitoBean
     private TenantConfigurationManagement confMgmt;
     @MockitoBean
     private SystemSecurityContext securityContext;
-    private StrSubstitutor substitutor;
+
+    private final VirtualPropertyResolver substitutor = new VirtualPropertyResolver();
 
     @BeforeEach
     void before() {
@@ -61,9 +59,6 @@ class VirtualPropertyResolverTest {
                 .thenReturn(TEST_POLLING_TIME_INTERVAL);
         when(confMgmt.getConfigurationValue(TenantConfigurationKey.POLLING_OVERDUE_TIME_INTERVAL, String.class))
                 .thenReturn(TEST_POLLING_OVERDUE_TIME_INTERVAL);
-
-        substitutor = new StrSubstitutor(resolverUnderTest, StrSubstitutor.DEFAULT_PREFIX,
-                StrSubstitutor.DEFAULT_SUFFIX, StrSubstitutor.DEFAULT_ESCAPE);
     }
 
     @Test
@@ -80,8 +75,8 @@ class VirtualPropertyResolverTest {
     @Description("Tests escape mechanism for placeholders (syntax is $${SOME_PLACEHOLDER}).")
     void handleEscapedPlaceholder() {
         final String placeholder = "${OVERDUE_TS}";
-        final String escaptedPlaceholder = StrSubstitutor.DEFAULT_ESCAPE + placeholder;
+        final String escapedPlaceholder = StringSubstitutor.DEFAULT_ESCAPE + placeholder;
-        final String testString = "lhs=lt=" + escaptedPlaceholder;
+        final String testString = "lhs=lt=" + escapedPlaceholder;
 
         final String resolvedPlaceholders = substitutor.replace(testString);
         assertThat(resolvedPlaceholders).as("Escaped OVERDUE_TS should not be resolved!").contains(placeholder);
@@ -89,14 +84,13 @@ class VirtualPropertyResolverTest {
 
     @ParameterizedTest
     @ValueSource(strings = { "${NOW_TS}", "${OVERDUE_TS}", "${overdue_ts}" })
-    @Description("Tests resolution of NOW_TS by using a StrSubstitutor configured with the VirtualPropertyResolver.")
+    @Description("Tests resolution of NOW_TS by using a StringSubstitutor configured with the VirtualPropertyResolver.")
     void resolveNowTimestampPlaceholder(final String placeholder) {
         when(securityContext.runAsSystem(Mockito.any())).thenAnswer(a -> ((Callable<?>) a.getArgument(0)).call());
         final String testString = "lhs=lt=" + placeholder;
 
         final String resolvedPlaceholders = substitutor.replace(testString);
-        assertThat(resolvedPlaceholders).as("'%s' placeholder was not replaced", placeholder)
+        assertThat(resolvedPlaceholders).as("'%s' placeholder was not replaced", placeholder).doesNotContain(placeholder);
-                .doesNotContain(placeholder);
     }
 
     @Configuration

hawkbit-repository/hawkbit-repository-test/pom.xml

@@ -104,10 +104,6 @@
             <groupId>org.springframework</groupId>
             <artifactId>spring-tx</artifactId>
         </dependency>
-        <dependency>
-            <groupId>org.apache.commons</groupId>
-            <artifactId>commons-lang3</artifactId>
-        </dependency>
         <dependency>
             <groupId>org.springframework.boot</groupId>
             <artifactId>spring-boot-starter-test</artifactId>

hawkbit-rest-core/pom.xml

@@ -65,10 +65,6 @@
             <scope>provided</scope>
         </dependency>
 
-        <dependency>
-            <groupId>org.apache.commons</groupId>
-            <artifactId>commons-lang3</artifactId>
-        </dependency>
         <dependency>
             <groupId>commons-io</groupId>
             <artifactId>commons-io</artifactId>

pom.xml

@@ -62,6 +62,7 @@
         <rsql-parser.version>2.1.0</rsql-parser.version>
         <commons-io.version>2.16.1</commons-io.version>
         <commons-collections4.version>4.4</commons-collections4.version>
+        <commons-text.version>1.13.0</commons-text.version>
         <io-protostuff.version>1.8.0</io-protostuff.version>
         <!-- test -->
         <rabbitmq.http-client.version>5.3.0</rabbitmq.http-client.version>
@@ -712,6 +713,11 @@
                 <artifactId>commons-collections4</artifactId>
                 <version>${commons-collections4.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.apache.commons</groupId>
+                <artifactId>commons-text</artifactId>
+                <version>${commons-text.version}</version>
+            </dependency>
 
             <!-- Test -->
             <dependency>