From b2c96305a8202e65387ce0363a362b8fdf542ee6 Mon Sep 17 00:00:00 2001 From: Rajaram Mallya Date: Thu, 10 Nov 2011 18:27:23 +0530 Subject: [PATCH] Refactored the natting queries --- melange/db/sqlalchemy/api.py | 30 +++++++++---------------- melange/ipam/models.py | 28 +++++++++++++++-------- melange/ipam/service.py | 6 +++-- melange/tests/unit/test_ipam_models.py | 27 +++++++++------------- melange/tests/unit/test_ipam_service.py | 30 +++++++++++-------------- 5 files changed, 58 insertions(+), 63 deletions(-) diff --git a/melange/db/sqlalchemy/api.py b/melange/db/sqlalchemy/api.py index 631e4289..599e1328 100644 --- a/melange/db/sqlalchemy/api.py +++ b/melange/db/sqlalchemy/api.py @@ -81,26 +81,18 @@ def update_all(query_func, model, conditions, values): query_func(model, **conditions).update(values) -def find_inside_globals_for(local_address_id, **kwargs): - marker_column = mappers.IpNat.inside_global_address_id - limit = kwargs.pop('limit', 200) - marker = kwargs.pop('marker', None) - - kwargs["inside_local_address_id"] = local_address_id - query = _limits(find_all_by, mappers.IpNat, kwargs, limit, marker, - marker_column) - return [nat.inside_global_address for nat in query] +def find_inside_globals(ip_model, local_address_id, **kwargs): + ip_nat = mappers.IpNat + return _base_query(ip_model).\ + join(ip_nat, ip_nat.inside_global_address_id == ip_model.id).\ + filter(ip_nat.inside_local_address_id == local_address_id) -def find_inside_locals_for(global_address_id, **kwargs): - marker_column = mappers.IpNat.inside_local_address_id - limit = kwargs.pop('limit', 200) - marker = kwargs.pop('marker', None) - - kwargs["inside_global_address_id"] = global_address_id - query = _limits(find_all_by, mappers.IpNat, kwargs, limit, marker, - marker_column) - return [nat.inside_local_address for nat in query] +def find_inside_locals(ip_model, global_address_id, **kwargs): + ip_nat = mappers.IpNat + return _base_query(ip_model).\ + join(ip_nat, ip_nat.inside_local_address_id == ip_model.id).\ + filter(ip_nat.inside_global_address_id == global_address_id) def save_nat_relationships(nat_relationships): @@ -172,7 +164,7 @@ def find_all_top_level_blocks_in_network(network_id): filter(parent_block.id == None) -def find_all_ips_in_network(network_id, **conditions): +def find_all_ips_in_network(model, network_id=None, **conditions): return _query_by(ipam.models.IpAddress, **conditions).\ join(ipam.models.IpBlock).\ filter(ipam.models.IpBlock.network_id == network_id) diff --git a/melange/ipam/models.py b/melange/ipam/models.py index d4597f6f..c51a1fc5 100644 --- a/melange/ipam/models.py +++ b/melange/ipam/models.py @@ -62,12 +62,13 @@ class Query(object): db_api.delete_all(self._query_func, self._model, **self._conditions) def limit(self, limit=200, marker=None, marker_column=None): - return db_api.list(db_api.find_all_by_limit(self._query_func, - self._model, - self._conditions, - limit=limit, - marker=marker, - marker_column=marker_column)) + limit_query = db_api.find_all_by_limit(self._query_func, + self._model, + self._conditions, + limit=limit, + marker=marker, + marker_column=marker_column) + return db_api.list(limit_query) def paginated_collection(self, limit=200, marker=None, marker_column=None): collection = self.limit(int(limit) + 1, marker, marker_column) @@ -570,7 +571,10 @@ class IpAddress(ModelBase): @classmethod def find_all_by_network(cls, network_id, **conditions): - return db_api.find_all_ips_in_network(network_id, **conditions) + return Query(cls, + query_func=db_api.find_all_ips_in_network, + network_id=network_id, + **conditions) @classmethod def find_all_allocated_ips(cls, **conditions): @@ -604,7 +608,10 @@ class IpAddress(ModelBase): self.update(marked_for_deallocation=False, deallocated_at=None) def inside_globals(self, **kwargs): - return db_api.find_inside_globals_for(self.id, **kwargs) + return Query(IpAddress, + query_func=db_api.find_inside_globals, + local_address_id=self.id, + **kwargs) def add_inside_globals(self, ip_addresses): db_api.save_nat_relationships([ @@ -615,7 +622,10 @@ class IpAddress(ModelBase): for global_address in ip_addresses]) def inside_locals(self, **kwargs): - return db_api.find_inside_locals_for(self.id, **kwargs) + return Query(IpAddress, + query_func=db_api.find_inside_locals, + global_address_id=self.id, + **kwargs) def remove_inside_globals(self, inside_global_address=None): return db_api.remove_inside_globals(self.id, inside_global_address) diff --git a/melange/ipam/service.py b/melange/ipam/service.py index 1e19f6b2..8346f068 100644 --- a/melange/ipam/service.py +++ b/melange/ipam/service.py @@ -224,7 +224,8 @@ class InsideGlobalsController(BaseController): def index(self, request, ip_block_id, tenant_id, address): ip_block = models.IpBlock.find_by(id=ip_block_id, tenant_id=tenant_id) ip = ip_block.find_ip(address) - global_ips = ip.inside_globals(**self._extract_limits(request.params)) + global_ips, marker = ip.inside_globals().paginated_collection( + **self._extract_limits(request.params)) return dict(ip_addresses=[ip.data() for ip in global_ips]) def delete(self, request, ip_block_id, address, tenant_id, @@ -252,7 +253,8 @@ class InsideLocalsController(BaseController): def index(self, request, ip_block_id, address, tenant_id): ip_block = models.IpBlock.find_by(id=ip_block_id, tenant_id=tenant_id) ip = ip_block.find_ip(address) - local_ips = ip.inside_locals(**self._extract_limits(request.params)) + local_ips, marker = ip.inside_locals().paginated_collection( + **self._extract_limits(request.params)) return dict(ip_addresses=[ip.data() for ip in local_ips]) def delete(self, request, ip_block_id, address, tenant_id, diff --git a/melange/tests/unit/test_ipam_models.py b/melange/tests/unit/test_ipam_models.py index 061d0aee..83ffda7c 100644 --- a/melange/tests/unit/test_ipam_models.py +++ b/melange/tests/unit/test_ipam_models.py @@ -1098,35 +1098,30 @@ class TestIpAddress(tests.BaseTest): def test_limited_show_inside_locals(self): global_block = factory_models.PrivateIpBlockFactory(cidr="192.0.0.1/8") local_block = factory_models.PrivateIpBlockFactory(cidr="10.0.0.1/8") - global_ip = _allocate_ip(global_block) local_ips = models.sort([_allocate_ip(local_block) for i in range(5)]) global_ip.add_inside_locals(local_ips) - limited_local_addresses = [ip.address for ip in - global_ip.inside_locals(limit=2, - marker=local_ips[1].id)] + inside_locals_query = global_ip.inside_locals() + expected_locals, next_mrk = inside_locals_query.paginated_collection( + limit=2, marker=local_ips[1].id) - self.assertEqual(len(limited_local_addresses), 2) - self.assertTrue(limited_local_addresses, [local_ips[2].address, - local_ips[3].address]) + self.assertModelsEqual(expected_locals, [local_ips[2], local_ips[3]]) def test_limited_show_inside_globals(self): global_block = factory_models.PrivateIpBlockFactory(cidr="192.0.0.1/8") local_block = factory_models.PrivateIpBlockFactory(cidr="10.0.0.1/8") - global_ips = models.sort([_allocate_ip(global_block) for i in range(5)]) local_ip = _allocate_ip(local_block) local_ip.add_inside_globals(global_ips) - limited_global_addresses = [ip.address for ip in - local_ip.inside_globals(limit=2, - marker=global_ips[1].id)] + inside_globals_query = local_ip.inside_globals() + inside_globals, next_mrk = inside_globals_query.paginated_collection( + limit=2, marker=global_ips[1].id) - self.assertEqual(len(limited_global_addresses), 2) - self.assertTrue(limited_global_addresses, [global_ips[2].address, - global_ips[3].address]) + self.assertModelsEqual(inside_globals, [global_ips[2], global_ips[3]]) + self.assertEqual(next_mrk, global_ips[3].id) def test_remove_inside_globals(self): global_block = factory_models.PrivateIpBlockFactory(cidr="192.0.0.1/8") @@ -1138,7 +1133,7 @@ class TestIpAddress(tests.BaseTest): local_ip.remove_inside_globals() - self.assertEqual(local_ip.inside_globals(), []) + self.assertEqual(local_ip.inside_globals().all(), []) def test_remove_inside_globals_for_specific_address(self): global_block = factory_models.PrivateIpBlockFactory(cidr="192.0.0.1/8") @@ -1177,7 +1172,7 @@ class TestIpAddress(tests.BaseTest): global_ip.remove_inside_locals() - self.assertEqual(global_ip.inside_locals(), []) + self.assertEqual(global_ip.inside_locals().all(), []) def test_data(self): ip_block = factory_models.PrivateIpBlockFactory(cidr="10.0.0.1/8") diff --git a/melange/tests/unit/test_ipam_service.py b/melange/tests/unit/test_ipam_service.py index 4f5f727e..fae3673f 100644 --- a/melange/tests/unit/test_ipam_service.py +++ b/melange/tests/unit/test_ipam_service.py @@ -1040,10 +1040,10 @@ class TestInsideGlobalsController(BaseTestController): }) self.assertEqual(response.status, "200 OK") - - self.assertEqual(len(local_ip.inside_globals()), 1) - self.assertEqual(global_ip.id, local_ip.inside_globals()[0].id) - self.assertEqual(local_ip.id, global_ip.inside_locals()[0].id) + expected_globals = local_ip.inside_globals().all() + expected_locals = global_ip.inside_locals().all() + self.assertEqual([global_ip], expected_globals) + self.assertEqual([local_ip], expected_locals) def test_create_throws_error_for_ips_of_other_tenants_blocks(self): local_block = factory_models.PublicIpBlockFactory(cidr="77.1.1.0/28") @@ -1112,7 +1112,7 @@ class TestInsideGlobalsController(BaseTestController): local_ip.address)) self.assertEqual(response.status, "200 OK") - self.assertEqual(local_ip.inside_globals(), []) + self.assertEqual(local_ip.inside_globals().all(), []) def test_delete_for_specific_address(self): local_block = factory_models.PrivateIpBlockFactory(cidr="10.1.1.1/8") @@ -1126,9 +1126,8 @@ class TestInsideGlobalsController(BaseTestController): local_ip.address), global_ips[1].address)) - globals_left = [ip.address for ip in local_ip.inside_globals()] - self.assertEqual(globals_left, [global_ips[0].address, - global_ips[2].address]) + globals_left = local_ip.inside_globals().all() + self.assertModelsEqual(globals_left, [global_ips[0], global_ips[2]]) def test_delete_for_nonexistent_block(self): non_existant_block_id = 12122 @@ -1243,14 +1242,11 @@ class TestInsideLocalsController(BaseTestController): request_data) self.assertEqual(response.status, "200 OK") - inside_locals = global_ip.inside_locals() + inside_locals = global_ip.inside_locals().all() - self.assertEqual(len(inside_locals), 2) self.assertModelsEqual(inside_locals, [local_ip1, local_ip2]) - self.assertEqual(inside_locals[0].inside_globals()[0].address, - global_ip.address) - self.assertEqual(inside_locals[1].inside_globals()[0].address, - global_ip.address) + [self.assertEqual(local.inside_globals().all(), [global_ip]) + for local in inside_locals] def test_create_throws_error_for_ips_of_other_tenants_blocks(self): global_block = factory_models.PublicIpBlockFactory(cidr="77.1.1.0/28") @@ -1304,8 +1300,8 @@ class TestInsideLocalsController(BaseTestController): local_ips[1].address)) locals_left = [ip.address for ip in global_ip.inside_locals()] - self.assertEqual(locals_left, - [local_ips[0].address, local_ips[2].address]) + self.assertItemsEqual(locals_left, + [local_ips[0].address, local_ips[2].address]) def test_delete(self): local_block = factory_models.PrivateIpBlockFactory(cidr="10.1.1.1/24") @@ -1319,7 +1315,7 @@ class TestInsideLocalsController(BaseTestController): global_ip.address)) self.assertEqual(response.status, "200 OK") - self.assertEqual(global_ip.inside_locals(), []) + self.assertEqual(global_ip.inside_locals().all(), []) def test_delete_for_nonexistent_block(self): non_existant_block_id = 12122