From c43780a1509072886a529b3e3a28d8771ec294a5 Mon Sep 17 00:00:00 2001 From: Tim Ward Date: Mon, 26 Feb 2024 16:37:17 +0000 Subject: [PATCH 1/4] Support placeholders in configuration values Sometimes it is not sufficient to use configuration values exactly as they exist in System Properties or other sources, or several properties need to be combined to create the configuration property. This commit adds support for Value Arguments to be included in the annotations. These arguments cause the Property value to be taken as a template string suitable for use with String.format, with the supplied arguments used to populate the template. Fixes #812 Signed-off-by: Tim Ward --- .../osgi/test/common/annotation/Property.java | 33 ++++++ .../properties/PropertiesConverter.java | 99 ++++++++--------- .../junit5/cm/test/ConfigAnnotationTest.java | 105 ++++++++++++++++-- .../osgi/test/common/annotation/Property.java | 33 ++++++ .../properties/PropertiesConverter.java | 99 ++++++++--------- 5 files changed, 255 insertions(+), 114 deletions(-) diff --git a/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java b/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java index 2576edd2..5a696e23 100644 --- a/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java +++ b/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java @@ -91,6 +91,21 @@ public enum ValueSource { TestUniqueId } + /** + * Used to provide an argument in the {@link Property#templateArguments()}. + * {@link ValueArgument} values are resolved in the same way as + * {@link Property} values but they are always of {@link Type#Scalar} + * + * @since 1.2 + */ + public @interface ValueArgument { + String[] value() default ""; + + Scalar scalar() default Scalar.String; + + ValueSource source() default ValueSource.Value; + } + String key(); String[] value() default ""; @@ -105,4 +120,22 @@ public enum ValueSource { */ ValueSource source() default ValueSource.Value; + /** + * If any template arguments are set then the resolved value of this + * {@link Property} annotation will be used as a template in + * {@link String#format} with the resolved {@link ValueArgument} values used + * as arguments. + *

