diff --git a/mixerclient/BUILD b/mixerclient/BUILD index 9f4657559bc..3dd1fba27c5 100644 --- a/mixerclient/BUILD +++ b/mixerclient/BUILD @@ -38,9 +38,8 @@ genrule( cc_library( name = "mixer_client_lib", srcs = [ - "src/attribute.cc", - "src/attribute_converter.cc", - "src/attribute_converter.h", + "src/attribute_compressor.cc", + "src/attribute_compressor.h", "src/check_cache.cc", "src/check_cache.h", "src/client_impl.cc", @@ -62,7 +61,7 @@ cc_library( "utils/status_test_util.h", ], hdrs = [ - "include/attribute.h", + "include/attributes_builder.h", "include/client.h", "include/options.h", "include/timer.h", @@ -102,20 +101,9 @@ cc_test( ) cc_test( - name = "attribute_test", + name = "attribute_compressor_test", size = "small", - srcs = ["src/attribute_test.cc"], - linkstatic = 1, - deps = [ - ":mixer_client_lib", - "//external:googletest_main", - ], -) - -cc_test( - name = "attribute_converter_test", - size = "small", - srcs = ["src/attribute_converter_test.cc"], + srcs = ["src/attribute_compressor_test.cc"], linkstatic = 1, deps = [ ":mixer_client_lib", diff --git a/mixerclient/include/attribute.h b/mixerclient/include/attribute.h deleted file mode 100644 index 78f31b4bb42..00000000000 --- a/mixerclient/include/attribute.h +++ /dev/null @@ -1,88 +0,0 @@ -/* 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 MIXERCLIENT_ATTRIBUTE_H -#define MIXERCLIENT_ATTRIBUTE_H - -#include -#include -#include -#include -#include - -namespace istio { -namespace mixer_client { - -// A structure to represent a bag of attributes with -// different types. -struct Attributes { - // A structure to hold different types of value. - struct Value { - // Data type - enum ValueType { - STRING, - INT64, - DOUBLE, - BOOL, - TIME, - BYTES, - DURATION, - STRING_MAP - } type; - - // Data value - union { - int64_t int64_v; - double double_v; - bool bool_v; - } value; - // Move types with constructor outside of union. - // It is not easy for union to support them. - std::string str_v; // for both STRING and BYTES - std::chrono::time_point time_v; - std::chrono::nanoseconds duration_nanos_v; - std::map string_map_v; - - // compare operator - bool operator==(const Value& v) const; - }; - - // Helper functions to construct different value types - static Value StringValue(const std::string& str); - static Value BytesValue(const std::string& bytes); - static Value Int64Value(int64_t value); - static Value DoubleValue(double value); - static Value BoolValue(bool value); - static Value TimeValue( - std::chrono::time_point value); - static Value DurationValue(std::chrono::nanoseconds value); - static Value StringMapValue(std::map&& string_map); - - // The attribute key to fill "quota" field in the QuotaRequest. - static const std::string kQuotaName; - // The attribute key to fill "amount" field in the QuotaRequest. - static const std::string kQuotaAmount; - - // Generates a string for logging or debugging. - std::string DebugString() const; - - // The attribute map. - std::map attributes; -}; - -} // namespace mixer_client -} // namespace istio - -#endif // MIXERCLIENT_ATTRIBUTE_H diff --git a/mixerclient/include/attributes_builder.h b/mixerclient/include/attributes_builder.h new file mode 100644 index 00000000000..2cb50ef3659 --- /dev/null +++ b/mixerclient/include/attributes_builder.h @@ -0,0 +1,95 @@ +/* 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 MIXERCLIENT_ATTRIBUTES_BUILDER_H +#define MIXERCLIENT_ATTRIBUTES_BUILDER_H + +#include +#include +#include + +#include "mixer/v1/attributes.pb.h" + +namespace istio { +namespace mixer_client { + +// Builder class to add attribute to protobuf Attributes. +// Its usage: +// builder(attribute).Add("key", value) +// .Add("key2", value2); +class AttributesBuilder { + public: + AttributesBuilder(::istio::mixer::v1::Attributes* attributes) + : attributes_(attributes) {} + + void AddString(const std::string& key, const std::string& str) { + (*attributes_->mutable_attributes())[key].set_string_value(str); + } + + void AddBytes(const std::string& key, const std::string& bytes) { + (*attributes_->mutable_attributes())[key].set_bytes_value(bytes); + } + + void AddInt64(const std::string& key, int64_t value) { + (*attributes_->mutable_attributes())[key].set_int64_value(value); + } + + void AddDouble(const std::string& key, double value) { + (*attributes_->mutable_attributes())[key].set_double_value(value); + } + + void AddBool(const std::string& key, bool value) { + (*attributes_->mutable_attributes())[key].set_bool_value(value); + } + + void AddTimestamp( + const std::string& key, + const std::chrono::time_point& value) { + auto time_stamp = + (*attributes_->mutable_attributes())[key].mutable_timestamp_value(); + long long nanos = std::chrono::duration_cast( + value.time_since_epoch()) + .count(); + time_stamp->set_seconds(nanos / 1000000000); + time_stamp->set_nanos(nanos % 1000000000); + } + + void AddDuration(const std::string& key, + const std::chrono::nanoseconds& value) { + auto duration = + (*attributes_->mutable_attributes())[key].mutable_duration_value(); + duration->set_seconds(value.count() / 1000000000); + duration->set_nanos(value.count() % 1000000000); + } + + void AddStringMap(const std::string& key, + const std::map& string_map) { + auto entries = (*attributes_->mutable_attributes())[key] + .mutable_string_map_value() + ->mutable_entries(); + entries->clear(); + for (const auto& map_it : string_map) { + (*entries)[map_it.first] = map_it.second; + } + } + + private: + ::istio::mixer::v1::Attributes* attributes_; +}; + +} // namespace mixer_client +} // namespace istio + +#endif // MIXERCLIENT_ATTRIBUTES_BUILDER_H diff --git a/mixerclient/include/client.h b/mixerclient/include/client.h index ee8fea48bf8..427cb700fdc 100644 --- a/mixerclient/include/client.h +++ b/mixerclient/include/client.h @@ -16,7 +16,6 @@ #ifndef MIXERCLIENT_CLIENT_H #define MIXERCLIENT_CLIENT_H -#include "attribute.h" #include "google/protobuf/stubs/status.h" #include "mixer/v1/service.pb.h" #include "options.h" @@ -94,11 +93,11 @@ class MixerClient { // The response data from mixer will be consumed by mixer client. // A check call. - virtual CancelFunc Check(const Attributes& attributes, + virtual CancelFunc Check(const ::istio::mixer::v1::Attributes& attributes, TransportCheckFunc transport, DoneFunc on_done) = 0; // A report call. - virtual void Report(const Attributes& attributes) = 0; + virtual void Report(const ::istio::mixer::v1::Attributes& attributes) = 0; }; // Creates a MixerClient object. diff --git a/mixerclient/src/attribute.cc b/mixerclient/src/attribute.cc deleted file mode 100644 index 6f104f51365..00000000000 --- a/mixerclient/src/attribute.cc +++ /dev/null @@ -1,177 +0,0 @@ -/* 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 "include/attribute.h" -#include -#include -#include - -namespace istio { -namespace mixer_client { - -namespace { - -std::string escape(const std::string& source) { - std::stringstream ss; - static const std::locale& loc = std::locale::classic(); - - for (const auto& c : source) { - if (std::isprint(c, loc)) { - ss << c; - } else { - ss << "\\x" << std::setfill('0') << std::setw(2) << std::hex - << static_cast(c); - } - } - return ss.str(); -} -} - -const std::string Attributes::kQuotaName = "quota.name"; -const std::string Attributes::kQuotaAmount = "quota.amount"; - -Attributes::Value Attributes::StringValue(const std::string& str) { - Attributes::Value v; - v.type = Attributes::Value::STRING; - v.str_v = str; - return v; -} - -Attributes::Value Attributes::BytesValue(const std::string& bytes) { - Attributes::Value v; - v.type = Attributes::Value::BYTES; - v.str_v = bytes; - return v; -} - -Attributes::Value Attributes::Int64Value(int64_t value) { - Attributes::Value v; - v.type = Attributes::Value::INT64; - v.value.int64_v = value; - return v; -} - -Attributes::Value Attributes::DoubleValue(double value) { - Attributes::Value v; - v.type = Attributes::Value::DOUBLE; - v.value.double_v = value; - return v; -} - -Attributes::Value Attributes::BoolValue(bool value) { - Attributes::Value v; - v.type = Attributes::Value::BOOL; - v.value.bool_v = value; - return v; -} - -Attributes::Value Attributes::TimeValue( - std::chrono::time_point value) { - Attributes::Value v; - v.type = Attributes::Value::TIME; - v.time_v = value; - return v; -} - -Attributes::Value Attributes::DurationValue(std::chrono::nanoseconds value) { - Attributes::Value v; - v.type = Attributes::Value::DURATION; - v.duration_nanos_v = value; - return v; -} - -Attributes::Value Attributes::StringMapValue( - std::map&& string_map) { - Attributes::Value v; - v.type = Attributes::Value::STRING_MAP; - v.string_map_v.swap(string_map); - return v; -} - -bool Attributes::Value::operator==(const Attributes::Value& v) const { - if (type != v.type) { - return false; - } - switch (type) { - case Attributes::Value::ValueType::STRING: - case Attributes::Value::ValueType::BYTES: - return str_v == v.str_v; - break; - case Attributes::Value::ValueType::INT64: - return value.int64_v == v.value.int64_v; - break; - case Attributes::Value::ValueType::DOUBLE: - return value.double_v == v.value.double_v; - break; - case Attributes::Value::ValueType::BOOL: - return value.bool_v == v.value.bool_v; - break; - case Attributes::Value::ValueType::TIME: - return time_v == v.time_v; - break; - case Attributes::Value::ValueType::DURATION: - return duration_nanos_v == v.duration_nanos_v; - break; - case Attributes::Value::ValueType::STRING_MAP: - return string_map_v == v.string_map_v; - break; - } - return false; -} - -std::string Attributes::DebugString() const { - std::stringstream ss; - for (const auto& it : attributes) { - ss << it.first << ": "; - switch (it.second.type) { - case Attributes::Value::ValueType::STRING: - ss << "(STRING): " << it.second.str_v; - break; - case Attributes::Value::ValueType::BYTES: - ss << "(BYTES): " << escape(it.second.str_v); - break; - case Attributes::Value::ValueType::INT64: - ss << "(INT64): " << it.second.value.int64_v; - break; - case Attributes::Value::ValueType::DOUBLE: - ss << "(DOUBLE): " << it.second.value.double_v; - break; - case Attributes::Value::ValueType::BOOL: - ss << "(BOOL): " << it.second.value.bool_v; - break; - case Attributes::Value::ValueType::TIME: - ss << "(TIME ms): " - << std::chrono::duration_cast( - it.second.time_v.time_since_epoch()) - .count(); - break; - case Attributes::Value::ValueType::DURATION: - ss << "(DURATION nanos): " << it.second.duration_nanos_v.count(); - break; - case Attributes::Value::ValueType::STRING_MAP: - ss << "(STRING MAP):"; - for (const auto& map_it : it.second.string_map_v) { - ss << std::endl; - ss << " " << map_it.first << ": " << map_it.second; - } - break; - } - ss << std::endl; - } - return ss.str(); -} - -} // namespace mixer_client -} // namespace istio diff --git a/mixerclient/src/attribute_converter.cc b/mixerclient/src/attribute_compressor.cc similarity index 66% rename from mixerclient/src/attribute_converter.cc rename to mixerclient/src/attribute_compressor.cc index 6289a49d99f..cb38f6b3ba4 100644 --- a/mixerclient/src/attribute_converter.cc +++ b/mixerclient/src/attribute_compressor.cc @@ -13,11 +13,16 @@ * limitations under the License. */ -#include "attribute_converter.h" +#include "attribute_compressor.h" #include "delta_update.h" #include "global_dictionary.h" #include "utils/protobuf.h" +using ::istio::mixer::v1::Attributes; +using ::istio::mixer::v1::Attributes_AttributeValue; +using ::istio::mixer::v1::Attributes_StringMap; +using ::istio::mixer::v1::CompressedAttributes; + namespace istio { namespace mixer_client { namespace { @@ -63,59 +68,59 @@ class MessageDictionary { }; ::istio::mixer::v1::StringMap CreateStringMap( - const std::map& string_map, - MessageDictionary& dict) { - ::istio::mixer::v1::StringMap map_msg; - auto* map_pb = map_msg.mutable_entries(); - for (const auto& it : string_map) { + const Attributes_StringMap& raw_map, MessageDictionary& dict) { + ::istio::mixer::v1::StringMap compressed_map; + auto* map_pb = compressed_map.mutable_entries(); + for (const auto& it : raw_map.entries()) { (*map_pb)[dict.GetIndex(it.first)] = dict.GetIndex(it.second); } - return map_msg; + return compressed_map; } -bool ConvertToPb(const Attributes& attributes, MessageDictionary& dict, - DeltaUpdate& delta_update, - ::istio::mixer::v1::CompressedAttributes* pb) { +bool CompressByDict(const Attributes& attributes, MessageDictionary& dict, + DeltaUpdate& delta_update, CompressedAttributes* pb) { delta_update.Start(); // Fill attributes. - for (const auto& it : attributes.attributes) { + for (const auto& it : attributes.attributes()) { const std::string& name = it.first; + const Attributes_AttributeValue& value = it.second; int index = dict.GetIndex(name); // Check delta update. If same, skip it. - if (delta_update.Check(index, it.second)) { + if (delta_update.Check(index, value)) { continue; } // Fill the attribute to proper map. - switch (it.second.type) { - case Attributes::Value::ValueType::STRING: - (*pb->mutable_strings())[index] = dict.GetIndex(it.second.str_v); + switch (value.value_case()) { + case Attributes_AttributeValue::kStringValue: + (*pb->mutable_strings())[index] = dict.GetIndex(value.string_value()); break; - case Attributes::Value::ValueType::BYTES: - (*pb->mutable_bytes())[index] = it.second.str_v; + case Attributes_AttributeValue::kBytesValue: + (*pb->mutable_bytes())[index] = value.bytes_value(); break; - case Attributes::Value::ValueType::INT64: - (*pb->mutable_int64s())[index] = it.second.value.int64_v; + case Attributes_AttributeValue::kInt64Value: + (*pb->mutable_int64s())[index] = value.int64_value(); break; - case Attributes::Value::ValueType::DOUBLE: - (*pb->mutable_doubles())[index] = it.second.value.double_v; + case Attributes_AttributeValue::kDoubleValue: + (*pb->mutable_doubles())[index] = value.double_value(); break; - case Attributes::Value::ValueType::BOOL: - (*pb->mutable_bools())[index] = it.second.value.bool_v; + case Attributes_AttributeValue::kBoolValue: + (*pb->mutable_bools())[index] = value.bool_value(); break; - case Attributes::Value::ValueType::TIME: - (*pb->mutable_timestamps())[index] = CreateTimestamp(it.second.time_v); + case Attributes_AttributeValue::kTimestampValue: + (*pb->mutable_timestamps())[index] = value.timestamp_value(); break; - case Attributes::Value::ValueType::DURATION: - (*pb->mutable_durations())[index] = - CreateDuration(it.second.duration_nanos_v); + case Attributes_AttributeValue::kDurationValue: + (*pb->mutable_durations())[index] = value.duration_value(); break; - case Attributes::Value::ValueType::STRING_MAP: + case Attributes_AttributeValue::kStringMapValue: (*pb->mutable_string_maps())[index] = - CreateStringMap(it.second.string_map_v, dict); + CreateStringMap(value.string_map_value(), dict); + break; + case Attributes_AttributeValue::VALUE_NOT_SET: break; } } @@ -123,9 +128,9 @@ bool ConvertToPb(const Attributes& attributes, MessageDictionary& dict, return delta_update.Finish(); } -class BatchConverterImpl : public BatchConverter { +class BatchCompressorImpl : public BatchCompressor { public: - BatchConverterImpl(const GlobalDictionary& global_dict) + BatchCompressorImpl(const GlobalDictionary& global_dict) : dict_(global_dict), delta_update_(DeltaUpdate::Create()), report_(new ::istio::mixer::v1::ReportRequest) { @@ -133,8 +138,8 @@ class BatchConverterImpl : public BatchConverter { } bool Add(const Attributes& attributes) override { - ::istio::mixer::v1::CompressedAttributes pb; - if (!ConvertToPb(attributes, dict_, *delta_update_, &pb)) { + CompressedAttributes pb; + if (!CompressByDict(attributes, dict_, *delta_update_, &pb)) { return false; } pb.GetReflection()->Swap(report_->add_attributes(), &pb); @@ -185,22 +190,23 @@ void GlobalDictionary::ShrinkToBase() { } } -void AttributeConverter::Convert( +void AttributeCompressor::Compress( const Attributes& attributes, ::istio::mixer::v1::CompressedAttributes* pb) const { MessageDictionary dict(global_dict_); std::unique_ptr delta_update = DeltaUpdate::CreateNoOp(); - ConvertToPb(attributes, dict, *delta_update, pb); + CompressByDict(attributes, dict, *delta_update, pb); for (const std::string& word : dict.GetWords()) { pb->add_words(word); } } -std::unique_ptr AttributeConverter::CreateBatchConverter() +std::unique_ptr AttributeCompressor::CreateBatchCompressor() const { - return std::unique_ptr(new BatchConverterImpl(global_dict_)); + return std::unique_ptr( + new BatchCompressorImpl(global_dict_)); } } // namespace mixer_client diff --git a/mixerclient/src/attribute_converter.h b/mixerclient/src/attribute_compressor.h similarity index 72% rename from mixerclient/src/attribute_converter.h rename to mixerclient/src/attribute_compressor.h index 84bbac881fa..916938b2839 100644 --- a/mixerclient/src/attribute_converter.h +++ b/mixerclient/src/attribute_compressor.h @@ -13,13 +13,13 @@ * limitations under the License. */ -#ifndef MIXERCLIENT_ATTRIBUTE_CONVERTER_H -#define MIXERCLIENT_ATTRIBUTE_CONVERTER_H +#ifndef MIXERCLIENT_ATTRIBUTE_COMPRESSOR_H +#define MIXERCLIENT_ATTRIBUTE_COMPRESSOR_H #include -#include "include/attribute.h" -#include "mixer/v1/service.pb.h" +#include "mixer/v1/attributes.pb.h" +#include "mixer/v1/report.pb.h" namespace istio { namespace mixer_client { @@ -44,14 +44,14 @@ class GlobalDictionary { int top_index_; }; -// A attribute batch converter for report. -class BatchConverter { +// A attribute batch compressor for report. +class BatchCompressor { public: - virtual ~BatchConverter() {} + virtual ~BatchCompressor() {} // Add an attribute set to the batch. // Return false if it could not be added for delta update. - virtual bool Add(const Attributes& attributes) = 0; + virtual bool Add(const ::istio::mixer::v1::Attributes& attributes) = 0; // Get the batched size. virtual int size() const = 0; @@ -60,14 +60,14 @@ class BatchConverter { virtual std::unique_ptr<::istio::mixer::v1::ReportRequest> Finish() = 0; }; -// Convert attributes from struct to protobuf -class AttributeConverter { +// Compress attributes. +class AttributeCompressor { public: - void Convert(const Attributes& attributes, - ::istio::mixer::v1::CompressedAttributes* attributes_pb) const; + void Compress(const ::istio::mixer::v1::Attributes& attributes, + ::istio::mixer::v1::CompressedAttributes* attributes_pb) const; - // Create a batch converter. - std::unique_ptr CreateBatchConverter() const; + // Create a batch compressor. + std::unique_ptr CreateBatchCompressor() const; int global_word_count() const { return global_dict_.size(); } @@ -81,4 +81,4 @@ class AttributeConverter { } // namespace mixer_client } // namespace istio -#endif // MIXERCLIENT_ATTRIBUTE_CONVERTER_H +#endif // MIXERCLIENT_ATTRIBUTE_COMPRESSOR_H diff --git a/mixerclient/src/attribute_compressor_test.cc b/mixerclient/src/attribute_compressor_test.cc new file mode 100644 index 00000000000..c4fb9811992 --- /dev/null +++ b/mixerclient/src/attribute_compressor_test.cc @@ -0,0 +1,271 @@ +/* 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/attribute_compressor.h" +#include "include/attributes_builder.h" + +#include +#include "google/protobuf/text_format.h" +#include "google/protobuf/util/message_differencer.h" +#include "gtest/gtest.h" + +using std::string; +using ::istio::mixer::v1::Attributes; +using ::istio::mixer::v1::Attributes_AttributeValue; +using ::istio::mixer::v1::Attributes_StringMap; +using ::istio::mixer::v1::CompressedAttributes; + +using ::google::protobuf::TextFormat; +using ::google::protobuf::util::MessageDifferencer; + +namespace istio { +namespace mixer_client { +namespace { + +const char kAttributes[] = R"( +words: "JWT-Token" +strings { + key: 2 + value: 127 +} +strings { + key: 6 + value: 101 +} +int64s { + key: 1 + value: 35 +} +int64s { + key: 8 + value: 8080 +} +doubles { + key: 78 + value: 99.9 +} +bools { + key: 71 + value: true +} +timestamps { + key: 132 + value { + } +} +durations { + key: 29 + value { + seconds: 5 + } +} +bytes { + key: 0 + value: "text/html; charset=utf-8" +} +string_maps { + key: 15 + value { + entries { + key: 50 + value: -1 + } + entries { + key: 58 + value: 104 + } + } +} +)"; + +const char kReportAttributes[] = R"( +attributes { + strings { + key: 2 + value: 127 + } + strings { + key: 6 + value: 101 + } + int64s { + key: 1 + value: 35 + } + int64s { + key: 8 + value: 8080 + } + doubles { + key: 78 + value: 99.9 + } + bools { + key: 71 + value: true + } + timestamps { + key: 132 + value { + } + } + durations { + key: 29 + value { + seconds: 5 + } + } + bytes { + key: 0 + value: "text/html; charset=utf-8" + } + string_maps { + key: 15 + value { + entries { + key: 50 + value: -1 + } + entries { + key: 58 + value: 104 + } + } + } +} +attributes { + int64s { + key: 1 + value: 135 + } + int64s { + key: 27 + value: 111 + } + doubles { + key: 78 + value: 123.99 + } + bools { + key: 71 + value: false + } + string_maps { + key: 15 + value { + entries { + key: 32 + value: 90 + } + entries { + key: 58 + value: 104 + } + } + } +} +default_words: "JWT-Token" +global_word_count: 111 +)"; + +class AttributeCompressorTest : public ::testing::Test { + protected: + void SetUp() { + // Have to use words from global dictionary. + // Otherwise test is flaky since protobuf::map order is not deterministic + // if a word has to be in the per-message dictionary, its index depends + // on the order it created. + AttributesBuilder builder(&attributes_); + builder.AddString("source.name", "connection.received.bytes_total"); + builder.AddBytes("source.ip", "text/html; charset=utf-8"); + builder.AddDouble("range", 99.9); + builder.AddInt64("source.port", 35); + builder.AddBool("keep-alive", true); + builder.AddString("source.user", "x-http-method-override"); + builder.AddInt64("target.port", 8080); + + std::chrono::time_point time_point; + std::chrono::seconds secs(5); + builder.AddTimestamp("context.timestamp", time_point); + builder.AddDuration( + "response.duration", + std::chrono::duration_cast(secs)); + + // JWT-token is only word not in the global dictionary. + std::map string_map = { + {"authorization", "JWT-Token"}, {"content-type", "application/json"}}; + builder.AddStringMap("request.headers", std::move(string_map)); + } + + Attributes attributes_; +}; + +TEST_F(AttributeCompressorTest, CompressTest) { + // A compressor with an empty global dictionary. + AttributeCompressor compressor; + ::istio::mixer::v1::CompressedAttributes attributes_pb; + compressor.Compress(attributes_, &attributes_pb); + + std::string out_str; + TextFormat::PrintToString(attributes_pb, &out_str); + GOOGLE_LOG(INFO) << "===" << out_str << "==="; + + ::istio::mixer::v1::CompressedAttributes expected_attributes_pb; + ASSERT_TRUE( + TextFormat::ParseFromString(kAttributes, &expected_attributes_pb)); + EXPECT_TRUE( + MessageDifferencer::Equals(attributes_pb, expected_attributes_pb)); +} + +TEST_F(AttributeCompressorTest, BatchCompressTest) { + // A compressor with an empty global dictionary. + AttributeCompressor compressor; + auto batch_compressor = compressor.CreateBatchCompressor(); + + EXPECT_TRUE(batch_compressor->Add(attributes_)); + + // modify some attributes + AttributesBuilder builder(&attributes_); + builder.AddDouble("range", 123.99); + builder.AddInt64("source.port", 135); + builder.AddInt64("response.size", 111); + builder.AddBool("keep-alive", false); + builder.AddStringMap("request.headers", {{"content-type", "application/json"}, + {":method", "GET"}}); + + // Since there is no deletion, batch is good + EXPECT_TRUE(batch_compressor->Add(attributes_)); + + // remove a key + attributes_.mutable_attributes()->erase("response.size"); + // Batch should fail. + EXPECT_FALSE(batch_compressor->Add(attributes_)); + + auto report_pb = batch_compressor->Finish(); + + std::string out_str; + TextFormat::PrintToString(*report_pb, &out_str); + GOOGLE_LOG(INFO) << "===" << out_str << "==="; + + ::istio::mixer::v1::ReportRequest expected_report_pb; + ASSERT_TRUE( + TextFormat::ParseFromString(kReportAttributes, &expected_report_pb)); + report_pb->set_global_word_count(111); + EXPECT_TRUE(MessageDifferencer::Equals(*report_pb, expected_report_pb)); +} + +} // namespace +} // namespace mixer_client +} // namespace istio diff --git a/mixerclient/src/attribute_converter_test.cc b/mixerclient/src/attribute_converter_test.cc deleted file mode 100644 index 781713273d5..00000000000 --- a/mixerclient/src/attribute_converter_test.cc +++ /dev/null @@ -1,325 +0,0 @@ -/* 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/attribute_converter.h" - -#include -#include "google/protobuf/text_format.h" -#include "google/protobuf/util/message_differencer.h" -#include "gtest/gtest.h" - -using std::string; -using ::google::protobuf::TextFormat; -using ::google::protobuf::util::MessageDifferencer; - -namespace istio { -namespace mixer_client { -namespace { - -const char kAttributes[] = R"( -words: "bool-key" -words: "bytes-key" -words: "double-key" -words: "duration-key" -words: "int-key" -words: "source user" -words: "string-key" -words: "this is a string value" -words: "string-map-key" -words: "key1" -words: "value1" -words: "key2" -words: "value2" -words: "time-key" -strings { - key: -7 - value: -8 -} -strings { - key: 6 - value: -6 -} -int64s { - key: -5 - value: 35 -} -int64s { - key: 8 - value: 8080 -} -doubles { - key: -3 - value: 99.9 -} -bools { - key: -1 - value: true -} -timestamps { - key: -14 - value { - } -} -durations { - key: -4 - value { - seconds: 5 - } -} -bytes { - key: -2 - value: "this is a bytes value" -} -string_maps { - key: -9 - value { - entries { - key: -12 - value: -13 - } - entries { - key: -10 - value: -11 - } - } -} -)"; - -const char kReportAttributes[] = R"( -attributes { - strings { - key: -7 - value: -8 - } - strings { - key: 6 - value: -6 - } - int64s { - key: -5 - value: 35 - } - int64s { - key: 8 - value: 8080 - } - doubles { - key: -3 - value: 99.9 - } - bools { - key: -1 - value: true - } - timestamps { - key: -14 - value { - } - } - durations { - key: -4 - value { - seconds: 5 - } - } - bytes { - key: -2 - value: "this is a bytes value" - } - string_maps { - key: -9 - value { - entries { - key: -12 - value: -13 - } - entries { - key: -10 - value: -11 - } - } - } -} -attributes { - int64s { - key: -15 - value: 111 - } - int64s { - key: -5 - value: 135 - } - doubles { - key: -3 - value: 123.99 - } - bools { - key: -1 - value: false - } - string_maps { - key: -9 - value { - entries { - key: -16 - value: -17 - } - entries { - key: 32 - value: 90 - } - } - } -} -default_words: "bool-key" -default_words: "bytes-key" -default_words: "double-key" -default_words: "duration-key" -default_words: "int-key" -default_words: "source user" -default_words: "string-key" -default_words: "this is a string value" -default_words: "string-map-key" -default_words: "key1" -default_words: "value1" -default_words: "key2" -default_words: "value2" -default_words: "time-key" -default_words: "int-key2" -default_words: "key" -default_words: "value" -global_word_count: 111 -)"; - -class AttributeConverterTest : public ::testing::Test { - protected: - void AddString(const string& key, const string& value) { - attributes_.attributes[key] = Attributes::StringValue(value); - } - - void AddBytes(const string& key, const string& value) { - attributes_.attributes[key] = Attributes::BytesValue(value); - } - - void AddTime( - const string& key, - const std::chrono::time_point& value) { - attributes_.attributes[key] = Attributes::TimeValue(value); - } - - void AddDoublePair(const string& key, double value) { - attributes_.attributes[key] = Attributes::DoubleValue(value); - } - - void AddInt64Pair(const string& key, int64_t value) { - attributes_.attributes[key] = Attributes::Int64Value(value); - } - - void AddBoolPair(const string& key, bool value) { - attributes_.attributes[key] = Attributes::BoolValue(value); - } - - void AddDuration(const string& key, std::chrono::nanoseconds value) { - attributes_.attributes[key] = Attributes::DurationValue(value); - } - - void AddStringMap(const string& key, - std::map&& value) { - attributes_.attributes[key] = Attributes::StringMapValue(std::move(value)); - } - - void SetUp() { - AddString("string-key", "this is a string value"); - AddBytes("bytes-key", "this is a bytes value"); - AddDoublePair("double-key", 99.9); - AddInt64Pair("int-key", 35); - AddBoolPair("bool-key", true); - - // add some global words - AddString("source.user", "source user"); - AddInt64Pair("target.port", 8080); - - // default to Clock's epoch. - std::chrono::time_point time_point; - AddTime("time-key", time_point); - - std::chrono::seconds secs(5); - AddDuration("duration-key", - std::chrono::duration_cast(secs)); - - std::map string_map = {{"key1", "value1"}, - {"key2", "value2"}}; - AddStringMap("string-map-key", std::move(string_map)); - } - - Attributes attributes_; -}; - -TEST_F(AttributeConverterTest, ConvertTest) { - // A converter with an empty global dictionary. - AttributeConverter converter({}); - ::istio::mixer::v1::CompressedAttributes attributes_pb; - converter.Convert(attributes_, &attributes_pb); - - std::string out_str; - TextFormat::PrintToString(attributes_pb, &out_str); - GOOGLE_LOG(INFO) << "===" << out_str << "==="; - - ::istio::mixer::v1::CompressedAttributes expected_attributes_pb; - ASSERT_TRUE( - TextFormat::ParseFromString(kAttributes, &expected_attributes_pb)); - EXPECT_TRUE( - MessageDifferencer::Equals(attributes_pb, expected_attributes_pb)); -} - -TEST_F(AttributeConverterTest, BatchConvertTest) { - // A converter with an empty global dictionary. - AttributeConverter converter({}); - auto batch_converter = converter.CreateBatchConverter(); - - EXPECT_TRUE(batch_converter->Add(attributes_)); - - // modify some attributes - AddDoublePair("double-key", 123.99); - AddInt64Pair("int-key", 135); - AddInt64Pair("int-key2", 111); - AddBoolPair("bool-key", false); - - AddStringMap("string-map-key", {{"key", "value"}, {":method", "GET"}}); - - // Since there is no deletion, batch is good - EXPECT_TRUE(batch_converter->Add(attributes_)); - - // remove a key - attributes_.attributes.erase("int-key2"); - // Batch should fail. - EXPECT_FALSE(batch_converter->Add(attributes_)); - - auto report_pb = batch_converter->Finish(); - - std::string out_str; - TextFormat::PrintToString(*report_pb, &out_str); - GOOGLE_LOG(INFO) << "===" << out_str << "==="; - - ::istio::mixer::v1::ReportRequest expected_report_pb; - ASSERT_TRUE( - TextFormat::ParseFromString(kReportAttributes, &expected_report_pb)); - report_pb->set_global_word_count(111); - EXPECT_TRUE(MessageDifferencer::Equals(*report_pb, expected_report_pb)); -} - -} // namespace -} // namespace mixer_client -} // namespace istio diff --git a/mixerclient/src/attribute_test.cc b/mixerclient/src/attribute_test.cc deleted file mode 100644 index 0dfb767139b..00000000000 --- a/mixerclient/src/attribute_test.cc +++ /dev/null @@ -1,83 +0,0 @@ -/* 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 "include/attribute.h" -#include "gtest/gtest.h" - -namespace istio { -namespace mixer_client { - -TEST(AttributeValuelTest, CompareString) { - Attributes::Value v1 = Attributes::StringValue("string1"); - Attributes::Value v2 = Attributes::StringValue("string2"); - EXPECT_TRUE(v1 == v1); - EXPECT_FALSE(v1 == v2); -} - -TEST(AttributeValuelTest, CompareInt64) { - Attributes::Value v1 = Attributes::Int64Value(1); - Attributes::Value v2 = Attributes::Int64Value(2); - EXPECT_TRUE(v1 == v1); - EXPECT_FALSE(v1 == v2); -} - -TEST(AttributeValuelTest, CompareDouble) { - Attributes::Value v1 = Attributes::DoubleValue(1.0); - Attributes::Value v2 = Attributes::DoubleValue(2.0); - EXPECT_TRUE(v1 == v1); - EXPECT_FALSE(v1 == v2); -} - -TEST(AttributeValuelTest, CompareBool) { - Attributes::Value v1 = Attributes::BoolValue(true); - Attributes::Value v2 = Attributes::BoolValue(false); - EXPECT_TRUE(v1 == v1); - EXPECT_FALSE(v1 == v2); -} - -TEST(AttributeValuelTest, CompareTime) { - std::chrono::time_point t1; - std::chrono::time_point t2 = - t1 + std::chrono::seconds(1); - Attributes::Value v1 = Attributes::TimeValue(t1); - Attributes::Value v2 = Attributes::TimeValue(t2); - EXPECT_TRUE(v1 == v1); - EXPECT_FALSE(v1 == v2); -} - -TEST(AttributeValuelTest, CompareDuration) { - std::chrono::seconds d1(1); - std::chrono::seconds d2(2); - Attributes::Value v1 = Attributes::DurationValue( - std::chrono::duration_cast(d1)); - Attributes::Value v2 = Attributes::DurationValue( - std::chrono::duration_cast(d2)); - EXPECT_TRUE(v1 == v1); - EXPECT_FALSE(v1 == v2); -} - -TEST(AttributeValuelTest, CompareStringMap) { - Attributes::Value v1 = Attributes::StringMapValue({{"key1", "value1"}}); - Attributes::Value v2 = Attributes::StringMapValue({{"key2", "value2"}}); - Attributes::Value v3 = - Attributes::StringMapValue({{"key2", "value2"}, {"key1", "value1"}}); - EXPECT_TRUE(v1 == v1); - EXPECT_FALSE(v1 == v2); - EXPECT_FALSE(v1 == v3); - EXPECT_FALSE(v2 == v3); -} - -} // namespace mixer_client -} // namespace istio diff --git a/mixerclient/src/check_cache.cc b/mixerclient/src/check_cache.cc index a1bbe612cf3..eeb9434f627 100644 --- a/mixerclient/src/check_cache.cc +++ b/mixerclient/src/check_cache.cc @@ -17,6 +17,7 @@ #include "utils/protobuf.h" using namespace std::chrono; +using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::CheckResponse; using ::google::protobuf::util::Status; using ::google::protobuf::util::error::Code; diff --git a/mixerclient/src/check_cache.h b/mixerclient/src/check_cache.h index 362c1b26ed3..c7522e6dc74 100644 --- a/mixerclient/src/check_cache.h +++ b/mixerclient/src/check_cache.h @@ -57,7 +57,7 @@ class CheckCache { ::google::protobuf::util::Status status() const { return status_; } void SetResponse(const ::google::protobuf::util::Status& status, - const Attributes& attributes, + const ::istio::mixer::v1::Attributes& attributes, const ::istio::mixer::v1::CheckResponse& response) { if (on_response_) { status_ = on_response_(status, attributes, response); @@ -71,12 +71,14 @@ class CheckCache { // The function to set check response. using OnResponseFunc = std::function<::google::protobuf::util::Status( - const ::google::protobuf::util::Status&, const Attributes& attributes, + const ::google::protobuf::util::Status&, + const ::istio::mixer::v1::Attributes& attributes, const ::istio::mixer::v1::CheckResponse&)>; OnResponseFunc on_response_; }; - void Check(const Attributes& attributes, CheckResult* result); + void Check(const ::istio::mixer::v1::Attributes& attributes, + CheckResult* result); private: friend class CheckCacheTest; @@ -84,13 +86,13 @@ class CheckCache { // If the check could not be handled by the cache, returns NOT_FOUND, // caller has to send the request to mixer. - ::google::protobuf::util::Status Check(const Attributes& request, - Tick time_now); + ::google::protobuf::util::Status Check( + const ::istio::mixer::v1::Attributes& request, Tick time_now); // Caches a response from a remote mixer call. // Return the converted status from response. ::google::protobuf::util::Status CacheResponse( - const Attributes& attributes, + const ::istio::mixer::v1::Attributes& attributes, const ::istio::mixer::v1::CheckResponse& response, Tick time_now); // Flushes out all cached check responses; clears all cache items. diff --git a/mixerclient/src/check_cache_test.cc b/mixerclient/src/check_cache_test.cc index 2c196023441..d30ded7afda 100644 --- a/mixerclient/src/check_cache_test.cc +++ b/mixerclient/src/check_cache_test.cc @@ -16,10 +16,12 @@ #include "src/check_cache.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "include/attributes_builder.h" #include "utils/protobuf.h" #include "utils/status_test_util.h" using namespace std::chrono; +using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReferencedAttributes; using ::google::protobuf::util::Status; @@ -39,8 +41,8 @@ class CheckCacheTest : public ::testing::Test { cache_ = std::unique_ptr(new CheckCache(options)); ASSERT_TRUE((bool)(cache_)); - attributes_.attributes["target.service"] = - Attributes::StringValue("this-is-a-string-value"); + AttributesBuilder(&attributes_) + .AddString("target.service", "this-is-a-string-value"); } void VerifyDisabledCache() { @@ -249,8 +251,8 @@ TEST_F(CheckCacheTest, TestTwoCacheKeys) { EXPECT_TRUE(result1.IsCacheHit()); Attributes attributes1; - attributes1.attributes["target.service"] = - Attributes::StringValue("different target service"); + AttributesBuilder(&attributes1) + .AddString("target.service", "different target service"); // Not in the cache since it has different value CheckCache::CheckResult result2; @@ -287,8 +289,7 @@ TEST_F(CheckCacheTest, TestTwoReferenced) { result.SetResponse(Status::OK, attributes_, ok_response); Attributes attributes1; - attributes1.attributes["target.name"] = - Attributes::StringValue("target name"); + AttributesBuilder(&attributes1).AddString("target.name", "target name"); // Not in the cache since it has different value CheckCache::CheckResult result1; diff --git a/mixerclient/src/client_impl.cc b/mixerclient/src/client_impl.cc index fc4991aa75b..e50731b5062 100644 --- a/mixerclient/src/client_impl.cc +++ b/mixerclient/src/client_impl.cc @@ -15,6 +15,7 @@ #include "src/client_impl.h" #include "utils/protobuf.h" +using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReportRequest; @@ -31,7 +32,7 @@ MixerClientImpl::MixerClientImpl(const MixerClientOptions &options) std::unique_ptr(new CheckCache(options.check_options)); report_batch_ = std::unique_ptr( new ReportBatch(options.report_options, options_.report_transport, - options.timer_create_func, converter_)); + options.timer_create_func, compressor_)); quota_cache_ = std::unique_ptr(new QuotaCache(options.quota_options)); @@ -71,8 +72,8 @@ CancelFunc MixerClientImpl::Check(const Attributes &attributes, } } - converter_.Convert(attributes, request.mutable_attributes()); - request.set_global_word_count(converter_.global_word_count()); + compressor_.Compress(attributes, request.mutable_attributes()); + request.set_global_word_count(compressor_.global_word_count()); request.set_deduplication_id(deduplication_id_base_ + std::to_string(deduplication_id_.fetch_add(1))); @@ -103,7 +104,7 @@ CancelFunc MixerClientImpl::Check(const Attributes &attributes, delete response; if (InvalidDictionaryStatus(status)) { - converter_.ShrinkGlobalDictionary(); + compressor_.ShrinkGlobalDictionary(); } }); } diff --git a/mixerclient/src/client_impl.h b/mixerclient/src/client_impl.h index 4069fde568d..5ff487ee955 100644 --- a/mixerclient/src/client_impl.h +++ b/mixerclient/src/client_impl.h @@ -17,7 +17,7 @@ #define MIXERCLIENT_CLIENT_IMPL_H #include "include/client.h" -#include "src/attribute_converter.h" +#include "src/attribute_compressor.h" #include "src/check_cache.h" #include "src/quota_cache.h" #include "src/report_batch.h" @@ -35,16 +35,16 @@ class MixerClientImpl : public MixerClient { // Destructor virtual ~MixerClientImpl(); - virtual CancelFunc Check(const Attributes& attributes, + virtual CancelFunc Check(const ::istio::mixer::v1::Attributes& attributes, TransportCheckFunc transport, DoneFunc on_done); - virtual void Report(const Attributes& attributes); + virtual void Report(const ::istio::mixer::v1::Attributes& attributes); private: // Store the options MixerClientOptions options_; - // To convert attributes into protobuf - AttributeConverter converter_; + // To compress attributes. + AttributeCompressor compressor_; // Cache for Check call. std::unique_ptr check_cache_; diff --git a/mixerclient/src/client_impl_test.cc b/mixerclient/src/client_impl_test.cc index 34fbf0710f4..43d784b9d17 100644 --- a/mixerclient/src/client_impl_test.cc +++ b/mixerclient/src/client_impl_test.cc @@ -16,8 +16,10 @@ #include "include/client.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "include/attributes_builder.h" #include "utils/status_test_util.h" +using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::google::protobuf::util::Status; @@ -47,8 +49,7 @@ class MockCheckTransport { class MixerClientImplTest : public ::testing::Test { public: MixerClientImplTest() { - request_.attributes[Attributes::kQuotaName] = - Attributes::StringValue(kRequestCount); + AttributesBuilder(&request_).AddString("quota.name", kRequestCount); CreateClient(true /* check_cache */, true /* quota_cache */); } @@ -78,7 +79,7 @@ TEST_F(MixerClientImplTest, TestSuccessCheck) { })); // Remove quota, not to test quota - request_.attributes.erase(Attributes::kQuotaName); + request_.mutable_attributes()->erase("quota.name"); Status done_status = Status::UNKNOWN; client_->Check(request_, empty_transport_, [&done_status](Status status) { done_status = status; }); @@ -107,7 +108,7 @@ TEST_F(MixerClientImplTest, TestPerRequestTransport) { })); // Remove quota, not to test quota - request_.attributes.erase(Attributes::kQuotaName); + request_.mutable_attributes()->erase("quota.name"); Status done_status = Status::UNKNOWN; client_->Check(request_, local_check_transport.GetFunc(), [&done_status](Status status) { done_status = status; }); diff --git a/mixerclient/src/delta_update.cc b/mixerclient/src/delta_update.cc index 9f84129f02a..a1456f682a1 100644 --- a/mixerclient/src/delta_update.cc +++ b/mixerclient/src/delta_update.cc @@ -14,8 +14,13 @@ */ #include "src/delta_update.h" +#include "google/protobuf/util/message_differencer.h" + #include +using ::istio::mixer::v1::Attributes_AttributeValue; +using ::google::protobuf::util::MessageDifferencer; + namespace istio { namespace mixer_client { namespace { @@ -30,11 +35,11 @@ class DeltaUpdateImpl : public DeltaUpdate { } } - bool Check(int index, const Attributes::Value& value) override { + bool Check(int index, const Attributes_AttributeValue& value) override { bool same = false; const auto& it = prev_map_.find(index); if (it != prev_map_.end()) { - if (it->second.type == value.type && it->second == value) { + if (MessageDifferencer::Equals(it->second, value)) { same = true; } } @@ -54,14 +59,14 @@ class DeltaUpdateImpl : public DeltaUpdate { std::set prev_set_; // The attribute map from previous. - std::map prev_map_; + std::map prev_map_; }; // An optimization for non-delta update case. class DeltaUpdateNoOpImpl : public DeltaUpdate { public: void Start() override {} - bool Check(int index, const Attributes::Value& value) override { + bool Check(int index, const Attributes_AttributeValue& value) override { return false; } bool Finish() override { return true; } diff --git a/mixerclient/src/delta_update.h b/mixerclient/src/delta_update.h index 5acb38702cf..b05d07a3509 100644 --- a/mixerclient/src/delta_update.h +++ b/mixerclient/src/delta_update.h @@ -16,7 +16,7 @@ #ifndef MIXERCLIENT_DELTA_UPDATE_H #define MIXERCLIENT_DELTA_UPDATE_H -#include "include/attribute.h" +#include "mixer/v1/attributes.pb.h" #include @@ -36,7 +36,9 @@ class DeltaUpdate { // Check an attribute, return true if it is in the previous // set with same value, so no need to send it again. // Each attribute in the current set needs to call this method. - virtual bool Check(int index, const Attributes::Value& value) = 0; + virtual bool Check( + int index, + const ::istio::mixer::v1::Attributes_AttributeValue& value) = 0; // Finish a delta update. // Return false if delta update is not supported. diff --git a/mixerclient/src/delta_update_test.cc b/mixerclient/src/delta_update_test.cc index 5a261bfdf1c..746d7de2bb9 100644 --- a/mixerclient/src/delta_update_test.cc +++ b/mixerclient/src/delta_update_test.cc @@ -16,36 +16,61 @@ #include "src/delta_update.h" #include "gtest/gtest.h" +using ::istio::mixer::v1::Attributes_AttributeValue; + namespace istio { namespace mixer_client { class DeltaUpdateTest : public ::testing::Test { public: void SetUp() { - string_map_value_ = Attributes::StringMapValue({{"foo", "bar"}}); + string_map_value_ = StringMapValue({{"foo", "bar"}}); update_ = DeltaUpdate::Create(); update_->Start(); - EXPECT_FALSE(update_->Check(1, Attributes::Int64Value(1))); - EXPECT_FALSE(update_->Check(2, Attributes::Int64Value(2))); + EXPECT_FALSE(update_->Check(1, Int64Value(1))); + EXPECT_FALSE(update_->Check(2, Int64Value(2))); EXPECT_FALSE(update_->Check(3, string_map_value_)); EXPECT_TRUE(update_->Finish()); } + ::istio::mixer::v1::Attributes_AttributeValue Int64Value(int64_t i) { + ::istio::mixer::v1::Attributes_AttributeValue v; + v.set_int64_value(i); + return v; + } + + ::istio::mixer::v1::Attributes_AttributeValue StringValue( + const std::string& str) { + ::istio::mixer::v1::Attributes_AttributeValue v; + v.set_string_value(str); + return v; + } + + ::istio::mixer::v1::Attributes_AttributeValue StringMapValue( + std::map&& string_map) { + ::istio::mixer::v1::Attributes_AttributeValue v; + auto entries = v.mutable_string_map_value()->mutable_entries(); + for (const auto& map_it : string_map) { + (*entries)[map_it.first] = map_it.second; + } + return v; + } + std::unique_ptr update_; - Attributes::Value string_map_value_; + Attributes_AttributeValue string_map_value_; }; TEST_F(DeltaUpdateTest, TestUpdateNoDelete) { update_->Start(); // 1: value is the same. - EXPECT_TRUE(update_->Check(1, Attributes::Int64Value(1))); + EXPECT_TRUE(update_->Check(1, Int64Value(1))); // 2: value is different. - EXPECT_FALSE(update_->Check(2, Attributes::Int64Value(3))); + EXPECT_FALSE(update_->Check(2, Int64Value(3))); // 3: compare string map. EXPECT_TRUE(update_->Check(3, string_map_value_)); // 4: an new attribute. - EXPECT_FALSE(update_->Check(4, Attributes::Int64Value(4))); + EXPECT_FALSE(update_->Check(4, Int64Value(4))); // No missing item EXPECT_TRUE(update_->Finish()); } @@ -53,15 +78,15 @@ TEST_F(DeltaUpdateTest, TestUpdateNoDelete) { TEST_F(DeltaUpdateTest, TestUpdateWithDelete) { update_->Start(); // 1: value is the same. - EXPECT_TRUE(update_->Check(1, Attributes::Int64Value(1))); + EXPECT_TRUE(update_->Check(1, Int64Value(1))); // 2: is missing // 3: compare string map - EXPECT_FALSE(update_->Check(3, Attributes::StringMapValue({}))); + EXPECT_FALSE(update_->Check(3, StringMapValue({}))); // 4: an new attribute. - EXPECT_FALSE(update_->Check(4, Attributes::Int64Value(4))); + EXPECT_FALSE(update_->Check(4, Int64Value(4))); // There is a missing item EXPECT_FALSE(update_->Finish()); @@ -70,7 +95,7 @@ TEST_F(DeltaUpdateTest, TestUpdateWithDelete) { TEST_F(DeltaUpdateTest, TestDifferentType) { update_->Start(); // 1 is differnt type. - EXPECT_FALSE(update_->Check(1, Attributes::StringValue(""))); + EXPECT_FALSE(update_->Check(1, StringValue(""))); } } // namespace mixer_client diff --git a/mixerclient/src/quota_cache.cc b/mixerclient/src/quota_cache.cc index 23fb8e353f5..23ee90d3f51 100644 --- a/mixerclient/src/quota_cache.cc +++ b/mixerclient/src/quota_cache.cc @@ -17,6 +17,8 @@ #include "utils/protobuf.h" using namespace std::chrono; +using ::istio::mixer::v1::Attributes; +using ::istio::mixer::v1::Attributes_AttributeValue; using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::google::protobuf::util::Status; @@ -24,6 +26,10 @@ using ::google::protobuf::util::error::Code; namespace istio { namespace mixer_client { +namespace { +const std::string kQuotaName = "quota.name"; +const std::string kQuotaAmount = "quota.amount"; +} QuotaCache::CacheElem::CacheElem(const std::string& name) : name_(name) { prefetch_ = QuotaPrefetch::Create( @@ -241,22 +247,25 @@ void QuotaCache::Check(const Attributes& request, bool use_cache, // Now, there is only one quota metric for a request. // But it should be very easy to support multiple quota metrics. static const std::vector> - kQuotaAttributes{{Attributes::kQuotaName, Attributes::kQuotaAmount}}; + kQuotaAttributes{{kQuotaName, kQuotaAmount}}; + const auto& attributes_map = request.attributes(); for (const auto& pair : kQuotaAttributes) { const std::string& name_attr = pair.first; const std::string& amount_attr = pair.second; - const auto& name_it = request.attributes.find(name_attr); - if (name_it == request.attributes.end() || - name_it->second.type != Attributes::Value::STRING) { + const auto& name_it = attributes_map.find(name_attr); + if (name_it == attributes_map.end() || + name_it->second.value_case() != + Attributes_AttributeValue::kStringValue) { continue; } CheckResult::Quota quota; - quota.name = name_it->second.str_v; + quota.name = name_it->second.string_value(); quota.amount = 1; - const auto& amount_it = request.attributes.find(amount_attr); - if (amount_it != request.attributes.end() && - amount_it->second.type == Attributes::Value::INT64) { - quota.amount = amount_it->second.value.int64_v; + const auto& amount_it = attributes_map.find(amount_attr); + if (amount_it != attributes_map.end() && + amount_it->second.value_case() == + Attributes_AttributeValue::kInt64Value) { + quota.amount = amount_it->second.int64_value(); } CheckCache(request, use_cache, "a); result->quotas_.push_back(quota); diff --git a/mixerclient/src/quota_cache.h b/mixerclient/src/quota_cache.h index 3d5beaf103a..c200f8c5255 100644 --- a/mixerclient/src/quota_cache.h +++ b/mixerclient/src/quota_cache.h @@ -24,7 +24,6 @@ #include "include/client.h" #include "prefetch/quota_prefetch.h" -#include "src/attribute_converter.h" #include "src/referenced.h" #include "utils/simple_lru_cache.h" #include "utils/simple_lru_cache_inl.h" @@ -61,7 +60,7 @@ class QuotaCache { ::google::protobuf::util::Status status() const { return status_; } void SetResponse(const ::google::protobuf::util::Status& status, - const Attributes& attributes, + const ::istio::mixer::v1::Attributes& attributes, const ::istio::mixer::v1::CheckResponse& response); private: @@ -81,7 +80,7 @@ class QuotaCache { // The function to set the quota response from server. using OnResponseFunc = std::function; OnResponseFunc response_func; }; @@ -93,11 +92,12 @@ class QuotaCache { }; // Check quota cache for a request, result will be stored in CacaheResult. - void Check(const Attributes& request, bool use_cache, CheckResult* result); + void Check(const ::istio::mixer::v1::Attributes& request, bool use_cache, + CheckResult* result); private: // Check quota cache. - void CheckCache(const Attributes& request, bool use_cache, + void CheckCache(const ::istio::mixer::v1::Attributes& request, bool use_cache, CheckResult::Quota* quota); // Invalidates expired check responses. @@ -144,7 +144,8 @@ class QuotaCache { // Set a quota response. void SetResponse( - const Attributes& attributes, const std::string& quota_name, + const ::istio::mixer::v1::Attributes& attributes, + const std::string& quota_name, const ::istio::mixer::v1::CheckResponse::QuotaResult* result); // A map from quota name to PerQuotaReferenced. diff --git a/mixerclient/src/quota_cache_test.cc b/mixerclient/src/quota_cache_test.cc index 201aae66a3b..e34a1b1690d 100644 --- a/mixerclient/src/quota_cache_test.cc +++ b/mixerclient/src/quota_cache_test.cc @@ -17,8 +17,10 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "include/attributes_builder.h" #include "utils/status_test_util.h" +using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::CheckRequest; using ::istio::mixer::v1::CheckResponse; using ::istio::mixer::v1::ReferencedAttributes; @@ -40,8 +42,7 @@ class QuotaCacheTest : public ::testing::Test { cache_ = std::unique_ptr(new QuotaCache(options)); ASSERT_TRUE((bool)(cache_)); - request_.attributes[Attributes::kQuotaName] = - Attributes::StringValue(kQuotaName); + AttributesBuilder(&request_).AddString("quota.name", kQuotaName); } void TestRequest(const Attributes& request, bool pass, @@ -193,11 +194,12 @@ TEST_F(QuotaCacheTest, TestInvalidQuotaReferenced) { (*response.mutable_quotas())[kQuotaName] = quota_result; Attributes attr(request_); - attr.attributes["source.name"] = Attributes::StringValue("user1"); + AttributesBuilder builder(&attr); + builder.AddString("source.name", "user1"); // response has invalid referenced, cache item still in pending. TestRequest(attr, true, response); - attr.attributes["source.name"] = Attributes::StringValue("user2"); + builder.AddString("source.name", "user2"); // it is a cache miss, use pending request. // Previous request has used up token, this request will be rejected. TestRequest(attr, false, response); @@ -219,13 +221,14 @@ TEST_F(QuotaCacheTest, TestMismatchedReferenced) { (*response.mutable_quotas())[kQuotaName] = quota_result; Attributes attr(request_); - attr.attributes["source.name"] = Attributes::StringValue("user1"); + AttributesBuilder builder(&attr); + builder.AddString("source.name", "user1"); // Since respones has mismatched Referenced, cache item still in pending. // Prefetch always allow the first call. TestRequest(attr, true, response); // second request with different users still use the pending request. - attr.attributes["source.name"] = Attributes::StringValue("user2"); + builder.AddString("source.name", "user2"); // it is a cache miss, use pending request. // Previous request has used up token, this request will be rejected. TestRequest(attr, false, response); @@ -245,9 +248,9 @@ TEST_F(QuotaCacheTest, TestOneReferencedWithTwoKeys) { (*response.mutable_quotas())[kQuotaName] = quota_result; Attributes attr1(request_); - attr1.attributes["source.name"] = Attributes::StringValue("user1"); + AttributesBuilder(&attr1).AddString("source.name", "user1"); Attributes attr2(request_); - attr2.attributes["source.name"] = Attributes::StringValue("user2"); + AttributesBuilder(&attr2).AddString("source.name", "user2"); // cache item is updated with 0 token in the pool. // it will be saved into cache key with user1. @@ -286,9 +289,9 @@ TEST_F(QuotaCacheTest, TestTwoReferencedWith) { (*response2.mutable_quotas())[kQuotaName] = quota_result2; Attributes attr1(request_); - attr1.attributes["source.name"] = Attributes::StringValue("name"); + AttributesBuilder(&attr1).AddString("source.name", "name"); Attributes attr2(request_); - attr2.attributes["source.uid"] = Attributes::StringValue("uid"); + AttributesBuilder(&attr2).AddString("source.uid", "uid"); // name request with 0 granted response TestRequest(attr1, true, response1); diff --git a/mixerclient/src/referenced.cc b/mixerclient/src/referenced.cc index 1a7d12ef023..06458e9bc1e 100644 --- a/mixerclient/src/referenced.cc +++ b/mixerclient/src/referenced.cc @@ -18,9 +18,12 @@ #include "global_dictionary.h" #include "utils/md5.h" +#include #include #include +using ::istio::mixer::v1::Attributes; +using ::istio::mixer::v1::Attributes_AttributeValue; using ::istio::mixer::v1::ReferencedAttributes; namespace istio { @@ -76,56 +79,74 @@ bool Referenced::Fill(const ReferencedAttributes& reference) { bool Referenced::Signature(const Attributes& attributes, const std::string& extra_key, std::string* signature) const { + const auto& attributes_map = attributes.attributes(); for (const std::string& key : absence_keys_) { // if an "absence" key exists, return false for mis-match. - if (attributes.attributes.find(key) != attributes.attributes.end()) { + if (attributes_map.find(key) != attributes_map.end()) { return false; } } MD5 hasher; for (const std::string& key : exact_keys_) { - const auto it = attributes.attributes.find(key); + const auto it = attributes_map.find(key); // if an "exact" attribute not present, return false for mismatch. - if (it == attributes.attributes.end()) { + if (it == attributes_map.end()) { return false; } hasher.Update(it->first); hasher.Update(kDelimiter, kDelimiterLength); - switch (it->second.type) { - case Attributes::Value::ValueType::STRING: - hasher.Update(it->second.str_v); - break; - case Attributes::Value::ValueType::BYTES: - hasher.Update(it->second.str_v); - break; - case Attributes::Value::ValueType::INT64: - hasher.Update(&it->second.value.int64_v, - sizeof(it->second.value.int64_v)); - break; - case Attributes::Value::ValueType::DOUBLE: - hasher.Update(&it->second.value.double_v, - sizeof(it->second.value.double_v)); - break; - case Attributes::Value::ValueType::BOOL: - hasher.Update(&it->second.value.bool_v, - sizeof(it->second.value.bool_v)); - break; - case Attributes::Value::ValueType::TIME: - hasher.Update(&it->second.time_v, sizeof(it->second.time_v)); + + const Attributes_AttributeValue& value = it->second; + switch (value.value_case()) { + case Attributes_AttributeValue::kStringValue: + hasher.Update(value.string_value()); break; - case Attributes::Value::ValueType::DURATION: - hasher.Update(&it->second.duration_nanos_v, - sizeof(it->second.duration_nanos_v)); + case Attributes_AttributeValue::kBytesValue: + hasher.Update(value.bytes_value()); break; - case Attributes::Value::ValueType::STRING_MAP: - for (const auto& sub_it : it->second.string_map_v) { + case Attributes_AttributeValue::kInt64Value: { + auto data = value.int64_value(); + hasher.Update(&data, sizeof(data)); + } break; + case Attributes_AttributeValue::kDoubleValue: { + auto data = value.double_value(); + hasher.Update(&data, sizeof(data)); + } break; + case Attributes_AttributeValue::kBoolValue: { + auto data = value.bool_value(); + hasher.Update(&data, sizeof(data)); + } break; + case Attributes_AttributeValue::kTimestampValue: { + auto seconds = value.timestamp_value().seconds(); + auto nanos = value.timestamp_value().nanos(); + hasher.Update(&seconds, sizeof(seconds)); + hasher.Update(kDelimiter, kDelimiterLength); + hasher.Update(&nanos, sizeof(nanos)); + } break; + case Attributes_AttributeValue::kDurationValue: { + auto seconds = value.duration_value().seconds(); + auto nanos = value.duration_value().nanos(); + hasher.Update(&seconds, sizeof(seconds)); + hasher.Update(kDelimiter, kDelimiterLength); + hasher.Update(&nanos, sizeof(nanos)); + } break; + case Attributes_AttributeValue::kStringMapValue: { + // protobuf map doesn't have deterministic order: + // each run the order is different. + // copy to std::map with deterministic order. + const auto& pb_map = value.string_map_value().entries(); + std::map stl_map(pb_map.begin(), + pb_map.end()); + for (const auto& sub_it : stl_map) { hasher.Update(sub_it.first); hasher.Update(kDelimiter, kDelimiterLength); hasher.Update(sub_it.second); hasher.Update(kDelimiter, kDelimiterLength); } + } break; + case Attributes_AttributeValue::VALUE_NOT_SET: break; } hasher.Update(kDelimiter, kDelimiterLength); diff --git a/mixerclient/src/referenced.h b/mixerclient/src/referenced.h index 87f681624cd..b10a1e84b8f 100644 --- a/mixerclient/src/referenced.h +++ b/mixerclient/src/referenced.h @@ -18,7 +18,6 @@ #include -#include "include/attribute.h" #include "mixer/v1/check.pb.h" namespace istio { @@ -38,8 +37,8 @@ class Referenced { // Return false if attributes are mismatched, such as "absence" attributes // present // or "exact" match attributes don't present. - bool Signature(const Attributes& attributes, const std::string& extra_key, - std::string* signature) const; + bool Signature(const ::istio::mixer::v1::Attributes& attributes, + const std::string& extra_key, std::string* signature) const; // A hash value to identify an instance. std::string Hash() const; diff --git a/mixerclient/src/referenced_test.cc b/mixerclient/src/referenced_test.cc index f0f6c1b0305..c47cd731179 100644 --- a/mixerclient/src/referenced_test.cc +++ b/mixerclient/src/referenced_test.cc @@ -14,11 +14,14 @@ */ #include "src/referenced.h" + +#include "include/attributes_builder.h" #include "utils/md5.h" #include "google/protobuf/text_format.h" #include "gtest/gtest.h" +using ::istio::mixer::v1::Attributes; using ::google::protobuf::TextFormat; namespace istio { @@ -136,12 +139,12 @@ TEST(ReferencedTest, NegativeSignature1Test) { Attributes attributes1; // "target.service" should be absence. - attributes1.attributes["target.service"] = Attributes::StringValue("foo"); + AttributesBuilder(&attributes1).AddString("target.service", "foo"); EXPECT_FALSE(referenced.Signature(attributes1, "", &signature)); Attributes attributes2; // many keys should exist. - attributes2.attributes["bytes-key"] = Attributes::StringValue("foo"); + AttributesBuilder(&attributes2).AddString("bytes-key", "foo"); EXPECT_FALSE(referenced.Signature(attributes2, "", &signature)); } @@ -152,29 +155,28 @@ TEST(ReferencedTest, OKSignature1Test) { EXPECT_TRUE(referenced.Fill(pb)); Attributes attributes; - attributes.attributes["string-key"] = - Attributes::StringValue("this is a string value"); - attributes.attributes["bytes-key"] = - Attributes::BytesValue("this is a bytes value"); - attributes.attributes["double-key"] = Attributes::DoubleValue(99.9); - attributes.attributes["int-key"] = Attributes::Int64Value(35); - attributes.attributes["bool-key"] = Attributes::BoolValue(true); + AttributesBuilder builder(&attributes); + builder.AddString("string-key", "this is a string value"); + builder.AddBytes("bytes-key", "this is a bytes value"); + builder.AddDouble("double-key", 99.9); + builder.AddInt64("int-key", 35); + builder.AddBool("bool-key", true); std::chrono::time_point time0; - attributes.attributes["time-key"] = Attributes::TimeValue(time0); std::chrono::seconds secs(5); - attributes.attributes["duration-key"] = Attributes::DurationValue( + builder.AddTimestamp("time-key", time0); + builder.AddDuration( + "duration-key", std::chrono::duration_cast(secs)); std::map string_map = {{"key1", "value1"}, {"key2", "value2"}}; - attributes.attributes["string-map-key"] = - Attributes::StringMapValue(std::move(string_map)); + builder.AddStringMap("string-map-key", std::move(string_map)); std::string signature; EXPECT_TRUE(referenced.Signature(attributes, "extra", &signature)); - EXPECT_EQ(MD5::DebugString(signature), "cfcf5f20f58a44da8832456742bf7b88"); + EXPECT_EQ(MD5::DebugString(signature), "cd3ee7fbe836b973f84f5075ef4ac29d"); } } // namespace diff --git a/mixerclient/src/report_batch.cc b/mixerclient/src/report_batch.cc index b129ca0a153..d893b01f3db 100644 --- a/mixerclient/src/report_batch.cc +++ b/mixerclient/src/report_batch.cc @@ -16,6 +16,7 @@ #include "src/report_batch.h" #include "utils/protobuf.h" +using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; using ::google::protobuf::util::Status; @@ -27,31 +28,31 @@ namespace mixer_client { ReportBatch::ReportBatch(const ReportOptions& options, TransportReportFunc transport, TimerCreateFunc timer_create, - AttributeConverter& converter) + AttributeCompressor& compressor) : options_(options), transport_(transport), timer_create_(timer_create), - converter_(converter) {} + compressor_(compressor) {} ReportBatch::~ReportBatch() { Flush(); } void ReportBatch::Report(const Attributes& request) { std::lock_guard lock(mutex_); - if (!batch_converter_) { - batch_converter_ = converter_.CreateBatchConverter(); + if (!batch_compressor_) { + batch_compressor_ = compressor_.CreateBatchCompressor(); } - if (!batch_converter_->Add(request)) { + if (!batch_compressor_->Add(request)) { FlushWithLock(); - batch_converter_ = converter_.CreateBatchConverter(); - batch_converter_->Add(request); + batch_compressor_ = compressor_.CreateBatchCompressor(); + batch_compressor_->Add(request); } - if (batch_converter_->size() >= options_.max_batch_entries) { + if (batch_compressor_->size() >= options_.max_batch_entries) { FlushWithLock(); } else { - if (batch_converter_->size() == 1 && timer_create_) { + if (batch_compressor_->size() == 1 && timer_create_) { if (!timer_) { timer_ = timer_create_([this]() { Flush(); }); } @@ -61,12 +62,12 @@ void ReportBatch::Report(const Attributes& request) { } void ReportBatch::FlushWithLock() { - if (!batch_converter_) { + if (!batch_compressor_) { return; } - std::unique_ptr request = batch_converter_->Finish(); - batch_converter_.reset(); + std::unique_ptr request = batch_compressor_->Finish(); + batch_compressor_.reset(); if (timer_) { timer_->Stop(); } @@ -77,7 +78,7 @@ void ReportBatch::FlushWithLock() { if (!status.ok()) { GOOGLE_LOG(ERROR) << "Mixer Report failed with: " << status.ToString(); if (InvalidDictionaryStatus(status)) { - converter_.ShrinkGlobalDictionary(); + compressor_.ShrinkGlobalDictionary(); } } }); diff --git a/mixerclient/src/report_batch.h b/mixerclient/src/report_batch.h index d65cb688dcb..035e8ecfd7b 100644 --- a/mixerclient/src/report_batch.h +++ b/mixerclient/src/report_batch.h @@ -19,7 +19,7 @@ #include #include "include/client.h" -#include "src/attribute_converter.h" +#include "src/attribute_compressor.h" namespace istio { namespace mixer_client { @@ -28,12 +28,12 @@ namespace mixer_client { class ReportBatch { public: ReportBatch(const ReportOptions& options, TransportReportFunc transport, - TimerCreateFunc timer_create, AttributeConverter& converter); + TimerCreateFunc timer_create, AttributeCompressor& compressor); virtual ~ReportBatch(); // Make batched report call - void Report(const Attributes& request); + void Report(const ::istio::mixer::v1::Attributes& request); // Flush out batched reports. void Flush(); @@ -50,8 +50,8 @@ class ReportBatch { // timer create func TimerCreateFunc timer_create_; - // Attribute converter. - AttributeConverter& converter_; + // Attribute compressor. + AttributeCompressor& compressor_; // Mutex guarding the access of batch data; std::mutex mutex_; @@ -59,8 +59,8 @@ class ReportBatch { // timer to flush out batched data. std::unique_ptr timer_; - // batched report converter - std::unique_ptr batch_converter_; + // batched report compressor + std::unique_ptr batch_compressor_; GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ReportBatch); }; diff --git a/mixerclient/src/report_batch_test.cc b/mixerclient/src/report_batch_test.cc index b2028eb851b..c3517908d3a 100644 --- a/mixerclient/src/report_batch_test.cc +++ b/mixerclient/src/report_batch_test.cc @@ -16,7 +16,9 @@ #include "src/report_batch.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "include/attributes_builder.h" +using ::istio::mixer::v1::Attributes; using ::istio::mixer::v1::ReportRequest; using ::istio::mixer::v1::ReportResponse; using ::google::protobuf::util::Status; @@ -49,10 +51,10 @@ class MockTimer : public Timer { class ReportBatchTest : public ::testing::Test { public: - ReportBatchTest() : mock_timer_(nullptr), converter_({}) { + ReportBatchTest() : mock_timer_(nullptr), compressor_({}) { batch_.reset(new ReportBatch(ReportOptions(3, 1000), mock_report_transport_.GetFunc(), - GetTimerFunc(), converter_)); + GetTimerFunc(), compressor_)); } TimerCreateFunc GetTimerFunc() { @@ -65,7 +67,7 @@ class ReportBatchTest : public ::testing::Test { MockReportTransport mock_report_transport_; MockTimer* mock_timer_; - AttributeConverter converter_; + AttributeCompressor compressor_; std::unique_ptr batch_; }; @@ -73,7 +75,7 @@ TEST_F(ReportBatchTest, TestBatchDisabled) { // max_batch_entries = 0 or 1 to disable batch batch_.reset(new ReportBatch(ReportOptions(1, 1000), mock_report_transport_.GetFunc(), nullptr, - converter_)); + compressor_)); // Expect report transport to be called. EXPECT_CALL(mock_report_transport_, Report(_, _, _)) @@ -114,12 +116,12 @@ TEST_F(ReportBatchTest, TestNoDeltaUpdate) { })); Attributes report; - report.attributes["key"] = Attributes::StringValue("value"); + AttributesBuilder(&report).AddString("key", "value"); batch_->Report(report); EXPECT_EQ(report_call_count, 0); // Erase a key, so delta update fail to push the batched result. - report.attributes.erase("key"); + report.mutable_attributes()->erase("key"); batch_->Report(report); EXPECT_EQ(report_call_count, 1);