This is an automated email from the ASF dual-hosted git repository. gmurthy pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git
The following commit(s) were added to refs/heads/master by this push: new 936d679 DISPATCH-1637: Added a hashtable to store names of link routes, auto links and address configs. This closes #732. 936d679 is described below commit 936d679f754639895dccccf85b665651ed4d73f3 Author: Ganesh Murthy <gmur...@apache.org> AuthorDate: Tue May 5 08:56:06 2020 -0400 DISPATCH-1637: Added a hashtable to store names of link routes, auto links and address configs. This closes #732. --- src/hash.c | 9 +++ src/router_core/agent_config_address.c | 24 +++++-- src/router_core/agent_config_auto_link.c | 15 ++-- src/router_core/agent_config_link_route.c | 15 ++-- src/router_core/route_control.c | 36 ++++++++++ src/router_core/route_tables.c | 5 +- src/router_core/router_core.c | 11 +++ src/router_core/router_core_private.h | 6 ++ tests/system_tests_autolinks.py | 114 ++++++++++++++++++++++++++++++ 9 files changed, 217 insertions(+), 18 deletions(-) diff --git a/src/hash.c b/src/hash.c index 03acfdf..89c2016 100644 --- a/src/hash.c +++ b/src/hash.c @@ -267,6 +267,11 @@ void qd_hash_retrieve_prefix_const(qd_hash_t *h, qd_iterator_t *iter, const void qd_error_t qd_hash_retrieve(qd_hash_t *h, qd_iterator_t *key, void **val) { + if (!key) { + *val = 0; + return QD_ERROR_NONE; + } + qd_hash_item_t *item = qd_hash_internal_retrieve(h, key); if (item) *val = item->v.val; @@ -322,10 +327,14 @@ const unsigned char *qd_hash_key_by_handle(const qd_hash_handle_t *handle) qd_error_t qd_hash_remove_by_handle(qd_hash_t *h, qd_hash_handle_t *handle) { + if (!handle) + return QD_ERROR_NONE; + unsigned char *key = 0; qd_error_t error = qd_hash_remove_by_handle2(h, handle, &key); if (key) free(key); + return error; } diff --git a/src/router_core/agent_config_address.c b/src/router_core/agent_config_address.c index b25779a..c04267e 100644 --- a/src/router_core/agent_config_address.c +++ b/src/router_core/agent_config_address.c @@ -49,6 +49,7 @@ const char *qdr_config_address_columns[] = 0}; const char *CONFIG_ADDRESS_TYPE = "org.apache.qpid.dispatch.router.config.address"; +const char CONFIG_ADDRESS_PREFIX = 'C'; static void qdr_config_address_insert_column_CT(qdr_address_config_t *addr, int col, qd_composed_field_t *body, bool as_map) { @@ -340,13 +341,15 @@ void qdra_config_address_create_CT(qdr_core_t *core, while (true) { // - // Ensure there isn't a duplicate name and that the body is a map + // Ensure there isn't a duplicate name // - qdr_address_config_t *addr = DEQ_HEAD(core->addr_config); - while (addr) { - if (name && addr->name && qd_iterator_equal(name, (const unsigned char*) addr->name)) - break; - addr = DEQ_NEXT(addr); + qdr_address_config_t *addr = 0; + if (name) { + qd_iterator_view_t iter_view = qd_iterator_get_view(name); + qd_iterator_annotate_prefix(name, CONFIG_ADDRESS_PREFIX); + qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH); + qd_hash_retrieve(core->addr_lr_al_hash, name, (void**) &addr); + qd_iterator_reset_view(name, iter_view); } if (!!addr) { @@ -356,6 +359,7 @@ void qdra_config_address_create_CT(qdr_core_t *core, break; } + // Ensure that the body is a map if (!qd_parse_is_map(in_body)) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Body of request must be a map"; @@ -476,8 +480,14 @@ void qdra_config_address_create_CT(qdr_core_t *core, qd_iterator_reset_view(iter, ITER_VIEW_ALL); qd_parse_tree_add_pattern(core->addr_parse_tree, iter, addr); - DEQ_INSERT_TAIL(core->addr_config, addr); + DEQ_INSERT_TAIL(core->addr_config, addr); + if (name) { + qd_iterator_view_t iter_view = qd_iterator_get_view(name); + qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH); + qd_hash_insert(core->addr_lr_al_hash, name, addr, &addr->hash_handle); + qd_iterator_reset_view(name, iter_view); + } // // Compose the result map for the response. // diff --git a/src/router_core/agent_config_auto_link.c b/src/router_core/agent_config_auto_link.c index db2f941..f576221 100644 --- a/src/router_core/agent_config_auto_link.c +++ b/src/router_core/agent_config_auto_link.c @@ -60,6 +60,7 @@ const char *qdr_config_auto_link_columns[] = 0}; const char *CONFIG_AUTOLINK_TYPE = "org.apache.qpid.dispatch.router.config.autoLink"; +const char CONFIG_AUTO_LINK_PREFIX = 'A'; static void qdr_config_auto_link_insert_column_CT(qdr_auto_link_t *al, int col, qd_composed_field_t *body, bool as_map) { @@ -374,13 +375,17 @@ void qdra_config_auto_link_create_CT(qdr_core_t *core, // // Ensure there isn't a duplicate name and that the body is a map // - qdr_auto_link_t *al = DEQ_HEAD(core->auto_links); - while (al) { - if (name && al->name && qd_iterator_equal(name, (const unsigned char*) al->name)) - break; - al = DEQ_NEXT(al); + qdr_auto_link_t *al = 0; + + if (name) { + qd_iterator_view_t iter_view = qd_iterator_get_view(name); + qd_iterator_annotate_prefix(name, CONFIG_AUTO_LINK_PREFIX); + qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH); + qd_hash_retrieve(core->addr_lr_al_hash, name, (void**) &al); + qd_iterator_reset_view(name, iter_view); } + if (!!al) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Name conflicts with an existing entity"; diff --git a/src/router_core/agent_config_link_route.c b/src/router_core/agent_config_link_route.c index c04daa2..a427194 100644 --- a/src/router_core/agent_config_link_route.c +++ b/src/router_core/agent_config_link_route.c @@ -55,6 +55,7 @@ const char *qdr_config_link_route_columns[] = 0}; const char *CONFIG_LINKROUTE_TYPE = "org.apache.qpid.dispatch.router.config.linkRoute"; +const char CONFIG_LINK_ROUTE_PREFIX = 'L'; const qd_amqp_error_t QD_AMQP_NAME_IDENTITY_MISSING = { 400, "No name or identity provided" }; @@ -396,13 +397,17 @@ void qdra_config_link_route_create_CT(qdr_core_t *core, // // Ensure there isn't a duplicate name and that the body is a map // - qdr_link_route_t *lr = DEQ_HEAD(core->link_routes); - while (lr) { - if (name && lr->name && qd_iterator_equal(name, (const unsigned char*) lr->name)) - break; - lr = DEQ_NEXT(lr); + qdr_link_route_t *lr = 0; + + if (name) { + qd_iterator_view_t iter_view = qd_iterator_get_view(name); + qd_iterator_annotate_prefix(name, CONFIG_LINK_ROUTE_PREFIX); + qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH); + qd_hash_retrieve(core->addr_lr_al_hash, name, (void**) &lr); + qd_iterator_reset_view(name, iter_view); } + if (!!lr) { query->status = QD_AMQP_BAD_REQUEST; query->status.description = "Name conflicts with an existing entity"; diff --git a/src/router_core/route_control.c b/src/router_core/route_control.c index 68de4a0..f6915b1 100644 --- a/src/router_core/route_control.c +++ b/src/router_core/route_control.c @@ -378,9 +378,22 @@ qdr_link_route_t *qdr_route_add_link_route_CT(qdr_core_t *core, } // + // If a name was provided, use that as the key to insert the this link route name into the hashtable so + // we can quickly find it later. + // + if (name) { + qd_iterator_view_t iter_view = qd_iterator_get_view(name); + qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH); + qd_hash_insert(core->addr_lr_al_hash, name, lr, &lr->hash_handle); + qd_iterator_reset_view(name, iter_view); + } + + // // Add the link route to the core list // DEQ_INSERT_TAIL(core->link_routes, lr); + + qd_log(core->log, QD_LOG_TRACE, "Link route %spattern added: pattern=%s name=%s", is_prefix ? "prefix " : "", lr->pattern, lr->name); @@ -441,6 +454,12 @@ void qdr_route_del_link_route_CT(qdr_core_t *core, qdr_link_route_t *lr) } } + if (lr->hash_handle) { + qd_hash_remove_by_handle(core->addr_lr_al_hash, lr->hash_handle); + qd_hash_handle_free(lr->hash_handle); + lr->hash_handle = 0; + } + // // Remove the link route from the core list. // @@ -520,6 +539,17 @@ qdr_auto_link_t *qdr_route_add_auto_link_CT(qdr_core_t *core, } // + // If a name was provided, use that as the key to insert the this auto link name into the hashtable so + // we can quickly find it later. + // + if (name) { + qd_iterator_view_t iter_view = qd_iterator_get_view(name); + qd_iterator_reset_view(name, ITER_VIEW_ADDRESS_HASH); + qd_hash_insert(core->addr_lr_al_hash, name, al, &al->hash_handle); + qd_iterator_reset_view(name, iter_view); + } + + // // Add the auto_link to the core list // DEQ_INSERT_TAIL(core->auto_links, al); @@ -543,6 +573,12 @@ void qdr_route_del_auto_link_CT(qdr_core_t *core, qdr_auto_link_t *al) } } + if (al->hash_handle) { + qd_hash_remove_by_handle(core->addr_lr_al_hash, al->hash_handle); + qd_hash_handle_free(al->hash_handle); + al->hash_handle = 0; + } + // // Remove the auto link from the core list. // diff --git a/src/router_core/route_tables.c b/src/router_core/route_tables.c index d6fdb15..7c55964 100644 --- a/src/router_core/route_tables.c +++ b/src/router_core/route_tables.c @@ -219,7 +219,10 @@ void qdr_route_table_setup_CT(qdr_core_t *core) { DEQ_INIT(core->addrs); DEQ_INIT(core->routers); - core->addr_hash = qd_hash(12, 32, 0); + core->addr_hash = qd_hash(12, 32, 0); + core->addr_lr_al_hash = qd_hash(12, 32, 0); + + core->conn_id_hash = qd_hash(6, 4, 0); core->cost_epoch = 1; core->addr_parse_tree = qd_parse_tree_new(QD_PARSE_TREE_ADDRESS); diff --git a/src/router_core/router_core.c b/src/router_core/router_core.c index 1a01ed3..ec8e0ad 100644 --- a/src/router_core/router_core.c +++ b/src/router_core/router_core.c @@ -149,7 +149,10 @@ void qdr_core_free(qdr_core_t *core) while ( (addr_config = DEQ_HEAD(core->addr_config))) { qdr_core_remove_address_config(core, addr_config); } + qd_hash_free(core->addr_hash); + qd_hash_free(core->addr_lr_al_hash); + qd_parse_tree_free(core->addr_parse_tree); qd_parse_tree_free(core->link_route_tree[QD_INCOMING]); qd_parse_tree_free(core->link_route_tree[QD_OUTGOING]); @@ -456,6 +459,7 @@ void qdr_core_delete_link_route(qdr_core_t *core, qdr_link_route_t *lr) free(lr->del_prefix); free(lr->name); free(lr->pattern); + qd_hash_handle_free(lr->hash_handle); free_qdr_link_route_t(lr); } @@ -472,6 +476,7 @@ void qdr_core_delete_auto_link(qdr_core_t *core, qdr_auto_link_t *al) free(al->name); free(al->external_addr); + qd_hash_handle_free(al->hash_handle); qdr_core_timer_free_CT(core, al->retry_timer); free_qdr_auto_link_t(al); } @@ -480,6 +485,7 @@ static void free_address_config(qdr_address_config_t *addr) { free(addr->name); free(addr->pattern); + qd_hash_handle_free(addr->hash_handle); free_qdr_address_config_t(addr); } @@ -678,6 +684,11 @@ void qdr_core_remove_address_config(qdr_core_t *core, qdr_address_config_t *addr qd_iterator_t *pattern = qd_iterator_string(addr->pattern, ITER_VIEW_ALL); // Remove the address from the list and the parse tree + if (addr->hash_handle) { + qd_hash_remove_by_handle(core->addr_lr_al_hash, addr->hash_handle); + qd_hash_handle_free(addr->hash_handle); + addr->hash_handle = 0; + } DEQ_REMOVE(core->addr_config, addr); qd_parse_tree_remove_pattern(core->addr_parse_tree, pattern); addr->ref_count--; diff --git a/src/router_core/router_core_private.h b/src/router_core/router_core_private.h index 6b209d0..5f358d0 100644 --- a/src/router_core/router_core_private.h +++ b/src/router_core/router_core_private.h @@ -607,6 +607,7 @@ struct qdr_address_config_t { int in_phase; int out_phase; int priority; + qd_hash_handle_t *hash_handle; }; DEQ_DECLARE(qdr_address_config_t, qdr_address_config_list_t); @@ -713,6 +714,7 @@ struct qdr_link_route_t { char *add_prefix; char *del_prefix; qdr_connection_t *parent_conn; + qd_hash_handle_t *hash_handle; }; void qdr_core_delete_link_route(qdr_core_t *core, qdr_link_route_t *lr); @@ -760,6 +762,7 @@ struct qdr_auto_link_t { qdr_core_timer_t *retry_timer; // If the auto link attach fails or gets disconnected, this timer retries the attach. char *last_error; bool fallback; // True iff this auto-link attaches to a fallback destination for an address. + qd_hash_handle_t *hash_handle; }; DEQ_DECLARE(qdr_auto_link_t, qdr_auto_link_list_t); @@ -847,6 +850,9 @@ struct qdr_core_t { int worker_thread_count; qdr_address_config_list_t addr_config; + // Hash to hold names of address configs, link routes and auto links. + // address config prefix = 'C', auto link prefix = 'A', link route prefix = 'L' + qd_hash_t *addr_lr_al_hash; qdr_auto_link_list_t auto_links; qdr_link_route_list_t link_routes; qd_hash_t *conn_id_hash; diff --git a/tests/system_tests_autolinks.py b/tests/system_tests_autolinks.py index 434c578..19d1c0e 100644 --- a/tests/system_tests_autolinks.py +++ b/tests/system_tests_autolinks.py @@ -32,6 +32,7 @@ from proton.handlers import MessagingHandler from proton.reactor import Container from subprocess import PIPE, STDOUT from qpid_dispatch.management.client import Node +from system_test import QdManager CONNECTION_PROPERTIES = {u'connection': u'properties', u'int_property': 6451} @@ -92,6 +93,119 @@ class AutoLinkDetachAfterAttachTest(MessagingHandler): def run(self): Container(self).run() +class NameCollisionTest(TestCase): + @classmethod + def setUpClass(cls): + super(NameCollisionTest, cls).setUpClass() + name = "test-router" + + config = Qdrouterd.Config([ + + ('router', {'mode': 'standalone', 'id': 'A'}), + ('listener', {'host': '127.0.0.1', 'role': 'normal', + 'port': cls.tester.get_port()}), + ('autoLink', {'name': 'autoLink', + 'address': 'autoLink1', + 'connection': 'brokerConnection', + 'direction': 'in'}), + ('linkRoute', {'name': 'linkRoute', + 'prefix': 'linkRoute', + 'connection': 'brokerConnection', + 'direction': 'in'}), + ('address', {'name': 'address', + 'prefix': 'address.1', + 'waypoint': 'yes'}), + ]) + + cls.router = cls.tester.qdrouterd(name, config) + cls.router.wait_ready() + cls.address = cls.router.addresses[0] + + def test_name_collision(self): + args = {"name": "autoLink", "address": "autoLink1", "connection": "broker", "dir": "in"} + # Add autoLink with the same name as the one already present. + al_long_type = 'org.apache.qpid.dispatch.router.config.autoLink' + addr_long_type = 'org.apache.qpid.dispatch.router.config.address' + lr_long_type = 'org.apache.qpid.dispatch.router.config.linkRoute' + mgmt = QdManager(self, address=self.router.addresses[0]) + test_pass = False + try: + mgmt.create(al_long_type, args) + except Exception as e: + if "BadRequestStatus: Name conflicts with an existing entity" in str(e): + test_pass = True + self.assertTrue(test_pass) + + # Try to add duplicate linkRoute and make sure it fails + args = {"name": "linkRoute", "prefix": "linkRoute", + "connection": "broker", "dir": "in"} + + mgmt = QdManager(self, address=self.router.addresses[0]) + test_pass = False + try: + mgmt.create(lr_long_type, args) + except Exception as e: + if "BadRequestStatus: Name conflicts with an existing entity" in str(e): + test_pass = True + self.assertTrue(test_pass) + + args = {"name": "address", "prefix": "address.1", + "waypoint": "yes"} + mgmt = QdManager(self, address=self.router.addresses[0]) + test_pass = False + try: + mgmt.create(addr_long_type, args) + except Exception as e: + if "BadRequestStatus: Name conflicts with an existing entity" in str(e): + test_pass = True + self.assertTrue(test_pass) + + # The linkRoutes, autoLinks and addrConfigs share the same hashtable + # but with a prefix. + # The following tests make sure that same names used on + # different entities are allowed. + + # insert a linkRoute with the name of an existing autoLink and make + # sure that is ok + args = {"name": "autoLink", "prefix": "linkRoute", + "connection": "broker", "dir": "in"} + mgmt = QdManager(self, address=self.router.addresses[0]) + mgmt.create(lr_long_type, args) + + # insert a linkRoute with the name of an existing addr config and make + # sure that is ok + args = {"name": "address", "prefix": "linkRoute", + "connection": "broker", "dir": "in"} + mgmt = QdManager(self, address=self.router.addresses[0]) + mgmt.create(lr_long_type, args) + + # insert an autoLink with the name of an existing linkRoute and make + # sure that is ok + args = {"name": "linkRoute", "address": "autoLink1", "connection": "broker", "dir": "in"} + mgmt = QdManager(self, address=self.router.addresses[0]) + mgmt.create(al_long_type, args) + + # insert an autoLink with the name of an existing address and make + # sure that is ok + args = {"name": "address", "address": "autoLink1", "connection": "broker", "dir": "in"} + al_long_type = 'org.apache.qpid.dispatch.router.config.autoLink' + mgmt = QdManager(self, address=self.router.addresses[0]) + mgmt.create(al_long_type, args) + + # insert an address with the name of an existing autoLink and make + # sure that is ok + args = {"name": "autoLink", "prefix": "address.2", + "waypoint": "yes"} + mgmt = QdManager(self, address=self.router.addresses[0]) + mgmt.create(addr_long_type, args) + + # insert an autoLink with the name of an existing linkRoute and make + # sure that is ok + args = {"name": "linkRoute", "prefix": "address.3", + "waypoint": "yes"} + mgmt = QdManager(self, address=self.router.addresses[0]) + mgmt.create(addr_long_type, args) + class DetachAfterAttachTest(TestCase): @classmethod --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org For additional commands, e-mail: commits-h...@qpid.apache.org