Merge branch 'getvalues' (pull request #2253)

Redmine#7116: adjust acceptance tests Redmine#7116: getvalues() will descend recursively into data containers Redmine#7116: getvalues() and getindices() accept indexed data containers Redmine#7116: getvalues() should always return a list
cfengine · Dec 11, 2015 · f235637 · f235637
2 parents f2e643b + 1bfcbc7
commit f235637
Show file tree

Hide file tree

Showing 6 changed files with 200 additions and 157 deletions.
diff --git a/libpromises/evalfunction.c b/libpromises/evalfunction.c
@@ -2133,39 +2133,83 @@ static FnCallResult FnCallRegArray(EvalContext *ctx, ARG_UNUSED const Policy *po
     return FnReturnContext(found);
 }
 
+/*********************************************************************/
+
+static char* JsonPrimitiveToString(const JsonElement *el)
+{
+    if (JsonGetElementType(el) != JSON_ELEMENT_TYPE_PRIMITIVE)
+    {
+        return NULL;
+    }
+
+    switch (JsonGetPrimitiveType(el))
+    {
+    case JSON_PRIMITIVE_TYPE_BOOL:
+        return xstrdup(JsonPrimitiveGetAsBool(el) ? "true" : "false");
+        break;
+
+    case JSON_PRIMITIVE_TYPE_INTEGER:
+        return StringFromLong(JsonPrimitiveGetAsInteger(el));
+        break;
+
+    case JSON_PRIMITIVE_TYPE_REAL:
+        return StringFromDouble(JsonPrimitiveGetAsReal(el));
+        break;
+
+    case JSON_PRIMITIVE_TYPE_STRING:
+        return xstrdup(JsonPrimitiveGetAsString(el));
+        break;
+
+    case JSON_PRIMITIVE_TYPE_NULL: // redundant
+        break;
+    }
+
+    return NULL;
+}
+
+/*********************************************************************/
 
 static FnCallResult FnCallGetIndices(EvalContext *ctx, ARG_UNUSED const Policy *policy, const FnCall *fp, const Rlist *finalargs)
 {
-    VarRef *ref = VarRefParse(RlistScalarValue(finalargs));
-    if (!VarRefIsQualified(ref))
+    VarRef *ref = ResolveAndQualifyVarName(fp, RlistScalarValue(finalargs));
+
+    DataType type = CF_DATA_TYPE_NONE;
+    EvalContextVariableGet(ctx, ref, &type);
+
+    if (type != CF_DATA_TYPE_CONTAINER)
     {
-        if (fp->caller)
-        {
-            const Bundle *caller_bundle = PromiseGetBundle(fp->caller);
-            VarRefQualify(ref, caller_bundle->ns, caller_bundle->name);
-        }
-        else
+        VarRefDestroy(ref);
+        ref = VarRefParse(RlistScalarValue(finalargs));
+        if (!VarRefIsQualified(ref))
         {
-            Log(LOG_LEVEL_WARNING,
-                "Function '%s' was given an unqualified variable reference, "
-                "and it was not called from a promise. No way to automatically qualify the reference '%s'.",
-                fp->name, RlistScalarValue(finalargs));
-            VarRefDestroy(ref);
-            return FnFailure();
+            if (fp->caller)
+            {
+                const Bundle *caller_bundle = PromiseGetBundle(fp->caller);
+                VarRefQualify(ref, caller_bundle->ns, caller_bundle->name);
+            }
+            else
+            {
+                Log(LOG_LEVEL_WARNING,
+                    "Function '%s' was given an unqualified variable reference, "
+                    "and it was not called from a promise. No way to automatically qualify the reference '%s'.",
+                    fp->name, RlistScalarValue(finalargs));
+                VarRefDestroy(ref);
+                return FnFailure();
+            }
         }
     }
 
-    DataType type = CF_DATA_TYPE_NONE;
-    const void *value = EvalContextVariableGet(ctx, ref, &type);
+    type = CF_DATA_TYPE_NONE;
+    const void *var_value = EvalContextVariableGet(ctx, ref, &type);
 
     Rlist *keys = NULL;
     if (type == CF_DATA_TYPE_CONTAINER)
     {
-        if (JsonGetElementType(value) == JSON_ELEMENT_TYPE_CONTAINER)
+        if (JsonGetElementType(var_value) == JSON_ELEMENT_TYPE_CONTAINER)
         {
-            if (JsonGetContainerType(value) == JSON_CONTAINER_TYPE_OBJECT)
+            if (JsonGetContainerType(var_value) == JSON_CONTAINER_TYPE_OBJECT)
             {
-                JsonIterator iter = JsonIteratorInit(value);
+                JsonIterator iter = JsonIteratorInit(var_value);
                 const char *key;
                 while ((key = JsonIteratorNextKey(&iter)))
                 {
@@ -2174,7 +2218,7 @@ static FnCallResult FnCallGetIndices(EvalContext *ctx, ARG_UNUSED const Policy *
             }
             else
             {
-                for (size_t i = 0; i < JsonLength(value); i++)
+                for (size_t i = 0; i < JsonLength(var_value); i++)
                 {
                     Rval key = (Rval) { StringFromLong(i), RVAL_TYPE_SCALAR };
                     RlistAppendRval(&keys, key);
@@ -2184,7 +2228,7 @@ static FnCallResult FnCallGetIndices(EvalContext *ctx, ARG_UNUSED const Policy *
     }
     else
     {
-        VariableTableIterator *iter = 
+        VariableTableIterator *iter =
                 EvalContextVariableTableFromRefIteratorNew(ctx, ref);
         const Variable *var;
         while ((var = VariableTableIteratorNext(iter)))
@@ -2204,79 +2248,78 @@ static FnCallResult FnCallGetIndices(EvalContext *ctx, ARG_UNUSED const Policy *
 
 /*********************************************************************/
 
-static char* JsonPrimitiveToString(const JsonElement *el)
+void CollectContainerValues(EvalContext *ctx, Rlist **values, const JsonElement *container)
 {
-    if (JsonGetElementType(el) != JSON_ELEMENT_TYPE_PRIMITIVE)
+    if (JsonGetElementType(container) == JSON_ELEMENT_TYPE_CONTAINER)
     {
-        return NULL;
+        JsonIterator iter = JsonIteratorInit(container);
+        const JsonElement *el;
+        while ((el = JsonIteratorNextValue(&iter)))
+        {
+            if (JsonGetElementType(el) == JSON_ELEMENT_TYPE_CONTAINER)
+            {
+                CollectContainerValues(ctx, values, el);
+            }
+            else
+            {
+                char *value = JsonPrimitiveToString(el);
+                if (NULL != value)
+                {
+                    RlistAppendScalar(values, value);
+                    free(value);
+                }
+            }
+        }
     }
-
-    switch (JsonGetPrimitiveType(el))
+    else if (JsonGetElementType(container) == JSON_ELEMENT_TYPE_PRIMITIVE)
     {
-    case JSON_PRIMITIVE_TYPE_BOOL:
-        return xstrdup(JsonPrimitiveGetAsBool(el) ? "true" : "false");
-        break;
-
-    case JSON_PRIMITIVE_TYPE_INTEGER:
-        return StringFromLong(JsonPrimitiveGetAsInteger(el));
-        break;
-
-    case JSON_PRIMITIVE_TYPE_REAL:
-        return StringFromDouble(JsonPrimitiveGetAsReal(el));
-        break;
-
-    case JSON_PRIMITIVE_TYPE_STRING:
-        return xstrdup(JsonPrimitiveGetAsString(el));
-        break;
-
-    case JSON_PRIMITIVE_TYPE_NULL: // redundant
-        break;
+        char *value = JsonPrimitiveToString(container);
+        if (NULL != value)
+        {
+            RlistAppendScalar(values, value);
+            free(value);
+        }
     }
-
-    return NULL;
 }
 
 static FnCallResult FnCallGetValues(EvalContext *ctx, ARG_UNUSED const Policy *policy, const FnCall *fp, const Rlist *finalargs)
 {
-    VarRef *ref = VarRefParse(RlistScalarValue(finalargs));
-    if (!VarRefIsQualified(ref))
+    VarRef *ref = ResolveAndQualifyVarName(fp, RlistScalarValue(finalargs));
+
+    DataType type = CF_DATA_TYPE_NONE;
+    EvalContextVariableGet(ctx, ref, &type);
+
+    if (type != CF_DATA_TYPE_CONTAINER)
     {
-        if (fp->caller)
-        {
-            const Bundle *caller_bundle = PromiseGetBundle(fp->caller);
-            VarRefQualify(ref, caller_bundle->ns, caller_bundle->name);
-        }
-        else
+        VarRefDestroy(ref);
+        ref = VarRefParse(RlistScalarValue(finalargs));
+        if (!VarRefIsQualified(ref))
         {
-            Log(LOG_LEVEL_WARNING,
-                "Function '%s' was given an unqualified variable reference, "
-                "and it was not called from a promise. No way to automatically qualify the reference '%s'.",
-                fp->name, RlistScalarValue(finalargs));
-            VarRefDestroy(ref);
-            return FnFailure();
+            if (fp->caller)
+            {
+                const Bundle *caller_bundle = PromiseGetBundle(fp->caller);
+                VarRefQualify(ref, caller_bundle->ns, caller_bundle->name);
+            }
+            else
+            {
+                Log(LOG_LEVEL_WARNING,
+                    "Function '%s' was given an unqualified variable reference, "
+                    "and it was not called from a promise. No way to automatically qualify the reference '%s'.",
+                    fp->name, RlistScalarValue(finalargs));
+                VarRefDestroy(ref);
+                return FnFailure();
+            }
         }
     }
 
-    DataType type = CF_DATA_TYPE_NONE;
-    const void *value = EvalContextVariableGet(ctx, ref, &type);
+    type = CF_DATA_TYPE_NONE;
+    const void *var_value = EvalContextVariableGet(ctx, ref, &type);
 
     Rlist *values = NULL;
+
     if (type == CF_DATA_TYPE_CONTAINER)
     {
-        if (JsonGetElementType(value) == JSON_ELEMENT_TYPE_CONTAINER)
-        {
-            JsonIterator iter = JsonIteratorInit(value);
-            const JsonElement *el;
-            while ((el = JsonIteratorNextValue(&iter)))
-            {
-                char *value = JsonPrimitiveToString(el);
-                if (NULL != value)
-                {
-                    RlistAppendScalar(&values, value);
-                    free(value);
-                }
-            }
-        }
+        CollectContainerValues(ctx, &values, var_value);
     }
     else
     {

diff --git a/tests/acceptance/01_vars/02_functions/getvalues_containers.cf b/tests/acceptance/01_vars/02_functions/getvalues_containers.cf
@@ -0,0 +1,32 @@
+body common control
+{
+      inputs => { "../../default.cf.sub" };
+      bundlesequence  => { default("$(this.promise_filename)") };
+      version => "1.0";
+}
+
+#######################################################
+
+bundle agent init
+{
+  vars:
+      "arr" data => '["a", [], { "x": 1 }, 2, 5.30, true]';
+}
+
+bundle agent test
+{
+  vars:
+      "arr_v" slist => getvalues("init.arr");
+
+reports:
+  DEBUG::
+    "arr_v $(arr_v)";
+}
+
+bundle agent check
+{
+  methods:
+      "check"  usebundle => dcs_check_state(test,
+                                           "$(this.promise_filename).expected.json",
+                                           $(this.promise_filename));
+}
diff --git a/tests/acceptance/01_vars/02_functions/getvalues_containers.cf.expected.json b/tests/acceptance/01_vars/02_functions/getvalues_containers.cf.expected.json
@@ -0,0 +1,9 @@
+{
+  "arr_v": [
+    "a",
+    "1",
+    "2",
+    "5.30",
+    "true"
+  ]
+}
diff --git a/...2_functions/getvalues_returns_all_values_for_given_datacontainer_index_merging_strings.cf b/...2_functions/getvalues_returns_all_values_for_given_datacontainer_index_merging_strings.cf
@@ -7,63 +7,48 @@
 
 body common control
 {
-      inputs => { "../../default.cf.sub" };
-      bundlesequence  => { default("$(this.promise_filename)") };
-      version => "1.0";
+        inputs => { "../../default.cf.sub" };
+        bundlesequence  => { default("$(this.promise_filename)") };
+        version => "1.0";
 }
 
 #######################################################
 
 bundle agent init
 {
   vars:
-    "data" data => parsejson('{
-  "foo": [
-          "alpha",
-          "bravo"
-         ],
+      "data" data => '
+{
+  "foo": [ "alpha", "bravo" ],
   "bar": "zulu"
-}');
+}';
+      "data2" data => '
+{
+  "foo": [ null, "bravo" ],
+  "zed": { "quu": "something else" },
+  "bar": null,
+  "bar2": 1233333
+}';
+
+      "data3" data => '[ "foo", null, "bravo", { "bar": "barista" } ]';
 }
 
 #######################################################
 
 bundle agent test
 {
-  meta:
-      "test_soft_fail" string => "any",
-        meta => { "redmine7116" };
-
   vars:
-    "values_data" slist => getvalues("init.data");
+      "values_data" slist => getvalues("init.data");
+      "values_data2" slist => getvalues("init.data2");
+      "values_data3" slist => getvalues("init.data3");
 }
 
 #######################################################
 
 bundle agent check
 {
-  vars:
-    "expected_elements" slist => { "alpha", "bravo", "zulu" };
-
-    "diff1" slist => difference( "test.values_data", expected_elements );
-    "diff2" slist => difference( expected_elements, "test.values_data");
-
-    "len_diff1" int => length(diff1);
-    "len_diff2" int => length(diff2);
-
-  classes:
-    "ok" expression => strcmp( $(len_diff1), $(len_diff2) );
-
-  reports:
-    DEBUG::
-      "DEBUG: data value: '$(test.values_data)'";
-      "DEBUG: expected value: '$(expected_elements)'";
-      "DEBUG: found '$(diff1)' in test.values_data, but not in expected_elements";
-      "DEBUG: found '$(diff2)' in expected_elements, but not in test.values_data";
-
-    ok::
-      "$(this.promise_filename) Pass";
-
-    !ok::
-      "$(this.promise_filename) FAIL";
+  methods:
+      "check"  usebundle => dcs_check_state(test,
+                                           "$(this.promise_filename).expected.json",
+                                           $(this.promise_filename));
 }