Skip to content

Commit

Permalink
Speficy handling of empty config property
Browse files Browse the repository at this point in the history
* [Spec] add section explaining how the Config API handles a config
  property with missing or empty value
* [Spec] mention that it is possible to set an empty value to reset a
  property from a lower-ordinal config source.
* [TCK] add EmptyConfigPropertyTest to test all cases of empty config
  property

Signed-off-by: Jeff Mesnil <[email protected]>
  • Loading branch information
jmesnil committed May 14, 2019
1 parent 8839aa6 commit 0aeb8cd
Show file tree
Hide file tree
Showing 6 changed files with 417 additions and 13 deletions.
41 changes: 41 additions & 0 deletions spec/src/main/asciidoc/converters.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,44 @@ If no built-in nor custom `Converter` exists for a requested Type `T`, an implic

If a `Converter` implements the `java.lang.AutoCloseable` interface then the `close()` method will be called when the underlying `Config` is being released.

=== Empty value

A config property can be configured with an empty value (the empty String `""`).

The config API interprets an empty string as a missing property and throws a `NoSuchElementException` when the output type is a `String`.

[NOTE]
This behaviour allows to _unconfigure_ a property. If a property is configured in a lower-ordinal ConfigSource with
an undesirable value, you can set the property value to an empty string in a higher-ordinal ConfigSource to discard the lower-ordinal value.

