doveadm: Add flag CMD_PARAM_FLAG_KEY_VALUE (named positional parameters)

diff --git a/src/doveadm/Makefile.am b/src/doveadm/Makefile.am
index 9f9e116eaa56eaf42345ad7f92921608e2c599fe..9103c0705355c34040961c7e919004e8f10f3fd3 100644 (file)
--- a/src/doveadm/Makefile.am
+++ b/src/doveadm/Makefile.am
@@ -178,6 +178,7 @@ noinst_HEADERS = \
        doveadm-who.h
 
 test_programs = \
+       test-doveadm-cmd \
        test-doveadm-util
 noinst_PROGRAMS = $(test_programs)
 
@@ -186,6 +187,10 @@ test_libs = \
        ../lib/liblib.la
 test_deps = $(noinst_LTLIBRARIES) $(test_libs)
 
+test_doveadm_cmd_SOURCES = doveadm-cmd-parse.c test-doveadm-cmd-parse.c
+test_doveadm_cmd_LDADD = $(test_libs) $(MODULE_LIBS)
+test_doveadm_cmd_DEPENDENCIES = $(test_deps)
+
 test_doveadm_util_SOURCES = doveadm-util.c test-doveadm-util.c
 test_doveadm_util_LDADD = $(test_libs) $(MODULE_LIBS)
 test_doveadm_util_DEPENDENCIES = $(test_deps)

