[CharlesDuboisSAP]

Pull Request Description 🤖

This PR addresses the backlog item #320 and introduces a new feature to the HttpClientWrapper class in the connectivity-apache-httpclient4 module.

Changes

The changes in this PR include:

• A new block of code in the HttpClientWrapper constructor that checks if the destination is configured correctly. If the destination is not configured correctly, it throws a DestinationAccessException with an appropriate error message.

The new block of code checks if the destination's PROXY_TYPE is ON_PREMISE and if the PROXY_HOST is empty. If these conditions are met, it further checks if the TENANT_ID is empty (indicating a provider) and if the AUTH_TYPE (or AUTH_TYPE_FALLBACK) is BASIC_AUTHENTICATION. If these conditions are met, it throws a DestinationAccessException with the message "Accessing an onpremise system on behalf of the provider tenant explicitly is currently not supported." Otherwise, it throws a DestinationAccessException with the message "Unable to resolve connectivity service binding."

Context

This change is part of the effort to migrate the DwC Spring E2E Test, as described in the backlog item. The new feature ensures that the destination is configured correctly before attempting to establish a connection, which helps prevent errors during runtime.

Reviewer Instructions

Please review the changes in the context of the associated backlog item. Pay particular attention to the new error handling code and ensure that it aligns with the expected behavior described in the backlog item.

[Johannes-Schneider]

I do have some concerns about the suggested solution (see comments).

Furthermore, personally, I think it's a pretty big design flaw to not have the two concerns of (1) I want to connect to a remote system and (2) I want to see how my destination in the Destination Service is configured (i.e. I just need the properties) separated.
With this flaw existing, I don't see a good solution to the problem we are facing.
The only somewhat viable solutions seems to be to store the original exception within the Destination object so that it can be thrown at a later point in time, which is pretty hacky and ugly overall IMO.

[CharlesDuboisSAP]

I agree, the two different use cases (reading properties and using the destination) for the same class is a design flaw. But with the timeframe that we have to release v4, I would accept it as tech debt and have usable exceptions messages.
@Johannes-Schneider Do you think the latest commit is better than storing the exception (or the try)?

comments applied