[refact] Share index instances across a client

Because of the search cache and the offline mode, index instances are not stateless. Therefore it is important to avoid having two instances pointing to the same index, which could cause inconsistencies and race conditions.

Author: Clément Le Provost

ChangeLog.md

@@ -44,6 +44,7 @@ Other improvements:
 - Improve cancellation of `Index.waitTask()`
 - Add tests for Objective-C bridging (online flavor only)
 - Make timeouts configurable
+- `Index` instances are now shared across a `Client`
 
 
 ## 3.7 (2016-09-07)

Source/Client.swift

@@ -133,6 +133,16 @@ public typealias CompletionHandler = (_ content: JSONObject?, _ error: Error?) -
     // NOTE: Not constant only for the sake of mocking during unit tests.
     var session: URLSession
 
+    /// Cache of already created indices.
+    ///
+    /// This dictionary is used to avoid creating two instances to represent the same index, as it is (1) inefficient
+    /// and (2) potentially harmful since instances are stateful (that's especially true of mirrored/offline indices,
+    /// but also of online indices because of the search cache).
+    ///
+    /// + Note: The values are zeroing weak references to avoid leaking memory when an index is no longer used.
+    ///
+    var indices: NSMapTable<NSString, AnyObject> = NSMapTable(keyOptions: [.strongMemory], valueOptions: [.weakMemory])
+    
     /// Operation queue used to keep track of requests.
     /// `Request` instances are inherently asynchronous, since they are merely wrappers around `NSURLSessionTask`.
     /// The sole purpose of the queue is to retain them for the duration of their execution!
@@ -239,6 +249,26 @@ public typealias CompletionHandler = (_ content: JSONObject?, _ error: Error?) -
         return headers[name]
     }
 
+    /// Obtain a proxy to an Algolia index (no server call required by this method).
+    ///
+    /// + Note: Only one instance can exist for a given index name. Subsequent calls to this method with the same
+    ///   index name will return the same instance, unless it has already been released.
+    ///
+    /// - parameter indexName: The name of the index.
+    /// - returns: A proxy to the specified index.
+    ///
+    @objc(indexWithName:)
+    public func index(withName indexName: String) -> Index {
+        if let index = indices.object(forKey: indexName as NSString) {
+            assert(index is Index, "An index with the same name but a different type has already been created") // may happen in offline mode
+            return index as! Index
+        } else {
+            let index = Index(client: self, name: indexName)
+            indices.setObject(index, forKey: indexName as NSString)
+            return index
+        }
+    }
+
     // MARK: - Operations
 
     /// List existing indexes.
@@ -305,19 +335,6 @@ public typealias CompletionHandler = (_ content: JSONObject?, _ error: Error?) -
         return performHTTPQuery(path: path, method: .POST, body: request as [String : Any]?, hostnames: writeHosts, completionHandler: completionHandler)
     }
 
-    /// Create a proxy to an Algolia index (no server call required by this method).
-    ///
-    /// - parameter indexName: The name of the index.
-    /// - returns: A new proxy to the specified index.
-    ///
-    @objc(indexWithName:)
-    public func index(withName indexName: String) -> Index {
-        // IMPLEMENTATION NOTE: This method is called `initIndex` in other clients, which better conveys its semantics.
-        // However, methods prefixed by `init` are automatically considered as initializers by the Objective-C bridge.
-        // Therefore, `initIndex` would fail to compile in Objective-C, because its return type is not `instancetype`.
-        return Index(client: self, name: indexName)
-    }
-    
     /// Strategy when running multiple queries. See `Client.multipleQueries(...)`.
     ///
     public enum MultipleQueriesStrategy: String {

Source/Offline/OfflineClient.swift

@@ -109,7 +109,14 @@ import Foundation
     /// + Note: The offline client returns mirror-capable indices.
     ///
     @objc public override func index(withName indexName: String) -> MirroredIndex {
-        return MirroredIndex(client: self, name: indexName)
+        if let index = indices.object(forKey: indexName as NSString) {
+            assert(index is MirroredIndex, "An index with the same name but a different type has already been created")
+            return index as! MirroredIndex
+        } else {
+            let index = MirroredIndex(client: self, name: indexName)
+            indices.setObject(index, forKey: indexName as NSString)
+            return index
+        }
     }
 
     // MARK: - Utils

Tests/ClientTests.swift

@@ -22,7 +22,7 @@
 //
 
 import XCTest
-import AlgoliaSearch
+@testable import AlgoliaSearch
 
 class ClientTests: XCTestCase {
     let expectationTimeout: TimeInterval = 100
@@ -419,4 +419,22 @@ class ClientTests: XCTestCase {
         ]
         XCTAssertEqual(client.headers["User-Agent"], "ABC (1.2.3); DEF (4.5.6)")
     }
+    
+    func testReusingIndices() {
+        let indexName = "name"
+        let initialCount = client.indices.count // another index is created during set up
+
+        autoreleasepool {
+            // Create twice the same index and verify that it is re-used.
+            let index1: Index? = client.index(withName: indexName)
+            XCTAssertEqual(initialCount + 1, client.indices.count)
+            let index2: Index? = client.index(withName: indexName)
+            XCTAssertEqual(initialCount + 1, client.indices.count)
+            XCTAssert(index1 === index2)
+        }
+        // Verify that the index instance has been destroyed.
+        // NOTE: It may (and probably will) still be in the map, so we cannot rely on the count.
+        let memorizedIndex = client.indices.object(forKey: indexName as NSString)
+        XCTAssertNil(memorizedIndex)
+    }
 }