refactor(networking): Fix storage tests

waku-org · Mar 8, 2023 · 1aa7301 · 1aa7301
1 parent 0275a39
commit 1aa7301
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 25 deletions.
diff --git a/tests/v2/test_peer_storage.nim b/tests/v2/test_peer_storage.nim
@@ -22,64 +22,86 @@ suite "Peer Storage":
       peerKey = generateEcdsaKey()
       peer = PeerInfo.new(peerKey, @[peerLoc])
       peerProto = "/waku/2/default-waku/codec"
-      stored = StoredInfo(peerId: peer.peerId, addrs: @[peerLoc], protos: @[peerProto], publicKey: peerKey.getPublicKey().tryGet())
-      conn = Connectedness.CanConnect
+      connectedness = Connectedness.CanConnect
       disconn = 999999
+      stored = RemotePeerInfo(
+        peerId: peer.peerId,
+        addrs: @[peerLoc],
+        protocols: @[peerProto],
+        publicKey: peerKey.getPublicKey().tryGet(),
+        connectedness: connectedness,
+        disconnectTime: disconn)
 
     defer: storage.close()
 
     # Test insert and retrieve
 
-    discard storage.put(peer.peerId, stored, conn, disconn)
+    require storage.put(peer.peerId, stored, connectedness, disconn).isOk
 
     var responseCount = 0
-    # flags to check data matches what was stored (default true)
-    var peerIdFlag, storedInfoFlag, connectednessFlag, disconnectFlag: bool
 
-    proc data(peerId: PeerID, storedInfo: StoredInfo,
+    # Fetched variables from callback
+    var resPeerId: PeerId
+    var resStoredInfo: RemotePeerInfo
+    var resConnectedness: Connectedness
+    var resDisconnect: int64
+
+    proc data(peerId: PeerID, storedInfo: RemotePeerInfo,
               connectedness: Connectedness, disconnectTime: int64) {.raises: [Defect].} =
       responseCount += 1
 
       # Note: cannot use `check` within `{.raises: [Defect].}` block
       # @TODO: /Nim/lib/pure/unittest.nim(577, 16) Error: can raise an unlisted exception: Exception
       # These flags are checked outside this block.
-      peerIdFlag = peerId == peer.peerId
-      storedInfoFlag = storedInfo == stored
-      connectednessFlag = connectedness == conn
-      disconnectFlag = disconnectTime == disconn
+      resPeerId = peerId
+      resStoredInfo = storedInfo
+      resConnectedness = connectedness
+      resDisconnect = disconnectTime
 
     let res = storage.getAll(data)
 
     check:
       res.isErr == false
       responseCount == 1
-      peerIdFlag
-      storedInfoFlag
-      connectednessFlag
-      disconnectFlag
+      resPeerId == peer.peerId
+      resStoredInfo.peerId == peer.peerId
+      resStoredInfo.addrs == @[peerLoc]
+      resStoredInfo.protocols == @[peerProto]
+      resStoredInfo.publicKey == peerKey.getPublicKey().tryGet()
+      # TODO: For compatibility, we don't store connectedness and disconnectTime
+      #resStoredInfo.connectedness == connectedness
+      #resStoredInfo.disconnectTime == disconn
+      resConnectedness == Connectedness.CanConnect
+      resDisconnect == disconn
 
     # Test replace and retrieve (update an existing entry)
-    discard storage.put(peer.peerId, stored, Connectedness.CannotConnect, disconn + 10)
+    require storage.put(peer.peerId, stored, Connectedness.CannotConnect, disconn + 10).isOk
 
     responseCount = 0
-    proc replacedData(peerId: PeerID, storedInfo: StoredInfo,
+    proc replacedData(peerId: PeerID, storedInfo: RemotePeerInfo,
                       connectedness: Connectedness, disconnectTime: int64) {.raises: [Defect].} =
       responseCount += 1
 
       # Note: cannot use `check` within `{.raises: [Defect].}` block
       # @TODO: /Nim/lib/pure/unittest.nim(577, 16) Error: can raise an unlisted exception: Exception
       # These flags are checked outside this block.
-      peerIdFlag = peerId == peer.peerId
-      storedInfoFlag = storedInfo == stored
-      connectednessFlag = connectedness == CannotConnect
-      disconnectFlag = disconnectTime == disconn + 10
+      resPeerId = peerId
+      resStoredInfo = storedInfo
+      resConnectedness = connectedness
+      resDisconnect = disconnectTime
 
     let repRes = storage.getAll(replacedData)
 
     check:
-      repRes.isErr == false
+      res.isErr == false
       responseCount == 1
-      peerIdFlag
-      storedInfoFlag
-      connectednessFlag
-      disconnectFlag
+      resPeerId == peer.peerId
+      resStoredInfo.peerId == peer.peerId
+      resStoredInfo.addrs == @[peerLoc]
+      resStoredInfo.protocols == @[peerProto]
+      resStoredInfo.publicKey == peerKey.getPublicKey().tryGet()
+      # TODO: For compatibility, we don't store connectedness and disconnectTime
+      #resStoredInfo.connectedness == connectedness
+      #resStoredInfo.disconnectTime == disconn
+      resConnectedness == Connectedness.CannotConnect
+      resDisconnect == disconn + 10
diff --git a/waku/v2/node/peer_manager/peer_store/waku_peer_storage.nim b/waku/v2/node/peer_manager/peer_store/waku_peer_storage.nim
@@ -39,6 +39,8 @@ proc init*(T: type RemotePeerInfo, buffer: seq[byte]): ProtoResult[T] =
   discard ? pb.getRepeatedField(3, protoSeq)
   discard ? pb.getField(4, storedInfo.publicKey)
 
+  # TODO: Store the rest of parameters such as connectedness and disconnectTime
+
   storedInfo.addrs = multiaddrSeq
   storedInfo.protocols = protoSeq
 
@@ -77,6 +79,8 @@ proc new*(T: type WakuPeerStorage, db: SqliteDatabase): PeerStorageResult[T] =
   ##  - stored info (serialised protobuf), stored as a blob
   ##  - last known enumerated connectedness state, stored as an integer
   ##  - disconnect time in epoch seconds, if applicable
+
+  # TODO: connectedness and disconnectTime are now stored in the storedInfo type
   let
     createStmt = db.prepareStmt("""
       CREATE TABLE IF NOT EXISTS Peer (