### Motivation:
To ensure NIOTLS concurrency safety.
### Modifications:
* Enable strict concurrency checking in the package manifest.
* Require `NegotiationResult` to be `Sendable`
* In `NIOTypedApplicationProtocolNegotiationHandler<NegotiationResult>`
the
result is used to fulfil promises and perform hops so we need it to be
sendable when accessed from these other concurrency domains.
### Result:
Builds will warn and CI will fail if regressions are introduced.
### 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
* Apply formatting
* Apply no block comments rule
* Apply OmitExplicitReturns
* Apple OnlyOneTrailingClosureArgument
* Apply NoAssignmentInExpressions
* Fix up DontRepeatTypeInStaticProperties lint errors
* Apply `OrderedImports`
* Apply `ReplaceForEachWithForLoop`
* format file
* Enable the formatting pipeline
* Adopt `AmbiguousTrailingClosureOverload`
* Fix license header
* Fix format check
* Fix `EndOfLineComment`
* Fix CI
* Adapt CI script to check if changes when running formatting
* Separate lint and format into to steps
* Fix format
* Adopt `UseEarlyExits`
* Revert "Adopt `UseEarlyExits`"
This reverts commit d1ac5bbe12.
# Motivation
Over the past months, we have been working on new async bridges to make using NIO's `Channel` from Swift Concurrency possible. Since this work was far reaching we have opted to land all of it as SPI. Now the time has come and we feel confident enough to make the SPI official API. This comes after testing the new APIs in various scenarios such as HTTP 1&2, HTTP upgrades, protocol negotiation and in benchmarks.
# Modification
This PR removes the SPI from the `NIOAsyncChannel`, the bootstrap methods, protocol negotiation and HTTP upgrade.
# Result
Everyone can use the our new APIs🚀
# Motivation
After playing around more with the new async bootstrap methods, I came to the conclusion that the `NIOProtocolNegotiationResult` isn't carrying its weight. The `NIOProtocolNegotiationResult` is a glorified `EventLoopFuture` with an easy way to recursively resolve nested futures. Furthermore, it forces any nested protocol negotiation to use the same generic type for the end result.
# Modification
This PR removes the `NIOProtocolNegotiationResult` and changes all the tests to be solely based on `EventLoopFuture`s.
# Result
Less code to support the async bootstrap and better composition of nested protocol negotiation handlers.
# Motivation
When we created the first round of new async bootstrap APIs we added a `ProtocolNegotiationHandler` protocol to identify handlers that are doing protocol negotiation. Since then, we have changed the way how the async bootstraps work and we no longer need this protocol.
# Modification
Remove the `ProtocolNegotiationHandler` protocol
# Motivation
Fixes https://github.com/apple/swift-nio/issues/2494
# Modification
This PR avoids using `deinit` to fulfil the protocol negotiation promise and opts to trap instead when it is being accessed before the handler is added. This allows us to use `handlerAdded` and `handlerRemoved`.
# Result
No more `deinit` usage that can be observed.
# Motivation
We sometimes leaked a promise in the `NIOTypedApplicationProtocolNegotiationHandler` when the handler was created and immediately deinited.
# Modification
This PR makes sure we always complete the negotiation promise of the handler.
# Result
No more leaked promises.
# 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:
In Swift, writing
var something: T?
init() {
self.something = someValue
}
means that the compiler will first set `self.something` to `nil` and
then in the init override it with `self.someValue`
(https://bugs.swift.org/browse/SR-11777). Unfortunately, because of
https://bugs.swift.org/browse/SR-11768 , stored property initialisation
cannot be made `@inlinable` (short of using `@frozen` which isn't
available in Swift 5.0).
The combination of SR-11768 and SR-11777 leads to `var something: T?`
having much worse code than `var something: Optional<T>` iff the `init`
is `public` and `@inlinable`.
Modifications:
Change all `var something: T?` to `var something: Optional<T>`
Result:
Faster code, sad NIO developers.
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:
Previously B2MDs didn't really have defined semantics regarding EOFs and
we didn't tell them if there was an EOF. Also `decodeLast` was optional
and all that was bad.
Modifications:
- require `decodeLast`
- add a `seenEOF: Bool` parameter to `decodeLast` which tells the
decoder if an EOF has been seen
Result:
- clearer semantics
- more information
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:
In Swift, abbreviations use the same case for all letters, therefore it
should be `SNI` and not `Sni`.
Modifications:
changes `Sni` to `SNI`
Result:
more consistent with naming guidelines
Motivation:
Explain here the context, and why you're making that change.
What is the problem you're trying to solve.
Modifications:
Describe the modifications you've done.
Result:
After your change, what will change.
Motivation:
NIO2 development starts now.
Modifications:
Made NIO Swift 5-only for everything else see docs/public-api-changes-NIO1-to-NIO2.md
Result:
NIO2 development can start.
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:
It's generally good style to explain why something needs to be force
unwrapped/tried if not obvious.
Modifications:
Add explanations to a bunch of places.
Result:
Code easier to understand.
Motivation:
Unsafe(Mutable)RawBufferPointers have a few APIs that make our code
easier to reason about. In this PR I'm trying to introduce some of it.
Modifications:
Improved UnsafeRawBufferPointer usage
Result:
code easier to reason about which hopefully leads to fewer bugs. Also:
In many cases we now get free bounds checking in debug mode, yay :).
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.
Motivation:
We used a bool to signal if we should continue decoding or not. Using an enum is more self-documenting.
Modifications:
Add DecodingState and use.
Result:
Code is more self-documenting.
Motivation:
The SNI handler used Foundation for something that can be done fairly
easily without. Also avoids going through Data.
Modifications:
Removed Foundation method and implemented directly on Swift stdlib's
String encoding methods.
Result:
one less Foundation import
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