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.

Sign up for GitHub

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: add private properties to contract definition entity #3534

Merged
merged 26 commits into from
Oct 25, 2023
Merged

feat: add private properties to contract definition entity #3534

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
182aabb
feat: add private properties to contract definition
mohannad-ezzo Sep 22, 2023
4ebadf1
feat: adjust JsonObjectFromContractDefinition transformer
mohannad-ezzo Sep 25, 2023
f261809
feat: adjust JsonObjectToContractDefinition transformer
mohannad-ezzo Sep 25, 2023
b1edd63
adjust sql schemam and ER diagram
mohannad-ezzo Sep 28, 2023
9e430ff
feat create contract definition with private properties
mohannad-ezzo Sep 28, 2023
96ed8b6
Merge branch 'eclipse-edc:main' into feat-contract-def-private-prop
mohannad-ezzo Sep 28, 2023
951cbb7
feat: add possibility to update contract definition with private prop…
mohannad-ezzo Sep 28, 2023
f02e148
Merge branch 'feat-contract-def-private-prop' of https://github.com/m…
mohannad-ezzo Sep 28, 2023
bb47b3d
feat: add possibility to find contract by id with private properties
mohannad-ezzo Sep 28, 2023
e4d604b
feat: retrieve properties when triggering findAll
mohannad-ezzo Sep 29, 2023
9fb44d0
feat: remove not required code
mohannad-ezzo Sep 29, 2023
e66c720
Merge branch 'eclipse-edc:main' into feat-contract-def-private-prop
mohannad-ezzo Oct 9, 2023
c486717
Merge branch 'eclipse-edc:main' into feat-contract-def-private-prop
mohannad-ezzo Oct 13, 2023
3dcaec7
fix: address reviewers comments
mohannad-ezzo Oct 13, 2023
a260e1a
Merge branch 'eclipse-edc:main' into feat-contract-def-private-prop
mohannad-ezzo Oct 13, 2023
09a9e83
fix: add more test cases
mohannad-ezzo Oct 15, 2023
4682385
fix: remove isPrivate from contract definition property table
mohannad-ezzo Oct 15, 2023
d30401f
fix: remove not required import statements
mohannad-ezzo Oct 15, 2023
a997c28
fix: handle checkstyle issues
mohannad-ezzo Oct 16, 2023
b253e75
fix: store private properties as JSON instead of having a separate table
mohannad-ezzo Oct 23, 2023
db7fc6d
Merge branch 'eclipse-edc:main' into feat-contract-def-private-prop
mohannad-ezzo Oct 23, 2023
6629cc5
fix: checkstyle and test cases
mohannad-ezzo Oct 23, 2023
be2076d
fix: one more checktyle in import statements
mohannad-ezzo Oct 24, 2023
ba282ff
fix: additional checkstyle issue - avoid import star
mohannad-ezzo Oct 24, 2023
1ee6dcd
fix: return the old behavior for for findById and FindAll
mohannad-ezzo Oct 24, 2023
c696c85
fix: checkstyle - indentation issue
mohannad-ezzo Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion ...lipse/edc/connector/api/management/contractdefinition/ContractDefinitionApiExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* Contributors:
* Microsoft Corporation - initial API and implementation
* Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - improvements
* SAP SE - add private properties to contract definition
*
*/

Expand All @@ -22,16 +23,20 @@
import org.eclipse.edc.connector.api.management.contractdefinition.transform.JsonObjectToContractDefinitionTransformer;
import org.eclipse.edc.connector.api.management.contractdefinition.validation.ContractDefinitionValidator;
import org.eclipse.edc.connector.spi.contractdefinition.ContractDefinitionService;
import org.eclipse.edc.policy.model.AtomicConstraint;
import org.eclipse.edc.policy.model.LiteralExpression;
import org.eclipse.edc.runtime.metamodel.annotation.Extension;
import org.eclipse.edc.runtime.metamodel.annotation.Inject;
import org.eclipse.edc.spi.system.ServiceExtension;
import org.eclipse.edc.spi.system.ServiceExtensionContext;
import org.eclipse.edc.spi.types.TypeManager;
import org.eclipse.edc.validator.spi.JsonObjectValidatorRegistry;
import org.eclipse.edc.web.spi.WebService;