The Config API also interprets an empty string as an empty array when the output type is `String[]`.
For consistency, the Config API also discards any leading and trailing commas `","` when the output type is a `String[]``

[[empty_value_table]]
.Examples of Config API
[options="header"]
|=======================
| Property value | Output type | `Config` method | Output result
| missing | `String` | `getValue` | throws `NoSuchElementException`
| `""` (empty string) | `String` | `getValue` | throws `NoSuchElementException`
| `" "` | `String` | `getValue` | `" "`
| `"foo"` | `String` | `getValue` | `"foo"`
| missing | `String[]` | `getValue` | `{ }` (empty array)
| `""` | `String[]` | `getValue` | `{ }`
| `" "` | `String[]` | `getValue` | `{ " " }`
| `","` | `String[]` | `getValue` | `{ }`
| `",,"` | `String[]` | `getValue` | `{ }`
| `"foo"` | `String[]` | `getValue` | `{ "foo" }`
| `"foo,bar"` | `String[]` | `getValue` | `{ "foo", "bar" }`
| `"foo,"` | `String[]` | `getValue` | `{ "foo" }`
| `",bar"` | `String[]` | `getValue` | `{ "bar" }`
| `",bar,"` | `String[]` | `getValue` | `{ "bar" }`
| missing | `String` |`getOptionalValue` | `Optional.empty()`
| `""` | `String` | `getOptionalValue` | `Optional.empty()`
| `" "` | `String` | `getOptionalValue` | `Optional.of(" ")`
| `"foo"` | `String` | `getOptionalValue` | `Optional.of("foo")`
| missing | `String[]` | `getOptionalValue` | `Optional.empty()`
| `""` | `String[]` | `getOptionalValue` | `Optional.empty()`
| `","` | `String[]` | `getOptionalValue` | `Optional.empty()`
| `",,"` | `String[]` | `getOptionalValue` | `Optional.empty()`
| `"foo,bar"` | `String[]` | `getOptionalValue` | `Optional.of({ "foo", "bar" })`
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright (c) 2019 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;

import static org.eclipse.microprofile.config.tck.EmptyConfigPropertyTest.EMPTY_PROP;

import java.util.List;
import java.util.Set;

import javax.enterprise.context.Dependent;
import javax.inject.Inject;

import org.eclipse.microprofile.config.inject.ConfigProperty;

@Dependent
public class EmptyConfigPropertyBean {

@Inject
@ConfigProperty(name = EMPTY_PROP)
private String[] myStrings;

@Inject
@ConfigProperty(name = EMPTY_PROP)
private List<String> myStringList;

@Inject
@ConfigProperty(name = EMPTY_PROP)
private Set<String> myStringSet;

public String[] getMyStrings() {
return myStrings;
}

public List<String> getMyStringList() {
return myStringList;
}

public Set<String> getMyStringSet() {
return myStringSet;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
/*
* Copyright (c) 2016-2017 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;

import static org.eclipse.microprofile.config.tck.base.AbstractTest.addFile;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.fail;

import java.util.NoSuchElementException;
import java.util.Optional;

import javax.inject.Inject;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
import org.eclipse.microprofile.config.tck.configsources.KeyValuesConfigSource;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.testng.Arquillian;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.EmptyAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.jboss.shrinkwrap.api.spec.WebArchive;
import org.testng.Assert;
import org.testng.annotations.Test;

public class EmptyConfigPropertyTest extends Arquillian {

@Deployment
public static WebArchive deploy() {
JavaArchive testJar = ShrinkWrap
.create(JavaArchive.class, "EmptyConfigPropertyTest.jar")
.addClass(EmptyConfigPropertyBean.class)
.addClass(KeyValuesConfigSource.class)
.addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml")
.as(JavaArchive.class);

addFile(testJar, "META-INF/microprofile-config.properties");

WebArchive war = ShrinkWrap
.create(WebArchive.class, "EmptyConfigPropertyTest.war")
.addAsLibrary(testJar);
return war;
}


private @Inject
Config config;

@Inject
private EmptyConfigPropertyBean bean;

static final String EMPTY_PROP = "tck.config.empty";
static final String MISSING_PROP = "tck.config.this.property.is.not.defined.anywhere";

@Test
public void testEmptyPropertyWithConfig() {
// | Property value | Output type | `Config` method | Output result
// | `""` (empty string) | `String` | `getValue` | throws `NoSuchElementException`
try {
config.getValue(EMPTY_PROP, String.class);
Assert.fail("An empty string is considered as missing and must raise a NoSuchElementException");
}
catch (NoSuchElementException e) {
}
try {
config.getValue(EMPTY_PROP, Double.class);
Assert.fail(("an empty String is considered missing regardless of the ouput type"));
}
catch (NoSuchElementException e) {
}

// but if it is accessed an array, it will returns an empty array
// | `""` | `String[]` | `getValue` | `{ }`
assertEquals(config.getValue(EMPTY_PROP, String[].class).length, 0);
// regardless of the output type
assertEquals(config.getValue(EMPTY_PROP, Double[].class).length, 0);


// | `""` | `String` | `getOptionalValue` | `Optional.empty()`
assertEquals(config.getOptionalValue(EMPTY_PROP, String.class), Optional.empty());
assertEquals(config.getOptionalValue(EMPTY_PROP, Double.class), Optional.empty());

// | `""` | `String[]` | `getOptionalValue` | `Optional.empty()`
assertEquals( config.getOptionalValue(EMPTY_PROP, String[].class), Optional.empty());
assertEquals( config.getOptionalValue(EMPTY_PROP, Double[].class), Optional.empty());
}

@Test
public void testMissingProperty() {
// | Property value | Output type | `Config` method | Output result
// | missing | `String` | `getValue` | throws `NoSuchElementException`
try {
config.getValue(MISSING_PROP, String.class);
Assert.fail("A missing property must raise a NoSuchElementException");
}
catch (NoSuchElementException e) {
}
// | missing | `String[]` | `getValue` | `{ }` (empty array)
assertEquals(config.getValue(MISSING_PROP, String[].class).length, 0);
// regardless of the output type
assertEquals(config.getValue(MISSING_PROP, Double[].class).length, 0);

// | missing | `String` |`getOptionalValue` | `Optional.empty()`
assertEquals( config.getOptionalValue(MISSING_PROP, String.class), Optional.empty());
// | missing | `String[]` | `getOptionalValue` | `Optional.empty()`
assertEquals( config.getOptionalValue(MISSING_PROP, String[].class), Optional.empty());
}

@Test
public void testArrayCommasRules() {
Config config = KeyValuesConfigSource.buildConfig(
"arrayWithSpace", " ",
"arrayWithSingleComma", ",",
"arrayWithTwoCommas", ",,",
"arrayWithSingleValue", "foo",
"arrayWithTwoValues", "foo,bar",
"arrayWithTrailingComma", "foo,",
"arrayWithLeadingComma", ",bar",
"arrayWithLeadingAndTrailingCommas", ",bar,");

// | Property value | Output type | `Config` method | Output result
// | `" "` | `String[]` | `getValue` | `{ " " }`
assertEquals(config.getValue("arrayWithSpace", String[].class), new String[] { " " });
// | `","` | `String[]` | `getValue` | `{ }`
assertEquals(config.getValue("arrayWithSingleComma", String[].class).length, 0);
// | `",,"` | `String[]` | `getValue` | `{ }`
assertEquals(config.getValue("arrayWithTwoCommas", String[].class).length, 0);
// | `"foo"` | `String[]` | `getValue` | `{ "foo" }`
assertEquals(config.getValue("arrayWithSingleValue", String[].class), new String[] { "foo" });
// | `"foo,bar"` | `String[]` | `getValue` | `{ "foo", "bar" }`
assertEquals(config.getValue("arrayWithTwoValues", String[].class), new String[] { "foo", "bar" });
// | `"foo,"` | `String[]` | `getValue` | `{ "foo" }`
assertEquals(config.getValue("arrayWithTrailingComma", String[].class), new String[] { "foo" });
// | `",bar"` | `String[]` | `getValue` | `{ "bar" }`
assertEquals(config.getValue("arrayWithLeadingComma", String[].class), new String[] { "bar" });
// | `",bar,"` | `String[]` | `getValue` | `{ "bar" }`
assertEquals(config.getValue("arrayWithLeadingAndTrailingCommas", String[].class), new String[] { "bar" });

// | `","` | `String[]` | `getOptionalValue` | `Optional.empty()`
assertEquals(config.getOptionalValue("arrayWithSingleComma", String[].class), Optional.empty());
// | `",,"` | `String[]` | `getOptionalValue` | `Optional.empty()`
assertEquals(config.getOptionalValue("arrayWithTwoCommas", String[].class), Optional.empty());
}

@Test
public void testEmptyPropertyWithConfigAccessor() {
try {
config.access(EMPTY_PROP, String.class).build().getValue();
Assert.fail("An empty string is considered as missing and must raise a NoSuchElementException");
}
catch (NoSuchElementException e) {

}
assertEquals(config.access(EMPTY_PROP, String.class).build().getOptionalValue(), Optional.empty());

assertEquals(config.access(EMPTY_PROP, String[].class).build().getValue().length, 0);
assertEquals(config.access(EMPTY_PROP, String[].class).build().getOptionalValue(), Optional.empty());
}

@Test
public void testEmptyPropertyWithInjectedConfigProperty() {
assertEquals(bean.getMyStrings().length, 0);
assertEquals(bean.getMyStringList().size(), 0);
assertEquals(bean.getMyStringSet().size(), 0);
}

@Test
public void testEmptyPropertyResetsLowerOrdinalProperty() {
Config config = ConfigProviderResolver.instance().getBuilder()
.withSources(
// lower-ordinal configures a prop
KeyValuesConfigSource.config(10,
"my.prop", "foo"),
// higher-ordinal resets it with an empty string
KeyValuesConfigSource.config(20,
"my.prop", ""))
.build();

try {
config.getValue("my.prop", String.class);
fail("The property must be reset by the empty string in the higher-ordinal config source");
}
catch (NoSuchElementException e) {
}

assertEquals(config.getOptionalValue("my.prop", String.class), Optional.empty());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
*/
package org.eclipse.microprofile.config.tck;

import static org.testng.Assert.assertEquals;

import java.util.NoSuchElementException;

import javax.inject.Inject;

import org.eclipse.microprofile.config.Config;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.testng.Arquillian;
Expand All @@ -30,10 +36,6 @@
import org.jboss.shrinkwrap.api.spec.WebArchive;
import org.testng.annotations.Test;

import javax.inject.Inject;

import static org.testng.Assert.assertEquals;

public class EmptyValuesTest extends Arquillian {

private static final String EMPTY_PROPERTY = "my.empty.property";
Expand Down Expand Up @@ -63,17 +65,31 @@ public void testEmptyStringValues() {
assertEquals(emptyValuesBean.getStringValue(), "");
}

@Test
@Test(expectedExceptions = NoSuchElementException.class)
public void testEmptyStringProgrammaticLookup() {
System.setProperty(EMPTY_PROPERTY, "");
String stringValue = config.getValue(EMPTY_PROPERTY, String.class);
assertEquals(stringValue, "");
System.clearProperty(EMPTY_PROPERTY);
try {
System.setProperty(EMPTY_PROPERTY, "");
config.getValue(EMPTY_PROPERTY, String.class);
}
finally {
System.clearProperty(EMPTY_PROPERTY);
}
}

@Test
@Test(expectedExceptions = NoSuchElementException.class)
public void testEmptyStringPropertyFromConfigFile() {
String stringValue = config.getValue(PROP_FILE_EMPTY_PROPERTY, String.class);
assertEquals(stringValue, "");
config.getValue(PROP_FILE_EMPTY_PROPERTY, String.class);
}

@Test
public void testEmptyStringLookupAsArray() {
try {
System.setProperty(EMPTY_PROPERTY, "");
String[] result = config.getValue(EMPTY_PROPERTY, String[].class);
assertEquals(result.length, 0);
}
finally {
System.clearProperty(EMPTY_PROPERTY);
}
}
}
Loading

0 comments on commit 0aeb8cd

Please sign in to comment.