Skip to content

Commit

Permalink
#363 Detect conflicts with root-path property, validate property path…
Browse files Browse the repository at this point in the history
…s more strictly
  • Loading branch information
ljacqu committed Aug 27, 2023
1 parent cab40d1 commit 7bf10c5
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

/**
* Builds a list of known properties in an ordered and grouped manner.
*
* <p>
* It guarantees that the added entries:
* <ul>
* <li>are grouped by path, e.g. all "DataSource.mysql" properties are together, and "DataSource.mysql" properties
Expand All @@ -24,25 +24,24 @@
*/
public class PropertyListBuilder {

private @NotNull Map<String, Object> rootEntries = new LinkedHashMap<>();
private final @NotNull Map<String, Object> rootEntries = new LinkedHashMap<>();

/**
* Adds the property to the list builder.
*
* @param property the property to add
*/
public void add(@NotNull Property<?> property) {
String[] paths = property.getPath().split("\\.");
Map<String, Object> map = rootEntries;
for (int i = 0; i < paths.length - 1; ++i) {
map = getChildMap(map, paths[i]);
}
String[] pathElements = property.getPath().split("\\.", -1);
Map<String, Object> mapForProperty = getMapBeforeLastElement(pathElements);

final String end = paths[paths.length - 1];
if (map.containsKey(end)) {
final String lastElement = pathElements[pathElements.length - 1];
if (mapForProperty.containsKey(lastElement)) {
throw new ConfigMeException("Path at '" + property.getPath() + "' already exists");
} else if (pathElements.length > 1 && lastElement.equals("")) {
throwExceptionForMalformedPath(property.getPath());
}
map.put(end, property);
mapForProperty.put(lastElement, property);
}

/**
Expand All @@ -54,9 +53,36 @@ public void add(@NotNull Property<?> property) {
public @NotNull List<Property<?>> create() {
List<Property<?>> result = new ArrayList<>();
collectEntries(rootEntries, result);
if (result.size() > 1 && rootEntries.containsKey("")) {
throw new ConfigMeException("A property at the root path (\"\") cannot be defined alongside "
+ "other properties as the paths would conflict");
}
return result;
}

/**
* Returns the nested map for the given path parts in which a property can be saved (for the last element
* in the path parts). Throws an exception if the path is malformed.
*
* @param pathParts the path elements (i.e. the property path split by ".")
* @return the map to store the property in
*/
protected @NotNull Map<String, Object> getMapBeforeLastElement(String @NotNull [] pathParts) {
Map<String, Object> map = rootEntries;
for (int i = 0; i < pathParts.length - 1; ++i) {
map = getChildMap(map, pathParts[i]);
if (pathParts[i].equals("")) {
throwExceptionForMalformedPath(String.join(".", pathParts));
}
}
return map;
}

protected void throwExceptionForMalformedPath(@NotNull String path) {
throw new ConfigMeException("The path at '" + path + "' is malformed: dots may not be at the beginning or end "
+ "of a path, and dots may not appear multiple times successively.");
}

protected final @NotNull Map<String, Object> getRootEntries() {
return rootEntries;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
import ch.jalu.configme.properties.Property;
import ch.jalu.configme.properties.StringProperty;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;

import static ch.jalu.configme.TestUtils.transform;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -102,6 +105,54 @@ void shouldThrowForUnknownInternalEntry() {
assertThat(properties.create(), hasSize(1));
}

@Test
void shouldSupportRootProperty() {
// given
PropertyListBuilder listBuilder = new PropertyListBuilder();
Property<?> rootProperty = createPropertyWithPath("");
listBuilder.add(rootProperty);

// when
List<Property<?>> properties = listBuilder.create();

// then
assertThat(properties, contains(rootProperty));
}

@Test
void shouldThrowForRootPathAndOtherProperty() {
// given
PropertyListBuilder properties = new PropertyListBuilder();
properties.add(createPropertyWithPath(""));
properties.add(createPropertyWithPath("enabled"));

// when
ConfigMeException ex = assertThrows(ConfigMeException.class, properties::create);

// then
assertThat(ex.getMessage(),
equalTo("A property at the root path (\"\") cannot be defined alongside other properties as the paths would conflict"));
}

@ParameterizedTest
@MethodSource("malformedPropertyPaths")
void shouldThrowForMalformedPropertyPath(String path) {
// given
PropertyListBuilder properties = new PropertyListBuilder();

// when
ConfigMeException ex = assertThrows(ConfigMeException.class,
() -> properties.add(createPropertyWithPath(path)));

// then
assertThat(ex.getMessage(), equalTo("The path at '" + path + "' is malformed: dots may not be at "
+ "the beginning or end of a path, and dots may not appear multiple times successively."));
}

static Stream<String> malformedPropertyPaths() {
return Stream.of(".", "..", ".security", "security.", "alf..beta", "security.hash..version.minor");
}

private static Property<?> createPropertyWithPath(String path) {
return new StringProperty(path, "");
}
Expand Down

0 comments on commit 7bf10c5

Please sign in to comment.