Skip to content

Commit

Permalink
Disallow size one input list in InputValidationIssue (#78)
Browse files Browse the repository at this point in the history 
Fixes #77.

The approach is to evaluate the setter methods as pure setters and the
'add' methods as methods to add input (or in, name, value) to what
already exist.

---------

Co-authored-by: Jimmy Praet <jimmy.praet@ksz-bcss.fgov.be>
  • Loading branch information
@usku01 @jpraet
usku01 and jpraet authored Oct 30, 2024
1 parent 5fde430 commit 2e472da
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 55 deletions.
112 changes: 83 additions & 29 deletions ...if-rest-problem/src/main/java/io/github/belgif/rest/problem/api/InputValidationIssue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
Expand Down Expand Up @@ -39,6 +41,9 @@ public class InputValidationIssue {
private static final String INPUTS_AND_IN_NAME_VALUE_ARE_MUTUALLY_EXCLUSIVE =
"inputs[] and in/name/value are mutually exclusive";

private static final String INPUTS_SETTER_ONE_ITEM =
"inputs[] can not be set with a single item, use in(in, name, value) instead";

private URI type;
private URI href;
private String title;
Expand Down Expand Up @@ -105,7 +110,7 @@ public InEnum getIn() {
}

public void setIn(InEnum in) {
verifyNoInputs();
verifyNoInputs(in);
this.in = in;
}

Expand All @@ -114,7 +119,7 @@ public String getName() {
}

public void setName(String name) {
verifyNoInputs();
verifyNoInputs(name);
this.name = name;
}

Expand All @@ -124,41 +129,49 @@ public Object getValue() {
}

public void setValue(Object value) {
verifyNoInputs();
verifyNoInputs(value);
this.value = value;
}

public List<Input<?>> getInputs() {
return Collections.unmodifiableList(inputs);
}

public void setInputs(List<Input<?>> inputs) {
verifyNoInNameValue();
this.inputs.clear();
this.inputs.addAll(inputs);
}

public void setInputs(Input<?>... inputs) {
verifyNoInNameValue();
this.inputs.clear();
this.inputs.addAll(Arrays.asList(inputs));
private void clearInNameValue() {
this.in = null;
this.name = null;
this.value = null;
}

public void addInput(Input<?> input) {
verifyNoInNameValue();
inputs.add(input);

if (input == null) {
return;
}

if (hasInNameValue()) {
this.inputs.add(new Input<>(in, name, value));
this.inputs.add(input);
clearInNameValue();
} else if (!inputs.isEmpty()) {
this.inputs.add(input);
} else {
in(input.getIn(), input.getName(), input.getValue());
}
}

private void verifyNoInputs() {
private void verifyNoInputs(Object valueToUpdate) {
if (valueToUpdate == null) {
return;
}

if (!inputs.isEmpty()) {
throw new IllegalArgumentException(INPUTS_AND_IN_NAME_VALUE_ARE_MUTUALLY_EXCLUSIVE);
}
}

private void verifyNoInNameValue() {
if (in != null || name != null || value != null) {
throw new IllegalArgumentException(INPUTS_AND_IN_NAME_VALUE_ARE_MUTUALLY_EXCLUSIVE);
}
private boolean hasInNameValue() {
return in != null || name != null || value != null;
}

@JsonAnyGetter
Expand Down Expand Up @@ -271,7 +284,9 @@ public InputValidationIssue value(Object value) {
* @param key the key
* @param value the value
* @return this InputValidationIssue
* @deprecated use {@link InputValidationIssue#input(Input)} to reference multiple input values
* @deprecated use {@link InputValidationIssue#inputs(Collection)} or
* {@link InputValidationIssue#inputs(Input, Input, Input[])} to
* reference multiple input values
*/
@SuppressWarnings("unchecked")
@Deprecated
Expand All @@ -288,18 +303,57 @@ public InputValidationIssue valueEntry(String key, Object value) {
return this;
}

public InputValidationIssue input(Input<?> input) {
addInput(input);
return this;
}
/**
* Set the inputs[] to the given collection.
*
* @param inputs collection with input items to initialize the inputs[] array. Any previous inputs are removed.
* @return this InputValidationIssue
*
* @throws IllegalArgumentException if in/name/value properties are not null (mutually exclusive with inputs[]),
* or when the collection only contains one non-null item.
*/
public InputValidationIssue inputs(Collection<Input<?>> inputs) {

if (inputs == null) {
return this;
}

if (hasInNameValue()) {
throw new IllegalArgumentException(INPUTS_AND_IN_NAME_VALUE_ARE_MUTUALLY_EXCLUSIVE);
}

List<Input<?>> filteredInputs = inputs.stream().filter(Objects::nonNull).collect(Collectors.toList());

public InputValidationIssue inputs(List<Input<?>> inputs) {
setInputs(inputs);
if (filteredInputs.size() == 1) {
throw new IllegalArgumentException(INPUTS_SETTER_ONE_ITEM);
}

this.inputs.clear();
this.inputs.addAll(filteredInputs);
return this;
}

public InputValidationIssue inputs(Input<?>... inputs) {
setInputs(inputs);
/**
* Set the inputs[] to the given inputs.
*
* @param firstInput the first input
* @param secondInput the second input
* @param otherInputs the varargs array with other inputs
* @return this InputValidationIssue
*
* @throws IllegalArgumentException if in/name/value properties are not null (mutually exclusive with inputs[]),
* or when the parameters only contain one non-null item.
*/
public InputValidationIssue inputs(Input<?> firstInput, Input<?> secondInput, Input<?>... otherInputs) {
if (firstInput == null && secondInput == null && otherInputs == null) {
return this;
}

List<Input<?>> inputsToAdd = new ArrayList<>(Arrays.asList(firstInput, secondInput));
Collections.addAll(inputsToAdd, otherInputs);

inputs(inputsToAdd);

return this;
}

Expand Down
6 changes: 4 additions & 2 deletions ...f-rest-problem/src/main/java/io/github/belgif/rest/problem/api/InputValidationIssues.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,11 @@ public static InputValidationIssue requiredInputsIfPresent(Input<?> target, List
.detail(String.format("All of these inputs must be present if %s is present: %s",
target.getName(), getJoinedNames(inputs)));
issue.addInput(target);
for (Input<?> input : inputs) {
issue.addInput(input);

if (inputs != null) {
inputs.forEach(issue::addInput);
}

return issue;
}

Expand Down
103 changes: 79 additions & 24 deletions ...est-problem/src/test/java/io/github/belgif/rest/problem/api/InputValidationIssueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@
import static org.assertj.core.api.Assertions.*;

import java.net.URI;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import org.assertj.core.api.InstanceOfAssertFactories;
import org.assertj.core.api.ThrowableAssert;
import org.junit.jupiter.api.Test;

class InputValidationIssueTest {

private static final String MUTUALLY_EXCLUSIVE_EXC = "mutually exclusive";
private static final String SINGLE_ITEM_EXC = "single item";

@Test
void construct() {
InputValidationIssue issue = new InputValidationIssue();
Expand Down Expand Up @@ -81,6 +87,12 @@ void detail() {
@Test
void in() {
InputValidationIssue issue = new InputValidationIssue();

issue.setIn(null);
assertThat(issue.getIn()).isNull();
issue.in(null);
assertThat(issue.getIn()).isNull();

issue.setIn(InEnum.QUERY);
assertThat(issue.getIn()).isEqualTo(InEnum.QUERY);
assertThat(new InputValidationIssue().in(InEnum.QUERY).getIn()).isEqualTo(InEnum.QUERY);
Expand All @@ -89,6 +101,10 @@ void in() {
@Test
void name() {
InputValidationIssue issue = new InputValidationIssue();

issue.setName(null);
issue.name(null);

issue.setName("name");
assertThat(issue.getName()).isEqualTo("name");
assertThat(new InputValidationIssue().name("name").getName()).isEqualTo("name");
Expand All @@ -97,6 +113,9 @@ void name() {
@Test
void value() {
InputValidationIssue issue = new InputValidationIssue();

issue.setValue(null);
issue.value(null);
issue.setValue("value");
assertThat(issue.getValue()).isEqualTo("value");
assertThat(new InputValidationIssue().value("value").getValue()).isEqualTo("value");
Expand All @@ -115,7 +134,8 @@ void valueEntry() {
void valueEntryFailsOnExistingNonMapValue() {
InputValidationIssue issue = new InputValidationIssue();
issue.value("not a map");
assertThatIllegalStateException().isThrownBy(() -> issue.valueEntry("key", "value"));
assertThatIllegalStateException().isThrownBy(() -> issue.valueEntry("key", "value"))
.withMessageContaining("value is already set");
}

@Test
Expand All @@ -129,45 +149,74 @@ void inNameValue() {
@Test
void inNameValueAndInputsAreMutuallyExclusive() {
Input<?> input = Input.query("name", "value");
assertThatIllegalArgumentException().isThrownBy(
() -> new InputValidationIssue().in(InEnum.QUERY).input(input));
assertThatIllegalArgumentException().isThrownBy(
() -> new InputValidationIssue().name("name").input(input));
assertThatIllegalArgumentException().isThrownBy(
() -> new InputValidationIssue().value("value").input(input));
assertThatIllegalArgumentException().isThrownBy(
() -> new InputValidationIssue().input(input).in(InEnum.QUERY));
assertThatIllegalArgumentException().isThrownBy(
() -> new InputValidationIssue().input(input).name("name"));
assertThatIllegalArgumentException().isThrownBy(
() -> new InputValidationIssue().input(input).value("value"));
List<Input<?>> inputs = Arrays.asList(input, input);

assertMutuallyExclusiveException(() -> new InputValidationIssue().in(InEnum.QUERY).inputs(null, null));
assertMutuallyExclusiveException(() -> new InputValidationIssue().name("name").inputs(null, null));
assertMutuallyExclusiveException(() -> new InputValidationIssue().value("value").inputs(null, null));
assertMutuallyExclusiveException(() -> new InputValidationIssue().in(InEnum.QUERY).inputs(input, input));
assertMutuallyExclusiveException(() -> new InputValidationIssue().name("name").inputs(input, input));
assertMutuallyExclusiveException(() -> new InputValidationIssue().value("value").inputs(input, input));
assertMutuallyExclusiveException(() -> new InputValidationIssue().inputs(input, input).in(InEnum.QUERY));
assertMutuallyExclusiveException(() -> new InputValidationIssue().inputs(input, input, input).name("name"));
assertMutuallyExclusiveException(() -> new InputValidationIssue().inputs(input, input).value("value"));
assertMutuallyExclusiveException(() -> new InputValidationIssue().in(InEnum.QUERY).inputs(inputs));
assertMutuallyExclusiveException(() -> new InputValidationIssue().name("name").inputs(inputs));
assertMutuallyExclusiveException(() -> new InputValidationIssue().value("value").inputs(inputs));
assertMutuallyExclusiveException(() -> new InputValidationIssue().inputs(inputs).in(InEnum.QUERY));
assertMutuallyExclusiveException(() -> new InputValidationIssue().inputs(inputs).name("name"));
assertMutuallyExclusiveException(() -> new InputValidationIssue().inputs(inputs).value("value"));
}

@Test
void inputs() {
List<Input<?>> inputs = Collections.singletonList(Input.query("name", "value"));
List<Input<?>> inputs;
InputValidationIssue issue = new InputValidationIssue();
issue.setInputs(inputs);
assertThat(issue.getInputs()).isUnmodifiable().isEqualTo(inputs);

assertThatIllegalArgumentException()
.isThrownBy(() -> issue.inputs(Collections.singletonList(Input.query("name", "value"))))
.withMessageContaining(SINGLE_ITEM_EXC);
assertThatIllegalArgumentException()
.isThrownBy(() -> issue.inputs(Arrays.asList(Input.query("name", "value"), null, null)))
.withMessageContaining(SINGLE_ITEM_EXC);

inputs = new ArrayList<>(Arrays.asList(Input.query("name", "value"), Input.query("name1", "value1")));
assertThat(new InputValidationIssue().inputs(inputs).getInputs()).isEqualTo(inputs);
}

@Test
void inputsVarargs() {
Input<?> input = Input.query("name", "value");
Input<?> input;
InputValidationIssue issue = new InputValidationIssue();
issue.setInputs(input);
assertThat(issue.getInputs()).isUnmodifiable().containsExactly(input);
assertThat(new InputValidationIssue().inputs(input).getInputs()).containsExactly(input);

assertThatIllegalArgumentException().isThrownBy(() -> issue.inputs(Input.query("name", "value"), null))
.withMessageContaining(SINGLE_ITEM_EXC);

input = Input.query("name", "value");
assertThat(new InputValidationIssue().inputs(input, input, input).getInputs()).containsExactly(input, input,
input);
}

@Test
void input() {
Input<?> input = Input.query("name", "value");
InputValidationIssue issue = new InputValidationIssue();

issue.addInput(null);
assertThat(issue.getInputs()).isEmpty();

Input<?> input = Input.query("name", "value");
issue = new InputValidationIssue();
issue.addInput(input);
assertThat(issue.getInputs()).containsExactly(input);
assertThat(new InputValidationIssue().input(input).getInputs()).containsExactly(input);
assertThat(issue.getInputs()).isEmpty();
assertThat(issue.getName()).isEqualTo(input.getName());
assertThat(issue.getIn()).isEqualTo(input.getIn());
assertThat(issue.getValue()).isEqualTo(input.getValue());

issue.addInput(input);
assertThat(issue.getName()).isNull();
assertThat(issue.getIn()).isNull();
assertThat(issue.getValue()).isNull();
assertThat(issue.getInputs()).containsExactly(input, input);
}

@Test
Expand All @@ -185,7 +234,7 @@ void equalsHashCodeToString() {
InputValidationIssue equal = new InputValidationIssue().detail("ok");
InputValidationIssue other = new InputValidationIssue().detail("other");

assertThat(issue).isEqualTo(issue);
assertThat(issue).isEqualTo(issue).hasSameHashCodeAs(issue);
assertThat(issue).hasSameHashCodeAs(issue);
assertThat(issue).isEqualTo(equal);
assertThat(issue).hasSameHashCodeAs(equal);
Expand All @@ -195,4 +244,10 @@ void equalsHashCodeToString() {
assertThat(issue).isNotEqualTo("other type");
}

private void assertMutuallyExclusiveException(ThrowableAssert.ThrowingCallable throwingCallable) {
assertThatIllegalArgumentException()
.isThrownBy(throwingCallable)
.withMessageContaining(MUTUALLY_EXCLUSIVE_EXC);
}

}
3 changes: 3 additions & 0 deletions src/main/asciidoc/release-notes.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
*belgif-rest-problem:*

* Remove @ApplicationException annotation on Problem, because it could potentially cause compilation errors when used in combination with annotation processors
* Disallow creation of InputValidationIssue with inputs[] of size 1
+
WARNING: *Potentially breaking:* removed InputValidationIssue.setInputs()

*belgif-rest-problem-java-ee:*

Expand Down

0 comments on commit 2e472da

Please sign in to comment.