PolicyManager: New EvaluationContext class.
The EvaluationContext class handles the life of a policy request
evaluation caching all the results and tracking the variables used
by a the policy implementation.
This patch adds the first part of the EvaluationContex class with
its public interface and minimal support.
BUG=chromium:338590
TEST=Unit test added and passing.
Change-Id: I70e04e7e10ea30ddfb887b494b2b40557565b2da
Reviewed-on: https://chromium-review.googlesource.com/185106
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/policy_manager/boxed_value.h b/policy_manager/boxed_value.h
new file mode 100644
index 0000000..f0b1ec2
--- /dev/null
+++ b/policy_manager/boxed_value.h
@@ -0,0 +1,90 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_BOXED_VALUE_H
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_BOXED_VALUE_H
+
+#include <base/basictypes.h>
+
+namespace chromeos_policy_manager {
+
+// BoxedValue is a class to hold pointers of a given type that deletes them when
+// the instance goes out of scope, as scoped_ptr<T> does. The main difference
+// with it is that the type T is not part of the class, i.e., this isn't a
+// parametric class. The class has a parametric contructor that accepts a
+// const T* which will define the type of the object passed on delete.
+//
+// It is safe to use this class in linked containers such as std::list and
+// std::map but the object can't be copied. This means that you need to
+// construct the BoxedValue inplace using a container method like emplace()
+// or move it with std::move().
+//
+// list<BoxedValue> lst;
+// lst.emplace_back(new const int(42));
+// lst.emplace_back(new const string("Hello world!"));
+//
+// map<int, BoxedValue> m;
+// m.emplace(123, std::move(BoxedValue(new const string("Hola mundo!"))));
+//
+// auto it = m.find(42);
+// if (it != m.end())
+// cout << "m[42] points to " << it->second.value() << endl;
+// cout << "m[33] points to " << m[33].value() << endl;
+//
+// Since copy and assign are not allowed, you can't create a copy of the
+// BoxedValue which means that you can only use a reference to it.
+//
+
+class BoxedValue {
+ public:
+ // Creates an empty BoxedValue. Since the pointer can't be assigned from other
+ // BoxedValues or pointers, this is only useful in places where a default
+ // constructor is required, such as std::map::operator[].
+ BoxedValue() : value_(NULL), deleter_(NULL) {}
+
+ // Creates a BoxedValue for the passed pointer |value|. The BoxedValue keeps
+ // the ownership of this pointer and can't be released.
+ template<typename T>
+ explicit BoxedValue(const T* value)
+ : value_(static_cast<const void*>(value)), deleter_(ValueDeleter<T>) {}
+
+ // The move constructor takes ownership of the pointer since the semantics of
+ // it allows to render the passed BoxedValue undefined. You need to use the
+ // move constructor explictly preventing it from accidental references,
+ // like in:
+ // BoxedValue new_box(std::move(other_box));
+ BoxedValue(BoxedValue&& other)
+ : value_(other.value_), deleter_(other.deleter_) {
+ other.value_ = NULL;
+ other.deleter_ = NULL;
+ }
+
+ // Deletes the |value| passed on construction using the delete for the passed
+ // type.
+ ~BoxedValue() {
+ if (deleter_)
+ deleter_(value_);
+ }
+
+ const void* value() const { return value_; }
+
+ // Static method to call the destructor of the right type.
+ template<typename T>
+ static void ValueDeleter(const void* value) {
+ delete reinterpret_cast<const T*>(value);
+ }
+
+ private:
+ // A pointer to the cached value.
+ const void* value_;
+
+ // A function that calls delete for the right type of value_.
+ void (*deleter_)(const void*);
+
+ DISALLOW_COPY_AND_ASSIGN(BoxedValue);
+};
+
+} // namespace chromeos_policy_manager
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_BOXED_VALUE_H
diff --git a/policy_manager/boxed_value_unittest.cc b/policy_manager/boxed_value_unittest.cc
new file mode 100644
index 0000000..3d56257
--- /dev/null
+++ b/policy_manager/boxed_value_unittest.cc
@@ -0,0 +1,78 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <gtest/gtest.h>
+#include <list>
+#include <map>
+#include <string>
+
+#include "update_engine/policy_manager/boxed_value.h"
+#include "update_engine/policy_manager/pmtest_utils.h"
+
+using std::list;
+using std::map;
+using std::string;
+
+namespace chromeos_policy_manager {
+
+// The DeleterMarker flags a bool variable when the class is destroyed.
+class DeleterMarker {
+ public:
+ DeleterMarker(bool* marker) : marker_(marker) { *marker_ = false; }
+
+ ~DeleterMarker() { *marker_ = true; }
+
+ private:
+ // Pointer to the bool marker.
+ bool* marker_;
+};
+
+TEST(PmBoxedValueTest, Deleted) {
+ bool marker = true;
+ const DeleterMarker* deleter_marker = new DeleterMarker(&marker);
+
+ EXPECT_FALSE(marker);
+ BoxedValue* box = new BoxedValue(deleter_marker);
+ EXPECT_FALSE(marker);
+ delete box;
+ EXPECT_TRUE(marker);
+}
+
+TEST(PmBoxedValueTest, MoveConstructor) {
+ bool marker = true;
+ const DeleterMarker* deleter_marker = new DeleterMarker(&marker);
+
+ BoxedValue* box = new BoxedValue(deleter_marker);
+ BoxedValue* new_box = new BoxedValue(std::move(*box));
+ // box is now undefined but valid.
+ delete box;
+ EXPECT_FALSE(marker);
+ // The deleter_marker gets deleted at this point.
+ delete new_box;
+ EXPECT_TRUE(marker);
+}
+
+TEST(PmBoxedValueTest, MixedList) {
+ list<BoxedValue> lst;
+ // This is mostly a compile test.
+ lst.emplace_back(new const int(42));
+ lst.emplace_back(new const string("Hello world!"));
+ bool marker;
+ lst.emplace_back(new const DeleterMarker(&marker));
+ EXPECT_FALSE(marker);
+ lst.clear();
+ EXPECT_TRUE(marker);
+}
+
+TEST(PmBoxedValueTest, MixedMap) {
+ map<int, BoxedValue> m;
+ m.emplace(42, std::move(BoxedValue(new const string("Hola mundo!"))));
+
+ auto it = m.find(42);
+ ASSERT_NE(it, m.end());
+ PMTEST_EXPECT_NOT_NULL(it->second.value());
+ PMTEST_EXPECT_NULL(m[33].value());
+}
+
+} // namespace chromeos_policy_manager
diff --git a/policy_manager/evaluation_context-inl.h b/policy_manager/evaluation_context-inl.h
new file mode 100644
index 0000000..1413059
--- /dev/null
+++ b/policy_manager/evaluation_context-inl.h
@@ -0,0 +1,42 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_EVALUATION_CONTEXT_INL_H
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_EVALUATION_CONTEXT_INL_H
+
+#include <base/logging.h>
+
+namespace chromeos_policy_manager {
+
+template<typename T>
+const T* EvaluationContext::GetValue(Variable<T>* var) {
+ if (var == NULL) {
+ LOG(ERROR) << "GetValue received an uninitialized variable.";
+ return NULL;
+ }
+
+ // Search for the value on the cache first.
+ ValueCacheMap::iterator it = value_cache_.find(var);
+ if (it != value_cache_.end())
+ return reinterpret_cast<const T*>(it->second.value());
+
+ // Get the value from the variable if not found on the cache.
+ std::string errmsg;
+ const T* result = var->GetValue(RemainingTime(), &errmsg);
+ if (result == NULL) {
+ LOG(WARNING) << "Error reading Variable " << var->GetName() << ": \""
+ << errmsg << "\"";
+ } else {
+ // Cache the value for the next time. The map of CachedValues keeps the
+ // ownership of the pointer until the map is destroyed.
+ value_cache_.emplace(
+ static_cast<BaseVariable*>(var),
+ std::move(BoxedValue(result)));
+ }
+ return result;
+}
+
+} // namespace chromeos_policy_manager
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_EVALUATION_CONTEXT_INL_H
diff --git a/policy_manager/evaluation_context.cc b/policy_manager/evaluation_context.cc
new file mode 100644
index 0000000..062ed26
--- /dev/null
+++ b/policy_manager/evaluation_context.cc
@@ -0,0 +1,17 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/policy_manager/evaluation_context.h"
+
+using base::TimeDelta;
+
+namespace chromeos_policy_manager {
+
+TimeDelta EvaluationContext::RemainingTime() const {
+ // TODO(deymo): Return a timeout based on the elapsed time on the current
+ // policy request evaluation.
+ return TimeDelta::FromSeconds(1.);
+}
+
+} // namespace chromeos_policy_manager
diff --git a/policy_manager/evaluation_context.h b/policy_manager/evaluation_context.h
new file mode 100644
index 0000000..3cbd614
--- /dev/null
+++ b/policy_manager/evaluation_context.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_EVALUATION_CONTEXT_H
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_EVALUATION_CONTEXT_H
+
+#include <map>
+
+#include "update_engine/policy_manager/variable.h"
+#include "update_engine/policy_manager/boxed_value.h"
+
+namespace chromeos_policy_manager {
+
+// The EvaluationContext class is the interface between a policy implementation
+// and the state. The EvaluationContext tracks the variables used by a policy
+// request and caches the returned values, owning those cached values.
+class EvaluationContext {
+ public:
+ EvaluationContext() {}
+
+ // Returns a pointer to the value returned by the passed variable |var|. The
+ // EvaluationContext instance keeps the ownership of the returned object. The
+ // returned object is valid during the life of the EvaluationContext, even if
+ // the passed Variable changes it.
+ //
+ // In case of error, a NULL value is returned.
+ template<typename T>
+ const T* GetValue(Variable<T>* var);
+
+ private:
+ // The remaining time for the current evaluation.
+ base::TimeDelta RemainingTime() const;
+
+ // A map to hold the cached values for every variable.
+ typedef std::map<BaseVariable*, BoxedValue> ValueCacheMap;
+
+ // The cached values of the called Variables.
+ ValueCacheMap value_cache_;
+
+ DISALLOW_COPY_AND_ASSIGN(EvaluationContext);
+};
+
+} // namespace chromeos_policy_manager
+
+// Include the implementation of the template methods.
+#include "update_engine/policy_manager/evaluation_context-inl.h"
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_POLICY_MANAGER_EVALUATION_CONTEXT_H
diff --git a/policy_manager/evaluation_context_unittest.cc b/policy_manager/evaluation_context_unittest.cc
new file mode 100644
index 0000000..9c50446
--- /dev/null
+++ b/policy_manager/evaluation_context_unittest.cc
@@ -0,0 +1,96 @@
+// Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <base/memory/scoped_ptr.h>
+#include <gtest/gtest.h>
+#include <string>
+
+#include "update_engine/policy_manager/evaluation_context.h"
+#include "update_engine/policy_manager/generic_variables.h"
+#include "update_engine/policy_manager/pmtest_utils.h"
+#include "update_engine/policy_manager/fake_variable.h"
+
+using std::string;
+
+namespace chromeos_policy_manager {
+
+class PmEvaluationContextTest : public ::testing::Test {
+ public:
+ PmEvaluationContextTest() : fake_int_var_("fake_int") {}
+
+ protected:
+ virtual void SetUp() {
+ eval_ctx_.reset(new EvaluationContext());
+ }
+
+ scoped_ptr<EvaluationContext> eval_ctx_;
+ FakeVariable<int> fake_int_var_;
+};
+
+TEST_F(PmEvaluationContextTest, GetValueFails) {
+ // FakeVariable is initialized as returning NULL.
+ PMTEST_EXPECT_NULL(eval_ctx_->GetValue(&fake_int_var_));
+}
+
+TEST_F(PmEvaluationContextTest, GetValueFailsWithInvalidVar) {
+ PMTEST_EXPECT_NULL(eval_ctx_->GetValue(
+ reinterpret_cast<Variable<int>*>(NULL)));
+}
+
+TEST_F(PmEvaluationContextTest, GetValueReturns) {
+ const int* p_fake_int;
+
+ fake_int_var_.reset(new int(42));
+ p_fake_int = eval_ctx_->GetValue(&fake_int_var_);
+ PMTEST_ASSERT_NOT_NULL(p_fake_int);
+ EXPECT_EQ(42, *p_fake_int);
+}
+
+TEST_F(PmEvaluationContextTest, GetValueCached) {
+ const int* p_fake_int;
+
+ fake_int_var_.reset(new int(42));
+ p_fake_int = eval_ctx_->GetValue(&fake_int_var_);
+
+ // Check that if the variable changes, the EvaluationContext keeps returning
+ // the cached value.
+ fake_int_var_.reset(new int(5));
+
+ p_fake_int = eval_ctx_->GetValue(&fake_int_var_);
+ PMTEST_ASSERT_NOT_NULL(p_fake_int);
+ EXPECT_EQ(42, *p_fake_int);
+}
+
+TEST_F(PmEvaluationContextTest, GetValueDontCacheNULL) {
+ const int* p_fake_int = eval_ctx_->GetValue(&fake_int_var_);
+ PMTEST_EXPECT_NULL(p_fake_int);
+
+ fake_int_var_.reset(new int(42));
+ // A second attempt to read the variable should work even on the same
+ // EvaluationContext.
+ p_fake_int = eval_ctx_->GetValue(&fake_int_var_);
+ PMTEST_ASSERT_NOT_NULL(p_fake_int);
+ EXPECT_EQ(42, *p_fake_int);
+}
+
+TEST_F(PmEvaluationContextTest, GetValueMixedTypes) {
+ FakeVariable<string> fake_string_var_("fake_string");
+ const int* p_fake_int;
+ const string* p_fake_string;
+
+ fake_int_var_.reset(new int(42));
+ fake_string_var_.reset(new string("Hello world!"));
+ // Check that the EvaluationContext can handle multiple Variable types. This
+ // is mostly a compile-time check due to the template nature of this method.
+ p_fake_int = eval_ctx_->GetValue(&fake_int_var_);
+ p_fake_string = eval_ctx_->GetValue(&fake_string_var_);
+
+ PMTEST_ASSERT_NOT_NULL(p_fake_int);
+ EXPECT_EQ(42, *p_fake_int);
+
+ PMTEST_ASSERT_NOT_NULL(p_fake_string);
+ EXPECT_EQ("Hello world!", *p_fake_string);
+}
+
+} // namespace chromeos_policy_manager
diff --git a/policy_manager/variable.h b/policy_manager/variable.h
index 7dcb5b9..29fa546 100644
--- a/policy_manager/variable.h
+++ b/policy_manager/variable.h
@@ -39,6 +39,10 @@
virtual ~Variable() {}
protected:
+ // Only allow to get values through the EvaluationContext class and not
+ // directly from the variable.
+ friend class EvaluationContext;
+
friend class PmRealRandomProviderTest;
FRIEND_TEST(PmRealRandomProviderTest, GetRandomValues);