From dce7c52de91a8717e2c16ff74ce13802e7d6a573 Mon Sep 17 00:00:00 2001
From: Morgan Fainberg <morgan.fainberg@gmail.com>
Date: Thu, 2 Feb 2017 14:18:19 -0800
Subject: [PATCH] Make use of Dict-base including extras explicit

This change replaces the use of DictBase with the ModelDictMixin
for any SQL models that do not contain an extra column and renames
the DictBase to a more descriptive name of ModelDictMixinWithExtras.

A Docstring has been added indicating the continued usage of
ModelDictMixinWithExtras should not be done for any "new"
models.

Change-Id: I9a4767cacf7620e878df70084060f3e43e1318df
---
 doc/source/devref/api_change_tutorial.rst  |  2 +-
 keystone/assignment/backends/sql.py        |  2 +-
 keystone/assignment/role_backends/sql.py   |  4 ++--
 keystone/catalog/backends/sql.py           |  6 ++---
 keystone/common/sql/core.py                | 27 ++++++++++++++++++++--
 keystone/credential/backends/sql.py        |  2 +-
 keystone/federation/backends/sql.py        | 10 ++++----
 keystone/identity/backends/sql_model.py    | 10 ++++----
 keystone/oauth1/backends/sql.py            |  6 ++---
 keystone/policy/backends/sql.py            |  2 +-
 keystone/resource/backends/sql.py          |  2 +-
 keystone/token/persistence/backends/sql.py |  2 +-
 keystone/trust/backends/sql.py             |  2 +-
 13 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/doc/source/devref/api_change_tutorial.rst b/doc/source/devref/api_change_tutorial.rst
index 2d9fdde37d..020c077eb5 100644
--- a/doc/source/devref/api_change_tutorial.rst
+++ b/doc/source/devref/api_change_tutorial.rst
@@ -87,7 +87,7 @@ Changing the SQL Model and Driver
 First, you need to change the role model to include the description attribute.
 Go to `keystone/assignment/role_backends/sql.py` and update it like::
 
-    class RoleTable(sql.ModelBase, sql.DictBase):
+    class RoleTable(sql.ModelBase, sql.ModelDictMixin):
 
         attributes = ['id', 'name', 'domain_id', 'description']
         description = sql.Column(sql.String(255), nullable=True)
diff --git a/keystone/assignment/backends/sql.py b/keystone/assignment/backends/sql.py
index a9a40716d8..ce0778daa1 100644
--- a/keystone/assignment/backends/sql.py
+++ b/keystone/assignment/backends/sql.py
@@ -288,7 +288,7 @@ class Assignment(base.AssignmentDriverBase):
             q.delete(False)
 
 
