Included a fix for duplicate open groups and added unit tests for it

SessionMessagingKit/Open Groups/OpenGroupAPI.swift

@@ -6,6 +6,7 @@ import Curve25519Kit
 public enum OpenGroupAPI {
     // MARK: - Settings
     
+    public static let legacyDefaultServerDNS = "open.getsession.org"
     public static let defaultServer = "http://116.203.70.33"
     public static let defaultServerPublicKey = "a03c383cf63c3c4efe67acc52112a6dd734b3a946b9545f488aaa93da7991238"
 

SessionMessagingKit/Open Groups/OpenGroupManager.swift

@@ -94,11 +94,52 @@ public final class OpenGroupManager: NSObject {
 
     // MARK: - Adding & Removing
     
+    public func hasExistingOpenGroup(roomToken: String, server: String, publicKey: String, using transaction: YapDatabaseReadWriteTransaction, dependencies: OGMDependencies = OGMDependencies()) -> Bool {
+        guard let serverUrl: URL = URL(string: server) else { return false }
+        
+        let schemeFreeServer: String = (serverUrl.host ?? server)
+        let schemeFreeDefaultServer: String = OpenGroupAPI.defaultServer.substring(from: "http://".count)
+        var serverOptions: Set<String> = Set([
+            schemeFreeServer,
+            "http://\(schemeFreeServer)",
+            "https://\(schemeFreeServer)"
+        ])
+        
+        if schemeFreeServer == OpenGroupAPI.legacyDefaultServerDNS {
+            let defaultServerOptions: Set<String> = Set([
+                schemeFreeDefaultServer,
+                OpenGroupAPI.defaultServer,
+                "https://\(schemeFreeDefaultServer)"
+            ])
+            serverOptions = serverOptions.union(defaultServerOptions)
+        }
+        else if schemeFreeServer == schemeFreeDefaultServer {
+            let legacyServerOptions: Set<String> = Set([
+                OpenGroupAPI.legacyDefaultServerDNS,
+                "http://\(OpenGroupAPI.legacyDefaultServerDNS)",
+                "https://\(OpenGroupAPI.legacyDefaultServerDNS)"
+            ])
+            serverOptions = serverOptions.union(legacyServerOptions)
+        }
+        
+        // First check if there is no poller for the specified server
+        if serverOptions.first(where: { dependencies.cache.pollers[$0] != nil }) == nil {
+            return false
+        }
+        
+        // Then check if there is an existing open group thread
+        let hasExistingThread: Bool = serverOptions.contains(where: { serverName in
+            let groupId: Data = LKGroupUtilities.getEncodedOpenGroupIDAsData("\(serverName).\(roomToken)")
+            
+            return (TSGroupThread.fetch(groupId: groupId, transaction: transaction) != nil)
+        })
+                                                                  
+        return hasExistingThread
+    }
+    
     public func add(roomToken: String, server: String, publicKey: String, isConfigMessage: Bool, using transaction: YapDatabaseReadWriteTransaction, dependencies: OGMDependencies = OGMDependencies()) -> Promise<Void> {
         // If we are currently polling for this server and already have a TSGroupThread for this room the do nothing
-        let groupId: Data = LKGroupUtilities.getEncodedOpenGroupIDAsData("\(server).\(roomToken)")
+        if hasExistingOpenGroup(roomToken: roomToken, server: server, publicKey: publicKey, using: transaction, dependencies: dependencies) {
-        
-        if dependencies.cache.pollers[server] != nil && TSGroupThread.fetch(groupId: groupId, transaction: transaction) != nil {
             SNLog("Ignoring join open group attempt (already joined), user initiated: \(!isConfigMessage)")
             return Promise.value(())
         }

SessionMessagingKit/Sending & Receiving/Pollers/OpenGroupPoller.swift

@@ -70,6 +70,7 @@ extension OpenGroupAPI {
                             cache.timeSinceLastPoll[server] = Date().timeIntervalSince1970
                             UserDefaults.standard[.lastOpen] = Date()
                         }
+                        SNLog("Open group polling finished for \(server).")
                         seal.fulfill(())
                     }
                     .catch(on: OpenGroupAPI.workQueue) { [weak self] error in

SessionMessagingKitTests/Open Groups/OpenGroupManagerSpec.swift

@@ -477,6 +477,235 @@ class OpenGroupManagerSpec: QuickSpec {
             
             // MARK: - Adding & Removing
             
+            // MARK: - --hasExistingOpenGroup
+            
+            context("when checking it has an existing open group") {
+                context("when there is a thread for the room and the cache has a poller") {
+                    beforeEach {
+                        testTransaction.mockData[.objectForKey] = testGroupThread
+                    }
+                    
+                    context("for the no-scheme variant") {
+                        beforeEach {
+                            mockOGMCache.when { $0.pollers }.thenReturn(["testServer": OpenGroupAPI.Poller(for: "testServer")])
+                        }
+                        
+                        it("returns true when no scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                        
+                        it("returns true when a http scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "http://testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                        
+                        it("returns true when a https scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "https://testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                    }
+                    
+                    context("for the http variant") {
+                        beforeEach {
+                            mockOGMCache.when { $0.pollers }.thenReturn(["http://testServer": OpenGroupAPI.Poller(for: "http://testServer")])
+                        }
+                        
+                        it("returns true when no scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                        
+                        it("returns true when a http scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "http://testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                        
+                        it("returns true when a https scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "https://testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                    }
+                    
+                    context("for the https variant") {
+                        beforeEach {
+                            mockOGMCache.when { $0.pollers }.thenReturn(["https://testServer": OpenGroupAPI.Poller(for: "https://testServer")])
+                        }
+                        
+                        it("returns true when no scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                        
+                        it("returns true when a http scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "http://testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                        
+                        it("returns true when a https scheme is provided") {
+                            expect(
+                                openGroupManager
+                                    .hasExistingOpenGroup(
+                                        roomToken: "testRoom",
+                                        server: "https://testServer",
+                                        publicKey: "testKey",
+                                        using: testTransaction,
+                                        dependencies: dependencies
+                                    )
+                            ).to(beTrue())
+                        }
+                    }
+                }
+                
+                context("when given the legacy DNS host and there is a cached poller for the default server") {
+                    it("returns true") {
+                        mockOGMCache.when { $0.pollers }.thenReturn(["http://116.203.70.33": OpenGroupAPI.Poller(for: "http://116.203.70.33")])
+                        testTransaction.mockData[.objectForKey] = testGroupThread
+                        
+                        expect(
+                            openGroupManager
+                                .hasExistingOpenGroup(
+                                    roomToken: "testRoom",
+                                    server: "http://open.getsession.org",
+                                    publicKey: "testKey",
+                                    using: testTransaction,
+                                    dependencies: dependencies
+                                )
+                        ).to(beTrue())
+                    }
+                }
+                
+                context("when given the default server and there is a cached poller for the legacy DNS host") {
+                    it("returns true") {
+                        mockOGMCache.when { $0.pollers }.thenReturn(["http://open.getsession.org": OpenGroupAPI.Poller(for: "http://open.getsession.org")])
+                        testTransaction.mockData[.objectForKey] = testGroupThread
+                        
+                        expect(
+                            openGroupManager
+                                .hasExistingOpenGroup(
+                                    roomToken: "testRoom",
+                                    server: "http://116.203.70.33",
+                                    publicKey: "testKey",
+                                    using: testTransaction,
+                                    dependencies: dependencies
+                                )
+                        ).to(beTrue())
+                    }
+                }
+                
+                it("returns false when given an invalid server") {
+                    mockOGMCache.when { $0.pollers }.thenReturn(["testServer": OpenGroupAPI.Poller(for: "testServer")])
+                    testTransaction.mockData[.objectForKey] = testGroupThread
+                    
+                    expect(
+                        openGroupManager
+                            .hasExistingOpenGroup(
+                                roomToken: "testRoom",
+                                server: "%%%",
+                                publicKey: "testKey",
+                                using: testTransaction,
+                                dependencies: dependencies
+                            )
+                    ).to(beFalse())
+                }
+                
+                it("returns false if there is not a poller for the server in the cache") {
+                    mockOGMCache.when { $0.pollers }.thenReturn([:])
+                    testTransaction.mockData[.objectForKey] = testGroupThread
+                    
+                    expect(
+                        openGroupManager
+                            .hasExistingOpenGroup(
+                                roomToken: "testRoom",
+                                server: "testServer",
+                                publicKey: "testKey",
+                                using: testTransaction,
+                                dependencies: dependencies
+                            )
+                    ).to(beFalse())
+                }
+                
+                it("returns false if there is a poller for the server in the cache but no thread for the room") {
+                    mockOGMCache.when { $0.pollers }.thenReturn(["testServer": OpenGroupAPI.Poller(for: "testServer")])
+                    testTransaction.mockData[.objectForKey] = nil
+                    
+                    expect(
+                        openGroupManager
+                            .hasExistingOpenGroup(
+                                roomToken: "testRoom",
+                                server: "testServer",
+                                publicKey: "testKey",
+                                using: testTransaction,
+                                dependencies: dependencies
+                            )
+                    ).to(beFalse())
+                }
+            }
+            
             // MARK: - --add
             
             context("when adding") {