reapi: use enum to pass match op

Problem: reapi has several layers of bindings that use strings to
pass the match option. Strings require validation and use more space
than is required.

Use match_op_t enum for reapi insead. Move match_op_t definition to its
own file and add function to convert match_op_t to string for resource
module match call back.

Author: zekemorton

resource/CMakeLists.txt

@@ -22,6 +22,7 @@ set(RESOURCE_HEADERS
     traversers/dfu_impl.hpp
     policies/base/dfu_match_cb.hpp
     policies/base/matcher.hpp
+    policies/base/match_op.h
     readers/resource_namespace_remapper.hpp
     readers/resource_reader_base.hpp
     readers/resource_spec_grug.hpp

resource/policies/base/match_op.h

@@ -0,0 +1,34 @@
+#ifndef MATCH_OP_H
+#define MATCH_OP_H
+
+
+typedef enum match_op_t { MATCH_UNKNOWN,
+                          MATCH_ALLOCATE,
+                          MATCH_ALLOCATE_W_SATISFIABILITY,
+                          MATCH_ALLOCATE_ORELSE_RESERVE,
+                          MATCH_SATISFIABILITY } match_op_t;
+
+static const char *match_op_to_string (match_op_t match_op) {
+    switch (match_op) {
+        case MATCH_ALLOCATE:                  return "allocate";
+        case MATCH_ALLOCATE_ORELSE_RESERVE:   return "allocate_orelse_reserve";
+        case MATCH_ALLOCATE_W_SATISFIABILITY: return "allocate_with_satisfiability";
+        case MATCH_SATISFIABILITY:            return "satisfiability";
+        default: return "error";
+    }
+}
+
+static bool match_op_valid (match_op_t match_op) {
+    
+    if ( (match_op != MATCH_ALLOCATE) &&
+         (match_op != MATCH_ALLOCATE_W_SATISFIABILITY) &&
+         (match_op != MATCH_ALLOCATE_ORELSE_RESERVE) &&
+         (match_op != MATCH_SATISFIABILITY) ) {
+        
+        return false;
+    }
+
+    return true;
+}
+
+#endif //MATCH_OP_H

resource/policies/base/matcher.hpp

@@ -19,6 +19,7 @@
 #include "resource/libjobspec/jobspec.hpp"
 #include "resource/schema/data_std.hpp"
 #include "resource/planner/c/planner.h"
+#include "resource/policies/base/match_op.h"
 
 namespace Flux {
 namespace resource_model {
@@ -27,11 +28,6 @@ const std::string ANY_RESOURCE_TYPE = "*";
 
 enum match_score_t { MATCH_UNMET = 0, MATCH_MET = 1 };
 
-enum class match_op_t { MATCH_ALLOCATE,
-                        MATCH_ALLOCATE_W_SATISFIABILITY,
-                        MATCH_ALLOCATE_ORELSE_RESERVE,
-                        MATCH_SATISFIABILITY };
-
 /*! Base matcher data class.
  *  Provide idioms to specify the target subsystems and
  *  resource relationship types which then allow for filtering the graph

resource/reapi/bindings/c++/reapi.hpp

@@ -99,9 +99,16 @@ class reapi_t {
      *                   detail. However, when it is used within a Flux's
      *                   service module, it is expected to be a pointer
      *                   to a flux_t object.
-     *  \param orelse_reserve
-     *                   Boolean: if false, only allocate; otherwise, first try
-     *                   to allocate and if that fails, reserve.
+     *  \param match_op  match_op_t: set to specify the specific match option
+     *                   from 1 of 4 choices:
+     *                   MATCH_ALLOCATE: try to allocate now and fail if resources 
+     *                   aren't available. 
+     *                   MATCH_ALLOCATE_ORELSE_RESERVE : Try to allocate and reseve
+     *                   if resources aren't available now.
+     *                   MATCH_SATISFIABILITY: Do a satisfiablity check and do not 
+     *                   allocate.
+     *                   MATCH_ALLOCATE_W_SATISFIABILITY: try to allocate and run 
+     *                   satisfiability check if resources are not available.
      *  \param jobspec   jobspec string.
      *  \param jobid     jobid of the uint64_t type.
      *  \param reserved  Boolean into which to return true if this job has been
@@ -131,9 +138,15 @@ class reapi_t {
      *                   detail. However, when it is used within a Flux's
      *                   service module, it is expected to be a pointer
      *                   to a flux_t object.
-     *  \param orelse_reserve
-     *                   Boolean: if false, only allocate; otherwise, first try
-     *                   to allocate and if that fails, reserve.
+     *  \param match_op  match_op_t: set to specify the specific match option
+     *                   from 1 of 4 choices:
+     *                   MATCH_ALLOCATE: try to allocate now and fail if resources 
+     *                   aren't available. 
+     *                   MATCH_ALLOCATE_ORELSE_RESERVE : Try to allocate and reseve
+     *                   if resources aren't available now.
+     *                   MATCH_SATISFIABILITY: Do a satisfiablity check and do not 
+     *                   allocate.
+     *                   MATCH_ALLOCATE_W_SATISFIABILITY: try to allocate and run 
      *  \param jobs      JSON array of jobspecs.
      *  \param adapter   queue_adapter_base_t object that provides
      *                   a set of callback methods to be called each time

resource/reapi/bindings/c++/reapi_cli.hpp

@@ -26,6 +26,7 @@ extern "C" {
 #include "resource/jobinfo/jobinfo.hpp"
 #include "resource/policies/dfu_match_policy_factory.hpp"
 #include "resource/traversers/dfu.hpp"
+#include "resource/policies/base/match_op.h"
 
 namespace Flux {
 namespace resource_model {

resource/reapi/bindings/c++/reapi_cli_impl.hpp

@@ -66,6 +66,14 @@ int reapi_cli_t::match_allocate (void *h, match_op_t match_op,
     std::stringstream o;
     bool matched = false;
 
+    if (!match_op_valid (match_op) ) {
+            m_err_msg += __FUNCTION__;
+            m_err_msg += ": ERROR: Invalid Match Option: "
+                          + std::string (match_op_to_string (match_op)) + "\n";
+            rc = -1;
+            goto out;
+        }
+
     try {
         Flux::Jobspec::Jobspec job {jobspec};
 

resource/reapi/bindings/c++/reapi_module.hpp

@@ -21,14 +21,15 @@ extern "C" {
 #include <cstdint>
 #include <string>
 #include "resource/reapi/bindings/c++/reapi.hpp"
+#include "resource/policies/base/match_op.h"
 
 namespace Flux {
 namespace resource_model {
 namespace detail {
 
 class reapi_module_t : public reapi_t {
 public:
-    static int match_allocate (void *h, const char *cmd,
+    static int match_allocate (void *h, match_op_t match_op,
                                const std::string &jobspec,
                                const uint64_t jobid, bool &reserved,
                                std::string &R, int64_t &at, double &ov);
@@ -39,7 +40,7 @@ class reapi_module_t : public reapi_t {
     static int match_allocate_multi (void *h, bool orelse_reserve,
                                      const char *jobs,
                                      queue_adapter_base_t *adapter);
-    static int match_allocate_multi (void *h, const char *cmd,
+    static int match_allocate_multi (void *h, match_op_t match_op,
                                      const char *jobs,
                                      queue_adapter_base_t *adapter);
     static int update_allocate (void *h, const uint64_t jobid,

resource/reapi/bindings/c++/reapi_module_impl.hpp

@@ -25,7 +25,7 @@ namespace Flux {
 namespace resource_model {
 namespace detail {
 
-int reapi_module_t::match_allocate (void *h, const char *cmd,
+int reapi_module_t::match_allocate (void *h, match_op_t match_op,
                                     const std::string &jobspec,
                                     const uint64_t jobid, bool &reserved,
                                     std::string &R, int64_t &at, double &ov)
@@ -36,6 +36,7 @@ int reapi_module_t::match_allocate (void *h, const char *cmd,
     flux_future_t *f = NULL;
     const char *rset = NULL;
     const char *status = NULL;
+    const char *cmd = match_op_to_string (match_op);
 
     if (!fh || jobspec == "" || jobid > INT64_MAX) {
         errno = EINVAL;
@@ -73,10 +74,10 @@ int reapi_module_t::match_allocate (void *h, bool orelse_reserve,
                                     const uint64_t jobid, bool &reserved,
                                     std::string &R, int64_t &at, double &ov)
 {
-    const char *cmd = (orelse_reserve)? "allocate_orelse_reserve"
-                                      : "allocate_with_satisfiability";
+    match_op_t match_op = (orelse_reserve)? match_op_t::MATCH_ALLOCATE_ORELSE_RESERVE
+                                       : match_op_t::MATCH_ALLOCATE_W_SATISFIABILITY;
 
-    return match_allocate (h, cmd, jobspec, jobid, reserved, R, at, ov); 
+    return match_allocate (h, match_op, jobspec, jobid, reserved, R, at, ov); 
 
 }
 
@@ -111,13 +112,14 @@ void match_allocate_multi_cont (flux_future_t *f, void *arg)
 }
 
 int reapi_module_t::match_allocate_multi (void *h,
-                                          const char *cmd,
+                                          match_op_t match_op,
                                           const char *jobs,
                                           queue_adapter_base_t *adapter)
 {
     int rc = -1;
     flux_t *fh = static_cast<flux_t *> (h);
     flux_future_t *f = nullptr;
+    const char *cmd = match_op_to_string (match_op);
 
     if (!fh) {
         errno = EINVAL;
@@ -147,9 +149,10 @@ int reapi_module_t::match_allocate_multi (void *h,
                                           const char *jobs,
                                           queue_adapter_base_t *adapter)
 {
-    const char *cmd = orelse_reserve ? "allocate_orelse_reserve"
-                                     : "allocate_with_satisfiability";
-    return match_allocate_multi (h, cmd, jobs, adapter);
+    match_op_t match_op = (orelse_reserve)? match_op_t::MATCH_ALLOCATE_ORELSE_RESERVE
+                                       : match_op_t::MATCH_ALLOCATE_W_SATISFIABILITY;
+
+    return match_allocate_multi (h, match_op, jobs, adapter);
 }
 
 int reapi_module_t::update_allocate (void *h, const uint64_t jobid,

resource/reapi/bindings/c/reapi_cli.cpp

@@ -91,33 +91,21 @@ extern "C" int reapi_cli_initialize (reapi_cli_ctx_t *ctx, const char *rgraph,
 }
 
 extern "C" int reapi_cli_match (reapi_cli_ctx_t *ctx,
-                                const char *match_op, const char *jobspec,
+                                match_op_t match_op, const char *jobspec,
                                 uint64_t *jobid, bool *reserved,
                                 char **R, int64_t *at, double *ov)
 {
     int rc = -1;
     std::string R_buf = "";
     char *R_buf_c = nullptr;
-    match_op_t op;
 
     if (!ctx || !ctx->rqt) {
         errno = EINVAL;
         goto out;
     }
 
-    if (!strcmp (match_op,"allocate"))
-        op = match_op_t::MATCH_ALLOCATE;
-    else if (!strcmp (match_op, "allocate_with_satisfiability"))
-        op = match_op_t::MATCH_ALLOCATE_W_SATISFIABILITY;
-    else if (!strcmp (match_op, "allocate_orelse_reserve"))
-        op = match_op_t::MATCH_ALLOCATE_ORELSE_RESERVE;
-    else if (!strcmp (match_op, "satisfiability"))
-        op = match_op_t::MATCH_SATISFIABILITY;
-    else
-        goto out;
-
     *jobid = ctx->rqt->get_job_counter ();
-    if ((rc = reapi_cli_t::match_allocate (ctx->rqt, op,
+    if ((rc = reapi_cli_t::match_allocate (ctx->rqt, match_op,
                                            jobspec, *jobid, *reserved,
                                            R_buf, *at, *ov)) < 0) {
         goto out;
@@ -141,8 +129,8 @@ extern "C" int reapi_cli_match_allocate (reapi_cli_ctx_t *ctx,
                                          uint64_t *jobid, bool *reserved,
                                          char **R, int64_t *at, double *ov)
 {
-    const char  *match_op = orelse_reserve ? "allocate_orelse_reserve" : 
-                                             "allocate";
+    match_op_t match_op = orelse_reserve ? match_op_t::MATCH_ALLOCATE_ORELSE_RESERVE : 
+                                             match_op_t::MATCH_ALLOCATE;
 
     return reapi_cli_match (ctx, match_op, jobspec, jobid, reserved, 
                                      R, at, ov);
@@ -152,7 +140,7 @@ extern "C" int reapi_cli_match_satisfy (reapi_cli_ctx_t *ctx,
                                         const char *jobspec, 
                                         bool *sat, double *ov)
 {
-    const char  *match_op = "satisfiability";
+    match_op_t match_op = match_op_t::MATCH_SATISFIABILITY;
     uint64_t jobid;
     bool reserved;
     char *R; 

resource/reapi/bindings/c/reapi_cli.h

@@ -19,6 +19,7 @@ extern "C" {
 #include "config.h"
 #endif
 #include <flux/core.h>
+#include "resource/policies/base/match_op.h"
 
 typedef struct reapi_cli_ctx reapi_cli_ctx_t;
 
@@ -46,15 +47,15 @@ int reapi_cli_initialize (reapi_cli_ctx_t *ctx, const char *rgraph,
  *  the selected match policy.
  *
  *  \param ctx       reapi_cli_ctx_t context object
- *  \param match_op  const char *: set to specify the specific match option
+ *  \param match_op  match_op_t: set to specify the specific match option
  *                   from 1 of 4 choices:
- *                   allocate: try to allocate now and fail if resources 
+ *                   MATCH_ALLOCATE: try to allocate now and fail if resources 
  *                   aren't available. 
- *                   allocate_orelse_reserve : Try to allocate and reserve if 
- *                   resources aren't available now.
- *                   satisfiability: Do a satisfiablity check and do not 
+ *                   MATCH_ALLOCATE_ORELSE_RESERVE : Try to allocate and reserve
+ *                   if resources aren't available now.
+ *                   MATCH_SATISFIABILITY: Do a satisfiablity check and do not 
  *                   allocate.
- *                   allocate_with_satisfiability: try to allocate and run 
+ *                   MATCH_ALLOCATE_W_SATISFIABILITY: try to allocate and run 
  *                   satisfiability check if resources are not available.
  *  \param jobspec   jobspec string.
  *  \param jobid     jobid of the uint64_t type.
@@ -69,7 +70,7 @@ int reapi_cli_initialize (reapi_cli_ctx_t *ctx, const char *rgraph,
  *                   the match operation.
  *  \return          0 on success; -1 on error.
  */
-int reapi_cli_match (reapi_cli_ctx_t *ctx, const char *match_op,
+int reapi_cli_match (reapi_cli_ctx_t *ctx, match_op_t match_op,
                      const char *jobspec, uint64_t *jobid,
                      bool *reserved,
                      char **R, int64_t *at, double *ov);