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

refactor: remove unused typeAlias feature #4340

Merged
merged 1 commit into from
Jul 9, 2024
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 @@ -65,16 +65,6 @@ public ProblemBuilder problem() {
.transform(object, this);
}

@Override
public Class<?> typeAlias(String type) {
return registry.typeAlias(type);
}

@Override
public Class<?> typeAlias(String type, Class<?> defaultType) {
return registry.typeAlias(type, defaultType);
}

@Override
public void setData(Class<?> type, String key, Object value) {
data.computeIfAbsent(type, t -> new HashMap<>()).put(key, new AtomicReference<>(value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,6 @@ public <INPUT, OUTPUT> Result<OUTPUT> transform(@NotNull INPUT input, @NotNull C
}
}

@Override
public Class<?> typeAlias(String type) {
return aliases.get(type);
}

@Override
public Class<?> typeAlias(String type, Class<?> defaultType) {
return aliases.getOrDefault(type, defaultType);
}

@Override
public void registerTypeAlias(String alias, Class<?> type) {
aliases.put(alias, type);
}

private static class ContextTransformerRegistry extends TypeTransformerRegistryImpl {

private final TypeTransformerRegistry parent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

import static java.lang.String.format;
import static java.util.stream.Collectors.toList;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VALUE;

/**
Expand All @@ -43,34 +42,23 @@ public JsonValueToGenericTypeTransformer(ObjectMapper mapper) {

@Override
public Object transform(@NotNull JsonValue value, @NotNull TransformerContext context) {
if (value instanceof JsonObject) {
var object = (JsonObject) value;
if (value instanceof JsonObject object) {
if (object.containsKey(VALUE)) {
var valueField = object.get(VALUE);
if (valueField == null) {
// parse it as a generic object type
return toJavaType(object, context);
}
return transform(valueField, context);
} else if (object.containsKey(TYPE)) {
var typeValue = object.get(TYPE).asJsonArray().get(0).toString().replace("\"", "");

var typeAlias = context.typeAlias(typeValue);
if (typeAlias != null) {
// delegate back to the context, using the type alias
return context.transform(object, typeAlias);
}
context.reportProblem(format("There is no type alias registered for %s = %s", TYPE, typeValue));
} else {
return toJavaType(object, context);
}
} else if (value instanceof JsonArray) {
var jsonArray = (JsonArray) value;
} else if (value instanceof JsonArray jsonArray) {
return jsonArray.stream().map(entry -> transform(entry, context)).collect(toList());
} else if (value instanceof JsonString) {
return ((JsonString) value).getString();
} else if (value instanceof JsonNumber) {
return ((JsonNumber) value).doubleValue(); // use to double to avoid loss of precision
} else if (value instanceof JsonString jsonString) {
return jsonString.getString();
} else if (value instanceof JsonNumber jsonNumber) {
return jsonNumber.doubleValue(); // use to double to avoid loss of precision
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,25 +107,4 @@ void transform_shouldThrowException_whenInputIsNull() {
}
}

@Nested
class TypeAlias {
@Test
void typeAlias_whenNoneExists() {
assertThat(registry.typeAlias("test-alias")).isNull();
assertThat(registry.typeAlias("test-alias", String.class)).isEqualTo(String.class);
}

@Test
void typeAlias_shouldThrowException_whenNull() {
assertThat(registry.typeAlias(null)).isNull();
}

@Test
void typeAlias_whenExists() {
registry.registerTypeAlias("test-alias", String.class);
assertThat(registry.typeAlias("test-alias")).isEqualTo(String.class);
assertThat(registry.typeAlias("test-alias", Integer.class)).isEqualTo(String.class);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import static org.eclipse.edc.spi.constants.CoreConstants.EDC_NAMESPACE;
import static org.eclipse.edc.spi.constants.CoreConstants.EDC_PREFIX;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.endsWith;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -78,26 +77,25 @@ void transform_withCustomProps() {
.add("my-test-prop", "some-test-value")
.build())
.build();
json = expand(json);

var dataAddress = transformer.transform(json, transformerContext);
var dataAddress = transformer.transform(expand(json), transformerContext);

assertThat(dataAddress).isNotNull();
assertThat(dataAddress.getType()).isEqualTo(TEST_TYPE);
assertThat(dataAddress.getStringProperty(EDC_NAMESPACE + "my-test-prop")).isEqualTo("some-test-value");
}

@Test
void transform_withComplexCustomProps_shouldReportProblem() {
when(transformerContext.typeAlias(endsWith("customPayload"))).thenAnswer(i -> Payload.class);
when(transformerContext.transform(isA(JsonValue.class), eq(Payload.class))).thenReturn(new Payload(CUSTOM_PAYLOAD_NAME, CUSTOM_PAYLOAD_AGE));
var json = createDataAddress()
.add(EDC_NAMESPACE + "properties", createObjectBuilder()
.add("payload", createPayloadBuilder().build())
.build())
.build();
json = expand(json);

var dataAddress = transformer.transform(json, transformerContext);
var dataAddress = transformer.transform(expand(json), transformerContext);

assertThat(dataAddress).isNotNull();
assertThat(dataAddress.getType()).isEqualTo(TEST_TYPE);
assertThat(dataAddress.getKeyName()).isEqualTo(TEST_KEY_NAME);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

import jakarta.json.Json;
import jakarta.json.JsonArrayBuilder;
import jakarta.json.JsonNumber;
import jakarta.json.JsonObject;
import jakarta.json.JsonString;
import jakarta.json.JsonValue;
import org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset;
import org.eclipse.edc.connector.controlplane.transform.Payload;
import org.eclipse.edc.spi.types.domain.DataAddress;
Expand All @@ -29,6 +32,7 @@
import static jakarta.json.Json.createArrayBuilder;
import static jakarta.json.Json.createObjectBuilder;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.InstanceOfAssertFactories.type;
import static org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset.EDC_ASSET_DATA_ADDRESS;
import static org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset.EDC_ASSET_PRIVATE_PROPERTIES;
import static org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset.EDC_ASSET_PROPERTIES;
Expand Down Expand Up @@ -131,7 +135,17 @@ void transform_customProperties_withCustomObject() {
var jsonObject = transformer.transform(asset, context);

assertThat(jsonObject).isNotNull();
assertThat(jsonObject.getJsonObject(EDC_ASSET_PROPERTIES).getJsonObject("https://foo.bar.org/schema/payload")).isInstanceOf(JsonObject.class);
assertThat(jsonObject.getJsonObject(EDC_ASSET_PROPERTIES).getJsonObject("https://foo.bar.org/schema/payload")).isInstanceOf(JsonObject.class)
.satisfies(payload -> {
assertThat(payload.get("name")).satisfies(name -> {
assertThat(name.getValueType()).isEqualTo(JsonValue.ValueType.STRING);
assertThat(name).asInstanceOf(type(JsonString.class)).extracting(JsonString::getString).isEqualTo("foo-bar");
});
assertThat(payload.get("age")).satisfies(age -> {
assertThat(age.getValueType()).isEqualTo(JsonValue.ValueType.NUMBER);
assertThat(age).asInstanceOf(type(JsonNumber.class)).extracting(JsonNumber::doubleValue).isEqualTo(42d);
});
});
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
import jakarta.json.JsonBuilderFactory;
import jakarta.json.JsonObjectBuilder;
import org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset;
import org.eclipse.edc.connector.controlplane.transform.Payload;
import org.eclipse.edc.connector.controlplane.transform.PayloadTransformer;
import org.eclipse.edc.connector.controlplane.transform.TestInput;
import org.eclipse.edc.spi.types.domain.DataAddress;
import org.eclipse.edc.transform.TypeTransformerRegistryImpl;
Expand All @@ -29,6 +27,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -45,6 +44,7 @@
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.CONTEXT;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.ID;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VALUE;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VOCAB;
import static org.eclipse.edc.jsonld.util.JacksonJsonLd.createObjectMapper;
import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat;
Expand All @@ -71,8 +71,6 @@ void setUp() {
typeTransformerRegistry.register(new JsonValueToGenericTypeTransformer(objectMapper));
typeTransformerRegistry.register(transformer);
typeTransformerRegistry.register(new JsonObjectToDataAddressTransformer());
typeTransformerRegistry.register(new PayloadTransformer());
typeTransformerRegistry.registerTypeAlias(EDC_NAMESPACE + "customPayload", Payload.class);
}

@Test
Expand Down Expand Up @@ -143,9 +141,10 @@ void transform_withCustomProperty() {
assertThat(asset).withFailMessage(asset::getFailureDetail).isSucceeded();
assertThat(asset.getContent().getProperties())
.hasSize(6)
.hasEntrySatisfying(EDC_NAMESPACE + "payload", o -> assertThat(o).isInstanceOf(Payload.class)
.hasFieldOrPropertyWithValue("age", CUSTOM_PAYLOAD_AGE)
.hasFieldOrPropertyWithValue("name", CUSTOM_PAYLOAD_NAME));
.hasEntrySatisfying(EDC_NAMESPACE + "payload", o -> assertThat(o).asInstanceOf(map(String.class, Object.class))
.containsEntry(EDC_NAMESPACE + "age", List.of(Map.of(VALUE, CUSTOM_PAYLOAD_AGE)))
.containsEntry(EDC_NAMESPACE + "name", List.of(Map.of(VALUE, CUSTOM_PAYLOAD_NAME)))
);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,6 @@ public interface TransformerContext {
*/
@Nullable <INPUT, OUTPUT> OUTPUT transform(INPUT input, Class<OUTPUT> outputType);

/**
* Returns a registered type alias for the schema type.
*/
@Nullable
default Class<?> typeAlias(String type) {
return null;
}

/**
* Returns a registered type alias for the schema type or the default alias if none is registered.
*/
default Class<?> typeAlias(String type, Class<?> defaultType) {
return defaultType;
}

/**
* Set context data to be consumed by the transformer operating on type.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,31 +58,4 @@ public interface TypeTransformerRegistry {
*/
<INPUT, OUTPUT> Result<OUTPUT> transform(@NotNull INPUT input, @NotNull Class<OUTPUT> outputType);

/**
* Returns a registered type alias for the schema type.
* <em>Optional: may not be supported by every implementation!</em>
*/
default Class<?> typeAlias(String type) {
return null;
}

/**
* Returns a registered type alias for the schema type or the default alias if none is registered.
* <em>Optional: may not be supported by every implementation!</em>
*/
default Class<?> typeAlias(String type, Class<?> defaultType) {
return defaultType;
}

/**
* Registers a type alias for a given {@code Class<?>}. That means, from that point forward a particular {@link TypeTransformer}
* can be resolved either by its {@code INPUT} and {@code OUTPUT} types, or by the alias.
* <em>Optional: may not be supported by every implementation!</em>
*
* @param alias Any arbitrary string
* @param type the class to be aliased
*/
default void registerTypeAlias(String alias, Class<?> type) {

}
}
Loading