From 30ea545159fe6e72bada8e621e2b6f9d59490ba2 Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Wed, 9 Apr 2014 16:19:44 +0200 Subject: [PATCH] parser: Make sure that each comparator matcher is a host matcher. So far, 'NOT (service|attribute). ' matches did not work correctly, because the object matcher was not correctly wrapped into a host matcher. Since we do not currently support querying any object types besides hosts, this was wrong. The respective code has been moved into sdb_store_matcher_parse_cmp() which also implements and checks the restriction on matching host objects only. A new set of tests has been introduced which exposed this problem and which (roughly) verifies the result of a lookup using a parsed matcher. --- src/core/store_lookup.c | 12 ++++++--- src/frontend/grammar.y | 27 ++------------------ t/core/store_lookup_test.c | 52 +++++++++++++++++++++++++++++++------- 3 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/core/store_lookup.c b/src/core/store_lookup.c index 7416800..e5735d3 100644 --- a/src/core/store_lookup.c +++ b/src/core/store_lookup.c @@ -550,7 +550,9 @@ sdb_store_matcher_parse_cmp(const char *obj_type, const char *attr, /* accept */ } else if (typ == SDB_ATTRIBUTE) - m = sdb_store_attr_matcher(attr, NULL, matcher, matcher_re); + m = sdb_store_host_matcher(/* name = */ NULL, NULL, + /* service = */ NULL, + sdb_store_attr_matcher(attr, NULL, matcher, matcher_re)); else return NULL; @@ -560,9 +562,13 @@ sdb_store_matcher_parse_cmp(const char *obj_type, const char *attr, else if (typ == SDB_HOST) m = sdb_store_host_matcher(matcher, matcher_re, NULL, NULL); else if (typ == SDB_SERVICE) - m = sdb_store_service_matcher(matcher, matcher_re, NULL); + m = sdb_store_host_matcher(/* name = */ NULL, NULL, + sdb_store_service_matcher(matcher, matcher_re, NULL), + /* attr = */ NULL); else if (typ == SDB_ATTRIBUTE) - m = sdb_store_attr_matcher(matcher, matcher_re, NULL, NULL); + m = sdb_store_host_matcher(/* name = */ NULL, NULL, + /* service = */ NULL, + sdb_store_attr_matcher(matcher, matcher_re, NULL, NULL)); if (m && inv) { sdb_store_matcher_t *tmp; diff --git a/src/frontend/grammar.y b/src/frontend/grammar.y index 1cf0685..a261c4c 100644 --- a/src/frontend/grammar.y +++ b/src/frontend/grammar.y @@ -232,8 +232,7 @@ lookup_statement: expression: matcher { - sdb_store_matcher_t *m = $1; - if (! m) { + if (! $1) { /* TODO: improve error reporting */ sdb_fe_yyerror(&yylloc, scanner, YY_("syntax error, invalid expression")); @@ -243,29 +242,7 @@ expression: $$ = SDB_CONN_NODE(sdb_object_create_dT(/* name = */ NULL, conn_node_matcher_t, conn_matcher_destroy)); $$->cmd = CONNECTION_EXPR; - - if ((M(m)->type == MATCHER_HOST) - || (M(m)->type == MATCHER_AND) - || (M(m)->type == MATCHER_OR) - || (M(m)->type == MATCHER_NOT)) - CONN_MATCHER($$)->matcher = m; - else if (M(m)->type == MATCHER_SERVICE) - CONN_MATCHER($$)->matcher = sdb_store_host_matcher(NULL, - /* name_re = */ NULL, /* service = */ m, - /* attr = */ NULL); - else if (M(m)->type == MATCHER_ATTR) - CONN_MATCHER($$)->matcher = sdb_store_host_matcher(NULL, - /* name_re = */ NULL, /* service = */ NULL, - /* attr = */ m); - else { - char errbuf[1024]; - snprintf(errbuf, sizeof(errbuf), - YY_("syntax error, unexpected matcher type %d"), - M(m)->type); - sdb_object_deref(SDB_OBJ($$)); - sdb_fe_yyerror(&yylloc, scanner, errbuf); - YYABORT; - } + CONN_MATCHER($$)->matcher = $1; } ; diff --git a/t/core/store_lookup_test.c b/t/core/store_lookup_test.c index a16d343..3b9a6d0 100644 --- a/t/core/store_lookup_test.c +++ b/t/core/store_lookup_test.c @@ -27,6 +27,7 @@ #include "core/store.h" #include "core/store-private.h" +#include "frontend/parser.h" #include "libsysdb_test.h" #include @@ -354,20 +355,20 @@ START_TEST(test_parse_cmp) { "host", "attr", "=", "hostname", -1 }, { "host", "attr", "!=", "hostname", -1 }, { "host", "name", "&^", "hostname", -1 }, - { "service", "name", "=", "srvname", MATCHER_SERVICE }, + { "service", "name", "=", "srvname", MATCHER_HOST }, { "service", "name", "!=", "srvname", MATCHER_NOT }, - { "service", "name", "=~", "srvname", MATCHER_SERVICE }, + { "service", "name", "=~", "srvname", MATCHER_HOST }, { "service", "name", "!~", "srvname", MATCHER_NOT }, { "service", "attr", "=", "srvname", -1 }, { "service", "attr", "!=", "srvname", -1 }, { "service", "name", "&^", "srvname", -1 }, - { "attribute", "name", "=", "attrname", MATCHER_ATTR }, + { "attribute", "name", "=", "attrname", MATCHER_HOST }, { "attribute", "name", "!=", "attrname", MATCHER_NOT }, - { "attribute", "name", "=~", "attrname", MATCHER_ATTR }, + { "attribute", "name", "=~", "attrname", MATCHER_HOST }, { "attribute", "name", "!~", "attrname", MATCHER_NOT }, - { "attribute", "attr", "=", "attrname", MATCHER_ATTR }, + { "attribute", "attr", "=", "attrname", MATCHER_HOST }, { "attribute", "attr", "!=", "attrname", MATCHER_NOT }, - { "attribute", "attr", "=~", "attrname", MATCHER_ATTR }, + { "attribute", "attr", "=~", "attrname", MATCHER_HOST }, { "attribute", "attr", "!~", "attrname", MATCHER_NOT }, { "attribute", "attr", "&^", "attrname", -1 }, }; @@ -404,7 +405,7 @@ END_TEST static int lookup_cb(sdb_store_base_t *obj, void *user_data) { - intptr_t *i = user_data; + int *i = user_data; fail_unless(obj != NULL, "sdb_store_lookup callback received NULL obj; expected: " @@ -419,14 +420,47 @@ lookup_cb(sdb_store_base_t *obj, void *user_data) START_TEST(test_lookup) { - intptr_t i = 0; - int check; + struct { + const char *query; + int expected; + } golden_data[] = { + { "host.name = 'a'", 1 }, + { "host.name =~ 'a|b'", 2 }, + { "host.name =~ 'host'", 0 }, + { "host.name =~ '.'", 3 }, + { "service.name = 's1'", 2 }, + { "service.name =~ 's'", 2 }, + { "service.name !~ 's'", 1 }, + { "attribute.name = 'k1'", 1 }, + { "attribute.name = 'x'", 0 }, + { "attribute.k1 = 'v1'", 1 }, + { "attribute.k1 != 'v1'", 2 }, + { "attribute.k1 != 'v2'", 3 }, + }; + + size_t i; + int check, n; + i = 0; check = sdb_store_lookup(NULL, lookup_cb, &i); fail_unless(check == 0, "sdb_store_lookup() = %d; expected: 0", check); fail_unless(i == 3, "sdb_store_lookup called callback %d times; expected: 3", (int)i); + + for (i = 0; i < SDB_STATIC_ARRAY_LEN(golden_data); ++i) { + sdb_store_matcher_t *m = sdb_fe_parse_matcher(golden_data[i].query, -1); + fail_unless(m != NULL, + "sdb_fe_parse_matcher(%s, -1) = NULL; expected: ", + golden_data[i].query); + + n = 0; + sdb_store_lookup(m, lookup_cb, &n); + fail_unless(n == golden_data[i].expected, + "sdb_store_lookup(matcher{%s}) found %d hosts; expected: %d", + golden_data[i].query, n, golden_data[i].expected); + sdb_object_deref(SDB_OBJ(m)); + } } END_TEST -- 2.30.2