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 to not send the same element through multiple objects #3282

Merged
merged 4 commits into from
Apr 18, 2023

Conversation

abhipsaMisra
Copy link
Member

The transport settings are available through the PipelineContext, but were being set on the TransportHandler explicitly again.

@@ -60,6 +60,7 @@ public abstract class IotHubBaseClient : IAsyncDisposable
ModelId = _clientOptions.ModelId,
PayloadConvention = _clientOptions.PayloadConvention,
IotHubClientTransportSettings = _clientOptions.TransportSettings,
FileUploadTransportSettings = _clientOptions.FileUploadTransportSettings,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is in preparation for adding file upload into the delegating handler pipeline

if (customCertificateValidation != null)
{
httpClientHandler = new HttpClientHandler
{
ServerCertificateCustomValidationCallback = customCertificateValidation,
SslProtocols = transportSettings.SslProtocols,
SslProtocols = SslProtocols.None,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the default value set on our transport settings.

@@ -197,11 +197,11 @@ internal MqttTransportHandler(PipelineContext context, IotHubClientMqttSettings
string uri = $"wss://{_hostName}/$iothub/websocket";
_mqttClientOptionsBuilder.WithWebSocketServer(uri);

IWebProxy proxy = _transportSettings.Proxy;
IWebProxy proxy = _mqttTransportSettings.Proxy;
Copy link
Member Author

Choose a reason for hiding this comment

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

Retrieve the required values from the transport settings passed in through the context

@@ -28,13 +28,12 @@ internal static MqttTransportHandler CreateTransportHandler(IMqttClient mockMqtt
{
var pipelineContext = new PipelineContext();
var clientConfigurationMock = new IotHubConnectionCredentials();
//clientConfigurationMock.SetupGet(x => x.ClientOptions).Returns(clientOptionsMock.Object);
Copy link
Member Author

Choose a reason for hiding this comment

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

This had been commented out, so I removed it.

@@ -38,6 +38,8 @@ protected set
}
}

public virtual bool IsUsable => NextHandler?.IsUsable ?? true;
Copy link
Member Author

Choose a reason for hiding this comment

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

Property before methods

@abhipsaMisra
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

};

using var httpTransport = new HttpTransportHandler(pipelineContext, transportSettings, httpClientHandler);
using var httpTransport = new HttpTransportHandler(pipelineContext, httpClientHandler);
Copy link
Member Author

Choose a reason for hiding this comment

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

This should hopefully be refactored out to not require a separate pipeline context instance once we have integrated http into the pipeline flow. (This might be similar to the file upload flow we have on the device client).

@abhipsaMisra
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@abhipsaMisra abhipsaMisra merged commit 86f50c3 into previews/v2 Apr 18, 2023
@abhipsaMisra abhipsaMisra deleted the abmisr/transport2 branch April 18, 2023 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants