Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate every item of QuerySpec filterExpression #3467

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -81,30 +81,63 @@ public static Builder newInstance(JsonLdPath path, JsonWalker walker) {
return new Builder(new JsonObjectValidator(path, walker));
}

public Builder verify(Function<JsonLdPath, Validator<JsonObject>> validatorProvider) {
validator.validators.add(validatorProvider.apply(validator.path));
/**
* Add a validator on the root object level.
*
* @param provider the validator provider.
* @return the builder.
*/
public Builder verify(Function<JsonLdPath, Validator<JsonObject>> provider) {
validator.validators.add(provider.apply(validator.path));
return this;
}

public Builder verify(String fieldName, Function<JsonLdPath, Validator<JsonObject>> validatorProvider) {
/**
* Add a validator on a specific field.
*
* @param fieldName the name of the field to be validated.
* @param provider the validator provider.
* @return the builder.
*/
public Builder verify(String fieldName, Function<JsonLdPath, Validator<JsonObject>> provider) {
var newPath = validator.path.append(fieldName);
validator.validators.add(validatorProvider.apply(newPath));
validator.validators.add(provider.apply(newPath));
return this;
}

public Builder verifyId(Function<JsonLdPath, Validator<JsonString>> idValidatorProvider) {
/**
* Add a validator on the @id field.
*
* @param provider the validator provider.
* @return the builder.
*/
public Builder verifyId(Function<JsonLdPath, Validator<JsonString>> provider) {
var newPath = validator.path.append(ID);
validator.validators.add(input -> idValidatorProvider.apply(newPath).validate(input.getJsonString(ID)));
validator.validators.add(input -> provider.apply(newPath).validate(input.getJsonString(ID)));
return this;
}

/**
* Add a validator on a specific nested object.
*
* @param fieldName the name of the nested object to be validated.
* @param provider the validator provider.
* @return the builder.
*/
public Builder verifyObject(String fieldName, UnaryOperator<JsonObjectValidator.Builder> provider) {
var newPath = validator.path.append(fieldName);
var builder = JsonObjectValidator.Builder.newInstance(newPath, NESTED_OBJECT);
validator.validators.add(provider.apply(builder).build());
return this;
}

/**
* Add a validator on a specific nested array.
*
* @param fieldName the name of the nested array to be validated.
* @param provider the validator provider.
* @return the builder.
*/
public Builder verifyArrayItem(String fieldName, UnaryOperator<JsonObjectValidator.Builder> provider) {
var newPath = validator.path.append(fieldName);
var builder = JsonObjectValidator.Builder.newInstance(newPath, ARRAY_ITEMS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
/**
* Extract objects from JsonObject sub-path.
*/
interface JsonWalker {
public interface JsonWalker {

/**
* Extract a {@link Stream} of {@link JsonObject} from the path passed that can then be validated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public MandatoryValue(JsonLdPath path) {
@Override
public ValidationResult validate(JsonObject input) {
return Optional.ofNullable(input.getJsonArray(path.last()))
.filter(it -> !it.isEmpty())
.map(it -> it.getJsonObject(0))
.map(it -> it.getString(VALUE))
.filter(it -> !it.isBlank())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import jakarta.json.JsonObject;
import org.eclipse.edc.validator.jsonobject.JsonLdPath;
import org.eclipse.edc.validator.jsonobject.JsonObjectValidator;
import org.eclipse.edc.validator.jsonobject.validators.MandatoryArray;
import org.eclipse.edc.validator.jsonobject.validators.MandatoryValue;
import org.eclipse.edc.validator.spi.ValidationResult;
import org.eclipse.edc.validator.spi.Validator;
Expand All @@ -40,6 +41,7 @@ public static JsonObjectValidator.Builder instance(JsonObjectValidator.Builder b
return builder
.verify(CRITERION_OPERAND_LEFT, MandatoryValue::new)
.verify(CRITERION_OPERATOR, MandatoryValue::new)
.verify(CRITERION_OPERAND_RIGHT, MandatoryArray.min(1))
.verify(OperandRightValidator::new);
}

Expand All @@ -57,9 +59,9 @@ public ValidationResult validate(JsonObject input) {
}

return Optional.ofNullable(input.getJsonArray(CRITERION_OPERAND_RIGHT))
.filter(it -> it.size() == 1)
.filter(it -> it.size() < 2)
.map(it -> ValidationResult.success())
.orElse(ValidationResult.failure(Violation.violation(format("%s cannot contain multiple values as the operator is not 'in'", path.toString()), CRITERION_OPERAND_RIGHT)));
.orElse(ValidationResult.failure(Violation.violation(format("%s cannot contain multiple values as the operator is not 'in'", path.append(CRITERION_OPERAND_RIGHT)), CRITERION_OPERAND_RIGHT)));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static JsonObjectValidator.Builder instance(JsonObjectValidator.Builder b
.verify(EDC_QUERY_SPEC_LIMIT, OptionalValueGreaterZero::new)
.verify(EDC_QUERY_SPEC_SORT_ORDER, OptionalValueSortField::new)
.verify(EDC_QUERY_SPEC_SORT_FIELD, OptionalValueNotBlank::new)
.verifyObject(EDC_QUERY_SPEC_FILTER_EXPRESSION, CriterionValidator::instance);
.verifyArrayItem(EDC_QUERY_SPEC_FILTER_EXPRESSION, CriterionValidator::instance);
}

private record OptionalValueGreaterEqualZero(JsonLdPath path) implements Validator<JsonObject> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,28 @@ void shouldFail_whenMandatoryFieldsAreMissing() {
var result = validator.validate(input);

assertThat(result).isFailed().extracting(ValidationFailure::getViolations).asInstanceOf(list(Violation.class))
.hasSize(2)
.hasSize(3)
.anySatisfy(violation -> assertThat(violation.path()).isEqualTo(CRITERION_OPERAND_LEFT))
.anySatisfy(violation -> assertThat(violation.path()).isEqualTo(CRITERION_OPERATOR));
.anySatisfy(violation -> assertThat(violation.path()).isEqualTo(CRITERION_OPERATOR))
.anySatisfy(violation -> assertThat(violation.path()).isEqualTo(CRITERION_OPERAND_RIGHT));
}

@Test
void shouldFail_whenOperandRightIsEmpty() {
var input = Json.createObjectBuilder()
.add(CRITERION_OPERAND_LEFT, value("operand left"))
.add(CRITERION_OPERATOR, value("="))
.add(CRITERION_OPERAND_RIGHT, createArrayBuilder().build())
.build();

var result = validator.validate(input);

assertThat(result).isFailed().extracting(ValidationFailure::getViolations).asInstanceOf(list(Violation.class))
.hasSize(1)
.anySatisfy(violation -> {
assertThat(violation.path()).isEqualTo(CRITERION_OPERAND_RIGHT);
assertThat(violation.message()).contains("contains '1' elements");
});
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import jakarta.json.Json;
import jakarta.json.JsonArrayBuilder;
import jakarta.json.JsonObject;
import org.eclipse.edc.spi.query.Criterion;
import org.eclipse.edc.validator.spi.ValidationFailure;
import org.eclipse.edc.validator.spi.Validator;
import org.eclipse.edc.validator.spi.Violation;
Expand Down Expand Up @@ -112,9 +113,19 @@ void shouldFail_whenSortFieldIsBlank() {
}

@Test
void shouldFail_whenFilterExpressionNotValid() {
void shouldFail_whenFilterExpressionEntryNotValid() {
var input = Json.createObjectBuilder()
.add(EDC_QUERY_SPEC_FILTER_EXPRESSION, createArrayBuilder().add(createObjectBuilder().add("a", "a")))
.add(EDC_QUERY_SPEC_FILTER_EXPRESSION, createArrayBuilder()
.add(createObjectBuilder()
.add(Criterion.CRITERION_OPERAND_LEFT, value("key"))
.add(Criterion.CRITERION_OPERATOR, value("="))
.add(Criterion.CRITERION_OPERAND_RIGHT, value("valid criterion"))
)
.add(createObjectBuilder()
.add(Criterion.CRITERION_OPERATOR, value("="))
.add(Criterion.CRITERION_OPERAND_RIGHT, value("invalid criterion"))
)
)
.build();

var result = validator.validate(input);
Expand Down
Loading