[ https://issues.apache.org/jira/browse/MESOS-3072?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gavin updated MESOS-3072: ------------------------- Comment: was deleted (was: www.rtat.net) > Unify initialization of modularized components > ---------------------------------------------- > > Key: MESOS-3072 > URL: https://issues.apache.org/jira/browse/MESOS-3072 > Project: Mesos > Issue Type: Improvement > Components: modules > Affects Versions: 0.22.0, 0.22.1, 0.23.0 > Reporter: Alexander Rojas > Assignee: Alexander Rojas > Priority: Major > Labels: mesosphere > Fix For: 0.27.0 > > > h1.Introduction > As it stands right now, default implementations of modularized components are > required to have a non parametrized {{create()}} static method. This allows > to write tests which can cover default implementations and modules based on > these default implementations on a uniform way. > For example, with the interface {{Foo}}: > {code} > class Foo { > public: > virtual ~Foo() {} > virtual Future<int> hello() = 0; > protected: > Foo() {} > }; > {code} > With a default implementation: > {code} > class LocalFoo { > public: > Try<Foo*> create() { > return new Foo; > } > virtual Future<int> hello() { > return 1; > } > }; > {code} > This allows to create typed tests which look as following: > {code} > typedef ::testing::Types<LocalFoo, > tests::Module<Foo, TestLocalFoo>> > FooTestTypes; > TYPED_TEST_CASE(FooTest, FooTestTypes); > TYPED_TEST(FooTest, ATest) > { > Try<Foo*> foo = TypeParam::create(); > ASSERT_SOME(foo); > AWAIT_CHECK_EQUAL(foo.get()->hello(), 1); > } > {code} > The test will be applied to each of types in the template parameters of > {{FooTestTypes}}. This allows to test different implementation of an > interface. In our code, it tests default implementations and a module which > uses the same default implementation. > The class {{tests::Module<typename T, ModuleID N>}} needs a little > explanation, it is a wrapper around {{ModuleManager}} which allows the tests > to encode information about the requested module in the type itself instead > of passing a string to the factory method. The wrapper around create, the > real important method looks as follows: > {code} > template<typename T, ModuleID N> > static Try<T*> test::Module<T, N>::create() > { > Try<std::string> moduleName = getModuleName(N); > if (moduleName.isError()) { > return Error(moduleName.error()); > } > return mesos::modules::ModuleManager::create<T>(moduleName.get()); > } > {code} > h1.The Problem > Consider the following implementation of {{Foo}}: > {code} > class ParameterFoo { > public: > Try<Foo*> create(int i) { > return new ParameterFoo(i); > } > ParameterFoo(int i) : i_(i) {} > virtual Future<int> hello() { > return i; > } > private: > int i_; > }; > {code} > As it can be seen, this implementation cannot be used as a default > implementation since its create API does not match the one of > {{test::Module<>}}: {{create()}} has a different signature for both types. It > is still a common situation to require initialization parameters for objects, > however this constraint (keeping both interfaces alike) forces default > implementations of modularized components to have default constructors, > therefore the tests are forcing the design of the interfaces. > Implementations which are supposed to be used as modules only, i.e. non > default implementations are allowed to have constructor parameters, since the > actual signature of their factory method is, this factory method's function > is to decode the parameters and call the appropriate constructor: > {code} > template<typename T> > T* Module<T>::create(const Parameters& params); > {code} > where parameters is just an array of key-value string pairs whose > interpretation is left to the specific module. Sadly, this call is wrapped by > {{ModuleManager}} which only allows module parameters to be passed from the > command line and does not offer a programmatic way to feed construction > parameters to modules. > h1.The Ugly Workaround > With the requirement of a default constructor and parameters devoid > {{create()}} factory function, a common pattern (see > [Authenticator|https://github.com/apache/mesos/blob/9d4ac11ed757aa5869da440dfe5343a61b07199a/include/mesos/authentication/authenticator.hpp]) > has been introduced to feed construction parameters into default > implementation, this leads to adding an {{initialize()}} call to the public > interface, which will have {{Foo}} become: > {code} > class Foo { > public: > virtual ~Foo() {} > virtual Try<Nothing> initialize(Option<int> i) = 0; > virtual Future<int> hello() = 0; > protected: > Foo() {} > }; > {code} > {{ParameterFoo}} will thus look as follows: > {code} > class ParameterFoo { > public: > Try<Foo*> create() { > return new ParameterFoo; > } > ParameterFoo() : i_(None()) {} > virtual Try<Nothing> initialize(Option<int> i) { > if (i.isNone()) { > return Error("Need value to initialize"); > } > i_ = i; > return Nothing; > } > virtual Future<int> hello() { > if (i_.isNone()) { > return Future<int>::failure("Not initialized"); > } > return i_.get(); > } > private: > Option<int> i_; > }; > {code} > Look that this {{initialize()}} method now has to be implemented by all > descendants of {{Foo}}, even if there's a {{DatabaseFoo}} which takes is > return value for {{hello()}} from a DB, it will need to support {{int}} as an > initialization parameter. > The problem is more severe the more specific the parameter to > {{initialize()}} is. For example, if there is a very complex structure > implementing ACLs, all implementations of an authorizer will need to import > this structure even if they can completely ignore it. > In the {{Foo}} example if {{ParameterFoo}} were to become the default > implementation of {{Foo}}, the tests would look as follows: > {code} > typedef ::testing::Types<ParameterFoo, > tests::Module<Foo, TestParameterFoo>> > FooTestTypes; > TYPED_TEST_CASE(FooTest, FooTestTypes); > TYPED_TEST(FooTest, ATest) > { > Try<Foo*> foo = TypeParam::create(); > ASSERT_SOME(foo); > int fooValue = 1; > foo.get()->initialize(fooValue); > AWAIT_CHECK_EQUAL(foo.get()->hello(), fooValue); > } > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)