Skip to content

Commit

Permalink
Improve error messages, remove node copy constructors
Browse files Browse the repository at this point in the history
  • Loading branch information
adrienlauer committed Aug 29, 2018
1 parent c0c2ad4 commit 70d792a
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Version 3.1.2 (2018-08-29)

* [fix] Fixed missing detailed error messages for configuration exceptions.
* [brk] Removed copy constructors from nodes (their shallow copy was risky and useless).

# Version 3.1.1 (2018-07-31)

Expand Down
3 changes: 3 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@
<exclude>org.seedstack.coffig.ConfigurationException</exclude>
<exclude>org.seedstack.coffig.ConfigurationValidationException</exclude>
<exclude>org.seedstack.coffig.PropertyNotFoundException</exclude>
<exclude>org.seedstack.coffig.node.ArrayNode</exclude>
<exclude>org.seedstack.coffig.node.MapNode</exclude>
<exclude>org.seedstack.coffig.node.ValueNode</exclude>
</excludes>
</parameter>
</configuration>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public enum ConfigurationErrorCode implements ErrorCode {
NON_ASSIGNABLE_CLASS,
PATH_NOT_FOUND,
PROPERTY_NOT_FOUND,
SPECIFIED_ITEM_CLASS_NOT_FOUND,
UNABLE_TO_LOAD_CLASS,
UNEXPECTED_EXCEPTION,
UNMAPPING_IS_NOT_SUPPORTED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ abstract class AbstractTreeNode implements TreeNode {
static String HIDDEN_PLACEHOLDER = "***";
private boolean hidden = false;

@Override
public boolean isHidden() {
return this.hidden;
}
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/org/seedstack/coffig/node/ArrayNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.seedstack.coffig.node;

import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -28,20 +27,21 @@ public ArrayNode() {
children = new ArrayList<>();
}

public ArrayNode(ArrayNode other) {
this.children = new ArrayList<>(other.children);
}

public ArrayNode(TreeNode... children) {
this.children = Arrays.stream(children).collect(toList());
this();
this.children.addAll(Arrays.asList(children));
}

public ArrayNode(List<TreeNode> children) {
this.children = new ArrayList<>(children);
this();
this.children.addAll(children);
}

public ArrayNode(String... children) {
this.children = Arrays.stream(children).map(ValueNode::new).collect(toList());
this();
for (String child : children) {
this.children.add(new ValueNode(child));
}
}

@Override
Expand Down
13 changes: 6 additions & 7 deletions src/main/java/org/seedstack/coffig/node/MapNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.seedstack.coffig.node;

import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
Expand All @@ -27,16 +26,16 @@ public MapNode() {
this.children = new HashMap<>();
}

public MapNode(MapNode other) {
this.children = new HashMap<>(other.children);
}

public MapNode(Map<String, TreeNode> children) {
this.children = new HashMap<>(children);
this();
this.children.putAll(children);
}

public MapNode(NamedNode... children) {
this.children = Arrays.stream(children).collect(Collectors.toMap(NamedNode::name, NamedNode::node));
this();
for (NamedNode child : children) {
this.children.put(child.name(), child.node());
}
}

@Override
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/org/seedstack/coffig/node/ValueNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ public ValueNode() {
this.value = null;
}

public ValueNode(ValueNode other) {
this.value = other.value;
}

public ValueNode(String value) {
this.value = value;
}
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/org/seedstack/coffig/spi/BaseComposite.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
import java.util.HashSet;
import java.util.Set;
import org.seedstack.coffig.Coffig;
import org.seedstack.coffig.internal.ConfigurationErrorCode;
import org.seedstack.coffig.internal.ConfigurationException;

public abstract class BaseComposite<T extends ConfigurationComponent> implements ConfigurationComponent {
protected final T[] items;
Expand Down Expand Up @@ -64,8 +62,7 @@ public <U extends T> U get(Class<U> itemClass) {
return (U) item;
}
}
throw ConfigurationException.createNew(ConfigurationErrorCode.SPECIFIED_ITEM_CLASS_NOT_FOUND)
.put("itemClass", itemClass.getCanonicalName());
throw new IllegalArgumentException("No " + itemClass.getName() + " item found in composite " + name());
}

@SuppressWarnings("unchecked")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,21 @@
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#

CANNOT_ACCESS_ARRAY_AS_SINGLE_VALUE=Illegal attempt to access an array node as a single value.
CANNOT_ACCESS_MAP_AS_SINGLE_VALUE=Illegal attempt to access an map node as a single value.
CANNOT_ACCESS_ARRAY_AS_SINGLE_VALUE=Illegal attempt to access an array as single value.
CANNOT_ACCESS_MAP_AS_SINGLE_VALUE=Illegal attempt to access a map as a single value.
CANNOT_SUPPLY_CONFIGURATION_OBJECT=An error occurred when trying to supply configuration object from method '${method}' of class '${class}'.
ERROR_DURING_FIELD_ACCESS=An error occurred when reading field '${field}' of class '${class}'.
ERROR_DURING_FIELD_INJECTION=An error occurred when setting field ${field} of class ${class}.
ERROR_DURING_GETTER_INVOCATION=An error occurred when invoking getter '${getter}' of class '${class}'.
ERROR_DURING_METHOD_INVOCATION=An error occurred when invoking the method '${method}'.
ERROR_DURING_SETTER_INVOCATION=An error occurred when invoking setter '${setter}' of class '${class}'.
ERROR_OCCURRED_DURING_COMPOSITE_PROVIDE=An error occurred during the aggregation of multiple providers.
ERROR_OCCURRED_DURING_COMPOSITE_PROVIDE=An error occurred during the aggregation of configuration providers.
FAILED_TO_READ_CONFIGURATION=Failed to read configuration from URL ${url}.
ILLEGAL_CONVERSION=Cannot convert to ${type}: '${value}'.
ILLEGAL_TREE_ACCESS=Illegal access to '${path}': ${reason}.
ILLEGAL_TREE_MERGE=Illegal merge from '${firstNodeType}' to '${secondNodeType}'.
NON_ASSIGNABLE_CLASS=Class '${assigned}' is not assignable to '${assignee}'.
PATH_NOT_FOUND=Path not found: '${path}'.
PROPERTY_NOT_FOUND=Property not found: '${property}'.
SPECIFIED_ITEM_CLASS_NOT_FOUND=No item of class '${itemClass}' can be found in the composite provider.
NON_ASSIGNABLE_CLASS=Class '${assigned}' is not compatible with type '${assignee}'.
PATH_NOT_FOUND=No configuration node could be found at '${path}'.
PROPERTY_NOT_FOUND=No configuration node named '${property}' could be found.
UNABLE_TO_LOAD_CLASS=Unable to load class '${class}'.
UNMAPPING_IS_NOT_SUPPORTED=Unmapping of type '${type}' is not supported.
2 changes: 1 addition & 1 deletion src/test/java/org/seedstack/coffig/CoffigTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public void testErrorMessages() throws Exception {
} catch (ConfigurationException e) {
assertThat(e.getMessage()).isEqualTo("[CONFIGURATION] Non assignable class");
assertThat(e.getDescription())
.isEqualTo("Class 'java.lang.Object' is not assignable to '? extends java.util.List'.");
.isEqualTo("Class 'java.lang.Object' is not compatible with type '? extends java.util.List'.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,23 @@

package org.seedstack.coffig.provider;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.Before;
import org.junit.Test;
import org.seedstack.coffig.Coffig;
import org.seedstack.coffig.fixture.PrefixFixture;
import org.seedstack.coffig.fixture.ProgrammaticFixture;
import org.seedstack.coffig.node.MapNode;

import static org.assertj.core.api.Assertions.assertThat;

public class ProgrammaticProviderTest {
private ProgrammaticProvider programmaticProvider = ((CompositeProvider) Coffig
.builder()
.withProviders(new ProgrammaticProvider())
.build().getProvider()).get(ProgrammaticProvider.class);
private ProgrammaticProvider programmaticProvider;

@Before
public void setUp() throws Exception {
programmaticProvider = new ProgrammaticProvider();
programmaticProvider.initialize(Coffig.basic());
}

@Test
public void testProvideEmpty() throws Exception {
Expand Down

0 comments on commit 70d792a

Please sign in to comment.