From 6b4dfd578f9758ac1ce79cc5e53e3f1c879d4439 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 2 Jul 2017 14:14:15 -0400 Subject: [PATCH 1/7] Use guard --- Source/SocketPacket.swift | 59 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/Source/SocketPacket.swift b/Source/SocketPacket.swift index d88ef4c..9ea4656 100644 --- a/Source/SocketPacket.swift +++ b/Source/SocketPacket.swift @@ -29,15 +29,15 @@ struct SocketPacket { enum PacketType: Int { case connect, disconnect, event, ack, error, binaryEvent, binaryAck } - + private let placeholders: Int - + private static let logType = "SocketPacket" let nsp: String let id: Int let type: PacketType - + var binary: [Data] var data: [Any] var args: [Any] { @@ -47,20 +47,20 @@ struct SocketPacket { return data } } - + var description: String { return "SocketPacket {type: \(String(type.rawValue)); data: " + "\(String(describing: data)); id: \(id); placeholders: \(placeholders); nsp: \(nsp)}" } - + var event: String { return String(describing: data[0]) } - + var packetString: String { return createPacketString() } - + init(type: PacketType, data: [Any] = [Any](), id: Int = -1, nsp: String, placeholders: Int = 0, binary: [Data] = [Data]()) { self.data = data @@ -70,14 +70,14 @@ struct SocketPacket { self.placeholders = placeholders self.binary = binary } - + mutating func addData(_ data: Data) -> Bool { if placeholders == binary.count { return true } - + binary.append(data) - + if placeholders == binary.count { fillInPlaceholders() return true @@ -85,22 +85,19 @@ struct SocketPacket { return false } } - + private func completeMessage(_ message: String) -> String { - if data.count == 0 { - return message + "[]" - } - + guard data.count != 0 else { return message + "[]" } guard let jsonSend = try? data.toJSON(), let jsonString = String(data: jsonSend, encoding: .utf8) else { DefaultSocketLogger.Logger.error("Error creating JSON object in SocketPacket.completeMessage", type: SocketPacket.logType) - + return message + "[]" } - + return message + jsonString } - + private func createPacketString() -> String { let typeString = String(type.rawValue) // Binary count? @@ -109,17 +106,17 @@ struct SocketPacket { let nspString = binaryCountString + (nsp != "/" ? "\(nsp)," : "") // Ack number? let idString = nspString + (id != -1 ? String(id) : "") - + return completeMessage(idString) } - + // Called when we have all the binary data for a packet // calls _fillInPlaceholders, which replaces placeholders with the // corresponding binary private mutating func fillInPlaceholders() { data = data.map(_fillInPlaceholders) } - + // Helper method that looks for placeholders // If object is a collection it will recurse // Returns the object if it is not a placeholder or the corresponding @@ -132,9 +129,9 @@ struct SocketPacket { } else { return dict.reduce(JSON(), {cur, keyValue in var cur = cur - + cur[keyValue.0] = _fillInPlaceholders(keyValue.1) - + return cur }) } @@ -161,12 +158,12 @@ extension SocketPacket { return .error } } - + static func packetFromEmit(_ items: [Any], id: Int, nsp: String, ack: Bool) -> SocketPacket { let (parsedData, binary) = deconstructData(items) let packet = SocketPacket(type: findType(binary.count, ack: ack), data: parsedData, id: id, nsp: nsp, binary: binary) - + return packet } } @@ -175,32 +172,32 @@ private extension SocketPacket { // Recursive function that looks for NSData in collections static func shred(_ data: Any, binary: inout [Data]) -> Any { let placeholder = ["_placeholder": true, "num": binary.count] as JSON - + switch data { case let bin as Data: binary.append(bin) - + return placeholder case let arr as [Any]: return arr.map({shred($0, binary: &binary)}) case let dict as JSON: return dict.reduce(JSON(), {cur, keyValue in var mutCur = cur - + mutCur[keyValue.0] = shred(keyValue.1, binary: &binary) - + return mutCur }) default: return data } } - + // Removes binary data from emit data // Returns a type containing the de-binaryed data and the binary static func deconstructData(_ data: [Any]) -> ([Any], [Data]) { var binary = [Data]() - + return (data.map({shred($0, binary: &binary)}), binary) } } From 1a42580826a9f9909b2b3b30c5ed774578932a85 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 4 Jul 2017 08:09:20 -0400 Subject: [PATCH 2/7] Clean up SocketPacket methods a bit --- Source/SocketPacket.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Source/SocketPacket.swift b/Source/SocketPacket.swift index 9ea4656..a8a8bf1 100644 --- a/Source/SocketPacket.swift +++ b/Source/SocketPacket.swift @@ -161,10 +161,9 @@ extension SocketPacket { static func packetFromEmit(_ items: [Any], id: Int, nsp: String, ack: Bool) -> SocketPacket { let (parsedData, binary) = deconstructData(items) - let packet = SocketPacket(type: findType(binary.count, ack: ack), data: parsedData, - id: id, nsp: nsp, binary: binary) - return packet + return SocketPacket(type: findType(binary.count, ack: ack), data: parsedData, id: id, nsp: nsp, + binary: binary) } } @@ -198,6 +197,6 @@ private extension SocketPacket { static func deconstructData(_ data: [Any]) -> ([Any], [Data]) { var binary = [Data]() - return (data.map({shred($0, binary: &binary)}), binary) + return (data.map({ shred($0, binary: &binary) }), binary) } } From 206e1eed4f30eedf32b40882350621e366172949 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 4 Jul 2017 08:29:02 -0400 Subject: [PATCH 3/7] Tell users what namespace was connected to --- Source/SocketIOClient.swift | 4 +-- Source/SocketIOClientSpec.swift | 2 +- Source/SocketParsable.swift | 48 ++++++++++++++++----------------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/Source/SocketIOClient.swift b/Source/SocketIOClient.swift index d2b4b3a..1aba6c1 100644 --- a/Source/SocketIOClient.swift +++ b/Source/SocketIOClient.swift @@ -203,12 +203,12 @@ open class SocketIOClient : NSObject, SocketIOClientSpec, SocketEngineClient, So return OnAckCallback(ackNumber: currentAck, items: items, socket: self) } - func didConnect() { + func didConnect(toNamespace namespace: String) { DefaultSocketLogger.Logger.log("Socket connected", type: logType) status = .connected - handleClientEvent(.connect, data: []) + handleClientEvent(.connect, data: [namespace]) } func didDisconnect(reason: String) { diff --git a/Source/SocketIOClientSpec.swift b/Source/SocketIOClientSpec.swift index b4b99a1..eaf63af 100644 --- a/Source/SocketIOClientSpec.swift +++ b/Source/SocketIOClientSpec.swift @@ -29,7 +29,7 @@ protocol SocketIOClientSpec : class { var nsp: String { get set } var waitingPackets: [SocketPacket] { get set } - func didConnect() + func didConnect(toNamespace namespace: String) func didDisconnect(reason: String) func didError(reason: String) func handleAck(_ ack: Int, data: [Any]) diff --git a/Source/SocketParsable.swift b/Source/SocketParsable.swift index 5c698dc..6b31cc8 100644 --- a/Source/SocketParsable.swift +++ b/Source/SocketParsable.swift @@ -31,15 +31,15 @@ extension SocketParsable where Self: SocketIOClientSpec { private func isCorrectNamespace(_ nsp: String) -> Bool { return nsp == self.nsp } - + private func handleConnect(_ packetNamespace: String) { if packetNamespace == "/" && nsp != "/" { joinNamespace(nsp) } else { - didConnect() + didConnect(toNamespace: packetNamespace) } } - + private func handlePacket(_ pack: SocketPacket) { switch pack.type { case .event where isCorrectNamespace(pack.nsp): @@ -60,22 +60,22 @@ extension SocketParsable where Self: SocketIOClientSpec { DefaultSocketLogger.Logger.log("Got invalid packet: %@", type: "SocketParser", args: pack.description) } } - + /// Parses a messsage from the engine. Returning either a string error or a complete SocketPacket func parseString(_ message: String) -> Either { var reader = SocketStringReader(message: message) - + guard let type = Int(reader.read(count: 1)).flatMap({ SocketPacket.PacketType(rawValue: $0) }) else { return .left("Invalid packet type") } - + if !reader.hasNext { return .right(SocketPacket(type: type, nsp: "/")) } - + var namespace = "/" var placeholders = -1 - + if type == .binaryEvent || type == .binaryAck { if let holders = Int(reader.readUntilOccurence(of: "-")) { placeholders = holders @@ -83,17 +83,17 @@ extension SocketParsable where Self: SocketIOClientSpec { return .left("Invalid packet") } } - + if reader.currentCharacter == "/" { - namespace = reader.readUntilOccurence(of: ",") + namespace = reader.readUntilOccurence(of: ",") } - + if !reader.hasNext { return .right(SocketPacket(type: type, nsp: namespace, placeholders: placeholders)) } - + var idString = "" - + if type == .error { reader.advance(by: -1) } else { @@ -106,13 +106,13 @@ extension SocketParsable where Self: SocketIOClientSpec { } } } - + var dataArray = String(message.utf16[message.utf16.index(reader.currentIndex, offsetBy: 1).. Either { do { @@ -130,13 +130,13 @@ extension SocketParsable where Self: SocketIOClientSpec { return .left("Error parsing data for packet") } } - + // Parses messages recieved func parseSocketMessage(_ message: String) { guard !message.isEmpty else { return } - + DefaultSocketLogger.Logger.log("Parsing %@", type: "SocketParser", args: message) - + switch parseString(message) { case let .left(err): DefaultSocketLogger.Logger.error("\(err): %@", type: "SocketParser", args: message) @@ -145,18 +145,18 @@ extension SocketParsable where Self: SocketIOClientSpec { handlePacket(pack) } } - + func parseBinaryData(_ data: Data) { guard !waitingPackets.isEmpty else { DefaultSocketLogger.Logger.error("Got data when not remaking packet", type: "SocketParser") return } - + // Should execute event? guard waitingPackets[waitingPackets.count - 1].addData(data) else { return } - + let packet = waitingPackets.removeLast() - + if packet.type != .binaryAck { handleEvent(packet.event, data: packet.args, isInternalMessage: false, withAck: packet.id) } else { From 141b0ce6bcf1ba348764687d80ee70b41098ab88 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 4 Jul 2017 08:37:18 -0400 Subject: [PATCH 4/7] Add test for namespace in connect --- SocketIO-MacTests/SocketSideEffectTest.swift | 40 +++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/SocketIO-MacTests/SocketSideEffectTest.swift b/SocketIO-MacTests/SocketSideEffectTest.swift index d1edcb4..2569260 100644 --- a/SocketIO-MacTests/SocketSideEffectTest.swift +++ b/SocketIO-MacTests/SocketSideEffectTest.swift @@ -226,11 +226,11 @@ class SocketSideEffectTest: XCTestCase { socket.setTestStatus(.notConnected) - socket.connect(timeoutAfter: 1, withHandler: { + socket.connect(timeoutAfter: 0.2, withHandler: { expect.fulfill() }) - waitForExpectations(timeout: 2) + waitForExpectations(timeout: 0.4) } func testConnectDoesNotTimeOutIfConnected() { @@ -238,22 +238,52 @@ class SocketSideEffectTest: XCTestCase { socket.setTestStatus(.notConnected) - socket.connect(timeoutAfter: 1, withHandler: { + socket.connect(timeoutAfter: 0.3, withHandler: { XCTFail("Should not call timeout handler if status is connected") }) - DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.2) { + DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.1) { // Fake connecting self.socket.setTestStatus(.connected) } - DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 1.1) { + DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.5) { expect.fulfill() } waitForExpectations(timeout: 2) } + func testConnectIsCalledWithNamepsace() { + let expect = expectation(description: "The client should not call the timeout function") + let nspString = "/swift" + + socket.setTestStatus(.notConnected) + + socket.on(clientEvent: .connect) {data, ack in + guard let nsp = data[0] as? String else { + XCTFail("Connect should be called with a namespace") + + return + } + + XCTAssertEqual(nspString, nsp, "It should connect with the correct namespace") + + expect.fulfill() + } + + socket.connect(timeoutAfter: 0.3, withHandler: { + XCTFail("Should not call timeout handler if status is connected") + }) + + DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.1) { + // Fake connecting + self.socket.parseEngineMessage("0/swift") + } + + waitForExpectations(timeout: 2) + } + func testErrorInCustomSocketDataCallsErrorHandler() { let expect = expectation(description: "The client should call the error handler for emit errors because of " + "custom data") From beaa4b695c6c514ab169a132b3c392b1383d6b97 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 4 Jul 2017 08:46:07 -0400 Subject: [PATCH 5/7] document connect data item --- Source/SocketIOClientSpec.swift | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Source/SocketIOClientSpec.swift b/Source/SocketIOClientSpec.swift index eaf63af..d707fbf 100644 --- a/Source/SocketIOClientSpec.swift +++ b/Source/SocketIOClientSpec.swift @@ -48,7 +48,15 @@ extension SocketIOClientSpec { /// The set of events that are generated by the client. public enum SocketClientEvent : String { - /// Emitted when the client connects. This is also called on a successful reconnection. + /// Emitted when the client connects. This is also called on a successful reconnection. A connect event gets one + /// data item: the namespace that was connected to. + /// + /// ```swift + /// socket.on(clientEvent: .connect) {data, ack in + /// guard let nsp = data[0] as? String else { return } + /// // Some logic using the nsp + /// } + /// ``` case connect /// Called when the socket has disconnected and will not attempt to try to reconnect. From 54c54a23b7a2f95c825b522f42e4117b3f49ea91 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 4 Jul 2017 08:49:18 -0400 Subject: [PATCH 6/7] increase timeouts for travis --- SocketIO-MacTests/SocketSideEffectTest.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/SocketIO-MacTests/SocketSideEffectTest.swift b/SocketIO-MacTests/SocketSideEffectTest.swift index 2569260..d913b82 100644 --- a/SocketIO-MacTests/SocketSideEffectTest.swift +++ b/SocketIO-MacTests/SocketSideEffectTest.swift @@ -226,11 +226,11 @@ class SocketSideEffectTest: XCTestCase { socket.setTestStatus(.notConnected) - socket.connect(timeoutAfter: 0.2, withHandler: { + socket.connect(timeoutAfter: 0.5, withHandler: { expect.fulfill() }) - waitForExpectations(timeout: 0.4) + waitForExpectations(timeout: 0.8) } func testConnectDoesNotTimeOutIfConnected() { @@ -238,7 +238,7 @@ class SocketSideEffectTest: XCTestCase { socket.setTestStatus(.notConnected) - socket.connect(timeoutAfter: 0.3, withHandler: { + socket.connect(timeoutAfter: 0.5, withHandler: { XCTFail("Should not call timeout handler if status is connected") }) @@ -247,7 +247,7 @@ class SocketSideEffectTest: XCTestCase { self.socket.setTestStatus(.connected) } - DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.5) { + DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.7) { expect.fulfill() } From 49961eb6cf700bd6584eaeaed08e1b0faad3ea9e Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 4 Jul 2017 11:08:04 -0400 Subject: [PATCH 7/7] Make timeOut(after:) Take a double for finer control of timeouts --- SocketIO-MacTests/SocketSideEffectTest.swift | 2 +- Source/SocketAckEmitter.swift | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/SocketIO-MacTests/SocketSideEffectTest.swift b/SocketIO-MacTests/SocketSideEffectTest.swift index d913b82..c427cb9 100644 --- a/SocketIO-MacTests/SocketSideEffectTest.swift +++ b/SocketIO-MacTests/SocketSideEffectTest.swift @@ -319,7 +319,7 @@ class SocketSideEffectTest: XCTestCase { expect.fulfill() } - socket.emitWithAck("myEvent", ThrowingData()).timingOut(after: 1, callback: {_ in + socket.emitWithAck("myEvent", ThrowingData()).timingOut(after: 0.8, callback: {_ in XCTFail("Ack callback should not be called") }) diff --git a/Source/SocketAckEmitter.swift b/Source/SocketAckEmitter.swift index cdff4a8..ea82ab0 100644 --- a/Source/SocketAckEmitter.swift +++ b/Source/SocketAckEmitter.swift @@ -104,7 +104,7 @@ public final class OnAckCallback : NSObject { /// - parameter after: The number of seconds before this emit times out if an ack hasn't been received. /// - parameter callback: The callback called when an ack is received, or when a timeout happens. /// To check for timeout, use `SocketAckStatus`'s `noAck` case. - public func timingOut(after seconds: Int, callback: @escaping AckCallback) { + public func timingOut(after seconds: Double, callback: @escaping AckCallback) { guard let socket = self.socket, ackNumber != -1 else { return } socket.ackHandlers.addAck(ackNumber, callback: callback) @@ -112,7 +112,7 @@ public final class OnAckCallback : NSObject { guard seconds != 0 else { return } - socket.handleQueue.asyncAfter(deadline: DispatchTime.now() + Double(seconds)) { + socket.handleQueue.asyncAfter(deadline: DispatchTime.now() + seconds) { socket.ackHandlers.timeoutAck(self.ackNumber, onQueue: socket.handleQueue) } }