-class RoleAssignment(sql.ModelBase, sql.DictBase):
+class RoleAssignment(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'assignment'
     attributes = ['type', 'actor_id', 'target_id', 'role_id', 'inherited']
     # NOTE(henry-nash): Postgres requires a name to be defined for an Enum
diff --git a/keystone/assignment/role_backends/sql.py b/keystone/assignment/role_backends/sql.py
index 52052df0a1..081e1517d4 100644
--- a/keystone/assignment/role_backends/sql.py
+++ b/keystone/assignment/role_backends/sql.py
@@ -145,7 +145,7 @@ class Role(base.RoleDriverBase):
             return ref.to_dict()
 
 
-class ImpliedRoleTable(sql.ModelBase, sql.DictBase):
+class ImpliedRoleTable(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'implied_role'
     attributes = ['prior_role_id', 'implied_role_id']
     prior_role_id = sql.Column(
@@ -174,7 +174,7 @@ class ImpliedRoleTable(sql.ModelBase, sql.DictBase):
         return d
 
 
-class RoleTable(sql.ModelBase, sql.DictBase):
+class RoleTable(sql.ModelBase, sql.ModelDictMixinWithExtras):
 
     def to_dict(self, include_extra_dict=False):
         d = super(RoleTable, self).to_dict(
diff --git a/keystone/catalog/backends/sql.py b/keystone/catalog/backends/sql.py
index a55ea3eb1a..329481c6c6 100644
--- a/keystone/catalog/backends/sql.py
+++ b/keystone/catalog/backends/sql.py
@@ -31,7 +31,7 @@ from keystone.i18n import _
 CONF = keystone.conf.CONF
 
 
-class Region(sql.ModelBase, sql.DictBase):
+class Region(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'region'
     attributes = ['id', 'description', 'parent_region_id']
     id = sql.Column(sql.String(255), primary_key=True)
@@ -50,7 +50,7 @@ class Region(sql.ModelBase, sql.DictBase):
     endpoints = sqlalchemy.orm.relationship("Endpoint", backref="region")
 
 
-class Service(sql.ModelBase, sql.DictBase):
+class Service(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'service'
     attributes = ['id', 'type', 'enabled']
     id = sql.Column(sql.String(64), primary_key=True)
@@ -61,7 +61,7 @@ class Service(sql.ModelBase, sql.DictBase):
     endpoints = sqlalchemy.orm.relationship("Endpoint", backref="service")
 
 
-class Endpoint(sql.ModelBase, sql.DictBase):
+class Endpoint(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'endpoint'
     attributes = ['id', 'interface', 'region_id', 'service_id', 'url',
                   'legacy_endpoint_id', 'enabled']
diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py
index 221ee81292..9a24618132 100644
--- a/keystone/common/sql/core.py
+++ b/keystone/common/sql/core.py
@@ -123,13 +123,31 @@ class JsonBlob(sql_types.TypeDecorator):
         return jsonutils.loads(value)
 
 
-class DictBase(models.ModelBase):
+class ModelDictMixinWithExtras(models.ModelBase):
+    """Mixin making model behave with dict-like interfaces includes extras.
+
+    NOTE: DO NOT USE THIS FOR FUTURE SQL MODELS. "Extra" column is a legacy
+          concept that should not be carried forward with new SQL models
+          as the concept of "arbitrary" properties is not in line with
+          the design philosophy of Keystone.
+    """
+
     attributes = []
+    _msg = ('Programming Error: Model does not have an "extra" column. '
+            'Unless the model already has an "extra" column and has '
+            'existed in a previous released version of keystone with '
+            'the extra column included, the model should use '
+            '"ModelDictMixin" instead.')
 
     @classmethod
     def from_dict(cls, d):
         new_d = d.copy()
 
+        if not hasattr(cls, 'extra'):
+            # NOTE(notmorgan): No translation here, This is an error for
+            # programmers NOT end users.
+            raise AttributeError(cls._msg)  # no qa
+
         new_d['extra'] = {k: new_d.pop(k) for k in d.keys()
                           if k not in cls.attributes and k != 'extra'}
 
@@ -143,6 +161,11 @@ class DictBase(models.ModelBase):
         with a broken implementation.
 
         """
+        if not hasattr(self, 'extra'):
+            # NOTE(notmorgan): No translation here, This is an error for
+            # programmers NOT end users.
+            raise AttributeError(self._msg)  # no qa
+
         d = self.extra.copy()
         for attr in self.__class__.attributes:
             d[attr] = getattr(self, attr)
@@ -159,7 +182,7 @@ class DictBase(models.ModelBase):
         return getattr(self, key)
 
 
-class ModelDictMixin(object):
+class ModelDictMixin(models.ModelBase):
 
     @classmethod
     def from_dict(cls, d):
diff --git a/keystone/credential/backends/sql.py b/keystone/credential/backends/sql.py
index 6eb0385f39..144307161b 100644
--- a/keystone/credential/backends/sql.py
+++ b/keystone/credential/backends/sql.py
@@ -18,7 +18,7 @@ from keystone.credential.backends import base
 from keystone import exception
 
 
-class CredentialModel(sql.ModelBase, sql.DictBase):
+class CredentialModel(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'credential'
     attributes = [
         'id', 'user_id', 'project_id', 'encrypted_blob', 'type', 'key_hash'
diff --git a/keystone/federation/backends/sql.py b/keystone/federation/backends/sql.py
index 85990e21ff..e2a075529b 100644
--- a/keystone/federation/backends/sql.py
+++ b/keystone/federation/backends/sql.py
@@ -26,7 +26,7 @@ from keystone.i18n import _
 LOG = log.getLogger(__name__)
 
 
-class FederationProtocolModel(sql.ModelBase, sql.DictBase):
+class FederationProtocolModel(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'federation_protocol'
     attributes = ['id', 'idp_id', 'mapping_id']
     mutable_attributes = frozenset(['mapping_id'])
@@ -49,7 +49,7 @@ class FederationProtocolModel(sql.ModelBase, sql.DictBase):
         return d
 
 
-class IdentityProviderModel(sql.ModelBase, sql.DictBase):
+class IdentityProviderModel(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'identity_provider'
     attributes = ['id', 'domain_id', 'enabled', 'description', 'remote_ids']
     mutable_attributes = frozenset(['description', 'enabled', 'remote_ids'])
@@ -89,7 +89,7 @@ class IdentityProviderModel(sql.ModelBase, sql.DictBase):
         return d
 
 
-class IdPRemoteIdsModel(sql.ModelBase, sql.DictBase):
+class IdPRemoteIdsModel(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'idp_remote_ids'
     attributes = ['idp_id', 'remote_id']
     mutable_attributes = frozenset(['idp_id', 'remote_id'])
@@ -113,7 +113,7 @@ class IdPRemoteIdsModel(sql.ModelBase, sql.DictBase):
         return d
 
 
-class MappingModel(sql.ModelBase, sql.DictBase):
+class MappingModel(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'mapping'
     attributes = ['id', 'rules']
 
@@ -135,7 +135,7 @@ class MappingModel(sql.ModelBase, sql.DictBase):
         return d
 
 
-class ServiceProviderModel(sql.ModelBase, sql.DictBase):
+class ServiceProviderModel(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'service_provider'
     attributes = ['auth_url', 'id', 'enabled', 'description',
                   'relay_state_prefix', 'sp_url']
diff --git a/keystone/identity/backends/sql_model.py b/keystone/identity/backends/sql_model.py
index 8dd9c9bb88..bef16d4712 100644
--- a/keystone/identity/backends/sql_model.py
+++ b/keystone/identity/backends/sql_model.py
@@ -29,7 +29,7 @@ from keystone.identity.backends import resource_options as iro
 CONF = keystone.conf.CONF
 
 
-class User(sql.ModelBase, sql.DictBase):
+class User(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'user'
     attributes = ['id', 'name', 'domain_id', 'password', 'enabled',
                   'default_project_id', 'password_expires_at']
@@ -255,7 +255,7 @@ class User(sql.ModelBase, sql.DictBase):
         return user_obj
 
 
-class LocalUser(sql.ModelBase, sql.DictBase):
+class LocalUser(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'local_user'
     attributes = ['id', 'user_id', 'domain_id', 'name']
     id = sql.Column(sql.Integer, primary_key=True)
@@ -279,7 +279,7 @@ class LocalUser(sql.ModelBase, sql.DictBase):
     )
 
 
-class Password(sql.ModelBase, sql.DictBase):
+class Password(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'password'
     attributes = ['id', 'local_user_id', 'password', 'created_at',
                   'expires_at']
@@ -330,7 +330,7 @@ class NonLocalUser(sql.ModelBase, sql.ModelDictMixin):
             onupdate='CASCADE', ondelete='CASCADE'),)
 
 
-class Group(sql.ModelBase, sql.DictBase):
+class Group(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'group'
     attributes = ['id', 'name', 'domain_id', 'description']
     id = sql.Column(sql.String(64), primary_key=True)
@@ -343,7 +343,7 @@ class Group(sql.ModelBase, sql.DictBase):
     __table_args__ = (sql.UniqueConstraint('domain_id', 'name'),)
 
 
-class UserGroupMembership(sql.ModelBase, sql.DictBase):
+class UserGroupMembership(sql.ModelBase, sql.ModelDictMixin):
     """Group membership join table."""
 
     __tablename__ = 'user_group_membership'
diff --git a/keystone/oauth1/backends/sql.py b/keystone/oauth1/backends/sql.py
index 6b05194be8..25be99c51d 100644
--- a/keystone/oauth1/backends/sql.py
+++ b/keystone/oauth1/backends/sql.py
@@ -29,7 +29,7 @@ from keystone.oauth1.backends import base
 random = _random.SystemRandom()
 
 
-class Consumer(sql.ModelBase, sql.DictBase):
+class Consumer(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'consumer'
     attributes = ['id', 'description', 'secret']
     id = sql.Column(sql.String(64), primary_key=True, nullable=False)
@@ -38,7 +38,7 @@ class Consumer(sql.ModelBase, sql.DictBase):
     extra = sql.Column(sql.JsonBlob(), nullable=False)
 
 
-class RequestToken(sql.ModelBase, sql.DictBase):
+class RequestToken(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'request_token'
     attributes = ['id', 'request_secret',
                   'verifier', 'authorizing_user_id', 'requested_project_id',
@@ -61,7 +61,7 @@ class RequestToken(sql.ModelBase, sql.DictBase):
         return dict(self.items())
 
 
-class AccessToken(sql.ModelBase, sql.DictBase):
+class AccessToken(sql.ModelBase, sql.ModelDictMixin):
     __tablename__ = 'access_token'
     attributes = ['id', 'access_secret', 'authorizing_user_id',
                   'project_id', 'role_ids', 'consumer_id',
diff --git a/keystone/policy/backends/sql.py b/keystone/policy/backends/sql.py
index 1ed9949c65..28fbec925b 100644
--- a/keystone/policy/backends/sql.py
+++ b/keystone/policy/backends/sql.py
@@ -17,7 +17,7 @@ from keystone import exception
 from keystone.policy.backends import rules
 
 
-class PolicyModel(sql.ModelBase, sql.DictBase):
+class PolicyModel(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'policy'
     attributes = ['id', 'blob', 'type']
     id = sql.Column(sql.String(64), primary_key=True)
diff --git a/keystone/resource/backends/sql.py b/keystone/resource/backends/sql.py
index 81f03dcbdd..c2e852dfe4 100644
--- a/keystone/resource/backends/sql.py
+++ b/keystone/resource/backends/sql.py
@@ -219,7 +219,7 @@ class Resource(base.ResourceDriverBase):
             query.delete(synchronize_session=False)
 
 
-class Project(sql.ModelBase, sql.DictBase):
+class Project(sql.ModelBase, sql.ModelDictMixinWithExtras):
     # NOTE(henry-nash): From the manager and above perspective, the domain_id
     # is nullable.  However, to ensure uniqueness in multi-process
     # configurations, it is better to still use the sql uniqueness constraint.
diff --git a/keystone/token/persistence/backends/sql.py b/keystone/token/persistence/backends/sql.py
index ca890093b9..df10fbcb2e 100644
--- a/keystone/token/persistence/backends/sql.py
+++ b/keystone/token/persistence/backends/sql.py
@@ -31,7 +31,7 @@ CONF = keystone.conf.CONF
 LOG = log.getLogger(__name__)
 
 
-class TokenModel(sql.ModelBase, sql.DictBase):
+class TokenModel(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'token'
     attributes = ['id', 'expires', 'user_id', 'trust_id']
     id = sql.Column(sql.String(64), primary_key=True)
diff --git a/keystone/trust/backends/sql.py b/keystone/trust/backends/sql.py
index 1356f4ec67..0a10e98438 100644
--- a/keystone/trust/backends/sql.py
+++ b/keystone/trust/backends/sql.py
@@ -25,7 +25,7 @@ from keystone.trust.backends import base
 MAXIMUM_CONSUME_ATTEMPTS = 10
 
 
-class TrustModel(sql.ModelBase, sql.DictBase):
+class TrustModel(sql.ModelBase, sql.ModelDictMixinWithExtras):
     __tablename__ = 'trust'
     attributes = ['id', 'trustor_user_id', 'trustee_user_id',
                   'project_id', 'impersonation', 'expires_at',