parser: Make sure that each comparator matcher is a host matcher.

So far, 'NOT (service|attribute).<attr> <cmp> <value>' 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.

diff --git a/src/core/store_lookup.c b/src/core/store_lookup.c
index 7416800a2281dd398ee90af83b5942c3eaf3eb07..e5735d3728e8157ebc90d3157c0278ea6b42acdb 100644 (file)
--- 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 1cf068538bc13f7686fc12df2d0ba22247f9d553..a261c4c81d3253320b9d8cf960378918f84d0249 100644 (file)
--- 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 a16d343d37d0a9991e244d419c6226ecc64c23d5..3b9a6d0cc9c65dec37cdd9fe16a109a7d2e370f8 100644 (file)
--- 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 <check.h>
@@ -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: <matcher>",
+                               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