Return-Path: X-Original-To: apmail-mesos-issues-archive@minotaur.apache.org Delivered-To: apmail-mesos-issues-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7381618EB7 for ; Mon, 19 Oct 2015 10:11:08 +0000 (UTC) Received: (qmail 70828 invoked by uid 500); 19 Oct 2015 10:11:05 -0000 Delivered-To: apmail-mesos-issues-archive@mesos.apache.org Received: (qmail 70797 invoked by uid 500); 19 Oct 2015 10:11:05 -0000 Mailing-List: contact issues-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@mesos.apache.org Delivered-To: mailing list issues@mesos.apache.org Received: (qmail 70782 invoked by uid 99); 19 Oct 2015 10:11:05 -0000 Received: from arcas.apache.org (HELO arcas) (140.211.11.28) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 19 Oct 2015 10:11:05 +0000 Received: from arcas.apache.org (localhost [127.0.0.1]) by arcas (Postfix) with ESMTP id 1943B2C14F3 for ; Mon, 19 Oct 2015 10:11:05 +0000 (UTC) Date: Mon, 19 Oct 2015 10:11:05 +0000 (UTC) From: "Bernd Mathiske (JIRA)" To: issues@mesos.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Updated] (MESOS-3072) Unify initialization of modularized components MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/MESOS-3072?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Bernd Mathiske updated MESOS-3072: ---------------------------------- Shepherd: Bernd Mathiske Story Points: 3 > 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 > Labels: mesosphere > > 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 hello() = 0; > protected: > Foo() {} > }; > {code} > With a default implementation: > {code} > class LocalFoo { > public: > Try create() { > return new Foo; > } > virtual Future hello() { > return 1; > } > }; > {code} > This allows to create typed tests which look as following: > {code} > typedef ::testing::Types tests::Module> > FooTestTypes; > TYPED_TEST_CASE(FooTest, FooTestTypes); > TYPED_TEST(FooTest, ATest) > { > Try 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}} 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 > static Try test::Module::create() > { > Try moduleName = getModuleName(N); > if (moduleName.isError()) { > return Error(moduleName.error()); > } > return mesos::modules::ModuleManager::create(moduleName.get()); > } > {code} > h1.The Problem > Consider the following implementation of {{Foo}}: > {code} > class ParameterFoo { > public: > Try create(int i) { > return new ParameterFoo(i); > } > ParameterFoo(int i) : i_(i) {} > virtual Future 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 > T* Module::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 initialize(Option i) = 0; > virtual Future hello() = 0; > protected: > Foo() {} > }; > {code} > {{ParameterFoo}} will thus look as follows: > {code} > class ParameterFoo { > public: > Try create() { > return new ParameterFoo; > } > ParameterFoo() : i_(None()) {} > virtual Try initialize(Option i) { > if (i.isNone()) { > return Error("Need value to initialize"); > } > i_ = i; > return Nothing; > } > virtual Future hello() { > if (i_.isNone()) { > return Future::failure("Not initialized"); > } > return i_.get(); > } > private: > Option 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 tests::Module> > FooTestTypes; > TYPED_TEST_CASE(FooTest, FooTestTypes); > TYPED_TEST(FooTest, ATest) > { > Try 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 (v6.3.4#6332)