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.

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

Fix CustomBaseRepositoryBean - not initialized in some cases #2241

Merged
merged 1 commit into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion hawkbit-repository/hawkbit-repository-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

import java.io.Serial;

import org.apache.commons.lang3.text.StrLookup;
import org.apache.commons.lang3.text.StrSubstitutor;
import org.apache.commons.text.StringSubstitutor;
import org.apache.commons.text.lookup.StringLookupFactory;
import org.eclipse.hawkbit.repository.TimestampCalculator;

/**
Expand Down Expand Up @@ -41,31 +41,30 @@
* 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;

@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;
}
private transient StringSubstitutor substitutor;

@Override
public String replace(final String input) {
if (substitutor == null) {
substitutor = new StrSubstitutor(this, StrSubstitutor.DEFAULT_PREFIX, StrSubstitutor.DEFAULT_SUFFIX,
StrSubstitutor.DEFAULT_ESCAPE);
substitutor = new StringSubstitutor(
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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
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
Expand All @@ -23,20 +25,24 @@
@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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@
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;
Expand Down Expand Up @@ -459,8 +458,7 @@ EntityInterceptorHolder entityInterceptorHolder() {
}

/**
* @return the singleton instance of the
* {@link AfterTransactionCommitExecutorHolder}
* @return the singleton instance of the {@link AfterTransactionCommitExecutorHolder}
*/
@Bean
AfterTransactionCommitExecutorHolder afterTransactionCommitExecutorHolder() {
Expand All @@ -475,6 +473,17 @@ ExceptionMappingAspectHandler createRepositoryExceptionHandlerAdvice() {
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.
*
Expand Down Expand Up @@ -1027,18 +1036,6 @@ JpaDistributionSetInvalidationManagement distributionSetInvalidationManagement(
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,10 +437,7 @@ void correctRsqlWithOverdueMacro() {
.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);
}

Expand All @@ -462,9 +459,7 @@ void correctRsqlWithUnknownMacro() {
.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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
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;
Expand All @@ -31,7 +31,6 @@
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;
Expand All @@ -47,23 +46,19 @@ 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() {
when(confMgmt.getConfigurationValue(TenantConfigurationKey.POLLING_TIME_INTERVAL, String.class))
.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
Expand All @@ -80,23 +75,22 @@ void handleUnknownPlaceholder() {
@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 testString = "lhs=lt=" + escaptedPlaceholder;
final String escapedPlaceholder = StringSubstitutor.DEFAULT_ESCAPE + placeholder;
final String testString = "lhs=lt=" + escapedPlaceholder;

final String resolvedPlaceholders = substitutor.replace(testString);
assertThat(resolvedPlaceholders).as("Escaped OVERDUE_TS should not be resolved!").contains(placeholder);
}

@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)
.doesNotContain(placeholder);
assertThat(resolvedPlaceholders).as("'%s' placeholder was not replaced", placeholder).doesNotContain(placeholder);
}

@Configuration
Expand All @@ -112,4 +106,4 @@ SystemSecurityContextHolder systemSecurityContextHolder() {
return SystemSecurityContextHolder.getInstance();
}
}
}
}
4 changes: 0 additions & 4 deletions hawkbit-repository/hawkbit-repository-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 0 additions & 4 deletions hawkbit-rest-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
Loading