From 605f77151802827eec0eecb5a5a6732e899aaab0 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Mon, 23 Apr 2018 13:46:24 +0300 Subject: [PATCH] MXS-1401 Extend rule parsing to handle array of rule objects It's now possible to have a rules file with an array of rule objects, e.g. [ { store: [ ... ], use: [ ... ] }, { store: [ ... ], use: [ ... ] } ] This commit only contains the low-level modifications for supporting that; the upper-level modifications are made in another commit. --- server/modules/filter/cache/rules.cc | 159 +++++++++++++++--- server/modules/filter/cache/rules.h | 28 ++- server/modules/filter/cache/test/testrules.cc | 109 +++++++----- 3 files changed, 218 insertions(+), 78 deletions(-) diff --git a/server/modules/filter/cache/rules.cc b/server/modules/filter/cache/rules.cc index 8c32f995d..a25b9f483 100644 --- a/server/modules/filter/cache/rules.cc +++ b/server/modules/filter/cache/rules.cc @@ -153,6 +153,8 @@ static void cache_rule_free(CACHE_RULE *rule); static void cache_rules_add_store_rule(CACHE_RULES* self, CACHE_RULE* rule); static void cache_rules_add_use_rule(CACHE_RULES* self, CACHE_RULE* rule); static CACHE_RULES* cache_rules_create_from_json(json_t* root, uint32_t debug); +static bool cache_rules_create_from_json(json_t* root, uint32_t debug, + CACHE_RULES*** ppRules, int32_t* pnRules); static bool cache_rules_parse_json(CACHE_RULES* self, json_t* root); typedef bool (*cache_rules_parse_element_t)(CACHE_RULES *self, json_t *object, size_t index); @@ -228,57 +230,65 @@ CACHE_RULES *cache_rules_create(uint32_t debug) return rules; } -CACHE_RULES *cache_rules_load(const char *path, uint32_t debug) +bool cache_rules_load(const char* zPath, uint32_t debug, + CACHE_RULES*** pppRules, int32_t* pnRules) { - CACHE_RULES *rules = NULL; + bool rv = false; - FILE *fp = fopen(path, "r"); + *pppRules = NULL; + *pnRules = 0; - if (fp) + FILE* pF = fopen(zPath, "r"); + + if (pF) { json_error_t error; - json_t *root = json_loadf(fp, JSON_DISABLE_EOF_CHECK, &error); + json_t* pRoot = json_loadf(pF, JSON_DISABLE_EOF_CHECK, &error); - if (root) + if (pRoot) { - rules = cache_rules_create_from_json(root, debug); + rv = cache_rules_create_from_json(pRoot, debug, pppRules, pnRules); - if (!rules) + if (!rv) { - json_decref(root); + json_decref(pRoot); } } else { MXS_ERROR("Loading rules file failed: (%s:%d:%d): %s", - path, error.line, error.column, error.text); + zPath, error.line, error.column, error.text); } - fclose(fp); + fclose(pF); } else { MXS_ERROR("Could not open rules file %s for reading: %s", - path, mxs_strerror(errno)); + zPath, mxs_strerror(errno)); } - return rules; + return rv; } -CACHE_RULES *cache_rules_parse(const char *json, uint32_t debug) +bool cache_rules_parse(const char* zJson, uint32_t debug, + CACHE_RULES*** pppRules, int32_t* pnRules) { - CACHE_RULES *rules = NULL; + bool rv = false; + + *pppRules = NULL; + *pnRules = 0; json_error_t error; - json_t *root = json_loads(json, JSON_DISABLE_EOF_CHECK, &error); + json_t* pRoot = json_loads(zJson, JSON_DISABLE_EOF_CHECK, &error); - if (root) + if (pRoot) { - rules = cache_rules_create_from_json(root, debug); + rv = cache_rules_create_from_json(pRoot, debug, pppRules, pnRules); - if (!rules) + if (!rv) { - json_decref(root); + json_decref(pRoot); } } else @@ -287,7 +297,7 @@ CACHE_RULES *cache_rules_parse(const char *json, uint32_t debug) error.line, error.column, error.text); } - return rules; + return rv; } void cache_rules_free(CACHE_RULES *rules) @@ -414,11 +424,17 @@ CacheRules* CacheRules::load(const char *zPath, uint32_t debug) { CacheRules* pThis = NULL; - CACHE_RULES* pRules = cache_rules_load(zPath, debug); + CACHE_RULES** ppRules; + int32_t nRules; - if (pRules) + if (cache_rules_load(zPath, debug, &ppRules, &nRules)) { - pThis = new (std::nothrow) CacheRules(pRules); + // TODO: Handle more that one CACHE_RULES object at this level. + ss_dassert(nRules == 1); + + pThis = new (std::nothrow) CacheRules(ppRules[0]); + + MXS_FREE(ppRules); } return pThis; @@ -1874,7 +1890,7 @@ static void cache_rules_add_use_rule(CACHE_RULES* self, CACHE_RULE* rule) /** * Creates a rules object from a JSON object. * - * @param root The root JSON object in the rules file. + * @param root The root JSON rule object. * @param debug The debug level. * * @return A rules object if the json object could be parsed, NULL otherwise. @@ -1901,6 +1917,99 @@ static CACHE_RULES* cache_rules_create_from_json(json_t* root, uint32_t debug) return rules; } +/** + * Parses the caching rules from a json object and returns corresponding object(s). + * + * @param pRoot The root JSON object in the rules file. + * @param debug The debug level. + * @param pppRules [out] Pointer to array of pointers to CACHE_RULES objects. + * @param pnRules [out] Pointer to number of items in *ppRules. + * + * @note The caller must free the array @c *ppRules and each rules + * object in the array. + * + * @return bool True, if the rules could be parsed, false otherwise. + */ +static bool cache_rules_create_from_json(json_t* pRoot, uint32_t debug, + CACHE_RULES*** pppRules, int32_t* pnRules) +{ + bool rv = false; + + *pppRules = NULL; + *pnRules = 0; + + if (json_is_array(pRoot)) + { + int32_t nRules = json_array_size(pRoot); + + CACHE_RULES** ppRules = (CACHE_RULES**)MXS_MALLOC(nRules * sizeof(CACHE_RULES*)); + + if (ppRules) + { + int i; + for (i = 0; i < nRules; ++i) + { + json_t* pObject = json_array_get(pRoot, i); + ss_dassert(pObject); + + CACHE_RULES* pRules = cache_rules_create_from_json(pObject, debug); + + if (pRules) + { + ppRules[i] = pRules; + } + else + { + break; + } + } + + if (i == nRules) + { + *pppRules = ppRules; + *pnRules = nRules; + + rv = true; + } + else + { + // Ok, so something went astray. + for (int j = 0; j < i; ++j) + { + cache_rules_free(ppRules[j]); + } + + MXS_FREE(ppRules); + } + } + } + else + { + CACHE_RULES** ppRules = (CACHE_RULES**)MXS_MALLOC(1 * sizeof(CACHE_RULES*)); + + if (ppRules) + { + CACHE_RULES* pRules = cache_rules_create_from_json(pRoot, debug); + + if (pRules) + { + ppRules[0] = pRules; + + *pppRules = ppRules; + *pnRules = 1; + + rv = true; + } + else + { + MXS_FREE(ppRules); + } + } + } + + return rv; +} + /** * Parses the JSON object used for configuring the rules. * diff --git a/server/modules/filter/cache/rules.h b/server/modules/filter/cache/rules.h index ae4ff208f..23cb8f816 100644 --- a/server/modules/filter/cache/rules.h +++ b/server/modules/filter/cache/rules.h @@ -108,22 +108,34 @@ void cache_rules_free(CACHE_RULES *rules); /** * Loads the caching rules from a file and returns corresponding object. * - * @param path The path of the file containing the rules. - * @param debug The debug level. + * @param path The path of the file containing the rules. + * @param debug The debug level. + * @param pppRules [out] Pointer to array of pointers to CACHE_RULES objects. + * @param pnRules [out] Pointer to number of items in @c *ppRules. * - * @return The corresponding rules object, or NULL in case of error. + * @note The caller must free the array @c *pppRules and each rules + * object in the array. + * + * @return bool True, if the rules could be loaded, false otherwise. */ -CACHE_RULES *cache_rules_load(const char *path, uint32_t debug); +bool cache_rules_load(const char* zPath, uint32_t debug, + CACHE_RULES*** pppRules, int32_t* pnRules); /** * Parses the caching rules from a string and returns corresponding object. * - * @param json String containing json. - * @param debug The debug level. + * @param json String containing json. + * @param debug The debug level. + * @param pppRules [out] Pointer to array of pointers to CACHE_RULES objects. + * @param pnRules [out] Pointer to number of items in *ppRules. * - * @return The corresponding rules object, or NULL in case of error. + * @note The caller must free the array @c *ppRules and each rules + * object in the array. + * + * @return bool True, if the rules could be parsed, false otherwise. */ -CACHE_RULES *cache_rules_parse(const char *json, uint32_t debug); +bool cache_rules_parse(const char *json, uint32_t debug, + CACHE_RULES*** pppRules, int32_t* pnRules); /** * Prints the rules. diff --git a/server/modules/filter/cache/test/testrules.cc b/server/modules/filter/cache/test/testrules.cc index 7628a3e6c..0c96d06c5 100644 --- a/server/modules/filter/cache/test/testrules.cc +++ b/server/modules/filter/cache/test/testrules.cc @@ -84,33 +84,42 @@ int test_user() for (size_t i = 0; i < n_user_test_cases; ++i) { - const struct user_test_case *test_case = &user_test_cases[i]; + const struct user_test_case& test_case = user_test_cases[i]; - CACHE_RULES *rules = cache_rules_parse(test_case->json, 0); - ss_dassert(rules); + CACHE_RULES** ppRules; + int32_t nRules; + bool rv = cache_rules_parse(test_case.json, 0, &ppRules, &nRules); + ss_dassert(rv); - CACHE_RULE *rule = rules->use_rules; - ss_dassert(rule); - - if (rule->op != test_case->expect.op) + for (int i = 0; i < nRules; ++i) { - printf("%s\nExpected: %s,\nGot : %s\n", - test_case->json, - cache_rule_op_to_string(test_case->expect.op), - cache_rule_op_to_string(rule->op)); - ++errors; + CACHE_RULES* pRules = ppRules[i]; + + CACHE_RULE* pRule = pRules->use_rules; + ss_dassert(pRule); + + if (pRule->op != test_case.expect.op) + { + printf("%s\nExpected: %s,\nGot : %s\n", + test_case.json, + cache_rule_op_to_string(test_case.expect.op), + cache_rule_op_to_string(pRule->op)); + ++errors; + } + + if (strcmp(pRule->value, test_case.expect.value) != 0) + { + printf("%s\nExpected: %s,\nGot : %s\n", + test_case.json, + test_case.expect.value, + pRule->value); + ++errors; + } + + cache_rules_free(pRules); } - if (strcmp(rule->value, test_case->expect.value) != 0) - { - printf("%s\nExpected: %s,\nGot : %s\n", - test_case->json, - test_case->expect.value, - rule->value); - ++errors; - } - - cache_rules_free(rules); + MXS_FREE(ppRules); } return errors; @@ -188,35 +197,45 @@ int test_store() for (size_t i = 0; i < n_store_test_cases; ++i) { printf("TC : %d\n", (int)(i + 1)); - const struct store_test_case *test_case = &store_test_cases[i]; + const struct store_test_case& test_case = store_test_cases[i]; - CACHE_RULES *rules = cache_rules_parse(test_case->rule, 0); - ss_dassert(rules); + CACHE_RULES** ppRules; + int32_t nRules; - CACHE_RULE *rule = rules->store_rules; - ss_dassert(rule); + bool rv = cache_rules_parse(test_case.rule, 0, &ppRules, &nRules); + ss_dassert(rv); - GWBUF *packet = create_gwbuf(test_case->query); - - bool matches = cache_rules_should_store(rules, 0, test_case->default_db, packet); - - if (matches != test_case->matches) + for (int i = 0; i < nRules; ++i) { - printf("Query : %s\n" - "Rule : %s\n" - "Def-db : %s\n" - "Expected: %s\n" - "Result : %s\n\n", - test_case->query, - test_case->rule, - test_case->default_db, - test_case->matches ? "A match" : "Not a match", - matches ? "A match" : "Not a match"); + CACHE_RULES* pRules = ppRules[i]; + + CACHE_RULE* pRule = pRules->store_rules; + ss_dassert(pRule); + + GWBUF* pPacket = create_gwbuf(test_case.query); + + bool matches = cache_rules_should_store(pRules, 0, test_case.default_db, pPacket); + + if (matches != test_case.matches) + { + printf("Query : %s\n" + "Rule : %s\n" + "Def-db : %s\n" + "Expected: %s\n" + "Result : %s\n\n", + test_case.query, + test_case.rule, + test_case.default_db, + test_case.matches ? "A match" : "Not a match", + matches ? "A match" : "Not a match"); + } + + gwbuf_free(pPacket); + + cache_rules_free(pRules); } - gwbuf_free(packet); - - cache_rules_free(rules); + MXS_FREE(ppRules); } return errors;