changelog, more tests, less logging

Author: fm3

Changelog.md

@@ -0,0 +1,9 @@
+# Changelog
+
+## Breaking Changes
+
+ - The `GetMultipleKeys` call now takes a `startAfterKey` instead of a `key` for pagination. The returned list will only start *after* this key. [#38](https://github.com/scalableminds/fossildb/pull/38)
+
+## Fixes
+
+ - Fixed a bug where the pagination for `GetMultipleKeys` could lead to an endless loop if some keys are prefixes of others. [#38](https://github.com/scalableminds/fossildb/pull/38)

src/main/scala/com/scalableminds/fossildb/db/VersionedKeyValueStore.scala

@@ -128,7 +128,6 @@ class VersionedKeyValueStore(underlying: RocksDBStore) {
   }
 
   def getMultipleKeys(startAfterKey: Option[String], prefix: Option[String] = None, version: Option[Long] = None, limit: Option[Int]): (Seq[String], Seq[Array[Byte]], Seq[Long]) = {
-    println(s"\ngetMultipleKeys! startAfterKey: $startAfterKey, prefix: $prefix, version: $version, limit: $limit")
     startAfterKey.foreach(requireValidKey)
     prefix.foreach{ p => requireValidKey(p)}
     val iterator: VersionFilterIterator = scanKeys(startAfterKey, prefix, version)
@@ -140,34 +139,24 @@ class VersionedKeyValueStore(underlying: RocksDBStore) {
      */
     val firstItemOpt: Option[VersionedKeyValuePair[Array[Byte]]] = if (iterator.hasNext) {
       val firstItem = iterator.next()
-      println(s"firstItem: ${firstItem.key}")
       if (startAfterKey.contains(firstItem.key)) {
         None
       } else {
         Some(firstItem)
       }
     } else None
 
-    if (firstItemOpt.isDefined) {
-      println(s"including first item $firstItemOpt")
-    } else {
-      println(s"dropping first item (it equals startAfterKey)")
-    }
-
     val limitPadded = limit.map(_ + 1).getOrElse(Int.MaxValue)
     val asVector = iterator.take(limitPadded).toVector
-    println(s"asVector: ${asVector.map{_.key}}")
     val asSequenceAdvancedIfNeeded = firstItemOpt.map(_ +: asVector).getOrElse(asVector).take(limit.getOrElse(Int.MaxValue))
     val keys = asSequenceAdvancedIfNeeded.map(_.key)
-    println(s"Returning keys ${keys.toList}")
     val values = asSequenceAdvancedIfNeeded.map(_.value)
     val versions = asSequenceAdvancedIfNeeded.map(_.version)
     (keys, values, versions)
   }
 
   private def scanKeys(startAfterKey: Option[String], prefix: Option[String] = None, version: Option[Long] = None): VersionFilterIterator = {
     val fullKey = startAfterKey.map(key => s"$key${VersionedKey.versionSeparator}").orElse(prefix).getOrElse("")
-    println(s"Scanning to $fullKey")
     new VersionFilterIterator(underlying.scan(fullKey, prefix), version)
   }
 

src/test/scala/com/scalableminds/fossildb/FossilDBSuite.scala

@@ -232,7 +232,7 @@ class FossilDBSuite extends FlatSpec with BeforeAndAfterEach with TestHelpers wi
     assert(reply.values.contains(testData2))
   }
 
-  it should "return keys of matching version, matching prefix (sorted alphabetically)" in {
+  it should "return keys of matching version, matching prefix" in {
     client.put(PutRequest(collectionA, aKey, Some(0), testData1))
     client.put(PutRequest(collectionA, aNotherKey, Some(0), testData1))
     client.put(PutRequest(collectionA, aThirdKey, Some(0), testData1))
@@ -242,8 +242,26 @@ class FossilDBSuite extends FlatSpec with BeforeAndAfterEach with TestHelpers wi
     client.put(PutRequest(collectionA, aKey, Some(2), testData3))
     client.put(PutRequest(collectionA, aNotherKey, Some(2), testData3))
     client.put(PutRequest(collectionA, aThirdKey, Some(2), testData3))
-    val reply = client.getMultipleKeys(GetMultipleKeysRequest(collectionA, None, Some("aK"), Some(1)))
+    val reply = client.getMultipleKeys(GetMultipleKeysRequest(collectionA, None, Some("aN"), Some(1)))
     assert(reply.keys.length == 1)
+    assert(reply.keys.contains(aNotherKey))
+    assert(reply.values.contains(testData2))
+    assert(reply.actualVersions.contains(1))
+  }
+
+  it should "return keys of matching version, matching prefix even if it is exact match" in {
+    client.put(PutRequest(collectionA, aKey, Some(0), testData1))
+    client.put(PutRequest(collectionA, aNotherKey, Some(0), testData1))
+    client.put(PutRequest(collectionA, aThirdKey, Some(0), testData1))
+    client.put(PutRequest(collectionA, aKey, Some(1), testData2))
+    client.put(PutRequest(collectionA, aNotherKey, Some(1), testData2))
+    client.put(PutRequest(collectionA, aThirdKey, Some(1), testData2))
+    client.put(PutRequest(collectionA, aKey, Some(2), testData3))
+    client.put(PutRequest(collectionA, aNotherKey, Some(2), testData3))
+    client.put(PutRequest(collectionA, aThirdKey, Some(2), testData3))
+    val reply = client.getMultipleKeys(GetMultipleKeysRequest(collectionA, None, Some(aNotherKey), Some(1)))
+    assert(reply.keys.length == 1)
+    assert(reply.keys.contains(aNotherKey))
     assert(reply.values.contains(testData2))
     assert(reply.actualVersions.contains(1))
   }