From 1a98539797b3325f7bc4fd481ff2e699de9e9127 Mon Sep 17 00:00:00 2001 From: Boden R Date: Mon, 22 May 2017 10:46:04 -0600 Subject: [PATCH] APIDefinitionFixture bugfix Currently APIDefinitionFixture backs up the contents of its target dict (attributes.RESOURCES and/or an API def's RESOURCE_ATTRIBUTE_MAP) and on restore replaces the respective target with its backup copy. While this works in a number of cases, it doesn't take into account "longer" references to the respective dict. In some cases consumers hold a reference to an attribute dict longer than the duration of the fixture in which case their reference is not cleaned up on restore. This patch address this bug and includes UTs. No release note is included as this is only an internal behavior change. Change-Id: Id61ba4e4649d3a73c9397632755f1cd0c38d7ab3 --- neutron_lib/fixture.py | 12 ++++++----- neutron_lib/tests/unit/test_fixture.py | 30 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/neutron_lib/fixture.py b/neutron_lib/fixture.py index 03a22c392..94735fe08 100644 --- a/neutron_lib/fixture.py +++ b/neutron_lib/fixture.py @@ -132,20 +132,22 @@ class APIDefinitionFixture(fixtures.Fixture): def _setUp(self): for api_def in self.definitions: self._orig_attr_maps[api_def.ALIAS] = ( - api_def, api_def.RESOURCE_ATTRIBUTE_MAP) - api_def.RESOURCE_ATTRIBUTE_MAP = copy.deepcopy( - api_def.RESOURCE_ATTRIBUTE_MAP) + api_def, {k: copy.deepcopy(v) + for k, v in api_def.RESOURCE_ATTRIBUTE_MAP.items()}) if self.backup_global_resources: for resource, attrs in attributes.RESOURCES.items(): self._orig_resources[resource] = copy.deepcopy(attrs) self.addCleanup(self._restore) def _restore(self): + # clear + repopulate so consumer refs don't change for alias, def_and_map in self._orig_attr_maps.items(): api_def, attr_map = def_and_map[0], def_and_map[1] - api_def.RESOURCE_ATTRIBUTE_MAP = attr_map + api_def.RESOURCE_ATTRIBUTE_MAP.clear() + api_def.RESOURCE_ATTRIBUTE_MAP.update(attr_map) if self.backup_global_resources: - attributes.RESOURCES = self._orig_resources + attributes.RESOURCES.clear() + attributes.RESOURCES.update(self._orig_resources) @classmethod def all_api_definitions_fixture(cls): diff --git a/neutron_lib/tests/unit/test_fixture.py b/neutron_lib/tests/unit/test_fixture.py index b959b985d..ab0e35bfa 100644 --- a/neutron_lib/tests/unit/test_fixture.py +++ b/neutron_lib/tests/unit/test_fixture.py @@ -17,6 +17,7 @@ from oslo_db import options from oslotest import base from neutron_lib.api import attributes +from neutron_lib.api.definitions import port from neutron_lib.callbacks import registry from neutron_lib.db import model_base from neutron_lib import fixture @@ -92,3 +93,32 @@ class APIDefinitionFixtureTestCase(base.BaseTestCase): def test_all_api_definitions_fixture_with_global_backup(self): self._test_all_api_definitions_fixture(global_cleanup=True) + + def test_global_resources_reference_updated(self): + resources_ref = attributes.RESOURCES + apis = fixture.APIDefinitionFixture() + + apis.setUp() + attributes.RESOURCES['test_resource'] = {} + self.assertIn('test_resource', resources_ref) + attributes.RESOURCES[port.COLLECTION_NAME]['test_port_attr'] = {} + self.assertIn('test_port_attr', + attributes.RESOURCES[port.COLLECTION_NAME]) + apis.cleanUp() + + self.assertNotIn('test_port_attr', + attributes.RESOURCES[port.COLLECTION_NAME]) + self.assertNotIn('test_resource', resources_ref) + + def test_api_def_reference_updated(self): + api_def_ref = port.RESOURCE_ATTRIBUTE_MAP + apis = fixture.APIDefinitionFixture() + + apis.setUp() + port.RESOURCE_ATTRIBUTE_MAP[port.COLLECTION_NAME]['test_attr'] = {} + self.assertIn('test_attr', api_def_ref[port.COLLECTION_NAME]) + apis.cleanUp() + + self.assertNotIn('test_attr', + port.RESOURCE_ATTRIBUTE_MAP[port.COLLECTION_NAME]) + self.assertNotIn('test_attr', api_def_ref[port.COLLECTION_NAME])