diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/ControlPlaneServicesExtension.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/ControlPlaneServicesExtension.java index 98db2d63c0a..d466c3a1764 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/ControlPlaneServicesExtension.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/ControlPlaneServicesExtension.java @@ -28,6 +28,7 @@ import org.eclipse.edc.connector.controlplane.policy.spi.observe.PolicyDefinitionObservableImpl; import org.eclipse.edc.connector.controlplane.policy.spi.store.PolicyDefinitionStore; import org.eclipse.edc.connector.controlplane.services.asset.AssetEventListener; +import org.eclipse.edc.connector.controlplane.services.asset.AssetQueryValidator; import org.eclipse.edc.connector.controlplane.services.asset.AssetServiceImpl; import org.eclipse.edc.connector.controlplane.services.catalog.CatalogProtocolServiceImpl; import org.eclipse.edc.connector.controlplane.services.catalog.CatalogServiceImpl; @@ -40,6 +41,7 @@ import org.eclipse.edc.connector.controlplane.services.policydefinition.PolicyDefinitionServiceImpl; import org.eclipse.edc.connector.controlplane.services.protocol.ProtocolTokenValidatorImpl; import org.eclipse.edc.connector.controlplane.services.protocol.VersionProtocolServiceImpl; +import org.eclipse.edc.connector.controlplane.services.query.QueryValidators; import org.eclipse.edc.connector.controlplane.services.secret.SecretEventListener; import org.eclipse.edc.connector.controlplane.services.secret.SecretServiceImpl; import org.eclipse.edc.connector.controlplane.services.spi.asset.AssetService; @@ -199,7 +201,8 @@ public void initialize(ServiceExtensionContext context) { public AssetService assetService() { var assetObservable = new AssetObservableImpl(); assetObservable.registerListener(new AssetEventListener(clock, eventRouter)); - return new AssetServiceImpl(assetIndex, contractNegotiationStore, transactionContext, assetObservable, dataAddressValidator); + return new AssetServiceImpl(assetIndex, contractNegotiationStore, transactionContext, assetObservable, + dataAddressValidator, new AssetQueryValidator()); } @Provider @@ -222,19 +225,20 @@ public CatalogProtocolService catalogProtocolService(ServiceExtensionContext con @Provider public ContractAgreementService contractAgreementService() { - return new ContractAgreementServiceImpl(contractNegotiationStore, transactionContext); + return new ContractAgreementServiceImpl(contractNegotiationStore, transactionContext, QueryValidators.contractAgreement()); } @Provider public ContractDefinitionService contractDefinitionService() { var contractDefinitionObservable = new ContractDefinitionObservableImpl(); contractDefinitionObservable.registerListener(new ContractDefinitionEventListener(clock, eventRouter)); - return new ContractDefinitionServiceImpl(contractDefinitionStore, transactionContext, contractDefinitionObservable); + return new ContractDefinitionServiceImpl(contractDefinitionStore, transactionContext, contractDefinitionObservable, QueryValidators.contractDefinition()); } @Provider public ContractNegotiationService contractNegotiationService() { - return new ContractNegotiationServiceImpl(contractNegotiationStore, consumerContractNegotiationManager, transactionContext, commandHandlerRegistry); + return new ContractNegotiationServiceImpl(contractNegotiationStore, consumerContractNegotiationManager, + transactionContext, commandHandlerRegistry, QueryValidators.contractNegotiation()); } @Provider @@ -248,13 +252,15 @@ transactionContext, contractValidationService, consumerOfferResolver, protocolTo public PolicyDefinitionService policyDefinitionService() { var policyDefinitionObservable = new PolicyDefinitionObservableImpl(); policyDefinitionObservable.registerListener(new PolicyDefinitionEventListener(clock, eventRouter)); - return new PolicyDefinitionServiceImpl(transactionContext, policyDefinitionStore, contractDefinitionStore, policyDefinitionObservable, policyEngine); + return new PolicyDefinitionServiceImpl(transactionContext, policyDefinitionStore, contractDefinitionStore, + policyDefinitionObservable, policyEngine, QueryValidators.policyDefinition()); } @Provider public TransferProcessService transferProcessService() { return new TransferProcessServiceImpl(transferProcessStore, transferProcessManager, transactionContext, - dataAddressValidator, commandHandlerRegistry, transferTypeParser, contractNegotiationStore); + dataAddressValidator, commandHandlerRegistry, transferTypeParser, contractNegotiationStore, + QueryValidators.transferProcess()); } @Provider diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidator.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidator.java index b34939552ed..bf87569cebb 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidator.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidator.java @@ -22,10 +22,10 @@ import static java.lang.String.format; -class AssetQueryValidator extends QueryValidator { +public class AssetQueryValidator extends QueryValidator { private static final Pattern VALID_QUERY_PATH_REGEX = Pattern.compile("^[A-Za-z_']+.*$"); - AssetQueryValidator() { + public AssetQueryValidator() { super(Asset.class); } diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImpl.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImpl.java index 0825fe6dcc7..5936fa810da 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImpl.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImpl.java @@ -43,13 +43,13 @@ public class AssetServiceImpl implements AssetService { public AssetServiceImpl(AssetIndex index, ContractNegotiationStore contractNegotiationStore, TransactionContext transactionContext, AssetObservable observable, - DataAddressValidatorRegistry dataAddressValidator) { + DataAddressValidatorRegistry dataAddressValidator, QueryValidator queryValidator) { this.index = index; this.contractNegotiationStore = contractNegotiationStore; this.transactionContext = transactionContext; this.observable = observable; this.dataAddressValidator = dataAddressValidator; - queryValidator = new AssetQueryValidator(); + this.queryValidator = queryValidator; } @Override diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImpl.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImpl.java index 8dad660d8a6..7e2691a0da3 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImpl.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImpl.java @@ -33,10 +33,10 @@ public class ContractAgreementServiceImpl implements ContractAgreementService { private final TransactionContext transactionContext; private final QueryValidator queryValidator; - public ContractAgreementServiceImpl(ContractNegotiationStore store, TransactionContext transactionContext) { + public ContractAgreementServiceImpl(ContractNegotiationStore store, TransactionContext transactionContext, QueryValidator queryValidator) { this.store = store; this.transactionContext = transactionContext; - queryValidator = new QueryValidator(ContractAgreement.class); + this.queryValidator = queryValidator; } @Override diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImpl.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImpl.java index ac1ec0076cf..c1d54186500 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImpl.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImpl.java @@ -33,11 +33,12 @@ public class ContractDefinitionServiceImpl implements ContractDefinitionService private final ContractDefinitionObservable observable; private final QueryValidator queryValidator; - public ContractDefinitionServiceImpl(ContractDefinitionStore store, TransactionContext transactionContext, ContractDefinitionObservable observable) { + public ContractDefinitionServiceImpl(ContractDefinitionStore store, TransactionContext transactionContext, + ContractDefinitionObservable observable, QueryValidator queryValidator) { this.store = store; this.transactionContext = transactionContext; this.observable = observable; - queryValidator = new QueryValidator(ContractDefinition.class); + this.queryValidator = queryValidator; } @Override diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImpl.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImpl.java index a85af31f667..1cf20c8be5e 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImpl.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImpl.java @@ -43,12 +43,13 @@ public class ContractNegotiationServiceImpl implements ContractNegotiationServic private final QueryValidator queryValidator; public ContractNegotiationServiceImpl(ContractNegotiationStore store, ConsumerContractNegotiationManager consumerManager, - TransactionContext transactionContext, CommandHandlerRegistry commandHandlerRegistry) { + TransactionContext transactionContext, CommandHandlerRegistry commandHandlerRegistry, + QueryValidator queryValidator) { this.store = store; this.consumerManager = consumerManager; this.transactionContext = transactionContext; this.commandHandlerRegistry = commandHandlerRegistry; - queryValidator = new QueryValidator(ContractNegotiation.class); + this.queryValidator = queryValidator; } @Override diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImpl.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImpl.java index c1c4b82764f..c06d406895f 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImpl.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImpl.java @@ -53,13 +53,14 @@ public class PolicyDefinitionServiceImpl implements PolicyDefinitionService { public PolicyDefinitionServiceImpl(TransactionContext transactionContext, PolicyDefinitionStore policyStore, - ContractDefinitionStore contractDefinitionStore, PolicyDefinitionObservable observable, PolicyEngine policyEngine) { + ContractDefinitionStore contractDefinitionStore, PolicyDefinitionObservable observable, + PolicyEngine policyEngine, QueryValidator queryValidator) { this.transactionContext = transactionContext; this.policyStore = policyStore; this.contractDefinitionStore = contractDefinitionStore; this.observable = observable; this.policyEngine = policyEngine; - queryValidator = new QueryValidator(PolicyDefinition.class, getSubtypeMap()); + this.queryValidator = queryValidator; } @Override diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidator.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidator.java index 0bd9b6a95d0..828c51bede4 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidator.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidator.java @@ -26,9 +26,9 @@ import java.util.Map; import java.util.Objects; import java.util.regex.Pattern; -import java.util.stream.Collectors; import static java.lang.String.format; +import static java.util.Collections.emptyMap; /** * Validates that a particular query is valid, i.e. contains a left-hand operand, that conforms to the canonical @@ -51,7 +51,7 @@ public QueryValidator(Class canonicalType, Map, List>> type } public QueryValidator(Class canonicalType) { - this(canonicalType, null); + this(canonicalType, emptyMap()); } /** @@ -105,13 +105,12 @@ protected Result isValid(String path) { private Field getFieldIncludingSubtypes(Class type, String token) { var field = ReflectionUtil.getFieldRecursive(type, token); - if (field == null && subtypeMap != null) { + if (field == null) { var subTypes = subtypeMap.get(type); if (subTypes != null) { - var foundTypes = subTypes.stream().map(st -> getFieldIncludingSubtypes(st, token)) + return subTypes.stream().map(st -> getFieldIncludingSubtypes(st, token)) .filter(Objects::nonNull) - .collect(Collectors.toList()); - return foundTypes.stream().findFirst().orElse(null); + .findFirst().orElse(null); } } return field; diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidators.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidators.java new file mode 100644 index 00000000000..3f30d3ebfb8 --- /dev/null +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidators.java @@ -0,0 +1,102 @@ +/* + * Copyright (c) 2024 Cofinity-X + * + * 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: + * Cofinity-X - initial API and implementation + * + */ + +package org.eclipse.edc.connector.controlplane.services.query; + +import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreement; +import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiation; +import org.eclipse.edc.connector.controlplane.contract.spi.types.offer.ContractDefinition; +import org.eclipse.edc.connector.controlplane.policy.spi.PolicyDefinition; +import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedContentResource; +import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedDataAddressResource; +import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedDataDestinationResource; +import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedResource; +import org.eclipse.edc.connector.controlplane.transfer.spi.types.TransferProcess; +import org.eclipse.edc.policy.model.AndConstraint; +import org.eclipse.edc.policy.model.AtomicConstraint; +import org.eclipse.edc.policy.model.Constraint; +import org.eclipse.edc.policy.model.Expression; +import org.eclipse.edc.policy.model.LiteralExpression; +import org.eclipse.edc.policy.model.MultiplicityConstraint; +import org.eclipse.edc.policy.model.OrConstraint; +import org.eclipse.edc.policy.model.XoneConstraint; + +import java.util.List; +import java.util.Map; + +/** + * Factory methods to instantiate {@link QueryValidators} + */ +public final class QueryValidators { + + /** + * Validator for {@link ContractAgreement} + * + * @return the validator. + */ + public static QueryValidator contractAgreement() { + return new QueryValidator(ContractAgreement.class); + } + + /** + * Validator for {@link ContractDefinition} + * + * @return the validator. + */ + public static QueryValidator contractDefinition() { + return new QueryValidator(ContractDefinition.class); + } + + /** + * Validator for {@link ContractNegotiation} + * + * @return the validator. + */ + public static QueryValidator contractNegotiation() { + return new QueryValidator(ContractNegotiation.class); + } + + /** + * Validator for {@link PolicyDefinition} + * + * @return the validator. + */ + public static QueryValidator policyDefinition() { + return new QueryValidator(PolicyDefinition.class, policySubtypeMap()); + } + + /** + * Validator for {@link TransferProcess} + * + * @return the validator. + */ + public static QueryValidator transferProcess() { + return new QueryValidator(TransferProcess.class, transferProcessSubtypeMap()); + } + + private static Map, List>> policySubtypeMap() { + return Map.of( + Constraint.class, List.of(MultiplicityConstraint.class, AtomicConstraint.class), + MultiplicityConstraint.class, List.of(AndConstraint.class, OrConstraint.class, XoneConstraint.class), + Expression.class, List.of(LiteralExpression.class) + ); + } + + private static Map, List>> transferProcessSubtypeMap() { + return Map.of( + ProvisionedResource.class, List.of(ProvisionedDataAddressResource.class), + ProvisionedDataAddressResource.class, List.of(ProvisionedDataDestinationResource.class, ProvisionedContentResource.class) + ); + } +} diff --git a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImpl.java b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImpl.java index e346f9a33aa..ee4bb0cb0e6 100644 --- a/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImpl.java +++ b/core/control-plane/control-plane-aggregate-services/src/main/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImpl.java @@ -23,10 +23,6 @@ import org.eclipse.edc.connector.controlplane.transfer.spi.store.TransferProcessStore; import org.eclipse.edc.connector.controlplane.transfer.spi.types.DeprovisionedResource; import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionResponse; -import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedContentResource; -import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedDataAddressResource; -import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedDataDestinationResource; -import org.eclipse.edc.connector.controlplane.transfer.spi.types.ProvisionedResource; import org.eclipse.edc.connector.controlplane.transfer.spi.types.TransferProcess; import org.eclipse.edc.connector.controlplane.transfer.spi.types.TransferProcessStates; import org.eclipse.edc.connector.controlplane.transfer.spi.types.TransferRequest; @@ -49,7 +45,6 @@ import org.jetbrains.annotations.Nullable; import java.util.List; -import java.util.Map; import java.util.Optional; import static java.lang.String.format; @@ -67,7 +62,7 @@ public class TransferProcessServiceImpl implements TransferProcessService { public TransferProcessServiceImpl(TransferProcessStore transferProcessStore, TransferProcessManager manager, TransactionContext transactionContext, DataAddressValidatorRegistry dataAddressValidator, CommandHandlerRegistry commandHandlerRegistry, TransferTypeParser transferTypeParser, - ContractNegotiationStore contractNegotiationStore) { + ContractNegotiationStore contractNegotiationStore, QueryValidator queryValidator) { this.transferProcessStore = transferProcessStore; this.manager = manager; this.transactionContext = transactionContext; @@ -75,7 +70,7 @@ public TransferProcessServiceImpl(TransferProcessStore transferProcessStore, Tra this.commandHandlerRegistry = commandHandlerRegistry; this.transferTypeParser = transferTypeParser; this.contractNegotiationStore = contractNegotiationStore; - queryValidator = new QueryValidator(TransferProcess.class, getSubtypes()); + this.queryValidator = queryValidator; } @Override @@ -182,11 +177,4 @@ private ServiceResult execute(EntityCommand command) { return transactionContext.execute(() -> commandHandlerRegistry.execute(command).flatMap(ServiceResult::from)); } - private Map, List>> getSubtypes() { - return Map.of( - ProvisionedResource.class, List.of(ProvisionedDataAddressResource.class), - ProvisionedDataAddressResource.class, List.of(ProvisionedDataDestinationResource.class, ProvisionedContentResource.class) - ); - } - } diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidatorTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidatorTest.java index e7fbce15907..a74818a3abd 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidatorTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetQueryValidatorTest.java @@ -17,13 +17,20 @@ import org.eclipse.edc.connector.controlplane.asset.spi.domain.Asset; import org.eclipse.edc.spi.query.Criterion; import org.eclipse.edc.spi.query.QuerySpec; +import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.ValueSource; import java.util.List; +import java.util.stream.Stream; -import static org.assertj.core.api.Assertions.assertThat; +import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.constants.CoreConstants.EDC_NAMESPACE; +import static org.eclipse.edc.spi.query.Criterion.criterion; +import static org.junit.jupiter.params.provider.Arguments.arguments; class AssetQueryValidatorTest { @@ -43,7 +50,9 @@ class AssetQueryValidatorTest { void validate_validProperty(String key) { var query = QuerySpec.Builder.newInstance().filter(List.of(new Criterion(key, "=", "someval"))).build(); - assertThat(validator.validate(query).succeeded()).isTrue(); + var result = validator.validate(query); + + assertThat(result).isSucceeded(); } @ParameterizedTest @@ -59,6 +68,28 @@ void validate_invalidProperty(String key) { .filter(List.of(new Criterion(key, "=", "something"))) .build(); - assertThat(validator.validate(query).failed()).isTrue(); + var result = validator.validate(query); + + assertThat(result).isFailed(); + } + + @ParameterizedTest + @ArgumentsSource(InvalidFilters.class) + void search_invalidFilter(Criterion filter) { + var query = QuerySpec.Builder.newInstance().filter(filter).build(); + + var result = validator.validate(query); + + assertThat(result).isFailed(); + } + + private static class InvalidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion(" asset_prop_id", "in", "(foo, bar)")), // invalid leading whitespace + arguments(criterion(".customProp", "=", "whatever")) // invalid leading dot + ); + } } } diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImplTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImplTest.java index d16809254d6..3388e402c37 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImplTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/asset/AssetServiceImplTest.java @@ -21,11 +21,11 @@ import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.store.ContractNegotiationStore; import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreement; import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiation; +import org.eclipse.edc.connector.controlplane.services.query.QueryValidator; import org.eclipse.edc.connector.controlplane.services.spi.asset.AssetService; import org.eclipse.edc.policy.model.Policy; -import org.eclipse.edc.spi.query.Criterion; import org.eclipse.edc.spi.query.QuerySpec; -import org.eclipse.edc.spi.result.Failure; +import org.eclipse.edc.spi.result.Result; import org.eclipse.edc.spi.result.ServiceFailure; import org.eclipse.edc.spi.result.ServiceResult; import org.eclipse.edc.spi.result.StoreResult; @@ -37,25 +37,18 @@ import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; -import org.junit.jupiter.params.provider.ValueSource; import java.util.UUID; import java.util.function.Predicate; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.list; import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; -import static org.eclipse.edc.spi.query.Criterion.criterion; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.BAD_REQUEST; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.CONFLICT; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.NOT_FOUND; import static org.eclipse.edc.validator.spi.Violation.violation; -import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.AdditionalMatchers.and; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; @@ -76,8 +69,10 @@ class AssetServiceImplTest { private final TransactionContext dummyTransactionContext = new NoopTransactionContext(); private final AssetObservable observable = mock(); private final DataAddressValidatorRegistry dataAddressValidator = mock(); + private final QueryValidator queryValidator = mock(); - private final AssetService service = new AssetServiceImpl(index, contractNegotiationStore, dummyTransactionContext, observable, dataAddressValidator); + private final AssetService service = new AssetServiceImpl(index, contractNegotiationStore, dummyTransactionContext, + observable, dataAddressValidator, queryValidator); @Test void findById_shouldRelyOnAssetIndex() { @@ -93,31 +88,22 @@ void findById_shouldRelyOnAssetIndex() { void search_shouldRelyOnAssetIndex() { var asset = createAsset("assetId"); when(index.queryAssets(any(QuerySpec.class))).thenReturn(Stream.of(asset)); + when(queryValidator.validate(any())).thenReturn(Result.success()); var assets = service.search(QuerySpec.none()); - assertThat(assets.succeeded()).isTrue(); - assertThat(assets.getContent()).hasSize(1).first().matches(hasId("assetId")); + assertThat(assets).isSucceeded().asInstanceOf(list(Asset.class)) + .hasSize(1).first().matches(hasId("assetId")); } - @ParameterizedTest - @ValueSource(strings = { Asset.PROPERTY_ID, Asset.PROPERTY_NAME, Asset.PROPERTY_DESCRIPTION, Asset.PROPERTY_VERSION, Asset.PROPERTY_CONTENT_TYPE }) - void search_validFilter(String filter) { - var query = QuerySpec.Builder.newInstance().filter(criterion(filter, "=", "somevalue")).build(); - - service.search(query); - - verify(index).queryAssets(query); - } - - @ParameterizedTest - @ArgumentsSource(InvalidFilters.class) - void search_invalidFilter(Criterion filter) { - var query = QuerySpec.Builder.newInstance().filter(filter).build(); + @Test + void search_shouldFail_whenQueryIsNotValid() { + when(queryValidator.validate(any())).thenReturn(Result.failure("not valid")); - var result = service.search(query); + var assets = service.search(QuerySpec.none()); - assertThat(result).isFailed().extracting(Failure::getMessages).asList().hasSize(1); + assertThat(assets).isFailed(); + verifyNoInteractions(contractNegotiationStore); } @Test @@ -266,15 +252,6 @@ void updateAsset_shouldFail_whenPropertiesAreDuplicated() { verifyNoInteractions(index); } - private static class InvalidFilters implements ArgumentsProvider { - @Override - public Stream provideArguments(ExtensionContext context) { - return Stream.of(arguments(criterion(" asset_prop_id", "in", "(foo, bar)")), // invalid leading whitespace - arguments(criterion(".customProp", "=", "whatever")) // invalid leading dot - ); - } - } - @NotNull private Predicate hasId(String assetId) { return it -> assetId.equals(it.getId()); diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImplTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImplTest.java index 55dd589a82b..9fbcd14fa73 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImplTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractagreement/ContractAgreementServiceImplTest.java @@ -17,9 +17,11 @@ import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.store.ContractNegotiationStore; import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreement; import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiation; +import org.eclipse.edc.connector.controlplane.services.query.QueryValidator; import org.eclipse.edc.connector.controlplane.services.spi.contractagreement.ContractAgreementService; import org.eclipse.edc.policy.model.Policy; import org.eclipse.edc.spi.query.QuerySpec; +import org.eclipse.edc.spi.result.Result; import org.eclipse.edc.spi.types.domain.callback.CallbackAddress; import org.eclipse.edc.transaction.spi.NoopTransactionContext; import org.eclipse.edc.transaction.spi.TransactionContext; @@ -31,19 +33,23 @@ import static java.util.UUID.randomUUID; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.list; +import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.query.Criterion.criterion; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; class ContractAgreementServiceImplTest { private final ContractNegotiationStore store = mock(); private final TransactionContext transactionContext = new NoopTransactionContext(); - private final ContractAgreementService service = new ContractAgreementServiceImpl(store, transactionContext); + private final QueryValidator queryValidator = mock(); + private final ContractAgreementService service = new ContractAgreementServiceImpl(store, transactionContext, queryValidator); @Test void findById_filtersById() { @@ -68,11 +74,22 @@ void findById_returnsNullIfNotFound() { void search_filtersBySpec() { var agreement = createContractAgreement("agreementId"); when(store.queryAgreements(isA(QuerySpec.class))).thenReturn(Stream.of(agreement)); + when(queryValidator.validate(any())).thenReturn(Result.success()); var result = service.search(QuerySpec.none()); - assertThat(result.succeeded()).isTrue(); - assertThat(result.getContent()).hasSize(1).first().matches(it -> it.getId().equals("agreementId")); + assertThat(result).isSucceeded().asInstanceOf(list(ContractAgreement.class)) + .hasSize(1).first().matches(it -> it.getId().equals("agreementId")); + } + + @Test + void search_shouldFail_whenQueryIsNotValid() { + when(queryValidator.validate(any())).thenReturn(Result.failure("not valid")); + + var result = service.search(QuerySpec.none()); + + assertThat(result).isFailed(); + verifyNoInteractions(store); } @Test diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImplTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImplTest.java index 4d7f8e85884..ee33876172b 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImplTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractdefinition/ContractDefinitionServiceImplTest.java @@ -19,30 +19,26 @@ import org.eclipse.edc.connector.controlplane.contract.spi.definition.observe.ContractDefinitionObservableImpl; import org.eclipse.edc.connector.controlplane.contract.spi.offer.store.ContractDefinitionStore; import org.eclipse.edc.connector.controlplane.contract.spi.types.offer.ContractDefinition; +import org.eclipse.edc.connector.controlplane.services.query.QueryValidator; import org.eclipse.edc.connector.controlplane.services.spi.contractdefinition.ContractDefinitionService; -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.spi.result.StoreResult; import org.eclipse.edc.transaction.spi.NoopTransactionContext; import org.eclipse.edc.transaction.spi.TransactionContext; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; import java.util.UUID; import java.util.function.Predicate; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; -import static org.eclipse.edc.spi.query.Criterion.criterion; +import static org.assertj.core.api.InstanceOfAssertFactories.list; +import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.CONFLICT; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.NOT_FOUND; -import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; @@ -60,8 +56,9 @@ class ContractDefinitionServiceImplTest { private final TransactionContext transactionContext = new NoopTransactionContext(); private final ContractDefinitionObservable observable = new ContractDefinitionObservableImpl(); private final ContractDefinitionListener listener = mock(); + private final QueryValidator queryValidator = mock(); - private final ContractDefinitionService service = new ContractDefinitionServiceImpl(store, transactionContext, observable); + private final ContractDefinitionService service = new ContractDefinitionServiceImpl(store, transactionContext, observable, queryValidator); @BeforeEach void setUp() { @@ -91,35 +88,22 @@ void findById_returnsNullIfNotFound() { void search() { var definition = createContractDefinition(); when(store.findAll(isA(QuerySpec.class))).thenReturn(Stream.of(definition)); + when(queryValidator.validate(any())).thenReturn(Result.success()); var result = service.search(QuerySpec.none()); - assertThat(result.succeeded()).isTrue(); - assertThat(result.getContent()).hasSize(1).first().matches(hasId(definition.getId())); + assertThat(result).isSucceeded().asInstanceOf(list(ContractDefinition.class)) + .hasSize(1).first().matches(hasId(definition.getId())); } - @ParameterizedTest - @ArgumentsSource(InvalidFilters.class) - void search_invalidFilter(Criterion invalidFilter) { - var query = QuerySpec.Builder.newInstance() - .filter(invalidFilter) - .build(); - - var result = service.search(query); - - assertThat(result.failed()).isTrue(); - } - - @ParameterizedTest - @ArgumentsSource(ValidFilters.class) - void search_validFilter(Criterion validFilter) { - var query = QuerySpec.Builder.newInstance() - .filter(validFilter) - .build(); + @Test + void search_shouldFail_whenQueryIsNotValid() { + when(queryValidator.validate(any())).thenReturn(Result.failure("not valid")); - service.search(query); + var result = service.search(QuerySpec.none()); - verify(store).findAll(query); + assertThat(result).isFailed(); + verifyNoInteractions(store); } @Test @@ -210,27 +194,6 @@ void update_shouldReturnNotFound_whenTheStoreFails() { verify(listener, never()).updated(any()); } - private static class InvalidFilters implements ArgumentsProvider { - @Override - public Stream provideArguments(ExtensionContext context) { - return Stream.of( - arguments(criterion("assetsSelector.leftHand", "=", "foo")), // invalid path - arguments(criterion("accessPolicyId'LIKE/**/?/**/LIMIT/**/?/**/OFFSET/**/?;DROP/**/TABLE/**/test/**/--%20", "=", "%20ABC--")) //some SQL injection - ); - } - } - - private static class ValidFilters implements ArgumentsProvider { - @Override - public Stream provideArguments(ExtensionContext context) { - return Stream.of( - arguments(criterion("assetsSelector.operandLeft", "=", "foo")), - arguments(criterion("assetsSelector.operator", "=", "LIKE")), - arguments(criterion("assetsSelector.operandRight", "=", "bar")) - ); - } - } - @NotNull private Predicate hasId(String id) { return d -> d.getId().equals(id); diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImplTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImplTest.java index c10a2f87569..264bc6e4cc4 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImplTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/contractnegotiation/ContractNegotiationServiceImplTest.java @@ -21,27 +21,27 @@ import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiation; import org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractRequest; import org.eclipse.edc.connector.controlplane.contract.spi.types.offer.ContractOffer; +import org.eclipse.edc.connector.controlplane.services.query.QueryValidator; import org.eclipse.edc.connector.controlplane.services.spi.contractnegotiation.ContractNegotiationService; import org.eclipse.edc.policy.model.Policy; import org.eclipse.edc.spi.command.CommandHandlerRegistry; import org.eclipse.edc.spi.command.CommandResult; -import org.eclipse.edc.spi.query.Criterion; import org.eclipse.edc.spi.query.QuerySpec; import org.eclipse.edc.spi.response.StatusResult; +import org.eclipse.edc.spi.result.Result; import org.eclipse.edc.spi.result.ServiceFailure; import org.eclipse.edc.transaction.spi.NoopTransactionContext; import org.eclipse.edc.transaction.spi.TransactionContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; import java.util.UUID; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.list; import static org.eclipse.edc.connector.controlplane.contract.spi.types.negotiation.ContractNegotiationStates.REQUESTED; import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.query.Criterion.criterion; @@ -51,6 +51,7 @@ import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -60,8 +61,9 @@ class ContractNegotiationServiceImplTest { private final ConsumerContractNegotiationManager consumerManager = mock(); private final CommandHandlerRegistry commandHandlerRegistry = mock(); private final TransactionContext transactionContext = new NoopTransactionContext(); + private final QueryValidator queryValidator = mock(); - private final ContractNegotiationService service = new ContractNegotiationServiceImpl(store, consumerManager, transactionContext, commandHandlerRegistry); + private final ContractNegotiationService service = new ContractNegotiationServiceImpl(store, consumerManager, transactionContext, commandHandlerRegistry, queryValidator); @Test void findById_filtersById() { @@ -86,36 +88,22 @@ void findById_returnsNullIfNotFound() { void search_filtersBySpec() { var negotiation = createContractNegotiation("negotiationId"); when(store.queryNegotiations(isA(QuerySpec.class))).thenReturn(Stream.of(negotiation)); + when(queryValidator.validate(any())).thenReturn(Result.success()); var result = service.search(QuerySpec.none()); - assertThat(result.succeeded()).isTrue(); - assertThat(result.getContent()).hasSize(1).first().matches(it -> it.getId().equals("negotiationId")); + assertThat(result).isSucceeded().asInstanceOf(list(ContractNegotiation.class)) + .hasSize(1).first().matches(it -> it.getId().equals("negotiationId")); } - @ParameterizedTest - @ArgumentsSource(InvalidFilters.class) - void search_invalidFilter(Criterion invalidFilter) { - var query = QuerySpec.Builder.newInstance() - .filter(invalidFilter) - .build(); + @Test + void search_shouldFail_whenQueryIsNotValid() { + when(queryValidator.validate(any())).thenReturn(Result.failure("not valid")); - var result = service.search(query); + var result = service.search(QuerySpec.none()); assertThat(result).isFailed(); - } - - @ParameterizedTest - @ArgumentsSource(ValidFilters.class) - void search_validFilter(Criterion validFilter) { - var query = QuerySpec.Builder.newInstance() - .filter(validFilter) - .build(); - - var result = service.search(query); - - assertThat(result).isSucceeded(); - verify(store).queryNegotiations(query); + verifyNoInteractions(store); } @Test diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImplTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImplTest.java index 63f7af4454d..24715af2d05 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImplTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/policydefinition/PolicyDefinitionServiceImplTest.java @@ -19,9 +19,9 @@ import org.eclipse.edc.connector.controlplane.policy.spi.PolicyDefinition; import org.eclipse.edc.connector.controlplane.policy.spi.observe.PolicyDefinitionObservable; import org.eclipse.edc.connector.controlplane.policy.spi.store.PolicyDefinitionStore; +import org.eclipse.edc.connector.controlplane.services.query.QueryValidator; import org.eclipse.edc.policy.engine.spi.PolicyEngine; import org.eclipse.edc.policy.model.Policy; -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.spi.result.StoreResult; @@ -29,26 +29,22 @@ import org.eclipse.edc.transaction.spi.TransactionContext; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; import java.util.function.Predicate; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.list; import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.query.Criterion.criterion; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.CONFLICT; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.NOT_FOUND; -import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; @@ -59,8 +55,8 @@ class PolicyDefinitionServiceImplTest { private final TransactionContext dummyTransactionContext = new NoopTransactionContext(); private final PolicyDefinitionObservable observable = mock(PolicyDefinitionObservable.class); private final PolicyEngine policyEngine = mock(); - private final PolicyDefinitionServiceImpl policyServiceImpl = new PolicyDefinitionServiceImpl(dummyTransactionContext, policyStore, contractDefinitionStore, observable, policyEngine); - + private final QueryValidator queryValidator = mock(); + private final PolicyDefinitionServiceImpl policyServiceImpl = new PolicyDefinitionServiceImpl(dummyTransactionContext, policyStore, contractDefinitionStore, observable, policyEngine, queryValidator); @Test void findById_shouldRelyOnPolicyStore() { @@ -75,34 +71,21 @@ void findById_shouldRelyOnPolicyStore() { void search_shouldRelyOnPolicyStore() { var policy = createPolicy("policyId"); when(policyStore.findAll(any(QuerySpec.class))).thenReturn(Stream.of(policy)); - var policies = policyServiceImpl.search(QuerySpec.none()); - - assertThat(policies.succeeded()).isTrue(); - assertThat(policies.getContent()).containsExactly(policy); - } + when(queryValidator.validate(any())).thenReturn(Result.success()); - @ParameterizedTest - @ArgumentsSource(InvalidFilters.class) - void search_invalidExpression_raiseException(Criterion invalidFilter) { - var query = QuerySpec.Builder.newInstance() - .filter(invalidFilter) - .build(); - - var result = policyServiceImpl.search(query); + var policies = policyServiceImpl.search(QuerySpec.none()); - assertThat(result).isFailed(); + assertThat(policies).isSucceeded().asInstanceOf(list(PolicyDefinition.class)).containsExactly(policy); } - @ParameterizedTest - @ArgumentsSource(ValidFilters.class) - void search_validExpression_privateProperties(Criterion validFilter) { - var query = QuerySpec.Builder.newInstance() - .filter(validFilter) - .build(); + @Test + void search_shouldFail_whenValidationFails() { + when(queryValidator.validate(any())).thenReturn(Result.failure("not valid")); - var result = policyServiceImpl.search(query); + var policies = policyServiceImpl.search(QuerySpec.none()); - assertThat(result).isSucceeded(); + assertThat(policies).isFailed(); + verifyNoInteractions(policyStore); } @Test @@ -274,32 +257,4 @@ private PolicyDefinition createPolicy(String policyId) { return PolicyDefinition.Builder.newInstance().policy(Policy.Builder.newInstance().build()).id(policyId).build(); } - private static class InvalidFilters implements ArgumentsProvider { - @Override - public Stream provideArguments(ExtensionContext context) { - return Stream.of( - arguments(criterion("policy.permissions.action.constraint.noexist", "=", "123455")), // wrong property - arguments(criterion("permissions.action.constraint.leftExpression", "=", "123455")), // missing root - arguments(criterion("policy.permissions.action.leftExpression", "=", "123455")) // skips path element - ); - } - } - - private static class ValidFilters implements ArgumentsProvider { - private static final String PRIVATE_PROPERTIES = "privateProperties"; - private static final String EDC_NAMESPACE = "'https://w3id.org/edc/v0.0.1/ns/'"; - private static final String KEY = "key"; - - private static final String VALUE = "123455"; - - @Override - public Stream provideArguments(ExtensionContext context) { - return Stream.of( - arguments(criterion(PRIVATE_PROPERTIES, "=", VALUE)), // path element with privateProperties - arguments(criterion(PRIVATE_PROPERTIES + "." + KEY, "=", VALUE)), // path element with privateProperties and key - arguments(criterion(PRIVATE_PROPERTIES + ".'" + KEY + "'", "=", VALUE)), // path element with privateProperties and 'key' - arguments(criterion(PRIVATE_PROPERTIES + "." + EDC_NAMESPACE + KEY, "=", VALUE)) // path element with privateProperties and edc_namespace key - ); - } - } } diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidatorsTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidatorsTest.java new file mode 100644 index 00000000000..0ffcca757da --- /dev/null +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/query/QueryValidatorsTest.java @@ -0,0 +1,280 @@ +/* + * Copyright (c) 2024 Cofinity-X + * + * 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: + * Cofinity-X - initial API and implementation + * + */ + +package org.eclipse.edc.connector.controlplane.services.query; + +import org.eclipse.edc.spi.query.Criterion; +import org.eclipse.edc.spi.query.QuerySpec; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + +import java.util.stream.Stream; + +import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; +import static org.eclipse.edc.spi.query.Criterion.criterion; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +class QueryValidatorsTest { + + @Nested + class PolicyDefinition { + + private final QueryValidator validator = QueryValidators.policyDefinition(); + + @Test + void shouldSucceed_whenQueryIsEmpty() { + var result = validator.validate(QuerySpec.none()); + + assertThat(result).isSucceeded(); + } + + @ParameterizedTest + @ArgumentsSource(InvalidFilters.class) + void shouldFail_whenInvalidFilters(Criterion invalidFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(invalidFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isFailed(); + } + + @ParameterizedTest + @ArgumentsSource(ValidFilters.class) + void shouldSucceed_whenValidFilters(Criterion validFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(validFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isSucceeded(); + } + + private static class InvalidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion("policy.permissions.action.constraint.noexist", "=", "123455")), // wrong property + arguments(criterion("permissions.action.constraint.leftExpression", "=", "123455")), // missing root + arguments(criterion("policy.permissions.action.leftExpression", "=", "123455")) // skips path element + ); + } + } + + private static class ValidFilters implements ArgumentsProvider { + private static final String PRIVATE_PROPERTIES = "privateProperties"; + private static final String EDC_NAMESPACE = "'https://w3id.org/edc/v0.0.1/ns/'"; + private static final String KEY = "key"; + + private static final String VALUE = "123455"; + + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion(PRIVATE_PROPERTIES, "=", VALUE)), // path element with privateProperties + arguments(criterion(PRIVATE_PROPERTIES + "." + KEY, "=", VALUE)), // path element with privateProperties and key + arguments(criterion(PRIVATE_PROPERTIES + ".'" + KEY + "'", "=", VALUE)), // path element with privateProperties and 'key' + arguments(criterion(PRIVATE_PROPERTIES + "." + EDC_NAMESPACE + KEY, "=", VALUE)) // path element with privateProperties and edc_namespace key + ); + } + } + } + + @Nested + class TransferProcess { + + private final QueryValidator validator = QueryValidators.transferProcess(); + + @Test + void shouldSucceed_whenQueryIsEmpty() { + var result = validator.validate(QuerySpec.none()); + + assertThat(result).isSucceeded(); + } + + @ParameterizedTest + @ArgumentsSource(InvalidFilters.class) + void shouldFail_whenInvalidFilters(Criterion invalidFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(invalidFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isFailed(); + } + + @ParameterizedTest + @ArgumentsSource(ValidFilters.class) + void shouldSucceed_whenValidFilters(Criterion validFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(validFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isSucceeded(); + } + + private static class InvalidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion("provisionedResourceSet.resources.hastoken", "=", "true")), // wrong case + arguments(criterion("resourceManifest.definitions.notexist", "=", "foobar")), // property not exist + arguments(criterion("contentDataAddress.properties[*].someKey", "=", "someval")) // map types not supported + ); + } + } + + private static class ValidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion("deprovisionedResources.provisionedResourceId", "=", "someval")), + arguments(criterion("type", "=", "CONSUMER")), + arguments(criterion("provisionedResourceSet.resources.hasToken", "=", "true")) + ); + } + } + + } + + @Nested + class ContractDefinition { + + private final QueryValidator validator = QueryValidators.contractDefinition(); + + @Test + void shouldSucceed_whenQueryIsEmpty() { + var result = validator.validate(QuerySpec.none()); + + assertThat(result).isSucceeded(); + } + + @ParameterizedTest + @ArgumentsSource(InvalidFilters.class) + void shouldFail_whenInvalidFilters(Criterion invalidFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(invalidFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isFailed(); + } + + @ParameterizedTest + @ArgumentsSource(ValidFilters.class) + void shouldSucceed_whenValidFilters(Criterion validFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(validFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isSucceeded(); + } + + private static class InvalidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion("assetsSelector.leftHand", "=", "foo")), // invalid path + arguments(criterion("accessPolicyId'LIKE/**/?/**/LIMIT/**/?/**/OFFSET/**/?;DROP/**/TABLE/**/test/**/--%20", "=", "%20ABC--")) //some SQL injection + ); + } + } + + private static class ValidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion("assetsSelector.operandLeft", "=", "foo")), + arguments(criterion("assetsSelector.operator", "=", "LIKE")), + arguments(criterion("assetsSelector.operandRight", "=", "bar")) + ); + } + } + + } + + @Nested + class ContractNegotiation { + + private final QueryValidator validator = QueryValidators.contractNegotiation(); + + @Test + void shouldSucceed_whenQueryIsEmpty() { + var result = validator.validate(QuerySpec.none()); + + assertThat(result).isSucceeded(); + } + + @ParameterizedTest + @ArgumentsSource(InvalidFilters.class) + void shouldFail_whenInvalidFilters(Criterion invalidFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(invalidFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isFailed(); + } + + @ParameterizedTest + @ArgumentsSource(ValidFilters.class) + void shouldSucceed_whenValidFilters(Criterion validFilter) { + var query = QuerySpec.Builder.newInstance() + .filter(validFilter) + .build(); + + var result = validator.validate(query); + + assertThat(result).isSucceeded(); + } + + + private static class InvalidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion("contractAgreement.contractStartDate.begin", "=", "123455")), // invalid path + arguments(criterion("contractOffers.policy.unexistent", "=", "123455")), // invalid path + arguments(criterion("contractOffers.policy.assetid", "=", "123455")), // wrong case + arguments(criterion("contractOffers.policy.=some-id", "=", "123455")) // incomplete path + ); + } + } + + private static class ValidFilters implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + arguments(criterion("contractAgreement.assetId", "=", "test-asset")), + arguments(criterion("contractAgreement.policy.assignee", "=", "123455")) + ); + } + } + } + +} diff --git a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImplTest.java b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImplTest.java index 7936a99dcde..9ed24420fb4 100644 --- a/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImplTest.java +++ b/core/control-plane/control-plane-aggregate-services/src/test/java/org/eclipse/edc/connector/controlplane/services/transferprocess/TransferProcessServiceImplTest.java @@ -17,6 +17,7 @@ import org.eclipse.edc.connector.controlplane.contract.spi.negotiation.store.ContractNegotiationStore; import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreement; +import org.eclipse.edc.connector.controlplane.services.query.QueryValidator; import org.eclipse.edc.connector.controlplane.services.spi.transferprocess.TransferProcessService; import org.eclipse.edc.connector.controlplane.transfer.spi.TransferProcessManager; import org.eclipse.edc.connector.controlplane.transfer.spi.flow.TransferTypeParser; @@ -31,7 +32,6 @@ import org.eclipse.edc.policy.model.Policy; import org.eclipse.edc.spi.command.CommandHandlerRegistry; import org.eclipse.edc.spi.command.CommandResult; -import org.eclipse.edc.spi.query.Criterion; import org.eclipse.edc.spi.query.QuerySpec; import org.eclipse.edc.spi.response.StatusResult; import org.eclipse.edc.spi.result.Result; @@ -46,16 +46,15 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; -import org.junit.jupiter.params.provider.ArgumentsSource; import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.InstanceOfAssertFactories.list; import static org.eclipse.edc.junit.assertions.AbstractResultAssert.assertThat; import static org.eclipse.edc.spi.query.Criterion.criterion; import static org.eclipse.edc.spi.result.ServiceFailure.Reason.BAD_REQUEST; @@ -83,9 +82,10 @@ class TransferProcessServiceImplTest { private final CommandHandlerRegistry commandHandlerRegistry = mock(); private final TransferTypeParser transferTypeParser = mock(); private final ContractNegotiationStore contractNegotiationStore = mock(); + private final QueryValidator queryValidator = mock(); private final TransferProcessService service = new TransferProcessServiceImpl(store, manager, transactionContext, - dataAddressValidator, commandHandlerRegistry, transferTypeParser, contractNegotiationStore); + dataAddressValidator, commandHandlerRegistry, transferTypeParser, contractNegotiationStore, queryValidator); @Test void findById_whenFound() { @@ -102,33 +102,23 @@ void findById_whenNotFound() { @Test void search() { + when(queryValidator.validate(any())).thenReturn(Result.success()); when(store.findAll(query)).thenReturn(Stream.of(process1, process2)); var result = service.search(query); - assertThat(result.getContent()).containsExactly(process1, process2); + assertThat(result).isSucceeded().asInstanceOf(list(TransferProcess.class)).containsExactly(process1, process2); verify(transactionContext).execute(any(TransactionContext.ResultTransactionBlock.class)); } - @ParameterizedTest - @ArgumentsSource(InvalidFilters.class) - void search_invalidFilter_raiseException(Criterion invalidFilter) { - var spec = QuerySpec.Builder.newInstance().filter(invalidFilter).build(); - - var result = service.search(spec); - - assertThat(result.failed()).isTrue(); - } - - @ParameterizedTest - @ArgumentsSource(ValidFilters.class) - void search_validFilter(Criterion validFilter) { - var spec = QuerySpec.Builder.newInstance().filter(validFilter).build(); + @Test + void search_shouldFail_whenValidationFails() { + when(queryValidator.validate(any())).thenReturn(Result.failure("not valid")); - service.search(spec); + var policies = service.search(QuerySpec.none()); - verify(store).findAll(spec); - verify(transactionContext).execute(any(TransactionContext.ResultTransactionBlock.class)); + assertThat(policies).isFailed(); + verifyNoInteractions(store); } @Test