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

refactor: get rid of managedResource #3297

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/dependency-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ jobs:
exit 1
fi
- name: print expected DEPENDENCIES file
run: |
cat DEPENDENCIES-gen
- name: Check for differences
run: |
Expand Down
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
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
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
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
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
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();
}
}
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
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
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
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
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