From 44292f4c9adc434c93c50492282c3bd1b85e4e54 Mon Sep 17 00:00:00 2001
From: Boden R <bodenvmw@gmail.com>
Date: Thu, 11 Jan 2018 15:40:52 -0700
Subject: [PATCH] consume neutron-lib resources attr map

Today we shim the RESOURCE_ATTRIBUTE_MAP in neutron; it references the
equivelant in neutron-lib named RESOURCES.

This patch removes neutron's RESOURCE_ATTRIBUTE_MAP and cleans up
neutron.api.v2.attributes in prep to delete it.
To do so:
- CORE_RESOURCES and RESOURCE_FOREIGN_KEYS are moved to the single
module that references them respectively and the are made private (no
consumers use them).
- get_collection_info is removed and instead the 2 uses in neutron
just use the get() method of the RESOURCES map. There are no external
uses of get_collection_info.
- References using RESOURCE_ATTRIBUTE_MAP are switched over to
neutron-lib's RESOURCES.
- The neutron.api.v2.attributes module is removed as it's empty now.
- A few api attribute UTs are removed; there's nothing to test as per
this patch.

NeutronLibImpact

Change-Id: Iaacee584d499c4d33d6d2dd9609c7ac0f2cfc386
---
 doc/source/contributor/internals/policy.rst   |  6 +--
 neutron/api/v2/attributes.py                  | 43 -------------------
 neutron/pecan_wsgi/controllers/quota.py       |  4 +-
 neutron/pecan_wsgi/controllers/root.py        | 12 +++++-
 neutron/pecan_wsgi/controllers/utils.py       |  4 +-
 neutron/pecan_wsgi/startup.py                 |  4 +-
 neutron/plugins/common/utils.py               |  3 +-
 neutron/policy.py                             | 14 ++++--
 neutron/tests/functional/api/test_policies.py |  6 +--
 .../tests/functional/pecan_wsgi/test_hooks.py |  4 +-
 neutron/tests/unit/api/v2/test_attributes.py  | 32 --------------
 neutron/tests/unit/api/v2/test_base.py        |  6 +--
 neutron/tests/unit/extensions/test_tag.py     |  4 +-
 neutron/tests/unit/test_policy.py             |  4 +-
 14 files changed, 42 insertions(+), 104 deletions(-)
 delete mode 100644 neutron/api/v2/attributes.py
 delete mode 100644 neutron/tests/unit/api/v2/test_attributes.py

diff --git a/doc/source/contributor/internals/policy.rst b/doc/source/contributor/internals/policy.rst
index a1ae5541ea7..636a00a30b5 100644
--- a/doc/source/contributor/internals/policy.rst
+++ b/doc/source/contributor/internals/policy.rst
@@ -195,8 +195,8 @@ The check, performed in the ``__call__`` method, works as follows:
     ``network`` resource;
   * if no parent resource or target field could be identified raise a
     ``PolicyCheckError`` exception;
-  * Retrieve a 'parent foreign key' from the ``RESOURCE_FOREIGN_KEYS`` data
-    structure in ``neutron.api.v2.attributes``. This foreign key is simply the
+  * Retrieve a 'parent foreign key' from the ``_RESOURCE_FOREIGN_KEYS`` data
+    structure in ``neutron.policy``. This foreign key is simply the
     attribute acting as a primary key in the parent resource. A
     ``PolicyCheckError`` exception will be raised if such 'parent foreign key'
     cannot be retrieved;
@@ -288,7 +288,7 @@ Notes
    imply a round-trip to the backend. This class of checks, when involving
    retrieving attributes for 'parent' resources should be used very sparingly.
  * In order for ``OwnerCheck`` rules to work, parent resources should have an
-   entry in ``neutron.api.v2.attributes.RESOURCE_FOREIGN_KEYS``; moreover the
+   entry in ``neutron.policy._RESOURCE_FOREIGN_KEYS``; moreover the
    resource must be managed by the 'core' plugin (ie: the one defined in the
    core_plugin configuration variable)
 
diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py
deleted file mode 100644
index 3be7f8a41dc..00000000000
--- a/neutron/api/v2/attributes.py
+++ /dev/null
@@ -1,43 +0,0 @@
-# Copyright (c) 2012 OpenStack Foundation.
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-from neutron_lib.api import attributes as attrs
-from neutron_lib.api.definitions import network as net_def
-from neutron_lib.api.definitions import port as port_def
-from neutron_lib.api.definitions import subnet as subnet_def
-from neutron_lib.api.definitions import subnetpool as subnetpool_def
-
-
-# Define constants for base resource name
-CORE_RESOURCES = {net_def.RESOURCE_NAME: net_def.COLLECTION_NAME,
-                  subnet_def.RESOURCE_NAME: subnet_def.COLLECTION_NAME,
-                  subnetpool_def.RESOURCE_NAME: subnetpool_def.COLLECTION_NAME,
-                  port_def.RESOURCE_NAME: port_def.COLLECTION_NAME}
-
-RESOURCE_ATTRIBUTE_MAP = attrs.RESOURCES
-
-# Identify the attribute used by a resource to reference another resource
-
-RESOURCE_FOREIGN_KEYS = {
-    net_def.COLLECTION_NAME: 'network_id'
-}
-
-
-def get_collection_info(collection):
-    """Helper function to retrieve attribute info.
-
-    :param collection: Collection or plural name of the resource
-    """
-    return RESOURCE_ATTRIBUTE_MAP.get(collection)
diff --git a/neutron/pecan_wsgi/controllers/quota.py b/neutron/pecan_wsgi/controllers/quota.py
index a11ce5d4c28..d0a70733e23 100644
--- a/neutron/pecan_wsgi/controllers/quota.py
+++ b/neutron/pecan_wsgi/controllers/quota.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron_lib.api import attributes
 from neutron_lib.api import converters
 from neutron_lib.db import constants as db_const
 from neutron_lib import exceptions as n_exc
@@ -22,7 +23,6 @@ import pecan
 from pecan import request
 
 from neutron._i18n import _
-from neutron.api.v2 import attributes
 from neutron.pecan_wsgi.controllers import utils
 from neutron.quota import resource_registry
 
@@ -83,7 +83,7 @@ class QuotaController(utils.NeutronPecanController):
             "%ss" % RESOURCE_NAME, RESOURCE_NAME)
 
         # Ensure limits for all registered resources are returned
