Skip to content

Commit

Permalink
Fixes to condition code
Browse files Browse the repository at this point in the history
When processing conditions, the original data in the request must be
passed in, but when conditions are applied for REST requests, a
recursive descent of the data was taking place, and it was the
descending tree that was passed to the condition processing code.

Fix the code that checks existence of a STEP to allow the data to
be scanned for the step and that value to be used.

Fix some small memory leaks discovered in testing.
  • Loading branch information
tony-vanderpeet authored and carlgsmith committed Oct 7, 2024
1 parent 103869e commit 487eb1f
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 36 deletions.
2 changes: 1 addition & 1 deletion models/test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
<REFRESH>Sample REFRESH node</REFRESH>
</NODE>
<NODE name="config" >
<NODE name="type">
<NODE name="type" mode="rw">
<VALUE name="big" value="1"/>
<VALUE name="little" value="2"/>
</NODE>
Expand Down
56 changes: 38 additions & 18 deletions sch_conditions.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,14 @@
#include <inttypes.h>
#define APTERYX_XML_LIBXML2


typedef struct _cond_result
{
bool result;
char *path;
char *value;
char *step_value; /* actual value of step if step_exists has been called */
} cond_result;

typedef struct _cond_exists
{
bool match;
char *path;
} cond_exists;

