Skip to content

Commit 0e09601

Browse files
ndr-brtndr-brt
authored
fix: remove address validation from AssetService, fix metadata deserialization (#5720)
fix: remove DataAddress validator from AssetService and fix DataplaneMetadata deserialization
1 parent f21e385 commit 0e09601

18 files changed

Lines changed: 163 additions & 118 deletions

File tree

‎core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/ControlPlaneServicesExtension.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public AssetService assetService() {
181181
var assetObservable = new AssetObservableImpl();
182182
assetObservable.registerListener(new AssetEventListener(eventRouter));
183183
return new AssetServiceImpl(assetIndex, contractNegotiationStore, transactionContext, assetObservable,
184-
dataAddressValidator, new AssetQueryValidator());
184+
new AssetQueryValidator());
185185
}
186186

187187
@Provider

‎core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImpl.java‎

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.eclipse.edc.spi.query.QuerySpec;
2727
import org.eclipse.edc.spi.result.ServiceResult;
2828
import org.eclipse.edc.transaction.spi.TransactionContext;
29-
import org.eclipse.edc.validator.spi.DataAddressValidatorRegistry;
3029

3130
import java.util.List;
3231
import java.util.function.Predicate;
@@ -41,17 +40,15 @@ public class AssetServiceImpl implements AssetService {
4140
private final ContractNegotiationStore contractNegotiationStore;
4241
private final TransactionContext transactionContext;
4342
private final AssetObservable observable;
44-
private final DataAddressValidatorRegistry dataAddressValidator;
4543
private final QueryValidator queryValidator;
4644

4745
public AssetServiceImpl(AssetIndex index, ContractNegotiationStore contractNegotiationStore,
4846
TransactionContext transactionContext, AssetObservable observable,
49-
DataAddressValidatorRegistry dataAddressValidator, QueryValidator queryValidator) {
47+
QueryValidator queryValidator) {
5048
this.index = index;
5149
this.contractNegotiationStore = contractNegotiationStore;
5250
this.transactionContext = transactionContext;
5351
this.observable = observable;
54-
this.dataAddressValidator = dataAddressValidator;
5552
this.queryValidator = queryValidator;
5653
}
5754

@@ -75,13 +72,6 @@ public ServiceResult<Asset> create(Asset asset) {
7572
return ServiceResult.badRequest(DUPLICATED_KEYS_MESSAGE);
7673
}
7774

78-
if (asset.getDataAddress() != null) {
79-
var validDataAddress = dataAddressValidator.validateSource(asset.getDataAddress());
80-
if (validDataAddress.failed()) {
81-
return ServiceResult.badRequest(validDataAddress.getFailureMessages());
82-
}
83-
}
84-
8575
return transactionContext.execute(() -> {
8676
var createResult = index.create(asset);
8777
if (createResult.succeeded()) {
@@ -118,11 +108,6 @@ public ServiceResult<Asset> update(Asset asset) {
118108
return ServiceResult.badRequest(DUPLICATED_KEYS_MESSAGE);
119109
}
120110

121-
var validDataAddress = dataAddressValidator.validateSource(asset.getDataAddress());
122-
if (validDataAddress.failed()) {
123-
return ServiceResult.badRequest(validDataAddress.getFailureMessages());
124-
}
125-
126111
return transactionContext.execute(() -> {
127112
var updatedAsset = index.updateAsset(asset);
128113
updatedAsset.onSuccess(a -> observable.invokeForEach(l -> l.updated(a)));

‎core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImplTest.java‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
package org.eclipse.edc.connector.controlplane.services.asset;
1616

17-
import org.assertj.core.api.Assertions;
1817
import org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset;
1918
import org.eclipse.edc.connector.controlplane.asset.spi.index.AssetIndex;
2019
import org.eclipse.edc.connector.controlplane.asset.spi.observe.AssetObservable;
@@ -28,13 +27,10 @@
2827
import org.eclipse.edc.spi.query.QuerySpec;
2928
import org.eclipse.edc.spi.result.Result;
3029
import org.eclipse.edc.spi.result.ServiceFailure;
31-
import org.eclipse.edc.spi.result.ServiceResult;
3230
import org.eclipse.edc.spi.result.StoreResult;
3331
import org.eclipse.edc.spi.types.domain.DataAddress;
3432
import org.eclipse.edc.transaction.spi.NoopTransactionContext;
3533
import org.eclipse.edc.transaction.spi.TransactionContext;
36-
import org.eclipse.edc.validator.spi.DataAddressValidatorRegistry;
37-
import org.eclipse.edc.validator.spi.ValidationResult;
3834
import org.jetbrains.annotations.NotNull;
3935
import org.junit.jupiter.api.DisplayName;
4036
import org.junit.jupiter.api.Nested;
@@ -53,7 +49,6 @@
5349
import static org.eclipse.edc.spi.result.ServiceFailure.Reason.BAD_REQUEST;
5450
import static org.eclipse.edc.spi.result.ServiceFailure.Reason.CONFLICT;
5551
import static org.eclipse.edc.spi.result.ServiceFailure.Reason.NOT_FOUND;
56-
import static org.eclipse.edc.validator.spi.Violation.violation;
5752
import static org.mockito.AdditionalMatchers.and;
5853
import static org.mockito.ArgumentMatchers.any;
5954
import static org.mockito.ArgumentMatchers.argThat;
@@ -73,11 +68,10 @@ class AssetServiceImplTest {
7368
private final ContractNegotiationStore contractNegotiationStore = mock();
7469
private final TransactionContext dummyTransactionContext = new NoopTransactionContext();
7570
private final AssetObservable observable = mock();
76-
private final DataAddressValidatorRegistry dataAddressValidator = mock();
7771
private final QueryValidator queryValidator = mock();
7872

7973
private final AssetService service = new AssetServiceImpl(index, contractNegotiationStore, dummyTransactionContext,
80-
observable, dataAddressValidator, queryValidator);
74+
observable, queryValidator);
8175

8276
@Test
8377
void findById_shouldRelyOnAssetIndex() {
@@ -115,7 +109,6 @@ void search_shouldFail_whenQueryIsNotValid() {
115109
class CreateAsset {
116110
@Test
117111
void shouldCreateAssetIfItDoesNotAlreadyExist() {
118-
when(dataAddressValidator.validateSource(any())).thenReturn(ValidationResult.success());
119112
var assetId = "assetId";
120113
var asset = createAsset(assetId);
121114
when(index.create(asset)).thenReturn(StoreResult.success());
@@ -131,7 +124,6 @@ void shouldCreateAssetIfItDoesNotAlreadyExist() {
131124

132125
@Test
133126
void shouldCreateAsset_whenDataAddressIsNull() {
134-
when(dataAddressValidator.validateSource(any())).thenReturn(ValidationResult.success());
135127
var assetId = "assetId";
136128
var asset = createAssetBuilder(assetId).dataAddress(null).build();
137129
when(index.create(asset)).thenReturn(StoreResult.success());
@@ -141,13 +133,11 @@ void shouldCreateAsset_whenDataAddressIsNull() {
141133
assertThat(inserted).isSucceeded().matches(hasId(assetId));
142134
verify(index).create(and(isA(Asset.class), argThat(it -> assetId.equals(it.getId()))));
143135
verifyNoMoreInteractions(index);
144-
verifyNoInteractions(dataAddressValidator);
145136
verify(observable).invokeForEach(any());
146137
}
147138

148139
@Test
149140
void shouldNotCreateAssetIfItAlreadyExists() {
150-
when(dataAddressValidator.validateSource(any())).thenReturn(ValidationResult.success());
151141
var asset = createAsset("assetId");
152142
when(index.create(asset)).thenReturn(StoreResult.alreadyExists("test"));
153143

@@ -156,21 +146,9 @@ void shouldNotCreateAssetIfItAlreadyExists() {
156146
assertThat(inserted).isFailed().extracting(ServiceFailure::getReason).isEqualTo(CONFLICT);
157147
}
158148

159-
@Test
160-
void shouldNotCreateAssetIfDataAddressInvalid() {
161-
var asset = createAsset("assetId");
162-
when(dataAddressValidator.validateSource(any())).thenReturn(ValidationResult.failure(violation("Data address is invalid", "path")));
163-
164-
var result = service.create(asset);
165-
166-
Assertions.assertThat(result).satisfies(ServiceResult::failed).extracting(ServiceResult::reason).isEqualTo(BAD_REQUEST);
167-
verifyNoInteractions(index);
168-
}
169-
170149
@Test
171150
void shouldFail_whenPropertiesAreDuplicated() {
172151
var asset = createAssetBuilder("assetId").property("property", "value").privateProperty("property", "other-value").build();
173-
when(dataAddressValidator.validateSource(any())).thenReturn(ValidationResult.success());
174152

175153
var result = service.create(asset);
176154

@@ -271,7 +249,6 @@ private static Stream<Arguments> nonFinalStates() {
271249
@Test
272250
void updateAsset_shouldUpdateWhenExists() {
273251
var asset = createAsset("assetId");
274-
when(dataAddressValidator.validateSource(any())).thenReturn(ValidationResult.success());
275252
when(index.updateAsset(asset)).thenReturn(StoreResult.success(asset));
276253

277254
var updated = service.update(asset);
@@ -285,7 +262,6 @@ void updateAsset_shouldUpdateWhenExists() {
285262
@Test
286263
void updateAsset_shouldReturnNotFound_whenNotExists() {
287264
var asset = createAsset("assetId");
288-
when(dataAddressValidator.validateSource(any())).thenReturn(ValidationResult.success());
289265
when(index.updateAsset(eq(asset))).thenReturn(StoreResult.notFound("test"));
290266

291267
var updated = service.update(asset);

‎core/control-plane/control-plane-transform/src/main/java/org/eclipse/edc/connector/controlplane/transform/edc/to/JsonObjectToDataplaneMetadataTransformer.java‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,22 @@
1414

1515
package org.eclipse.edc.connector.controlplane.transform.edc.to;
1616

17+
import jakarta.json.JsonArray;
1718
import jakarta.json.JsonObject;
19+
import jakarta.json.JsonString;
20+
import jakarta.json.JsonValue;
1821
import org.eclipse.edc.connector.controlplane.asset.spi.domain.DataplaneMetadata;
1922
import org.eclipse.edc.jsonld.spi.transformer.AbstractJsonLdTransformer;
2023
import org.eclipse.edc.transform.spi.TransformerContext;
2124
import org.jetbrains.annotations.NotNull;
2225
import org.jetbrains.annotations.Nullable;
2326

27+
import java.util.Objects;
28+
import java.util.Optional;
29+
30+
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.JSON;
31+
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
32+
2433
public class JsonObjectToDataplaneMetadataTransformer extends AbstractJsonLdTransformer<JsonObject, DataplaneMetadata> {
2534

2635
public JsonObjectToDataplaneMetadataTransformer() {
@@ -33,21 +42,40 @@ public JsonObjectToDataplaneMetadataTransformer() {
3342

3443
var labels = jsonObject.getJsonArray(DataplaneMetadata.EDC_DATAPLANE_METADATA_LABELS);
3544
if (labels != null) {
36-
transformArray(labels, Object.class, context).forEach(label -> builder.label(label.toString()));
45+
labels.stream()
46+
.map(this::nodeJsonValue)
47+
.map(value -> deserializeLabel(value, context))
48+
.filter(Objects::nonNull)
49+
.forEach(builder::label);
3750
}
3851

3952
var properties = jsonObject.get(DataplaneMetadata.EDC_DATAPLANE_METADATA_PROPERTIES);
40-
if (properties != null) {
41-
var jsonValue = nodeJsonValue(properties);
42-
if (jsonValue instanceof JsonObject json) {
43-
visitProperties(json, (key, value) -> builder.property(key, transformGenericProperty(value, context)));
53+
if (properties instanceof JsonArray propertiesArray && !propertiesArray.isEmpty()) {
54+
55+
var propertiesEntry = propertiesArray.get(0);
56+
if (propertiesEntry instanceof JsonObject object) {
57+
var map = Optional.ofNullable(object.getJsonString(TYPE))
58+
.map(JsonString::getString)
59+
.filter(it -> it.equals(JSON))
60+
.map(i -> nodeJsonValue(propertiesEntry).asJsonObject())
61+
.orElse(object);
62+
63+
visitProperties(map, (key, value) -> builder.property(key, transformGenericProperty(value, context)));
4464
} else {
4565
context.reportProblem("Expected properties to be a JsonObject");
4666
return null;
47-
4867
}
4968
}
5069

5170
return builder.build();
5271
}
72+
73+
private @Nullable String deserializeLabel(JsonValue value, @NotNull TransformerContext context) {
74+
if (value instanceof JsonString string) {
75+
return string.getString();
76+
}
77+
78+
context.reportProblem("DataplaneMetadata labels should be strings, but label '%s' is a '%s'".formatted(value, value.getValueType()));
79+
return null;
80+
}
5381
}

‎core/control-plane/control-plane-transform/src/test/java/org/eclipse/edc/connector/controlplane/transform/TestInput.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414

1515
package org.eclipse.edc.connector.controlplane.transform;
1616

17-
import com.apicatalog.jsonld.document.JsonDocument;
1817
import jakarta.json.JsonObject;
19-
20-
import static com.apicatalog.jsonld.JsonLd.expand;
18+
import org.eclipse.edc.jsonld.CachedDocumentRegistry;
19+
import org.eclipse.edc.jsonld.TitaniumJsonLd;
20+
import org.eclipse.edc.spi.monitor.ConsoleMonitor;
2121

2222
/**
2323
* Functions for shaping test input.
@@ -28,11 +28,11 @@ public class TestInput {
2828
* Expands test input as Json-ld is required to be in this form
2929
*/
3030
public static JsonObject getExpanded(JsonObject message) {
31-
try {
32-
return expand(JsonDocument.of(message)).get().asJsonArray().getJsonObject(0);
33-
} catch (Exception e) {
34-
throw new AssertionError(e);
35-
}
31+
var jsonLd = new TitaniumJsonLd(new ConsoleMonitor());
32+
CachedDocumentRegistry.getDocuments().forEach(result -> result
33+
.onSuccess(c -> jsonLd.registerCachedDocument(c.url(), c.resource()))
34+
);
35+
return jsonLd.expand(message).orElseThrow(f -> new AssertionError(f.getFailureDetail()));
3636
}
3737

3838
private TestInput() {

‎core/control-plane/control-plane-transform/src/test/java/org/eclipse/edc/connector/controlplane/transform/edc/to/JsonObjectToAssetTransformerTest.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import static org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset.PROPERTY_ID;
4444
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.CONTEXT;
4545
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.ID;
46-
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.JSON;
4746
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
4847
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VALUE;
4948
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VOCAB;
@@ -90,8 +89,7 @@ void transform_onlyKnownProperties() {
9089
.add("labels", jsonFactory.createArrayBuilder().add("label"))
9190
.add("properties",
9291
jsonFactory.createObjectBuilder()
93-
.add(TYPE, JSON)
94-
.add(VALUE, jsonFactory.createObjectBuilder().add("property", "value")))
92+
.add("property", "value"))
9593
)
9694
.build();
9795

@@ -105,7 +103,7 @@ void transform_onlyKnownProperties() {
105103
.containsEntry(PROPERTY_DESCRIPTION, TEST_ASSET_DESCRIPTION);
106104
assertThat(asset.getDataAddress()).isNotNull().extracting(DataAddress::getType).isEqualTo("address-type");
107105
assertThat(asset.getDataplaneMetadata().getLabels()).containsExactly("label");
108-
assertThat(asset.getDataplaneMetadata().getProperties()).containsExactly(entry("property", "value"));
106+
assertThat(asset.getDataplaneMetadata().getProperties()).containsExactly(entry(EDC_NAMESPACE + "property", "value"));
109107
});
110108

111109
}

0 commit comments

Comments
 (0)