[mesos] 01/02: Restructured /roles code.

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alexr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit dda549be430921e4e1c4ae670b47f455b215a20c
Author: Benno Evers <be...@mesosphere.com>
AuthorDate: Thu Sep 6 14:07:56 2018 +0200

    Restructured /roles code.
    
    Rework the structure of the role handling in both v0 and
    v1 APIs in preparation for the subsequent commit.
    
    As a nice side bonus on top, this also saves one trip
    through the master queue for every call of this endpoint.
    
    Review: https://reviews.apache.org/r/68567/
---
 src/master/http.cpp   | 83 +++++++++++++++++++++++++--------------------------
 src/master/master.hpp |  5 ++--
 2 files changed, 42 insertions(+), 46 deletions(-)

diff --git a/src/master/http.cpp b/src/master/http.cpp
index 0ac3475..8a37349 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2631,51 +2631,46 @@ string Master::Http::ROLES_HELP()
 }
 
 
-Future<vector<string>> Master::Http::_roles(
-    const Option<Principal>& principal) const
+vector<string> Master::Http::_filterRoles(
+    const Owned<ObjectApprovers>& approvers) const
 {
-  return ObjectApprovers::create(master->authorizer, principal, {VIEW_ROLE})
-    .then(defer(master->self(),
-        [this](const Owned<ObjectApprovers>& approvers)
-          -> vector<string> {
-      JSON::Object object;
+  JSON::Object object;
 
-      // Compute the role names to return results for. When an explicit
-      // role whitelist has been configured, we use that list of names.
-      // When using implicit roles, the right behavior is a bit more
-      // subtle. There are no constraints on possible role names, so we
-      // instead list all the "interesting" roles: all roles with one or
-      // more registered frameworks, and all roles with a non-default
-      // weight or quota.
-      //
-      // NOTE: we use a `std::set` to store the role names to ensure a
-      // deterministic output order.
-      set<string> roleList;
-      if (master->roleWhitelist.isSome()) {
-        const hashset<string>& whitelist = master->roleWhitelist.get();
-        roleList.insert(whitelist.begin(), whitelist.end());
-      } else {
-        hashset<string> roles = master->roles.keys();
-        roleList.insert(roles.begin(), roles.end());
+  // Compute the role names to return results for. When an explicit
+  // role whitelist has been configured, we use that list of names.
+  // When using implicit roles, the right behavior is a bit more
+  // subtle. There are no constraints on possible role names, so we
+  // instead list all the "interesting" roles: all roles with one or
+  // more registered frameworks, and all roles with a non-default
+  // weight or quota.
+  //
+  // NOTE: we use a `std::set` to store the role names to ensure a
+  // deterministic output order.
+  set<string> roleList;
+  if (master->roleWhitelist.isSome()) {
+    const hashset<string>& whitelist = master->roleWhitelist.get();
+    roleList.insert(whitelist.begin(), whitelist.end());
+  } else {
+    hashset<string> roles = master->roles.keys();
+    roleList.insert(roles.begin(), roles.end());
 
-        hashset<string> weights = master->weights.keys();
-        roleList.insert(weights.begin(), weights.end());
+    hashset<string> weights = master->weights.keys();
+    roleList.insert(weights.begin(), weights.end());
 
-        hashset<string> quotas = master->quotas.keys();
-        roleList.insert(quotas.begin(), quotas.end());
-      }
+    hashset<string> quotas = master->quotas.keys();
+    roleList.insert(quotas.begin(), quotas.end());
+  }
 
-      vector<string> filteredRoleList;
-      filteredRoleList.reserve(roleList.size());
+  vector<string> filteredRoleList;
+  filteredRoleList.reserve(roleList.size());
 
-      foreach (const string& role, roleList) {
-        if (approvers->approved<VIEW_ROLE>(role)) {
-          filteredRoleList.push_back(role);
-        }
-      }
+  foreach (const string& role, roleList) {
+    if (approvers->approved<VIEW_ROLE>(role)) {
+      filteredRoleList.push_back(role);
+    }
+  }
 
-      return filteredRoleList;
-    }));
+  return filteredRoleList;
 }
 
 
@@ -2697,11 +2692,12 @@ Future<Response> Master::Http::roles(
     return redirect(request);
   }
 
-  return _roles(principal)
+  return ObjectApprovers::create(master->authorizer, principal, {VIEW_ROLE})
     .then(defer(master->self(),
-        [this, request](const vector<string>& filteredRoles)
+        [this, request](const Owned<ObjectApprovers>& approvers)
           -> Response {
       JSON::Object object;
+      const vector<string> filteredRoles = _filterRoles(approvers);
 
       {
         JSON::Array array;
@@ -2791,11 +2787,12 @@ Future<Response> Master::Http::getRoles(
     ContentType contentType) const
 {
   CHECK_EQ(mesos::master::Call::GET_ROLES, call.type());
-
-  return _roles(principal)
+  return ObjectApprovers::create(master->authorizer, principal, {VIEW_ROLE})
     .then(defer(master->self(),
-        [this, contentType](const vector<string>& filteredRoles)
+        [this, contentType](const Owned<ObjectApprovers>& approvers)
           -> Response {
+      const vector<string> filteredRoles = _filterRoles(approvers);
+
       mesos::master::Response response;
       response.set_type(mesos::master::Response::GET_ROLES);
 
diff --git a/src/master/master.hpp b/src/master/master.hpp
index eecb66c..5d01de4 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1695,9 +1695,8 @@ private:
         Resources required,
         const Offer::Operation& operation) const;
 
-    process::Future<std::vector<std::string>> _roles(
-        const Option<process::http::authentication::Principal>&
-            principal) const;
+    std::vector<std::string> _filterRoles(
+        const process::Owned<ObjectApprovers>& approvers) const;
 
     // Master API handlers.