Support cache key for partial string map value (envoyproxy#50)

* Support cache key for string map.

* Update comment

mixerclient/BUILD

@@ -23,6 +23,8 @@ cc_library(
         "src/attribute_context.cc",
         "src/attribute_context.h",
         "src/attribute_converter.h",
+        "src/cache_key_set.cc",
+        "src/cache_key_set.h",
         "src/check_cache.cc",
         "src/check_cache.h",
         "src/client_impl.cc",
@@ -123,6 +125,17 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "cache_key_set_test",
+    size = "small",
+    srcs = ["src/cache_key_set_test.cc"],
+    linkstatic = 1,
+    deps = [
+        ":mixer_client_lib",
+        "//external:googletest_main",
+    ],
+)
+
 cc_test(
     name = "signature_test",
     size = "small",

mixerclient/include/options.h

@@ -18,6 +18,7 @@
 
 #include <memory>
 #include <set>
+#include <vector>
 
 namespace istio {
 namespace mixer_client {
@@ -59,7 +60,7 @@ struct CheckOptions {
 
   // Only the attributes in this set are used to caclculate cache key.
   // If empty, check cache is disabled.
-  std::set<std::string> cache_keys;
+  std::vector<std::string> cache_keys;
 };
 
 // Options controlling report behavior.

mixerclient/src/cache_key_set.cc

@@ -0,0 +1,49 @@
+/* Copyright 2017 Istio Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/cache_key_set.h"
+
+namespace istio {
+namespace mixer_client {
+
+CacheKeySet::CacheKeySet(const std::vector<std::string>& cache_keys) {
+  for (const std::string& key : cache_keys) {
+    size_t pos = key.find_first_of('/');
+    if (pos == std::string::npos) {
+      // Always override with an empty one.
+      // It handles case where
+      // "key/sub_key" comes first, then "key" come.
+      // In this case, "key" wins.
+      key_map_[key] = SubKeySet();
+    } else {
+      std::string split_key = key.substr(0, pos);
+      std::string split_sub_key = key.substr(pos + 1);
+      auto it = key_map_.find(split_key);
+      if (it == key_map_.end()) {
+        key_map_[split_key] = SubKeySet();
+        key_map_[split_key].sub_keys_.insert(split_sub_key);
+      } else {
+        // If map to empty set, it was created by "key"
+        // without sub_key, keep it as empty set.
+        if (!it->second.sub_keys_.empty()) {
+          it->second.sub_keys_.insert(split_sub_key);
+        }
+      }
+    }
+  }
+}
+
+}  // namespace mixer_client
+}  // namespace istio

mixerclient/src/cache_key_set.h

@@ -0,0 +1,69 @@
+/* Copyright 2017 Istio Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef MIXER_CLIENT_CACHE_KEY_SET_H_
+#define MIXER_CLIENT_CACHE_KEY_SET_H_
+
+#include <map>
+#include <set>
+#include <string>
+#include <vector>
+
+namespace istio {
+namespace mixer_client {
+
+// A class to store sub key set
+class SubKeySet {
+ public:
+  // Check if the sub key is in the set.
+  bool Found(const std::string& sub_key) const {
+    if (sub_keys_.empty()) {
+      return true;
+    } else {
+      return sub_keys_.find(sub_key) != sub_keys_.end();
+    }
+  }
+
+ private:
+  friend class CacheKeySet;
+  std::set<std::string> sub_keys_;
+};
+
+// A class to handle cache key set
+// A std::set will not work for string map sub keys.
+// For a StringMap attruibute,
+// If cache key is "attribute.key", all values will be used.
+// If cache key is "attribute.key/map.key", only the map key is used.
+// If both format exists, the whole map will be used.
+class CacheKeySet {
+ public:
+  CacheKeySet(const std::vector<std::string>& cache_keys);
+
+  // Return nullptr if the key is not in the set.
+  const SubKeySet* Find(const std::string& key) const {
+    const auto it = key_map_.find(key);
+    return it == key_map_.end() ? nullptr : &it->second;
+  }
+
+  bool empty() const { return key_map_.empty(); }
+
+ private:
+  std::map<std::string, SubKeySet> key_map_;
+};
+
+}  // namespace mixer_client
+}  // namespace istio
+
+#endif  // MIXER_CLIENT_CACHE_KEY_SET_H_

mixerclient/src/cache_key_set_test.cc

@@ -0,0 +1,53 @@
+/* Copyright 2017 Istio Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/cache_key_set.h"
+#include "gtest/gtest.h"
+
+namespace istio {
+namespace mixer_client {
+namespace {
+
+TEST(CacheKeySetTest, TestNonSubkey) {
+  CacheKeySet s({"key1", "key2"});
+  EXPECT_TRUE(s.Find("key1") != nullptr);
+  EXPECT_TRUE(s.Find("key2") != nullptr);
+  EXPECT_TRUE(s.Find("xyz") == nullptr);
+}
+
+TEST(CacheKeySetTest, TestSubkeyLater) {
+  // Whole key comes first, then sub key comes.
+  CacheKeySet s({"key1", "key2", "key2/sub"});
+  EXPECT_TRUE(s.Find("key2")->Found("sub"));
+  EXPECT_TRUE(s.Find("key2")->Found("xyz"));
+}
+
+TEST(CacheKeySetTest, TestSubkeyFirst) {
+  // The sub key comes first, then the whole key comes
+  CacheKeySet s({"key1", "key2/sub", "key2"});
+  EXPECT_TRUE(s.Find("key2")->Found("sub"));
+  EXPECT_TRUE(s.Find("key2")->Found("xyz"));
+}
+
+TEST(CacheKeySetTest, TestSubkey) {
+  CacheKeySet s({"key1", "key2/sub1", "key2/sub2"});
+  EXPECT_TRUE(s.Find("key2")->Found("sub1"));
+  EXPECT_TRUE(s.Find("key2")->Found("sub2"));
+  EXPECT_FALSE(s.Find("key2")->Found("xyz"));
+}
+
+}  // namespace
+}  // namespace mixer_client
+}  // namespace istio

mixerclient/src/check_cache.cc

@@ -27,12 +27,13 @@ using ::google::protobuf::util::error::Code;
 namespace istio {
 namespace mixer_client {
 
-CheckCache::CheckCache(const CheckOptions& options) : options_(options) {
+CheckCache::CheckCache(const CheckOptions& options)
+    : options_(options), cache_keys_(options.cache_keys) {
   // Converts flush_interval_ms to Cycle used by SimpleCycleTimer.
   flush_interval_in_cycle_ =
       options_.flush_interval_ms * SimpleCycleTimer::Frequency() / 1000;
 
-  if (options.num_entries >= 0 && !options.cache_keys.empty()) {
+  if (options.num_entries >= 0 && !cache_keys_.empty()) {
     cache_.reset(new CheckLRUCache(
         options.num_entries, std::bind(&CheckCache::OnCacheEntryDelete, this,
                                        std::placeholders::_1)));
@@ -57,8 +58,7 @@ Status CheckCache::Check(const Attributes& attributes, CheckResponse* response,
     return Status(Code::NOT_FOUND, "");
   }
 
-  std::string request_signature =
-      GenerateSignature(attributes, options_.cache_keys);
+  std::string request_signature = GenerateSignature(attributes, cache_keys_);
   if (signature) {
     *signature = request_signature;
   }

mixerclient/src/check_cache.h

@@ -27,7 +27,7 @@
 #include "include/client.h"
 #include "include/options.h"
 #include "mixer/v1/service.pb.h"
-#include "src/signature.h"
+#include "src/cache_key_set.h"
 #include "utils/simple_lru_cache.h"
 #include "utils/simple_lru_cache_inl.h"
 
@@ -117,6 +117,9 @@ class CheckCache {
   // The check options.
   CheckOptions options_;
 
+  // The cache keys.
+  CacheKeySet cache_keys_;
+
   // Mutex guarding the access of cache_;
   std::mutex cache_mutex_;
 

mixerclient/src/signature.cc

@@ -28,12 +28,13 @@ const int kDelimiterLength = 1;
 }  // namespace
 
 string GenerateSignature(const Attributes& attributes,
-                         const std::set<std::string>& cache_keys) {
+                         const CacheKeySet& cache_keys) {
   MD5 hasher;
 
   for (const auto& attribute : attributes.attributes) {
+    const SubKeySet* sub_keys = cache_keys.Find(attribute.first);
     // Skip the attributes not in the cache keys
-    if (cache_keys.find(attribute.first) == cache_keys.end()) {
+    if (sub_keys == nullptr) {
       continue;
     }
     hasher.Update(attribute.first);
@@ -67,10 +68,12 @@ string GenerateSignature(const Attributes& attributes,
         break;
       case Attributes::Value::ValueType::STRING_MAP:
         for (const auto& it : attribute.second.string_map_v) {
-          hasher.Update(it.first);
-          hasher.Update(kDelimiter, kDelimiterLength);
-          hasher.Update(it.second);
-          hasher.Update(kDelimiter, kDelimiterLength);
+          if (sub_keys->Found(it.first)) {
+            hasher.Update(it.first);
+            hasher.Update(kDelimiter, kDelimiterLength);
+            hasher.Update(it.second);
+            hasher.Update(kDelimiter, kDelimiterLength);
+          }
         }
         break;
     }

mixerclient/src/signature.h

@@ -21,13 +21,14 @@
 
 #include <string>
 #include "include/client.h"
+#include "src/cache_key_set.h"
 
 namespace istio {
 namespace mixer_client {
 
 // Generates signature for Attributes.
 std::string GenerateSignature(const Attributes& attributes,
-                              const std::set<std::string>& cache_keys);
+                              const CacheKeySet& cache_keys);
 
 }  // namespace mixer_client
 }  // namespace istio

mixerclient/src/signature_test.cc

@@ -64,12 +64,12 @@ class SignatureUtilTest : public ::testing::Test {
     attributes_.attributes[key] = Attributes::StringMapValue(std::move(value));
   }
 
-  std::set<std::string> GetKeys() const {
-    std::set<std::string> keys;
+  CacheKeySet GetKeys() const {
+    std::vector<std::string> keys;
     for (const auto& it : attributes_.attributes) {
-      keys.insert(it.first);
+      keys.push_back(it.first);
     }
-    return keys;
+    return CacheKeySet(keys);
   }
 
   Attributes attributes_;