typedef enum
{
PROC_F_IF_FEATURE = (1 << 0), /* Parent is a if-feature */
Expand Down Expand Up @@ -120,13 +115,17 @@ sch_axis_parent (char *path, char *step_path, cond_result *presult, bool self, i
static gboolean
sch_step_exists_traverse_nodes (GNode *node, gpointer data)
{
cond_exists *exists = (cond_exists *) data;
cond_result *exists = (cond_result *) data;
bool ret = false;
char *path = apteryx_node_path (node);

if (g_strcmp0 (path, exists->path) == 0)
{
exists->match = true;
exists->result = true;
if (APTERYX_HAS_VALUE (node))
{
exists->value = g_strdup (APTERYX_VALUE (node));
}

/* Returning true stops the tree traverse */
ret = true;
Expand All @@ -139,19 +138,24 @@ sch_step_exists_traverse_nodes (GNode *node, gpointer data)
static void
sch_step_exists (GNode *root, char *path, cond_result *presult)
{
cond_exists exists = { };
cond_result exists = { };

exists.path = path;
g_node_traverse (root, G_IN_ORDER, G_TRAVERSE_ALL, -1,
sch_step_exists_traverse_nodes, &exists);
presult->result = exists.match;
presult->result = exists.result;
presult->value = exists.value;
if (!presult->result)
{
GNode *tree = apteryx_get_tree (path);

if (tree)
{
presult->result = true;
if (APTERYX_HAS_VALUE (tree))
{
presult->value = g_strdup (APTERYX_VALUE (tree));
}
apteryx_free_tree (tree);
}
}
Expand All @@ -174,7 +178,6 @@ sch_process_operator (sch_instance *instance, GNode *root, char *path,
if (xnode->left)
sch_process_xnode (instance, root, path, step_path, xnode->left, &lresult,
depth + 1, flags);

if (xnode->right)
{
sch_process_xnode (instance, root, path, step_path, xnode->right, &rresult,
Expand All @@ -197,15 +200,21 @@ sch_process_operator (sch_instance *instance, GNode *root, char *path,

if (lresult.result)
{
if (xnode->left->type == XPATH_TYPE_STEP ||
(xnode->left->type == XPATH_TYPE_FUNCTION &&
(g_strcmp0 (xnode->left->name, "current") == 0 ||
g_strcmp0 (xnode->left->name, "boolean") == 0)))
if (xnode->left->type == XPATH_TYPE_FUNCTION &&
(g_strcmp0 (xnode->left->name, "current") == 0 ||
g_strcmp0 (xnode->left->name, "boolean") == 0))
{
new_step_path = lresult.value;
lresult.value = apteryx_get_string (new_step_path, NULL);
g_free (new_step_path);
}
else if (xnode->left->type == XPATH_TYPE_STEP)
{
/* We already looked up step value and saved it. */
g_free (lresult.value);
lresult.value = lresult.step_value;
lresult.step_value = NULL;
}
}

if (is_number)
Expand Down Expand Up @@ -403,6 +412,7 @@ sch_function_derived_from (sch_instance *instance, GNode *root, char *path,
if (!result.result)
{
g_free (result.value);
g_free (result.step_value);
return;
}
}
Expand All @@ -413,6 +423,7 @@ sch_function_derived_from (sch_instance *instance, GNode *root, char *path,
if (!s_node)
{
g_free (result.value);
g_free (result.step_value);
return;
}

Expand All @@ -423,6 +434,7 @@ sch_function_derived_from (sch_instance *instance, GNode *root, char *path,

xmlFree (name);
g_free (result.value);
g_free (result.step_value);
}

static void
Expand Down Expand Up @@ -452,6 +464,7 @@ sch_process_arg_list (sch_instance *instance, GNode *root, char *path,
}

g_free (result.value);
g_free (result.step_value);
}
}

Expand Down Expand Up @@ -485,6 +498,7 @@ sch_function_name (sch_instance *instance, GNode *root, char *path,
}
}
g_free (result.value);
g_free (result.step_value);
}
}

Expand Down Expand Up @@ -537,6 +551,7 @@ sch_function_if_feature (sch_instance *instance, GNode *root, char *path,
*flags &= ~PROC_F_IF_FEATURE;
presult->result = result.result;
g_free (result.value);
g_free (result.step_value);
}
}

Expand Down Expand Up @@ -567,6 +582,7 @@ sch_function_count (sch_instance *instance, GNode *root, char *path, char *step_
apteryx_free_tree (tree);
}
g_free (result.value);
g_free (result.step_value);
presult->value = g_strdup_printf ("%u", match_count);
presult->result = true;
}
Expand Down Expand Up @@ -685,11 +701,14 @@ sch_process_xnode (sch_instance *instance, GNode *root, char *path, char *step_p
g_free (new_step_path);
}

if (depth == 0 && presult->result)
if (presult->result)
{
/* This tests if a path exists */
/* This tests if a path exists, and gets its value */
sch_step_exists (root, presult->value, &result);
presult->result = result.result;
g_free (presult->step_value);
presult->step_value = result.value;
result.value = NULL;
}
*flags &= ~PROC_F_FIRST_CHILD;
break;
Expand Down Expand Up @@ -901,6 +920,7 @@ sch_process_condition (sch_instance *instance, GNode *root, char *path,
xnode = sch_xpath_parser (condition);
sch_process_xnode (instance, root, path, NULL, xnode, &result, 0, &flags);
g_free (result.value);
g_free (result.step_value);
sch_xpath_free_xnode_tree (xnode);

return result.result;
Expand Down
44 changes: 27 additions & 17 deletions schema.c
Original file line number Diff line number Diff line change
Expand Up @@ -3527,8 +3527,12 @@ static int _sch_strcmp_ll (const char *stra, const char *strb)
return a - b;
}

/*
* root - pointer to original top of tree
* node - where we are in recursive descent
*/
static json_t *
_sch_gnode_to_json (sch_instance * instance, sch_node * schema, xmlNs *ns, GNode * node, int flags, int depth)
_sch_gnode_to_json (sch_instance * instance, sch_node * schema, xmlNs *ns, GNode * root, GNode * node, int flags, int depth)
{
json_t *data = NULL;
char *colon;
Expand All @@ -3539,7 +3543,7 @@ _sch_gnode_to_json (sch_instance * instance, sch_node * schema, xmlNs *ns, GNode
/* Get the actual node name */
if (depth == 0 && strlen (APTERYX_NAME (node)) == 1)
{
return _sch_gnode_to_json (instance, schema, ns, node->children, flags, depth);
return _sch_gnode_to_json (instance, schema, ns, root, node->children, flags, depth);
}
else if (depth == 0 && APTERYX_NAME (node)[0] == '/')
{
Expand Down Expand Up @@ -3628,7 +3632,7 @@ _sch_gnode_to_json (sch_instance * instance, sch_node * schema, xmlNs *ns, GNode
sch_check_condition (schema, node, flags, &path, &condition);
if (condition)
{
if (!sch_process_condition (instance, node, path, condition))
if (!sch_process_condition (instance, root, path, condition))
{
g_free (condition);
g_free (path);
Expand Down Expand Up @@ -3690,7 +3694,7 @@ _sch_gnode_to_json (sch_instance * instance, sch_node * schema, xmlNs *ns, GNode
sch_gnode_sort_children (sch_node_child_first (schema), child);
for (GNode * field = child->children; field; field = field->next)
{
json_t *node = _sch_gnode_to_json (instance, sch_node_child_first (schema), ns, field, flags, depth + 1);
json_t *node = _sch_gnode_to_json (instance, sch_node_child_first (schema), ns, root, field, flags, depth + 1);
bool added = false;
if (flags & SCH_F_NS_PREFIX)
{
Expand Down Expand Up @@ -3724,7 +3728,7 @@ _sch_gnode_to_json (sch_instance * instance, sch_node * schema, xmlNs *ns, GNode
if (!child->data && (flags & SCH_F_DEPTH))
continue;

json_t *node = _sch_gnode_to_json (instance, schema, ns, child, flags, depth + 1);
json_t *node = _sch_gnode_to_json (instance, schema, ns, root, child, flags, depth + 1);
bool added = false;
if (flags & SCH_F_NS_PREFIX)
{
Expand Down Expand Up @@ -3817,7 +3821,7 @@ sch_gnode_to_json (sch_instance * instance, sch_node * schema, GNode * node, int
json_t *child;

tl_error = SCH_E_SUCCESS;
child = _sch_gnode_to_json (instance, pschema, ns, node, flags, g_node_depth (node) - 1);
child = _sch_gnode_to_json (instance, pschema, ns, node, node, flags, g_node_depth (node) - 1);
if (child)
{
char *name;
Expand Down Expand Up @@ -3930,9 +3934,9 @@ _sch_json_to_gnode (sch_instance * instance, sch_node * schema, xmlNs *ns,
return NULL;
}
}
char *key = generate_list_key_from_value (value);
APTERYX_LEAF (tree, key, value);
DEBUG (flags, "%*s%s = %s\n", depth * 2, " ", key, value);
char *lkey = generate_list_key_from_value (value);
APTERYX_LEAF (tree, lkey, value);
DEBUG (flags, "%*s%s = %s\n", depth * 2, " ", lkey, value);
}
}
/* LIST */
Expand Down Expand Up @@ -3960,11 +3964,12 @@ _sch_json_to_gnode (sch_instance * instance, sch_node * schema, xmlNs *ns,
{
ERROR (flags, SCH_E_KEYMISSING, "List \"%s\" missing key \"%s\"\n", name, key);
apteryx_free_tree (tree);
g_free (key);
return NULL;
}

char *key = generate_list_key_from_value (kname);
node = APTERYX_NODE (tree, key);
char *lkey = generate_list_key_from_value (kname);
node = APTERYX_NODE (tree, lkey);
free (kname);
DEBUG (flags, "%*s%s\n", depth * 2, " ", APTERYX_NAME (node));

Expand All @@ -3975,11 +3980,13 @@ _sch_json_to_gnode (sch_instance * instance, sch_node * schema, xmlNs *ns,
if (!cn)
{
apteryx_free_tree (tree);
g_free (key);
return NULL;
}
g_node_prepend (node, cn);
}
}
g_free (key);
}
/* CONTAINER */
else if (!sch_is_leaf (schema))
Expand Down Expand Up @@ -4024,7 +4031,6 @@ _sch_json_to_gnode (sch_instance * instance, sch_node * schema, xmlNs *ns,
return tree;
}

free (key);
return tree;
}

Expand Down Expand Up @@ -4078,8 +4084,12 @@ sch_json_to_gnode (sch_instance * instance, sch_node * schema, json_t * json, in
return root;
}

/*
* root - the original entire data tree
* parent - the place in the tree we are currently working from
*/
static bool
_sch_apply_conditions (sch_instance * instance, sch_node * schema, GNode * parent, int flags)
_sch_apply_conditions (sch_instance * instance, sch_node * schema, GNode * root, GNode * parent, int flags)
{
char *name = sch_name (schema);
GNode *child = apteryx_find_child (parent, name);
Expand Down Expand Up @@ -4147,7 +4157,7 @@ _sch_apply_conditions (sch_instance * instance, sch_node * schema, GNode * paren
sch_check_condition (schema, child, flags, &path, &condition);
if (condition)
{
if (!sch_process_condition (instance, child, path, condition))
if (!sch_process_condition (instance, root, path, condition))
{
g_free (condition);
g_free (path);
Expand All @@ -4167,7 +4177,7 @@ _sch_apply_conditions (sch_instance * instance, sch_node * schema, GNode * paren
{
for (sch_node *s = sch_node_child_first (schema); s; s = sch_node_next_sibling (s))
{
rc = _sch_apply_conditions (instance, s, child, flags);
rc = _sch_apply_conditions (instance, s, root, child, flags);
if (!rc)
goto exit;
}
Expand All @@ -4177,7 +4187,7 @@ _sch_apply_conditions (sch_instance * instance, sch_node * schema, GNode * paren
{
for (sch_node *s = sch_node_child_first (schema); s; s = sch_node_next_sibling (s))
{
rc = _sch_apply_conditions (instance, s, child, flags);
rc = _sch_apply_conditions (instance, s, root, child, flags);
if (!rc)
goto exit;
}
Expand All @@ -4199,7 +4209,7 @@ sch_apply_conditions (sch_instance * instance, sch_node * schema, GNode *node, i
{
for (sch_node *s = sch_node_child_first (schema); s; s = sch_node_next_sibling (s))
{
rc = _sch_apply_conditions (instance, s, node, flags);
rc = _sch_apply_conditions (instance, s, node, node, flags);
if (!rc)
break;
}
Expand Down

0 comments on commit 487eb1f

Please sign in to comment.