PVer4: V4Store: Check whether a hash prefix exists for a full hash

components/safe_browsing_db/v4_store_unittest.cc

Index: components/safe_browsing_db/v4_store_unittest.cc
diff --git a/components/safe_browsing_db/v4_store_unittest.cc b/components/safe_browsing_db/v4_store_unittest.cc
index 2f25a45513ca05e0b0c423df81204ffe9343670a..59d2ebb8e2208ffec82e0d7eb15bbda8d2f7bf71 100644
--- a/components/safe_browsing_db/v4_store_unittest.cc
+++ b/components/safe_browsing_db/v4_store_unittest.cc
@@ -530,4 +530,87 @@ TEST_F(V4StoreTest, TestReadFullResponseWithInvalidHashPrefixMap) {
   EXPECT_TRUE(read_store.hash_prefix_map_.empty());
 }
 
+TEST_F(V4StoreTest, TestHashPrefixExistsAtTheBeginning) {
+  HashPrefixes hash_prefixes = "abcdebbbbbccccc";
+  HashPrefix hash_prefix = "abcde";
+  EXPECT_TRUE(V4Store::HashPrefixMatches(hash_prefix, std::begin(hash_prefixes),

[Nathan Parker, 2016/07/19 23:01:31]

nit: These many-short-tests could fit well into one larger test with a comment
separating each.  That would help avoid the friend-clutter in the .h.

[vakh (use Gerrit instead), 2016/07/20 00:24:52]

IIRC, during my Noogler orientation this came up and the recommendation  was
that multiple short tests are usually better because the failing test name gives
a good indication of what is causing the failure.

The friend clutter can be avoided using friend class.

+                                         std::end(hash_prefixes)));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixExistsInTheMiddle) {
+  HashPrefixes hash_prefixes = "abcdebbbbbccccc";
+  HashPrefix hash_prefix = "bbbbb";
+  EXPECT_TRUE(V4Store::HashPrefixMatches(hash_prefix, std::begin(hash_prefixes),
+                                         std::end(hash_prefixes)));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixExistsAtTheEnd) {
+  HashPrefixes hash_prefixes = "abcdebbbbbccccc";
+  HashPrefix hash_prefix = "ccccc";
+  EXPECT_TRUE(V4Store::HashPrefixMatches(hash_prefix, std::begin(hash_prefixes),
+                                         std::end(hash_prefixes)));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixExistsAtTheBeginningOfEven) {
+  HashPrefixes hash_prefixes = "abcdebbbbb";
+  HashPrefix hash_prefix = "abcde";
+  EXPECT_TRUE(V4Store::HashPrefixMatches(hash_prefix, std::begin(hash_prefixes),
+                                         std::end(hash_prefixes)));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixExistsAtTheEndOfEven) {
+  HashPrefixes hash_prefixes = "abcdebbbbb";
+  HashPrefix hash_prefix = "bbbbb";
+  EXPECT_TRUE(V4Store::HashPrefixMatches(hash_prefix, std::begin(hash_prefixes),
+                                         std::end(hash_prefixes)));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixDoesNotExistInConcatenatedList) {
+  HashPrefixes hash_prefixes = "abcdebbbbb";
+  HashPrefix hash_prefix = "bbbbc";
+  EXPECT_FALSE(V4Store::HashPrefixMatches(
+      hash_prefix, std::begin(hash_prefixes), std::end(hash_prefixes)));
+}
+
+TEST_F(V4StoreTest, TestFullHashExistsInMapWithSingleSize) {
+  V4Store store(task_runner_, store_path_);
+  store.hash_prefix_map_[32] =
+      "0111222233334444555566667777888811112222333344445555666677778888";
+  FullHash full_hash = "11112222333344445555666677778888";
+  EXPECT_EQ("11112222333344445555666677778888",
+            store.GetMatchingHashPrefix(full_hash));
+}
+
+TEST_F(V4StoreTest, TestFullHashExistsInMapWithDifferentSizes) {
+  V4Store store(task_runner_, store_path_);
+  store.hash_prefix_map_[4] = "22223333aaaa";
+  store.hash_prefix_map_[32] = "11112222333344445555666677778888";
+  FullHash full_hash = "11112222333344445555666677778888";
+  EXPECT_EQ("11112222333344445555666677778888",
+            store.GetMatchingHashPrefix(full_hash));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixExistsInMapWithSingleSize) {
+  V4Store store(task_runner_, store_path_);
+  store.hash_prefix_map_[4] = "22223333aaaa";
+  FullHash full_hash = "22222222222222222222222222222222";
+  EXPECT_EQ("2222", store.GetMatchingHashPrefix(full_hash));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixExistsInMapWithDifferentSizes) {
+  V4Store store(task_runner_, store_path_);
+  store.hash_prefix_map_[4] = "22223333aaaa";
+  store.hash_prefix_map_[5] = "11111hhhhh";
+  FullHash full_hash = "22222222222222222222222222222222";
+  EXPECT_EQ("2222", store.GetMatchingHashPrefix(full_hash));
+}
+
+TEST_F(V4StoreTest, TestHashPrefixDoesNotExistInMapWithDifferentSizes) {
+  V4Store store(task_runner_, store_path_);
+  store.hash_prefix_map_[4] = "3333aaaa";
+  store.hash_prefix_map_[5] = "11111hhhhh";
+  FullHash full_hash = "22222222222222222222222222222222";
+  EXPECT_TRUE(store.GetMatchingHashPrefix(full_hash).empty());

[Nathan Parker, 2016/07/19 23:01:31]

How about a test that has no matching prefix?

Question: If there is a match against two different sized prefixes, which one
should be returned?  Right now it's not defined, since it's an undordered map. 
Should we return the longest match?  We could accomplish that by making it an
ordered map and then traversing it backwards.  Or maybe it doesn't matter, and
it should never happen.

[vakh (use Gerrit instead), 2016/07/20 00:24:52]

This test. The name has "DoesNot" which may be hard to catch. I could change the
name to make it clearer.
That should never happen.

[Nathan Parker, 2016/07/20 00:36:47]

ok, maybe we could add a comment in the code that says what happens if it did
("we pick one of the hashes, and which one is undefined").  

[vakh (use Gerrit instead), 2016/07/20 06:12:04]

Done.

+}
+
 }  // namespace safe_browsing