import java.util.Map;

import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_TYPE;
import static org.eclipse.edc.spi.CoreConstants.JSON_LD;

@Extension(value = ContractDefinitionApiExtension.NAME)
public class ContractDefinitionApiExtension implements ServiceExtension {
Expand All @@ -53,6 +58,9 @@ public class ContractDefinitionApiExtension implements ServiceExtension {
@Inject
JsonObjectValidatorRegistry validatorRegistry;

@Inject
private TypeManager typeManager;

@Override
public String name() {
return NAME;
Expand All @@ -61,7 +69,9 @@ public String name() {
@Override
public void initialize(ServiceExtensionContext context) {
var jsonFactory = Json.createBuilderFactory(Map.of());
transformerRegistry.register(new JsonObjectFromContractDefinitionTransformer(jsonFactory));
var mapper = typeManager.getMapper(JSON_LD);
mapper.registerSubtypes(AtomicConstraint.class, LiteralExpression.class);
mohannad-ezzo marked this conversation as resolved.
Show resolved Hide resolved
transformerRegistry.register(new JsonObjectFromContractDefinitionTransformer(jsonFactory, mapper));
transformerRegistry.register(new JsonObjectToContractDefinitionTransformer());

validatorRegistry.register(CONTRACT_DEFINITION_TYPE, ContractDefinitionValidator.instance());
Expand Down
19 changes: 15 additions & 4 deletions .../management/contractdefinition/transform/JsonObjectFromContractDefinitionTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
*
* Contributors:
* Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
* SAP SE - add private properties to contract definition
*
*/

package org.eclipse.edc.connector.api.management.contractdefinition.transform;

import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.json.JsonBuilderFactory;
import jakarta.json.JsonObject;
import org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition;
Expand All @@ -31,11 +33,13 @@
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;

public class JsonObjectFromContractDefinitionTransformer extends AbstractJsonLdTransformer<ContractDefinition, JsonObject> {
private final ObjectMapper mapper;
private final JsonBuilderFactory jsonFactory;

public JsonObjectFromContractDefinitionTransformer(JsonBuilderFactory jsonFactory) {
public JsonObjectFromContractDefinitionTransformer(JsonBuilderFactory jsonFactory, ObjectMapper jsonLdMapper) {
super(ContractDefinition.class, JsonObject.class);
this.jsonFactory = jsonFactory;
this.mapper = jsonLdMapper;
}

@Override
Expand All @@ -44,12 +48,19 @@ public JsonObjectFromContractDefinitionTransformer(JsonBuilderFactory jsonFactor
.map(criterion -> context.transform(criterion, JsonObject.class))
.collect(toJsonArray());

return jsonFactory.createObjectBuilder()
var builder = jsonFactory.createObjectBuilder()
.add(ID, contractDefinition.getId())
.add(TYPE, CONTRACT_DEFINITION_TYPE)
.add(CONTRACT_DEFINITION_ACCESSPOLICY_ID, contractDefinition.getAccessPolicyId())
.add(CONTRACT_DEFINITION_CONTRACTPOLICY_ID, contractDefinition.getContractPolicyId())
.add(CONTRACT_DEFINITION_ASSETS_SELECTOR, criteria)
.build();
.add(CONTRACT_DEFINITION_ASSETS_SELECTOR, criteria);

if (contractDefinition.getPrivateProperties() != null && !contractDefinition.getPrivateProperties().isEmpty()) {
mohannad-ezzo marked this conversation as resolved.
Show resolved Hide resolved
var privatePropBuilder = jsonFactory.createObjectBuilder();
transformProperties(contractDefinition.getPrivateProperties(), privatePropBuilder, mapper, context);
builder.add(ContractDefinition.CONTRACT_DEFINITION_PRIVATE_PROPERTIES, privatePropBuilder);
}

return builder.build();
}
}
33 changes: 19 additions & 14 deletions ...pi/management/contractdefinition/transform/JsonObjectToContractDefinitionTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,23 @@
*
* Contributors:
* Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
* SAP SE - add private properties to contract definition
*
*/

package org.eclipse.edc.connector.api.management.contractdefinition.transform;

import jakarta.json.JsonArray;
import jakarta.json.JsonObject;
import jakarta.json.JsonValue;
import org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition;
import org.eclipse.edc.jsonld.spi.transformer.AbstractJsonLdTransformer;
import org.eclipse.edc.spi.query.Criterion;
import org.eclipse.edc.transform.spi.TransformerContext;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_ACCESSPOLICY_ID;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_ASSETS_SELECTOR;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_CONTRACTPOLICY_ID;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.*;


public class JsonObjectToContractDefinitionTransformer extends AbstractJsonLdTransformer<JsonObject, ContractDefinition> {
Expand All @@ -36,19 +37,23 @@ public JsonObjectToContractDefinitionTransformer() {
@Override
public @Nullable ContractDefinition transform(@NotNull JsonObject object, @NotNull TransformerContext context) {
var builder = ContractDefinition.Builder.newInstance();

builder.id(nodeId(object));

visitProperties(object, (key, value) -> {
switch (key) {
case CONTRACT_DEFINITION_ACCESSPOLICY_ID -> builder.accessPolicyId(transformString(value, context));
case CONTRACT_DEFINITION_CONTRACTPOLICY_ID -> builder.contractPolicyId(transformString(value, context));
case CONTRACT_DEFINITION_ASSETS_SELECTOR -> builder.assetsSelector(transformArray(value, Criterion.class, context));
default -> { }
}
});

visitProperties(object, (s, jsonValue) -> transformProperties(s, jsonValue, builder, context));
return builderResult(builder::build, context);
}

private void transformProperties(String key, JsonValue jsonValue, ContractDefinition.Builder builder, TransformerContext context) {
if (CONTRACT_DEFINITION_ACCESSPOLICY_ID.equals(key) ) {
mohannad-ezzo marked this conversation as resolved.
Show resolved Hide resolved
builder.accessPolicyId(transformString(jsonValue, context));
} else if (CONTRACT_DEFINITION_CONTRACTPOLICY_ID.equals(key) ) {
builder.contractPolicyId(transformString(jsonValue, context));
} else if (CONTRACT_DEFINITION_ASSETS_SELECTOR.equals(key) ) {
builder.assetsSelector(transformArray(jsonValue, Criterion.class, context));
} else if (CONTRACT_DEFINITION_PRIVATE_PROPERTIES.equals(key) && jsonValue instanceof JsonArray) {
var props = jsonValue.asJsonArray().getJsonObject(0);
visitProperties(props, (k, val) -> transformProperties(k, val, builder, context));
} else {
builder.privateProperty(key, transformGenericProperty(jsonValue, context));
}
}
}
29 changes: 24 additions & 5 deletions ...agement/contractdefinition/transform/JsonObjectFromContractDefinitionTransformerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*
* Contributors:
* Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
* SAP SE - add private properties to contract definition
*
*/

Expand All @@ -26,12 +27,10 @@
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_ACCESSPOLICY_ID;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_ASSETS_SELECTOR;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_CONTRACTPOLICY_ID;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_TYPE;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.*;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.ID;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
import static org.eclipse.edc.jsonld.util.JacksonJsonLd.createObjectMapper;
import static org.eclipse.edc.spi.query.Criterion.criterion;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
Expand All @@ -46,7 +45,7 @@ class JsonObjectFromContractDefinitionTransformerTest {
private final JsonBuilderFactory jsonFactory = Json.createBuilderFactory(Map.of());
private final TransformerContext context = mock(TransformerContext.class);

private final JsonObjectFromContractDefinitionTransformer transformer = new JsonObjectFromContractDefinitionTransformer(jsonFactory);
private final JsonObjectFromContractDefinitionTransformer transformer = new JsonObjectFromContractDefinitionTransformer(jsonFactory, createObjectMapper());

@Test
void transform() {
Expand All @@ -72,4 +71,24 @@ void transform() {
verify(context, never()).reportProblem(anyString());
}


@Test
void transform_withPrivateProperties_simpleTypes() {
var criterionJson = jsonFactory.createObjectBuilder().build();
when(context.transform(isA(Criterion.class), eq(JsonObject.class))).thenReturn(criterionJson);
var criterion = criterion("left", "=", "right");
var contractDefinition =ContractDefinition.Builder.newInstance()
mohannad-ezzo marked this conversation as resolved.
Show resolved Hide resolved
.id("id")
.accessPolicyId("accessPolicyId")
.contractPolicyId("contractPolicyId")
.assetsSelector(List.of(criterion))
.privateProperty("some-key", "some-value")
.build();

var jsonObject = transformer.transform(contractDefinition, context);

assertThat(jsonObject).isNotNull();
assertThat(jsonObject.getJsonObject(CONTRACT_DEFINITION_PRIVATE_PROPERTIES).getJsonString("some-key").getString()).isEqualTo("some-value");
}

}
47 changes: 42 additions & 5 deletions ...anagement/contractdefinition/transform/JsonObjectToContractDefinitionTransformerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,28 @@
*
* Contributors:
* Bayerische Motoren Werke Aktiengesellschaft (BMW AG) - initial API and implementation
* SAP SE - add private properties to contract definition
*
*/

package org.eclipse.edc.connector.api.management.contractdefinition.transform;

import jakarta.json.JsonObject;
import jakarta.json.JsonObjectBuilder;
import org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition;
import org.eclipse.edc.jsonld.TitaniumJsonLd;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.spi.query.Criterion;
import org.eclipse.edc.transform.spi.TransformerContext;
import org.junit.jupiter.api.Test;

import static jakarta.json.Json.createArrayBuilder;
import static jakarta.json.Json.createObjectBuilder;
import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_ACCESSPOLICY_ID;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_ASSETS_SELECTOR;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_CONTRACTPOLICY_ID;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.CONTRACT_DEFINITION_TYPE;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.TYPE;
import static org.eclipse.edc.connector.contract.spi.types.offer.ContractDefinition.*;
import static org.eclipse.edc.jsonld.spi.JsonLdKeywords.*;
import static org.eclipse.edc.spi.CoreConstants.EDC_NAMESPACE;
import static org.eclipse.edc.spi.CoreConstants.EDC_PREFIX;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
Expand All @@ -40,6 +43,8 @@ class JsonObjectToContractDefinitionTransformerTest {
private final JsonObjectToContractDefinitionTransformer transformer = new JsonObjectToContractDefinitionTransformer();
private final TransformerContext context = mock(TransformerContext.class);

private final TitaniumJsonLd jsonLd = new TitaniumJsonLd(mock(Monitor.class));

@Test
void types() {
assertThat(transformer.getInputType()).isEqualTo(JsonObject.class);
Expand Down Expand Up @@ -84,4 +89,36 @@ void transform_whenNoAssetSelectorItShouldBeAnEmptyList() {
verify(context, never()).transform(any(), any());
}


@Test
void transform_withPrivateProperties() {
when(context.transform(any(), eq(Object.class))).thenReturn("test-val");
var jsonObj = createObjectBuilder()
.add(CONTEXT, createContextBuilder().addNull(EDC_PREFIX).build())
.add(ID, "some-contract-definition-id")
.add(TYPE, CONTRACT_DEFINITION_TYPE)
.add(CONTRACT_DEFINITION_ACCESSPOLICY_ID, "accessPolicyId")
.add(CONTRACT_DEFINITION_CONTRACTPOLICY_ID, "contractPolicyId")
.add(CONTRACT_DEFINITION_PRIVATE_PROPERTIES, createArrayBuilder().add(createObjectBuilder().add("test-prop", "test-val").build()).build())
.build();

jsonObj = expand(jsonObj);
var contractDefinition = transformer.transform(jsonObj, context);

//AbstractResultAssert.assertThat(contractDefinition).withFailMessage(contractDefinition::getFailureDetail).isSucceeded();
mohannad-ezzo marked this conversation as resolved.
Show resolved Hide resolved
assertThat(contractDefinition.getPrivateProperties())
.hasSize(1)
.containsEntry(EDC_NAMESPACE + "test-prop", "test-val");
}

private JsonObject expand(JsonObject jsonObject) {
return jsonLd.expand(jsonObject).orElseThrow(f -> new AssertionError(f.getFailureDetail()));
}

private JsonObjectBuilder createContextBuilder() {
return createObjectBuilder()
.add(VOCAB, EDC_NAMESPACE)
.add(EDC_PREFIX, EDC_NAMESPACE);
}

}
10 changes: 10 additions & 0 deletions extensions/control-plane/store/sql/contract-definition-store-sql/docs/er.puml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,14 @@ entity edc_contract_definitions {
* contract_policy: string <<json>>
* assets_selector: string <<json>>
}

entity edc_contract_definition_property {
* contract_definition_id_fk: string <<PK>>
* property_name: string
* property_value: string
* property_type: string
--
}

edc_contract_definitions ||--o{ edc_contract_definition_property
@enduml
28 changes: 28 additions & 0 deletions extensions/control-plane/store/sql/contract-definition-store-sql/docs/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
-- Contributors:
-- Daimler TSS GmbH - Initial SQL Query
-- Microsoft Corporation - refactoring
-- SAP SE - add private properties to contract definition
--

-- table: edc_contract_definitions
Expand All @@ -23,3 +24,30 @@ CREATE TABLE IF NOT EXISTS edc_contract_definitions
assets_selector JSON NOT NULL,
PRIMARY KEY (contract_definition_id)
);


-- table: edc_contract_definition_property
CREATE TABLE IF NOT EXISTS edc_contract_definition_property
(
contract_definition_id_fk VARCHAR(255) NOT NULL,
property_name VARCHAR(255) NOT NULL,
property_value TEXT NOT NULL,
property_type VARCHAR(255) NOT NULL,
property_is_private BOOLEAN,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed, since contract definitions can only have private properties.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was copied by the sql asset index implementation, but to be fair I'd prefer to follow the transfer process one, where the private properties are serialized into a json field.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the same pattern which is used for Asset. Therefore, I followed this approach. In addition, I intended to add property_is_private as a preparation for adding non-private properties for the contract definition. To be honest, our usecase for contract definition is just to have private properties, but not for non-private properties. So, as requested, the property property_is_private is removed and the corresponding code is adjusted.

PRIMARY KEY (contract_definition_id_fk, property_name),
FOREIGN KEY (contract_definition_id_fk) REFERENCES edc_contract_definitions (contract_definition_id) ON DELETE CASCADE
);


CREATE INDEX IF NOT EXISTS idx_edc_contract_definition_property_value
ON edc_contract_definition_property (property_name, property_value);


COMMENT ON COLUMN edc_contract_definition_property.property_name IS
'Contract definition property key';
COMMENT ON COLUMN edc_contract_definition_property.property_value IS
'Contract definition property value';
COMMENT ON COLUMN edc_contract_definition_property.property_type IS
'Contract definition property class name';
COMMENT ON COLUMN edc_contract_definition_property.property_is_private IS
'Contract definition property private flag';
Loading
Loading