Skip to content

Commit

Permalink
refactor(api): counterPartyAddress (eclipse-edc#3597)
Browse files Browse the repository at this point in the history
* refactor(management-api): counterPartyAddress (eclipse-edc#3343)

Signed-off-by: Sascha Isele <sascha.isele.external@zf.com>

* checkstyle fix

Signed-off-by: Sascha Isele <sascha.isele.external@zf.com>

* review suggestions and checkstyle

Signed-off-by: Sascha Isele <sascha.isele.external@zf.com>

* moved deprecation warning to validators

Signed-off-by: Sascha Isele <sascha.isele.external@zf.com>

* added tests checking for deprecation warning in validators

Signed-off-by: Sascha Isele <sascha.isele.external@zf.com>

---------

Signed-off-by: Sascha Isele <sascha.isele.external@zf.com>
  • Loading branch information
saschaisele-zf committed Nov 16, 2023
1 parent 209f33d commit dd99058
Show file tree
Hide file tree
Showing 25 changed files with 205 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ private TransferRequest createTransferRequest() {
.assetId("assetId")
.dataDestination(DataAddress.Builder.newInstance().type("any").build())
.protocol("test")
.connectorAddress("http://an/address")
.counterPartyAddress("http://an/address")
.contractId("contractId")
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public StatusResult<TransferProcess> initiateConsumerRequest(TransferRequest tra
.assetId(transferRequest.getAssetId())
.connectorId(transferRequest.getConnectorId())
.dataDestination(transferRequest.getDataDestination())
.connectorAddress(transferRequest.getConnectorAddress())
.connectorAddress(transferRequest.getCounterPartyAddress())
.contractId(transferRequest.getContractId())
.destinationType(transferRequest.getDataDestination().getType())
.protocol(transferRequest.getProtocol())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ public String name() {

@Override
public void initialize(ServiceExtensionContext context) {
transformerRegistry.register(new JsonObjectToCatalogRequestTransformer(context.getMonitor()));
transformerRegistry.register(new JsonObjectToCatalogRequestTransformer());
transformerRegistry.register(new JsonObjectToDatasetRequestTransformer());

webService.registerResource(config.getContextAlias(), new CatalogApiController(service, transformerRegistry, validatorRegistry));

validatorRegistry.register(CATALOG_REQUEST_TYPE, CatalogRequestValidator.instance());
validatorRegistry.register(CATALOG_REQUEST_TYPE, CatalogRequestValidator.instance(context.getMonitor()));
validatorRegistry.register(DATASET_REQUEST_TYPE, DatasetRequestValidator.instance());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,29 @@
import jakarta.json.JsonObject;
import org.eclipse.edc.catalog.spi.CatalogRequest;
import org.eclipse.edc.jsonld.spi.transformer.AbstractJsonLdTransformer;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.spi.query.QuerySpec;
import org.eclipse.edc.transform.spi.TransformerContext;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.Optional;

import static java.lang.String.format;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_COUNTER_PARTY_ADDRESS;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_PROTOCOL;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_PROVIDER_URL;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_QUERY_SPEC;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_TYPE;

public class JsonObjectToCatalogRequestTransformer extends AbstractJsonLdTransformer<JsonObject, CatalogRequest> {

private final Monitor monitor;

public JsonObjectToCatalogRequestTransformer(Monitor monitor) {
public JsonObjectToCatalogRequestTransformer() {
super(JsonObject.class, CatalogRequest.class);
this.monitor = monitor;
}

@Override
public @Nullable CatalogRequest transform(@NotNull JsonObject object, @NotNull TransformerContext context) {
var counterPartyAddress = Optional.of(object)
.map(it -> it.get(CATALOG_REQUEST_COUNTER_PARTY_ADDRESS))
.orElseGet(() -> {
monitor.warning(format("The attribute %s has been deprecated in type %s, please use %s",
CATALOG_REQUEST_PROVIDER_URL, CATALOG_REQUEST_TYPE, CATALOG_REQUEST_COUNTER_PARTY_ADDRESS));
return object.get(CATALOG_REQUEST_PROVIDER_URL);
});
.orElseGet(() -> object.get(CATALOG_REQUEST_PROVIDER_URL));

var querySpec = Optional.of(object)
.map(it -> it.get(CATALOG_REQUEST_QUERY_SPEC))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,43 +15,50 @@
package org.eclipse.edc.connector.api.management.catalog.validation;

import jakarta.json.JsonObject;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.validator.jsonobject.JsonLdPath;
import org.eclipse.edc.validator.jsonobject.JsonObjectValidator;
import org.eclipse.edc.validator.jsonobject.validators.MandatoryValue;
import org.eclipse.edc.validator.jsonobject.validators.model.QuerySpecValidator;
import org.eclipse.edc.validator.spi.ValidationResult;
import org.eclipse.edc.validator.spi.Validator;

import static java.lang.String.format;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_COUNTER_PARTY_ADDRESS;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_PROTOCOL;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_PROVIDER_URL;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_QUERY_SPEC;
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_TYPE;

public class CatalogRequestValidator {

public static Validator<JsonObject> instance() {
public static Validator<JsonObject> instance(Monitor monitor) {
return JsonObjectValidator.newValidator()
.verify(MandatoryCounterPartyAddressOrProviderUrl::new)
.verify(path -> new MandatoryCounterPartyAddressOrProviderUrl(path, monitor))
.verify(CATALOG_REQUEST_PROTOCOL, MandatoryValue::new)
.verifyObject(CATALOG_REQUEST_QUERY_SPEC, QuerySpecValidator::instance)
.build();
}

private record MandatoryCounterPartyAddressOrProviderUrl(JsonLdPath path) implements Validator<JsonObject> {

/**
* This custom validator can be removed once `providerUrl` is deleted and exists only for legacy reasons
*/
private record MandatoryCounterPartyAddressOrProviderUrl(JsonLdPath path, Monitor monitor) implements Validator<JsonObject> {
@Override
public ValidationResult validate(JsonObject input) {
var counterPartyAddress = new MandatoryValue(path.append(CATALOG_REQUEST_COUNTER_PARTY_ADDRESS));
var providerUrl = new MandatoryValue(path.append(CATALOG_REQUEST_PROVIDER_URL));

var validateCounterParty = counterPartyAddress.validate(input);
if (validateCounterParty.succeeded()) {
return ValidationResult.success();
}
var providerUrl = new MandatoryValue(path.append(CATALOG_REQUEST_PROVIDER_URL));
var validateProviderUrl = providerUrl.validate(input);

if (validateCounterParty.succeeded() || validateProviderUrl.succeeded()) {
if (validateProviderUrl.succeeded()) {
monitor.warning(format("The attribute %s has been deprecated in type %s, please use %s",
CATALOG_REQUEST_PROVIDER_URL, CATALOG_REQUEST_TYPE, CATALOG_REQUEST_COUNTER_PARTY_ADDRESS));
return ValidationResult.success();
} else {
return validateCounterParty;
}
return validateCounterParty;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.eclipse.edc.jsonld.JsonLdExtension;
import org.eclipse.edc.jsonld.spi.JsonLd;
import org.eclipse.edc.jsonld.util.JacksonJsonLd;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.transform.spi.TypeTransformerRegistry;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -55,14 +56,14 @@ class CatalogApiTest {

@BeforeEach
void setUp() {
transformer.register(new JsonObjectToCatalogRequestTransformer(mock()));
transformer.register(new JsonObjectToCatalogRequestTransformer());
transformer.register(new JsonObjectToDatasetRequestTransformer());
transformer.register(new JsonObjectToQuerySpecTransformer());
}

@Test
void catalogRequestExample() throws JsonProcessingException {
var validator = CatalogRequestValidator.instance();
var validator = CatalogRequestValidator.instance(mock(Monitor.class));

var jsonObject = objectMapper.readValue(CATALOG_REQUEST_EXAMPLE, JsonObject.class);
assertThat(jsonObject).isNotNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import jakarta.json.Json;
import jakarta.json.JsonObject;
import org.eclipse.edc.catalog.spi.CatalogRequest;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.spi.query.QuerySpec;
import org.eclipse.edc.transform.spi.TransformerContext;
import org.junit.jupiter.api.Test;
Expand All @@ -30,7 +29,6 @@
import static org.eclipse.edc.catalog.spi.CatalogRequest.CATALOG_REQUEST_TYPE;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand All @@ -40,8 +38,7 @@
class JsonObjectToCatalogRequestTransformerTest {

private final TransformerContext context = mock();
private final Monitor monitor = mock();
private final JsonObjectToCatalogRequestTransformer transformer = new JsonObjectToCatalogRequestTransformer(monitor);
private final JsonObjectToCatalogRequestTransformer transformer = new JsonObjectToCatalogRequestTransformer();

@Test
void types() {
Expand Down Expand Up @@ -89,7 +86,6 @@ void transform_shouldUseProviderId_whenCounterPartyAddressIsMissing() {
assertThat(result.getCounterPartyAddress()).isEqualTo("http://provider/url");
assertThat(result.getQuerySpec()).isEqualTo(querySpec);
verify(context).transform(querySpecJson, QuerySpec.class);
verify(monitor).warning(anyString());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import jakarta.json.Json;
import jakarta.json.JsonArrayBuilder;
import jakarta.json.JsonObject;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.validator.spi.ValidationFailure;
import org.eclipse.edc.validator.spi.Validator;
import org.eclipse.edc.validator.spi.Violation;
Expand All @@ -33,10 +34,14 @@
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VALUE;
import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat;
import static org.eclipse.edc.spi.query.QuerySpec.EDC_QUERY_SPEC_SORT_FIELD;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

class CatalogRequestValidatorTest {

private final Validator<JsonObject> validator = CatalogRequestValidator.instance();
private final Monitor monitor = mock();
private final Validator<JsonObject> validator = CatalogRequestValidator.instance(monitor);

@Test
void shouldSucceed_whenInputIsValid() {
Expand All @@ -60,6 +65,7 @@ void shouldSucceed_whenDeprecatedProviderUrlIsUsed() {
var result = validator.validate(input);

assertThat(result).isSucceeded();
verify(monitor).warning(anyString());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,12 @@ record ContractRequestSchema(
String type,
@Schema(requiredMode = REQUIRED)
String protocol,
@Schema(requiredMode = REQUIRED)
@Deprecated(since = "0.3.2")
@Schema(deprecated = true, description = "please use counterPartyAddress instead")
String connectorAddress,
@Schema(requiredMode = REQUIRED)
String counterPartyAddress,
@Schema(requiredMode = REQUIRED)
String providerId,
ContractOfferDescriptionSchema offer,
List<ManagementApiSchema.CallbackAddressSchema> callbackAddresses) {
Expand All @@ -139,7 +142,7 @@ record ContractRequestSchema(
{
"@context": { "@vocab": "https://w3id.org/edc/v0.0.1/ns/" },
"@type": "https://w3id.org/edc/v0.0.1/ns/ContractRequest",
"connectorAddress": "http://provider-address",
"counterPartyAddress": "http://provider-address",
"protocol": "dataspace-protocol-http",
"providerId": "provider-id",
"offer": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void initialize(ServiceExtensionContext context) {
transformerRegistry.register(new JsonObjectFromContractNegotiationTransformer(factory));
transformerRegistry.register(new JsonObjectFromNegotiationStateTransformer(factory));

validatorRegistry.register(CONTRACT_REQUEST_TYPE, ContractRequestValidator.instance());
validatorRegistry.register(CONTRACT_REQUEST_TYPE, ContractRequestValidator.instance(context.getMonitor()));
validatorRegistry.register(TERMINATE_NEGOTIATION_TYPE, TerminateNegotiationValidator.instance());

var monitor = context.getMonitor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.CALLBACK_ADDRESSES;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.CONNECTOR_ADDRESS;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.CONTRACT_REQUEST_COUNTER_PARTY_ADDRESS;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.OFFER;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.PROTOCOL;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.PROVIDER_ID;
Expand All @@ -42,7 +43,7 @@ public JsonObjectToContractRequestTransformer() {
public @Nullable ContractRequest transform(@NotNull JsonObject jsonObject, @NotNull TransformerContext context) {
var contractRequestBuilder = ContractRequest.Builder.newInstance()
.providerId(getProviderId(jsonObject, context))
.counterPartyAddress(transformString(jsonObject.get(CONNECTOR_ADDRESS), context))
.counterPartyAddress(counterPartyAddressOrConnectorAddress(jsonObject, context))
.protocol(transformString(jsonObject.get(PROTOCOL), context));

var contractOfferDescription = transformObject(jsonObject.get(OFFER), ContractOfferDescription.class, context);
Expand All @@ -69,8 +70,15 @@ private String getProviderId(@NotNull JsonObject jsonObject, @NotNull Transforme
return transformString(providerId, context);
}

return transformString(jsonObject.get(CONNECTOR_ADDRESS), context);
return counterPartyAddressOrConnectorAddress(jsonObject, context);

}

/**
* This method can be removed once `connectorAddress` is deleted and exists only for legacy reasons
*/
private String counterPartyAddressOrConnectorAddress(@NotNull JsonObject jsonObject, @NotNull TransformerContext context) {
var counterPartyAddress = transformString(jsonObject.get(CONTRACT_REQUEST_COUNTER_PARTY_ADDRESS), context);
return counterPartyAddress != null ? counterPartyAddress : transformString(jsonObject.get(CONNECTOR_ADDRESS), context);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,28 @@
package org.eclipse.edc.connector.api.management.contractnegotiation.validation;

import jakarta.json.JsonObject;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.validator.jsonobject.JsonLdPath;
import org.eclipse.edc.validator.jsonobject.JsonObjectValidator;
import org.eclipse.edc.validator.jsonobject.validators.MandatoryObject;
import org.eclipse.edc.validator.jsonobject.validators.MandatoryValue;
import org.eclipse.edc.validator.spi.ValidationResult;
import org.eclipse.edc.validator.spi.Validator;

import static java.lang.String.format;
import static org.eclipse.edc.connector.api.management.contractnegotiation.model.ContractOfferDescription.ASSET_ID;
import static org.eclipse.edc.connector.api.management.contractnegotiation.model.ContractOfferDescription.OFFER_ID;
import static org.eclipse.edc.connector.api.management.contractnegotiation.model.ContractOfferDescription.POLICY;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.CONNECTOR_ADDRESS;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.CONTRACT_REQUEST_COUNTER_PARTY_ADDRESS;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.CONTRACT_REQUEST_TYPE;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.OFFER;
import static org.eclipse.edc.connector.contract.spi.types.negotiation.ContractRequest.PROTOCOL;

public class ContractRequestValidator {
public static Validator<JsonObject> instance() {
public static Validator<JsonObject> instance(Monitor monitor) {
return JsonObjectValidator.newValidator()
.verify(CONNECTOR_ADDRESS, MandatoryValue::new)
.verify(path -> new MandatoryCounterPartyAddressOrConnectorAddress(path, monitor))
.verify(PROTOCOL, MandatoryValue::new)
.verify(OFFER, MandatoryObject::new)
.verifyObject(OFFER, v -> v
Expand All @@ -40,4 +46,27 @@ public static Validator<JsonObject> instance() {
)
.build();
}

/**
* This custom validator can be removed once `connectorAddress` is deleted and exists only for legacy reasons
*/
private record MandatoryCounterPartyAddressOrConnectorAddress(JsonLdPath path, Monitor monitor) implements Validator<JsonObject> {

@Override
public ValidationResult validate(JsonObject input) {
var counterPartyAddress = new MandatoryValue(path.append(CONTRACT_REQUEST_COUNTER_PARTY_ADDRESS));
var validateCounterParty = counterPartyAddress.validate(input);
if (validateCounterParty.succeeded()) {
return ValidationResult.success();
}
var connectorAddress = new MandatoryValue(path.append(CONNECTOR_ADDRESS));
var validateConnectorAddress = connectorAddress.validate(input);
if (validateConnectorAddress.succeeded()) {
monitor.warning(format("The attribute %s has been deprecated in type %s, please use %s",
CONNECTOR_ADDRESS, CONTRACT_REQUEST_TYPE, CONTRACT_REQUEST_COUNTER_PARTY_ADDRESS));
return ValidationResult.success();
}
return validateCounterParty;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.eclipse.edc.jsonld.JsonLdExtension;
import org.eclipse.edc.jsonld.spi.JsonLd;
import org.eclipse.edc.jsonld.util.JacksonJsonLd;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.transform.spi.TypeTransformerRegistry;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -44,6 +45,7 @@
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.VALUE;
import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat;
import static org.eclipse.edc.junit.extensions.TestServiceExtensionContext.testServiceExtensionContext;
import static org.mockito.Mockito.mock;

class ContractNegotiationApiTest {

Expand All @@ -62,7 +64,7 @@ void setUp() {

@Test
void contractRequestExample() throws JsonProcessingException {
var validator = ContractRequestValidator.instance();
var validator = ContractRequestValidator.instance(mock(Monitor.class));

var jsonObject = objectMapper.readValue(CONTRACT_REQUEST_EXAMPLE, JsonObject.class);
assertThat(jsonObject).isNotNull();
Expand All @@ -72,9 +74,7 @@ void contractRequestExample() throws JsonProcessingException {
.satisfies(exp -> assertThat(validator.validate(exp)).isSucceeded())
.extracting(e -> transformer.transform(e, ContractRequest.class))
.satisfies(transformResult -> assertThat(transformResult).isSucceeded()
.satisfies(transformed -> {
assertThat(transformed.getProviderId()).isNotBlank();
}));
.satisfies(transformed -> assertThat(transformed.getProviderId()).isNotBlank()));
}

@Test
Expand Down
Loading

0 comments on commit dd99058

Please sign in to comment.