fix: stop method property from being mandatory for push http transfers

[ndr-brt]

What this PR changes/adds

The method property was mandatory when the proxyMethod was true, but, in fact, also if a source DataAddress defines that property, it could be that the same asset is used in a PUSH type transfer, this fix permits to do that by adding an additional check.

Why it does that

fix bug

Further notes

• in the future (see dataplane signaling DR) this information will be taken out from the transferType field that will be mandatory, while the dataDestination won't be supplied for pull transfers.

Linked Issue(s)

Closes #3768

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

[efiege]

I don't expect that this will solve the issue regarding the HttpPush. This will disable the feature of proxying http methods for the HttpPush, as now just "GET" requests will be sent to the providing backend.

Our expectation would be that http-methods also could be proxied for the HttpPush. We could also help providing this fix, if this is more convenient.

[ndr-brt]

simply, for http push this condition won't ever met, and it will pass through line 60, so the method will be taken from the DataAddress (as it already does) if it's defined, otherwise it will use the default GET.

To verify that, just try to remove the proxyMethod property and set method instead on the source data address.

The proxy was made to work with pull, not for push

[efiege]

simply, for http push this condition won't ever met, and it will pass through line 60, so the method will be taken from the DataAddress (as it already does) if it's defined, otherwise it will use the default GET.

Ohh yes, I have overlooked this: The HttpPush will use the method from the DataAddress.

The proxy was made to work with pull, not for push

This would implicate that certain HttpData assets can just be used for HttpProxy transfer-types and not for HttpData transfer-types. However it would make sense to also support this for the HttpPush.

So basically this would be a feature request?

[ndr-brt]

theoretically yes, I'm not sure which would be the applications for that: the consumer should be the one who chose how the call to its service will be made.

[efiege]

The applications would be the same as for the http-pull I think.

So for the http-pull the consumer is able to select the preferred http-method, which is used to access the provider system. This is enabled by using the prxoyMethod=true property in the asset of the provider.

For the http-push the transfer is also started by the consumer. Like for the http-pull the consumer should be able to select the prefered http-method, which is used to access the provider system.

[ndr-brt]

in any case, also without the proxyMethod flag to true, the consumer can choose the method, that by default is GET. Really, all the proxyXXX stuff is there for the proxy use case ("nomen omen").

[efiege]

Hey @ndr-brt,

I'm sry for coming back on this so lately, but this change is really important to us and I may not have understood this correctly.

After giving this a second look, I am also convinced, that this change won't affect passing methods for the HttpPush from the consumer.

My problem was, that I though the HttpDataAddress address parameter is always populated by the SourceDataAddress:

Connector/extensions/data-plane/data-plane-http/src/main/java/org/eclipse/edc/connector/dataplane/http/params/decorators/BaseSourceHttpParamsDecorator.java

Lines 55 to 61 in cf629c9

	private @NotNull String extractMethod(HttpDataAddress address, DataFlowStartMessage request) {
	if (Boolean.parseBoolean(address.getProxyMethod()) && "HttpProxy".equals(request.getDestinationDataAddress().getType())) {
	return Optional.ofNullable(request.getProperties().get(METHOD))
	.orElseThrow(() -> new EdcException(format("DataFlowRequest %s: 'method' property is missing", request.getId())));
	}
	return Optional.ofNullable(address.getMethod()).orElse(DEFAULT_METHOD);
	}

but it turns out it also gets called for the DestinationDataAddress:

Connector/extensions/data-plane/data-plane-http/src/main/java/org/eclipse/edc/connector/dataplane/http/params/HttpRequestParamsProviderImpl.java

Lines 54 to 68 in cf629c9

	@Override
	public HttpRequestParams provideSourceParams(DataFlowStartMessage request) {
	var params = HttpRequestParams.Builder.newInstance();
	var address = HttpDataAddress.Builder.newInstance().copyFrom(request.getSourceDataAddress()).build();
	sourceDecorators.forEach(decorator -> decorator.decorate(request, address, params));
	return params.build();
	}
	@Override
	public HttpRequestParams provideSinkParams(DataFlowStartMessage request) {
	var params = HttpRequestParams.Builder.newInstance();
	var address = HttpDataAddress.Builder.newInstance().copyFrom(request.getDestinationDataAddress()).build();
	sinkDecorators.forEach(decorator -> decorator.decorate(request, address, params));
	return params.build();
	}

So indeed the extractMethod returns the method specified by the DestinationDataAddress.

Did I understood this correctly?

Thanks for fixing this!