From cb07b3a8e9cde563a589979a8124eedc36a2020d Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Mon, 10 Feb 2025 11:20:41 +0100 Subject: [PATCH] Unit tests covering builtin mode Add unit test for builtin mode logic as well as few tidy-up in the code (adding some assertions and API documentation), in particular, to make it clear that builtin-mode is tight to an opened clause, so it must be set to a currently opened clause. --- lib/isc/cfgmgr.c | 5 ++ lib/isc/include/isc/cfgmgr.h | 5 +- tests/isc/cfgmgr_test.c | 89 ++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/lib/isc/cfgmgr.c b/lib/isc/cfgmgr.c index b7fc77060c..8a1857fcb9 100644 --- a/lib/isc/cfgmgr.c +++ b/lib/isc/cfgmgr.c @@ -391,6 +391,7 @@ isc_cfgmgr_builtinon(void) { REQUIRE(isc__cfgmgr_env != NULL); REQUIRE(isc__cfgmgr_ctx.txn != NULL); REQUIRE(isc__cfgmgr_ctx.cursor != NULL); + REQUIRE(ISC_LIST_EMPTY(isc__cfgmgr_ctx.openedclauses) == false); REQUIRE(isc__cfgmgr_ctx.builtinon == false); isc__cfgmgr_ctx.builtinon = true; @@ -406,6 +407,7 @@ isc_cfgmgr_builtinoff(void) { REQUIRE(isc__cfgmgr_ctx.txn != NULL); REQUIRE(isc__cfgmgr_ctx.cursor != NULL); REQUIRE(isc__cfgmgr_ctx.builtinon == true); + REQUIRE(ISC_LIST_EMPTY(isc__cfgmgr_ctx.openedclauses) == false); REQUIRE(strncmp(isc__cfgmgr_ctx.prefix, BUILTIN_PREFIX, strlen(BUILTIN_PREFIX)) == 0); @@ -488,6 +490,9 @@ isc_cfgmgr_close(void) { REQUIRE(isc__cfgmgr_ctx.txn != NULL); REQUIRE(isc__cfgmgr_ctx.cursor != NULL); + if (isc__cfgmgr_ctx.builtinon) { + isc_cfgmgr_builtinoff(); + } clause = ISC_LIST_HEAD(isc__cfgmgr_ctx.openedclauses); ISC_LIST_UNLINK(isc__cfgmgr_ctx.openedclauses, clause, link); isc_mem_free(isc__cfgmgr_mctx, clause->name); diff --git a/lib/isc/include/isc/cfgmgr.h b/lib/isc/include/isc/cfgmgr.h index 9d731b611f..a642083e3a 100644 --- a/lib/isc/include/isc/cfgmgr.h +++ b/lib/isc/include/isc/cfgmgr.h @@ -48,7 +48,10 @@ typedef struct isc_cfgmgr_val { * Thoe both function enable or disable the "builtin" mode. (disabled by * default, and automatically disabled anytime a transaction finished). This * implicitely namespacing the clause name when reading or - * writting values in cfgmgr. Must be called under a transaction. + * writting values in cfgmgr. Must be called under a transaction and with an + * opened clause. + * + * Closing the close automatically turn the builtin mode off. */ void isc_cfgmgr_builtinon(void); diff --git a/tests/isc/cfgmgr_test.c b/tests/isc/cfgmgr_test.c index 6dc382b8d2..b38433bc51 100644 --- a/tests/isc/cfgmgr_test.c +++ b/tests/isc/cfgmgr_test.c @@ -1531,6 +1531,94 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_opentests) { isc_cfgmgr_deinit(); } +ISC_RUN_TEST_IMPL(isc_cfgmgr_builtintests) { + isc_result_t result; + isc_cfgmgr_val_t val; + + result = isc_cfgmgr_init(mctx, TEST_DBPATH); + assert_int_equal(result, ISC_R_SUCCESS); + + result = isc_cfgmgr_rwtransaction(); + assert_int_equal(result, ISC_R_SUCCESS); + + isc_cfgmgr_newclause("foo"); + + /* + * Set a user property and check that it is not possible to read it + * from builtin mode + */ + val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, + .string = "userval1" }; + result = isc_cfgmgr_setval("userprop1", &val); + assert_int_equal(result, ISC_R_SUCCESS); + + expect_assert_failure(isc_cfgmgr_builtinoff()); + isc_cfgmgr_builtinon(); + expect_assert_failure(isc_cfgmgr_builtinon()); + + result = isc_cfgmgr_getval("userprop1", &val); + assert_int_equal(result, ISC_R_NOTFOUND); + + /* + * Set a builtin property and check that is is readable from non-builtin + * mode + */ + val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_UINT32, .uint32 = 1234 }; + result = isc_cfgmgr_setval("builtinval1", &val); + assert_int_equal(result, ISC_R_SUCCESS); + + val = (isc_cfgmgr_val_t){}; + isc_cfgmgr_builtinoff(); + expect_assert_failure(isc_cfgmgr_builtinoff()); + + result = isc_cfgmgr_getval("builtinval1", &val); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(val.type, ISC_CFGMGR_UINT32); + assert_int_equal(val.uint32, 1234); + + /* + * user override builtin property, now only the user value is visible in + * builtin mode off, but the original value is still readable with + * builtin mode on + */ + val.uint32 = 5678; + result = isc_cfgmgr_setval("builtinval1", &val); + assert_int_equal(result, ISC_R_SUCCESS); + + val = (isc_cfgmgr_val_t){}; + result = isc_cfgmgr_getval("builtinval1", &val); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(val.type, ISC_CFGMGR_UINT32); + assert_int_equal(val.uint32, 5678); + + isc_cfgmgr_builtinon(); + val = (isc_cfgmgr_val_t){}; + result = isc_cfgmgr_getval("builtinval1", &val); + assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(val.type, ISC_CFGMGR_UINT32); + assert_int_equal(val.uint32, 1234); + + /* + * builtin mode on is automatically turned off when closing a clause + * (commit automatically clause opened clauses) + */ + result = isc_cfgmgr_commit(); + assert_int_equal(result, ISC_R_SUCCESS); + + result = isc_cfgmgr_transaction(); + result = isc_cfgmgr_open("foo"); + expect_assert_failure(isc_cfgmgr_builtinoff()); + isc_cfgmgr_builtinon(); + isc_cfgmgr_close(); + + result = isc_cfgmgr_open("foo"); + expect_assert_failure(isc_cfgmgr_builtinoff()); + isc_cfgmgr_builtinon(); + + isc_cfgmgr_rollback(); + isc_cfgmgr_deinit(); +} + ISC_TEST_LIST_START ISC_TEST_ENTRY(isc_cfgmgr_assertions) ISC_TEST_ENTRY(isc_cfgmgr_rw) @@ -1546,5 +1634,6 @@ ISC_TEST_ENTRY(isc_cfgmgr_parseid) ISC_TEST_ENTRY(isc_cfgmgr_validation_error) ISC_TEST_ENTRY(isc_cfgmgr_getparent) ISC_TEST_ENTRY(isc_cfgmgr_opentests) +ISC_TEST_ENTRY(isc_cfgmgr_builtintests) ISC_TEST_LIST_END ISC_TEST_MAIN