Skip to content

chore: conditionally process data address on resume flow #64

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

Closed
ronjaquensel wants to merge 1 commit into from
+11 −4
Closed

chore: conditionally process data address on resume flow #64

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
15 changes: 11 additions & 4 deletions src/main/java/org/eclipse/dataplane/Dataplane.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
import static jakarta.ws.rs.core.HttpHeaders.AUTHORIZATION;
import static java.util.Collections.emptyMap;
import static org.eclipse.dataplane.domain.dataflow.DataFlow.Type.CONSUMER;
import static org.eclipse.dataplane.domain.dataflow.DataFlow.Type.PROVIDER;

public class Dataplane {

Expand Down Expand Up @@ -132,7 +134,7 @@ public Result<DataFlowStatusMessage> prepare(String controlplaneId, DataFlowPrep
.counterPartyId(message.counterPartyId())
.dataspaceContext(message.dataspaceContext())
.controlplaneId(controlplaneId)
.type(DataFlow.Type.CONSUMER)
.type(CONSUMER)
.build();

return checkControlPlane(controlplaneId)
Expand Down Expand Up @@ -167,7 +169,7 @@ public Result<DataFlowStatusMessage> start(String controlplaneId, DataFlowStartM
.counterPartyId(message.counterPartyId())
.dataspaceContext(message.dataspaceContext())
.controlplaneId(controlplaneId)
.type(DataFlow.Type.PROVIDER)
.type(PROVIDER)
.build();

return checkControlPlane(controlplaneId)
Expand Down Expand Up @@ -201,7 +203,9 @@ public Result<Void> suspend(String flowId, DataFlowSuspendMessage message) {
public Result<DataFlowStatusMessage> resume(String flowId, DataFlowResumeMessage message) {
return dataFlowStore.findById(flowId)
.map(dataFlow -> {
if (message.dataAddress() != null) {
var shouldReceiveDataAddress = (PROVIDER.equals(dataFlow.getType()) && dataFlow.isPush()) ||
(CONSUMER.equals(dataFlow.getType()) && dataFlow.isPull());
if (shouldReceiveDataAddress) {
Comment on lines +206 to +208
Copy link
Copy Markdown
Contributor

@ndr-brt ndr-brt Mar 27, 2026

Choose a reason for hiding this comment

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

I'm not sure about this check, wouldn't the non null one already covering these cases? If the data plane receives a data address, it updates it, otherwise it uses the original one, unless there's a specific use case that we should avoid (eventually demonstrated by a test) I'd just leave the loose non-null check

dataFlow.setDataAddress(message.dataAddress());
}
return dataFlow;
Expand All @@ -210,7 +214,10 @@ public Result<DataFlowStatusMessage> resume(String flowId, DataFlowResumeMessage
.compose(dataFlow -> {
dataFlow.transitionToStarted();

var response = new DataFlowStatusMessage(id, flowId, dataFlow.getState().name(), dataFlow.getDataAddress(), null);
var shouldProvideDataAddress = (PROVIDER.equals(dataFlow.getType()) && dataFlow.isPull()) ||
(CONSUMER.equals(dataFlow.getType()) && dataFlow.isPush());
var dataAddress = shouldProvideDataAddress ? dataFlow.getDataAddress() : null;
var response = new DataFlowStatusMessage(id, flowId, dataFlow.getState().name(), dataAddress, null);
Comment on lines +217 to +220
Copy link
Copy Markdown
Contributor

@ndr-brt ndr-brt Mar 27, 2026

Choose a reason for hiding this comment

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

same, I think this is excessively defensive, unless there's a specific case for which the data address shouldn't be sent back, eventually let's try to describe it through a test


return save(dataFlow).map(it -> response);
});
Expand Down