Skip to content

Commit

Permalink
fix: handle iterable Criterion operandRight (#3218)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ndr-brt
ndr-brt authored Jun 23, 2023
1 parent 36aa899 commit 6afc66d
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 95 deletions.
24 changes: 12 additions & 12 deletions ...re/src/main/java/org/eclipse/edc/api/transformer/JsonObjectToCriterionDtoTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import static java.util.stream.Collectors.toList;
import static org.eclipse.edc.api.model.CriterionDto.CRITERION_OPERAND_LEFT;
import static org.eclipse.edc.api.model.CriterionDto.CRITERION_OPERAND_RIGHT;
import static org.eclipse.edc.api.model.CriterionDto.CRITERION_OPERATOR;
Expand All @@ -35,18 +36,17 @@ public JsonObjectToCriterionDtoTransformer() {
public @Nullable CriterionDto transform(@NotNull JsonObject object, @NotNull TransformerContext context) {
var builder = CriterionDto.Builder.newInstance();

visitProperties(object, k -> {
switch (k) {
case CRITERION_OPERAND_LEFT:
return v -> builder.operandLeft(transformString(v, context));
case CRITERION_OPERAND_RIGHT:
return v -> builder.operandRight(transformString(v, context));
case CRITERION_OPERATOR:
return v -> builder.operator(transformString(v, context));
default:
return doNothing();
}
});
builder.operandLeft(transformString(object.get(CRITERION_OPERAND_LEFT), context));

var operator = transformString(object.get(CRITERION_OPERATOR), context);
builder.operator(operator);

var operandRight = object.get(CRITERION_OPERAND_RIGHT);
if ("in".equals(operator)) {
builder.operandRight(operandRight.asJsonArray().stream().map(this::nodeValue).collect(toList()));
} else {
builder.operandRight(transformString(operandRight, context));
}

return builder.build();
}
Expand Down
52 changes: 41 additions & 11 deletions ...rc/test/java/org/eclipse/edc/api/transformer/JsonObjectToCriterionDtoTransformerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,21 @@

package org.eclipse.edc.api.transformer;

import jakarta.json.Json;
import jakarta.json.JsonArrayBuilder;
import jakarta.json.JsonValue;
import org.eclipse.edc.jsonld.spi.JsonLdKeywords;
import org.eclipse.edc.jsonld.transformer.to.JsonValueToGenericTypeTransformer;
import org.eclipse.edc.transform.spi.TransformerContext;
import org.junit.jupiter.api.Test;

import static jakarta.json.Json.createArrayBuilder;
import static jakarta.json.Json.createObjectBuilder;
import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.edc.api.model.CriterionDto.CRITERION_OPERAND_LEFT;
import static org.eclipse.edc.api.model.CriterionDto.CRITERION_OPERAND_RIGHT;
import static org.eclipse.edc.api.model.CriterionDto.CRITERION_OPERATOR;
import static org.eclipse.edc.api.model.CriterionDto.CRITERION_TYPE;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VALUE;
import static org.eclipse.edc.jsonld.util.JacksonJsonLd.createObjectMapper;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -36,22 +39,49 @@ class JsonObjectToCriterionDtoTransformerTest {

private final JsonObjectToCriterionDtoTransformer transformer = new JsonObjectToCriterionDtoTransformer();
private final JsonValueToGenericTypeTransformer genericTypeTransformer = new JsonValueToGenericTypeTransformer(createObjectMapper());
private final TransformerContext context = mock(TransformerContext.class);

@Test
void transform() {
when(context.transform(any(JsonValue.class), eq(Object.class))).thenAnswer(a -> genericTypeTransformer.transform(a.getArgument(0), context));
var json = createObjectBuilder()
.add(JsonLdKeywords.TYPE, CRITERION_TYPE)
.add(CRITERION_OPERAND_LEFT, value("foo"))
.add(CRITERION_OPERAND_RIGHT, value("bar"))
.add(CRITERION_OPERATOR, value("="))
.build();

var result = transformer.transform(json, context);

assertThat(result).isNotNull();
assertThat(result.getOperator()).isEqualTo("=");
assertThat(result.getOperandLeft()).isEqualTo("foo");
assertThat(result.getOperandRight()).isEqualTo("bar");
}

@Test
void transforms_shouldTransformRightOperandAsList_whenOperatorIsIn() {
var context = mock(TransformerContext.class);
when(context.transform(any(JsonValue.class), eq(Object.class))).thenAnswer(a -> genericTypeTransformer.transform(a.getArgument(0), context));
var json = Json.createObjectBuilder()
var json = createObjectBuilder()
.add(JsonLdKeywords.TYPE, CRITERION_TYPE)
.add(CRITERION_OPERAND_LEFT, "foo")
.add(CRITERION_OPERAND_RIGHT, "bar")
.add(CRITERION_OPERATOR, "=")
.add(CRITERION_OPERAND_LEFT, value("foo"))
.add(CRITERION_OPERATOR, "in")
.add(CRITERION_OPERAND_RIGHT, createArrayBuilder()
.add(createObjectBuilder().add(VALUE, "bar"))
.add(createObjectBuilder().add(VALUE, "baz"))
)
.build();

var crit = transformer.transform(json, context);
assertThat(crit).isNotNull();
assertThat(crit.getOperator()).isEqualTo("=");
assertThat(crit.getOperandLeft()).isEqualTo("foo");
assertThat(crit.getOperandRight()).isEqualTo("bar");
var result = transformer.transform(json, context);

assertThat(result).isNotNull();
assertThat(result.getOperator()).isEqualTo("in");
assertThat(result.getOperandLeft()).isEqualTo("foo");
assertThat(result.getOperandRight()).asList().containsExactly("bar", "baz");
}

private JsonArrayBuilder value(String value) {
return createArrayBuilder().add(createObjectBuilder().add(VALUE, value));
}
}
}
2 changes: 1 addition & 1 deletion spi/common/core-spi/src/main/java/org/eclipse/edc/spi/query/Criterion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) {
return false;
}
Criterion criterion = (Criterion) o;
var criterion = (Criterion) o;
return Objects.equals(operandLeft, criterion.operandLeft) && Objects.equals(operator, criterion.operator) && Objects.equals(operandRight, criterion.operandRight);
}

