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

Jump to bottom

refactor: put DataAddress as Asset field #3244

Merged
merged 1 commit into from
Jul 5, 2023
Merged

refactor: put DataAddress as Asset field #3244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 11 additions & 14 deletions ...e/src/test/java/org/eclipse/edc/connector/catalog/DatasetResolverImplIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.eclipse.edc.spi.query.QuerySpec;
import org.eclipse.edc.spi.types.domain.DataAddress;
import org.eclipse.edc.spi.types.domain.asset.Asset;
import org.eclipse.edc.spi.types.domain.asset.AssetEntry;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -77,9 +76,9 @@ void shouldLimitResult_withHeterogenousChunks() {
var assets2 = range(24, 113).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
var assets3 = range(113, 178).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());

store(assets1);
store(assets2);
store(assets3);
assets1.forEach(assetIndex::create);
assets2.forEach(assetIndex::create);
assets3.forEach(assetIndex::create);

var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
Expand All @@ -104,8 +103,8 @@ void should_return_offers_subset_when_across_multiple_contract_definitions(int f
var maximumRange = max(0, (assets1.size() + assets2.size()) - from);
var requestedRange = to - from;

store(assets1);
store(assets2);
assets1.forEach(assetIndex::create);
assets2.forEach(assetIndex::create);

var contractDefinition1 = getContractDefBuilder("contract-definition-")
.assetsSelector(selectorFrom(assets1)).build();
Expand All @@ -125,8 +124,8 @@ void shouldLimitResult_insufficientAssets() {
var assets1 = range(0, 12).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
var assets2 = range(12, 18).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());

store(assets1);
store(assets2);
assets1.forEach(assetIndex::create);
assets2.forEach(assetIndex::create);

var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
Expand Down Expand Up @@ -162,11 +161,6 @@ private ParticipantAgent createAgent() {
return new ParticipantAgent(emptyMap(), emptyMap());
}

private void store(Collection<Asset> assets) {
assets.stream().map(a -> new AssetEntry(a, DataAddress.Builder.newInstance().type("test-type").build()))
.forEach(assetIndex::create);
}

private List<Criterion> selectorFrom(Collection<Asset> assets1) {
var ids = assets1.stream().map(Asset::getId).collect(Collectors.toList());
return List.of(new Criterion(Asset.PROPERTY_ID, "in", ids));
Expand All @@ -181,7 +175,10 @@ private ContractDefinition.Builder getContractDefBuilder(String id) {
}

private Asset.Builder createAsset(String id) {
return Asset.Builder.newInstance().id(id).name("test asset " + id);
return Asset.Builder.newInstance()
.id(id)
.name("test asset " + id)
.dataAddress(DataAddress.Builder.newInstance().type("test-type").build());
}

static class RangeProvider implements ArgumentsProvider {
Expand Down
15 changes: 6 additions & 9 deletions ...e/src/test/java/org/eclipse/edc/connector/catalog/DatasetResolverImplPerformanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.eclipse.edc.spi.query.QuerySpec;
import org.eclipse.edc.spi.types.domain.DataAddress;
import org.eclipse.edc.spi.types.domain.asset.Asset;
import org.eclipse.edc.spi.types.domain.asset.AssetEntry;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -70,7 +69,7 @@ void oneAssetPerDefinition(DatasetResolver datasetResolver, ContractDefinitionSt
.contractPolicyId("policy")
.assetsSelectorCriterion(criterion(Asset.PROPERTY_ID, "=", String.valueOf(i))).build()
).forEach(contractDefinitionStore::save);
range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).map(this::createAssetEntry).forEach(assetIndex::create);
range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).forEach(assetIndex::create);

var firstPageQuery = QuerySpec.Builder.newInstance().offset(0).limit(100).build();
var firstPageDatasets = queryDatasetsIn(datasetResolver, firstPageQuery, ofSeconds(1));
Expand All @@ -87,7 +86,7 @@ void oneAssetPerDefinition(DatasetResolver datasetResolver, ContractDefinitionSt
void fewDefinitionsSelectAllAssets(DatasetResolver datasetResolver, ContractDefinitionStore contractDefinitionStore, AssetIndex assetIndex, PolicyDefinitionStore policyDefinitionStore) {
policyDefinitionStore.create(createPolicyDefinition("policy").build());
range(0, 10).mapToObj(i -> createContractDefinition(String.valueOf(i)).accessPolicyId("policy").contractPolicyId("policy").build()).forEach(contractDefinitionStore::save);
range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).map(this::createAssetEntry).forEach(assetIndex::create);
range(0, 10000).mapToObj(i -> createAsset(String.valueOf(i)).build()).forEach(assetIndex::create);

var firstPageQuery = QuerySpec.Builder.newInstance().offset(0).limit(100).build();
var firstPageDatasets = queryDatasetsIn(datasetResolver, firstPageQuery, ofSeconds(1));
Expand Down Expand Up @@ -116,12 +115,10 @@ private ContractDefinition.Builder createContractDefinition(String id) {
.contractPolicyId("contract");
}

@NotNull
private AssetEntry createAssetEntry(Asset it) {
return new AssetEntry(it, DataAddress.Builder.newInstance().type("type").build());
}

private Asset.Builder createAsset(String id) {
return Asset.Builder.newInstance().id(id).name("test asset " + id);
return Asset.Builder.newInstance()
.id(id)
.name("test asset " + id)
.dataAddress(DataAddress.Builder.newInstance().type("type").build());
}
}
3 changes: 2 additions & 1 deletion ...va/org/eclipse/edc/connector/contract/negotiation/ContractNegotiationIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.eclipse.edc.connector.policy.spi.store.PolicyDefinitionStore;
import org.eclipse.edc.connector.service.contractnegotiation.ContractNegotiationProtocolServiceImpl;
import org.eclipse.edc.connector.spi.contractnegotiation.ContractNegotiationProtocolService;
import org.eclipse.edc.junit.annotations.ComponentTest;
import org.eclipse.edc.policy.model.Action;
import org.eclipse.edc.policy.model.Duty;
import org.eclipse.edc.policy.model.Policy;
Expand Down Expand Up @@ -76,7 +77,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

//@ComponentTest
@ComponentTest
class ContractNegotiationIntegrationTest {
private static final String CONSUMER_ID = "consumer";
private static final String PROVIDER_ID = "provider";
Expand Down
25 changes: 11 additions & 14 deletions ...va/org/eclipse/edc/connector/contract/offer/ContractOfferResolverImplIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.eclipse.edc.spi.query.Criterion;
import org.eclipse.edc.spi.types.domain.DataAddress;
import org.eclipse.edc.spi.types.domain.asset.Asset;
import org.eclipse.edc.spi.types.domain.asset.AssetEntry;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtensionContext;
Expand Down Expand Up @@ -90,9 +89,9 @@ void shouldLimitResult_withHeterogenousChunks() {
var assets2 = range(24, 113).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
var assets3 = range(113, 178).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());

store(assets1);
store(assets2);
store(assets3);
assets1.forEach(assetIndex::create);
assets2.forEach(assetIndex::create);
assets3.forEach(assetIndex::create);

var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
Expand Down Expand Up @@ -123,8 +122,8 @@ void should_return_offers_subset_when_across_multiple_contract_definitions(int f
var maximumRange = max(0, (assets1.size() + assets2.size()) - from);
var requestedRange = to - from;

store(assets1);
store(assets2);
assets1.forEach(assetIndex::create);
assets2.forEach(assetIndex::create);

var contractDefinition1 = getContractDefBuilder("contract-definition-")
.assetsSelector(selectorFrom(assets1)).build();
Expand All @@ -144,8 +143,8 @@ void shouldLimitResult_insufficientAssets() {
var assets1 = range(0, 12).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());
var assets2 = range(12, 18).mapToObj(i -> createAsset("asset" + i).build()).collect(Collectors.toList());

store(assets1);
store(assets2);
assets1.forEach(assetIndex::create);
assets2.forEach(assetIndex::create);

var def1 = getContractDefBuilder("def1").assetsSelector(selectorFrom(assets1)).build();
var def2 = getContractDefBuilder("def2").assetsSelector(selectorFrom(assets2)).build();
Expand Down Expand Up @@ -186,11 +185,6 @@ void shouldLimitResult_pageOffsetLargerThanNumAssets() {
verify(policyStore, never()).findById("contract");
}

private void store(Collection<Asset> assets) {
assets.stream().map(a -> new AssetEntry(a, DataAddress.Builder.newInstance().type("test-type").build()))
.forEach(assetIndex::create);
}

private List<Criterion> selectorFrom(Collection<Asset> assets1) {
var ids = assets1.stream().map(Asset::getId).toList();
return List.of(new Criterion(Asset.PROPERTY_ID, "in", ids));
Expand All @@ -204,7 +198,10 @@ private ContractDefinition.Builder getContractDefBuilder(String id) {
}

private Asset.Builder createAsset(String id) {
return Asset.Builder.newInstance().id(id).name("test asset " + id);
return Asset.Builder.newInstance()
.id(id)
.name("test asset " + id)
.dataAddress(DataAddress.Builder.newInstance().type("test-type").build());
}

static class RangeProvider implements ArgumentsProvider {
Expand Down
9 changes: 7 additions & 2 deletions ...gate-services/src/main/java/org/eclipse/edc/connector/service/asset/AssetServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,18 @@ public ServiceResult<Stream<Asset>> query(QuerySpec query) {

@Override
public ServiceResult<Asset> create(Asset asset, DataAddress dataAddress) {
var validDataAddress = dataAddressValidator.validate(dataAddress);
return create(asset.toBuilder().dataAddress(dataAddress).build());
}

@Override
public ServiceResult<Asset> create(Asset asset) {
var validDataAddress = dataAddressValidator.validate(asset.getDataAddress());
if (validDataAddress.failed()) {
return ServiceResult.badRequest(validDataAddress.getFailureMessages());
}

return transactionContext.execute(() -> {
var createResult = index.create(asset, dataAddress);
var createResult = index.create(asset);
if (createResult.succeeded()) {
observable.invokeForEach(l -> l.created(asset));
return ServiceResult.success(asset);
Expand Down
64 changes: 54 additions & 10 deletions ...-services/src/test/java/org/eclipse/edc/connector/service/asset/AssetServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import org.eclipse.edc.connector.contract.spi.negotiation.store.ContractNegotiationStore;
import org.eclipse.edc.connector.contract.spi.types.agreement.ContractAgreement;
import org.eclipse.edc.connector.contract.spi.types.negotiation.ContractNegotiation;
import org.eclipse.edc.connector.spi.asset.AssetService;
import org.eclipse.edc.policy.model.Policy;
import org.eclipse.edc.service.spi.result.ServiceFailure;
import org.eclipse.edc.service.spi.result.ServiceResult;
import org.eclipse.edc.spi.asset.AssetIndex;
import org.eclipse.edc.spi.dataaddress.DataAddressValidator;
Expand Down Expand Up @@ -53,9 +55,11 @@
import static org.eclipse.edc.service.spi.result.ServiceFailure.Reason.NOT_FOUND;
import static org.eclipse.edc.spi.query.Criterion.criterion;
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;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
Expand All @@ -72,7 +76,7 @@ class AssetServiceImplTest {
private final AssetObservable observable = mock(AssetObservable.class);
private final DataAddressValidator dataAddressValidator = mock(DataAddressValidator.class);

private final AssetServiceImpl service = new AssetServiceImpl(index, contractNegotiationStore, dummyTransactionContext,
private final AssetService service = new AssetServiceImpl(index, contractNegotiationStore, dummyTransactionContext,
observable, dataAddressValidator);

@Test
Expand Down Expand Up @@ -129,37 +133,77 @@ void query_invalidFilter(Criterion filter) {

@Test
void createAsset_shouldCreateAssetIfItDoesNotAlreadyExist() {
when(dataAddressValidator.validate(any())).thenReturn(Result.success());
var assetId = "assetId";
var asset = createAsset(assetId);
when(index.create(asset)).thenReturn(StoreResult.success());

var inserted = service.create(asset);

assertThat(inserted.succeeded()).isTrue();
assertThat(inserted.getContent()).matches(hasId(assetId));
verify(index).create(and(isA(Asset.class), argThat(it -> assetId.equals(it.getId()))));
verifyNoMoreInteractions(index);
verify(observable).invokeForEach(any());
}

@Test
void createAsset_shouldNotCreateAssetIfItAlreadyExists() {
when(dataAddressValidator.validate(any())).thenReturn(Result.success());
var asset = createAsset("assetId");
when(index.create(asset)).thenReturn(StoreResult.alreadyExists("test"));

var inserted = service.create(asset);

assertThat(inserted).isFailed().extracting(ServiceFailure::getReason).isEqualTo(CONFLICT);
}

@Test
void createAsset_shouldNotCreateAssetIfDataAddressInvalid() {
var asset = createAsset("assetId");
when(dataAddressValidator.validate(any())).thenReturn(Result.failure("Data address is invalid"));

var result = service.create(asset);

Assertions.assertThat(result).satisfies(ServiceResult::failed)
.extracting(ServiceResult::reason)
.isEqualTo(BAD_REQUEST);
verifyNoInteractions(index);
}

@Test
@Deprecated(since = "0.1.2")
void createAssetDeprecated_shouldCreateAssetIfItDoesNotAlreadyExist() {
when(dataAddressValidator.validate(any())).thenReturn(Result.success());
var assetId = "assetId";
var asset = createAsset(assetId);
var addressType = "addressType";
var dataAddress = DataAddress.Builder.newInstance().type(addressType).build();
when(index.create(asset, dataAddress)).thenReturn(StoreResult.success());
when(index.create(isA(Asset.class))).thenReturn(StoreResult.success());

var inserted = service.create(asset, dataAddress);

assertThat(inserted.succeeded()).isTrue();
assertThat(inserted.getContent()).matches(hasId(assetId));
verify(index).create(argThat(it -> assetId.equals(it.getId())), argThat(it -> addressType.equals(it.getType())));
verifyNoMoreInteractions(index);
verify(observable).invokeForEach(any());
}

@Test
void createAsset_shouldNotCreateAssetIfItAlreadyExists() {
@Deprecated(since = "0.1.2")
void createAssetDeprecated_shouldNotCreateAssetIfItAlreadyExists() {
when(dataAddressValidator.validate(any())).thenReturn(Result.success());
var asset = createAsset("assetId");
var dataAddress = DataAddress.Builder.newInstance().type("addressType").build();
when(index.create(asset, dataAddress)).thenReturn(StoreResult.alreadyExists("test"));
when(index.create(isA(Asset.class))).thenReturn(StoreResult.alreadyExists("test"));

var inserted = service.create(asset, dataAddress);

assertThat(inserted.succeeded()).isFalse();
assertThat(inserted.reason()).isEqualTo(CONFLICT);
assertThat(inserted).isFailed().extracting(ServiceFailure::getReason).isEqualTo(CONFLICT);
}

@Test
void createAsset_shouldNotCreateAssetIfDataAddressInvalid() {
@Deprecated(since = "0.1.2")
void createAssetDeprecated_shouldNotCreateAssetIfDataAddressInvalid() {
var asset = createAsset("assetId");
var dataAddress = DataAddress.Builder.newInstance().type("addressType").build();
when(dataAddressValidator.validate(any())).thenReturn(Result.failure("Data address is invalid"));
Expand Down Expand Up @@ -307,6 +351,6 @@ private Predicate<Asset> hasId(String assetId) {
}

private Asset createAsset(String assetId) {
return Asset.Builder.newInstance().id(assetId).build();
return Asset.Builder.newInstance().id(assetId).dataAddress(DataAddress.Builder.newInstance().type("any").build()).build();
}
}
2 changes: 1 addition & 1 deletion ...lipse/edc/connector/service/contractnegotiation/ContractNegotiationEventDispatchTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ void shouldDispatchEventsOnProviderContractNegotiationStateChanges(EventRouter e
.build();
contractDefinitionStore.save(contractDefinition);
policyDefinitionStore.create(PolicyDefinition.Builder.newInstance().id("policyId").policy(policy).build());
assetIndex.create(Asset.Builder.newInstance().id("assetId").build(), DataAddress.Builder.newInstance().type("any").build());
assetIndex.create(Asset.Builder.newInstance().id("assetId").dataAddress(DataAddress.Builder.newInstance().type("any").build()).build());

service.notifyRequested(createContractOfferRequest(policy, "assetId"), token);

Expand Down
Loading