### Motivation:
Opening the `swift-nio` repository made me warning blind because there
were always so many trivially fixable warnings about things that were
correct but cannot be understood by the compiler.
### Modifications:
Fix all the sendable warnings that popped up, except for one test where
`NIOLockedValueBox<Thread?>` isn't sendable because `Foundation.Thread`
seemingly isn't `Sendable` which is odd. Guessing that'll be fixed on
their end.
### Result:
- Fewer warnings
- Less warning-blindness
- More checks
Motivation:
RemovableChannelHandlers have a large API surface in NIOCore. That API
surface is a bit awkward with regard to strict concurrency, and needs
some cleanup.
Modifications:
This patch adds some new API that is necessary to safely work with
RemovableChannelHandlers, deprecates some API that cannot plausibly be
used, and cleans up some other parts of the API.
Result:
Easier to work with RemovableChannelHandlers
# Motivation
When adding new async methods for all the bootstrap we had to create 3 sets of method for every bootstrap. This resulted in lots of code and only provided little benefit for users.
# Modification
This PR removes all bind/connect methods except the most generic ones. This leaves it up to the user to wrap their channels in a `NIOAsyncChannel` or to await the protocol negotiation result.
# Result
Less code to maintain on our side.
* Add `AsyncChannel` based `ServerBootstrap.bind()` methods
# Motivation
In my previous PR, we added a new async bridge from a NIO `Channel` to Swift Concurrency primitives in the from of the `NIOAsyncChannel`. This type alone is already helpful in bridging `Channel`s to Concurrency; however, it is hard to use since it requires to wrap the `Channel` at the right time otherwise we will drop reads. Furthermore, in the case of protocol negotiation this becomes even trickier since we need to wait until it finishes and then wrap the `Channel`.
# Modification
This PR introduces a few things:
1. New methods on the `ServerBootstrap` which allow the creation of `NIOAsyncChannel` based channels. This can be used in all cases where no protocol negotiation is involved.
2. A new protocol and type called `NIOProtocolNegotiationHandler` and `NIOProtocolNegotiationResult` which is used to identify channel handlers that are doing protocol negotiation.
3. New methods on the `ServerBootstrap` that are aware of protocol negotiation.
# Result
We can now easily and safely create new `AsyncChannel`s from the `ServerBootstrap`
* Code review
* Fix typo
* Fix up tests
* Stop finishing the writer when an error is caught
* Code review
* Fix up writer tests
* Introduce shared protocol negotiation handler state machine
* Correctly handle multi threaded event loops
* Adapt test to assert the channel was closed correctly.
* Code review
# Motivation
I spotted a bug in the ALPNHandler where it doesn't properly unbuffer reentrant reads. This can lead to dropped reads.
# Modification
Instead of buffering into an array we are now buffering into a Deque and unbuffer as long as there are reads in the Deque.
# Result
No more dropped reads.
Motivation:
As we've largely completed our move to split out our core abstractions,
we now have an opportunity to clean up our dependencies and imports. We
should arrange for everything to only import NIO if it actually needs
it, and to correctly express dependencies on NIOCore and NIOEmbedded
where they exist.
We aren't yet splitting out tests that only test functionality in
NIOCore, that will follow in a separate patch.
Modifications:
- Fixed up imports
- Made sure our protocols only require NIOCore.
Result:
Better expression of dependencies.
Co-authored-by: George Barnett <gbarnett@apple.com>
Motivation:
Currently ApplicationProtocolNegotiationHandler accepts a closure that takes only one argument, the ALPN result. This forces the user to capture the Channel in the closure so that it can be mutated.
Modifications:
Add a new init which takes a closure that has both the result and the Channel as parameters. Modify the original init to call the new init (wrapping the passed in closure.)
Result:
New APNL init available that takes a 2 parameter closure. Original APNL init will now be a convenience init.
Motivation:
Spotted a couple of issues with the documentation and fixed them.
Modifications:
- explicitly marked all `public class`es as `public final class` to get
consistency in the Jazzy documentation.
- removed `public` extension methods of the `internal struct
PriorityQueue` which Jazzy showed in the docs.
- improved the largely missing `EmbeddedChannel` and `EmbeddedEventLoop`
documentation.
- clear up lies in the B2MD docs
- add some other missing docs
Result:
better docs makes happier users
Motivation:
`ctx` was always an abbreviation was 'context` and in Swift we don't
really use abbreviations, so let's fix it.
Modifications:
- rename all instances of `ctx` to `context`
Result:
- fixes#483
Motivation:
- `ChannelPipeline.add(name:handler:...)` had a strange order of arguments
- `remove(handler:)` and `remove(ctx:)` both remove `ChannelHandler`s
but they read like they remove different things
So let's just fix the argument order and name them `addHandler` and
`removeHandler` making clear what they do.
Modifications:
- rename all `ChannelPipeline.add(name:handler:...)`s to `ChannelPipeline.addHandler(_:name:...)`
- rename all `ChannelPipeline.remove(...)`s to `ChannelPipeline.removeHandler(...)`
Result:
more readable and consistent code
Motivation:
If ChannelHandler removal worked correctly, it was often either by
accident or by intricate knowledge about the implementation of the
ChannelHandler that is to be removed. Especially when it comes to
re-entrancy it mostly didn't work correctly.
Modifications:
- introduce a `RemovableChannelHandler` API
- raise allocation limit per HTTP connection by 1
(https://bugs.swift.org/browse/SR-9905)
Result:
Make things work by contruction rather than accident
Motivation:
No code is the best code, let's have the compiler generate more
Equatable instances for us.
Modifications:
remove some hand-written Equatable conformances
Result:
less code, possibly fewer bugs
Motivation:
Now that the stdlib has introduced the Result type, we can use it in the
implementation (and the whenComplete) function of EventLoopFuture
Modifications:
- replace EventLoopValue with Result
- make whenComplete provide the Result
Result:
use the new shiny stuff
Motivation:
_ = expression() is ugly and in many cases unimportant. In fact we train
ourselves to overread it which makes special cases where you'd actually
expect a result to be used just look normal.
Modifications:
remove _ = from a lot of places. In many cases we already had a better
(and sometimes cheaper way) to not return a value but in some cases
(mostly `remove` functions) I added `@discardableResult`
Result:
NIO source code looks nicer
Motivation:
There are quite a few `switch`es that cover just two cases. Explicitly state this via an `if case` + `else` structure.
Modifications:
Changes something like:
switch a {
case .b:
return c
default:
return d
}
to
if case .b = a {
return c
} else {
return d
}
Result:
This should prevent misinterpretation and thus potential bugs in the future.
Motivation:
This changes two things:
- it removes the negation of non-used associated types (the removal of `(_)`).
- it prefers exhausting `switch`es over `default`ing.
This change does not impact bahavior; it is pure refactoring.
Modifications:
Omitting needless `(_)`'s and explicitly exhausting switches.
Result:
This change is pure refactoring.
to not return a value
Motivation:
We recently had a bug where we had `EventLoopFuture<EventLoopFuture<()>>` which didn't make any sense. The compiler couldn't catch that problem because we just ignored a closure's argument like this:
future.then { _ in
...
}
which is dangerous. For closures that take an empty tuple, the `_ in`
isn't actually required and the others should state the type they want
to ignore.
And most whenComplete calls can be better (and often shorter) expressed
by other combinators.
Modifications:
remove pretty much all closures which just blanket ignore their
parameter.
Result:
- no closures which just ignore their parameter without at least stating
its type.
- rewrote all whenCompletes that actually used the value
Motivation:
Lots of our most important operations had redundant labels like
func write(data: NIOAny)
the `data: ` label doesn't add anything meaningful and therefore it
should be removed.
Modifications:
removed lots of redundant labels
Result:
less redundant labels
Johannes Weiss
Cory Benfield
Franz Busch
David Nadoba
Jake
Bas Broek