diff --git a/src/doveadm/doveadm-cmd-parse.c b/src/doveadm/doveadm-cmd-parse.c
index c9f425e9743f9fad2054be74796ffb8395cb6219..c33bd969e54b7bf855175f901fd0df63bfb75a73 100644 (file)
--- a/src/doveadm/doveadm-cmd-parse.c
+++ b/src/doveadm/doveadm-cmd-parse.c
@@ -344,28 +344,45 @@ int doveadm_cmdline_run(int argc, const char *const argv[],
        unsigned int pargc;
        pool_t pool = pool_datastack_create();
 
+       i_getopt_reset();
        p_array_init(&pargv, pool, 20);
        if (doveadm_cmd_process_options(argc, argv, cctx, pool, &pargv) < 0)
                return -1;
 
+       unsigned int ptr_count;
+       struct doveadm_cmd_param *ptr = array_get_modifiable(&pargv, &ptr_count);
+       struct doveadm_cmd_param *ptr_end = ptr + ptr_count;
+       struct doveadm_cmd_param *ptr_last_non_kv = ptr;
+
        /* process positional arguments */
        for (; optind < argc; optind++) {
-               struct doveadm_cmd_param *ptr;
                bool found = FALSE;
-               array_foreach_modifiable(&pargv, ptr) {
-                       if ((ptr->flags & CMD_PARAM_FLAG_POSITIONAL) != 0 &&
-                           (ptr->value_set == FALSE ||
-                            ptr->type == CMD_PARAM_ARRAY)) {
-                               const char *error;
-                               if (doveadm_fill_param(ptr, argv[optind], pool, &error) < 0) {
-                                       i_error("Invalid parameter: %s",
-                                               t_strarray_join(argv + optind, " "));
-                                       doveadm_cmd_params_clean(&pargv);
-                                       return -1;
-                               }
-                               found = TRUE;
-                               break;
+               bool is_keyvalue = FALSE;
+               for (; ptr < ptr_end; ptr++) {
+                       if ((ptr->flags & CMD_PARAM_FLAG_POSITIONAL) == 0)
+                               continue;
+                       if ((ptr->flags & CMD_PARAM_FLAG_KEY_VALUE) == 0)
+                               ptr_last_non_kv = ptr;
+                       if (ptr->value_set && ptr->type != CMD_PARAM_ARRAY)
+                               continue;
+                       if ((ptr->flags & CMD_PARAM_FLAG_KEY_VALUE) != 0) {
+                               if (optind + 1 >= argc)
+                                       continue;
+                               if (strcmp(ptr->key, argv[optind]) != 0)
+                                       continue;
+                               optind++;
+                               is_keyvalue = TRUE;
+                       }
+
+                       const char *error;
+                       if (doveadm_fill_param(ptr, argv[optind], pool, &error) < 0) {
+                               i_error("Invalid parameter: %s",
+                                       t_strarray_join(argv + optind, " "));
+                               doveadm_cmd_params_clean(&pargv);
+                               return -1;
                        }
+                       found = TRUE;
+                       break;
                }
                if (!found) {
                        i_error("Extraneous arguments found: %s",
@@ -373,6 +390,8 @@ int doveadm_cmdline_run(int argc, const char *const argv[],
                        doveadm_cmd_params_clean(&pargv);
                        return -1;
                }
+               if (is_keyvalue)
+                       ptr = ptr_last_non_kv;
        }
 
        doveadm_cmd_params_null_terminate_arrays(&pargv);

diff --git a/src/doveadm/doveadm-cmd-parse.h b/src/doveadm/doveadm-cmd-parse.h
index 0b3c0f3eac49f4fe89cc1cef1dcf35cf6c53922a..eee77dfdd1cf3b254d4ac76f58560fd4e2459efe 100644 (file)
--- a/src/doveadm/doveadm-cmd-parse.h
+++ b/src/doveadm/doveadm-cmd-parse.h
@@ -4,7 +4,8 @@
 #include "net.h"
 
 #define DOVEADM_CMD_PARAMS_START .parameters = (const struct doveadm_cmd_param[]){
-#define DOVEADM_CMD_PARAM(optP, nameP, typeP, flagP ) { .short_opt = optP, .name = nameP, .type = typeP, .flags = flagP },
+#define DOVEADM_CMD_PARAM(optP, nameP, typeP, flagP) DOVEADM_CMD_PARAMKV(optP, nameP, nameP, typeP, flagP)
+#define DOVEADM_CMD_PARAMKV(optP, nameP, keyP, typeP, flagP) { .short_opt = optP, .name = nameP, .key = keyP, .type = typeP, .flags = flagP },
 #define DOVEADM_CMD_PARAMS_END { .short_opt = '\0', .name = NULL, .type = CMD_PARAM_BOOL, .flags = CMD_PARAM_FLAG_NONE } }
 
 struct doveadm_cmd_context;
@@ -21,9 +22,31 @@ typedef enum {
 
 typedef enum {
        CMD_PARAM_FLAG_NONE             = 0x0,
+
+       /* Meaningful only while parsing a command line command */
        CMD_PARAM_FLAG_POSITIONAL       = 0x1,
+
        CMD_PARAM_FLAG_DO_NOT_EXPOSE    = 0x2,
-       CMD_PARAM_FLAG_UNSIGNED         = 0x4,  /* int64 checked to be non negative */
+
+       /* Grant that the parsed int64 is non negative */
+       CMD_PARAM_FLAG_UNSIGNED         = 0x4,
+
+       /* While parsing a command line command,
+          if the current argument matches the parameter key, the following
+          argument is consumed as its value. Multiple consecutive key_value
+          parameters are all evaluated independent of their ordering against
+          the current argument.
+
+          The parameter key is by default the same as the parameter name,
+          unless DOVEADM_CMD_PARAMKV is used to specify a distinct key.
+
+          If the current argument doesn't match any key_value parameter for
+          the current position, the next plain positional argument will be
+          filled (if any).
+
+          See test-doveadm-cmd-parse.c for more details.
+         */
+       CMD_PARAM_FLAG_KEY_VALUE        = 0x8,
 } doveadm_cmd_param_flag_t;
 
 typedef enum {
@@ -41,6 +64,7 @@ typedef enum {
 struct doveadm_cmd_param {
        char short_opt;
        const char *name;
+       const char *key;
        doveadm_cmd_param_t type;
        bool value_set;
        struct {

diff --git a/src/doveadm/test-doveadm-cmd-parse.c b/src/doveadm/test-doveadm-cmd-parse.c
new file mode 100644 (file)
index 0000000..ece3f7f
--- /dev/null
+++ b/src/doveadm/test-doveadm-cmd-parse.c
@@ -0,0 +1,284 @@
+/* Copyright (c) 2016-2018 Dovecot authors, see the included COPYING file */
+
+#include "lib.h"
+#include "test-common.h"
+#include "doveadm.h"
+#include "doveadm-cmd-parse.h"
+
+static inline void
+assert_param_bool(struct doveadm_cmd_context *cctx, const char *name,
+                 bool expected)
+{
+       test_assert_cmp(expected, ==, doveadm_cmd_param_flag(cctx, name));
+}
+
+static void
+assert_param_str(struct doveadm_cmd_context *cctx, const char *name,
+                const char* expected)
+{
+       const char *value;
+       if (!doveadm_cmd_param_str(cctx, name, &value)) {
+               if (expected != NULL)
+                       test_failed(t_strdup_printf(
+                               "doveadm_cmd_param_str(%s) failed", name));
+       }
+       else
+               test_assert_strcmp(expected, value);
+}
+
+static void test_case(const char *name, int expected_rc,
+                      doveadm_command_ver2_t assertfn,
+                     struct doveadm_cmd_ver2 *cmd,const char **argv )
+{
+       T_BEGIN {
+               test_begin(name);
+
+               struct doveadm_cmd_ver2 *cmds = t_new(struct doveadm_cmd_ver2, 2);
+               struct doveadm_cmd_context *cctx = t_new(struct doveadm_cmd_context, 1);
+
+               cmd->cmd = assertfn;
+               cmds[0] = *cmd;
+               cctx->cmd = cmds;
+
+               if (expected_rc < 0)
+                       test_expect_errors(1);
+
+               int actual_rc = doveadm_cmdline_run(
+                       str_array_length(argv), argv, cctx);
+
+               test_assert_cmp(expected_rc, ==, actual_rc);
+               test_end();
+       } T_END;
+}
+
+struct doveadm_cmd_ver2 cmdv2_posargs = {
+       .flags = 0,
+DOVEADM_CMD_PARAMS_START
+DOVEADM_CMD_PARAM('\0', "pos1", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAM('\0', "pos2", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAMS_END
+};
+
+struct doveadm_cmd_ver2 cmdv2_kvargs = {
+       .flags = 0,
+DOVEADM_CMD_PARAMS_START
+DOVEADM_CMD_PARAM('\0', "pos1", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAM('\0', "key1", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL|CMD_PARAM_FLAG_KEY_VALUE)
+DOVEADM_CMD_PARAM('\0', "key2", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL|CMD_PARAM_FLAG_KEY_VALUE)
+DOVEADM_CMD_PARAM('\0', "pos2", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAMS_END
+};
+
+struct doveadm_cmd_ver2 cmdv2_kvpositions = {
+       .flags = 0,
+DOVEADM_CMD_PARAMS_START
+DOVEADM_CMD_PARAM('\0', "pos1", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAM('\0', "key1", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL|CMD_PARAM_FLAG_KEY_VALUE)
+DOVEADM_CMD_PARAM('\0', "key2", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL|CMD_PARAM_FLAG_KEY_VALUE)
+DOVEADM_CMD_PARAM('\0', "pos2", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAM('\0', "key3", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL|CMD_PARAM_FLAG_KEY_VALUE)
+DOVEADM_CMD_PARAM('\0', "key4", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL|CMD_PARAM_FLAG_KEY_VALUE)
+DOVEADM_CMD_PARAM('\0', "pos3", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAMS_END
+};
+
+struct doveadm_cmd_ver2 cmdv2_switches = {
+       .flags = 0,
+DOVEADM_CMD_PARAMS_START
+DOVEADM_CMD_PARAM('1', "switch1", CMD_PARAM_BOOL, CMD_PARAM_FLAG_NONE)
+DOVEADM_CMD_PARAM('2', "switch2", CMD_PARAM_STR, CMD_PARAM_FLAG_NONE)
+DOVEADM_CMD_PARAM('\0', "pos1", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAM('\0', "pos2", CMD_PARAM_STR, CMD_PARAM_FLAG_POSITIONAL)
+DOVEADM_CMD_PARAMS_END
+};
+
+static void assert_no_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "pos1", NULL);
+       assert_param_str(cctx, "pos2", NULL);
+}
+
+static void assert_1_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "pos1", "arg1");
+       assert_param_str(cctx, "pos2", NULL);
+}
+
+static void assert_2_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "pos1", "arg1");
+       assert_param_str(cctx, "pos2", "arg2");
+}
+
+static void assert_3_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "pos1", "arg1");
+       assert_param_str(cctx, "pos2", "arg2");
+       assert_param_str(cctx, "pos3", "arg3");
+}
+
+static void assert_kv1_in_pos12(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "pos1", "key1");
+       assert_param_str(cctx, "pos2", "value1");
+}
+
+static void assert_kv3_in_pos23(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "pos1", "arg1");
+       assert_param_str(cctx, "pos2", "key3");
+       assert_param_str(cctx, "pos3", "value3");
+}
+
+static void assert_key12(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "key1", "value1");
+       assert_param_str(cctx, "key2", "value2");
+}
+
+static void assert_key34(struct doveadm_cmd_context *cctx)
+{
+       assert_param_str(cctx, "key3", "value3");
+       assert_param_str(cctx, "key4", "value4");
+}
+
+static void assert_key12_1_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_1_pos_args(cctx);
+       assert_key12(cctx);
+}
+
+static void assert_key12_2_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_2_pos_args(cctx);
+       assert_key12(cctx);
+}
+
+static void assert_key1234_2_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_2_pos_args(cctx);
+       assert_key12(cctx);
+       assert_key34(cctx);
+}
+
+static void assert_no_switch12(struct doveadm_cmd_context *cctx)
+{
+       assert_param_bool(cctx, "switch1", FALSE);
+       assert_param_str(cctx, "switch2", NULL);
+}
+
+static void assert_switch12(struct doveadm_cmd_context *cctx)
+{
+       assert_param_bool(cctx, "switch1", TRUE);
+       assert_param_str(cctx, "switch2", "value2");
+}
+
+static void assert_no_switch_no_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_no_pos_args(cctx);
+       assert_no_switch12(cctx);
+}
+
+static void assert_no_switch_1_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_1_pos_args(cctx);
+       assert_no_switch12(cctx);
+}
+
+static void assert_switch12_1_pos_args(struct doveadm_cmd_context *cctx)
+{
+       assert_1_pos_args(cctx);
+       assert_switch12(cctx);
+}
+
+static void assert_not_execd(struct doveadm_cmd_context *cctx ATTR_UNUSED)
+{
+       test_failed("doveadm_cmdline_run() expected to fail and not execute the cmd");
+}
+
+#define line(l) t_strsplit_spaces(l, " ")
+
+static void test_posargs(void)
+{
+       test_case("pos_0args", 0, assert_no_pos_args, &cmdv2_posargs,
+               line("cmd"));
+       test_case("pos_1args", 0, assert_1_pos_args,  &cmdv2_posargs,
+               line("cmd arg1"));
+       test_case("pos_2args", 0, assert_2_pos_args,  &cmdv2_posargs,
+               line("cmd arg1 arg2"));
+       test_case("pos_3args", -1, assert_not_execd,  &cmdv2_posargs,
+               line("cmd arg1 arg2 arg3"));
+}
+
+static void test_kvargs(void)
+{
+       test_case("kv_0args", 0, assert_no_pos_args, &cmdv2_kvargs,
+               line("cmd"));
+       test_case("kv_1args", 0, assert_1_pos_args,  &cmdv2_kvargs,
+               line("cmd arg1"));
+       test_case("kv_2args", 0, assert_2_pos_args,  &cmdv2_kvargs,
+               line("cmd arg1 arg2"));
+       test_case("kv_3args", -1, assert_not_execd,  &cmdv2_kvargs,
+               line("cmd arg1 arg2 arg3"));
+
+       /* not in expected position, named args are expected in 2nd arg position not 1st */
+       test_case("kv_early", 0, assert_kv1_in_pos12, &cmdv2_kvargs,
+               line("cmd key1 value1"));
+
+       /* not in expected position, named args are expected in 2nd arg position not 3rd */
+       test_case("kv_late", -1, assert_not_execd, &cmdv2_kvargs,
+               line("cmd arg1 arg2 key1 value1"));
+
+       test_case("kv_k12_1args", 0, assert_key12_1_pos_args, &cmdv2_kvargs,
+               line("cmd arg1 key1 value1 key2 value2"));
+       test_case("kv_k21_1args", 0, assert_key12_1_pos_args, &cmdv2_kvargs,
+               line("cmd arg1 key2 value2 key1 value1"));
+
+       test_case("kv_k12_2args", 0, assert_key12_2_pos_args, &cmdv2_kvargs,
+               line("cmd arg1 key1 value1 key2 value2 arg2"));
+       test_case("kv_k21_2args", 0, assert_key12_2_pos_args, &cmdv2_kvargs,
+               line("cmd arg1 key2 value2 key1 value1 arg2"));
+}
+
+static void test_kvpos(void)
+{
+       test_case("kvpos_3args", 0, assert_3_pos_args, &cmdv2_kvpositions,
+               line("cmd arg1 arg2 arg3"));
+       test_case("kvpos_k12_k43", 0, assert_key1234_2_pos_args, &cmdv2_kvpositions,
+               line("cmd arg1 key1 value1 key2 value2 arg2 key4 value4 key3 value3"));
+       test_case("kvpos_k21_k34", 0, assert_key1234_2_pos_args, &cmdv2_kvpositions,
+               line("cmd arg1 key2 value2 key1 value1 arg2 key3 value3 key4 value4"));
+       test_case("kvpos_k3", 0, assert_kv3_in_pos23, &cmdv2_kvpositions,
+               line("cmd arg1 key3 value3"));
+       test_case("kvpos_k2_k1", -1, assert_not_execd, &cmdv2_kvpositions,
+               line("cmd arg1 key2 value2 key1 value1 arg2 key1 value1"));
+}
+
+static void test_switches(void)
+{
+       test_case("kwswitch_none", 0, assert_no_switch_no_pos_args, &cmdv2_switches,
+               line("cmd"));
+       test_case("kwswitch_1args", 0, assert_no_switch_1_pos_args, &cmdv2_switches,
+               line("cmd arg1"));
+       test_case("kwswitch_before_12", 0, assert_switch12_1_pos_args, &cmdv2_switches,
+               line("cmd -1 -2 value2 arg1"));
+       test_case("kwswitch_before_21", 0, assert_switch12_1_pos_args, &cmdv2_switches,
+               line("cmd -2 value2 -1 arg1"));
+       test_case("kwswitch_after_12", 0, assert_switch12_1_pos_args, &cmdv2_switches,
+               line("cmd arg1 -1 -2 value2"));
+       test_case("kwswitch_fater_21", 0, assert_switch12_1_pos_args, &cmdv2_switches,
+               line("cmd arg1 -2 value2 -1"));
+}
+
+static void (*const test_functions[])(void) = {
+       test_posargs,
+       test_kvargs,
+       test_kvpos,
+       test_switches,
+       NULL
+};
+
+int main(void)
+{
+       return test_run(test_functions);
+}

diff --git a/src/lib-test/test-common.h b/src/lib-test/test-common.h
index 305586448a5785003b6b5fc5fd6ebf142eaeb925..d0a166a1e79ba06fe39c0ebbd6c203f5902cb1b1 100644 (file)
--- a/src/lib-test/test-common.h
+++ b/src/lib-test/test-common.h
@@ -23,6 +23,7 @@ struct ostream *test_ostream_create_nonblocking(buffer_t *output,
 void test_ostream_set_max_output_size(struct ostream *output, size_t max_size);
 
 void test_begin(const char *name);
+#define test_failed(reason) test_assert_failed(reason, __FILE__, __LINE__)
 #define test_assert(code) STMT_START { \
        if (!(code)) test_assert_failed(#code, __FILE__, __LINE__); \
        } STMT_END
@@ -51,6 +52,9 @@ void test_begin(const char *name);
                                                      __FILE__, __LINE__, _temp_s1, _temp_s2, i); \
        } STMT_END
 
+#define test_assert_cmp_bool(_bool_value1, _op, _value2) \
+       test_assert_cmp((unsigned int) _bool_value1, _op, (unsigned int _bool_value2))
+
 #define test_assert_cmp(_value1, _op, _value2) \
        test_assert_cmp_idx(_value1, _op, _value2, LLONG_MIN)
 #define test_assert_cmp_idx(_value1, _op, _value2, _idx) STMT_START { \