Fix/declarative config no exceptions (#8079)

Co-authored-by: Jack Berg <34418638+jack-berg@users.noreply.github.com>

Author: Bhagirath00

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/DeclarativeConfigProperties.java

@@ -46,8 +46,8 @@ static Map<String, Object> toMap(DeclarativeConfigProperties declarativeConfigPr
   /**
    * Returns a {@link String} configuration property.
    *
-   * @return null if the property has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar string
+   * @return null if the property has not been configured or if the property is not a valid scalar
+   *     string
    */
   @Nullable
   String getString(String name);
@@ -56,8 +56,7 @@ static Map<String, Object> toMap(DeclarativeConfigProperties declarativeConfigPr
    * Returns a {@link String} configuration property.
    *
    * @return a {@link String} configuration property or {@code defaultValue} if a property with
-   *     {@code name} has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar string
+   *     {@code name} has not been configured or is not a valid scalar string
    */
   default String getString(String name, String defaultValue) {
     return defaultIfNull(getString(name), defaultValue);
@@ -67,8 +66,8 @@ default String getString(String name, String defaultValue) {
    * Returns a {@link Boolean} configuration property. Implementations should use the same rules as
    * {@link Boolean#parseBoolean(String)} for handling the values.
    *
-   * @return null if the property has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar boolean
+   * @return null if the property has not been configured or if the property is not a valid scalar
+   *     boolean
    */
   @Nullable
   Boolean getBoolean(String name);
@@ -77,8 +76,7 @@ default String getString(String name, String defaultValue) {
    * Returns a {@link Boolean} configuration property.
    *
    * @return a {@link Boolean} configuration property or {@code defaultValue} if a property with
-   *     {@code name} has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar boolean
+   *     {@code name} has not been configured or is not a valid scalar boolean
    */
   default boolean getBoolean(String name, boolean defaultValue) {
     return defaultIfNull(getBoolean(name), defaultValue);
@@ -90,8 +88,8 @@ default boolean getBoolean(String name, boolean defaultValue) {
    * <p>If the underlying config property is {@link Long}, it is converted to {@link Integer} with
    * {@link Long#intValue()} which may result in loss of precision.
    *
-   * @return null if the property has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar integer
+   * @return null if the property has not been configured or if the property is not a valid scalar
+   *     integer
    */
   @Nullable
   Integer getInt(String name);
@@ -103,8 +101,7 @@ default boolean getBoolean(String name, boolean defaultValue) {
    * {@link Long#intValue()} which may result in loss of precision.
    *
    * @return a {@link Integer} configuration property or {@code defaultValue} if a property with
-   *     {@code name} has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar integer
+   *     {@code name} has not been configured or is not a valid scalar integer
    */
   default int getInt(String name, int defaultValue) {
     return defaultIfNull(getInt(name), defaultValue);
@@ -113,8 +110,8 @@ default int getInt(String name, int defaultValue) {
   /**
    * Returns a {@link Long} configuration property.
    *
-   * @return null if the property has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar long
+   * @return null if the property has not been configured or if the property is not a valid scalar
+   *     long
    */
   @Nullable
   Long getLong(String name);
@@ -123,8 +120,7 @@ default int getInt(String name, int defaultValue) {
    * Returns a {@link Long} configuration property.
    *
    * @return a {@link Long} configuration property or {@code defaultValue} if a property with {@code
-   *     name} has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar long
+   *     name} has not been configured or is not a valid scalar long
    */
   default long getLong(String name, long defaultValue) {
     return defaultIfNull(getLong(name), defaultValue);
@@ -133,8 +129,8 @@ default long getLong(String name, long defaultValue) {
   /**
    * Returns a {@link Double} configuration property.
    *
-   * @return null if the property has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar double
+   * @return null if the property has not been configured or if the property is not a valid scalar
+   *     double
    */
   @Nullable
   Double getDouble(String name);
@@ -143,8 +139,7 @@ default long getLong(String name, long defaultValue) {
    * Returns a {@link Double} configuration property.
    *
    * @return a {@link Double} configuration property or {@code defaultValue} if a property with
-   *     {@code name} has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid scalar double
+   *     {@code name} has not been configured or is not a valid scalar double
    */
   default double getDouble(String name, double defaultValue) {
     return defaultIfNull(getDouble(name), defaultValue);
@@ -158,8 +153,8 @@ default double getDouble(String name, double defaultValue) {
    * @param scalarType the scalar type, one of {@link String}, {@link Boolean}, {@link Long} or
    *     {@link Double}
    * @return a {@link List} configuration property, or null if the property has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid sequence of scalars, or if
-   *     {@code scalarType} is not supported
+   *     or if the property is not a valid sequence of scalars
+   * @throws DeclarativeConfigException if {@code scalarType} is not supported
    */
   @Nullable
   <T> List<T> getScalarList(String name, Class<T> scalarType);
@@ -172,8 +167,7 @@ default double getDouble(String name, double defaultValue) {
    * @param scalarType the scalar type, one of {@link String}, {@link Boolean}, {@link Long} or
    *     {@link Double}
    * @return a {@link List} configuration property or {@code defaultValue} if a property with {@code
-   *     name} has not been configured
-   * @throws DeclarativeConfigException if the property is not a valid sequence of scalars
+   *     name} has not been configured or is not a valid sequence of scalars
    */
   default <T> List<T> getScalarList(String name, Class<T> scalarType, List<T> defaultValue) {
     return defaultIfNull(getScalarList(name, scalarType), defaultValue);
@@ -183,8 +177,7 @@ default <T> List<T> getScalarList(String name, Class<T> scalarType, List<T> defa
    * Returns a {@link DeclarativeConfigProperties} configuration property.
    *
    * @return a map-valued configuration property, or {@code null} if {@code name} has not been
-   *     configured
-   * @throws DeclarativeConfigException if the property is not a mapping
+   *     configured or is not a mapping
    */
   @Nullable
   DeclarativeConfigProperties getStructured(String name);
@@ -193,8 +186,7 @@ default <T> List<T> getScalarList(String name, Class<T> scalarType, List<T> defa
    * Returns a list of {@link DeclarativeConfigProperties} configuration property.
    *
    * @return a map-valued configuration property, or {@code defaultValue} if {@code name} has not
-   *     been configured
-   * @throws DeclarativeConfigException if the property is not a mapping
+   *     been configured or is not a mapping
    */
   default DeclarativeConfigProperties getStructured(
       String name, DeclarativeConfigProperties defaultValue) {
@@ -210,8 +202,7 @@ default DeclarativeConfigProperties getStructured(
    * but empty vs. not set, use {@link #getStructured(String)}.
    *
    * @return a map-valued configuration property, or an empty {@link DeclarativeConfigProperties}
-   *     instance if {@code name} has not been configured
-   * @throws DeclarativeConfigException if the property is not a mapping
+   *     instance if {@code name} has not been configured or is not a mapping
    */
   default DeclarativeConfigProperties get(String name) {
     return getStructured(name, empty());
@@ -221,8 +212,7 @@ default DeclarativeConfigProperties get(String name) {
    * Returns a list of {@link DeclarativeConfigProperties} configuration property.
    *
    * @return a list of map-valued configuration property, or {@code null} if {@code name} has not
-   *     been configured
-   * @throws DeclarativeConfigException if the property is not a sequence of mappings
+   *     been configured or is not a sequence of mappings
    */
   @Nullable
   List<DeclarativeConfigProperties> getStructuredList(String name);
@@ -231,8 +221,7 @@ default DeclarativeConfigProperties get(String name) {
    * Returns a list of {@link DeclarativeConfigProperties} configuration property.
    *
    * @return a list of map-valued configuration property, or {@code defaultValue} if {@code name}
-   *     has not been configured
-   * @throws DeclarativeConfigException if the property is not a sequence of mappings
+   *     has not been configured or is not a sequence of mappings
    */
   default List<DeclarativeConfigProperties> getStructuredList(
       String name, List<DeclarativeConfigProperties> defaultValue) {

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigProperties.java

@@ -156,18 +156,21 @@ private static boolean isMap(Object object) {
   @Nullable
   @Override
   public String getString(String name) {
+    Objects.requireNonNull(name, "Null configuration property name");
     return stringOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
   @Override
   public Boolean getBoolean(String name) {
+    Objects.requireNonNull(name, "Null configuration property name");
     return booleanOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
   @Override
   public Integer getInt(String name) {
+    Objects.requireNonNull(name, "Null configuration property name");
     Object value = simpleEntries.get(name);
     if (value instanceof Integer) {
       return (Integer) value;
@@ -184,18 +187,21 @@ public Integer getInt(String name) {
   @Nullable
   @Override
   public Long getLong(String name) {
+    Objects.requireNonNull(name, "Null configuration property name");
     return longOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
   @Override
   public Double getDouble(String name) {
+    Objects.requireNonNull(name, "Null configuration property name");
     return doubleOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
   @Override
   public <T> List<T> getScalarList(String name, Class<T> scalarType) {
+    Objects.requireNonNull(name, "Null configuration property name");
     if (!SUPPORTED_SCALAR_TYPES.contains(scalarType)) {
       throw new DeclarativeConfigException(
           "Unsupported scalar type "
@@ -290,12 +296,14 @@ private static Double doubleOrNull(@Nullable Object value, String name) {
   @Nullable
   @Override
   public DeclarativeConfigProperties getStructured(String name) {
+    Objects.requireNonNull(name, "Null configuration property name");
     return mapEntries.get(name);
   }
 
   @Nullable
   @Override
   public List<DeclarativeConfigProperties> getStructuredList(String name) {
+    Objects.requireNonNull(name, "Null configuration property name");
     List<YamlDeclarativeConfigProperties> value = listEntries.get(name);
     if (value != null) {
       return Collections.unmodifiableList(value);
@@ -314,7 +322,8 @@ public Set<String> getPropertyKeys() {
 
   @Override
   public String toString() {
-    StringJoiner joiner = new StringJoiner(", ", "YamlDeclarativeConfigProperties{", "}");
+    StringJoiner joiner =
+        new StringJoiner(", ", YamlDeclarativeConfigProperties.class.getSimpleName() + "{", "}");
     simpleEntries.forEach((key, value) -> joiner.add(key + "=" + value));
     listEntries.forEach((key, value) -> joiner.add(key + "=" + value));
     mapEntries.forEach((key, value) -> joiner.add(key + "=" + value));

sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigPropertiesTest.java

@@ -130,7 +130,8 @@ void additionalProperties() {
         .isEqualTo(Arrays.asList(1.1d, 2.2d));
     assertThat(otherProps.getScalarList("bool_list_key", Boolean.class))
         .isEqualTo(Arrays.asList(true, false));
-    // If reading a scalar list which is mixed, entries which are not aligned with the requested
+    // If reading a scalar list which is mixed, entries which are not aligned with
+    // the requested
     // type are filtered out
     assertThat(otherProps.getScalarList("mixed_list_key", String.class))
         .isEqualTo(Collections.singletonList("val1"));
@@ -230,8 +231,13 @@ void wrongType() {
     assertThat(otherProps.getDouble("str_key")).isNull();
     assertThat(otherProps.getBoolean("str_key")).isNull();
     assertThat(otherProps.getScalarList("str_key", String.class)).isNull();
+    assertThat(otherProps.getScalarList("str_list_key", Long.class)).isNull();
+    assertThat(otherProps.getScalarList("str_list_key", Boolean.class)).isNull();
+    assertThat(otherProps.getScalarList("str_list_key", Double.class)).isNull();
     assertThat(otherProps.getStructured("str_key")).isNull();
     assertThat(otherProps.getStructuredList("str_key")).isNull();
+    assertThat(otherProps.getStructured("str_list_key")).isNull();
+    assertThat(otherProps.getStructuredList("map_key")).isNull();
 
     assertWarning("Ignoring value for key [int_key] because it is Integer instead of String: 1");
     assertWarning(
@@ -240,6 +246,26 @@ void wrongType() {
         "Ignoring value for key [str_key] because it is String instead of Double: str_value");
     assertWarning(
         "Ignoring value for key [str_key] because it is String instead of Boolean: str_value");
+    assertWarning(
+        "Ignoring value for key [str_list_key] because it is String instead of Long: val1");
+  }
+
+  @Test
+  void wrongTypeWithDefault() {
+    DeclarativeConfigProperties otherProps = structuredConfigProps.getStructured("other");
+    assertThat(otherProps).isNotNull();
+
+    assertThat(otherProps.getString("int_key", "default")).isEqualTo("default");
+    assertThat(otherProps.getInt("str_key", 100)).isEqualTo(100);
+    assertThat(otherProps.getLong("str_key", 100L)).isEqualTo(100L);
+    assertThat(otherProps.getDouble("str_key", 1.1)).isEqualTo(1.1);
+    assertThat(otherProps.getBoolean("str_key", true)).isTrue();
+    assertThat(
+            otherProps.getScalarList("str_key", String.class, Collections.singletonList("default")))
+        .isEqualTo(Collections.singletonList("default"));
+    assertThat(otherProps.getStructured("str_key", empty())).isEqualTo(empty());
+    assertThat(otherProps.getStructuredList("str_key", Collections.emptyList()))
+        .isEqualTo(Collections.emptyList());
   }
 
   private void assertWarning(String message) {