Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: query policydefinition, contractdefinition via privateProperties #3691

Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
ebce944
feat: search by privateProperties in policy and contractdefinition
suh-rao Dec 5, 2023
a40e05a
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Dec 5, 2023
5e4da16
contractdefinition search by privateproperties
suh-rao Dec 5, 2023
25a4ab9
policydefinition search by privateproperties
suh-rao Dec 5, 2023
152d299
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Dec 6, 2023
657a14c
policydefinition api test
suh-rao Dec 6, 2023
40a5981
Merge remote-tracking branch 'origin/feat/search_policy_contractdefin…
suh-rao Dec 6, 2023
c79e5e1
policydefinition api test
suh-rao Dec 6, 2023
e8dcebf
policydefinition api test
suh-rao Dec 6, 2023
8afd26f
log response
suh-rao Dec 6, 2023
394101b
log response
suh-rao Dec 8, 2023
014dd83
log response
suh-rao Dec 8, 2023
8893d26
log response
suh-rao Dec 8, 2023
3b6a75a
log response
suh-rao Dec 8, 2023
6a3f887
check response body size
suh-rao Dec 8, 2023
9602a55
check body string
suh-rao Dec 11, 2023
1de2e04
policy query validator
suh-rao Dec 11, 2023
33bdd0f
unit tests
suh-rao Dec 11, 2023
57fc689
unit tests
suh-rao Dec 11, 2023
d0a69ab
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Dec 11, 2023
af4b02c
tests
suh-rao Dec 11, 2023
f8c2134
Merge remote-tracking branch 'origin/feat/search_policy_contractdefin…
suh-rao Dec 11, 2023
8699f06
fix: tests
suh-rao Dec 11, 2023
c90a9be
fix: tests
suh-rao Dec 12, 2023
daa052d
fix: review comments
suh-rao Dec 14, 2023
85ee2a0
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Dec 14, 2023
2cc218e
fix: review comments
suh-rao Dec 14, 2023
b95a651
Merge remote-tracking branch 'origin/feat/search_policy_contractdefin…
suh-rao Dec 14, 2023
f20d83f
fix: review comments
suh-rao Dec 14, 2023
22c89e2
fix: review comments
suh-rao Dec 14, 2023
128d101
fix: review comments
suh-rao Dec 14, 2023
0df6886
fix: review comments
suh-rao Dec 21, 2023
4b5a897
fix: review comments
suh-rao Dec 21, 2023
575bbc7
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Dec 21, 2023
0fa5ff2
fix: build errors
suh-rao Dec 21, 2023
3adf704
Merge remote-tracking branch 'origin/feat/search_policy_contractdefin…
suh-rao Dec 21, 2023
ee38ffe
fix: revert jayway lib usage
suh-rao Dec 22, 2023
b6c5cbd
fix: revert jayway lib usage
suh-rao Dec 22, 2023
c83c4fe
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Dec 29, 2023
90f9dfd
fix: review comments
suh-rao Jan 2, 2024
6600209
fix: review comments
suh-rao Jan 2, 2024
8e2a5e3
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Jan 3, 2024
4eb4dc6
Merge branch 'eclipse-edc:main' into feat/search_policy_contractdefin…
suh-rao Jan 9, 2024
b2d1a45
fix: review comments
suh-rao Jan 9, 2024
030fe6a
fix: review comments
suh-rao Jan 9, 2024
b6d50bb
fix: review comments
suh-rao Jan 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ dependencies {
implementation(project(":spi:common:transaction-spi"))
implementation(project(":spi:control-plane:asset-spi"))
implementation(project(":spi:control-plane:transfer-data-plane-spi"))

suh-rao marked this conversation as resolved.
Show resolved Hide resolved
implementation(libs.opentelemetry.instrumentation.annotations)

testImplementation(project(":core:control-plane:catalog-core"))
testImplementation(project(":core:control-plane:contract-core"))
testImplementation(project(":core:control-plane:control-plane-core"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import static java.lang.String.format;
Expand All @@ -40,9 +42,9 @@ public class QueryValidator {
/**
* Constructs a new QueryValidator instance.
*
* @param canonicalType The Java class of the object to validate against.
* @param canonicalType The Java class of the object to validate against.
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
* @param typeHierarchyMap Contains mapping from superclass to list of subclasses. Every superclass must be
* represented as separate entry in the map, even if it is also a subclass of another.
* represented as separate entry in the map, even if it is also a subclass of another.
*/
public QueryValidator(Class<?> canonicalType, Map<Class<?>, List<Class<?>>> typeHierarchyMap) {
this.canonicalType = canonicalType;
Expand Down Expand Up @@ -83,7 +85,10 @@ protected Result<Void> isValid(String path) {

// cannot query on extensible (=Map) types
if (type == Map.class) {
return Result.failure("Querying Map types is not yet supported");
Pattern pattern = Pattern.compile("^[0-9A-Za-z.':/]*$");
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
Matcher matcher = pattern.matcher(path);
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
return matcher.find() ? Result.success() :
Result.failure("Querying Map types is not yet supported");
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
}
var field = getFieldIncludingSubtypes(type, token);
if (field != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,18 @@ void search_invalidExpression_raiseException(Criterion invalidFilter) {
assertThat(result).isFailed();
}

@ParameterizedTest
@ArgumentsSource(ValidFilters.class)
void search_validExpression_privateProperties(Criterion validFilter) {
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
var query = QuerySpec.Builder.newInstance()
.filter(validFilter)
.build();

var result = policyServiceImpl.search(query);

assertThat(result).isSucceeded();
}

@Test
void createPolicy_shouldCreatePolicyIfItDoesNotAlreadyExist() {
var policy = createPolicy("policyId");
Expand Down Expand Up @@ -241,6 +253,24 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
}
}

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<? extends Arguments> 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
);
}
}

@NotNull
private Predicate<PolicyDefinition> hasId(String policyId) {
return it -> policyId.equals(it.getUid());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,22 @@ void validate_interface_withoutSubtypeMap() {
}

@Test
void validate_isMapType() {
void validate_isMapTypeTrue() {
queryValidator = new QueryValidator(TestObject.class);
var query = with(criterion("someMap.foo", "=", "bar"));

var result = queryValidator.validate(query);

assertThat(result.succeeded()).isTrue();
}

@Test
void validate_isMapTypeFalse() {
queryValidator = new QueryValidator(TestObject.class);
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
var query = with(criterion("someMap.foo[*].test", "=", "bar"));

var result = queryValidator.validate(query);

assertThat(result.succeeded()).isFalse();
assertThat(result.getFailureDetail()).isEqualTo("Querying Map types is not yet supported");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public Stream<? extends Arguments> 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
arguments(criterion("contentDataAddress.properties[*].someKey", "=", "someval")) // map types not supported
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ public ContractDefinitionMapping(ContractDefinitionStatements statements) {
add("contractPolicyId", statements.getContractPolicyIdColumn());
add("contractPolicy", statements.getContractPolicyIdColumn());
add("assetsSelector", new JsonFieldMapping(statements.getAssetsSelectorAlias()));
add("privateProperties", new JsonFieldMapping(statements.getPrivatePropertiesColumn()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.eclipse.edc.sql.dialect.PostgresDialect;
import org.eclipse.edc.sql.translation.SqlQueryStatement;

import static java.lang.String.format;
import static org.eclipse.edc.sql.dialect.PostgresDialect.getSelectFromJsonArrayTemplate;

/**
Expand All @@ -38,6 +39,10 @@ public SqlQueryStatement createQuery(QuerySpec querySpec) {
var select = getSelectFromJsonArrayTemplate(getSelectStatement(), getAssetsSelectorColumn(), getAssetsSelectorAlias());
return new SqlQueryStatement(select, querySpec, new ContractDefinitionMapping(this));
}
if (querySpec.containsAnyLeftOperand("privateProperties.")) {
var select = format("SELECT * FROM %s", getContractDefinitionTable());
return new SqlQueryStatement(select, querySpec, new ContractDefinitionMapping(this));
}
return super.createQuery(querySpec);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import org.eclipse.edc.connector.policy.spi.PolicyDefinition;
import org.eclipse.edc.connector.store.sql.policydefinition.store.schema.SqlPolicyStoreStatements;
import org.eclipse.edc.sql.translation.JsonFieldMapping;
import org.eclipse.edc.sql.translation.TranslationMapping;

/**
Expand All @@ -28,5 +29,6 @@ public PolicyDefinitionMapping(SqlPolicyStoreStatements statements) {
add("id", statements.getPolicyIdColumn());
add("createdAt", statements.getCreatedAtColumn());
add("policy", new PolicyMapping(statements));
add("privateProperties", new JsonFieldMapping(statements.getPrivatePropertiesColumn()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.eclipse.edc.sql.dialect.PostgresDialect;
import org.eclipse.edc.sql.translation.SqlQueryStatement;

import static java.lang.String.format;
import static org.eclipse.edc.sql.dialect.PostgresDialect.getSelectFromJsonArrayTemplate;

/**
Expand All @@ -30,6 +31,8 @@ public class PostgresDialectStatements extends BaseSqlDialectStatements {
public static final String PERMISSIONS_ALIAS = "perm";
public static final String OBLIGATIONS_ALIAS = "oblig";
public static final String EXT_PROPERTIES_ALIAS = "extprop";
private static final String PRIVATE_PROPERTIES = "privateProperties.";
private static final String SELECT_QUERY = "SELECT * FROM %s";

@Override
public String getFormatAsJsonOperator() {
Expand All @@ -50,6 +53,9 @@ public SqlQueryStatement createQuery(QuerySpec querySpec) {
} else if (querySpec.containsAnyLeftOperand("policy.extensibleProperties")) {
var select = getSelectFromJsonArrayTemplate(getSelectTemplate(), getExtensiblePropertiesColumn(), EXT_PROPERTIES_ALIAS);
return new SqlQueryStatement(select, querySpec, new PolicyDefinitionMapping(this));
} else if (querySpec.containsAnyLeftOperand(PRIVATE_PROPERTIES)) {
var select = format(SELECT_QUERY, getPolicyTable());
return new SqlQueryStatement(select, querySpec, new PolicyDefinitionMapping(this));
} else {
return super.createQuery(querySpec);
}
Expand Down
2 changes: 0 additions & 2 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ kafkaClients = "3.6.1"
testcontainers = "1.19.3"
tink = "1.12.0"


suh-rao marked this conversation as resolved.
Show resolved Hide resolved
[libraries]
iron-vc = { module = "com.apicatalog:iron-verifiable-credentials", version.ref = "apicatalog" }
iron-ed25519 = { module = "com.apicatalog:iron-ed25519-cryptosuite-2020", version.ref = "apicatalog" }
Expand Down Expand Up @@ -91,7 +90,6 @@ testcontainers-junit = { module = "org.testcontainers:junit-jupiter", version.re
testcontainers-postgres = { module = "org.testcontainers:postgresql", version.ref = "testcontainers" }
tink = { module = "com.google.crypto.tink:tink", version.ref = "tink" }


# other technology extensions
apache-commons-pool = { module = "org.apache.commons:commons-pool2", version.ref = "apacheCommonsPool2" }
atomikos-jta = { module = "com.atomikos:transactions-jta", version.ref = "atomikos" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,67 @@ void shouldReturnAll_with_private_properties_whenNoFiltersApplied() {
var definitionsRetrieved = getContractDefinitionStore().findAll(QuerySpec.max());
assertThat(definitionsRetrieved).isNotNull().hasSize(2);
}

@Test
void shouldReturn_with_private_propertiesFilter() {
var definition1 = createContractDefinition("definition1", "policyId", "contractId", Map.of("key1", "value1"));
getContractDefinitionStore().save(definition1);
var definition2 = createContractDefinition("definition2", "policyId", "contractId", Map.of("key2", "value2"));
getContractDefinitionStore().save(definition2);


var spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.key1", "=", "value1"))
.build();

var definitionsRetrieved = getContractDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);

spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.key2", "=", "value2"))
.build();

definitionsRetrieved = getContractDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);
suh-rao marked this conversation as resolved.
Show resolved Hide resolved

spec = QuerySpec.Builder.newInstance()
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
.filter(new Criterion("privateProperties.key1", "=", "value2"))
.build();

definitionsRetrieved = getContractDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(0);
}

@Test
void shouldReturn_with_complex_private_propertiesFilter() {

var definition1 = createContractDefinition("definition1", "policyId", "contractId", Map.of("myProp", Map.of("description", "test desc 1", "number", 42)));
getContractDefinitionStore().save(definition1);
var definition2 = createContractDefinition("definition2", "policyId", "contractId", Map.of("myProp", Map.of("description", "test desc 2", "number", 42)));
getContractDefinitionStore().save(definition2);


var spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.'myProp'.'description'", "=", "test desc 1"))
.build();

var definitionsRetrieved = getContractDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);
suh-rao marked this conversation as resolved.
Show resolved Hide resolved

spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.'myProp'.'description'", "=", "test desc 2"))
.build();

definitionsRetrieved = getContractDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);
suh-rao marked this conversation as resolved.
Show resolved Hide resolved

spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.'myProp'.'description'", "=", "test desc 3"))
.build();

definitionsRetrieved = getContractDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(0);
}
}

@Nested
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,75 @@ void findAll_with_privateProperties() {
assertThat(list).hasSize(2).usingRecursiveFieldByFieldElementComparator().isSubsetOf(policy1, policy2);
}


@Test
void shouldReturn_with_private_propertiesFilter() {
suh-rao marked this conversation as resolved.
Show resolved Hide resolved
var policy1 = createPolicy(getRandomId(), null, Map.of("key1", "value1", "key2", "value2"));
var policy2 = createPolicy(getRandomId(), null, Map.of("key3", "value3", "key4", "value4"));


getPolicyDefinitionStore().create(policy1);
getPolicyDefinitionStore().create(policy2);
var spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.key1", "=", "value1"))
.build();

var definitionsRetrieved = getPolicyDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);

spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.key2", "=", "value2"))
.build();

definitionsRetrieved = getPolicyDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);

spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.key3", "=", "value3"))
.build();

definitionsRetrieved = getPolicyDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);


spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.key3", "=", "value4"))
.build();

definitionsRetrieved = getPolicyDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(0);
}

@Test
void shouldReturn_with_complex_private_propertiesFilter() {
var policy1 = createPolicy(getRandomId(), null, Map.of("myProp", Map.of("description", "test desc 1", "number", 42)));
var policy2 = createPolicy(getRandomId(), null, Map.of("myProp", Map.of("description", "test desc 2", "number", 42)));

getPolicyDefinitionStore().create(policy1);
getPolicyDefinitionStore().create(policy2);

var spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.'myProp'.'description'", "=", "test desc 1"))
.build();

var definitionsRetrieved = getPolicyDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);

spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.'myProp'.'description'", "=", "test desc 2"))
.build();

definitionsRetrieved = getPolicyDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(1);

spec = QuerySpec.Builder.newInstance()
.filter(new Criterion("privateProperties.'myProp'.'description'", "=", "test desc 3"))
.build();

definitionsRetrieved = getPolicyDefinitionStore().findAll(spec);
assertThat(definitionsRetrieved).isNotNull().hasSize(0);
}

@Test
void whenEqualFilter() {
var policy1 = createPolicy(getRandomId());
Expand Down