Skip to content

Commit

Permalink
refactor(test): improve QueryValidator testing strategy (#4574)
Browse files Browse the repository at this point in the history
  • Loading branch information
ndr-brt authored Oct 23, 2024
1 parent 6c56933 commit 95c5ca8
Show file tree
Hide file tree
Showing 18 changed files with 540 additions and 241 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -51,7 +51,7 @@ public QueryValidator(Class<?> canonicalType, Map<Class<?>, List<Class<?>>> type
}

public QueryValidator(Class<?> canonicalType) {
this(canonicalType, null);
this(canonicalType, emptyMap());
}

/**
Expand Down Expand Up @@ -105,13 +105,12 @@ protected Result<Void> 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Class<?>, List<Class<?>>> 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<Class<?>, List<Class<?>>> transferProcessSubtypeMap() {
return Map.of(
ProvisionedResource.class, List.of(ProvisionedDataAddressResource.class),
ProvisionedDataAddressResource.class, List.of(ProvisionedDataDestinationResource.class, ProvisionedContentResource.class)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -67,15 +62,15 @@ 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;
this.dataAddressValidator = dataAddressValidator;
this.commandHandlerRegistry = commandHandlerRegistry;
this.transferTypeParser = transferTypeParser;
this.contractNegotiationStore = contractNegotiationStore;
queryValidator = new QueryValidator(TransferProcess.class, getSubtypes());
this.queryValidator = queryValidator;
}

@Override
Expand Down Expand Up @@ -182,11 +177,4 @@ private ServiceResult<Void> execute(EntityCommand command) {
return transactionContext.execute(() -> commandHandlerRegistry.execute(command).flatMap(ServiceResult::from));
}

private Map<Class<?>, List<Class<?>>> getSubtypes() {
return Map.of(
ProvisionedResource.class, List.of(ProvisionedDataAddressResource.class),
ProvisionedDataAddressResource.class, List.of(ProvisionedDataDestinationResource.class, ProvisionedContentResource.class)
);
}

}
Loading

0 comments on commit 95c5ca8

Please sign in to comment.