From 131098afdab9917e8041b9b782e25bb82c43f0d1 Mon Sep 17 00:00:00 2001 From: Christophe Loiseau Date: Wed, 2 Oct 2024 11:53:11 +0200 Subject: [PATCH 1/6] fix: Abide to the DataOfferCreationRequest.PolicyEnum rule --- extensions/wrapper/wrapper/build.gradle.kts | 1 + .../data_offer/DataOfferPageApiService.java | 77 ++++++++++++---- .../de/sovity/edc/e2e/UiApiWrapperTest.java | 87 ++++++++++++++++++- 3 files changed, 142 insertions(+), 23 deletions(-) diff --git a/extensions/wrapper/wrapper/build.gradle.kts b/extensions/wrapper/wrapper/build.gradle.kts index 027f3c567..85be60875 100644 --- a/extensions/wrapper/wrapper/build.gradle.kts +++ b/extensions/wrapper/wrapper/build.gradle.kts @@ -8,6 +8,7 @@ dependencies { annotationProcessor(libs.lombok) compileOnly(libs.lombok) + api(project(":extensions:policy-always-true")) api(project(":extensions:wrapper:wrapper-api")) api(project(":extensions:wrapper:wrapper-common-mappers")) api(project(":utils:catalog-parser")) diff --git a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java index 4bdfd949a..f595139f7 100644 --- a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java +++ b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java @@ -13,6 +13,7 @@ import de.sovity.edc.ext.wrapper.api.ui.pages.asset.AssetApiService; import de.sovity.edc.ext.wrapper.api.ui.pages.contract_definitions.ContractDefinitionApiService; import de.sovity.edc.ext.wrapper.api.ui.pages.policy.PolicyDefinitionApiService; +import de.sovity.edc.extension.policy.AlwaysTruePolicyConstants; import lombok.RequiredArgsConstructor; import lombok.val; import org.eclipse.edc.spi.types.domain.asset.Asset; @@ -66,22 +67,30 @@ private boolean isIdAvailable(DSLContext dsl, Table table, TableField createButDontPublish(dsl, dataOfferCreationRequest, commonId); + case PUBLISH_UNRESTRICTED -> createAndPublishUnrestricted(dsl, dataOfferCreationRequest, commonId); + case PUBLISH_RESTRICTED -> createAndPublishRestricted(dsl, dataOfferCreationRequest, commonId); + }; + } - val policyIdExists = checkIfPolicyIdAvailable(dsl, commonId).isAvailable(); - if (!policyIdExists) { - throw new InvalidRequestException("Policy with id %s already exists".formatted(commonId)); - } + private @NotNull IdResponseDto createAndPublishUnrestricted(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { + checkAssetIdAvailable(dsl, commonId); + checkContractDefinitionIdAvailable(dsl, commonId); + val policyId = AlwaysTruePolicyConstants.POLICY_DEFINITION_ID; - val contractDefinitionIdExists = checkIfContractDefinitionIdAvailable(dsl, commonId).isAvailable(); - if (!contractDefinitionIdExists) { - throw new InvalidRequestException("Contract definition with id %s already exists".formatted(commonId)); - } + assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); + + return createContractDefinition(commonId, policyId, commonId); + } + + private @NotNull IdResponseDto createAndPublishRestricted(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { + checkAssetIdAvailable(dsl, commonId); + checkPolicyIdAvailable(dsl, commonId); + checkContractDefinitionIdAvailable(dsl, commonId); assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); @@ -90,21 +99,51 @@ public IdResponseDto createDataOffer(DSLContext dsl, DataOfferCreationRequest da maybeNewPolicy.ifPresent( policy -> policyDefinitionApiService.createPolicyDefinitionV2(new PolicyDefinitionCreateDto(commonId, policy))); + createContractDefinition(commonId, commonId, commonId); + + return new IdResponseDto(commonId, OffsetDateTime.now()); + } + + private @NotNull IdResponseDto createButDontPublish(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { + checkAssetIdAvailable(dsl, commonId); + return assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); + } + + private void checkContractDefinitionIdAvailable(DSLContext dsl, String commonId) { + val contractDefinitionIdExists = checkIfContractDefinitionIdAvailable(dsl, commonId).isAvailable(); + if (!contractDefinitionIdExists) { + throw new InvalidRequestException("Contract definition with id %s already exists".formatted(commonId)); + } + } + + private void checkPolicyIdAvailable(DSLContext dsl, String commonId) { + val policyIdExists = checkIfPolicyIdAvailable(dsl, commonId).isAvailable(); + if (!policyIdExists) { + throw new InvalidRequestException("Policy with id %s already exists".formatted(commonId)); + } + } + + private void checkAssetIdAvailable(DSLContext dsl, String commonId) { + val assetIdExists = checkIfAssetIdAvailable(dsl, commonId).isAvailable(); + if (!assetIdExists) { + throw new InvalidRequestException("Asset with id %s already exists".formatted(commonId)); + } + } + + private @NotNull IdResponseDto createContractDefinition(String assetId, String policyId, String contractDefinitionId) { val cd = new ContractDefinitionRequest(); cd.setAssetSelector(List.of(UiCriterion.builder() .operandLeft(Asset.PROPERTY_ID) .operator(UiCriterionOperator.EQ) .operandRight(UiCriterionLiteral.builder() .type(UiCriterionLiteralType.VALUE) - .value(commonId) + .value(assetId) .build()) .build())); - cd.setAccessPolicyId(commonId); - cd.setContractPolicyId(commonId); - cd.setContractDefinitionId(commonId); + cd.setAccessPolicyId(policyId); + cd.setContractPolicyId(policyId); + cd.setContractDefinitionId(contractDefinitionId); - contractDefinitionApiService.createContractDefinition(cd); - - return new IdResponseDto(commonId, OffsetDateTime.now()); + return contractDefinitionApiService.createContractDefinition(cd); } } diff --git a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java index 0b9875b62..a61e33a1d 100644 --- a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java +++ b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java @@ -726,9 +726,7 @@ void canCreateDataOfferWithoutAnyNewPolicy( assertThat(getAllPoliciesExceptTheAlwaysTruePolicy(providerClient)).hasSize(0); assertThat(providerClient.uiApi().getContractDefinitionPage().getContractDefinitions()) - .extracting(ContractDefinitionEntry::getContractDefinitionId) - .first() - .isEqualTo(assetId); + .hasSize(0); } @Test @@ -753,7 +751,7 @@ void canCreateDataOfferWithNewPolicy( val dataOfferCreateRequest = new DataOfferCreationRequest( asset, - DataOfferCreationRequest.PolicyEnum.DONT_PUBLISH, + DataOfferCreationRequest.PolicyEnum.PUBLISH_RESTRICTED, UiPolicyExpression.builder() .constraint(UiPolicyConstraint.builder() .left("foo") @@ -919,6 +917,87 @@ void dontCreateAnythingIfTheContractDefinitionAlreadyExists( .isEqualTo(assetId); } + @Test + void reuseTheAlwaysTruePolicyWhenPublishingUnrestricted( + E2eScenario scenario, + @Provider EdcClient providerClient + ) { + // arrange + val assetId = "assetId"; + + // act + providerClient.uiApi() + .createDataOffer(DataOfferCreationRequest.builder() + .uiAssetCreateRequest(UiAssetCreateRequest.builder() + .id(assetId) + .dataSource(UiDataSource.builder() + .type(DataSourceType.ON_REQUEST) + .onRequest(UiDataSourceOnRequest.builder() + .contactEmail("foo@example.com") + .contactPreferredEmailSubject("Subject") + .build()) + .build()) + .build()) + .policy(DataOfferCreationRequest.PolicyEnum.PUBLISH_UNRESTRICTED) + .build()); + + // assert + assertThat(providerClient.uiApi().getAssetPage().getAssets()) + // the asset used for the placeholder contract definition + .hasSize(1) + .extracting(UiAsset::getAssetId) + .first() + .isEqualTo(assetId); + + assertThat(getAllPoliciesExceptTheAlwaysTruePolicy(providerClient)).hasSize(0); + + assertThat(providerClient.uiApi().getContractDefinitionPage().getContractDefinitions()) + .hasSize(1) + .filteredOn(it -> it.getContractDefinitionId().equals(assetId)) + .extracting(ContractDefinitionEntry::getContractDefinitionId) + .first() + // the already existing one, before the data offer creation attempt + .isEqualTo(assetId); + } + + @Test + void onlyCreateTheAssetWhenDontPublish( + E2eScenario scenario, + @Provider EdcClient providerClient + ) { + // arrange + val assetId = "assetId"; + + // act + providerClient.uiApi() + .createDataOffer(DataOfferCreationRequest.builder() + .uiAssetCreateRequest(UiAssetCreateRequest.builder() + .id(assetId) + .dataSource(UiDataSource.builder() + .type(DataSourceType.ON_REQUEST) + .onRequest(UiDataSourceOnRequest.builder() + .contactEmail("foo@example.com") + .contactPreferredEmailSubject("Subject") + .build()) + .build()) + .build()) + .policy(DataOfferCreationRequest.PolicyEnum.DONT_PUBLISH) + .build()); + + // assert + assertThat(providerClient.uiApi().getAssetPage().getAssets()) + // the asset used for the placeholder contract definition + .hasSize(1) + .extracting(UiAsset::getAssetId) + .first() + .isEqualTo(assetId); + + assertThat(getAllPoliciesExceptTheAlwaysTruePolicy(providerClient)).hasSize(0); + + assertThat(providerClient.uiApi().getContractDefinitionPage().getContractDefinitions()) + .hasSize(0); + } + private static @NotNull List getAllPoliciesExceptTheAlwaysTruePolicy(EdcClient edcClient) { return edcClient.uiApi().getPolicyDefinitionPage().getPolicies().stream().filter(it -> !it.getPolicyDefinitionId().equals( AlwaysTruePolicyConstants.POLICY_DEFINITION_ID)).toList(); From fb80788b0e020f3dd22f1dff31920f4776c0d4ba Mon Sep 17 00:00:00 2001 From: Christophe Loiseau Date: Wed, 2 Oct 2024 11:55:01 +0200 Subject: [PATCH 2/6] Changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 939153876..6968f6804 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ please see [changelog_updates.md](docs/dev/changelog_updates.md). #### Patch Changes +- Fix issues with the Create Data Offer Endpoint ([PR#1055](https://github.com/sovity/edc-ce/pull/1055)) + ### Deployment Migration Notes _No special deployment migration steps required_ From f141509ad9922fd6c599e553817696d4d7c6331f Mon Sep 17 00:00:00 2001 From: Christophe Loiseau Date: Wed, 2 Oct 2024 12:50:41 +0200 Subject: [PATCH 3/6] self review --- .../ui/model/DataOfferCreationRequest.java | 2 +- .../ui/model/PolicyDefinitionChoiceEnum.java | 7 ++++++ .../data_offer/DataOfferPageApiService.java | 23 ++++++++++++------- .../de/sovity/edc/e2e/UiApiWrapperTest.java | 2 +- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/DataOfferCreationRequest.java b/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/DataOfferCreationRequest.java index 705aff710..74aa91d59 100644 --- a/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/DataOfferCreationRequest.java +++ b/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/DataOfferCreationRequest.java @@ -35,7 +35,7 @@ public class DataOfferCreationRequest { @Schema(description = "The asset to create", requiredMode = REQUIRED) private UiAssetCreateRequest uiAssetCreateRequest; - @Schema(description = "Which policy to apply to this asset.", requiredMode = REQUIRED) + @Schema(description = "Which policy to apply to this asset creation.", requiredMode = REQUIRED) private PolicyDefinitionChoiceEnum policy; @Schema(description = "Policy Expression.", requiredMode = NOT_REQUIRED) diff --git a/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/PolicyDefinitionChoiceEnum.java b/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/PolicyDefinitionChoiceEnum.java index 3f494f240..a8ed13c7a 100644 --- a/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/PolicyDefinitionChoiceEnum.java +++ b/extensions/wrapper/wrapper-api/src/main/java/de/sovity/edc/ext/wrapper/api/ui/model/PolicyDefinitionChoiceEnum.java @@ -14,8 +14,15 @@ package de.sovity.edc.ext.wrapper.api.ui.model; +import io.swagger.v3.oas.annotations.media.Schema; + +import static io.swagger.v3.oas.annotations.media.Schema.RequiredMode.REQUIRED; + public enum PolicyDefinitionChoiceEnum { + @Schema(description = "Only create the asset", requiredMode = REQUIRED) DONT_PUBLISH, + @Schema(description = "Create the asset and assigns the always-true policy in the contract definition", requiredMode = REQUIRED) PUBLISH_UNRESTRICTED, + @Schema(description = "Create the asset, a policy and a contract definition", requiredMode = REQUIRED) PUBLISH_RESTRICTED } diff --git a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java index f595139f7..03fe76e3d 100644 --- a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java +++ b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java @@ -78,28 +78,35 @@ public IdResponseDto createDataOffer(DSLContext dsl, DataOfferCreationRequest da } private @NotNull IdResponseDto createAndPublishUnrestricted(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { - checkAssetIdAvailable(dsl, commonId); - checkContractDefinitionIdAvailable(dsl, commonId); + val assetId = commonId; + val contractDefinitionId = commonId; val policyId = AlwaysTruePolicyConstants.POLICY_DEFINITION_ID; + checkAssetIdAvailable(dsl, assetId); + checkContractDefinitionIdAvailable(dsl, contractDefinitionId); + assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); - return createContractDefinition(commonId, policyId, commonId); + return createContractDefinition(assetId, policyId, contractDefinitionId); } private @NotNull IdResponseDto createAndPublishRestricted(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { - checkAssetIdAvailable(dsl, commonId); - checkPolicyIdAvailable(dsl, commonId); - checkContractDefinitionIdAvailable(dsl, commonId); + val assetId = commonId; + val policyId = commonId; + val contractDefinitionId = commonId; + + checkAssetIdAvailable(dsl, assetId); + checkPolicyIdAvailable(dsl, policyId); + checkContractDefinitionIdAvailable(dsl, contractDefinitionId); assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); val maybeNewPolicy = Optional.ofNullable(dataOfferCreationRequest.getUiPolicyExpression()); maybeNewPolicy.ifPresent( - policy -> policyDefinitionApiService.createPolicyDefinitionV2(new PolicyDefinitionCreateDto(commonId, policy))); + policy -> policyDefinitionApiService.createPolicyDefinitionV2(new PolicyDefinitionCreateDto(policyId, policy))); - createContractDefinition(commonId, commonId, commonId); + createContractDefinition(assetId, policyId, contractDefinitionId); return new IdResponseDto(commonId, OffsetDateTime.now()); } diff --git a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java index a61e33a1d..1bfb63c09 100644 --- a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java +++ b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java @@ -687,7 +687,7 @@ void canMakeAnOnDemandDataSourceAvailable( } @Test - void canCreateDataOfferWithoutAnyNewPolicy( + void canCreateDataOfferWithoutAnyNewPolicyNotContractDefinition( @Provider EdcClient providerClient ) { // arrange From 17d2cff5bde1436dcfe9fe0f6e815ef6cffc97b5 Mon Sep 17 00:00:00 2001 From: Christophe Loiseau Date: Wed, 2 Oct 2024 15:03:47 +0200 Subject: [PATCH 4/6] Core review --- docs/api/sovity-edc-api-wrapper.yaml | 6 +- .../edc/ext/wrapper/WrapperExtension.java | 4 ++ .../WrapperExtensionContextBuilder.java | 10 +++- .../data_offer/DataOfferPageApiService.java | 32 +++++++--- .../edc/e2e/AlwaysTrueMigrationTest.java | 4 +- .../de/sovity/edc/e2e/UiApiWrapperTest.java | 59 +++++++++++++++++-- 6 files changed, 98 insertions(+), 17 deletions(-) diff --git a/docs/api/sovity-edc-api-wrapper.yaml b/docs/api/sovity-edc-api-wrapper.yaml index 0b2e49462..a6400de63 100644 --- a/docs/api/sovity-edc-api-wrapper.yaml +++ b/docs/api/sovity-edc-api-wrapper.yaml @@ -623,11 +623,11 @@ components: DataSourceType: type: string description: Supported Data Source Types by UiDataSource - default: CUSTOM enum: - HTTP_DATA - ON_REQUEST - CUSTOM + default: CUSTOM SecretValue: type: object properties: @@ -835,7 +835,6 @@ components: UiDataSourceHttpDataMethod: type: string description: Supported HTTP Methods by UiDataSource - default: GET enum: - GET - POST @@ -843,6 +842,7 @@ components: - PATCH - DELETE - OPTIONS + default: GET UiDataSourceOnRequest: required: - contactEmail @@ -948,7 +948,7 @@ components: $ref: '#/components/schemas/UiAssetCreateRequest' policy: type: string - description: Which policy to apply to this asset. + description: Which policy to apply to this asset creation. enum: - DONT_PUBLISH - PUBLISH_UNRESTRICTED diff --git a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtension.java b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtension.java index 5863f579a..5dbd5e9e9 100644 --- a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtension.java +++ b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtension.java @@ -34,6 +34,7 @@ import org.eclipse.edc.connector.transfer.spi.store.TransferProcessStore; import org.eclipse.edc.jsonld.spi.JsonLd; import org.eclipse.edc.policy.engine.spi.PolicyEngine; +import org.eclipse.edc.policy.engine.spi.RuleBindingRegistry; import org.eclipse.edc.protocol.dsp.api.configuration.DspApiConfiguration; import org.eclipse.edc.runtime.metamodel.annotation.Inject; import org.eclipse.edc.spi.CoreConstants; @@ -74,6 +75,8 @@ public class WrapperExtension implements ServiceExtension { @Inject private PolicyEngine policyEngine; @Inject + private RuleBindingRegistry ruleBindingRegistry; + @Inject private TransferProcessService transferProcessService; @Inject private TransferProcessStore transferProcessStore; @@ -118,6 +121,7 @@ public void initialize(ServiceExtensionContext context) { policyDefinitionService, policyDefinitionStore, policyEngine, + ruleBindingRegistry, transferProcessService, transferProcessStore, typeTransformerRegistry diff --git a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java index e63f5ff30..5311492bf 100644 --- a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java +++ b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java @@ -79,6 +79,8 @@ import de.sovity.edc.ext.wrapper.controller.PlaceholderEndpointController; import de.sovity.edc.extension.contacttermination.ContractAgreementTerminationService; import de.sovity.edc.extension.db.directaccess.DslContextFactory; +import de.sovity.edc.extension.policy.services.AlwaysTruePolicyDefinitionService; +import de.sovity.edc.extension.policy.services.AlwaysTruePolicyService; import de.sovity.edc.utils.catalog.DspCatalogService; import de.sovity.edc.utils.catalog.mapper.DspDataOfferBuilder; import de.sovity.edc.utils.config.ConfigProps; @@ -97,6 +99,7 @@ import org.eclipse.edc.connector.transfer.spi.store.TransferProcessStore; import org.eclipse.edc.jsonld.spi.JsonLd; import org.eclipse.edc.policy.engine.spi.PolicyEngine; +import org.eclipse.edc.policy.engine.spi.RuleBindingRegistry; import org.eclipse.edc.runtime.metamodel.annotation.Inject; import org.eclipse.edc.spi.asset.AssetIndex; import org.eclipse.edc.spi.monitor.Monitor; @@ -136,6 +139,7 @@ public static WrapperExtensionContext buildContext( PolicyDefinitionService policyDefinitionService, PolicyDefinitionStore policyDefinitionStore, PolicyEngine policyEngine, + RuleBindingRegistry ruleBindingRegistry, TransferProcessService transferProcessService, TransferProcessStore transferProcessStore, TypeTransformerRegistry typeTransformerRegistry @@ -251,10 +255,14 @@ public static WrapperExtensionContext buildContext( miwConfigBuilder, selfDescriptionService ); + var alwaysTruePolicyService = new AlwaysTruePolicyDefinitionService( + policyDefinitionService + ); var dataOfferPageApiService = new DataOfferPageApiService( assetApiService, contractDefinitionApiService, - policyDefinitionApiService + policyDefinitionApiService, + alwaysTruePolicyService ); var uiResource = new UiResourceImpl( contractAgreementApiService, diff --git a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java index 03fe76e3d..81fd36c2e 100644 --- a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java +++ b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/api/ui/pages/data_offer/DataOfferPageApiService.java @@ -14,6 +14,7 @@ import de.sovity.edc.ext.wrapper.api.ui.pages.contract_definitions.ContractDefinitionApiService; import de.sovity.edc.ext.wrapper.api.ui.pages.policy.PolicyDefinitionApiService; import de.sovity.edc.extension.policy.AlwaysTruePolicyConstants; +import de.sovity.edc.extension.policy.services.AlwaysTruePolicyDefinitionService; import lombok.RequiredArgsConstructor; import lombok.val; import org.eclipse.edc.spi.types.domain.asset.Asset; @@ -33,6 +34,7 @@ public class DataOfferPageApiService { private final AssetApiService assetApiService; private final ContractDefinitionApiService contractDefinitionApiService; private final PolicyDefinitionApiService policyDefinitionApiService; + private final AlwaysTruePolicyDefinitionService alwaysTruePolicyDefinitionService; @NotNull public IdAvailabilityResponse checkIfPolicyIdAvailable(DSLContext dsl, String id) { @@ -77,7 +79,11 @@ public IdResponseDto createDataOffer(DSLContext dsl, DataOfferCreationRequest da }; } - private @NotNull IdResponseDto createAndPublishUnrestricted(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { + private @NotNull IdResponseDto createAndPublishUnrestricted( + DSLContext dsl, + DataOfferCreationRequest dataOfferCreationRequest, + String commonId + ) { val assetId = commonId; val contractDefinitionId = commonId; val policyId = AlwaysTruePolicyConstants.POLICY_DEFINITION_ID; @@ -85,12 +91,21 @@ public IdResponseDto createDataOffer(DSLContext dsl, DataOfferCreationRequest da checkAssetIdAvailable(dsl, assetId); checkContractDefinitionIdAvailable(dsl, contractDefinitionId); + if (!alwaysTruePolicyDefinitionService.exists()) { + // the default always-true policy has been deleted, recreate it. + alwaysTruePolicyDefinitionService.create(); + } + assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); return createContractDefinition(assetId, policyId, contractDefinitionId); } - private @NotNull IdResponseDto createAndPublishRestricted(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { + private @NotNull IdResponseDto createAndPublishRestricted( + DSLContext dsl, + DataOfferCreationRequest dataOfferCreationRequest, + String commonId + ) { val assetId = commonId; val policyId = commonId; val contractDefinitionId = commonId; @@ -101,17 +116,20 @@ public IdResponseDto createDataOffer(DSLContext dsl, DataOfferCreationRequest da assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); - val maybeNewPolicy = Optional.ofNullable(dataOfferCreationRequest.getUiPolicyExpression()); - - maybeNewPolicy.ifPresent( - policy -> policyDefinitionApiService.createPolicyDefinitionV2(new PolicyDefinitionCreateDto(policyId, policy))); + val policyExpression = Optional.ofNullable(dataOfferCreationRequest.getUiPolicyExpression()) + .orElseThrow(() -> new InvalidRequestException("Missing policy expression")); + policyDefinitionApiService.createPolicyDefinitionV2(new PolicyDefinitionCreateDto(policyId, policyExpression)); createContractDefinition(assetId, policyId, contractDefinitionId); return new IdResponseDto(commonId, OffsetDateTime.now()); } - private @NotNull IdResponseDto createButDontPublish(DSLContext dsl, DataOfferCreationRequest dataOfferCreationRequest, String commonId) { + private @NotNull IdResponseDto createButDontPublish( + DSLContext dsl, + DataOfferCreationRequest dataOfferCreationRequest, + String commonId + ) { checkAssetIdAvailable(dsl, commonId); return assetApiService.createAsset(dataOfferCreationRequest.getUiAssetCreateRequest()); } diff --git a/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationTest.java b/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationTest.java index d9a22c7cb..d3bdc53ce 100644 --- a/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationTest.java +++ b/tests/src/test/java/de/sovity/edc/e2e/AlwaysTrueMigrationTest.java @@ -47,10 +47,10 @@ class AlwaysTrueMigrationTest { private static final E2eTestExtension E2E_TEST_EXTENSION = new E2eTestExtension( withModule(":launchers:connectors:sovity-dev").toBuilder() .consumerConfigCustomizer(config -> config.setProperty( - ConfigProps.EDC_FLYWAY_ADDITIONAL_MIGRATION_LOCATIONS, "classpath:db/additional-test-data/always-true-policy-legacy" + ConfigProps.EDC_FLYWAY_ADDITIONAL_MIGRATION_LOCATIONS, "classpath:db/additional-test-data/always-true-policy-migrated" )) .providerConfigCustomizer(config -> config.setProperty( - ConfigProps.EDC_FLYWAY_ADDITIONAL_MIGRATION_LOCATIONS, "classpath:db/additional-test-data/always-true-policy-migrated" + ConfigProps.EDC_FLYWAY_ADDITIONAL_MIGRATION_LOCATIONS, "classpath:db/additional-test-data/always-true-policy-legacy" )) .build() ); diff --git a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java index 061f46f1c..fd30c3ad5 100644 --- a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java +++ b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java @@ -583,8 +583,6 @@ void checkIdAvailability(E2eScenario scenario, @Provider EdcClient providerClien scenario.createPolicy(policyId, OffsetDateTime.MIN, OffsetDateTime.MAX); var contractDefinitionId = scenario.createContractDefinition(policyId, assetId); - val asset = providerClient.uiApi().getAssetPage(); - // act val negAssetResponse = providerClient.uiApi().isAssetIdAvailable(assetId); val negPolicyResponse = providerClient.uiApi().isPolicyIdAvailable(policyId); @@ -921,7 +919,6 @@ void dontCreateAnythingIfTheContractDefinitionAlreadyExists( @Test void reuseTheAlwaysTruePolicyWhenPublishingUnrestricted( - E2eScenario scenario, @Provider EdcClient providerClient ) { // arrange @@ -964,7 +961,6 @@ void reuseTheAlwaysTruePolicyWhenPublishingUnrestricted( @Test void onlyCreateTheAssetWhenDontPublish( - E2eScenario scenario, @Provider EdcClient providerClient ) { // arrange @@ -1000,6 +996,61 @@ void onlyCreateTheAssetWhenDontPublish( .hasSize(0); } + @Test + void recreateTheAlwaysTruePolicyIfDeleted( + @Provider EdcClient providerClient + ) { + // arrange + val assetId = "assetId"; + providerClient.uiApi().deletePolicyDefinition(AlwaysTruePolicyConstants.POLICY_DEFINITION_ID); + + List withoutDefaultAlwaysTrue = + providerClient.uiApi() + .getPolicyDefinitionPage() + .getPolicies() + .stream() + .filter(it -> !it.getPolicyDefinitionId().equals(AlwaysTruePolicyConstants.POLICY_DEFINITION_ID)) + .toList(); + + assertThat(withoutDefaultAlwaysTrue).hasSize(0); + + // act + providerClient.uiApi() + .createDataOffer(DataOfferCreationRequest.builder() + .uiAssetCreateRequest(UiAssetCreateRequest.builder() + .id(assetId) + .dataSource(UiDataSource.builder() + .type(DataSourceType.ON_REQUEST) + .onRequest(UiDataSourceOnRequest.builder() + .contactEmail("foo@example.com") + .contactPreferredEmailSubject("Subject") + .build()) + .build()) + .build()) + .policy(DataOfferCreationRequest.PolicyEnum.PUBLISH_UNRESTRICTED) + .build()); + + // assert + assertThat(providerClient.uiApi().getAssetPage().getAssets()) + // the asset used for the placeholder contract definition + .hasSize(1) + .extracting(UiAsset::getAssetId) + .first() + .isEqualTo(assetId); + + List policies = + providerClient.uiApi() + .getPolicyDefinitionPage() + .getPolicies() + .stream() + .toList(); + + assertThat(policies).hasSize(1); + + assertThat(providerClient.uiApi().getContractDefinitionPage().getContractDefinitions()) + .hasSize(1); + } + private static @NotNull List getAllPoliciesExceptTheAlwaysTruePolicy(EdcClient edcClient) { return edcClient.uiApi().getPolicyDefinitionPage().getPolicies().stream().filter(it -> !it.getPolicyDefinitionId().equals( AlwaysTruePolicyConstants.POLICY_DEFINITION_ID)).toList(); From de076b092fca22a56c09e07f6acbbe0804e2502d Mon Sep 17 00:00:00 2001 From: Christophe Loiseau Date: Wed, 2 Oct 2024 15:20:12 +0200 Subject: [PATCH 5/6] checkstyle --- .../sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java index 5311492bf..6df3d7068 100644 --- a/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java +++ b/extensions/wrapper/wrapper/src/main/java/de/sovity/edc/ext/wrapper/WrapperExtensionContextBuilder.java @@ -80,7 +80,6 @@ import de.sovity.edc.extension.contacttermination.ContractAgreementTerminationService; import de.sovity.edc.extension.db.directaccess.DslContextFactory; import de.sovity.edc.extension.policy.services.AlwaysTruePolicyDefinitionService; -import de.sovity.edc.extension.policy.services.AlwaysTruePolicyService; import de.sovity.edc.utils.catalog.DspCatalogService; import de.sovity.edc.utils.catalog.mapper.DspDataOfferBuilder; import de.sovity.edc.utils.config.ConfigProps; From 0d4d51708cfc45efc2a8d8a6701dd14462b56971 Mon Sep 17 00:00:00 2001 From: Christophe Loiseau Date: Wed, 2 Oct 2024 16:47:13 +0200 Subject: [PATCH 6/6] Add a unit test to ensure that it's possible to create a data offer with an empty (not null) policy definition --- .../de/sovity/edc/e2e/UiApiWrapperTest.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java index fd30c3ad5..c4d6950fa 100644 --- a/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java +++ b/tests/src/test/java/de/sovity/edc/e2e/UiApiWrapperTest.java @@ -786,6 +786,55 @@ void canCreateDataOfferWithNewPolicy( .isEqualTo(assetId); } + @Test + void canCreateDataOfferWithNewEmptyPolicyAndRestrictedPublishing( + @Provider EdcClient providerClient + ) { + // arrange + val dataSource = UiDataSource.builder() + .httpData(UiDataSourceHttpData.builder() + .baseUrl("http://example.com") + .method(UiDataSourceHttpDataMethod.GET) + .build()) + .type(DataSourceType.HTTP_DATA) + .build(); + + val assetId = "asset"; + val asset = UiAssetCreateRequest.builder() + .dataSource(dataSource) + .id(assetId) + .title("My asset") + .build(); + + val dataOfferCreateRequest = new DataOfferCreationRequest( + asset, + DataOfferCreationRequest.PolicyEnum.PUBLISH_RESTRICTED, + UiPolicyExpression.builder().build() + ); + + // act + val returnedId = providerClient.uiApi().createDataOffer(dataOfferCreateRequest).getId(); + + // assert + assertThat(returnedId).isEqualTo(assetId); + + assertThat(providerClient.uiApi().getAssetPage().getAssets()) + .extracting(UiAsset::getAssetId) + .first() + .isEqualTo(assetId); + + assertThat(getAllPoliciesExceptTheAlwaysTruePolicy(providerClient)) + .hasSize(1) + .extracting(PolicyDefinitionDto::getPolicyDefinitionId) + .first() + .isEqualTo(assetId); + + assertThat(providerClient.uiApi().getContractDefinitionPage().getContractDefinitions()) + .extracting(ContractDefinitionEntry::getContractDefinitionId) + .first() + .isEqualTo(assetId); + } + @Test void dontCreateAnythingIfTheAssetAlreadyExists( E2eScenario scenario,