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

Added support for connection viability updates #207

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

Cartisim
Copy link
Contributor

@Cartisim Cartisim commented Sep 9, 2024

Enhancement: Integration of Viability Handler in Network.framework

We have successfully integrated the viabilityUpdateHandler from Network.framework into NIOTransportServices, enabling developers to leverage the full potential of the Network.framework API.

This enhancement provides adopters with critical insights into the viability of connections to remote endpoints. By being informed about the current network status, developers can implement a more responsive and adaptive approach to network behavior and conditions.

Modifications

  • Introduced two new methods along with corresponding calls to these methods within the Network.framework handler.
  • These handlers trigger the NIO InboundEventsTriggered method, allowing consumers to respond to network changes effectively.
  • Developed two structs within NIOTSNetworkEvents to accompany the InboundEventsTriggered methods, ensuring seamless data transfer.
  • Conformed these structs to Sendability for compatibility with the required operating systems.

Result

Clients can now receive viability events from Network.framework, enhancing their ability to manage network connections dynamically

Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, it seems like a useful addition.

Presumably we get a notification when a connection is established? If so it means we should be able to write a test that verifies we actually get the notification. Do you think you'd be able to add one? It looks like NIOTSEndToEndTests.swift would be a good place to start.

Sources/NIOTransportServices/NIOTSNetworkEvents.swift Outdated Show resolved Hide resolved
… test to prove that we are receiving viability updates
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Left a couple of small comments but this looks great otherwise

Sources/NIOTransportServices/NIOTSNetworkEvents.swift Outdated Show resolved Hide resolved
Tests/NIOTransportServicesTests/NIOTSEndToEndTests.swift Outdated Show resolved Hide resolved
Tests/NIOTransportServicesTests/NIOTSEndToEndTests.swift Outdated Show resolved Hide resolved
Tests/NIOTransportServicesTests/NIOTSEndToEndTests.swift Outdated Show resolved Hide resolved
@glbrntt glbrntt added the 🆕 semver/minor Adds new public API. label Sep 11, 2024
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

This looks good to me, thank you! @Lukasa did you want to look as well?

@glbrntt
Copy link
Contributor

glbrntt commented Sep 13, 2024

@swift-server-bot test this please

Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Nice one, thanks @Cartisim!

@Lukasa Lukasa merged commit 99d28e0 into apple:main Sep 16, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants