Skip to content

Commit

Permalink
refactor: get rid of managedResource
Browse files Browse the repository at this point in the history
  • Loading branch information
@ndr-brt
ndr-brt committed Jul 13, 2023
1 parent c37de26 commit 1140c50
Show file tree
Hide file tree
Showing 23 changed files with 31 additions and 174 deletions.
4 changes: 0 additions & 4 deletions ...a/org/eclipse/edc/connector/service/transferprocess/TransferProcessEventDispatchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ void shouldDispatchEventsOnTransferProcessStateChanges(TransferProcessService se
.assetId("assetId")
.destinationType("any")
.protocol("test")
.managedResources(false)
.connectorAddress("http://an/address")
.contractId("contractId")
.build();
Expand Down Expand Up @@ -173,7 +172,6 @@ void shouldTerminateOnInvalidPolicy(TransferProcessService service, EventRouter
.assetId("assetId")
.destinationType("any")
.protocol("test")
.managedResources(false)
.connectorAddress("http://an/address")
.contractId("contractId")
.build();
Expand All @@ -200,7 +198,6 @@ void shouldDispatchEventOnTransferProcessTerminated(TransferProcessService servi
.assetId("assetId")
.destinationType("any")
.protocol("test")
.managedResources(true)
.connectorAddress("http://an/address")
.build();

Expand All @@ -225,7 +222,6 @@ void shouldDispatchEventOnTransferProcessFailure(TransferProcessService service,
.assetId("assetId")
.destinationType("any")
.protocol("test")
.managedResources(false)
.connectorAddress("http://an/address")
.build();

Expand Down
14 changes: 3 additions & 11 deletions .../src/main/java/org/eclipse/edc/connector/transfer/process/TransferProcessManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.eclipse.edc.connector.transfer.spi.types.ProvisionedContentResource;
import org.eclipse.edc.connector.transfer.spi.types.ProvisionedDataAddressResource;
import org.eclipse.edc.connector.transfer.spi.types.ProvisionedDataDestinationResource;
import org.eclipse.edc.connector.transfer.spi.types.ProvisionedResource;
import org.eclipse.edc.connector.transfer.spi.types.ResourceManifest;
import org.eclipse.edc.connector.transfer.spi.types.TransferProcess;
import org.eclipse.edc.connector.transfer.spi.types.TransferProcessStates;
Expand Down Expand Up @@ -75,7 +74,6 @@

import static java.lang.String.format;
import static java.lang.String.join;
import static java.util.Collections.emptyList;
import static org.eclipse.edc.connector.transfer.TransferCoreExtension.DEFAULT_BATCH_SIZE;
import static org.eclipse.edc.connector.transfer.TransferCoreExtension.DEFAULT_ITERATION_WAIT;
import static org.eclipse.edc.connector.transfer.TransferCoreExtension.DEFAULT_SEND_RETRY_BASE_DELAY;
Expand Down Expand Up @@ -500,16 +498,10 @@ private boolean processStarted(TransferProcess transferProcess) {
private Boolean checkCompletion(TransferProcess transferProcess) {
var checker = statusCheckerRegistry.resolve(transferProcess.getDataRequest().getDestinationType());
if (checker == null) {
if (transferProcess.getDataRequest().isManagedResources()) {
monitor.warning(format("No checker found for process %s. The process will not advance to the COMPLETED state.", transferProcess.getId()));
return false;
} else {
//no checker, transition the process to the COMPLETED state automatically
transitionToCompleting(transferProcess);
}
return true;
monitor.warning(format("No checker found for process %s. The process will not advance to the COMPLETED state.", transferProcess.getId()));
return false;
} else {
List<ProvisionedResource> resources = transferProcess.getDataRequest().isManagedResources() ? transferProcess.getProvisionedResourceSet().getResources() : emptyList();
var resources = transferProcess.getProvisionedResources();
if (checker.isComplete(transferProcess, resources)) {
transitionToCompleting(transferProcess);
return true;
Expand Down
3 changes: 0 additions & 3 deletions ...main/java/org/eclipse/edc/connector/transfer/provision/ResourceManifestGeneratorImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ public void registerGenerator(ProviderResourceDefinitionGenerator generator) {

@Override
public Result<ResourceManifest> generateConsumerResourceManifest(DataRequest dataRequest, Policy policy) {
if (!dataRequest.isManagedResources()) {
return Result.success(ResourceManifest.Builder.newInstance().build());
}
var definitions = consumerGenerators.stream()
.filter(generator -> generator.canGenerate(dataRequest, policy))
.map(generator -> generator.generate(dataRequest, policy))
Expand Down
3 changes: 1 addition & 2 deletions ...org/eclipse/edc/connector/transfer/process/TransferProcessManagerImplIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,9 @@ private ProvisionedResourceSet provisionedResourceSet() {
}

private TransferProcess.Builder createInitialTransferProcess() {
String processId = UUID.randomUUID().toString();
var processId = UUID.randomUUID().toString();
var dataRequest = DataRequest.Builder.newInstance()
.id(processId)
.managedResources(true)
.destinationType("test-type")
.contractId(UUID.randomUUID().toString())
.build();
Expand Down
49 changes: 4 additions & 45 deletions .../test/java/org/eclipse/edc/connector/transfer/process/TransferProcessManagerImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,25 +651,8 @@ void starting_whenShouldWait_updatesStateCount() {
}

@Test
void started_shouldComplete_whenManagedResourcesAndCheckerCompleted() {
var process = createTransferProcess(STARTED);
process.getProvisionedResourceSet().addResource(provisionedDataDestinationResource());
process.getProvisionedResourceSet().addResource(provisionedDataDestinationResource());

when(transferProcessStore.nextNotLeased(anyInt(), stateIs(STARTED.code()))).thenReturn(List.of(process)).thenReturn(emptyList());
when(statusCheckerRegistry.resolve(anyString())).thenReturn((tp, resources) -> true);

manager.start();

await().untilAsserted(() -> {
verify(statusCheckerRegistry, atLeastOnce()).resolve(any());
verify(transferProcessStore).updateOrCreate(argThat(p -> p.getState() == COMPLETING.code()));
});
}

@Test
void started_shouldComplete_whenNotManagedResourcesAndCheckerCompleted() {
var process = createTransferProcessBuilder(STARTED, false)
void started_shouldComplete_whenCheckerCompleted() {
var process = createTransferProcessBuilder(STARTED)
.provisionedResourceSet(ProvisionedResourceSet.Builder.newInstance()
.resources(List.of(provisionedDataDestinationResource(), provisionedDataDestinationResource()))
.build())
Expand Down Expand Up @@ -703,7 +686,7 @@ void started_shouldBreakLeaseAndNotComplete_whenNotAllYetCompleted() {
}

@Test
void started_shouldNotComplete_whenNoCheckerForManaged() {
void started_shouldNotComplete_whenNoChecker() {
var process = createTransferProcess(STARTED);
process.getProvisionedResourceSet().addResource(provisionedDataDestinationResource());
process.getProvisionedResourceSet().addResource(provisionedDataDestinationResource());
Expand All @@ -718,25 +701,6 @@ void started_shouldNotComplete_whenNoCheckerForManaged() {
});
}

@Test
void started_shouldComplete_whenNoCheckerForNotManaged() {
var process = createTransferProcessBuilder(STARTED, false)
.provisionedResourceSet(ProvisionedResourceSet.Builder.newInstance()
.resources(List.of(provisionedDataDestinationResource(), provisionedDataDestinationResource()))
.build())
.build();

when(transferProcessStore.nextNotLeased(anyInt(), stateIs(STARTED.code()))).thenReturn(List.of(process)).thenReturn(emptyList());
when(statusCheckerRegistry.resolve(anyString())).thenReturn(null);

manager.start();

await().untilAsserted(() -> {
verify(statusCheckerRegistry, atLeastOnce()).resolve(any());
verify(transferProcessStore).updateOrCreate(argThat(p -> p.getState() == COMPLETING.code()));
});
}

@Test
void completing_shouldTransitionToCompleted_whenSendingMessageSucceed() {
var process = createTransferProcess(COMPLETING);
Expand Down Expand Up @@ -1001,20 +965,15 @@ private DataFlowResponse createDataFlowResponse() {
}

private TransferProcess createTransferProcess(TransferProcessStates inState) {
return createTransferProcessBuilder(inState, true).build();
return createTransferProcessBuilder(inState).build();
}

private TransferProcess.Builder createTransferProcessBuilder(TransferProcessStates inState) {
return createTransferProcessBuilder(inState, true);
}

private TransferProcess.Builder createTransferProcessBuilder(TransferProcessStates inState, boolean managed) {
var processId = UUID.randomUUID().toString();
var dataRequest = createDataRequestBuilder()
.processId(processId)
.protocol("protocol")
.connectorAddress("http://an/address")
.managedResources(managed)
.build();

return TransferProcess.Builder.newInstance()
Expand Down
27 changes: 7 additions & 20 deletions .../java/org/eclipse/edc/connector/transfer/provision/ResourceManifestGeneratorImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void setUp() {

@Test
void shouldGenerateResourceManifestForConsumerManagedTransferProcess() {
var dataRequest = createDataRequest(true);
var dataRequest = createDataRequest();
var resourceDefinition = TestResourceDefinition.Builder.newInstance().id(UUID.randomUUID().toString()).build();
when(consumerGenerator.canGenerate(any(), any())).thenReturn(true);
when(consumerGenerator.generate(any(), any())).thenReturn(resourceDefinition);
Expand All @@ -70,7 +70,7 @@ void shouldGenerateResourceManifestForConsumerManagedTransferProcess() {

@Test
void shouldGenerateEmptyResourceManifestForNotGeneratedFilter() {
var dataRequest = createDataRequest(true);
var dataRequest = createDataRequest();
when(consumerGenerator.canGenerate(any(), any())).thenReturn(false);
when(policyEngine.evaluate(any(), any(), isA(PolicyContext.class))).thenReturn(Result.success());

Expand All @@ -80,21 +80,9 @@ void shouldGenerateEmptyResourceManifestForNotGeneratedFilter() {
verifyNoInteractions(providerGenerator);
}

@Test
void shouldGenerateEmptyResourceManifestForEmptyConsumerNotManagedTransferProcess() {
var dataRequest = createDataRequest(false);

var result = generator.generateConsumerResourceManifest(dataRequest, policy);

assertThat(result.succeeded()).isTrue();
assertThat(result.getContent().getDefinitions()).isEmpty();
verifyNoInteractions(consumerGenerator);
verifyNoInteractions(providerGenerator);
}

@Test
void shouldReturnFailedResultForConsumerWhenPolicyEvaluationFailed() {
var dataRequest = createDataRequest(true);
var dataRequest = createDataRequest();
var resourceDefinition = TestResourceDefinition.Builder.newInstance().id(UUID.randomUUID().toString()).build();
when(consumerGenerator.generate(any(), any())).thenReturn(resourceDefinition);
when(policyEngine.evaluate(any(), any(), isA(PolicyContext.class))).thenReturn(Result.failure("error"));
Expand All @@ -106,7 +94,7 @@ void shouldReturnFailedResultForConsumerWhenPolicyEvaluationFailed() {

@Test
void shouldGenerateResourceManifestForProviderTransferProcess() {
var process = createDataRequest(false);
var process = createDataRequest();
var resourceDefinition = TestResourceDefinition.Builder.newInstance().id(UUID.randomUUID().toString()).build();
when(providerGenerator.canGenerate(any(), any(), any())).thenReturn(true);
when(providerGenerator.generate(any(), any(), any())).thenReturn(resourceDefinition);
Expand All @@ -119,7 +107,7 @@ void shouldGenerateResourceManifestForProviderTransferProcess() {

@Test
void shouldGenerateEmptyResourceManifestForProviderTransferProcess() {
var process = createDataRequest(false);
var process = createDataRequest();
when(providerGenerator.canGenerate(any(), any(), any())).thenReturn(false);

var resourceManifest = generator.generateProviderResourceManifest(process, dataAddress, policy);
Expand All @@ -128,9 +116,8 @@ void shouldGenerateEmptyResourceManifestForProviderTransferProcess() {
verifyNoInteractions(consumerGenerator);
}


private DataRequest createDataRequest(boolean managedResources) {
private DataRequest createDataRequest() {
var destination = DataAddress.Builder.newInstance().type("any").build();
return DataRequest.Builder.newInstance().managedResources(managedResources).dataDestination(destination).build();
return DataRequest.Builder.newInstance().dataDestination(destination).build();
}
}
11 changes: 0 additions & 11 deletions ...va/org/eclipse/edc/connector/api/management/transferprocess/model/TransferRequestDto.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public class TransferRequestDto {
public static final String EDC_TRANSFER_REQUEST_DTO_CONNECTOR_ADDRESS = EDC_NAMESPACE + "connectorAddress";
public static final String EDC_TRANSFER_REQUEST_DTO_CONTRACT_ID = EDC_NAMESPACE + "contractId";
public static final String EDC_TRANSFER_REQUEST_DTO_DATA_DESTINATION = EDC_NAMESPACE + "dataDestination";
public static final String EDC_TRANSFER_REQUEST_DTO_MANAGED_RESOURCES = EDC_NAMESPACE + "managedResources";
public static final String EDC_TRANSFER_REQUEST_DTO_PROPERTIES = EDC_NAMESPACE + "properties";

public static final String EDC_TRANSFER_REQUEST_DTO_PRIVATE_PROPERTIES = EDC_NAMESPACE + "privateProperties";
Expand All @@ -48,7 +47,6 @@ public class TransferRequestDto {
private String connectorAddress; // TODO change to callbackAddress
private String contractId;
private DataAddress dataDestination;
private boolean managedResources = true;
private Map<String, String> properties = new HashMap<>();

private Map<String, String> privateProperties = new HashMap<>();
Expand Down Expand Up @@ -76,10 +74,6 @@ public DataAddress getDataDestination() {
return dataDestination;
}

public boolean isManagedResources() {
return managedResources;
}

public Map<String, String> getProperties() {
return properties;
}
Expand Down Expand Up @@ -137,11 +131,6 @@ public Builder dataDestination(DataAddress dataDestination) {
return this;
}

public Builder managedResources(boolean managedResources) {
request.managedResources = managedResources;
return this;
}

public Builder properties(Map<String, String> properties) {
request.properties = properties;
return this;
Expand Down
3 changes: 0 additions & 3 deletions ...r/api/management/transferprocess/transform/JsonObjectToTransferRequestDtoTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_CONNECTOR_ID;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_CONTRACT_ID;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_DATA_DESTINATION;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_MANAGED_RESOURCES;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_PRIVATE_PROPERTIES;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_PROPERTIES;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_PROTOCOL;
Expand All @@ -62,8 +61,6 @@ public JsonObjectToTransferRequestDtoTransformer() {
return (v) -> builder.contractId(transformString(v, context));
case EDC_TRANSFER_REQUEST_DTO_DATA_DESTINATION:
return (v) -> builder.dataDestination(transformObject(v, DataAddress.class, context));
case EDC_TRANSFER_REQUEST_DTO_MANAGED_RESOURCES:
return (v) -> builder.managedResources(transformBoolean(v, context));
case EDC_TRANSFER_REQUEST_DTO_PROPERTIES:
return (v) -> transformProperties(v, builder::properties, context);
case EDC_TRANSFER_REQUEST_DTO_CALLBACK_ADDRESSES:
Expand Down
1 change: 0 additions & 1 deletion .../management/transferprocess/transform/TransferRequestDtoToTransferRequestTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public Class<TransferRequest> getOutputType() {
.connectorAddress(object.getConnectorAddress())
.contractId(object.getContractId())
.destinationType(object.getDataDestination().getType())
.managedResources(object.isManagedResources())
.protocol(object.getProtocol())
.dataDestination(object.getDataDestination())
.build();
Expand Down
3 changes: 0 additions & 3 deletions ...i/management/transferprocess/transform/JsonObjectToTransferRequestDtoTransformerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_CONNECTOR_ID;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_CONTRACT_ID;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_DATA_DESTINATION;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_MANAGED_RESOURCES;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_PRIVATE_PROPERTIES;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_PROPERTIES;
import static org.eclipse.edc.connector.api.management.transferprocess.model.TransferRequestDto.EDC_TRANSFER_REQUEST_DTO_PROTOCOL;
Expand Down Expand Up @@ -72,7 +71,6 @@ void transform() {
.add(EDC_TRANSFER_REQUEST_DTO_CONNECTOR_ADDRESS, "address")
.add(EDC_TRANSFER_REQUEST_DTO_CONTRACT_ID, "contractId")
.add(EDC_TRANSFER_REQUEST_DTO_DATA_DESTINATION, dataDestinationJson)
.add(EDC_TRANSFER_REQUEST_DTO_MANAGED_RESOURCES, false)
.add(EDC_TRANSFER_REQUEST_DTO_PROPERTIES, propertiesJson)
.add(EDC_TRANSFER_REQUEST_DTO_PRIVATE_PROPERTIES, privatePropertiesJson)
.add(EDC_TRANSFER_REQUEST_DTO_PROTOCOL, "protocol")
Expand All @@ -87,7 +85,6 @@ void transform() {
assertThat(result.getConnectorAddress()).isEqualTo("address");
assertThat(result.getContractId()).isEqualTo("contractId");
assertThat(result.getDataDestination()).isSameAs(dataDestination);
assertThat(result.isManagedResources()).isFalse();
assertThat(result.getProperties()).containsAllEntriesOf(properties);
assertThat(result.getPrivateProperties()).containsAllEntriesOf(privateProperties);
assertThat(result.getProtocol()).isEqualTo("protocol");
Expand Down
4 changes: 2 additions & 2 deletions ...agement/transferprocess/transform/TransferRequestDtoToTransferRequestTransformerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,14 @@ void getOutputType() {

@Test
void transform() {
String destinationType = "test-type";
var destinationType = "test-type";
var context = mock(TransformerContext.class);
var transferReq = transferRequestDto()
.id(UUID.randomUUID().toString())
.build();

var transferRequest = transformer.transform(transferReq, context);

assertThat(transferRequest.getDataRequest())
.isNotNull()
.extracting(DataRequest::getDestinationType)
Expand All @@ -70,7 +71,6 @@ void transform() {
assertThat(dataRequest.getDestinationType()).isEqualTo(transferReq.getDataDestination().getType());
assertThat(dataRequest.getContractId()).isEqualTo(transferReq.getContractId());
assertThat(dataRequest.getProtocol()).isEqualTo(transferReq.getProtocol());
assertThat(dataRequest.isManagedResources()).isEqualTo(transferReq.isManagedResources());

assertThat(transferRequest.getCallbackAddresses()).hasSize(transferReq.getCallbackAddresses().size());
assertThat(transferRequest.getPrivateProperties()).isEqualTo(transferReq.getPrivateProperties());
Expand Down
Loading

0 comments on commit 1140c50

Please sign in to comment.