[ 
https://issues.apache.org/jira/browse/MESOS-3248?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Alexander Rojas updated MESOS-3248:
-----------------------------------
    Description: 
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<>}}, however it is a common situation to require initialization 
parameters for objects, however this constraint forces default implementations 
of modularized components to have default constructors.

Module only implementations are allowed to have constructor parameters, since 
the actual signature of their factory method is:

{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. However, this call is wrapped by 
{{ModuleManager}} which only allows module parameters to be passed from the 
command line and do 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 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}

  was:
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<>}}, however it is a common situation to require initialization 
parameters for objects, however this constraint forces default implementations 
of modularized components to have default constructors.

Module only implementations are allowed to have constructor parameters, since 
the actual signature of their factory method is:

{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. However, this call is wrapped by 
{{ModuleManager}} which only allows module parameters to be passed from the 
command line and do 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 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}


> Allow developers the option to pass parameters to modules programatically
> -------------------------------------------------------------------------
>
>                 Key: MESOS-3248
>                 URL: https://issues.apache.org/jira/browse/MESOS-3248
>             Project: Mesos
>          Issue Type: Bug
>          Components: modules
>            Reporter: 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<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<>}}, however it is a common situation to require 
> initialization parameters for objects, however this constraint forces default 
> implementations of modularized components to have default constructors.
> Module only implementations are allowed to have constructor parameters, since 
> the actual signature of their factory method is:
> {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. However, this call is wrapped 
> by 
> {{ModuleManager}} which only allows module parameters to be passed from the 
> command line and do 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 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
(v6.3.4#6332)

Reply via email to