-        attr_dict = attributes.RESOURCE_ATTRIBUTE_MAP[self.collection]
+        attr_dict = attributes.RESOURCES[self.collection]
         for quota_resource in resource_registry.get_all_resources().keys():
             attr_dict[quota_resource] = {
                 'allow_post': False,
diff --git a/neutron/pecan_wsgi/controllers/root.py b/neutron/pecan_wsgi/controllers/root.py
index 670447444e2..7c1df62caef 100644
--- a/neutron/pecan_wsgi/controllers/root.py
+++ b/neutron/pecan_wsgi/controllers/root.py
@@ -14,13 +14,16 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron_lib.api.definitions import network as net_def
+from neutron_lib.api.definitions import port as port_def
+from neutron_lib.api.definitions import subnet as subnet_def
+from neutron_lib.api.definitions import subnetpool as subnetpool_def
 from oslo_config import cfg
 from oslo_log import log
 import pecan
 from pecan import request
 import six.moves.urllib.parse as urlparse
 
-from neutron.api.v2 import attributes
 from neutron.api.views import versions as versions_view
 from neutron import manager
 from neutron.pecan_wsgi.controllers import extensions as ext_ctrl
@@ -31,6 +34,11 @@ CONF = cfg.CONF
 
 LOG = log.getLogger(__name__)
 _VERSION_INFO = {}
+_CORE_RESOURCES = {net_def.RESOURCE_NAME: net_def.COLLECTION_NAME,
+                   subnet_def.RESOURCE_NAME: subnet_def.COLLECTION_NAME,
+                   subnetpool_def.RESOURCE_NAME:
+                       subnetpool_def.COLLECTION_NAME,
+                   port_def.RESOURCE_NAME: port_def.COLLECTION_NAME}
 
 
 def _load_version_info(version_info):
@@ -88,7 +96,7 @@ class V2Controller(object):
             pecan.abort(404)
 
         layout = []
-        for name, collection in attributes.CORE_RESOURCES.items():
+        for name, collection in _CORE_RESOURCES.items():
             href = urlparse.urljoin(pecan.request.path_url, collection)
             resource = {'name': name,
                         'collection': collection,
diff --git a/neutron/pecan_wsgi/controllers/utils.py b/neutron/pecan_wsgi/controllers/utils.py
index 0ee1101d524..f0f1403b7d1 100644
--- a/neutron/pecan_wsgi/controllers/utils.py
+++ b/neutron/pecan_wsgi/controllers/utils.py
@@ -17,6 +17,7 @@ from collections import defaultdict
 import copy
 import functools
 
+from neutron_lib.api import attributes
 from neutron_lib import constants
 from oslo_log import log as logging
 from oslo_utils import excutils
@@ -25,7 +26,6 @@ from pecan import request
 
 from neutron._i18n import _
 from neutron.api import api_common
-from neutron.api.v2 import attributes as api_attributes
 from neutron.db import api as db_api
 from neutron import manager
 from neutron_lib import exceptions
@@ -210,7 +210,7 @@ class NeutronPecanController(object):
     @property
     def resource_info(self):
         if not self._resource_info:
-            self._resource_info = api_attributes.get_collection_info(
+            self._resource_info = attributes.RESOURCES.get(
                 self.collection)
         return self._resource_info
 
diff --git a/neutron/pecan_wsgi/startup.py b/neutron/pecan_wsgi/startup.py
index 0b442990456..52f7bc52ef5 100644
--- a/neutron/pecan_wsgi/startup.py
+++ b/neutron/pecan_wsgi/startup.py
@@ -13,10 +13,10 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron_lib.api import attributes
 from neutron_lib.plugins import directory
 
 from neutron.api import extensions
-from neutron.api.v2 import attributes
 from neutron.api.v2 import base
 from neutron import manager
 from neutron.pecan_wsgi.controllers import resource as res_ctrl
@@ -38,7 +38,7 @@ RESOURCES = {'network': 'networks',
 def initialize_all():
     manager.init()
     ext_mgr = extensions.PluginAwareExtensionManager.get_instance()
-    ext_mgr.extend_resources("2.0", attributes.RESOURCE_ATTRIBUTE_MAP)
+    ext_mgr.extend_resources("2.0", attributes.RESOURCES)
     # At this stage we have a fully populated resource attribute map;
     # build Pecan controllers and routes for all core resources
     plugin = directory.get_plugin()
diff --git a/neutron/plugins/common/utils.py b/neutron/plugins/common/utils.py
index 2d0c9a29723..7a65cca4434 100644
--- a/neutron/plugins/common/utils.py
+++ b/neutron/plugins/common/utils.py
@@ -33,7 +33,6 @@ from oslo_utils import excutils
 import webob.exc
 
 from neutron._i18n import _
-from neutron.api.v2 import attributes
 from neutron.common import exceptions as n_exc
 
 
@@ -153,7 +152,7 @@ def in_pending_status(status):
 
 
 def _fixup_res_dict(context, attr_name, res_dict, check_allow_post=True):
-    attr_info = attributes.RESOURCE_ATTRIBUTE_MAP[attr_name]
+    attr_info = lib_attrs.RESOURCES[attr_name]
     attr_ops = lib_attrs.AttributeInfo(attr_info)
     try:
         attr_ops.populate_project_id(context, res_dict, True)
diff --git a/neutron/policy.py b/neutron/policy.py
index 29effa62057..995c206c72b 100644
--- a/neutron/policy.py
+++ b/neutron/policy.py
@@ -16,6 +16,8 @@
 import collections
 import re
 
+from neutron_lib.api import attributes
+from neutron_lib.api.definitions import network as net_apidef
 from neutron_lib import constants
 from neutron_lib import context
 from neutron_lib import exceptions
@@ -28,7 +30,6 @@ from oslo_utils import excutils
 import six
 
 from neutron._i18n import _
-from neutron.api.v2 import attributes
 from neutron.common import cache_utils as cache
 from neutron.common import constants as const
 
@@ -39,6 +40,11 @@ _ENFORCER = None
 ADMIN_CTX_POLICY = 'context_is_admin'
 ADVSVC_CTX_POLICY = 'context_is_advsvc'
 
+# Identify the attribute used by a resource to reference another resource
+_RESOURCE_FOREIGN_KEYS = {
+    net_apidef.COLLECTION_NAME: 'network_id'
+}
+
 
 def reset():
     global _ENFORCER
@@ -160,7 +166,7 @@ def _build_match_rule(action, target, pluralized):
         action, pluralized)
     if enforce_attr_based_check:
         # assigning to variable with short name for improving readability
-        res_map = attributes.RESOURCE_ATTRIBUTE_MAP
+        res_map = attributes.RESOURCES
         if resource in res_map:
             for attribute_name in res_map[resource]:
                 if _is_attribute_explicitly_set(attribute_name,
@@ -264,7 +270,7 @@ class OwnerCheck(policy.Check):
                 raise exceptions.PolicyCheckError(
                     policy="%s:%s" % (self.kind, self.match),
                     reason=err_reason)
-            parent_foreign_key = attributes.RESOURCE_FOREIGN_KEYS.get(
+            parent_foreign_key = _RESOURCE_FOREIGN_KEYS.get(
                 "%ss" % parent_res, None)
             if not parent_foreign_key:
                 err_reason = (_("Unable to verify match:%(match)s as the "
@@ -296,7 +302,7 @@ class FieldCheck(policy.Check):
 
         # Value might need conversion - we need help from the attribute map
         try:
-            attr = attributes.RESOURCE_ATTRIBUTE_MAP[resource][field]
+            attr = attributes.RESOURCES[resource][field]
             conv_func = attr['convert_to']
         except KeyError:
             conv_func = lambda x: x
diff --git a/neutron/tests/functional/api/test_policies.py b/neutron/tests/functional/api/test_policies.py
index 6e8f2f2effc..6b8b2d58e31 100644
--- a/neutron/tests/functional/api/test_policies.py
+++ b/neutron/tests/functional/api/test_policies.py
@@ -15,11 +15,11 @@
 
 import os.path
 
+from neutron_lib.api import attributes
 from neutron_lib import context
 from neutron_lib import fixture
 
 from neutron.api import extensions
-from neutron.api.v2 import attributes
 from neutron import policy
 from neutron.tests import base
 
@@ -69,7 +69,7 @@ class APIPolicyTestCase(base.BaseTestCase):
         admin_context = context.get_admin_context()
         tenant_context = context.Context('test_user', 'test_tenant_id', False)
         extension_manager.extend_resources(self.api_version,
-                                           attributes.RESOURCE_ATTRIBUTE_MAP)
+                                           attributes.RESOURCES)
         self.assertTrue(self._check_external_router_policy(admin_context))
         self.assertFalse(self._check_external_router_policy(tenant_context))
 
@@ -82,7 +82,7 @@ class APIPolicyTestCase(base.BaseTestCase):
         policy.reset()
         extension_manager = extensions.ExtensionManager(self.extension_path)
         extension_manager.extend_resources(self.api_version,
-                                           attributes.RESOURCE_ATTRIBUTE_MAP)
+                                           attributes.RESOURCES)
         policy.init()
         admin_context = context.get_admin_context()
         tenant_context = context.Context('test_user', 'test_tenant_id', False)
diff --git a/neutron/tests/functional/pecan_wsgi/test_hooks.py b/neutron/tests/functional/pecan_wsgi/test_hooks.py
index 00572da4071..d42f1f9f107 100644
--- a/neutron/tests/functional/pecan_wsgi/test_hooks.py
+++ b/neutron/tests/functional/pecan_wsgi/test_hooks.py
@@ -14,6 +14,7 @@
 #    under the License.
 
 import mock
+from neutron_lib.api import attributes
 from neutron_lib.callbacks import events
 from neutron_lib import context
 from neutron_lib.db import constants as db_const
@@ -22,7 +23,6 @@ from oslo_config import cfg
 from oslo_policy import policy as oslo_policy
 from oslo_serialization import jsonutils
 
-from neutron.api.v2 import attributes
 from neutron.db.quota import driver as quota_driver
 from neutron import manager
 from neutron.pecan_wsgi.controllers import resource
@@ -159,7 +159,7 @@ class TestPolicyEnforcementHook(test_functional.PecanFunctionalTest):
         # tests, or at least I hope so)
         super(TestPolicyEnforcementHook, self).setUp()
         self.mock_plugin = mock.Mock()
-        attributes.RESOURCE_ATTRIBUTE_MAP.update(self.FAKE_RESOURCE)
+        attributes.RESOURCES.update(self.FAKE_RESOURCE)
         manager.NeutronManager.set_plugin_for_resource('mehs',
                                                        self.mock_plugin)
         fake_controller = resource.CollectionsController('mehs', 'meh')
diff --git a/neutron/tests/unit/api/v2/test_attributes.py b/neutron/tests/unit/api/v2/test_attributes.py
deleted file mode 100644
index e269c5b6f9a..00000000000
--- a/neutron/tests/unit/api/v2/test_attributes.py
+++ /dev/null
@@ -1,32 +0,0 @@
-# Copyright 2012 OpenStack Foundation
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-from neutron.api.v2 import attributes
-from neutron.tests import base
-
-
-class TestHelpers(base.DietTestCase):
-
-    def _verify_port_attributes(self, attrs):
-        for test_attribute in ('id', 'name', 'mac_address', 'network_id',
-                               'tenant_id', 'fixed_ips', 'status'):
-            self.assertIn(test_attribute, attrs)
-
-    def test_get_collection_info(self):
-        attrs = attributes.get_collection_info('ports')
-        self._verify_port_attributes(attrs)
-
-    def test_get_collection_info_missing(self):
-        self.assertFalse(attributes.get_collection_info('meh'))
diff --git a/neutron/tests/unit/api/v2/test_base.py b/neutron/tests/unit/api/v2/test_base.py
index 5d1d7d93066..5cb367345d9 100644
--- a/neutron/tests/unit/api/v2/test_base.py
+++ b/neutron/tests/unit/api/v2/test_base.py
@@ -16,6 +16,7 @@
 import os
 
 import mock
+from neutron_lib.api import attributes
 from neutron_lib.api import converters
 from neutron_lib.callbacks import registry
 from neutron_lib import constants
@@ -35,7 +36,6 @@ import webtest
 
 from neutron.api import api_common
 from neutron.api import extensions
-from neutron.api.v2 import attributes
 from neutron.api.v2 import base as v2_base
 from neutron.api.v2 import router
 from neutron import policy
@@ -125,7 +125,7 @@ class APIv2TestCase(APIv2TestBase):
         return sorted(policy_attrs)
 
     def _do_field_list(self, resource, base_fields):
-        attr_info = attributes.RESOURCE_ATTRIBUTE_MAP[resource]
+        attr_info = attributes.RESOURCES[resource]
         policy_attrs = self._get_policy_attrs(attr_info)
         for name, info in attr_info.items():
             if info.get('primary_key'):
@@ -1260,7 +1260,7 @@ class V2Views(base.BaseTestCase):
     def _view(self, keys, collection, resource):
         data = dict((key, 'value') for key in keys)
         data['fake'] = 'value'
-        attr_info = attributes.RESOURCE_ATTRIBUTE_MAP[collection]
+        attr_info = attributes.RESOURCES[collection]
         controller = v2_base.Controller(None, collection, resource, attr_info)
         res = controller._view(context.get_admin_context(), data)
         self.assertNotIn('fake', res)
diff --git a/neutron/tests/unit/extensions/test_tag.py b/neutron/tests/unit/extensions/test_tag.py
index 0db7736e98c..e2ba1b673ae 100644
--- a/neutron/tests/unit/extensions/test_tag.py
+++ b/neutron/tests/unit/extensions/test_tag.py
@@ -10,12 +10,12 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron_lib.api import attributes
 from neutron_lib import context
 from oslo_utils import uuidutils
 import testscenarios
 
 from neutron.api import extensions
-from neutron.api.v2 import attributes
 from neutron.common import config
 import neutron.extensions
 from neutron.objects.qos import policy
@@ -87,7 +87,7 @@ class TestTagApiBase(test_securitygroup.SecurityGroupsTestCase,
             extensions_path, {'router': l3_plugin, 'TAG': plugin,
                               'sec': sec_plugin}
         )
-        ext_mgr.extend_resources("2.0", attributes.RESOURCE_ATTRIBUTE_MAP)
+        ext_mgr.extend_resources("2.0", attributes.RESOURCES)
         app = config.load_paste_app('extensions_test_app')
         self.ext_api = extensions.ExtensionMiddleware(app, ext_mgr=ext_mgr)
 
diff --git a/neutron/tests/unit/test_policy.py b/neutron/tests/unit/test_policy.py
index 3b707bcd1ef..3984e4ebf50 100644
--- a/neutron/tests/unit/test_policy.py
+++ b/neutron/tests/unit/test_policy.py
@@ -16,6 +16,7 @@
 """Test of Policy Engine For Neutron"""
 
 import mock
+from neutron_lib.api import attributes
 from neutron_lib import constants
 from neutron_lib import context
 from neutron_lib import exceptions
@@ -29,7 +30,6 @@ from oslo_serialization import jsonutils
 from oslo_utils import importutils
 
 import neutron
-from neutron.api.v2 import attributes
 from neutron.common import constants as n_const
 from neutron import policy
 from neutron.tests import base
@@ -207,7 +207,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
     def setUp(self):
         super(NeutronPolicyTestCase, self).setUp()
         # Add Fake resources to RESOURCE_ATTRIBUTE_MAP
-        attributes.RESOURCE_ATTRIBUTE_MAP.update(FAKE_RESOURCES)
+        attributes.RESOURCES.update(FAKE_RESOURCES)
         self._set_rules()
 
         self.patcher = mock.patch.object(neutron.policy,