Clarify the behaviour when null is returned by the converters

Signed-off-by: Emily Jiang <emijiang6@googlemail.com>

api/src/main/java/org/eclipse/microprofile/config/Config.java

@@ -128,7 +128,7 @@ public interface Config {
  * @throws IllegalArgumentException
  * if the property cannot be converted to the specified type
  * @throws java.util.NoSuchElementException
- * if the property is not defined or is defined as an empty string
+ * if the property is not defined or is defined as an empty string or the converter returns {@code null}
  */
  <T> T getValue(String propertyName, Class<T> propertyType);
 
@@ -169,7 +169,8 @@ public interface Config {
  * @throws java.lang.IllegalArgumentException
  * if the property values cannot be converted to the specified type
  * @throws java.util.NoSuchElementException
- * if the property isn't present in the configuration
+ * if the property isn't present in the configuration or is defined as an empty string or the converter
+ * returns {@code null}
  */
  default <T> List<T> getValues(String propertyName, Class<T> propertyType) {
  @SuppressWarnings("unchecked")

api/src/main/java/org/eclipse/microprofile/config/inject/ConfigProperty.java

@@ -70,8 +70,9 @@
  *
  * <p>
  * Contrary to natively injecting, if the property is not specified, this will not lead to a DeploymentException. The
- * following code injects a Long value to the {@code my.optional.long.property}. If the property does not exist, the
- * value {@code 123} will be assigned. to {@code injectedLongValue}.
+ * following code injects a Long value to the {@code my.optional.long.property}. If the property value does not exist or
+ * the converter returns `null` for the value specified, the value {@code 123} will be assigned to
+ * {@code injectedLongValue}.
  *
  * <pre>
  * &#064;Inject
@@ -139,8 +140,8 @@
  String name() default "";
 
  /**
- * The default value if the configured property value does not exist. Empty string as the default value will be
- * ignored, which is same as not setting the default value.
+ * The default value if the configured property value does not exist or the value is being converted to `null`.
+ * Empty string as the default value will be ignored, which is same as not setting the default value.
  * <p>
  * If the target Type is not String a proper {@link org.eclipse.microprofile.config.spi.Converter} will get applied.
  * That means that any default value string should follow the formatting rules of the registered Converters.

spec/src/main/asciidoc/converters.asciidoc

@@ -102,6 +102,8 @@ If no built-in nor custom `Converter` exists for a requested Type `T`, an implic
 * The target type `T` has a `public static T parse(CharSequence)` method, or
 * The target type `T` has a public Constructor with a `String` parameter
 
+If a converter returns `null` for a given config value, the property will be treated as non-existent. If it is a required property, `NoSuchElementException` will be thrown. If `defaultValue` is specified on the property injection, the `defaultValue` will be used.
+
 === Cleaning up a Converter
 
 If a `Converter` implements the `java.lang.AutoCloseable` interface then the `close()` method will be called when the underlying `Config` is being released.

tck/src/main/java/org/eclipse/microprofile/config/tck/configsources/DefaultConfigSourceOrdinalTest.java

@@ -20,6 +20,7 @@
 package org.eclipse.microprofile.config.tck.configsources;
 
 import javax.inject.Inject;
+
 import org.eclipse.microprofile.config.Config;
 import org.jboss.arquillian.container.test.api.Deployment;
 import org.jboss.arquillian.testng.Arquillian;
@@ -44,52 +45,50 @@ public class DefaultConfigSourceOrdinalTest extends Arquillian {
  @Deployment
  public static Archive deployment() {
  JavaArchive testJar = ShrinkWrap
- .create(JavaArchive.class, "DefaultConfigSourceOrdinalTest.jar")
- .addClasses(DefaultConfigSourceOrdinalTest.class)
- .addAsManifestResource(
- new StringAsset(
- "config_ordinal=200\n" +
- "customer_name=Bill\n" +
- "customer.hobby=Badminton"
- ),
+ .create(JavaArchive.class, "DefaultConfigSourceOrdinalTest.jar")
+ .addClasses(DefaultConfigSourceOrdinalTest.class)
+ .addAsManifestResource(
+ new StringAsset(
+ "config_ordinal=200\n" +
+ "customer_name=Bill\n" +
+ "customer.hobby=Badminton"),
  "microprofile-config.properties")
- .addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml")
- .as(JavaArchive.class);
+  .addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml")
+  .as(JavaArchive.class);
 
  WebArchive war = ShrinkWrap
- .create(WebArchive.class, "DefaultConfigSourceOrdinalTest.war")
- .addAsLibrary(testJar);
+  .create(WebArchive.class, "DefaultConfigSourceOrdinalTest.war")
+  .addAsLibrary(testJar);
  return war;
  }
 
  @BeforeClass
  public void checkSetup() {
- //check whether the environment variables were populated by the executor correctly
+  // check whether the environment variables were populated by the executor correctly
 
  if (!"45".equals(System.getenv("config_ordinal"))) {
- Assert.fail("Before running this test, the environment variable \"config_ordinal\" must be set with the value of 45");
+ Assert.fail(
+ "Before running this test, the environment variable \"config_ordinal\" must be set with the value of 45");
  }
  if (!"Bob".equals(System.getenv("customer_name"))) {
- Assert.fail("Before running this test, the environment variable \"customer_name\" must be set with the value of Bob");
+ Assert.fail(
+ "Before running this test, the environment variable \"customer_name\" must be set with the value of Bob");
  }
  System.setProperty("customer.hobby", "Tennis");
  System.setProperty("config_ordinal", "120");
-
-
-
+
  }
- 
+
  @Test
  public void testOrdinalForEnv() {
  Assert.assertEquals("Bill", config.getValue("customer_name", String.class));
  Assert.assertEquals(200, config.getConfigValue("customer_name").getSourceOrdinal());
  }
- 
+
  @Test
  public void testOrdinalForSystemProps() {
  Assert.assertEquals("Badminton", config.getValue("customer.hobby", String.class));
  Assert.assertEquals(200, config.getConfigValue("customer.hobby").getSourceOrdinal());
  }
-
-
+
 }

tck/src/main/java/org/eclipse/microprofile/config/tck/converters/ConvertedNullValueTest.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) 2020 Contributors to the Eclipse Foundation
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * You may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package org.eclipse.microprofile.config.tck.converters;
+
+import java.util.NoSuchElementException;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.inject.Inject;
+
+import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+import org.eclipse.microprofile.config.spi.Converter;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.testng.Arquillian;
+import org.jboss.shrinkwrap.api.Archive;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.EmptyAsset;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+/**
+ * This is to test that when a converter returns null for some config value, null will be assigned. This means the
+ * property still exists but the value is set to null on purpose.
+ */
+public class ConvertedNullValueTest extends Arquillian {
+ private @Inject Config config;
+ private @Inject MyBean myBean;
+
+ @Deployment
+ public static Archive deployment() {
+ return ShrinkWrap
+ .create(WebArchive.class, "ConvertedNullValueTest.war")
+ .addClasses(ConvertedNullValueTest.class, Pizza.class, PizzaConverter.class, MyBean.class)
+ .addAsResource(
+ new StringAsset(
+ "partial.pizza=cheese"),
+ "META-INF/microprofile-config.properties")
+ .addAsServiceProvider(Converter.class, PizzaConverter.class)
+ .addAsWebInfResource(EmptyAsset.INSTANCE, "beans.xml");
+
+ }
+
+ @Test
+ public void testDefaultValue() {
+ // The converter returns null as the converter returns null if the property does not contain :
+ // Therefore, the defaultValue was used.
+ Assert.assertEquals(myBean.getPizza(), new Pizza("chicken", "medium"));
+ }
+
+ @Test
+ public void testGetValue() {
+ // The converter returns null as the converter returns null if the property does not contain :
+ // Therefore, it will be treated as non-existence.
+ Assert.assertThrows(NoSuchElementException.class, () -> config.getValue("partial.pizza", Pizza.class));
+ }
+
+ @Test
+ public void testGetOptionalValue() {
+ // The converter returns null as the converter returns null if the property does not contain :
+ // Therefore, it will be treated as non-existence.
+ Assert.assertFalse(config.getOptionalValue("partial.pizza", Pizza.class).isPresent());
+ }
+
+ @ApplicationScoped
+ public static class MyBean {
+
+ private @Inject @ConfigProperty(name = "partial.pizza", defaultValue = "medium:chicken") Pizza myPizza;
+
+ public Pizza getPizza() {
+ return myPizza;
+ }
+ }
+
+}