From 2e40c234f84049681a6e8303bc377f59ac9e034f Mon Sep 17 00:00:00 2001 From: Feodor Tersin Date: Wed, 12 Aug 2015 19:51:31 +0300 Subject: [PATCH] Remove project_id from db.add_item interface Since there is no part of code which adds an db item with other project, this backdoor is not needed. This patch removes this argument and its proxying by util functions. This reverts commit b51eb4f35028d4861d4fc7df5eb9dc39b29c26e7. Change-Id: I70b5d2e7246152ac2b1014f51f89f44862acab44 --- ec2api/api/ec2utils.py | 14 ++++---------- ec2api/db/api.py | 6 +++--- ec2api/db/sqlalchemy/api.py | 10 ++++------ ec2api/tests/unit/test_address.py | 3 +-- ec2api/tests/unit/test_customer_gateway.py | 3 +-- ec2api/tests/unit/test_ec2utils.py | 4 ++-- ec2api/tests/unit/test_image.py | 6 ++---- ec2api/tests/unit/test_instance.py | 15 ++++++--------- ec2api/tests/unit/test_internet_gateway.py | 2 +- ec2api/tests/unit/test_network_interface.py | 6 ++---- ec2api/tests/unit/test_route_table.py | 3 +-- ec2api/tests/unit/test_security_group.py | 9 +++------ ec2api/tests/unit/test_snapshot.py | 3 +-- ec2api/tests/unit/test_subnet.py | 3 +-- ec2api/tests/unit/test_volume.py | 6 ++---- ec2api/tests/unit/test_vpc.py | 6 ++---- ec2api/tests/unit/test_vpn_connection.py | 3 +-- ec2api/tests/unit/test_vpn_gateway.py | 3 +-- ec2api/tests/unit/tools.py | 4 +--- 19 files changed, 39 insertions(+), 70 deletions(-) diff --git a/ec2api/api/ec2utils.py b/ec2api/api/ec2utils.py index 1f00c020..71b85604 100644 --- a/ec2api/api/ec2utils.py +++ b/ec2api/api/ec2utils.py @@ -302,21 +302,16 @@ def register_auto_create_db_item_extension(kind, extension): _auto_create_db_item_extensions[kind] = extension -# TODO(Alex): The project_id passing mechanism can be potentially -# reconsidered in future. -def auto_create_db_item(context, kind, os_id, project_id=None, - **extension_kwargs): +def auto_create_db_item(context, kind, os_id, **extension_kwargs): item = {'os_id': os_id} extension = _auto_create_db_item_extensions.get(kind) if extension: extension(context, item, **extension_kwargs) - return db_api.add_item(context, kind, item, project_id=project_id) + return db_api.add_item(context, kind, item) -# TODO(Alex): The project_id passing mechanism can be potentially -# reconsidered in future. def get_db_item_by_os_id(context, kind, os_id, items_by_os_id=None, - project_id=None, **extension_kwargs): + **extension_kwargs): """Get DB item by OS id (create if it doesn't exist). Args: @@ -346,8 +341,7 @@ def get_db_item_by_os_id(context, kind, os_id, items_by_os_id=None, item = next((i for i in db_api.get_items(context, kind) if i['os_id'] == os_id), None) if not item: - item = auto_create_db_item(context, kind, os_id, project_id=project_id, - **extension_kwargs) + item = auto_create_db_item(context, kind, os_id, **extension_kwargs) if items_by_os_id is not None: items_by_os_id[os_id] = item return item diff --git a/ec2api/db/api.py b/ec2api/db/api.py index 255dcc16..3dd7f369 100644 --- a/ec2api/db/api.py +++ b/ec2api/db/api.py @@ -79,12 +79,12 @@ IMPL = EC2DBAPI() LOG = logging.getLogger(__name__) -def add_item(context, kind, data, project_id=None): - return IMPL.add_item(context, kind, data, project_id=project_id) +def add_item(context, kind, data): + return IMPL.add_item(context, kind, data) def add_item_id(context, kind, os_id, project_id=None): - return IMPL.add_item_id(context, kind, os_id, project_id=project_id) + return IMPL.add_item_id(context, kind, os_id, project_id) def update_item(context, item): diff --git a/ec2api/db/sqlalchemy/api.py b/ec2api/db/sqlalchemy/api.py index 2cfe79d8..1a973f22 100644 --- a/ec2api/db/sqlalchemy/api.py +++ b/ec2api/db/sqlalchemy/api.py @@ -90,12 +90,10 @@ def _new_id(kind): @require_context -def add_item(context, kind, data, project_id=None): - if not project_id: - project_id = context.project_id +def add_item(context, kind, data): item_ref = models.Item() item_ref.update({ - "project_id": project_id, + "project_id": context.project_id, "id": _new_id(kind), }) item_ref.update(_pack_item_data(data)) @@ -107,14 +105,14 @@ def add_item(context, kind, data, project_id=None): raise item_ref = (model_query(context, models.Item). filter_by(os_id=data["os_id"]). - filter(or_(models.Item.project_id == project_id, + filter(or_(models.Item.project_id == context.project_id, models.Item.project_id.is_(None))). filter(models.Item.id.like('%s-%%' % kind)). one()) item_data = _unpack_item_data(item_ref) item_data.update(data) item_ref.update(_pack_item_data(item_data)) - item_ref.project_id = project_id + item_ref.project_id = context.project_id item_ref.save() return _unpack_item_data(item_ref) diff --git a/ec2api/tests/unit/test_address.py b/ec2api/tests/unit/test_address.py index 19e06250..cf3872db 100644 --- a/ec2api/tests/unit/test_address.py +++ b/ec2api/tests/unit/test_address.py @@ -64,8 +64,7 @@ class AddressTestCase(base.ApiTestCase): self.db_api.add_item.assert_called_once_with( mock.ANY, 'eipalloc', tools.purge_dict(fakes.DB_ADDRESS_1, - ('id', 'vpc_id')), - project_id=None) + ('id', 'vpc_id'))) self.neutron.create_floatingip.assert_called_once_with( {'floatingip': { 'floating_network_id': diff --git a/ec2api/tests/unit/test_customer_gateway.py b/ec2api/tests/unit/test_customer_gateway.py index d688d4e0..e4dfd8ae 100644 --- a/ec2api/tests/unit/test_customer_gateway.py +++ b/ec2api/tests/unit/test_customer_gateway.py @@ -34,8 +34,7 @@ class CustomerGatewayTestCase(base.ApiTestCase): resp) self.db_api.add_item.assert_called_once_with( mock.ANY, 'cgw', - {'ip_address': fakes.IP_CUSTOMER_GATEWAY_ADDRESS_2}, - project_id=None) + {'ip_address': fakes.IP_CUSTOMER_GATEWAY_ADDRESS_2}) resp = self.execute('CreateCustomerGateway', {'IpAddress': fakes.IP_CUSTOMER_GATEWAY_ADDRESS_2, diff --git a/ec2api/tests/unit/test_ec2utils.py b/ec2api/tests/unit/test_ec2utils.py index 5c838995..67f78aaa 100644 --- a/ec2api/tests/unit/test_ec2utils.py +++ b/ec2api/tests/unit/test_ec2utils.py @@ -178,7 +178,7 @@ class EC2UtilsTestCase(testtools.TestCase): item_id = ec2utils.os_id_to_ec2_id(fake_context, 'fake', fake_os_id) self.assertEqual(fake_id, item_id) db_api.add_item_id.assert_called_once_with( - fake_context, 'fake', fake_os_id, project_id=None) + fake_context, 'fake', fake_os_id, None) # no item in cache, item isn't found db_api.reset_mock() @@ -189,7 +189,7 @@ class EC2UtilsTestCase(testtools.TestCase): self.assertIn(fake_os_id, ids_cache) self.assertEqual(fake_id, ids_cache[fake_os_id]) db_api.add_item_id.assert_called_once_with( - fake_context, 'fake', fake_os_id, project_id=None) + fake_context, 'fake', fake_os_id, None) # no item in cache, item is found db_api.reset_mock() diff --git a/ec2api/tests/unit/test_image.py b/ec2api/tests/unit/test_image.py index 9c124517..2a234efc 100644 --- a/ec2api/tests/unit/test_image.py +++ b/ec2api/tests/unit/test_image.py @@ -125,8 +125,7 @@ class ImageTestCase(base.ApiTestCase): self.db_api.add_item.assert_called_once_with( mock.ANY, 'ami', {'os_id': image_id, 'is_public': False, - 'description': 'fake desc'}, - project_id=None) + 'description': 'fake desc'}) if not no_reboot: os_instance.stop.assert_called_once_with() os_instance.get.assert_called_once_with() @@ -205,8 +204,7 @@ class ImageTestCase(base.ApiTestCase): self.db_api.add_item.assert_called_once_with( mock.ANY, 'ami', {'os_id': fakes.ID_OS_IMAGE_2, 'is_public': False, - 'description': None}, - project_id=None) + 'description': None}) self.assertEqual(1, self.glance.images.create.call_count) self.assertEqual((), self.glance.images.create.call_args[0]) self.assertIn('properties', self.glance.images.create.call_args[1]) diff --git a/ec2api/tests/unit/test_instance.py b/ec2api/tests/unit/test_instance.py index 0bf7d1e7..f3950b92 100644 --- a/ec2api/tests/unit/test_instance.py +++ b/ec2api/tests/unit/test_instance.py @@ -128,8 +128,7 @@ class InstanceTestCase(base.ApiTestCase): nics=[{'port-id': fakes.ID_OS_PORT_1}], key_name=None, userdata=None) self.db_api.add_item.assert_called_once_with( - mock.ANY, 'i', tools.purge_dict(fakes.DB_INSTANCE_1, ('id',)), - project_id=None) + mock.ANY, 'i', tools.purge_dict(fakes.DB_INSTANCE_1, ('id',))) (self.network_interface_api. _attach_network_interface_item.assert_called_once_with( mock.ANY, fakes.DB_NETWORK_INTERFACE_1, @@ -270,8 +269,7 @@ class InstanceTestCase(base.ApiTestCase): [0, 1] * 2, [True, False, True, False])])) self.db_api.add_item.assert_has_calls([ - mock.call(mock.ANY, 'i', tools.purge_dict(db_instance, ['id']), - project_id=None) + mock.call(mock.ANY, 'i', tools.purge_dict(db_instance, ['id'])) for db_instance in self.DB_INSTANCES]) @mock.patch('ec2api.api.instance._parse_block_device_mapping') @@ -330,7 +328,7 @@ class InstanceTestCase(base.ApiTestCase): 'client_token': 'fake_client_token'} db_instance.update(extra_db_instance) self.db_api.add_item.assert_called_once_with( - mock.ANY, 'i', db_instance, project_id=None) + mock.ANY, 'i', db_instance) self.db_api.reset_mock() parse_block_device_mapping.assert_called_once_with( mock.ANY, @@ -1720,10 +1718,9 @@ class InstancePrivateTestCase(test_base.BaseTestCase): fake_context, instance, os_instance, [], {}, None, None, fake_flavors, []) db_api.add_item_id.assert_has_calls( - [mock.call(mock.ANY, 'ami', os_instance.image['id'], - project_id=None), - mock.call(mock.ANY, 'aki', kernel_id, project_id=None), - mock.call(mock.ANY, 'ari', ramdisk_id, project_id=None)], + [mock.call(mock.ANY, 'ami', os_instance.image['id'], None), + mock.call(mock.ANY, 'aki', kernel_id, None), + mock.call(mock.ANY, 'ari', ramdisk_id, None)], any_order=True) @mock.patch('cinderclient.client.Client') diff --git a/ec2api/tests/unit/test_internet_gateway.py b/ec2api/tests/unit/test_internet_gateway.py index 8d5db2c6..db71d49c 100644 --- a/ec2api/tests/unit/test_internet_gateway.py +++ b/ec2api/tests/unit/test_internet_gateway.py @@ -39,7 +39,7 @@ class IgwTestCase(base.ApiTestCase): igw = resp['internetGateway'] self.assertThat(fakes.EC2_IGW_2, matchers.DictMatches(igw)) self.db_api.add_item.assert_called_with( - mock.ANY, 'igw', {}, project_id=None) + mock.ANY, 'igw', {}) def test_attach_igw(self): self.configure(external_network=fakes.NAME_OS_PUBLIC_NETWORK) diff --git a/ec2api/tests/unit/test_network_interface.py b/ec2api/tests/unit/test_network_interface.py index a0147b40..70a4a335 100644 --- a/ec2api/tests/unit/test_network_interface.py +++ b/ec2api/tests/unit/test_network_interface.py @@ -41,8 +41,7 @@ class NetworkInterfaceTestCase(base.ApiTestCase): matchers.DictMatches(resp['networkInterface'])) self.db_api.add_item.assert_called_once_with( mock.ANY, 'eni', - tools.purge_dict(fakes.DB_NETWORK_INTERFACE_1, ('id',)), - project_id=None) + tools.purge_dict(fakes.DB_NETWORK_INTERFACE_1, ('id',))) if auto_ips: self.neutron.create_port.assert_called_once_with( {'port': @@ -129,8 +128,7 @@ class NetworkInterfaceTestCase(base.ApiTestCase): 'device_index', 'instance_id', 'delete_on_termination', - 'attach_time')), - project_id=None) + 'attach_time'))) self.neutron.update_port.assert_called_once_with( fakes.ID_OS_PORT_2, {'port': {'name': diff --git a/ec2api/tests/unit/test_route_table.py b/ec2api/tests/unit/test_route_table.py index 7c962592..5d8774c7 100644 --- a/ec2api/tests/unit/test_route_table.py +++ b/ec2api/tests/unit/test_route_table.py @@ -44,8 +44,7 @@ class RouteTableTestCase(base.ApiTestCase): 'rtb', {'vpc_id': fakes.ID_EC2_VPC_1, 'routes': [{'destination_cidr_block': fakes.CIDR_VPC_1, - 'gateway_id': None}]}, - project_id=None) + 'gateway_id': None}]}) self.db_api.get_item_by_id.assert_called_once_with( mock.ANY, fakes.ID_EC2_VPC_1) diff --git a/ec2api/tests/unit/test_security_group.py b/ec2api/tests/unit/test_security_group.py index 44e63835..4056fdc7 100644 --- a/ec2api/tests/unit/test_security_group.py +++ b/ec2api/tests/unit/test_security_group.py @@ -55,8 +55,7 @@ class SecurityGroupTestCase(base.ApiTestCase): 'groupname', 'Group description') db_group = tools.purge_dict(fakes.DB_SECURITY_GROUP_2, ('id',)) db_group['vpc_id'] = None - self.db_api.add_item.assert_called_once_with( - mock.ANY, 'sg', db_group, project_id=None) + self.db_api.add_item.assert_called_once_with(mock.ANY, 'sg', db_group) self.nova.security_groups.reset_mock() self.db_api.add_item.reset_mock() @@ -70,8 +69,7 @@ class SecurityGroupTestCase(base.ApiTestCase): self.assertEqual(fakes.ID_EC2_SECURITY_GROUP_2, resp['groupId']) self.db_api.add_item.assert_called_once_with( mock.ANY, 'sg', - tools.purge_dict(fakes.DB_SECURITY_GROUP_2, ('id',)), - project_id=None) + tools.purge_dict(fakes.DB_SECURITY_GROUP_2, ('id',))) self.nova.security_groups.create.assert_called_once_with( 'groupname', 'Group description') @@ -328,8 +326,7 @@ class SecurityGroupTestCase(base.ApiTestCase): resp = self.execute('DescribeSecurityGroups', {}) self.db_api.add_item.assert_called_once_with( mock.ANY, 'sg', - tools.purge_dict(fakes.DB_SECURITY_GROUP_1, ('id',)), - project_id=None) + tools.purge_dict(fakes.DB_SECURITY_GROUP_1, ('id',))) self.nova.security_groups.create.assert_called_once_with( fakes.ID_EC2_VPC_1, 'Default VPC security group') diff --git a/ec2api/tests/unit/test_snapshot.py b/ec2api/tests/unit/test_snapshot.py index fdb4200e..26ae6acd 100644 --- a/ec2api/tests/unit/test_snapshot.py +++ b/ec2api/tests/unit/test_snapshot.py @@ -113,8 +113,7 @@ class SnapshotTestCase(base.ApiTestCase): self.assertThat(fakes.EC2_SNAPSHOT_1, matchers.DictMatches(resp)) self.db_api.add_item.assert_called_once_with( mock.ANY, 'snap', - tools.purge_dict(fakes.DB_SNAPSHOT_1, ('id',)), - project_id=None) + tools.purge_dict(fakes.DB_SNAPSHOT_1, ('id',))) self.cinder.volume_snapshots.create.assert_called_once_with( fakes.ID_OS_VOLUME_2, force=True, display_description=None) diff --git a/ec2api/tests/unit/test_subnet.py b/ec2api/tests/unit/test_subnet.py index b52e6584..2fccf520 100644 --- a/ec2api/tests/unit/test_subnet.py +++ b/ec2api/tests/unit/test_subnet.py @@ -44,8 +44,7 @@ class SubnetTestCase(base.ApiTestCase): resp['subnet'])) self.db_api.add_item.assert_called_once_with( mock.ANY, 'subnet', - tools.purge_dict(subnet_1, ('id',)), - project_id=None) + tools.purge_dict(subnet_1, ('id',))) self.neutron.create_network.assert_called_once_with( {'network': {}}) self.neutron.update_network.assert_called_once_with( diff --git a/ec2api/tests/unit/test_volume.py b/ec2api/tests/unit/test_volume.py index 2c478078..3bb76422 100644 --- a/ec2api/tests/unit/test_volume.py +++ b/ec2api/tests/unit/test_volume.py @@ -116,8 +116,7 @@ class VolumeTestCase(base.ApiTestCase): self.assertThat(fakes.EC2_VOLUME_1, matchers.DictMatches(resp)) self.db_api.add_item.assert_called_once_with( mock.ANY, 'vol', - tools.purge_dict(fakes.DB_VOLUME_1, ('id',)), - project_id=None) + tools.purge_dict(fakes.DB_VOLUME_1, ('id',))) self.cinder.volumes.create.assert_called_once_with( None, snapshot_id=None, volume_type=None, @@ -137,8 +136,7 @@ class VolumeTestCase(base.ApiTestCase): self.assertThat(fakes.EC2_VOLUME_3, matchers.DictMatches(resp)) self.db_api.add_item.assert_called_once_with( mock.ANY, 'vol', - tools.purge_dict(fakes.DB_VOLUME_3, ('id',)), - project_id=None) + tools.purge_dict(fakes.DB_VOLUME_3, ('id',))) self.cinder.volumes.create.assert_called_once_with( None, snapshot_id=fakes.ID_OS_SNAPSHOT_1, volume_type=None, diff --git a/ec2api/tests/unit/test_vpc.py b/ec2api/tests/unit/test_vpc.py index 7d558f1c..056c58ae 100644 --- a/ec2api/tests/unit/test_vpc.py +++ b/ec2api/tests/unit/test_vpc.py @@ -49,13 +49,11 @@ class VpcTestCase(base.ApiTestCase): self.db_api.add_item.assert_any_call( mock.ANY, 'vpc', tools.purge_dict(fakes.DB_VPC_1, - ('id', 'vpc_id', 'route_table_id')), - project_id=None) + ('id', 'vpc_id', 'route_table_id'))) self.db_api.add_item.assert_any_call( mock.ANY, 'rtb', tools.purge_dict(fakes.DB_ROUTE_TABLE_1, - ('id',)), - project_id=None) + ('id',))) self.db_api.update_item.assert_called_once_with( mock.ANY, fakes.DB_VPC_1) diff --git a/ec2api/tests/unit/test_vpn_connection.py b/ec2api/tests/unit/test_vpn_connection.py index 4f013dbd..a542e0a5 100644 --- a/ec2api/tests/unit/test_vpn_connection.py +++ b/ec2api/tests/unit/test_vpn_connection.py @@ -68,8 +68,7 @@ class VpnConnectionTestCase(base.ApiTestCase): 'os_ipsec_site_connections': {}}) self.db_api.add_item.assert_called_once_with( mock.ANY, 'vpn', - tools.purge_dict(new_vpn_connection_1, ('id', 'vpc_id', 'os_id')), - project_id=None) + tools.purge_dict(new_vpn_connection_1, ('id', 'vpc_id', 'os_id'))) self.neutron.update_ikepolicy.assert_called_once_with( fakes.ID_OS_IKEPOLICY_1, {'ikepolicy': {'name': fakes.ID_EC2_VPN_CONNECTION_1}}) diff --git a/ec2api/tests/unit/test_vpn_gateway.py b/ec2api/tests/unit/test_vpn_gateway.py index d3c25fd0..036b4a27 100644 --- a/ec2api/tests/unit/test_vpn_gateway.py +++ b/ec2api/tests/unit/test_vpn_gateway.py @@ -45,8 +45,7 @@ class VpnGatewayTestCase(base.ApiTestCase): resp = self.execute('CreateVpnGateway', {'Type': 'ipsec.1'}) self.assertEqual({'vpnGateway': fakes.EC2_VPN_GATEWAY_2}, resp) - self.db_api.add_item.assert_called_once_with( - mock.ANY, 'vgw', {}, project_id=None) + self.db_api.add_item.assert_called_once_with(mock.ANY, 'vgw', {}) @mock.patch('ec2api.api.vpn_connection._reset_vpn_connections', wraps=vpn_connection_api._reset_vpn_connections) diff --git a/ec2api/tests/unit/tools.py b/ec2api/tests/unit/tools.py index 0fa908ef..d454a835 100644 --- a/ec2api/tests/unit/tools.py +++ b/ec2api/tests/unit/tools.py @@ -49,14 +49,12 @@ def patch_dict(dict1, dict2, trash_iter): def get_db_api_add_item(item_id_dict): """Generate db_api.add_item mock function.""" - def db_api_add_item(context, kind, data, project_id=None): + def db_api_add_item(context, kind, data): if isinstance(item_id_dict, dict): item_id = item_id_dict[kind] else: item_id = item_id_dict data = update_dict(data, {'id': item_id}) - if project_id: - data = update_dict(data, {'project_id': project_id}) data.setdefault('os_id') data.setdefault('vpc_id') return data