Expand Down
73 changes: 2 additions & 71 deletions ...d-spi/src/main/java/org/eclipse/edc/jsonld/spi/transformer/AbstractJsonLdTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,12 @@
import static jakarta.json.JsonValue.ValueType.FALSE;
import static jakarta.json.JsonValue.ValueType.NUMBER;
import static jakarta.json.JsonValue.ValueType.OBJECT;
import static jakarta.json.JsonValue.ValueType.STRING;
import static jakarta.json.JsonValue.ValueType.TRUE;
import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toList;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.ID;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.KEYWORDS;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VALUE;

/**
Expand Down Expand Up @@ -206,8 +204,7 @@ protected void visitArray(JsonValue value, Consumer<JsonValue> resultFunction, T

@Nullable
protected Object transformGenericProperty(JsonValue value, TransformerContext context) {
if (value instanceof JsonArray) {
var jsonArray = (JsonArray) value;
if (value instanceof JsonArray jsonArray) {
if (jsonArray.size() == 1) {
// unwrap array
return context.transform(jsonArray.get(0), Object.class);
Expand Down Expand Up @@ -352,8 +349,7 @@ protected boolean transformBoolean(JsonValue value, TransformerContext context)
* @param <T> the desired result type
*/
protected <T> void transformArrayOrObject(JsonValue value, Class<T> type, Consumer<T> resultFunction, TransformerContext context) {
if (value instanceof JsonArray) {
var jsonArray = (JsonArray) value;
if (value instanceof JsonArray jsonArray) {
jsonArray.stream().map(entry -> context.transform(entry, type)).forEach(resultFunction);
} else if (value instanceof JsonObject) {
var result = context.transform(value, type);
Expand Down Expand Up @@ -465,71 +461,6 @@ protected String nodeValue(JsonValue object) {
}
}

/**
* Returns the {@code @value} of the JSON object.
*/
protected String nodeValue(JsonValue value, TransformerContext context) {
if (value instanceof JsonString) {
var jsonString = (JsonString) value;
return jsonString.getString();
} else if (value instanceof JsonObject) {
var object = (JsonObject) value;
return object.getString(VALUE);
} else if (value instanceof JsonArray) {
var array = (JsonArray) value;
return nodeValue(array.get(0), context);
} else {
context.problem()
.invalidProperty()
.property(VALUE)
.value(value != null ? value.toString() : null)
.report();
return null;
}
}

/**
* Returns the {@code @type} of the JSON object. If more than one type is specified, this method will return the first.
*/
protected String nodeType(JsonObject object, TransformerContext context) {
var typeNode = object.get(TYPE);
if (typeNode == null) {
context.problem()
.missingProperty()
.property(TYPE)
.report();
return null;
}

if (typeNode instanceof JsonString) {
return ((JsonString) typeNode).getString();
} else if (typeNode instanceof JsonArray) {
var array = (JsonArray) typeNode;
if (array.isEmpty()) {
return null;
}
var typeValue = array.get(0); // a note can have more than one type, take the first
if (!(typeValue instanceof JsonString)) {
var problem = context.problem().unexpectedType().property(TYPE).expected(STRING);
if (typeValue != null) {
problem.actual(typeValue.getValueType());
}
problem.report();
return null;
}
return ((JsonString) typeValue).getString();
}

context.problem()
.unexpectedType()
.property(TYPE)
.actual(typeNode.getValueType())
.expected(STRING)
.expected(ARRAY)
.report();
return null;
}

/**
* Tries to return the instance given by a supplier (a builder's build method). If this fails due to validation errors, e.g. a required property is missing,
* reports a problem to the context.
Expand Down

0 comments on commit 6afc66d

Please sign in to comment.