From: Grigorii Demidov <grigorii.demidov@nic.cz>
Date: Fri, 29 May 2015 11:12:17 +0000 (+0200)
Subject: lib\cache: removing of global instance of cache storage api
X-Git-Tag: v1.0.0-beta1~125^2~3^2~5
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e6e3434e23a4de49e0e9a0d7bc2f03ef8fda1cd7;p=thirdparty%2Fknot-resolver.git

lib\cache: removing of global instance of cache storage api
---

diff --git a/daemon/bindings.c b/daemon/bindings.c
index ac0001241..3c936f605 100644
--- a/daemon/bindings.c
+++ b/daemon/bindings.c
@@ -299,7 +299,7 @@ static int cache_backends(lua_State *L)
 	lua_newtable(L);
 	for (unsigned i = 0; i < registry->len; ++i) {
 		struct storage_api *storage = &registry->at[i];
-		lua_pushboolean(L, storage->api() == kr_cache_storage());
+		lua_pushboolean(L, storage->api() == engine->resolver.cache.api); /* kr_cache_storage()); */
 		lua_setfield(L, -2, storage->prefix);
 	}
 	return 1;
@@ -309,7 +309,7 @@ static int cache_backends(lua_State *L)
 static int cache_count(lua_State *L)
 {
 	struct engine *engine = engine_luaget(L);
-	const namedb_api_t *storage = kr_cache_storage();
+	const namedb_api_t *storage = engine->resolver.cache.api; /* kr_cache_storage(); */
 
 	/* Fetch item count */
 	struct kr_cache_txn txn;
@@ -390,9 +390,9 @@ static int cache_open(lua_State *L)
 	kr_cache_close(&engine->resolver.cache);
 
 	/* Reopen cache */
-	kr_cache_storage_set(storage->api);
+	/* kr_cache_storage_set(storage->api); */
 	void *storage_opts = storage->opts_create(conf, cache_size);
-	int ret = kr_cache_open(&engine->resolver.cache, storage_opts, engine->pool);
+	int ret = kr_cache_open(&engine->resolver.cache, storage->api(), storage_opts, engine->pool);
 	free(storage_opts);
 	if (ret != 0) {
 		format_error(L, "can't open cache");
diff --git a/lib/cache.c b/lib/cache.c
index fb9c100e1..9f639d6d0 100644
--- a/lib/cache.c
+++ b/lib/cache.c
@@ -30,22 +30,15 @@
 
 /* Key size */
 #define KEY_SIZE (sizeof(uint8_t) + KNOT_DNAME_MAXLEN + sizeof(uint16_t))
+#define db_api(cache) (cache->api)
 
-/**
- * Used cache storage engine (default LMDB)
- * @todo namedb_t should change so it always contains a pointer
- *       to its API, so it can be carried around instead of keeping it in a
- *       global variable.
- */
-const namedb_api_t *(*kr_cache_storage)(void) = namedb_lmdb_api;
-#define db_api kr_cache_storage()
-
-int kr_cache_open(struct kr_cache *cache, void *opts, mm_ctx_t *mm)
+int kr_cache_open(struct kr_cache *cache, const namedb_api_t *api, void *opts, mm_ctx_t *mm)
 {
 	if (!cache) {
 		return kr_error(EINVAL);
 	}
-	int ret = db_api->init(&cache->db, mm, opts);
+	db_api(cache) = (api == NULL) ? namedb_lmdb_api() : api;
+	int ret = db_api(cache)->init(&cache->db, mm, opts);
 	if (ret != 0) {
 		return ret;
 	}
@@ -56,8 +49,8 @@ int kr_cache_open(struct kr_cache *cache, void *opts, mm_ctx_t *mm)
 void kr_cache_close(struct kr_cache *cache)
 {
 	if (cache && cache->db) {
-		if (db_api) {
-			db_api->deinit(cache->db);
+		if (db_api(cache)) {
+			db_api(cache)->deinit(cache->db);
 		}
 		cache->db = NULL;
 	}
@@ -65,7 +58,7 @@ void kr_cache_close(struct kr_cache *cache)
 
 int kr_cache_txn_begin(struct kr_cache *cache, struct kr_cache_txn *txn, unsigned flags)
 {
-	if (!cache || !cache->db || !txn) {
+	if (!cache || !cache->db || !db_api(cache) || !txn ) {
 		return kr_error(EINVAL);
 	}
 
@@ -75,16 +68,16 @@ int kr_cache_txn_begin(struct kr_cache *cache, struct kr_cache_txn *txn, unsigne
 		cache->stats.txn_write += 1;
 	}
 	txn->owner = cache;
-	return db_api->txn_begin(cache->db, (namedb_txn_t *)txn, flags);
+	return db_api(cache)->txn_begin(cache->db, (namedb_txn_t *)txn, flags);
 }
 
 int kr_cache_txn_commit(struct kr_cache_txn *txn)
 {
-	if (!txn) {
+	if (!txn || !txn->owner) {
 		return kr_error(EINVAL);
 	}
 
-	int ret = db_api->txn_commit((namedb_txn_t *)txn);
+	int ret = db_api(txn->owner)->txn_commit((namedb_txn_t *)txn);
 	if (ret != 0) {
 		kr_cache_txn_abort(txn);
 	}
@@ -93,8 +86,8 @@ int kr_cache_txn_commit(struct kr_cache_txn *txn)
 
 void kr_cache_txn_abort(struct kr_cache_txn *txn)
 {
-	if (txn) {
-		db_api->txn_abort((namedb_txn_t *)txn);
+	if (txn && txn->owner) {
+		db_api(txn->owner)->txn_abort((namedb_txn_t *)txn);
 	}
 }
 
@@ -108,15 +101,18 @@ static size_t cache_key(uint8_t *buf, uint8_t tag, const knot_dname_t *name, uin
 	return len + sizeof(type);
 }
 
-static struct kr_cache_entry *cache_entry(namedb_txn_t *txn, uint8_t tag, const knot_dname_t *name, uint16_t type)
+static struct kr_cache_entry *cache_entry(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *name, uint16_t type)
 {
 	uint8_t keybuf[KEY_SIZE];
 	size_t key_len = cache_key(keybuf, tag, name, type);
+	if (!txn || !txn->owner) {
+		return NULL;
+	}
 
 	/* Look up and return value */
 	namedb_val_t key = { keybuf, key_len };
 	namedb_val_t val = { NULL, 0 };
-	int ret = db_api->find(txn, &key, &val, 0);
+	int ret = db_api(txn->owner)->find((namedb_txn_t *)txn, &key, &val, 0);
 	if (ret != KNOT_EOK) {
 		return NULL;
 	}
@@ -131,7 +127,7 @@ struct kr_cache_entry *kr_cache_peek(struct kr_cache_txn *txn, uint8_t tag, cons
 		return NULL;
 	}
 
-	struct kr_cache_entry *entry = cache_entry((namedb_txn_t *)txn, tag, name, type);
+	struct kr_cache_entry *entry = cache_entry(txn, tag, name, type);
 	if (!entry) {
 		txn->owner->stats.miss += 1;
 		return NULL;
@@ -170,7 +166,7 @@ static void entry_write(struct kr_cache_entry *dst, struct kr_cache_entry *heade
 int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *name, uint16_t type,
                     struct kr_cache_entry *header, namedb_val_t data)
 {
-	if (!txn || !name || !tag || !header) {
+	if (!txn || !txn->owner || !name || !tag || !header) {
 		return kr_error(EINVAL);
 	}
 
@@ -182,8 +178,8 @@ int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *n
 
 	/* LMDB can do late write and avoid copy */
 	txn->owner->stats.insert += 1;
-	if (db_api == namedb_lmdb_api()) {
-		int ret = db_api->insert((namedb_txn_t *)txn, &key, &entry, 0);
+	if (db_api(txn->owner) == namedb_lmdb_api()) {
+		int ret = db_api(txn->owner)->insert((namedb_txn_t *)txn, &key, &entry, 0);
 		if (ret != 0) {
 			return ret;
 		}
@@ -195,7 +191,7 @@ int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *n
 			return kr_error(ENOMEM);
 		}
 		entry_write(entry.data, header, data);
-		int ret = db_api->insert((namedb_txn_t *)txn, &key, &entry, 0);
+		int ret = db_api(txn->owner)->insert((namedb_txn_t *)txn, &key, &entry, 0);
 		free(entry.data);
 		if (ret != 0) {
 			return ret;
@@ -207,7 +203,7 @@ int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *n
 
 int kr_cache_remove(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *name, uint16_t type)
 {
-	if (!txn || !tag || !name ) {
+	if (!txn || !txn->owner || !tag || !name ) {
 		return kr_error(EINVAL);
 	}
 
@@ -215,16 +211,16 @@ int kr_cache_remove(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *n
 	size_t key_len = cache_key(keybuf, tag, name, type);
 	namedb_val_t key = { keybuf, key_len };
 	txn->owner->stats.delete += 1;
-	return db_api->del((namedb_txn_t *)txn, &key);
+	return db_api(txn->owner)->del((namedb_txn_t *)txn, &key);
 }
 
 int kr_cache_clear(struct kr_cache_txn *txn)
 {
-	if (!txn) {
+	if (!txn || !txn->owner ) {
 		return kr_error(EINVAL);
 	}
 
-	return db_api->clear((namedb_txn_t *)txn);
+	return db_api(txn->owner)->clear((namedb_txn_t *)txn);
 }
 
 int kr_cache_peek_rr(struct kr_cache_txn *txn, knot_rrset_t *rr, uint32_t *timestamp)
diff --git a/lib/cache.h b/lib/cache.h
index 55c2eb027..1b7c48526 100644
--- a/lib/cache.h
+++ b/lib/cache.h
@@ -43,15 +43,16 @@ struct kr_cache_entry
  */
 struct kr_cache
 {
-    namedb_t *db;             /**< Storage instance */
-    struct {
-        uint32_t hit;         /**< Number of cache hits */
-        uint32_t miss;        /**< Number of cache misses */
-        uint32_t insert;      /**< Number of insertions */
-        uint32_t delete;      /**< Number of deletions */
-        uint32_t txn_read;    /**< Number of read transactions */
-        uint32_t txn_write;   /**< Number of write transactions */
-    } stats;
+	namedb_t *db;		      /**< Storage instance */
+	const namedb_api_t *api;      /**< Storage engine */
+	struct {
+		uint32_t hit;         /**< Number of cache hits */
+		uint32_t miss;        /**< Number of cache misses */
+		uint32_t insert;      /**< Number of insertions */
+		uint32_t delete;      /**< Number of deletions */
+		uint32_t txn_read;    /**< Number of read transactions */
+		uint32_t txn_write;   /**< Number of write transactions */
+	} stats;
 };
 
 /** Cache transaction */
@@ -61,22 +62,23 @@ struct kr_cache_txn {
 };
 
 /** Used storage backend for cache (default LMDB) */
-extern const namedb_api_t *(*kr_cache_storage)(void);
+//extern const namedb_api_t *(*kr_cache_storage)(void);
 
 /** Replace used cache storage backend. */
-static inline void kr_cache_storage_set(const namedb_api_t *(*api)(void))
-{
-	kr_cache_storage = api;
-}
+//static inline void kr_cache_storage_set(const namedb_api_t *(*api)(void))
+//{
+//	kr_cache_storage = api;
+//}
 
 /**
  * Open/create cache with provided storage options.
  * @param cache cache structure to be initialized
+ * @param api Storage engine
  * @param storage_opts Storage-specific options (may be NULL for default)
  * @param mm Memory context.
  * @return 0 or an error code
  */
-int kr_cache_open(struct kr_cache *cache, void *opts, mm_ctx_t *mm);
+int kr_cache_open(struct kr_cache *cache, const namedb_api_t *api, void *opts, mm_ctx_t *mm);
 
 /**
  * Close persistent cache.
diff --git a/modules/cachectl/cachectl.c b/modules/cachectl/cachectl.c
index 39cf66110..e7dc22c2b 100644
--- a/modules/cachectl/cachectl.c
+++ b/modules/cachectl/cachectl.c
@@ -51,7 +51,7 @@ static bool is_expired(struct kr_cache_entry *entry, uint32_t drift)
 static char* prune(void *env, struct kr_module *module, const char *args)
 {
 	struct engine *engine = env;
-	const namedb_api_t *storage = kr_cache_storage();
+	const namedb_api_t *storage = engine->resolver.cache.api; /* kr_cache_storage(); */
 
 	struct kr_cache_txn txn;
 	int ret = kr_cache_txn_begin(&engine->resolver.cache, &txn, 0);
diff --git a/tests/test_cache.c b/tests/test_cache.c
index a4125d834..9312c74c2 100644
--- a/tests/test_cache.c
+++ b/tests/test_cache.c
@@ -39,32 +39,32 @@ namedb_val_t global_namedb_data = {namedb_data, NAMEDB_DATA_SIZE};
 #define CACHE_TIME 0
 
 /* Simulate init failure */
-static int test_init_failure(namedb_t **db_ptr, mm_ctx_t *mm, void *arg)
+static int fake_test_init(namedb_t **db_ptr, mm_ctx_t *mm, void *arg)
 {
-	return KNOT_EINVAL;
+	return mock();
 }
 
 /* Simulate commit failure */
-static int test_commit_failure(namedb_txn_t *txn)
+static int fake_test_commit(namedb_txn_t *txn)
 {
 	return KNOT_ESPACE;
 }
 
 /* Dummy abort */
-static void test_abort(namedb_txn_t *txn)
+static void fake_test_abort(namedb_txn_t *txn)
 {
 	return;
 }
 
 /* Stub for find */
-static int test_find(namedb_txn_t *txn, namedb_val_t *key, namedb_val_t *val, unsigned flags)
+static int fake_test_find(namedb_txn_t *txn, namedb_val_t *key, namedb_val_t *val, unsigned flags)
 {
 	val->data = &global_fake_ce;
 	return KNOT_EOK;
 }
 
 /* Stub for insert */
-static int test_ins(namedb_txn_t *txn, namedb_val_t *key, namedb_val_t *val, unsigned flags)
+static int fake_test_ins(namedb_txn_t *txn, namedb_val_t *key, namedb_val_t *val, unsigned flags)
 {
 	int err = KNOT_EINVAL, res_cmp;
 	struct kr_cache_entry *header = val->data;
@@ -81,19 +81,51 @@ static int test_ins(namedb_txn_t *txn, namedb_val_t *key, namedb_val_t *val, uns
 	return err;
 }
 
+/* Fake api */
+static namedb_api_t *fake_namedb_lmdb_api(void)
+{
+	static namedb_api_t fake_api = {
+		"lmdb_fake_api",
+		fake_test_init, NULL,
+		NULL, fake_test_commit, fake_test_abort,
+		NULL, NULL, fake_test_find, fake_test_ins, NULL,
+		NULL, NULL, NULL, NULL, NULL, NULL
+	};
+
+	return &fake_api;
+}
+
 /* Test cache open */
-static void test_open(void **state)
+static int test_open(void **state, namedb_api_t *api)
 {
 	static struct kr_cache cache;
 	struct namedb_lmdb_opts opts;
 	memset(&opts, 0, sizeof(opts));
 	opts.path = global_env;
 	opts.mapsize = CACHE_SIZE;
-	int ret = kr_cache_open(&cache, &opts, &global_mm);
-	assert_int_equal(ret, 0);
 	*state = &cache;
+	return kr_cache_open(&cache, api, &opts, &global_mm);
+}
+
+/* fake api test open */
+static void test_open_fake_api(void **state)
+{
+	bool res;
+	will_return(fake_test_init,KNOT_EOK);
+	assert_int_equal(test_open(state, fake_namedb_lmdb_api()),KNOT_EOK);
+	res = (((struct kr_cache *)(*state))->api == fake_namedb_lmdb_api());
+	assert_true(res);
+}
+
+static void test_open_conventional_api(void **state)
+{
+	bool res;
+	assert_int_equal(test_open(state, NULL),KNOT_EOK);
+	res = (((struct kr_cache *)(*state))->api == namedb_lmdb_api());
+	assert_true(res);
 }
 
+
 /* Test cache teardown. */
 static void test_close(void **state)
 {
@@ -101,7 +133,6 @@ static void test_close(void **state)
 	*state = NULL;
 }
 
-
 /* Open transaction */
 static struct kr_cache_txn *test_txn_write(void **state)
 {
@@ -118,46 +149,18 @@ static struct kr_cache_txn *test_txn_rdonly(void **state)
 	return &global_txn;
 }
 
-/* Fake api */
-static const namedb_api_t *namedb_lmdb_api_fake(void)
+/* test invalid parameters and some api failures */
+static void test_fake_invalid (void **state)
 {
-	static const namedb_api_t api_fake = {
-		"lmdb_api_fake",
-		test_init_failure, NULL,
-		NULL, test_commit_failure, test_abort,
-		NULL, NULL, test_find, test_ins, NULL,
-		NULL, NULL, NULL, NULL, NULL, NULL
-	};
-
-	return &api_fake;
+//	knot_dname_t dname[] = "";
+//	assert_null(kr_cache_open(NULL, NULL, NULL));
+//	assert_int_equal(kr_cache_peek(&global_txn, KR_CACHE_USER, dname, KNOT_RRTYPE_TSIG, 0),
+//		&global_fake_ce);
+//	assert_int_not_equal(kr_cache_txn_commit(&global_txn), KNOT_EOK);
+	will_return(fake_test_init,KNOT_EINVAL);
+	assert_int_equal(test_open(state, fake_namedb_lmdb_api()),KNOT_EINVAL);
 }
 
-static void test_failures(void **state)
-{
-	const namedb_api_t *(*kr_cache_storage_saved)(void);
-	void *ret_cache_peek;
-	int ret_commit;
-	int ret_open;
-	knot_dname_t dname[] = "";
-
-	/* Get read transaction */
-	struct kr_cache_txn *txn = test_txn_rdonly(state);
-	/* save original api */
-	kr_cache_storage_saved = kr_cache_storage;
-	/* fake to simulate failures or constant success */
-	kr_cache_storage_set(namedb_lmdb_api_fake);
-
-	/* call kr_cache_peek() with no time constraint */
-	ret_cache_peek = kr_cache_peek(txn, KR_CACHE_USER, dname, KNOT_RRTYPE_TSIG, 0);
-	ret_open = kr_cache_open(NULL, NULL, NULL);
-	ret_commit = kr_cache_txn_commit(txn);
-
-	/* restore */
-	kr_cache_storage_set(kr_cache_storage_saved);
-	assert_int_equal(ret_cache_peek, &global_fake_ce);
-	assert_int_not_equal(ret_open, KNOT_EOK);
-	assert_int_not_equal(ret_commit, KNOT_EOK);
-}
 
 /* Test invalid parameters and some api failures. */
 static void test_invalid(void **state)
@@ -175,6 +178,7 @@ static void test_invalid(void **state)
 	assert_int_not_equal(kr_cache_peek_rr(&global_txn, NULL, NULL), 0);
 	assert_int_not_equal(kr_cache_insert_rr(&global_txn, NULL, 0), 0);
 	assert_int_not_equal(kr_cache_insert_rr(NULL, NULL, 0), 0);
+	assert_int_equal(kr_cache_insert_rr(&global_txn, &global_rr, 0), 0);
 	assert_int_not_equal(kr_cache_insert(NULL, KR_CACHE_USER, dname,
 		KNOT_RRTYPE_TSIG, &global_fake_ce, global_namedb_data), 0);
 	assert_int_not_equal(kr_cache_insert(&global_txn, 0, dname,
@@ -190,13 +194,9 @@ static void test_invalid(void **state)
 }
 
 /* Test cache write */
-static void test_insert(void **state)
+static void test_insert_rr(void **state)
 {
-	const namedb_api_t *(*kr_cache_storage_saved)(void);
-	int i, ret_cache_insert;
-	knot_dname_t dname[] = "";
 	test_random_rr(&global_rr, CACHE_TTL);
-
 	struct kr_cache_txn *txn = test_txn_write(state);
 	int ret = kr_cache_insert_rr(txn, &global_rr, CACHE_TIME);
 	if (ret == KNOT_EOK) {
@@ -204,26 +204,7 @@ static void test_insert(void **state)
 	} else {
 		kr_cache_txn_abort(txn);
 	}
-
 	assert_int_equal(ret, KNOT_EOK);
-
-	memset(&global_fake_ce,0xAA,sizeof(global_fake_ce));
-	srand(time(NULL));
-	for (i = 0; i < NAMEDB_DATA_SIZE; i += 4)
-	{
-	    int r = rand();
-	    namedb_data[i] = r;
-	    namedb_data[i + 1] = r >> 8;
-	    namedb_data[i + 2] = r >> 16;
-	    namedb_data[i + 3] = r >> 24;
-	}
-
-	kr_cache_storage_saved = kr_cache_storage;
-	kr_cache_storage_set(namedb_lmdb_api_fake);
-	ret_cache_insert = kr_cache_insert(&global_txn, KR_CACHE_USER, dname,
-		KNOT_RRTYPE_TSIG, &global_fake_ce, global_namedb_data);
-	kr_cache_storage_set(kr_cache_storage_saved);
-	assert_int_equal(ret_cache_insert, KNOT_EOK);
 }
 
 /* Test cache read */
@@ -324,16 +305,15 @@ int main(void)
 
 	/* Invalid input */
 	const UnitTest tests_bad[] = {
-		group_test_setup(test_open),
-		unit_test(test_invalid),
-		unit_test(test_failures),
+		group_test_setup(test_open_fake_api),
+		unit_test(test_fake_invalid),
 		group_test_teardown(test_close)
 	};
 
 	const UnitTest tests[] = {
 	        /* Cache persistence */
-	        group_test_setup(test_open),
-	        unit_test(test_insert),
+	        group_test_setup(test_open_conventional_api),
+	        unit_test(test_insert_rr),
 	        unit_test(test_query),
 	        /* Cache aging */
 	        unit_test(test_query_aged),
diff --git a/tests/test_integration.c b/tests/test_integration.c
index 357129c8e..1a3220296 100644
--- a/tests/test_integration.c
+++ b/tests/test_integration.c
@@ -71,8 +71,7 @@ static PyObject* init(PyObject* self, PyObject* args)
 	memset(&opts, 0, sizeof(opts));
 	opts.path = global_tmpdir;
 	opts.mapsize = 100 * 4096;
-	kr_cache_storage_set(namedb_lmdb_api);
-	int ret = kr_cache_open(&global_context.cache, &opts, &global_mm);
+	int ret = kr_cache_open(&global_context.cache, NULL, &opts, &global_mm);
 	assert(ret == 0);
 
 	/* No configuration parsing support yet. */