Skip to content

Commit

Permalink
Fix numerous swift warnings (#251)
Browse files Browse the repository at this point in the history
* Fix numerous swift warnings

- Fix incorrect return value for  BufferList.count
- Fix availability check for FileManager.temporaryDirectory
- Better BufferList.fill() implementation from @weissi
  • Loading branch information
dannys42 authored Apr 9, 2021
1 parent d870257 commit b1c10f7
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 40 deletions.
37 changes: 30 additions & 7 deletions Sources/KituraNet/BufferList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public class BufferList {
Get the number of bytes stored in the `BufferList`.
*/
public var count: Int {
return byteBuffer.capacity
return byteBuffer.readableBytes
}

/**
Expand Down Expand Up @@ -147,7 +147,19 @@ public class BufferList {
*/
public func fill(array: inout [UInt8]) -> Int {
return fill(buffer: UnsafeMutablePointer(mutating: array), length: array.count)
let bytesToFill = min(array.count, self.byteBuffer.readableBytes)

// Get a `ByteBufferView` (a `Collection`) of all readable bytes.
let readableByteView = self.byteBuffer.readableBytesView

// We want to overwrite the first `bytesToFill` bytes with the first `bytesToFill` bytes from the `ByteBufferView`.
array.replaceSubrange(0 ..< bytesToFill,
with: readableByteView.prefix(bytesToFill))

// And then move the reader index to indicate that we consumed the bytes.
self.byteBuffer.moveReaderIndex(forwardBy: bytesToFill)

return bytesToFill
}

/**
Expand All @@ -165,11 +177,22 @@ public class BufferList {
*/
public func fill(buffer: UnsafeMutablePointer<UInt8>, length: Int) -> Int {
let fillLength = min(length, byteBuffer.readableBytes)
return byteBuffer.readWithUnsafeReadableBytes { bytes in
UnsafeMutableRawPointer(buffer).copyMemory(from: bytes.baseAddress!, byteCount: fillLength)
return fillLength
}
// NOTE: This API should be deprecated and expressed as either `UnsafeMutableBufferPointer<UInt8>` or
// preferrable `UnsafeMutableRawBufferPointer` or even better: replaced with nothing.

let bytesToFill = min(length, self.byteBuffer.readableBytes)

// Get a `ByteBufferView` (a `Collection`) of all readable bytes.
let readableByteView = self.byteBuffer.readableBytesView

// We want to overwrite the first `bytesToFill` bytes with the first `bytesToFill` bytes from the `ByteBufferView`.
_ = UnsafeMutableBufferPointer(start: buffer, count: bytesToFill)
.initialize(from: readableByteView.prefix(bytesToFill))

// And then move the reader index to indicate that we consumed the bytes.
self.byteBuffer.moveReaderIndex(forwardBy: bytesToFill)

return bytesToFill
}

/**
Expand Down
6 changes: 3 additions & 3 deletions Sources/KituraNet/HTTP/HTTPRequestHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ internal class HTTPRequestHandler: ChannelInboundHandler, RemovableChannelHandle
} catch {
Log.error("Failed to send error response")
}
context.close()
context.close(promise: nil)
}
}
serverRequest = HTTPServerRequest(channel: context.channel, requestHead: header, enableSSL: enableSSLVerification)
Expand Down Expand Up @@ -125,7 +125,7 @@ internal class HTTPRequestHandler: ChannelInboundHandler, RemovableChannelHandle

case .end:
requestSize = 0
server.connectionCount.add(1)
_ = server.connectionCount.add(1)
if let connectionLimit = server.options.connectionLimit {
if server.connectionCount.load() > connectionLimit {
do {
Expand Down Expand Up @@ -201,6 +201,6 @@ internal class HTTPRequestHandler: ChannelInboundHandler, RemovableChannelHandle
}

func channelInactive(context: ChannelHandlerContext) {
server.connectionCount.sub(1)
_ = server.connectionCount.sub(1)
}
}
16 changes: 6 additions & 10 deletions Sources/KituraNet/HTTP/HTTPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public class HTTPServer: Server {
public var options: ServerOptions = ServerOptions()

//counter for no of connections
var connectionCount = Atomic(value: 0)
var connectionCount = NIOAtomic.makeAtomic(value: 0)

/**
Creates an HTTP server object.
Expand Down Expand Up @@ -259,11 +259,7 @@ public class HTTPServer: Server {

private func createNIOSSLServerHandler() -> NIOSSLServerHandler? {
if let sslContext = self.sslContext {
do {
return try NIOSSLServerHandler(context: sslContext)
} catch let error {
Log.error("Failed to create NIOSSLServerHandler. Error: \(error)")
}
return NIOSSLServerHandler(context: sslContext)
}
return nil
}
Expand Down Expand Up @@ -347,7 +343,7 @@ public class HTTPServer: Server {
}

let bootstrap = ServerBootstrap(group: eventLoopGroup)
.serverChannelOption(ChannelOptions.backlog, value: BacklogOption.Value(self.maxPendingConnections))
.serverChannelOption(ChannelOptions.backlog, value: .init(self.maxPendingConnections))
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.serverChannelOption(ChannelOptions.socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEPORT), value: allowPortReuse ? 1 : 0)
.serverChannelInitializer { channel in
Expand All @@ -357,7 +353,7 @@ public class HTTPServer: Server {
}
.childChannelInitializer { channel in
let httpHandler = HTTPRequestHandler(for: self)
let config: HTTPUpgradeConfiguration = (upgraders: upgraders, completionHandler: {_ in
let config: NIOHTTPServerUpgradeConfiguration = (upgraders: upgraders, completionHandler: {_ in
_ = channel.pipeline.removeHandler(httpHandler)
})
return channel.pipeline.configureHTTPServerPipeline(withServerUpgrade: config, withErrorHandling: true).flatMap {
Expand Down Expand Up @@ -392,8 +388,8 @@ public class HTTPServer: Server {
self.state = .failed
self.lifecycleListener.performFailCallbacks(with: error)
switch socket {
case .tcp(let port):
Log.error("Error trying to bind to \(port): \(error)")
case .tcp(let port, let address):
Log.error("Error trying to bind to \(address ?? "")\(port): \(error)")
case .unix(let socketPath):
Log.error("Error trying to bind to \(socketPath): \(error)")
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/KituraNet/Server/ServerDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

/// The protocol defining the delegate for the HTTPServer and the FastCGIServer classes.
/// The delegate's handle function is invoked when new requests arrive at the server for processing.
public protocol ServerDelegate: class {
public protocol ServerDelegate: AnyObject {
/// Handle new incoming requests to the server
///
/// - Parameter request: The ServerRequest class instance for working with this request.
Expand Down
2 changes: 1 addition & 1 deletion Sources/KituraNet/ServerRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import Foundation
/// The ServerRequest protocol allows requests to be abstracted
/// across different networking protocols in an agnostic way to the
/// Kitura project Router.
public protocol ServerRequest: class {
public protocol ServerRequest: AnyObject {

/// The set of headers received with the incoming request
var headers: HeadersContainer { get }
Expand Down
2 changes: 1 addition & 1 deletion Sources/KituraNet/ServerResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import Foundation
/// The ServerResponse protocol allows responses to be abstracted
/// across different networking protocols in an agnostic way to the
/// Kitura project Router.
public protocol ServerResponse: class {
public protocol ServerResponse: AnyObject {

/// The status code to send in the HTTP response.
var statusCode: HTTPStatusCode? { get set }
Expand Down
28 changes: 18 additions & 10 deletions Tests/KituraNetTests/BufferListTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ class BufferListTests: XCTestCase {

func testAppendUnsafePointerLength() {
let array: [UInt8] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
let pointer: UnsafeMutablePointer<UInt8> = UnsafeMutablePointer(mutating: array)
bufferList.append(bytes: pointer, length: 10)
bufferList.reset()
array.withUnsafeBufferPointer { bufferPointer in
bufferList.append(bytes: bufferPointer.baseAddress!, length: 10)
bufferList.reset()
}
}

func testAppendData() {
Expand Down Expand Up @@ -73,13 +74,20 @@ class BufferListTests: XCTestCase {
}

func testFillUnsafeMutablePointer() {
let data = Data(repeating: 1, count: 512)
let arrayValuesToFill = 64
let valueToFill: UInt8 = 3
let expectedValue = Int(valueToFill) * arrayValuesToFill

let data = Data(repeating: valueToFill, count: 512)
bufferList.append(data: data)
let array = [UInt8](repeating: 0, count: 64)
let pointer = UnsafeMutablePointer(mutating: array)
let result = bufferList.fill(buffer: pointer, length: 64)
XCTAssertEqual(result, 64)
XCTAssertEqual(Array(UnsafeBufferPointer(start: pointer, count: 64)).reduce(0) { Int($0) + Int($1) }, 64)
var array = [UInt8](repeating: 0, count: arrayValuesToFill)
array.withUnsafeMutableBufferPointer { bufferPointer in
var bytesFilled: Int
bytesFilled = bufferList.fill(buffer: bufferPointer.baseAddress!, length: 64)
XCTAssertEqual(bytesFilled, 64)
}
let resolvedValue = array.reduce(0) { Int($0) + Int($1) }
XCTAssertEqual(resolvedValue, expectedValue)
bufferList.reset()
}

Expand All @@ -100,7 +108,7 @@ class BufferListTests: XCTestCase {
let data = Data(repeating: 1, count: 64)
bufferList.append(data: data)
XCTAssertEqual(bufferList.data.count, 64)
XCTAssertEqual(bufferList.count, 4096)
XCTAssertEqual(bufferList.count, 64)
bufferList.reset()
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/KituraNetTests/ConnectionLimitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ class ConnectionLimitTests: KituraNetTest {
let payload = "Hello, World!"
var payloadBuffer = ByteBufferAllocator().buffer(capacity: 1024)
payloadBuffer.writeString(payload)
_ = self.establishConnection(expectation: expectation, responseHandler: HTTPResponseHandler(expectedStatus:HTTPResponseStatus.ok, expectation: expectation))
self.establishConnection(expectation: expectation, responseHandler: HTTPResponseHandler(expectedStatus:HTTPResponseStatus.ok, expectation: expectation))
}, { expectation in
let payload = "Hello, World!"
var payloadBuffer = ByteBufferAllocator().buffer(capacity: 1024)
payloadBuffer.writeString(payload)
_ = self.establishConnection(expectation: expectation, responseHandler: HTTPResponseHandler(expectedStatus:HTTPResponseStatus.serviceUnavailable, expectation: expectation))
self.establishConnection(expectation: expectation, responseHandler: HTTPResponseHandler(expectedStatus:HTTPResponseStatus.serviceUnavailable, expectation: expectation))
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/KituraNetTests/HTTPResponseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class HTTPResponseTests: KituraNetTest {
let headers = HeadersContainer()

headers.append("Content-Type", value: "text/html")
var values = headers["Content-Type"]
let values = headers["Content-Type"]
XCTAssertNotNil(values, "Couldn't retrieve just set Content-Type header")
XCTAssertEqual(values?.count, 1, "Content-Type header should only have one value")
XCTAssertEqual(values?[0], "text/html")
Expand Down
4 changes: 1 addition & 3 deletions Tests/KituraNetTests/KituraNIOTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class KituraNetTest: XCTestCase {
let temporaryDirectory = "/tmp"
#else
var temporaryDirectory: String
if #available(OSX 10.12, *) {
if #available(macOS 10.12, iOS 10.0, *) {
temporaryDirectory = FileManager.default.temporaryDirectory.path
} else {
temporaryDirectory = "/tmp"
Expand Down Expand Up @@ -141,7 +141,6 @@ class KituraNetTest: XCTestCase {

func performServerTestWithUnixSocket(serverConfig: ServerOptions = ServerOptions(), delegate: ServerDelegate?, useSSL: Bool = useSSLDefault, allowPortReuse: Bool = portReuseDefault, line: Int = #line, asyncTasks: [(XCTestExpectation) -> Void]) {
do {
var serverConfig = serverConfig
var server: HTTPServer
self.useSSL = useSSL
self.unixDomainSocketPath = self.socketFilePath
Expand Down Expand Up @@ -169,7 +168,6 @@ class KituraNetTest: XCTestCase {

func performServerTestWithTCPPort(serverConfig: ServerOptions = ServerOptions(), delegate: ServerDelegate?, useSSL: Bool = useSSLDefault, allowPortReuse: Bool = portReuseDefault, line: Int = #line, asyncTasks: [(XCTestExpectation) -> Void]) {
do {
var serverConfig = serverConfig
var server: HTTPServer
var ephemeralPort: Int = 0
self.useSSL = useSSL
Expand Down
2 changes: 1 addition & 1 deletion Tests/KituraNetTests/PipeliningTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import LoggerAPI
@testable import KituraNet

func randomNumber(limit: Int) -> Int {
#if os(OSX)
#if os(OSX) || os(macOS) || os(iOS) || os(tvOS)
return Int(arc4random_uniform(UInt32(limit)))
#else
let random: Int = Int(rand())
Expand Down

0 comments on commit b1c10f7

Please sign in to comment.