From e5947f4db66862e660e59a03014955834f65b658 Mon Sep 17 00:00:00 2001
From: Jordan Hrycaj <jordan@status.im>
Date: Tue, 25 May 2021 19:52:09 +0100
Subject: [PATCH] Deep copy semantics for LRU cache

why:
  follows standard nim semantics

details:
  changed Table to TableRef in previous patch which was the
  wrong choice (see andri's comment.)
---
 nimbus/utils/lru_cache.nim | 10 ++++++----
 tests/test_lru_cache.nim   | 25 ++++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/nimbus/utils/lru_cache.nim b/nimbus/utils/lru_cache.nim
index 432720168..a332d367a 100644
--- a/nimbus/utils/lru_cache.nim
+++ b/nimbus/utils/lru_cache.nim
@@ -18,6 +18,9 @@
 ## (so long as the table does not degrade into one-bucket linear mode, or
 ## some bucket-adjustment algorithm takes over.)
 ##
+## For consistency with every other data type in Nim these have value
+## semantics, this means that `=` performs a deep copy of the LRU cache.
+##
 
 import
   math,
@@ -50,7 +53,7 @@ type
   LruData[K,V] = object
     maxItems: int                 ## Max number of entries
     first, last: K                ## Doubly linked item list queue
-    tab: TableRef[K,LruItem[K,V]] ## (`key`,encapsulated(`value`)) data table
+    tab: Table[K,LruItem[K,V]]    ## (`key`,encapsulated(`value`)) data table
 
   LruCache*[T,K,V,E] = object
     data*: LruData[K,V]           ## Cache data, can be serialised
@@ -78,7 +81,7 @@ proc clearLruCache*[T,K,V,E](cache: var LruCache[T,K,V,E])
   ## Reset/clear an initialised LRU cache.
   cache.data.first.reset
   cache.data.last.reset
-  cache.data.tab = newTable[K,LruItem[K,V]](cache.data.maxItems.nextPowerOfTwo)
+  cache.data.tab = initTable[K,LruItem[K,V]](cache.data.maxItems.nextPowerOfTwo)
 
 
 proc initLruCache*[T,K,V,E](cache: var LruCache[T,K,V,E];
@@ -176,14 +179,13 @@ proc read*[K,V](rlp: var Rlp; Q: type LruData[K,V]): Q {.inline.} =
   result.maxItems = rlp.read(int)
   result.first = rlp.read(K)
   result.last = rlp.read(K)
-  result.tab = newTable[K,LruItem[K,V]](result.maxItems.nextPowerOfTwo)
   for w in rlp.items:
     let (key,value) = w.read((K,LruItem[K,V]))
     result.tab[key] = value
 
 
 proc specs*[T,K,V,E](cache: var LruCache[T,K,V,E]):
-                                  (int, K, K, TableRef[K,LruItem[K,V]]) =
+                                  (int, K, K, Table[K,LruItem[K,V]]) =
   ## Returns cache data & specs `(maxItems,firstKey,lastKey,tableRef)` for
   ## debugging and testing.
   (cache.data.maxItems, cache.data.first, cache.data.last, cache.data.tab)
diff --git a/tests/test_lru_cache.nim b/tests/test_lru_cache.nim
index 3bdf7b990..d7a9311e8 100644
--- a/tests/test_lru_cache.nim
+++ b/tests/test_lru_cache.nim
@@ -46,7 +46,7 @@ proc first[T,K,V,E](cache: var LruCache[T,K,V,E]): K =
 proc last[T,K,V,E](cache: var LruCache[T,K,V,E]): K =
   cache.specs[2]
 
-proc tab[T,K,V,E](cache: var LruCache[T,K,V,E]): TableRef[K,LruItem[K,V]] =
+proc tab[T,K,V,E](cache: var LruCache[T,K,V,E]): Table[K,LruItem[K,V]] =
   cache.specs[3]
 
 
@@ -135,6 +135,26 @@ proc filledTestCache(noisy: bool): LruCache[int,int,string,int] =
 proc doFillUpTest(noisy: bool) =
   discard filledTestCache(noisy)
 
+
+proc doDeepCopyTest(noisy: bool) =
+
+  proc say(a: varargs[string]) =
+    say(noisy = noisy, args = a)
+
+  var
+    c1 = filledTestCache(false)
+    c2 = c1
+
+  doAssert c1 == c2
+  discard c1.getLruItem(77)
+
+  say &"c1Specs: {c1.maxItems} {c1.first} {c1.last} ..."
+  say &"c2Specs: {c2.maxItems} {c2.first} {c2.last} ..."
+
+  doAssert c1 != c2
+  doAssert c1.tab != c2.tab
+
+
 proc doSerialiserTest(noisy: bool) =
 
   proc say(a: varargs[string]) =
@@ -193,6 +213,9 @@ proc lruCacheMain*(noisy = defined(debug)) =
     test "Fill Up":
       doFillUpTest(noisy)
 
+    test "Deep Copy Semantics":
+      doDeepCopyTest(noisy)
+
     test "Rlp Serialise & Load":
       doSerialiserTest(noisy)