Skip to content

Commit

Permalink
fix: specialize ContractId only for offers, not agreements
Browse files Browse the repository at this point in the history
  • Loading branch information
ndr-brt committed Sep 1, 2023
1 parent b95eb25 commit b3ec2a2
Show file tree
Hide file tree
Showing 21 changed files with 202 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import org.eclipse.edc.catalog.spi.Dataset;
import org.eclipse.edc.catalog.spi.DatasetResolver;
import org.eclipse.edc.catalog.spi.DistributionResolver;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.ContractOfferId;
import org.eclipse.edc.connector.contract.spi.offer.ContractDefinitionResolver;
import org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition;
import org.eclipse.edc.connector.policy.spi.store.PolicyDefinitionStore;
Expand Down Expand Up @@ -91,7 +91,7 @@ private Dataset toDataset(List<ContractDefinition> contractDefinitions, Asset as
.forEach(contractDefinition -> {
var policyDefinition = policyDefinitionStore.findById(contractDefinition.getContractPolicyId());
if (policyDefinition != null) {
var contractId = ContractId.create(contractDefinition.getId(), asset.getId());
var contractId = ContractOfferId.create(contractDefinition.getId(), asset.getId());
datasetBuilder.offer(contractId.toString(), policyDefinition.getPolicy().withTarget(asset.getId()));
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.eclipse.edc.catalog.spi.Distribution;
import org.eclipse.edc.catalog.spi.DistributionResolver;
import org.eclipse.edc.connector.asset.CriterionToAssetPredicateConverterImpl;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.ContractOfferId;
import org.eclipse.edc.connector.contract.spi.offer.ContractDefinitionResolver;
import org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition;
import org.eclipse.edc.connector.policy.spi.PolicyDefinition;
Expand Down Expand Up @@ -89,7 +89,7 @@ void query_shouldReturnOneDatasetPerAsset() {
assertThat(dataset.getId()).isEqualTo("assetId");
assertThat(dataset.getDistributions()).hasSize(1).first().isEqualTo(distribution);
assertThat(dataset.getOffers()).hasSize(1).allSatisfy((id, policy) -> {
assertThat(ContractId.parseId(id)).isSucceeded().extracting(ContractId::definitionPart).asString().isEqualTo("definitionId");
assertThat(ContractOfferId.parseId(id)).isSucceeded().extracting(ContractOfferId::definitionPart).asString().isEqualTo("definitionId");
assertThat(policy.getTarget()).isEqualTo("assetId");
});
assertThat(dataset.getProperties()).contains(entry("key", "value"));
Expand Down Expand Up @@ -126,11 +126,11 @@ void query_shouldReturnOneDataset_whenMultipleDefinitionsOnSameAsset() {
assertThat(dataset.getId()).isEqualTo("assetId");
assertThat(dataset.getOffers()).hasSize(2)
.anySatisfy((id, policy) -> {
assertThat(ContractId.parseId(id)).isSucceeded().extracting(ContractId::definitionPart).asString().isEqualTo("definition1");
assertThat(ContractOfferId.parseId(id)).isSucceeded().extracting(ContractOfferId::definitionPart).asString().isEqualTo("definition1");
assertThat(policy.getType()).isEqualTo(SET);
})
.anySatisfy((id, policy) -> {
assertThat(ContractId.parseId(id)).isSucceeded().extracting(ContractId::definitionPart).asString().isEqualTo("definition2");
assertThat(ContractOfferId.parseId(id)).isSucceeded().extracting(ContractOfferId::definitionPart).asString().isEqualTo("definition2");
assertThat(policy.getType()).isEqualTo(OFFER);
});
});
Expand Down Expand Up @@ -238,11 +238,11 @@ void getById_shouldReturnDataset() {
assertThat(dataset.getId()).isEqualTo("datasetId");
assertThat(dataset.getOffers()).hasSize(2)
.anySatisfy((id, policy) -> {
assertThat(ContractId.parseId(id)).isSucceeded().extracting(ContractId::definitionPart).isEqualTo("definition1");
assertThat(ContractOfferId.parseId(id)).isSucceeded().extracting(ContractOfferId::definitionPart).isEqualTo("definition1");
assertThat(policy.getType()).isEqualTo(SET);
})
.anySatisfy((id, policy) -> {
assertThat(ContractId.parseId(id)).isSucceeded().extracting(ContractId::definitionPart).isEqualTo("definition2");
assertThat(ContractOfferId.parseId(id)).isSucceeded().extracting(ContractOfferId::definitionPart).isEqualTo("definition2");
assertThat(policy.getType()).isEqualTo(OFFER);
});
verify(assetIndex).findById("datasetId");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.eclipse.edc.connector.contract.negotiation;

import io.opentelemetry.instrumentation.annotations.WithSpan;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.negotiation.ConsumerContractNegotiationManager;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreement;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreementMessage;
Expand Down Expand Up @@ -156,16 +155,8 @@ private boolean processRequesting(ContractNegotiation negotiation) {
private boolean processAccepting(ContractNegotiation negotiation) {
var lastOffer = negotiation.getLastContractOffer();

var contractIdResult = ContractId.parseId(lastOffer.getId());
if (contractIdResult.failed()) {
monitor.severe("ConsumerContractNegotiationManagerImpl.approveContractOffers(): Offer Id not correctly formatted: " + contractIdResult.getFailureDetail());
return false;
}
var contractId = contractIdResult.getContent();

var policy = lastOffer.getPolicy();
var agreement = ContractAgreement.Builder.newInstance()
.id(contractId.derive().toString())
.contractSigningDate(clock.instant().getEpochSecond())
.providerId(negotiation.getCounterPartyId())
.consumerId(participantId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.eclipse.edc.connector.contract.negotiation;

import io.opentelemetry.instrumentation.annotations.WithSpan;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.negotiation.ProviderContractNegotiationManager;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreement;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreementMessage;
Expand Down Expand Up @@ -165,16 +164,7 @@ private boolean processAgreeing(ContractNegotiation negotiation) {
if (retrievedAgreement == null) {
var lastOffer = negotiation.getLastContractOffer();

var contractIdResult = ContractId.parseId(lastOffer.getId());
if (contractIdResult.failed()) {
monitor.severe("ProviderContractNegotiationManagerImpl.checkConfirming(): Offer Id not correctly formatted: " + contractIdResult.getFailureDetail());
return false;
}

var contractId = contractIdResult.getContent();

agreement = ContractAgreement.Builder.newInstance()
.id(contractId.derive().toString())
.contractSigningDate(clock.instant().getEpochSecond())
.providerId(participantId)
.consumerId(negotiation.getCounterPartyId())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package org.eclipse.edc.connector.contract.validation;

import org.eclipse.edc.connector.contract.policy.PolicyEquality;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.ContractOfferId;
import org.eclipse.edc.connector.contract.spi.offer.ContractDefinitionResolver;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreement;
import org.eclipse.edc.connector.contract.spi.types.negotiation.ContractNegotiation;
Expand Down Expand Up @@ -80,7 +80,7 @@ public Result<ValidatedConsumerOffer> validateInitialOffer(ClaimToken token, Con

@Override
public @NotNull Result<ValidatedConsumerOffer> validateInitialOffer(ClaimToken token, String offerId) {
return ContractId.parseId(offerId)
return ContractOfferId.parseId(offerId)
.compose(contractId -> {
var agent = agentService.createFor(token);

Expand All @@ -96,7 +96,7 @@ public Result<ValidatedConsumerOffer> validateInitialOffer(ClaimToken token, Con
@Override
@NotNull
public Result<ContractAgreement> validateAgreement(ClaimToken token, ContractAgreement agreement) {
return ContractId.parseId(agreement.getId())
return ContractOfferId.parseId(agreement.getId())
.compose(contractId -> {
var agent = agentService.createFor(token);
var consumerIdentity = agent.getIdentity();
Expand Down Expand Up @@ -158,26 +158,26 @@ public Result<Void> validateConfirmed(ClaimToken token, ContractAgreement agreem
* Validates an initial contract offer, ensuring that the referenced asset exists, is selected by the corresponding policy definition and the agent fulfills the contract policy.
* A sanitized policy definition is returned to avoid clients injecting manipulated policies.
*/
private Result<Policy> validateInitialOffer(ContractId contractId, ParticipantAgent agent) {
private Result<Policy> validateInitialOffer(ContractOfferId contractOfferId, ParticipantAgent agent) {
var consumerIdentity = agent.getIdentity();
if (consumerIdentity == null) {
return failure("Invalid consumer identity");
}

var contractDefinition = contractDefinitionResolver.definitionFor(agent, contractId.definitionPart());
var contractDefinition = contractDefinitionResolver.definitionFor(agent, contractOfferId.definitionPart());
if (contractDefinition == null) {
return failure("The ContractDefinition with id %s either does not exist or the access to it is not granted.");
}

// verify the target asset exists
var targetAsset = assetIndex.findById(contractId.assetIdPart());
var targetAsset = assetIndex.findById(contractOfferId.assetIdPart());
if (targetAsset == null) {
return failure("Invalid target: " + contractId.assetIdPart());
return failure("Invalid target: " + contractOfferId.assetIdPart());
}

// verify that the asset in the offer is actually in the contract definition
var testCriteria = new ArrayList<>(contractDefinition.getAssetsSelector());
testCriteria.add(new Criterion(Asset.PROPERTY_ID, "=", contractId.assetIdPart()));
testCriteria.add(new Criterion(Asset.PROPERTY_ID, "=", contractOfferId.assetIdPart()));
if (assetIndex.countAssets(testCriteria) <= 0) {
return failure("Asset ID from the ContractOffer is not included in the ContractDefinition");
}
Expand All @@ -187,7 +187,7 @@ private Result<Policy> validateInitialOffer(ContractId contractId, ParticipantAg
return failure(format("Policy %s not found", contractDefinition.getContractPolicyId()));
}

var policy = policyDefinition.getPolicy().withTarget(contractId.assetIdPart());
var policy = policyDefinition.getPolicy().withTarget(contractOfferId.assetIdPart());
var policyContext = PolicyContextImpl.Builder.newInstance().additional(ParticipantAgent.class, agent).build();
var policyResult = policyEngine.evaluate(NEGOTIATION_SCOPE, policy, policyContext);
if (policyResult.failed()) {
Expand All @@ -197,11 +197,11 @@ private Result<Policy> validateInitialOffer(ContractId contractId, ParticipantAg
}

@NotNull
private ContractOffer createContractOffer(Policy policy, ContractId contractId) {
private ContractOffer createContractOffer(Policy policy, ContractOfferId contractOfferId) {
return ContractOffer.Builder.newInstance()
.id(contractId.toString())
.id(contractOfferId.toString())
.policy(policy)
.assetId(contractId.assetIdPart())
.assetId(contractOfferId.assetIdPart())
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package org.eclipse.edc.connector.contract.negotiation;

import org.eclipse.edc.connector.contract.observe.ContractNegotiationObservableImpl;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.ContractOfferId;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreement;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreementMessage;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreementVerificationMessage;
Expand Down Expand Up @@ -325,7 +325,7 @@ private CompletableFuture<?> toFuture(ServiceResult<ContractNegotiation> result)
*/
private ContractOffer getContractOffer() {
return ContractOffer.Builder.newInstance()
.id(ContractId.create("1", "test-asset-id").toString())
.id(ContractOfferId.create("1", "test-asset-id").toString())
.assetId(randomUUID().toString())
.policy(Policy.Builder.newInstance()
.type(PolicyType.CONTRACT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package org.eclipse.edc.connector.contract.negotiation;

import org.eclipse.edc.connector.contract.observe.ContractNegotiationObservableImpl;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.ContractOfferId;
import org.eclipse.edc.connector.contract.spi.negotiation.ContractNegotiationPendingGuard;
import org.eclipse.edc.connector.contract.spi.negotiation.observe.ContractNegotiationListener;
import org.eclipse.edc.connector.contract.spi.negotiation.store.ContractNegotiationStore;
Expand Down Expand Up @@ -274,7 +274,7 @@ private ContractNegotiation.Builder contractNegotiationBuilder() {

private ContractAgreement.Builder contractAgreementBuilder() {
return ContractAgreement.Builder.newInstance()
.id(ContractId.create(UUID.randomUUID().toString(), "test-asset-id").toString())
.id(ContractOfferId.create(UUID.randomUUID().toString(), "test-asset-id").toString())
.providerId("any")
.consumerId("any")
.assetId("default")
Expand All @@ -283,7 +283,7 @@ private ContractAgreement.Builder contractAgreementBuilder() {

private ContractOffer contractOffer() {
return ContractOffer.Builder.newInstance()
.id(ContractId.create("1", "test-asset-id").toString())
.id(ContractOfferId.create("1", "test-asset-id").toString())
.policy(Policy.Builder.newInstance().build())
.assetId("assetId")
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
package org.eclipse.edc.connector.contract.validation;

import org.eclipse.edc.connector.contract.policy.PolicyEquality;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.ContractOfferId;
import org.eclipse.edc.connector.contract.spi.offer.ContractDefinitionResolver;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreement;
import org.eclipse.edc.connector.contract.spi.types.negotiation.ContractNegotiation;
Expand Down Expand Up @@ -473,7 +473,7 @@ void validateAgreement_failWhenOutsideInForcePeriod_fixed() {

var claimToken = ClaimToken.Builder.newInstance().build();
var agreement = createContractAgreement()
.id(ContractId.create("1", "2").toString())
.id(ContractOfferId.create("1", "2").toString())
.contractSigningDate(Instant.now().toEpochMilli())
.build();

Expand All @@ -488,7 +488,7 @@ private Result<ContractAgreement> validateAgreementDate(long signingDate) {

var claimToken = ClaimToken.Builder.newInstance().build();
var agreement = createContractAgreement()
.id(ContractId.create("1", "2").toString())
.id(ContractOfferId.create("1", "2").toString())
.contractSigningDate(signingDate)
.build();

Expand All @@ -497,7 +497,7 @@ private Result<ContractAgreement> validateAgreementDate(long signingDate) {

private ContractOffer createContractOffer(Asset asset, Policy policy) {
return ContractOffer.Builder.newInstance()
.id(ContractId.create("1", asset.getId()).toString())
.id(ContractOfferId.create("1", asset.getId()).toString())
.assetId(asset.getId())
.policy(policy)
.build();
Expand All @@ -514,7 +514,7 @@ private ContractDefinition createContractDefinition() {
}

private ContractAgreement.Builder createContractAgreement() {
return ContractAgreement.Builder.newInstance().id(ContractId.create("1", "2").toString())
return ContractAgreement.Builder.newInstance().id(ContractOfferId.create("1", "2").toString())
.providerId(PROVIDER_ID)
.consumerId(CONSUMER_ID)
.policy(Policy.Builder.newInstance().build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package org.eclipse.edc.connector.service.transferprocess;

import io.opentelemetry.instrumentation.annotations.WithSpan;
import org.eclipse.edc.connector.contract.spi.ContractId;
import org.eclipse.edc.connector.contract.spi.negotiation.store.ContractNegotiationStore;
import org.eclipse.edc.connector.contract.spi.validation.ContractValidationService;
import org.eclipse.edc.connector.spi.transferprocess.TransferProcessProtocolService;
Expand Down Expand Up @@ -83,11 +82,6 @@ public TransferProcessProtocolServiceImpl(TransferProcessStore transferProcessSt
@WithSpan
@NotNull
public ServiceResult<TransferProcess> notifyRequested(TransferRequestMessage message, ClaimToken claimToken) {
var contractIdResult = ContractId.parseId(message.getContractId());
if (contractIdResult.failed()) {
return ServiceResult.badRequest("ContractId is not valid: " + contractIdResult.getFailureDetail());
}

var destination = message.getDataDestination();
if (destination != null) {
var validDestination = dataAddressValidator.validate(destination);
Expand All @@ -99,7 +93,7 @@ public ServiceResult<TransferProcess> notifyRequested(TransferRequestMessage mes
return transactionContext.execute(() ->
Optional.ofNullable(negotiationStore.findContractAgreement(message.getContractId()))
.filter(agreement -> contractValidationService.validateAgreement(claimToken, agreement).succeeded())
.map(agreement -> requestedAction(message, contractIdResult.getContent()))
.map(agreement -> requestedAction(message, agreement.getAssetId()))
.orElse(ServiceResult.conflict(format("Cannot process %s because %s", message.getClass().getSimpleName(), "agreement not found or not valid"))));
}

Expand Down Expand Up @@ -135,9 +129,7 @@ public ServiceResult<TransferProcess> findById(String id, ClaimToken claimToken)
}

@NotNull
private ServiceResult<TransferProcess> requestedAction(TransferRequestMessage message, ContractId contractId) {
var assetId = contractId.assetIdPart();

private ServiceResult<TransferProcess> requestedAction(TransferRequestMessage message, String assetId) {
var destination = message.getDataDestination() != null ? message.getDataDestination() :
DataAddress.Builder.newInstance().type(HTTP_PROXY).build();
var dataRequest = DataRequest.Builder.newInstance()
Expand Down
Loading

0 comments on commit b3ec2a2

Please sign in to comment.