+ * Note that any defaulting or processing according to the {@link #source()} + * will happen before the formatting is applied, so template + * arguments cannot be used, for example, to change the name of a system + * property. Conversely, the scalar conversion will happen after + * the template is applied, meaning that numeric values can be assembled + * from multiple template arguments. + * + * @return the arguments that should be used with the template + * @since 1.2 + */ + ValueArgument[] templateArguments() default {}; + } diff --git a/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java b/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java index 7aa1fcb8..7b03b90e 100644 --- a/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java +++ b/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java @@ -26,6 +26,7 @@ import org.osgi.test.common.annotation.Property; import org.osgi.test.common.annotation.Property.Scalar; import org.osgi.test.common.annotation.Property.Type; +import org.osgi.test.common.annotation.Property.ValueSource; public class PropertiesConverter { @@ -41,60 +42,19 @@ private static Object toValue(Description desc, Property entry) { boolean primitive = entry.type() .equals(Type.PrimitiveArray); - String[] value = getRawValue(desc, entry); + String[] value = getRawValue(desc, entry.value(), entry.source(), entry.type()); + + Object[] templateParams = Arrays.stream(entry.templateArguments()) + .map(ta -> getRawValue(desc, ta.value(), ta.source(), Type.Scalar)[0]) + .toArray(); Object result = createArray(entry.scalar(), primitive, value.length); int i = 0; for (String v : value) { - Object val = null; - - if (v != null) { - switch (entry.scalar()) { - case Boolean : - Boolean booleanValue = Boolean.valueOf(v); - val = primitive ? booleanValue.booleanValue() : booleanValue; - break; - - case Byte : - Byte byteVal = Byte.valueOf(v); - val = primitive ? byteVal.byteValue() : byteVal; - break; - - case Character : - char charVal = v.charAt(0); - val = primitive ? charVal : Character.valueOf(charVal); - break; - - case Double : - Double doubleVal = Double.valueOf(v); - val = primitive ? doubleVal.doubleValue() : doubleVal; - break; - - case Float : - Float floatVal = Float.valueOf(v); - val = primitive ? floatVal.floatValue() : floatVal; - break; - - case Integer : - Integer integerVal = Integer.valueOf(v); - val = primitive ? integerVal.intValue() : integerVal; - break; - - case Long : - Long longVal = Long.valueOf(v); - val = primitive ? longVal.longValue() : longVal; - break; - - case Short : - Short shortVal = Short.valueOf(v); - val = primitive ? shortVal.shortValue() : shortVal; - break; - - case String : - val = v; - break; - } + if (v != null && templateParams.length > 0) { + v = String.format(v, templateParams); } + Object val = convertScalar(entry.scalar(), v); if (Type.Scalar.equals(entry.type())) { result = val; @@ -119,10 +79,43 @@ private static Object toValue(Description desc, Property entry) { } - private static String[] getRawValue(Description desc, Property entry) { - String[] value = entry.value(); + private static Object convertScalar(Scalar scalar, String v) { + if (v != null) { + switch (scalar) { + case Boolean : + return Boolean.valueOf(v); + + case Byte : + return Byte.valueOf(v); + + case Character : + return v.charAt(0); + + case Double : + return Double.valueOf(v); + + case Float : + return Float.valueOf(v); + + case Integer : + return Integer.valueOf(v); + + case Long : + return Long.valueOf(v); + + case Short : + return Short.valueOf(v); + + case String : + return v; + } + } + return null; + } + + private static String[] getRawValue(Description desc, String[] value, ValueSource source, Type type) { String prop = null; - switch (entry.source()) { + switch (source) { case EnvironmentVariable : if (value.length == 0) { throw new RuntimeException("A property name must be supplied for source EnvironmentVariable"); @@ -178,7 +171,7 @@ private static String[] getRawValue(Description desc, Property entry) { throw new RuntimeException("conversion error - unknown source"); } - return entry.type() == Type.Scalar ? new String[] { + return type == Type.Scalar ? new String[] { prop } : prop.split("\\s*,\\s*"); } diff --git a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java index 177f4304..42922c88 100644 --- a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java +++ b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java @@ -40,6 +40,9 @@ import org.osgi.test.assertj.dictionary.DictionaryAssert; import org.osgi.test.common.annotation.InjectService; import org.osgi.test.common.annotation.Property; +import org.osgi.test.common.annotation.Property.Scalar; +import org.osgi.test.common.annotation.Property.Type; +import org.osgi.test.common.annotation.Property.ValueArgument; import org.osgi.test.common.annotation.config.InjectConfiguration; import org.osgi.test.common.annotation.config.WithConfiguration; import org.osgi.test.common.annotation.config.WithFactoryConfiguration; @@ -286,7 +289,13 @@ void unsetConfig() { @Test @WithConfiguration(pid = "foo", properties = { @Property(key = "testScalar", value = METHOD_NAME, source = SystemProperty), - @Property(key = "testArray", value = ARRAY_NAME, source = SystemProperty, type = PrimitiveArray, scalar = Byte) + @Property(key = "testArray", value = ARRAY_NAME, source = SystemProperty, type = PrimitiveArray, scalar = Byte), + @Property(key = "testTemplate", value = { + "Method : %s", "Easy As %2$s" + }, type = Type.Collection, templateArguments = { + @ValueArgument(value = METHOD_NAME, source = SystemProperty), + @ValueArgument(value = ARRAY_NAME, source = SystemProperty) + }) }) void testAnnotated(@InjectService ConfigurationAdmin ca) throws Exception { @@ -298,12 +307,21 @@ void testAnnotated(@InjectService 1, 2, 3 }, (byte[]) cs.getProperties() .get("testArray")); + assertThat(cs.getProperties() + .get("testTemplate")).asList() + .containsExactly("Method : testAnnotated", "Easy As 1,2,3"); } @Test void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "foo", properties = { @Property(key = "testScalar", value = METHOD_NAME, source = SystemProperty), - @Property(key = "testArray", value = ARRAY_NAME, source = SystemProperty, type = PrimitiveArray, scalar = Byte) + @Property(key = "testArray", value = ARRAY_NAME, source = SystemProperty, type = PrimitiveArray, scalar = Byte), + @Property(key = "testTemplate", value = { + "Method : %s", "Easy As %2$s" + }, type = Type.Collection, templateArguments = { + @ValueArgument(value = METHOD_NAME, source = SystemProperty), + @ValueArgument(value = ARRAY_NAME, source = SystemProperty) + }) })) Configuration cs) throws Exception { assertThat(cs).isNotNull(); @@ -313,18 +331,50 @@ void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "fo 1, 2, 3 }, (byte[]) cs.getProperties() .get("testArray")); + assertThat(cs.getProperties() + .get("testTemplate")).asList() + .containsExactly("Method : testInjected", "Easy As 1,2,3"); } @Test void testFallback(@InjectConfiguration(withConfig = @WithConfiguration(pid = "foo", properties = { @Property(key = "testFallback", value = { "missing", "default" - }, source = SystemProperty) + }, source = SystemProperty), @Property(key = "testTemplateFallback", value = { + "Method : %s", "Easy As %2$s" + }, type = Type.Collection, templateArguments = { + @ValueArgument(value = { + "missing", "default2" + }, source = SystemProperty), @ValueArgument(value = { + "missing", "default3" + }, source = SystemProperty) + }) })) Configuration cs) throws Exception { assertThat(cs).isNotNull(); assertThat(cs.getProperties() .get("testFallback")).isEqualTo("default"); + assertThat(cs.getProperties() + .get("testTemplateFallback")).asList() + .containsExactly("Method : default2", "Easy As default3"); + } + + @Test + @WithConfiguration(pid = "foo", properties = { + @Property(key = "testNumber", scalar = Scalar.Double, value = "%s.%s", templateArguments = { + @ValueArgument(value = { + "missing", "5" + }, source = SystemProperty), + @ValueArgument(value = "java.specification.version", source = SystemProperty) + }) + }) + void testNumeric(@InjectService + ConfigurationAdmin ca) throws Exception { + Configuration cs = ConfigUtil.getConfigsByServicePid(ca, "foo"); + assertThat(cs).isNotNull(); + assertThat(cs.getProperties() + .get("testNumber")) + .isEqualTo(Double.parseDouble("5." + System.getProperty("java.specification.version"))); } } @@ -333,7 +383,10 @@ class EnvironmentPropertyTests { @Test @WithConfiguration(pid = "foo", properties = { - @Property(key = "testScalar", value = "PATH", source = EnvironmentVariable) + @Property(key = "testScalar", value = "PATH", source = EnvironmentVariable), + @Property(key = "testTemplate", value = "Easy As %s", templateArguments = { + @ValueArgument(value = "PATH", source = EnvironmentVariable) + }) }) void testAnnotated(@InjectService ConfigurationAdmin ca) throws Exception { @@ -341,29 +394,47 @@ void testAnnotated(@InjectService assertThat(cs).isNotNull(); assertThat(cs.getProperties() .get("testScalar")).isEqualTo(System.getenv("PATH")); + assertThat(cs.getProperties() + .get("testTemplate")).isEqualTo("Easy As " + System.getenv("PATH")); } @Test void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "foo", properties = { - @Property(key = "testScalar", value = "PATH", source = EnvironmentVariable) + @Property(key = "testScalar", value = "PATH", source = EnvironmentVariable), + @Property(key = "testTemplate", value = "Easy As %s", templateArguments = { + @ValueArgument(value = "PATH", source = EnvironmentVariable) + }) })) Configuration cs) throws Exception { assertThat(cs).isNotNull(); assertThat(cs).isNotNull(); assertThat(cs.getProperties() .get("testScalar")).isEqualTo(System.getenv("PATH")); + assertThat(cs.getProperties() + .get("testTemplate")).isEqualTo("Easy As " + System.getenv("PATH")); } @Test void testFallback(@InjectConfiguration(withConfig = @WithConfiguration(pid = "foo", properties = { @Property(key = "testFallback", value = { "missing", "default" - }, source = EnvironmentVariable) + }, source = EnvironmentVariable), @Property(key = "testTemplateFallback", value = { + "Method : %s", "Easy As %2$s" + }, type = Type.Collection, templateArguments = { + @ValueArgument(value = { + "missing", "default2" + }, source = EnvironmentVariable), @ValueArgument(value = { + "missing", "default3" + }, source = EnvironmentVariable) + }) })) Configuration cs) throws Exception { assertThat(cs).isNotNull(); assertThat(cs.getProperties() .get("testFallback")).isEqualTo("default"); + assertThat(cs.getProperties() + .get("testTemplateFallback")).asList() + .containsExactly("Method : default2", "Easy As default3"); } } @@ -383,7 +454,12 @@ public class TestPropertyTests { @Test @WithConfiguration(pid = "foo", properties = { @Property(key = "testName", source = TestClass), @Property(key = "testMethod", source = TestMethod), - @Property(key = "testId", source = TestUniqueId) + @Property(key = "testId", source = TestUniqueId), @Property(key = "testTemplate", value = { + "Class : %s", "Test : %2$s", "Id : %3$s" + }, type = Type.Collection, templateArguments = { + @ValueArgument(source = TestClass), @ValueArgument(source = TestMethod), + @ValueArgument(source = TestUniqueId) + }) }) void testAnnotated(@InjectService ConfigurationAdmin ca) throws Exception { @@ -395,12 +471,21 @@ void testAnnotated(@InjectService .get("testMethod")).isEqualTo("testAnnotated"); assertThat(cs.getProperties() .get("testId")).isEqualTo(uniqueId); + assertThat(cs.getProperties() + .get("testTemplate")).asList() + .containsExactly("Class : " + TestPropertyTests.class.getName(), "Test : testAnnotated", + "Id : " + uniqueId); } @Test void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "foo", properties = { @Property(key = "testName", source = TestClass), @Property(key = "testMethod", source = TestMethod), - @Property(key = "testId", source = TestUniqueId) + @Property(key = "testId", source = TestUniqueId), @Property(key = "testTemplate", value = { + "Class : %s", "Test : %2$s", "Id : %3$s" + }, type = Type.Collection, templateArguments = { + @ValueArgument(source = TestClass), @ValueArgument(source = TestMethod), + @ValueArgument(source = TestUniqueId) + }) })) Configuration cs) throws Exception { assertThat(cs).isNotNull(); @@ -410,6 +495,10 @@ void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "fo .get("testMethod")).isEqualTo("testInjected"); assertThat(cs.getProperties() .get("testId")).isEqualTo(uniqueId); + assertThat(cs.getProperties() + .get("testTemplate")).asList() + .containsExactly("Class : " + TestPropertyTests.class.getName(), "Test : testInjected", + "Id : " + uniqueId); } } } diff --git a/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java b/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java index 2576edd2..5a696e23 100644 --- a/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java +++ b/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java @@ -91,6 +91,21 @@ public enum ValueSource { TestUniqueId } + /** + * Used to provide an argument in the {@link Property#templateArguments()}. + * {@link ValueArgument} values are resolved in the same way as + * {@link Property} values but they are always of {@link Type#Scalar} + * + * @since 1.2 + */ + public @interface ValueArgument { + String[] value() default ""; + + Scalar scalar() default Scalar.String; + + ValueSource source() default ValueSource.Value; + } + String key(); String[] value() default ""; @@ -105,4 +120,22 @@ public enum ValueSource { */ ValueSource source() default ValueSource.Value; + /** + * If any template arguments are set then the resolved value of this + * {@link Property} annotation will be used as a template in + * {@link String#format} with the resolved {@link ValueArgument} values used + * as arguments. + *

+ * Note that any defaulting or processing according to the {@link #source()} + * will happen before the formatting is applied, so template + * arguments cannot be used, for example, to change the name of a system + * property. Conversely, the scalar conversion will happen after + * the template is applied, meaning that numeric values can be assembled + * from multiple template arguments. + * + * @return the arguments that should be used with the template + * @since 1.2 + */ + ValueArgument[] templateArguments() default {}; + } diff --git a/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java b/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java index a17ac6bf..1808f2ec 100644 --- a/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java +++ b/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java @@ -26,6 +26,7 @@ import org.osgi.test.common.annotation.Property; import org.osgi.test.common.annotation.Property.Scalar; import org.osgi.test.common.annotation.Property.Type; +import org.osgi.test.common.annotation.Property.ValueSource; public class PropertiesConverter { @@ -41,60 +42,19 @@ private static Object toValue(ExtensionContext context, Property entry) { boolean primitive = entry.type() .equals(Type.PrimitiveArray); - String[] value = getRawValue(context, entry); + String[] value = getRawValue(context, entry.value(), entry.source(), entry.type()); + + Object[] templateParams = Arrays.stream(entry.templateArguments()) + .map(ta -> getRawValue(context, ta.value(), ta.source(), Type.Scalar)[0]) + .toArray(); Object result = createArray(entry.scalar(), primitive, value.length); int i = 0; for (String v : value) { - Object val = null; - - if (v != null) { - switch (entry.scalar()) { - case Boolean : - Boolean booleanValue = Boolean.valueOf(v); - val = primitive ? booleanValue.booleanValue() : booleanValue; - break; - - case Byte : - Byte byteVal = Byte.valueOf(v); - val = primitive ? byteVal.byteValue() : byteVal; - break; - - case Character : - char charVal = v.charAt(0); - val = primitive ? charVal : Character.valueOf(charVal); - break; - - case Double : - Double doubleVal = Double.valueOf(v); - val = primitive ? doubleVal.doubleValue() : doubleVal; - break; - - case Float : - Float floatVal = Float.valueOf(v); - val = primitive ? floatVal.floatValue() : floatVal; - break; - - case Integer : - Integer integerVal = Integer.valueOf(v); - val = primitive ? integerVal.intValue() : integerVal; - break; - - case Long : - Long longVal = Long.valueOf(v); - val = primitive ? longVal.longValue() : longVal; - break; - - case Short : - Short shortVal = Short.valueOf(v); - val = primitive ? shortVal.shortValue() : shortVal; - break; - - case String : - val = v; - break; - } + if (v != null && templateParams.length > 0) { + v = String.format(v, templateParams); } + Object val = convertScalar(entry.scalar(), v); if (Type.Scalar.equals(entry.type())) { result = val; @@ -119,10 +79,43 @@ private static Object toValue(ExtensionContext context, Property entry) { } - private static String[] getRawValue(ExtensionContext context, Property entry) { - String[] value = entry.value(); + private static Object convertScalar(Scalar scalar, String v) { + if (v != null) { + switch (scalar) { + case Boolean : + return Boolean.valueOf(v); + + case Byte : + return Byte.valueOf(v); + + case Character : + return v.charAt(0); + + case Double : + return Double.valueOf(v); + + case Float : + return Float.valueOf(v); + + case Integer : + return Integer.valueOf(v); + + case Long : + return Long.valueOf(v); + + case Short : + return Short.valueOf(v); + + case String : + return v; + } + } + return null; + } + + private static String[] getRawValue(ExtensionContext context, String[] value, ValueSource source, Type type) { String prop = null; - switch (entry.source()) { + switch (source) { case EnvironmentVariable : if (value.length == 0) { throw new RuntimeException("A property name must be supplied for source EnvironmentVariable"); @@ -180,7 +173,7 @@ private static String[] getRawValue(ExtensionContext context, Property entry) { throw new RuntimeException("conversion error - unknown source"); } - return entry.type() == Type.Scalar ? new String[] { + return type == Type.Scalar ? new String[] { prop } : prop.split("\\s*,\\s*"); } From 715b02fcb3efcca0d40de62f136c1e44684eb2e0 Mon Sep 17 00:00:00 2001 From: Tim Ward Date: Mon, 26 Feb 2024 17:39:29 +0000 Subject: [PATCH 2/4] Review comments * Rename to TemplateArgument * Ensure that the tests actually test the scalar member of the TemplateArgument Signed-off-by: Tim Ward --- .../osgi/test/common/annotation/Property.java | 8 ++-- .../properties/PropertiesConverter.java | 2 +- .../junit5/cm/test/ConfigAnnotationTest.java | 45 ++++++++++--------- .../osgi/test/common/annotation/Property.java | 8 ++-- .../properties/PropertiesConverter.java | 2 +- 5 files changed, 35 insertions(+), 30 deletions(-) diff --git a/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java b/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java index 5a696e23..632c2e33 100644 --- a/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java +++ b/org.osgi.test.junit4/src/main/java/org/osgi/test/common/annotation/Property.java @@ -93,12 +93,12 @@ public enum ValueSource { /** * Used to provide an argument in the {@link Property#templateArguments()}. - * {@link ValueArgument} values are resolved in the same way as + * {@link TemplateArgument} values are resolved in the same way as * {@link Property} values but they are always of {@link Type#Scalar} * * @since 1.2 */ - public @interface ValueArgument { + public @interface TemplateArgument { String[] value() default ""; Scalar scalar() default Scalar.String; @@ -123,7 +123,7 @@ public enum ValueSource { /** * If any template arguments are set then the resolved value of this * {@link Property} annotation will be used as a template in - * {@link String#format} with the resolved {@link ValueArgument} values used + * {@link String#format} with the resolved {@link TemplateArgument} values used * as arguments. *

* Note that any defaulting or processing according to the {@link #source()} @@ -136,6 +136,6 @@ public enum ValueSource { * @return the arguments that should be used with the template * @since 1.2 */ - ValueArgument[] templateArguments() default {}; + TemplateArgument[] templateArguments() default {}; } diff --git a/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java b/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java index 7b03b90e..a9de8e58 100644 --- a/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java +++ b/org.osgi.test.junit4/src/main/java/org/osgi/test/junit4/properties/PropertiesConverter.java @@ -45,7 +45,7 @@ private static Object toValue(Description desc, Property entry) { String[] value = getRawValue(desc, entry.value(), entry.source(), entry.type()); Object[] templateParams = Arrays.stream(entry.templateArguments()) - .map(ta -> getRawValue(desc, ta.value(), ta.source(), Type.Scalar)[0]) + .map(ta -> convertScalar(ta.scalar(), getRawValue(desc, ta.value(), ta.source(), Type.Scalar)[0])) .toArray(); Object result = createArray(entry.scalar(), primitive, value.length); diff --git a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java index 42922c88..2ea9a08a 100644 --- a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java +++ b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java @@ -41,8 +41,8 @@ import org.osgi.test.common.annotation.InjectService; import org.osgi.test.common.annotation.Property; import org.osgi.test.common.annotation.Property.Scalar; +import org.osgi.test.common.annotation.Property.TemplateArgument; import org.osgi.test.common.annotation.Property.Type; -import org.osgi.test.common.annotation.Property.ValueArgument; import org.osgi.test.common.annotation.config.InjectConfiguration; import org.osgi.test.common.annotation.config.WithConfiguration; import org.osgi.test.common.annotation.config.WithFactoryConfiguration; @@ -293,8 +293,8 @@ void unsetConfig() { @Property(key = "testTemplate", value = { "Method : %s", "Easy As %2$s" }, type = Type.Collection, templateArguments = { - @ValueArgument(value = METHOD_NAME, source = SystemProperty), - @ValueArgument(value = ARRAY_NAME, source = SystemProperty) + @TemplateArgument(value = METHOD_NAME, source = SystemProperty), + @TemplateArgument(value = ARRAY_NAME, source = SystemProperty) }) }) void testAnnotated(@InjectService @@ -319,8 +319,8 @@ void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "fo @Property(key = "testTemplate", value = { "Method : %s", "Easy As %2$s" }, type = Type.Collection, templateArguments = { - @ValueArgument(value = METHOD_NAME, source = SystemProperty), - @ValueArgument(value = ARRAY_NAME, source = SystemProperty) + @TemplateArgument(value = METHOD_NAME, source = SystemProperty), + @TemplateArgument(value = ARRAY_NAME, source = SystemProperty) }) })) Configuration cs) throws Exception { @@ -343,9 +343,9 @@ void testFallback(@InjectConfiguration(withConfig = @WithConfiguration(pid = "fo }, source = SystemProperty), @Property(key = "testTemplateFallback", value = { "Method : %s", "Easy As %2$s" }, type = Type.Collection, templateArguments = { - @ValueArgument(value = { + @TemplateArgument(value = { "missing", "default2" - }, source = SystemProperty), @ValueArgument(value = { + }, source = SystemProperty), @TemplateArgument(value = { "missing", "default3" }, source = SystemProperty) }) @@ -361,20 +361,25 @@ void testFallback(@InjectConfiguration(withConfig = @WithConfiguration(pid = "fo @Test @WithConfiguration(pid = "foo", properties = { - @Property(key = "testNumber", scalar = Scalar.Double, value = "%s.%s", templateArguments = { - @ValueArgument(value = { + @Property(key = "testNumber", scalar = Scalar.Double, value = "%d.%.0f", templateArguments = { + @TemplateArgument(value = { "missing", "5" - }, source = SystemProperty), - @ValueArgument(value = "java.specification.version", source = SystemProperty) + }, source = SystemProperty, scalar = Scalar.Integer), + @TemplateArgument(value = "java.specification.version", source = SystemProperty, scalar = Scalar.Double) }) }) void testNumeric(@InjectService ConfigurationAdmin ca) throws Exception { Configuration cs = ConfigUtil.getConfigsByServicePid(ca, "foo"); assertThat(cs).isNotNull(); + String javaVersion = System.getProperty("java.specification.version"); + int idx = javaVersion.indexOf('.'); + if (idx >= 0) { + javaVersion = javaVersion.substring(0, idx); + } assertThat(cs.getProperties() .get("testNumber")) - .isEqualTo(Double.parseDouble("5." + System.getProperty("java.specification.version"))); + .isEqualTo(Double.parseDouble("5." + javaVersion)); } } @@ -385,7 +390,7 @@ class EnvironmentPropertyTests { @WithConfiguration(pid = "foo", properties = { @Property(key = "testScalar", value = "PATH", source = EnvironmentVariable), @Property(key = "testTemplate", value = "Easy As %s", templateArguments = { - @ValueArgument(value = "PATH", source = EnvironmentVariable) + @TemplateArgument(value = "PATH", source = EnvironmentVariable) }) }) void testAnnotated(@InjectService @@ -402,7 +407,7 @@ void testAnnotated(@InjectService void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "foo", properties = { @Property(key = "testScalar", value = "PATH", source = EnvironmentVariable), @Property(key = "testTemplate", value = "Easy As %s", templateArguments = { - @ValueArgument(value = "PATH", source = EnvironmentVariable) + @TemplateArgument(value = "PATH", source = EnvironmentVariable) }) })) Configuration cs) throws Exception { @@ -421,9 +426,9 @@ void testFallback(@InjectConfiguration(withConfig = @WithConfiguration(pid = "fo }, source = EnvironmentVariable), @Property(key = "testTemplateFallback", value = { "Method : %s", "Easy As %2$s" }, type = Type.Collection, templateArguments = { - @ValueArgument(value = { + @TemplateArgument(value = { "missing", "default2" - }, source = EnvironmentVariable), @ValueArgument(value = { + }, source = EnvironmentVariable), @TemplateArgument(value = { "missing", "default3" }, source = EnvironmentVariable) }) @@ -457,8 +462,8 @@ public class TestPropertyTests { @Property(key = "testId", source = TestUniqueId), @Property(key = "testTemplate", value = { "Class : %s", "Test : %2$s", "Id : %3$s" }, type = Type.Collection, templateArguments = { - @ValueArgument(source = TestClass), @ValueArgument(source = TestMethod), - @ValueArgument(source = TestUniqueId) + @TemplateArgument(source = TestClass), @TemplateArgument(source = TestMethod), + @TemplateArgument(source = TestUniqueId) }) }) void testAnnotated(@InjectService @@ -483,8 +488,8 @@ void testInjected(@InjectConfiguration(withConfig = @WithConfiguration(pid = "fo @Property(key = "testId", source = TestUniqueId), @Property(key = "testTemplate", value = { "Class : %s", "Test : %2$s", "Id : %3$s" }, type = Type.Collection, templateArguments = { - @ValueArgument(source = TestClass), @ValueArgument(source = TestMethod), - @ValueArgument(source = TestUniqueId) + @TemplateArgument(source = TestClass), @TemplateArgument(source = TestMethod), + @TemplateArgument(source = TestUniqueId) }) })) Configuration cs) throws Exception { diff --git a/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java b/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java index 5a696e23..632c2e33 100644 --- a/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java +++ b/org.osgi.test.junit5/src/main/java/org/osgi/test/common/annotation/Property.java @@ -93,12 +93,12 @@ public enum ValueSource { /** * Used to provide an argument in the {@link Property#templateArguments()}. - * {@link ValueArgument} values are resolved in the same way as + * {@link TemplateArgument} values are resolved in the same way as * {@link Property} values but they are always of {@link Type#Scalar} * * @since 1.2 */ - public @interface ValueArgument { + public @interface TemplateArgument { String[] value() default ""; Scalar scalar() default Scalar.String; @@ -123,7 +123,7 @@ public enum ValueSource { /** * If any template arguments are set then the resolved value of this * {@link Property} annotation will be used as a template in - * {@link String#format} with the resolved {@link ValueArgument} values used + * {@link String#format} with the resolved {@link TemplateArgument} values used * as arguments. *

* Note that any defaulting or processing according to the {@link #source()} @@ -136,6 +136,6 @@ public enum ValueSource { * @return the arguments that should be used with the template * @since 1.2 */ - ValueArgument[] templateArguments() default {}; + TemplateArgument[] templateArguments() default {}; } diff --git a/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java b/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java index 1808f2ec..18ffff22 100644 --- a/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java +++ b/org.osgi.test.junit5/src/main/java/org/osgi/test/junit5/properties/PropertiesConverter.java @@ -45,7 +45,7 @@ private static Object toValue(ExtensionContext context, Property entry) { String[] value = getRawValue(context, entry.value(), entry.source(), entry.type()); Object[] templateParams = Arrays.stream(entry.templateArguments()) - .map(ta -> getRawValue(context, ta.value(), ta.source(), Type.Scalar)[0]) + .map(ta -> convertScalar(ta.scalar(), getRawValue(context, ta.value(), ta.source(), Type.Scalar)[0])) .toArray(); Object result = createArray(entry.scalar(), primitive, value.length); From b1f7115f6151c24bcd4c3659a7f50dc24e62a9ce Mon Sep 17 00:00:00 2001 From: Tim Ward Date: Tue, 27 Feb 2024 09:43:36 +0000 Subject: [PATCH 3/4] Ensure proper handling of inconsistent Java version syntax Java 8 uses a specification version of 1.8, but later versions omit the '1.' - this caused the test assertion to fail on Java 8, even though the code was working Signed-off-by: Tim Ward --- .../osgi/test/junit5/cm/test/ConfigAnnotationTest.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java index 2ea9a08a..6b08cdc1 100644 --- a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java +++ b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java @@ -372,14 +372,10 @@ void testNumeric(@InjectService ConfigurationAdmin ca) throws Exception { Configuration cs = ConfigUtil.getConfigsByServicePid(ca, "foo"); assertThat(cs).isNotNull(); - String javaVersion = System.getProperty("java.specification.version"); - int idx = javaVersion.indexOf('.'); - if (idx >= 0) { - javaVersion = javaVersion.substring(0, idx); - } + double javaVersion = Double.parseDouble(System.getProperty("java.specification.version")); assertThat(cs.getProperties() .get("testNumber")) - .isEqualTo(Double.parseDouble("5." + javaVersion)); + .isEqualTo(Double.parseDouble("5." + Math.round(javaVersion))); } } From 1231ae5983babc65b36b70eb9e2c8f589a5ab20c Mon Sep 17 00:00:00 2001 From: Tim Ward Date: Tue, 27 Feb 2024 10:30:47 +0000 Subject: [PATCH 4/4] Allow factory configurations to use a random name It is possible to set the name for a factory configuration, however this is not required. Users should be permitted to omit the name member of the WithFactoryConfiguration annotation and have an auto-generated name used instead. This simplifies test setup where the configuration does not need to be injected or overridden elsewhere. Signed-off-by: Tim Ward --- .../config/WithFactoryConfiguration.java | 2 +- .../annotation/config/package-info.java | 2 +- .../junit5/cm/ConfigurationExtension.java | 31 +++++++++++++------ .../junit5/cm/test/ConfigAnnotationTest.java | 25 +++++++++++++++ 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/WithFactoryConfiguration.java b/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/WithFactoryConfiguration.java index e6d6e855..e8cc3eeb 100644 --- a/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/WithFactoryConfiguration.java +++ b/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/WithFactoryConfiguration.java @@ -59,7 +59,7 @@ * * @return The name */ - String name(); + String name() default Property.NOT_SET; /** * The location of the Configuration.
diff --git a/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/package-info.java b/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/package-info.java index bc8c26ff..14316a9d 100644 --- a/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/package-info.java +++ b/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/common/annotation/config/package-info.java @@ -17,6 +17,6 @@ *******************************************************************************/ @org.osgi.annotation.bundle.Export -@org.osgi.annotation.versioning.Version("1.1.0") +@org.osgi.annotation.versioning.Version("1.1.1") package org.osgi.test.common.annotation.config; diff --git a/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/junit5/cm/ConfigurationExtension.java b/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/junit5/cm/ConfigurationExtension.java index df1c9242..25c39220 100644 --- a/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/junit5/cm/ConfigurationExtension.java +++ b/org.osgi.test.junit5.cm/src/main/java/org/osgi/test/junit5/cm/ConfigurationExtension.java @@ -185,25 +185,36 @@ private ConfigurationHolder handleWithFactoryConfiguration(ExtensionContext cont WithFactoryConfiguration configAnnotation, ConfigurationAdmin configurationAdmin) { try { - Configuration configBefore = ConfigUtil.getConfigsByServicePid(configurationAdmin, - configAnnotation.factoryPid() + "~" + configAnnotation.name(), 0l); + Configuration configBefore; + Configuration configuration; - Optional copyOfBefore = createConfigurationCopy(configBefore); + if (Property.NOT_SET.equals(configAnnotation.name())) { + configBefore = null; - Configuration configuration; - if (Property.NOT_SET.equals(configAnnotation.location())) { - configuration = configurationAdmin.getFactoryConfiguration(configAnnotation.factoryPid(), - configAnnotation.name(), null); + if (Property.NOT_SET.equals(configAnnotation.location())) { + configuration = configurationAdmin.createFactoryConfiguration(configAnnotation.factoryPid(), null); + } else { + configuration = configurationAdmin.createFactoryConfiguration(configAnnotation.factoryPid(), + configAnnotation.location()); + } } else { - configuration = configurationAdmin.getFactoryConfiguration(configAnnotation.factoryPid(), - configAnnotation.name(), configAnnotation.location()); + configBefore = ConfigUtil.getConfigsByServicePid(configurationAdmin, + configAnnotation.factoryPid() + "~" + configAnnotation.name(), 0l); + + if (Property.NOT_SET.equals(configAnnotation.location())) { + configuration = configurationAdmin.getFactoryConfiguration(configAnnotation.factoryPid(), + configAnnotation.name(), null); + } else { + configuration = configurationAdmin.getFactoryConfiguration(configAnnotation.factoryPid(), + configAnnotation.name(), configAnnotation.location()); + } } updateConfigurationRespectNew(context, configuration, PropertiesConverter.of(context, configAnnotation.properties()), configBefore == null); - return new ConfigurationHolder(ConfigurationCopy.of(configuration), copyOfBefore); + return new ConfigurationHolder(ConfigurationCopy.of(configuration), createConfigurationCopy(configBefore)); } catch (Exception e) { throw new ParameterResolutionException( String.format("Unable to obtain Configuration for %s.", configAnnotation.factoryPid()), e); diff --git a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java index 6b08cdc1..d4021d98 100644 --- a/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java +++ b/org.osgi.test.junit5.cm/src/test/java/org/osgi/test/junit5/cm/test/ConfigAnnotationTest.java @@ -169,6 +169,31 @@ public void testMethodConfigurationFactoryCreate() throws Exception { } + @Test + @WithFactoryConfiguration(factoryPid = FACTORY_CONFIGURATION_PID, properties = { + @Property(key = "foo", value = "bar") + }) + @WithFactoryConfiguration(factoryPid = FACTORY_CONFIGURATION_PID, properties = { + @Property(key = "fizz", value = "buzz") + }) + public void testMethodConfigurationFactoryUnboundNames() throws Exception { + + Configuration[] cfgs = ca.listConfigurations("(foo=bar)"); + assertThat(cfgs).isNotNull() + .hasSize(1); + assertThat(cfgs[0].getFactoryPid()).isEqualTo(FACTORY_CONFIGURATION_PID); + DictionaryAssert.assertThat(cfgs[0].getProperties()) + .containsEntry("foo", "bar"); + + cfgs = ca.listConfigurations("(fizz=buzz)"); + assertThat(cfgs[0].getFactoryPid()).isEqualTo(FACTORY_CONFIGURATION_PID); + assertThat(cfgs).isNotNull() + .hasSize(1); + DictionaryAssert.assertThat(cfgs[0].getProperties()) + .containsEntry("fizz", "buzz"); + + } + @Nested class LocationTests {