Fix findings from overdue review:

* Introduce VirtualPropertyLookup interface
* Remove Apache StrLookup from signature and use VirtualPropertyLookup
* Use (internal) adapter to adapt VirtualPropertyLookup to StrLookup
* Use interface instead of implementation for auto-wiring
* Extract calculation for overdue_ts to service TimestampCalculator

Signed-off-by: Marcel Mager (INST-IOT/ESB) <Marcel.Mager@bosch-si.com>
This commit is contained in:
Marcel Mager (INST-IOT/ESB)
2016-09-15 11:38:38 +02:00
parent 76c3c96075
commit 00aaed3027
16 changed files with 283 additions and 181 deletions

View File

@@ -20,7 +20,6 @@ import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root;
import javax.persistence.metamodel.Attribute;
import org.apache.commons.lang3.text.StrLookup;
import org.eclipse.hawkbit.repository.DistributionSetFields;
import org.eclipse.hawkbit.repository.FieldNameProvider;
import org.eclipse.hawkbit.repository.SoftwareModuleFields;
@@ -28,6 +27,7 @@ import org.eclipse.hawkbit.repository.TargetFields;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.repository.exception.RSQLParameterSyntaxException;
import org.eclipse.hawkbit.repository.exception.RSQLParameterUnsupportedFieldException;
import org.eclipse.hawkbit.repository.jpa.TimestampCalculator;
import org.eclipse.hawkbit.repository.model.SoftwareModule;
import org.eclipse.hawkbit.repository.model.TenantConfigurationValue;
import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationKey;
@@ -49,11 +49,14 @@ import ru.yandex.qatools.allure.annotations.Stories;
public class RSQLUtilityTest {
@Spy
VirtualPropertyMakroResolver makroResolver = new VirtualPropertyMakroResolver();
VirtualPropertyResolver macroResolver = new VirtualPropertyResolver();
@Mock
TenantConfigurationManagement confMgmt;
@Mock
TimestampCalculator timestampCalculator;
@Mock
private Root<Object> baseSoftwareModuleRootMock;
@@ -271,7 +274,7 @@ public class RSQLUtilityTest {
@Test
@Description("Tests the resolution of overdue_ts placeholder in context of a RSQL expression.")
public void correctRsqlWithOverdueMakro() {
public void correctRsqlWithOverdueMacro() {
reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
final String overdueProp = "overdue_ts";
final String overduePropPlaceholder = "${" + overdueProp + "}";
@@ -283,12 +286,12 @@ public class RSQLUtilityTest {
.thenReturn(mock(Predicate.class));
// test
Predicate result = RSQLUtility.parse(correctRsql, TestFieldEnum.class, setupMakroLookup())
Predicate result = RSQLUtility.parse(correctRsql, TestFieldEnum.class, setupMacroLookup())
.toPredicate(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
// verfication
verify(makroResolver, times(1)).lookup(overdueProp);
// the makro is already replaced when passed to #lessThanOrEqualTo -> the method is never invoked with the
verify(macroResolver, times(1)).lookup(overdueProp);
// the macro is already replaced when passed to #lessThanOrEqualTo -> the method is never invoked with the
// placeholder:
verify(criteriaBuilderMock, never()).lessThanOrEqualTo(eq(pathOfString(baseSoftwareModuleRootMock)),
eq(overduePropPlaceholder));
@@ -296,7 +299,7 @@ public class RSQLUtilityTest {
@Test
@Description("Tests RSQL expression with an unknown placeholder.")
public void correctRsqlWithUnknownMakro() {
public void correctRsqlWithUnknownMacro() {
reset(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
final String overdueProp = "unknown";
final String overduePropPlaceholder = "${" + overdueProp + "}";
@@ -308,24 +311,30 @@ public class RSQLUtilityTest {
.thenReturn(mock(Predicate.class));
// test
Predicate result = RSQLUtility.parse(correctRsql, TestFieldEnum.class, setupMakroLookup())
Predicate result = RSQLUtility.parse(correctRsql, TestFieldEnum.class, setupMacroLookup())
.toPredicate(baseSoftwareModuleRootMock, criteriaQueryMock, criteriaBuilderMock);
// verfication
verify(makroResolver, times(1)).lookup(overdueProp);
// the makro is unknown and hence never replaced -> #lessThanOrEqualTo is invoked with the placeholder:
verify(macroResolver, times(1)).lookup(overdueProp);
// the macro is unknown and hence never replaced -> #lessThanOrEqualTo is invoked with the placeholder:
verify(criteriaBuilderMock, times(1)).lessThanOrEqualTo(eq(pathOfString(baseSoftwareModuleRootMock)),
eq(overduePropPlaceholder));
}
public StrLookup<String> setupMakroLookup() {
public VirtualPropertyLookup setupMacroLookup() {
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);
when(makroResolver.getTenantConfigurationManagement()).thenReturn(confMgmt);
return makroResolver;
when(macroResolver.getTimestampCalculator()).thenReturn(new TimestampCalculator() {
@Override
protected TenantConfigurationManagement getTenantConfigurationManagement() {
return confMgmt;
}
});
return macroResolver;
}
@SuppressWarnings("unchecked")

View File

@@ -15,6 +15,7 @@ import java.time.Instant;
import org.apache.commons.lang3.text.StrSubstitutor;
import org.eclipse.hawkbit.repository.TenantConfigurationManagement;
import org.eclipse.hawkbit.repository.jpa.TimestampCalculator;
import org.eclipse.hawkbit.repository.model.TenantConfigurationValue;
import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationKey;
import org.junit.Before;
@@ -31,10 +32,10 @@ import ru.yandex.qatools.allure.annotations.Stories;
@Features("Unit Tests - Repository")
@Stories("Placeholder resolution for virtual properties")
@RunWith(MockitoJUnitRunner.class)
public class VirtualPropertyMakroResolverTest {
public class VirtualPropertyResolverTest {
@Spy
VirtualPropertyMakroResolver resolverUnderTest = new VirtualPropertyMakroResolver();
VirtualPropertyResolver resolverUnderTest = new VirtualPropertyResolver();
@Mock
TenantConfigurationManagement confMgmt;
@@ -55,14 +56,21 @@ public class VirtualPropertyMakroResolverTest {
.thenReturn(TEST_POLLING_TIME_INTERVAL);
when(confMgmt.getConfigurationValue(TenantConfigurationKey.POLLING_OVERDUE_TIME_INTERVAL, String.class))
.thenReturn(TEST_POLLING_OVERDUE_TIME_INTERVAL);
when(resolverUnderTest.getTenantConfigurationManagement()).thenReturn(confMgmt);
this.substitutor = new StrSubstitutor(resolverUnderTest, StrSubstitutor.DEFAULT_PREFIX,
when(resolverUnderTest.getTimestampCalculator()).thenReturn(new TimestampCalculator() {
@Override
protected TenantConfigurationManagement getTenantConfigurationManagement() {
return confMgmt;
}
});
this.substitutor = new StrSubstitutor(new RSQLUtility.StrLookupAdapter(resolverUnderTest),
StrSubstitutor.DEFAULT_PREFIX,
StrSubstitutor.DEFAULT_SUFFIX, StrSubstitutor.DEFAULT_ESCAPE);
}
@Test
@Description("Tests resolution of NOW_TS by using a StrSubstitutor configured with the VirtualPropertyMakroResolver.")
@Description("Tests resolution of NOW_TS by using a StrSubstitutor configured with the VirtualPropertyResolver.")
public void resolveNowTimestampPlaceholder() {
String placeholder = "${NOW_TS}";
String testString = "lhs=lt=" + placeholder;
@@ -72,7 +80,7 @@ public class VirtualPropertyMakroResolverTest {
}
@Test
@Description("Tests resolution of OVERDUE_TS by using a StrSubstitutor configured with the VirtualPropertyMakroResolver.")
@Description("Tests resolution of OVERDUE_TS by using a StrSubstitutor configured with the VirtualPropertyResolver.")
public void resolveOverdueTimestampPlaceholder() {
String placeholder = "${OVERDUE_TS}";
String testString = "lhs=lt=" + placeholder;
@@ -82,7 +90,7 @@ public class VirtualPropertyMakroResolverTest {
}
@Test
@Description("Tests case insensititity of VirtualPropertyMakroResolver.")
@Description("Tests case insensititity of VirtualPropertyResolver.")
public void resolveOverdueTimestampPlaceholderLowerCase() {
String placeholder = "${overdue_ts}";
String testString = "lhs=lt=" + placeholder;
@@ -92,7 +100,7 @@ public class VirtualPropertyMakroResolverTest {
}
@Test
@Description("Tests VirtualPropertyMakroResolver with a placeholder unknown to VirtualPropertyMakroResolver.")
@Description("Tests VirtualPropertyResolver with a placeholder unknown to VirtualPropertyResolver.")
public void handleUnknownPlaceholder() {
String placeholder = "${unknown}";
String testString = "lhs=lt=" + placeholder;