diff --git a/core/common/util/build.gradle.kts b/core/common/util/build.gradle.kts index f3eafa8f42e..6c4a550defa 100644 --- a/core/common/util/build.gradle.kts +++ b/core/common/util/build.gradle.kts @@ -18,6 +18,8 @@ plugins { } dependencies { + api(project(":spi:common:core-spi")) + testImplementation(libs.junit.pioneer) } diff --git a/core/common/util/src/main/java/org/eclipse/edc/util/reflection/ReflectionUtil.java b/core/common/util/src/main/java/org/eclipse/edc/util/reflection/ReflectionUtil.java index 1cf0c39c381..68facfb6885 100644 --- a/core/common/util/src/main/java/org/eclipse/edc/util/reflection/ReflectionUtil.java +++ b/core/common/util/src/main/java/org/eclipse/edc/util/reflection/ReflectionUtil.java @@ -14,6 +14,8 @@ package org.eclipse.edc.util.reflection; +import org.eclipse.edc.spi.types.PathItem; + import java.lang.reflect.Field; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; @@ -47,29 +49,34 @@ public static T getFieldValue(String propertyName, Object object) { Objects.requireNonNull(propertyName, "propertyName"); Objects.requireNonNull(object, "object"); - if (propertyName.contains(".")) { - var dotIx = propertyName.indexOf("."); - var field = propertyName.substring(0, dotIx); - var rest = propertyName.substring(dotIx + 1); - object = getFieldValue(field, object); - if (object == null) { + var path = PathItem.parse(propertyName); + return getFieldValue(path, object); + } + + private static T getFieldValue(List path, Object object) { + var first = path.get(0); + + if (path.size() > 1) { + var nested = getFieldValue(List.of(first), object); + if (nested == null) { return null; } - return getFieldValue(rest, object); - } else if (propertyName.matches(ARRAY_INDEXER_REGEX)) { //array indexer - var openingBracketIx = propertyName.indexOf(OPENING_BRACKET); - var closingBracketIx = propertyName.indexOf(CLOSING_BRACKET); - var propName = propertyName.substring(0, openingBracketIx); - var arrayIndex = Integer.parseInt(propertyName.substring(openingBracketIx + 1, closingBracketIx)); + var rest = path.stream().skip(1).toList(); + return getFieldValue(rest, nested); + } else if (first.toString().matches(ARRAY_INDEXER_REGEX)) { //array indexer + var openingBracketIx = first.toString().indexOf(OPENING_BRACKET); + var closingBracketIx = first.toString().indexOf(CLOSING_BRACKET); + var propName = first.toString().substring(0, openingBracketIx); + var arrayIndex = Integer.parseInt(first.toString().substring(openingBracketIx + 1, closingBracketIx)); var iterableObject = (List) getFieldValue(propName, object); return (T) iterableObject.get(arrayIndex); } else { if (object instanceof Map map) { - return (T) map.get(propertyName); + return (T) map.get(first.toString()); } else if (object instanceof List list) { - return (T) list.stream().filter(Objects::nonNull).map(it -> getRecursiveValue(propertyName, it)).toList(); + return (T) list.stream().filter(Objects::nonNull).map(it -> getRecursiveValue(first.toString(), it)).toList(); } else { - return getRecursiveValue(propertyName, object); + return getRecursiveValue(first.toString(), object); } } } diff --git a/core/common/util/src/test/java/org/eclipse/edc/util/reflection/PathItemTest.java b/core/common/util/src/test/java/org/eclipse/edc/util/reflection/PathItemTest.java new file mode 100644 index 00000000000..2844aa49310 --- /dev/null +++ b/core/common/util/src/test/java/org/eclipse/edc/util/reflection/PathItemTest.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2023 Bayerische Motoren Werke Aktiengesellschaft (BMW AG) + * + * This program and the accompanying materials are made available under the + * terms of the Apache License, Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + * + * Contributors: + * Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation + * + */ + +package org.eclipse.edc.util.reflection; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class PathItemTest { + + @Test + void shouldParse_singleElement() { + var result = PathItem.parse("test"); + + assertThat(result).hasSize(1).first().extracting(PathItem::toString).isEqualTo("test"); + } + + @Test + void shouldParse_singleWrappedElement() { + var result = PathItem.parse("'test.data'"); + + assertThat(result).hasSize(1).first().extracting(PathItem::toString).isEqualTo("test.data"); + } + + @Test + void shouldParse_multipleSeparatedByDots() { + var result = PathItem.parse("test.data.path"); + + assertThat(result).hasSize(3).extracting(PathItem::toString) + .containsExactly("test", "data", "path"); + } + + @Test + void shouldParse_multipleWrappedSeparatedByDots() { + var result = PathItem.parse("'test.test'.data.'path.path'"); + + assertThat(result).hasSize(3).extracting(PathItem::toString) + .containsExactly("test.test", "data", "path.path"); + } +} diff --git a/core/common/util/src/test/java/org/eclipse/edc/util/reflection/ReflectionUtilTest.java b/core/common/util/src/test/java/org/eclipse/edc/util/reflection/ReflectionUtilTest.java index 72ecb1dc1f7..c054e26c14d 100644 --- a/core/common/util/src/test/java/org/eclipse/edc/util/reflection/ReflectionUtilTest.java +++ b/core/common/util/src/test/java/org/eclipse/edc/util/reflection/ReflectionUtilTest.java @@ -15,6 +15,7 @@ package org.eclipse.edc.util.reflection; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import java.lang.reflect.Field; @@ -27,154 +28,177 @@ class ReflectionUtilTest { - @Test - void getFieldValue() { - var value = ReflectionUtil.getFieldValue("description", new TestObject("test-desc", 1)); - assertThat(value).isInstanceOf(String.class).isEqualTo("test-desc"); + @Nested + class GetFieldValue { + + @Test + void getFieldValue() { + var value = ReflectionUtil.getFieldValue("description", new TestObject("test-desc", 1)); + assertThat(value).isInstanceOf(String.class).isEqualTo("test-desc"); - var value2 = ReflectionUtil.getFieldValue("priority", new TestObject("test-desc", 1)); - assertThat(value2).isInstanceOf(Integer.class).isEqualTo(1); - } + var value2 = ReflectionUtil.getFieldValue("priority", new TestObject("test-desc", 1)); + assertThat(value2).isInstanceOf(Integer.class).isEqualTo(1); + } - @Test - void getFieldValue_isNull() { - var value = ReflectionUtil.getFieldValue("description", new TestObject(null, 1)); - assertThat(value).isNull(); - } + @Test + void getFieldValue_isNull() { + var value = ReflectionUtil.getFieldValue("description", new TestObject(null, 1)); + assertThat(value).isNull(); + } - @Test - void getFieldValue_notExist() { - assertThatThrownBy(() -> ReflectionUtil.getFieldValue("notExist", new TestObject("test-desc", 1))) - .isInstanceOf(ReflectionException.class); - } + @Test + void getFieldValue_notExist() { + assertThatThrownBy(() -> ReflectionUtil.getFieldValue("notExist", new TestObject("test-desc", 1))) + .isInstanceOf(ReflectionException.class); + } - @Test - void getFieldValue_invalidArgs() { + @Test + void getFieldValue_invalidArgs() { - assertThatThrownBy(() -> ReflectionUtil.getFieldValue("", new TestObject("test-desc", 1))) - .isInstanceOf(ReflectionException.class); + assertThatThrownBy(() -> ReflectionUtil.getFieldValue("", new TestObject("test-desc", 1))) + .isInstanceOf(ReflectionException.class); - assertThatThrownBy(() -> ReflectionUtil.getFieldValue(null, new TestObject("test-desc", 1))) - .isInstanceOf(NullPointerException.class).hasMessage("propertyName"); + assertThatThrownBy(() -> ReflectionUtil.getFieldValue((String) null, new TestObject("test-desc", 1))) + .isInstanceOf(NullPointerException.class).hasMessage("propertyName"); - assertThatThrownBy(() -> ReflectionUtil.getFieldValue("description", null)) - .isInstanceOf(NullPointerException.class).hasMessage("object"); - } + assertThatThrownBy(() -> ReflectionUtil.getFieldValue("description", null)) + .isInstanceOf(NullPointerException.class).hasMessage("object"); + } - @Test - void getFieldValue_fromMap() { - var value = ReflectionUtil.getFieldValue("key", Map.of("key", "value")); + @Test + void getFieldValue_fromMap() { + var value = ReflectionUtil.getFieldValue("key", Map.of("key", "value")); - assertThat(value).isEqualTo("value"); - } + assertThat(value).isEqualTo("value"); + } - @Test - void getAllFieldsRecursive() { - var to = new TestObjectSubclass("test-desc", 1, "foobar"); + @Test + void getFieldValue_whenParentExist() { + var to = new TestObjectSubSubclass("test-desc", 1, "foobar"); + to.setAnotherObject(new AnotherObject("another-desc")); - assertThat(ReflectionUtil.getAllFieldsRecursive(to.getClass())).hasSize(5).extracting(Field::getName) - .containsExactlyInAnyOrder("description", "priority", "testProperty", "listField", "embedded"); - } + String fieldValue = ReflectionUtil.getFieldValue("anotherObject.anotherDescription", to); + assertThat(fieldValue).isEqualTo("another-desc"); + } - @Test - void getFieldRecursive_whenDeclaredInSuperclass() { - var to = new TestObjectSubclass("test-desc", 1, "foobar"); - assertThat(ReflectionUtil.getFieldRecursive(to.getClass(), "description")).isNotNull(); - } + @Test + void getFieldValue_whenParentNotExist() { + var to = new TestObjectSubSubclass("test-desc", 1, "foobar"); + to.setAnotherObject(null); - @Test - void getFieldRecursive_whenDeclaredInClass() { - var to = new TestObjectSubclass("test-desc", 1, "foobar"); + String fieldValue = ReflectionUtil.getFieldValue("anotherObject.anotherDescription", to); + assertThat(fieldValue).isNull(); + } - var testProperty = ReflectionUtil.getFieldRecursive(to.getClass(), "testProperty"); + @Test + void getFieldValue_withArrayIndex() { + var to1 = new TestObject("to1", 420); + var o = new TestObjectWithList("test-desc", 0, List.of(to1, new TestObject("to2", 69))); + assertThat((TestObject) ReflectionUtil.getFieldValue("nestedObjects[0]", o)).isEqualTo(to1); + } - assertThat(testProperty).isNotNull(); - } + @Test + void getFieldValue_arrayWithoutIndex() { + var nestedObjects = List.of( + new TestObject("to1", 420), + new TestObject("to2", 69) + ); + var object = new TestObjectWithList("test-desc", 0, nestedObjects); - @Test - @DisplayName("Should pick the property from the object highest in the object hierarchy") - void getFieldRecursive_whenDeclaredInBoth() { - var to = new TestObjectSubSubclass("test-desc", 2, "foobar"); - assertThat(ReflectionUtil.getFieldRecursive(to.getClass(), "description")).isNotNull().extracting(Field::getDeclaringClass) - .isEqualTo(TestObject.class); - } + var result = ReflectionUtil.getFieldValue("nestedObjects.description", object); - @Test - void getFieldRecursive_whenNotDeclared() { - var to = new TestObjectSubclass("test-desc", 1, "foobar"); - assertThat(ReflectionUtil.getFieldRecursive(to.getClass(), "notExist")).isNull(); - } + assertThat(result).isEqualTo(List.of("to1", "to2")); + } - @Test - void getFieldValue_whenParentExist() { - var to = new TestObjectSubSubclass("test-desc", 1, "foobar"); - to.setAnotherObject(new AnotherObject("another-desc")); + @Test + void getFieldValue_withArrayIndex_andDotAccess() { + var to1 = new TestObject("to1", 420); + var o = new TestObjectWithList("test-desc", 0, List.of(to1, new TestObject("to2", 69))); - String fieldValue = ReflectionUtil.getFieldValue("anotherObject.anotherDescription", to); - assertThat(fieldValue).isEqualTo("another-desc"); - } + var fieldValue = ReflectionUtil.getFieldValue("nestedObjects[0].priority", o); - @Test - void getFieldValue_whenParentNotExist() { - var to = new TestObjectSubSubclass("test-desc", 1, "foobar"); - to.setAnotherObject(null); + assertThat(fieldValue).isEqualTo(420); + } - String fieldValue = ReflectionUtil.getFieldValue("anotherObject.anotherDescription", to); - assertThat(fieldValue).isNull(); - } + @Test + void getFieldValue_withArrayIndex_outOfBounds() { + var o = new TestObjectWithList("test-desc", 0, List.of(new TestObject("to1", 420), new TestObject("to2", 69))); + assertThatThrownBy(() -> ReflectionUtil.getFieldValue("nestedObjects[3]", o)).isInstanceOf(IndexOutOfBoundsException.class); + } - @Test - void getFieldValue_withArrayIndex() { - var to1 = new TestObject("to1", 420); - var o = new TestObjectWithList("test-desc", 0, List.of(to1, new TestObject("to2", 69))); - assertThat((TestObject) ReflectionUtil.getFieldValue("nestedObjects[0]", o)).isEqualTo(to1); - } + @Test + void shouldGetNestedValue_whenKeyContainsDot() { + var object = Map.of("http://namespace.domain/nested", Map.of("http://namespace.domain/key", "value")); - @Test - void getFieldValue_arrayWithoutIndex() { - var nestedObjects = List.of( - new TestObject("to1", 420), - new TestObject("to2", 69) - ); - var object = new TestObjectWithList("test-desc", 0, nestedObjects); + var value = ReflectionUtil.getFieldValue("'http://namespace.domain/nested'.'http://namespace.domain/key'", object); - var result = ReflectionUtil.getFieldValue("nestedObjects.description", object); + assertThat(value).isInstanceOf(String.class).isEqualTo("value"); + } - assertThat(result).isEqualTo(List.of("to1", "to2")); } - @Test - void getFieldValue_withArrayIndex_andDotAccess() { - var to1 = new TestObject("to1", 420); - var o = new TestObjectWithList("test-desc", 0, List.of(to1, new TestObject("to2", 69))); + @Nested + class GetFieldRecursive { - var fieldValue = ReflectionUtil.getFieldValue("nestedObjects[0].priority", o); + @Test + void getAllFieldsRecursive() { + var to = new TestObjectSubclass("test-desc", 1, "foobar"); - assertThat(fieldValue).isEqualTo(420); - } + assertThat(ReflectionUtil.getAllFieldsRecursive(to.getClass())).hasSize(5).extracting(Field::getName) + .containsExactlyInAnyOrder("description", "priority", "testProperty", "listField", "embedded"); + } - @Test - void getFieldValue_withArrayIndex_outOfBounds() { - var o = new TestObjectWithList("test-desc", 0, List.of(new TestObject("to1", 420), new TestObject("to2", 69))); - assertThatThrownBy(() -> ReflectionUtil.getFieldValue("nestedObjects[3]", o)).isInstanceOf(IndexOutOfBoundsException.class); - } + @Test + void getFieldRecursive_whenDeclaredInSuperclass() { + var to = new TestObjectSubclass("test-desc", 1, "foobar"); + assertThat(ReflectionUtil.getFieldRecursive(to.getClass(), "description")).isNotNull(); + } - @Test - void getSingleSuperTypeGenericArgument() { - var fields = ReflectionUtil.getSingleSuperTypeGenericArgument(TestGenericSubclass.class, TestGenericObject.class); - assertThat(fields).isEqualTo(String.class); - } + @Test + void getFieldRecursive_whenDeclaredInClass() { + var to = new TestObjectSubclass("test-desc", 1, "foobar"); + + var testProperty = ReflectionUtil.getFieldRecursive(to.getClass(), "testProperty"); + + assertThat(testProperty).isNotNull(); + } + + @Test + @DisplayName("Should pick the property from the object highest in the object hierarchy") + void getFieldRecursive_whenDeclaredInBoth() { + var to = new TestObjectSubSubclass("test-desc", 2, "foobar"); + assertThat(ReflectionUtil.getFieldRecursive(to.getClass(), "description")).isNotNull().extracting(Field::getDeclaringClass) + .isEqualTo(TestObject.class); + } + + @Test + void getFieldRecursive_whenNotDeclared() { + var to = new TestObjectSubclass("test-desc", 1, "foobar"); + assertThat(ReflectionUtil.getFieldRecursive(to.getClass(), "notExist")).isNull(); + } - @Test - void getSingleSuperTypeGenericArgument_whenNoGenericSuperclass() { - var fields = ReflectionUtil.getSingleSuperTypeGenericArgument(TestObjectWithList.class, TestObject.class); - assertThat(fields).isNull(); } - @Test - void getSingleSuperTypeGenericArgument_whenGenericClass() { - var genericList = new TestGenericArrayList(); - var fields = ReflectionUtil.getSingleSuperTypeGenericArgument(genericList.getClass(), ArrayList.class); - assertThat(fields).isNull(); + @Nested + class GetSingleSuperTypeGenericArgument { + @Test + void getSingleSuperTypeGenericArgument() { + var fields = ReflectionUtil.getSingleSuperTypeGenericArgument(TestGenericSubclass.class, TestGenericObject.class); + assertThat(fields).isEqualTo(String.class); + } + + @Test + void getSingleSuperTypeGenericArgument_whenNoGenericSuperclass() { + var fields = ReflectionUtil.getSingleSuperTypeGenericArgument(TestObjectWithList.class, TestObject.class); + assertThat(fields).isNull(); + } + + @Test + void getSingleSuperTypeGenericArgument_whenGenericClass() { + var genericList = new TestGenericArrayList(); + var fields = ReflectionUtil.getSingleSuperTypeGenericArgument(genericList.getClass(), ArrayList.class); + assertThat(fields).isNull(); + } } + } diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/service/asset/AssetServiceImpl.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/service/asset/AssetServiceImpl.java index 181ecb187f3..75063fb0762 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/service/asset/AssetServiceImpl.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/service/asset/AssetServiceImpl.java @@ -23,7 +23,6 @@ import org.eclipse.edc.spi.dataaddress.DataAddressValidator; import org.eclipse.edc.spi.query.Criterion; import org.eclipse.edc.spi.query.QuerySpec; -import org.eclipse.edc.spi.types.domain.DataAddress; import org.eclipse.edc.spi.types.domain.asset.Asset; import org.eclipse.edc.transaction.spi.TransactionContext; @@ -124,12 +123,4 @@ public ServiceResult update(Asset asset) { }); } - @Override - public ServiceResult update(String assetId, DataAddress dataAddress) { - return transactionContext.execute(() -> { - var result = index.updateDataAddress(assetId, dataAddress); - result.onSuccess(da -> observable.invokeForEach(l -> l.updated(findById(assetId)))); - return ServiceResult.from(result); - }); - } } diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/service/asset/AssetServiceImplTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/service/asset/AssetServiceImplTest.java index c6501e01da2..de9b2be6cc2 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/service/asset/AssetServiceImplTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/service/asset/AssetServiceImplTest.java @@ -290,36 +290,6 @@ void updateAsset_shouldFail_whenPropertiesAreDuplicated() { verifyNoInteractions(index); } - @Test - void updateDataAddress_shouldUpdateWhenExists() { - when(dataAddressValidator.validate(any())).thenReturn(Result.success()); - var assetId = "assetId"; - var asset = createAsset(assetId); - var dataAddress = DataAddress.Builder.newInstance().type("test-type").build(); - when(index.updateDataAddress(assetId, dataAddress)).thenReturn(StoreResult.success(dataAddress)); - - var updated = service.update(assetId, dataAddress); - - assertThat(updated.succeeded()).isTrue(); - verify(index).updateDataAddress(eq(assetId), eq(dataAddress)); - verify(observable).invokeForEach(any()); - } - - @Test - void updateDataAddress_shouldReturnNotFound_whenNotExists() { - var assetId = "assetId"; - var dataAddress = DataAddress.Builder.newInstance().type("test-type").build(); - when(index.updateDataAddress(assetId, dataAddress)).thenReturn(StoreResult.notFound("test")); - - var updated = service.update(assetId, dataAddress); - - assertThat(updated.failed()).isTrue(); - assertThat(updated.reason()).isEqualTo(NOT_FOUND); - verify(index).updateDataAddress(eq(assetId), eq(dataAddress)); - verifyNoMoreInteractions(index); - verify(observable, never()).invokeForEach(any()); - } - private static class InvalidFilters implements ArgumentsProvider { @Override public Stream provideArguments(ExtensionContext context) { diff --git a/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/asset/CriterionToAssetPredicateConverterImpl.java b/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/asset/CriterionToAssetPredicateConverterImpl.java index 5a9bad4f938..36ac6fc64f7 100644 --- a/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/asset/CriterionToAssetPredicateConverterImpl.java +++ b/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/asset/CriterionToAssetPredicateConverterImpl.java @@ -26,14 +26,17 @@ public class CriterionToAssetPredicateConverterImpl extends CriterionToPredicate public Object property(String key, Object object) { if (object instanceof Asset asset) { - if (asset.getProperties().containsKey(key)) { - return asset.getProperty(key); + var property = super.property(key, asset.getProperties()); + if (property != null) { + return property; } - if (asset.getPrivateProperties().containsKey(key)) { - return asset.getPrivateProperty(key); + + var privateProperty = super.property(key, asset.getPrivateProperties()); + if (privateProperty != null) { + return privateProperty; } - return super.property(key, object); + return super.property(key, asset); } throw new IllegalArgumentException("Can only handle objects of type " + Asset.class.getSimpleName() + " but received an " + object.getClass().getSimpleName()); } diff --git a/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/defaults/storage/assetindex/InMemoryAssetIndex.java b/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/defaults/storage/assetindex/InMemoryAssetIndex.java index 5e86214faab..225ce53f103 100644 --- a/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/defaults/storage/assetindex/InMemoryAssetIndex.java +++ b/core/control-plane/control-plane-core/src/main/java/org/eclipse/edc/connector/defaults/storage/assetindex/InMemoryAssetIndex.java @@ -129,22 +129,6 @@ public StoreResult updateAsset(Asset asset) { } } - @Override - public StoreResult updateDataAddress(String assetId, DataAddress dataAddress) { - lock.writeLock().lock(); - try { - Objects.requireNonNull(dataAddress, "dataAddress"); - Objects.requireNonNull(assetId, "asset.getId()"); - if (dataAddresses.containsKey(assetId)) { - dataAddresses.put(assetId, dataAddress); - return StoreResult.success(dataAddress); - } - return StoreResult.notFound(format(DATA_ADDRESS_NOT_FOUND_TEMPLATE, assetId)); - } finally { - lock.writeLock().unlock(); - } - } - @Override public DataAddress resolveForAsset(String assetId) { Objects.requireNonNull(assetId, "assetId"); diff --git a/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/JsonFieldMapping.java b/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/JsonFieldMapping.java index ab3624b1179..95900f4837f 100644 --- a/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/JsonFieldMapping.java +++ b/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/JsonFieldMapping.java @@ -14,6 +14,10 @@ package org.eclipse.edc.sql.translation; +import org.eclipse.edc.spi.types.PathItem; + +import java.util.List; + import static java.lang.String.format; public class JsonFieldMapping extends TranslationMapping { @@ -24,18 +28,17 @@ public JsonFieldMapping(String columnName) { } @Override - public String getStatement(String canonicalPropertyName, Class type) { - var tokens = canonicalPropertyName.split("\\."); - + public String getStatement(List path, Class type) { var statementBuilder = new StringBuilder(columnName); - var length = tokens.length; + + var length = path.size(); for (var i = 0; i < length - 1; i++) { statementBuilder.append(" -> "); - statementBuilder.append("'").append(tokens[i]).append("'"); + statementBuilder.append("'").append(path.get(i)).append("'"); } statementBuilder.append(" ->> "); - statementBuilder.append("'").append(tokens[length - 1]).append("'"); + statementBuilder.append("'").append(path.get(length - 1)).append("'"); var statement = statementBuilder.toString(); if (type.equals(Boolean.class)) { diff --git a/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/TranslationMapping.java b/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/TranslationMapping.java index be8734d388b..c956397fc6d 100644 --- a/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/TranslationMapping.java +++ b/extensions/common/sql/sql-core/src/main/java/org/eclipse/edc/sql/translation/TranslationMapping.java @@ -14,7 +14,10 @@ package org.eclipse.edc.sql.translation; +import org.eclipse.edc.spi.types.PathItem; + import java.util.HashMap; +import java.util.List; import java.util.Map; import static java.lang.String.format; @@ -38,25 +41,27 @@ public abstract class TranslationMapping { */ public String getStatement(String canonicalPropertyName, Class type) { if (canonicalPropertyName == null) { - throw new IllegalArgumentException(format("Translation failed for Model '%s' input canonicalPropertyName is null", getClass().getName())); + throw new IllegalArgumentException(format("Translation failed for Model '%s' input path is null", getClass().getName())); } - var leftHandTokens = canonicalPropertyName.split("\\.", 2); - var key = leftHandTokens[0]; - var entry = fieldMap.get(key); + return getStatement(PathItem.parse(canonicalPropertyName), type); + } + + public String getStatement(List path, Class type) { + var key = path.get(0); + var entry = fieldMap.get(key.toString()); if (entry == null) { return null; } + if (entry instanceof TranslationMapping mappingEntry) { - var nextToken = leftHandTokens.length < 2 ? null : leftHandTokens[1]; - //recursively descend into the metamodel tree - return mappingEntry.getStatement(nextToken, type); + var remainingPath = path.stream().skip(1).toList(); + return mappingEntry.getStatement(remainingPath, type); } return entry.toString(); } - protected void add(String fieldId, Object value) { fieldMap.put(fieldId, value); } diff --git a/extensions/common/sql/sql-core/src/testFixtures/java/org/eclipse/edc/sql/testfixtures/PostgresqlLocalInstance.java b/extensions/common/sql/sql-core/src/testFixtures/java/org/eclipse/edc/sql/testfixtures/PostgresqlLocalInstance.java index f2158aef69d..4aec8447673 100644 --- a/extensions/common/sql/sql-core/src/testFixtures/java/org/eclipse/edc/sql/testfixtures/PostgresqlLocalInstance.java +++ b/extensions/common/sql/sql-core/src/testFixtures/java/org/eclipse/edc/sql/testfixtures/PostgresqlLocalInstance.java @@ -56,18 +56,6 @@ public Connection getTestConnection(String hostName, int port, String dbName) { } } - public Connection getConnection() { - try { - return DriverManager.getConnection(jdbcUrlPrefix, username, password); - } catch (SQLException e) { - throw new RuntimeException(e); - } - } - - public String getJdbcUrlPrefix() { - return jdbcUrlPrefix; - } - private DataSource createTestDataSource(String hostName, int port, String dbName) { var dataSource = new PGSimpleDataSource(); dataSource.setServerNames(new String[]{ hostName }); diff --git a/extensions/control-plane/store/sql/asset-index-sql/README.md b/extensions/control-plane/store/sql/asset-index-sql/README.md index 40109204ce8..ea4e9bb4f75 100644 --- a/extensions/control-plane/store/sql/asset-index-sql/README.md +++ b/extensions/control-plane/store/sql/asset-index-sql/README.md @@ -42,3 +42,25 @@ edc_asset ||--o{ edc_asset_property | Key | Description | Mandatory | |:---|:---|---| | edc.datasource.asset.name | Datasource used by this extension | X | + +## Migrate from 0.3.1 to 0.3.2 + +This table structure has been changed, from 3 tables (one for the asset, one for properties and one for data address) to +a single table with json fields. +To migrate an existing database, please first add the 3 new columns in the `edc_asset` table: +```sql +alter table edc_asset +add column properties json default '{}', +add column private_properties json default '{}', +add column data_address json default '{}'; +``` + +Then migrate the existing data in the new format: +```sql +update edc_asset set +properties=(select json_object_agg(property_name, property_value) from edc_asset_property where asset_id_fk = edc_asset.asset_id and property_is_private = false), +private_properties=(select json_object_agg(property_name, property_value) from edc_asset_property where asset_id_fk = edc_asset.asset_id and property_is_private = true), +data_address=(select properties from edc_asset_dataaddress where asset_id_fk = edc_asset.asset_id); +``` + +After the migration, the tables `edc_asset_dataaddress` and `edc_asset_property` can be deleted. diff --git a/extensions/control-plane/store/sql/asset-index-sql/docs/schema.sql b/extensions/control-plane/store/sql/asset-index-sql/docs/schema.sql index 3c166451de8..6274b5f88a2 100644 --- a/extensions/control-plane/store/sql/asset-index-sql/docs/schema.sql +++ b/extensions/control-plane/store/sql/asset-index-sql/docs/schema.sql @@ -1,5 +1,5 @@ -- --- Copyright (c) 2022 Daimler TSS GmbH +-- Copyright (c) 2022 - 2023 Daimler TSS GmbH -- -- This program and the accompanying materials are made available under the -- terms of the Apache License, Version 2.0 which is available at @@ -9,6 +9,7 @@ -- -- Contributors: -- Daimler TSS GmbH - Initial SQL Query +-- Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - improvements -- -- THIS SCHEMA HAS BEEN WRITTEN AND TESTED ONLY FOR POSTGRES @@ -16,41 +17,14 @@ -- table: edc_asset CREATE TABLE IF NOT EXISTS edc_asset ( - asset_id VARCHAR NOT NULL, - created_at BIGINT NOT NULL, + asset_id VARCHAR NOT NULL, + created_at BIGINT NOT NULL, + properties JSON DEFAULT '{}', + private_properties JSON DEFAULT '{}', + data_address JSON DEFAULT '{}', PRIMARY KEY (asset_id) ); --- table: edc_asset_dataaddress -CREATE TABLE IF NOT EXISTS edc_asset_dataaddress -( - asset_id_fk VARCHAR NOT NULL, - properties JSON NOT NULL, - PRIMARY KEY (asset_id_fk), - FOREIGN KEY (asset_id_fk) REFERENCES edc_asset (asset_id) ON DELETE CASCADE -); -COMMENT ON COLUMN edc_asset_dataaddress.properties IS 'DataAddress properties serialized as JSON'; - --- table: edc_asset_property -CREATE TABLE IF NOT EXISTS edc_asset_property -( - asset_id_fk VARCHAR NOT NULL, - property_name VARCHAR NOT NULL, - property_value VARCHAR NOT NULL, - property_type VARCHAR NOT NULL, - property_is_private BOOLEAN NOT NULL, - PRIMARY KEY (asset_id_fk, property_name), - FOREIGN KEY (asset_id_fk) REFERENCES edc_asset (asset_id) ON DELETE CASCADE -); - -COMMENT ON COLUMN edc_asset_property.property_name IS - 'Asset property key'; -COMMENT ON COLUMN edc_asset_property.property_value IS - 'Asset property value'; -COMMENT ON COLUMN edc_asset_property.property_type IS - 'Asset property class name'; -COMMENT ON COLUMN edc_asset_property.property_is_private IS - 'Asset property private flag'; - -CREATE INDEX IF NOT EXISTS idx_edc_asset_property_value - ON edc_asset_property (property_name, property_value); \ No newline at end of file +COMMENT ON COLUMN edc_asset.properties IS 'Asset properties serialized as JSON'; +COMMENT ON COLUMN edc_asset.private_properties IS 'Asset private properties serialized as JSON'; +COMMENT ON COLUMN edc_asset.data_address IS 'Asset DataAddress serialized as JSON'; diff --git a/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/SqlAssetIndex.java b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/SqlAssetIndex.java index 3e39a0aa53c..c13ddc5005d 100644 --- a/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/SqlAssetIndex.java +++ b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/SqlAssetIndex.java @@ -16,7 +16,6 @@ package org.eclipse.edc.connector.store.sql.assetindex; -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import org.eclipse.edc.connector.store.sql.assetindex.schema.AssetStatements; import org.eclipse.edc.spi.asset.AssetIndex; @@ -35,14 +34,13 @@ import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; -import java.util.AbstractMap; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.stream.Stream; import static java.lang.String.format; -import static java.util.stream.Collectors.partitioningBy; -import static java.util.stream.Collectors.toMap; +import static org.eclipse.edc.spi.query.Criterion.criterion; public class SqlAssetIndex extends AbstractSqlStore implements AssetIndex { @@ -61,9 +59,7 @@ public Stream queryAssets(QuerySpec querySpec) { return transactionContext.execute(() -> { try { var statement = assetStatements.createQuery(querySpec); - - return queryExecutor.query(getConnection(), true, this::mapAssetIds, statement.getQueryAsString(), statement.getParameters()) - .map(this::findById); + return queryExecutor.query(getConnection(), true, this::mapAsset, statement.getQueryAsString(), statement.getParameters()); } catch (SQLException e) { throw new EdcPersistenceException(e); } @@ -75,39 +71,12 @@ public Stream queryAssets(QuerySpec querySpec) { Objects.requireNonNull(assetId); try (var connection = getConnection()) { - - return transactionContext.execute(() -> { - if (!existsById(assetId, connection)) { - return null; - } - - var selectAssetByIdSql = assetStatements.getSelectAssetByIdTemplate(); - var findPropertyByIdSql = assetStatements.getFindPropertyByIdTemplate(); - try ( - var createdAtStream = queryExecutor.query(connection, false, this::mapCreatedAt, selectAssetByIdSql, assetId); - var allPropertiesStream = queryExecutor.query(connection, false, this::mapPropertyResultSet, findPropertyByIdSql, assetId) - ) { - var createdAt = createdAtStream.findFirst().orElse(0L); - var groupedProperties = allPropertiesStream.collect(partitioningBy(SqlPropertyWrapper::isPrivate)); - var assetProperties = groupedProperties.get(false).stream().collect(toMap(SqlPropertyWrapper::getPropertyKey, SqlPropertyWrapper::getPropertyValue)); - var assetPrivateProperties = groupedProperties.get(true).stream().collect(toMap(SqlPropertyWrapper::getPropertyKey, SqlPropertyWrapper::getPropertyValue)); - var dataAddress = resolveForAsset(assetId); - return Asset.Builder.newInstance() - .id(assetId) - .properties(assetProperties) - .privateProperties(assetPrivateProperties) - .createdAt(createdAt) - .dataAddress(dataAddress) - .build(); - } - }); - - } catch (Exception e) { - if (e instanceof EdcPersistenceException) { - throw (EdcPersistenceException) e; - } else { - throw new EdcPersistenceException(e.getMessage(), e); - } + var querySpec = QuerySpec.Builder.newInstance().filter(criterion("id", "=", assetId)).build(); + var statement = assetStatements.createQuery(querySpec); + return queryExecutor.query(connection, true, this::mapAsset, statement.getQueryAsString(), statement.getParameters()) + .findFirst().orElse(null); + } catch (SQLException e) { + throw new EdcPersistenceException(e); } } @@ -126,11 +95,13 @@ public StoreResult create(Asset asset) { return StoreResult.alreadyExists(msg); } - queryExecutor.execute(connection, assetStatements.getInsertAssetTemplate(), assetId, asset.getCreatedAt()); - var insertDataAddressTemplate = assetStatements.getInsertDataAddressTemplate(); - queryExecutor.execute(connection, insertDataAddressTemplate, assetId, toJson(dataAddress.getProperties())); - - insertProperties(asset, assetId, connection); + queryExecutor.execute(connection, assetStatements.getInsertAssetTemplate(), + assetId, + asset.getCreatedAt(), + toJson(asset.getProperties()), + toJson(asset.getPrivateProperties()), + toJson(asset.getDataAddress().getProperties()) + ); return StoreResult.success(); } catch (Exception e) { @@ -178,28 +149,15 @@ public StoreResult updateAsset(Asset asset) { try (var connection = getConnection()) { var assetId = asset.getId(); if (existsById(assetId, connection)) { - queryExecutor.execute(connection, assetStatements.getDeletePropertyByIdTemplate(), assetId); - insertProperties(asset, assetId, connection); - var updateTemplate = assetStatements.getUpdateDataAddressTemplate(); - queryExecutor.execute(connection, updateTemplate, toJson(asset.getDataAddress().getProperties()), assetId); - return StoreResult.success(asset); - } - return StoreResult.notFound(format(ASSET_NOT_FOUND_TEMPLATE, assetId)); - } catch (Exception e) { - throw new EdcPersistenceException(e); - } - }); - } + queryExecutor.execute(connection, assetStatements.getUpdateAssetTemplate(), + toJson(asset.getProperties()), + toJson(asset.getPrivateProperties()), + toJson(asset.getDataAddress().getProperties()), + assetId + ); - @Override - public StoreResult updateDataAddress(String assetId, DataAddress dataAddress) { - return transactionContext.execute(() -> { - try (var connection = getConnection()) { - if (existsById(assetId, connection)) { - var updateTemplate = assetStatements.getUpdateDataAddressTemplate(); - queryExecutor.execute(connection, updateTemplate, toJson(dataAddress.getProperties()), assetId); - return StoreResult.success(dataAddress); + return StoreResult.success(asset); } return StoreResult.notFound(format(ASSET_NOT_FOUND_TEMPLATE, assetId)); @@ -211,50 +169,13 @@ public StoreResult updateDataAddress(String assetId, DataAddress da @Override public DataAddress resolveForAsset(String assetId) { - Objects.requireNonNull(assetId); - - return transactionContext.execute(() -> { - var sql = assetStatements.getFindDataAddressByIdTemplate(); - try { - return queryExecutor.single(getConnection(), true, this::mapDataAddress, sql, assetId); - } catch (Exception e) { - if (e instanceof EdcPersistenceException) { - throw (EdcPersistenceException) e; - } else { - throw new EdcPersistenceException(e.getMessage(), e); - } - } - }); - } - - private long mapCreatedAt(ResultSet resultSet) throws SQLException { - return resultSet.getLong(assetStatements.getCreatedAtColumn()); + return Optional.ofNullable(findById(assetId)).map(Asset::getDataAddress).orElse(null); } private int mapRowCount(ResultSet resultSet) throws SQLException { return resultSet.getInt(assetStatements.getCountVariableName()); } - private SqlPropertyWrapper mapPropertyResultSet(ResultSet resultSet) throws SQLException, ClassNotFoundException { - var name = resultSet.getString(assetStatements.getAssetPropertyNameColumn()); - var value = resultSet.getString(assetStatements.getAssetPropertyValueColumn()); - var type = resultSet.getString(assetStatements.getAssetPropertyTypeColumn()); - var isPrivate = resultSet.getBoolean(assetStatements.getAssetPropertyIsPrivateColumn()); - return new SqlPropertyWrapper(isPrivate, new AbstractMap.SimpleImmutableEntry<>(name, fromPropertyValue(value, type))); - } - - /** - * Deserializes a value into an object using the object mapper. Note: if type is {@code java.lang.String} simply - * {@code value.toString()} is returned. - */ - private Object fromPropertyValue(String value, String type) throws ClassNotFoundException { - var clazz = Class.forName(type); - if (clazz == String.class) { - return value; - } - return fromJson(value, clazz); - } - private boolean existsById(String assetId, Connection connection) { var sql = assetStatements.getCountAssetByIdClause(); try (var stream = queryExecutor.query(connection, false, this::mapRowCount, sql, assetId)) { @@ -262,57 +183,16 @@ private boolean existsById(String assetId, Connection connection) { } } - private DataAddress mapDataAddress(ResultSet resultSet) throws SQLException { - return DataAddress.Builder.newInstance() - .properties(fromJson(resultSet.getString(assetStatements.getDataAddressPropertiesColumn()), new TypeReference<>() { - })) + private Asset mapAsset(ResultSet resultSet) throws SQLException { + return Asset.Builder.newInstance() + .id(resultSet.getString(assetStatements.getAssetIdColumn())) + .createdAt(resultSet.getLong(assetStatements.getCreatedAtColumn())) + .properties(fromJson(resultSet.getString(assetStatements.getPropertiesColumn()), getTypeRef())) + .privateProperties(fromJson(resultSet.getString(assetStatements.getPrivatePropertiesColumn()), getTypeRef())) + .dataAddress(DataAddress.Builder.newInstance() + .properties(fromJson(resultSet.getString(assetStatements.getDataAddressColumn()), getTypeRef())) + .build()) .build(); } - private String mapAssetIds(ResultSet resultSet) throws SQLException { - return resultSet.getString(assetStatements.getAssetIdColumn()); - } - - private void insertProperties(Asset asset, String assetId, Connection connection) { - for (var property : asset.getProperties().entrySet()) { - queryExecutor.execute(connection, - assetStatements.getInsertPropertyTemplate(), - assetId, - property.getKey(), - toJson(property.getValue()), - property.getValue().getClass().getName(), - false); - } - for (var privateProperty : asset.getPrivateProperties().entrySet()) { - queryExecutor.execute(connection, - assetStatements.getInsertPropertyTemplate(), - assetId, - privateProperty.getKey(), - toJson(privateProperty.getValue()), - privateProperty.getValue().getClass().getName(), - true); - } - } - - private static class SqlPropertyWrapper { - private final boolean isPrivate; - private final AbstractMap.SimpleImmutableEntry property; - - protected SqlPropertyWrapper(boolean isPrivate, AbstractMap.SimpleImmutableEntry kvSimpleImmutableEntry) { - this.isPrivate = isPrivate; - this.property = kvSimpleImmutableEntry; - } - - protected boolean isPrivate() { - return isPrivate; - } - - protected String getPropertyKey() { - return property.getKey(); - } - - protected Object getPropertyValue() { - return property.getValue(); - } - } } diff --git a/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/AssetStatements.java b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/AssetStatements.java index 96266af4a54..61836e0dd4e 100644 --- a/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/AssetStatements.java +++ b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/AssetStatements.java @@ -43,6 +43,18 @@ default String getAssetIdColumn() { return "asset_id"; } + default String getPropertiesColumn() { + return "properties"; + } + + default String getPrivatePropertiesColumn() { + return "private_properties"; + } + + default String getDataAddressColumn() { + return "data_address"; + } + /** * The data address table name. */ @@ -110,30 +122,15 @@ default String getCreatedAtColumn() { String getInsertAssetTemplate(); /** - * INSERT clause for data addresses. + * UPDATE clause for assets. */ - String getInsertDataAddressTemplate(); - - /** - * INSERT clause for properties. - */ - String getInsertPropertyTemplate(); + String getUpdateAssetTemplate(); /** * SELECT COUNT clause for assets. */ String getCountAssetByIdClause(); - /** - * SELECT clause for properties. - */ - String getFindPropertyByIdTemplate(); - - /** - * SELECT clause for data addresses. - */ - String getFindDataAddressByIdTemplate(); - /** * SELECT clause for all assets. */ @@ -144,27 +141,11 @@ default String getCreatedAtColumn() { */ String getDeleteAssetByIdTemplate(); - /** - * UPDATE statement for data addresses - */ - String getUpdateDataAddressTemplate(); - - /** - * DELETE statement for properties of an Asset. Useful for delete-insert (=update) operations - */ - String getDeletePropertyByIdTemplate(); - /** * The COUNT variable used in SELECT COUNT queries. */ String getCountVariableName(); - /** - * Provides a dynamically assembled SELECT statement for use with - * {@link QuerySpec} queries. - */ - String getQuerySubSelectTemplate(); - /** * Generates a SQL query using sub-select statements out of the query spec. * @@ -179,10 +160,4 @@ default String getCreatedAtColumn() { */ SqlQueryStatement createQuery(List query); - /** - * Select single asset by ID - */ - String getSelectAssetByIdTemplate(); - - } diff --git a/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/BaseSqlDialectStatements.java b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/BaseSqlDialectStatements.java index 9552d4c8870..8633b75d575 100644 --- a/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/BaseSqlDialectStatements.java +++ b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/BaseSqlDialectStatements.java @@ -16,10 +16,9 @@ package org.eclipse.edc.connector.store.sql.assetindex.schema; +import org.eclipse.edc.connector.store.sql.assetindex.schema.postgres.AssetMapping; import org.eclipse.edc.spi.query.Criterion; import org.eclipse.edc.spi.query.QuerySpec; -import org.eclipse.edc.spi.result.Result; -import org.eclipse.edc.sql.translation.SqlConditionExpression; import org.eclipse.edc.sql.translation.SqlQueryStatement; import java.util.List; @@ -33,26 +32,19 @@ public String getInsertAssetTemplate() { return executeStatement() .column(getAssetIdColumn()) .column(getCreatedAtColumn()) + .jsonColumn(getPropertiesColumn()) + .jsonColumn(getPrivatePropertiesColumn()) + .jsonColumn(getDataAddressColumn()) .insertInto(getAssetTable()); } @Override - public String getInsertDataAddressTemplate() { + public String getUpdateAssetTemplate() { return executeStatement() - .column(getDataAddressAssetIdFkColumn()) - .jsonColumn(getDataAddressPropertiesColumn()) - .insertInto(getDataAddressTable()); - } - - @Override - public String getInsertPropertyTemplate() { - return executeStatement() - .column(getPropertyAssetIdFkColumn()) - .column(getAssetPropertyNameColumn()) - .column(getAssetPropertyValueColumn()) - .column(getAssetPropertyTypeColumn()) - .column(getAssetPropertyIsPrivateColumn()) - .insertInto(getAssetPropertyTable()); + .jsonColumn(getPropertiesColumn()) + .jsonColumn(getPrivatePropertiesColumn()) + .jsonColumn(getDataAddressColumn()) + .update(getAssetTable(), getAssetIdColumn()); } @Override @@ -63,20 +55,6 @@ public String getCountAssetByIdClause() { getAssetIdColumn()); } - @Override - public String getFindPropertyByIdTemplate() { - return format("SELECT * FROM %s WHERE %s = ?", - getAssetPropertyTable(), - getPropertyAssetIdFkColumn()); - } - - @Override - public String getFindDataAddressByIdTemplate() { - return format("SELECT * FROM %s WHERE %s = ?", - getDataAddressTable(), - getDataAddressAssetIdFkColumn()); - } - @Override public String getSelectAssetTemplate() { return format("SELECT * FROM %s AS a", getAssetTable()); @@ -88,53 +66,14 @@ public String getDeleteAssetByIdTemplate() { .delete(getAssetTable(), getAssetIdColumn()); } - @Override - public String getUpdateDataAddressTemplate() { - return executeStatement() - .jsonColumn(getDataAddressPropertiesColumn()) - .update(getDataAddressTable(), getDataAddressAssetIdFkColumn()); - } - - @Override - public String getDeletePropertyByIdTemplate() { - return executeStatement() - .delete(getAssetPropertyTable(), getPropertyAssetIdFkColumn()); - } - @Override public String getCountVariableName() { return "COUNT"; } - @Override - public String getQuerySubSelectTemplate() { - return format("EXISTS (SELECT 1 FROM %s WHERE %s = a.%s AND %s = ? AND %s", - getAssetPropertyTable(), - getPropertyAssetIdFkColumn(), - getAssetIdColumn(), - getAssetPropertyNameColumn(), - getAssetPropertyValueColumn()); - } - @Override public SqlQueryStatement createQuery(QuerySpec querySpec) { - var criteria = querySpec.getFilterExpression(); - var conditions = criteria.stream().map(SqlConditionExpression::new).toList(); - var validation = conditions.stream() - .map(SqlConditionExpression::isValidExpression) - .reduce(Result::merge) - .orElse(Result.success()); - - if (validation.failed()) { - throw new IllegalArgumentException(validation.getFailureDetail()); - } - - var statement = new SqlQueryStatement(getSelectAssetTemplate(), querySpec.getLimit(), querySpec.getOffset()); - - conditions.forEach(condition -> statement - .addWhereClause(this.toSubSelect(condition), condition.toStatementParameter().toArray())); - - return statement; + return new SqlQueryStatement(getSelectAssetTemplate(), querySpec, new AssetMapping(this)); } @Override @@ -146,28 +85,4 @@ public SqlQueryStatement createQuery(List criteria) { .build()); } - @Override - public String getSelectAssetByIdTemplate() { - return format("SELECT * FROM %s WHERE %s=?", getAssetTable(), getAssetIdColumn()); - } - - /** - * Concatenates all SELECT statements on all properties into one big statement, or returns "" if list is empty. - */ - private String concatSubSelects(List subSelects) { - if (subSelects.isEmpty()) { - return ""; - } - return format(" WHERE %s", String.join(" AND ", subSelects)); - } - - /** - * Converts a {@linkplain Criterion} into a dynamically assembled SELECT statement. - */ - private String toSubSelect(SqlConditionExpression c) { - return format("%s %s %s)", getQuerySubSelectTemplate(), - c.getCriterion().getOperator(), - c.toValuePlaceholder()); - } - } diff --git a/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/postgres/AssetMapping.java b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/postgres/AssetMapping.java new file mode 100644 index 00000000000..a9188c18bf0 --- /dev/null +++ b/extensions/control-plane/store/sql/asset-index-sql/src/main/java/org/eclipse/edc/connector/store/sql/assetindex/schema/postgres/AssetMapping.java @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2022 Microsoft Corporation + * + * This program and the accompanying materials are made available under the + * terms of the Apache License, Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + * + * Contributors: + * Microsoft Corporation - initial API and implementation + * + */ + +package org.eclipse.edc.connector.store.sql.assetindex.schema.postgres; + +import org.eclipse.edc.connector.store.sql.assetindex.schema.AssetStatements; +import org.eclipse.edc.spi.types.PathItem; +import org.eclipse.edc.sql.translation.JsonFieldMapping; +import org.eclipse.edc.sql.translation.TranslationMapping; + +/** + * Maps fields of a {@link org.eclipse.edc.spi.types.domain.asset.Asset} onto the + * corresponding SQL schema (= column names) enabling access through Postgres JSON operators where applicable + */ +public class AssetMapping extends TranslationMapping { + + public AssetMapping(AssetStatements statements) { + add("id", statements.getAssetIdColumn()); + add("createdAt", statements.getCreatedAtColumn()); + add("properties", new JsonFieldMapping(statements.getPropertiesColumn())); + add("privateProperties", new JsonFieldMapping(statements.getPrivatePropertiesColumn())); + add("dataAddress", new JsonFieldMapping(statements.getDataAddressColumn())); + } + + @Override + public String getStatement(String canonicalPropertyName, Class type) { + var standardPath = getStatement(PathItem.parse(canonicalPropertyName), type); + + if (standardPath == null) { + var amendedCanonicalPropertyName = canonicalPropertyName.contains("'") + ? "properties.%s".formatted(canonicalPropertyName) + : "properties.'%s'".formatted(canonicalPropertyName); + return getStatement(amendedCanonicalPropertyName, type); + } + + return standardPath; + } + +} diff --git a/extensions/control-plane/store/sql/asset-index-sql/src/test/java/org/eclipse/edc/connector/store/sql/assetindex/PostgresAssetIndexTest.java b/extensions/control-plane/store/sql/asset-index-sql/src/test/java/org/eclipse/edc/connector/store/sql/assetindex/PostgresAssetIndexTest.java index 758b6dc1a9b..7f65aef3f7f 100644 --- a/extensions/control-plane/store/sql/asset-index-sql/src/test/java/org/eclipse/edc/connector/store/sql/assetindex/PostgresAssetIndexTest.java +++ b/extensions/control-plane/store/sql/asset-index-sql/src/test/java/org/eclipse/edc/connector/store/sql/assetindex/PostgresAssetIndexTest.java @@ -55,8 +55,6 @@ void setUp(PostgresqlStoreSetupExtension setupExtension, QueryExecutor queryExec @AfterEach void tearDown(PostgresqlStoreSetupExtension setupExtension) { setupExtension.runQuery("DROP TABLE " + sqlStatements.getAssetTable() + " CASCADE"); - setupExtension.runQuery("DROP TABLE " + sqlStatements.getDataAddressTable() + " CASCADE"); - setupExtension.runQuery("DROP TABLE " + sqlStatements.getAssetPropertyTable() + " CASCADE"); } @Override diff --git a/spi/common/core-spi/src/main/java/org/eclipse/edc/spi/asset/AssetIndex.java b/spi/common/core-spi/src/main/java/org/eclipse/edc/spi/asset/AssetIndex.java index a3525096e9d..012413e1f98 100644 --- a/spi/common/core-spi/src/main/java/org/eclipse/edc/spi/asset/AssetIndex.java +++ b/spi/common/core-spi/src/main/java/org/eclipse/edc/spi/asset/AssetIndex.java @@ -20,7 +20,6 @@ import org.eclipse.edc.spi.query.Criterion; import org.eclipse.edc.spi.query.QuerySpec; import org.eclipse.edc.spi.result.StoreResult; -import org.eclipse.edc.spi.types.domain.DataAddress; import org.eclipse.edc.spi.types.domain.asset.Asset; import java.util.List; @@ -38,7 +37,6 @@ public interface AssetIndex extends DataAddressResolver { String ASSET_EXISTS_TEMPLATE = "Asset with ID %s already exists"; String ASSET_NOT_FOUND_TEMPLATE = "Asset with ID %s not found"; - String DATA_ADDRESS_NOT_FOUND_TEMPLATE = "DataAddress with ID %s not found"; /** * Finds all assets that are covered by a specific {@link QuerySpec}. Results are always sorted. If no {@link QuerySpec#getSortField()} @@ -98,15 +96,4 @@ public interface AssetIndex extends DataAddressResolver { */ StoreResult updateAsset(Asset asset); - /** - * Updates a {@link DataAddress} that is associated with the {@link Asset} that is identified by the {@code assetId} argument. - * If the asset is not found, no further database interaction takes place. - * - * @param assetId the database of the Asset to update - * @param dataAddress The DataAddress containing the new values. - * @return {@link StoreResult#success(Object)} if the object was updated, {@link StoreResult#notFound(String)} when an object with that ID was not found. - * @deprecated Updating only the DataAddress is deprecated and will be removed in future releases. Please use the {@link AssetIndex#updateAsset(Asset)} method. - */ - @Deprecated(since = "0.3.0", forRemoval = true) - StoreResult updateDataAddress(String assetId, DataAddress dataAddress); } diff --git a/spi/common/core-spi/src/main/java/org/eclipse/edc/spi/types/PathItem.java b/spi/common/core-spi/src/main/java/org/eclipse/edc/spi/types/PathItem.java new file mode 100644 index 00000000000..080846dd98f --- /dev/null +++ b/spi/common/core-spi/src/main/java/org/eclipse/edc/spi/types/PathItem.java @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2023 Bayerische Motoren Werke Aktiengesellschaft (BMW AG) + * + * This program and the accompanying materials are made available under the + * terms of the Apache License, Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + * + * Contributors: + * Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation + * + */ + +package org.eclipse.edc.spi.types; + +import java.util.ArrayList; +import java.util.List; + +/** + * Represent an object path instance, e.g. "properties.'https://w3id.org/edc/v0.0.1/ns/id'" can be parsed into two + * {@link PathItem}s one representing "properties" and the other "https://w3id.org/edc/v0.0.1/ns/id" + */ +public class PathItem { + + public static List parse(String propertyName) { + var result = new ArrayList(); + result.add(new PathItem()); + for (var i = 0; i < propertyName.length(); i++) { + var character = propertyName.charAt(i); + + var lastEntry = result.get(result.size() - 1); + + switch (character) { + case '\'' -> lastEntry.toggle(); + case '.' -> { + if (lastEntry.opened) { + lastEntry.append(character); + } else { + result.add(new PathItem()); + } + } + default -> lastEntry.append(character); + } + + } + return result.stream().toList(); + } + + private boolean opened; + private final StringBuilder builder = new StringBuilder(); + + public void open() { + this.opened = true; + } + + public void close() { + this.opened = false; + } + + public void append(char character) { + builder.append(character); + } + + @Override + public String toString() { + return builder.toString(); + } + + public void toggle() { + if (opened) { + close(); + } else { + open(); + } + } +} diff --git a/spi/common/core-spi/src/testFixtures/java/org/eclipse/edc/spi/testfixtures/asset/AssetIndexTestBase.java b/spi/common/core-spi/src/testFixtures/java/org/eclipse/edc/spi/testfixtures/asset/AssetIndexTestBase.java index 44e742d6640..b52b23ce288 100644 --- a/spi/common/core-spi/src/testFixtures/java/org/eclipse/edc/spi/testfixtures/asset/AssetIndexTestBase.java +++ b/spi/common/core-spi/src/testFixtures/java/org/eclipse/edc/spi/testfixtures/asset/AssetIndexTestBase.java @@ -41,6 +41,7 @@ import static java.util.stream.IntStream.range; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.eclipse.edc.spi.CoreConstants.EDC_NAMESPACE; import static org.eclipse.edc.spi.query.Criterion.criterion; import static org.eclipse.edc.spi.result.StoreFailure.Reason.ALREADY_EXISTS; import static org.eclipse.edc.spi.result.StoreFailure.Reason.NOT_FOUND; @@ -113,9 +114,6 @@ private DataAddress getDataAddress() { .build(); } - public record TestObject(String text, int number, boolean bool) { - } - @Nested class Create { @Test @@ -289,15 +287,15 @@ void nonExistValue() { @Test @DisplayName("Verifies an asset query, that contains a filter expression") void withFilterExpression() { - var expected = createAssetBuilder("id1").property("version", "2.0").property("contenttype", "whatever").build(); - var differentVersion = createAssetBuilder("id2").property("version", "2.1").property("contenttype", "whatever").build(); - var differentContentType = createAssetBuilder("id3").property("version", "2.0").property("contenttype", "different").build(); + var expected = createAssetBuilder("id1").property("version", "2.0").property("contentType", "whatever").build(); + var differentVersion = createAssetBuilder("id2").property("version", "2.1").property("contentType", "whatever").build(); + var differentContentType = createAssetBuilder("id3").property("version", "2.0").property("contentType", "different").build(); getAssetIndex().create(expected); getAssetIndex().create(differentVersion); getAssetIndex().create(differentContentType); var filter = filter( new Criterion("version", "=", "2.0"), - new Criterion("contenttype", "=", "whatever") + new Criterion("contentType", "=", "whatever") ); var assets = getAssetIndex().queryAssets(filter); @@ -305,12 +303,29 @@ void withFilterExpression() { assertThat(assets).hasSize(1).usingRecursiveFieldByFieldElementComparator().containsOnly(expected); } + @Test + void shouldFilterByNestedProperty() { + var nested = EDC_NAMESPACE + "nested"; + var version = EDC_NAMESPACE + "version"; + var expected = createAssetBuilder("id1").property(nested, Map.of(version, "2.0")).build(); + var differentVersion = createAssetBuilder("id2").property(nested, Map.of(version, "2.1")).build(); + getAssetIndex().create(expected); + getAssetIndex().create(differentVersion); + + var assets = getAssetIndex().queryAssets(filter(criterion("'%s'.'%s'".formatted(nested, version), "=", "2.0"))); + + assertThat(assets).hasSize(1).usingRecursiveFieldByFieldElementComparator().containsOnly(expected); + } + @Test @DisplayName("Verify an asset query based on an Asset property, where the property value is actually a complex object") void query_assetPropertyAsObject() { + var nested = Map.of("text", "test123", "number", 42, "bool", false); var dataAddress = createDataAddress(); - var asset = createAssetBuilder("id1").dataAddress(dataAddress).build(); - asset.getProperties().put("testobj", new TestObject("test123", 42, false)); + var asset = createAssetBuilder("id1") + .dataAddress(dataAddress) + .property("testobj", nested) + .build(); getAssetIndex().create(asset); var assetsFound = getAssetIndex().queryAssets(QuerySpec.Builder.newInstance() @@ -318,7 +333,6 @@ void query_assetPropertyAsObject() { .build()); assertThat(assetsFound).hasSize(1).first().usingRecursiveComparison().isEqualTo(asset); - assertThat(asset.getProperty("testobj")).isInstanceOf(TestObject.class); } @Test @@ -349,8 +363,8 @@ void in() { } @Test - @DisplayName("Query assets using the IN operator, invalid righ-operand") - void in_shouldThrowException_whenInvalidRightOperand() { + @DisplayName("Query assets using the IN operator, invalid right operand") + void shouldThrowException_whenOperatorInAndInvalidRightOperand() { var asset1 = getAsset("id1"); getAssetIndex().create(asset1); var asset2 = getAsset("id2"); @@ -409,7 +423,8 @@ void like() { @DisplayName("Query assets using the LIKE operator on a json value") void likeJson() throws JsonProcessingException { var asset = getAsset("id1"); - asset.getProperties().put("myjson", new ObjectMapper().writeValueAsString(new TestObject("test123", 42, false))); + var nested = Map.of("text", "test123", "number", 42, "bool", false); + asset.getProperties().put("myjson", new ObjectMapper().writeValueAsString(nested)); getAssetIndex().create(asset); var criterion = new Criterion("myjson", "LIKE", "%test123%"); @@ -579,83 +594,5 @@ void exists_updateDataAddress() { } } - @Nested - class UpdateDataAddress { - @Test - @DisplayName("Update DataAddress where the Asset does not yet exist") - void doesNotExist() { - var id = "id1"; - var assetExpected = getDataAddress(); - var assetIndex = getAssetIndex(); - - var updated = assetIndex.updateDataAddress(id, assetExpected); - Assertions.assertThat(updated).isNotNull().extracting(StoreResult::reason).isEqualTo(NOT_FOUND); - } - - @Test - @DisplayName("Update a DataAddress that exists, adding a new property") - void exists_addsProperty() { - var id = "id1"; - var asset = getAsset(id); - var assetIndex = getAssetIndex(); - assetIndex.create(asset); - - var updatedDataAddress = getDataAddress(); - updatedDataAddress.getProperties().put("newKey", "newValue"); - var updated = assetIndex.updateDataAddress(id, updatedDataAddress); - - Assertions.assertThat(updated).isNotNull(); - - var addressFound = getAssetIndex().resolveForAsset("id1"); - - assertThat(addressFound).isNotNull(); - assertThat(addressFound).usingRecursiveComparison().isEqualTo(updatedDataAddress); - } - - @Test - @DisplayName("Update a DataAddress that exists, removing a property") - void exists_removesProperty() { - var id = "id1"; - var asset = getAsset(id); - var assetIndex = getAssetIndex(); - var dataAddress = getDataAddress(); - dataAddress.getProperties().put("newKey", "newValue"); - assetIndex.create(asset); - - var updatedDataAddress = dataAddress; - updatedDataAddress.getProperties().remove("newKey"); - var updated = assetIndex.updateDataAddress(id, updatedDataAddress); - - Assertions.assertThat(updated).isNotNull(); - - var addressFound = getAssetIndex().resolveForAsset("id1"); - - assertThat(addressFound).isNotNull(); - assertThat(addressFound).usingRecursiveComparison().isEqualTo(updatedDataAddress); - assertThat(addressFound.getProperties()).doesNotContainKeys("newKey"); - } - - @Test - @DisplayName("Update a DataAddress that exists, replacing a property") - void exists_replacesProperty() { - var id = "id1"; - var asset = getAsset(id); - var assetIndex = getAssetIndex(); - var dataAddress = getDataAddress(); - dataAddress.getProperties().put("newKey", "originalValue"); - assetIndex.create(asset); - - dataAddress.getProperties().put("newKey", "newValue"); - var updated = assetIndex.updateDataAddress(id, dataAddress); - - Assertions.assertThat(updated).isNotNull(); - - var addressFound = getAssetIndex().resolveForAsset("id1"); - - assertThat(addressFound).isNotNull(); - assertThat(addressFound).usingRecursiveComparison().isEqualTo(dataAddress); - assertThat(addressFound.getProperties()).containsEntry("newKey", "newValue"); - } - } } diff --git a/spi/control-plane/control-plane-spi/src/main/java/org/eclipse/edc/connector/spi/asset/AssetService.java b/spi/control-plane/control-plane-spi/src/main/java/org/eclipse/edc/connector/spi/asset/AssetService.java index 19d988ef212..f094b644594 100644 --- a/spi/control-plane/control-plane-spi/src/main/java/org/eclipse/edc/connector/spi/asset/AssetService.java +++ b/spi/control-plane/control-plane-spi/src/main/java/org/eclipse/edc/connector/spi/asset/AssetService.java @@ -16,7 +16,6 @@ import org.eclipse.edc.service.spi.result.ServiceResult; import org.eclipse.edc.spi.query.QuerySpec; -import org.eclipse.edc.spi.types.domain.DataAddress; import org.eclipse.edc.spi.types.domain.asset.Asset; import java.util.stream.Stream; @@ -63,12 +62,4 @@ public interface AssetService { */ ServiceResult update(Asset asset); - /** - * Updates a {@link DataAddress}. If the associated asset does not yet exist, {@link ServiceResult#notFound(String)} will be returned; - * - * @param assetId The ID of the asset to update. - * @param dataAddress The content of the DataAddress. - * @return successful if updated, a failure otherwise. - */ - ServiceResult update(String assetId, DataAddress dataAddress); } diff --git a/system-tests/e2e-transfer-test/runner/src/test/java/org/eclipse/edc/test/e2e/PostgresUtil.java b/system-tests/e2e-transfer-test/runner/src/test/java/org/eclipse/edc/test/e2e/PostgresUtil.java index 4277afd7cca..bc805f838fa 100644 --- a/system-tests/e2e-transfer-test/runner/src/test/java/org/eclipse/edc/test/e2e/PostgresUtil.java +++ b/system-tests/e2e-transfer-test/runner/src/test/java/org/eclipse/edc/test/e2e/PostgresUtil.java @@ -31,11 +31,11 @@ public class PostgresUtil { - public static void createDatabase(EndToEndTransferParticipant consumer) throws ClassNotFoundException, SQLException, IOException { + public static void createDatabase(EndToEndTransferParticipant participant) throws ClassNotFoundException, SQLException, IOException { Class.forName("org.postgresql.Driver"); - var helper = new PostgresqlLocalInstance(USER, PASSWORD, JDBC_URL_PREFIX, consumer.getName()); - helper.createDatabase(consumer.getName()); + var helper = new PostgresqlLocalInstance(USER, PASSWORD, JDBC_URL_PREFIX, participant.getName()); + helper.createDatabase(participant.getName()); var scripts = Stream.of( "extensions/control-plane/store/sql/asset-index-sql", @@ -50,7 +50,7 @@ public static void createDatabase(EndToEndTransferParticipant consumer) throws C .map(Paths::get) .toList(); - try (var connection = DriverManager.getConnection(consumer.jdbcUrl(), USER, PASSWORD)) { + try (var connection = DriverManager.getConnection(participant.jdbcUrl(), USER, PASSWORD)) { for (var script : scripts) { var sql = Files.readString(script);