Add a Trait Type model and db table

The event api will provide a way to list all trait types for
a given event type. The current data model does not provide
an efficient way to get all trait types.

This change introduces a trait_type table, and TraitType model.
Trait information can be retrieved with this simple query into
TraitType:

SELECT id, desc, dtype FROM  trait_type;

The UniqueName table is no longer needed, so it has been removed.

Partial-Bug: 1211015
Change-Id: I6ec29dc1d71b5590204d7e7b1bf121c2c1ca1904

ceilometer/storage/impl_sqlalchemy.py

@@ -818,33 +818,31 @@ class Connection(base.Connection):
             session.flush()
 
     @staticmethod
-    def _get_unique(session, key):
+    def _get_or_create_trait_type(trait_type, data_type, session=None):
-        return session.query(models.UniqueName)\
+        """Find if this trait already exists in the database, and
-            .filter(models.UniqueName.key == key).first()
+        if it does not, create a new entry in the trait type table.
-
-    def _get_or_create_unique_name(self, key, session=None):
-        """Find the UniqueName entry for a given key, creating
-           one if necessary.
-
-           This may result in a flush.
         """
         if session is None:
             session = sqlalchemy_session.get_session()
         with session.begin(subtransactions=True):
-            unique = self._get_unique(session, key)
+            tt = session.query(models.TraitType).filter(
-            if not unique:
+                models.TraitType.desc == trait_type,
-                unique = models.UniqueName(key=key)
+                models.TraitType.data_type == data_type).first()
-                session.add(unique)
+            if not tt:
+                tt = models.TraitType(trait_type, data_type)
+                session.add(tt)
                 session.flush()
-        return unique
+        return tt
 
-    def _make_trait(self, trait_model, event, session=None):
+    @classmethod
+    def _make_trait(cls, trait_model, event, session=None):
         """Make a new Trait from a Trait model.
 
         Doesn't flush or add to session.
         """
-        name = self._get_or_create_unique_name(trait_model.name,
+        trait_type = cls._get_or_create_trait_type(trait_model.name,
-                                               session=session)
+                                                   trait_model.dtype,
+                                                   session)
         value_map = models.Trait._value_map
         values = {'t_string': None, 't_float': None,
                   't_int': None, 't_datetime': None}
@@ -852,7 +850,7 @@ class Connection(base.Connection):
         if trait_model.dtype == api_models.Trait.DATETIME_TYPE:
             value = utils.dt_to_decimal(value)
         values[value_map[trait_model.dtype]] = value
-        return models.Trait(name, event, trait_model.dtype, **values)
+        return models.Trait(trait_type, event, **values)
 
     @staticmethod
     def _get_or_create_event_type(event_type, session=None):
@@ -872,12 +870,13 @@ class Connection(base.Connection):
                 session.flush()
         return et
 
-    def _record_event(self, session, event_model):
+    @classmethod
+    def _record_event(cls, session, event_model):
         """Store a single Event, including related Traits.
         """
         with session.begin(subtransactions=True):
-            event_type = self._get_or_create_event_type(event_model.event_type,
+            event_type = cls._get_or_create_event_type(event_model.event_type,
-                                                        session=session)
+                                                       session=session)
 
             generated = utils.dt_to_decimal(event_model.generated)
             event = models.Event(event_model.message_id, event_type, generated)
@@ -886,11 +885,11 @@ class Connection(base.Connection):
             new_traits = []
             if event_model.traits:
                 for trait in event_model.traits:
-                    t = self._make_trait(trait, event, session=session)
+                    t = cls._make_trait(trait, event, session=session)
                     session.add(t)
                     new_traits.append(t)
 
-        # Note: we don't flush here, explicitly (unless a new event_type
+        # Note: we don't flush here, explicitly (unless a new trait or event
         # does it). Otherwise, just wait until all the Events are staged.
         return (event, new_traits)
 
@@ -902,6 +901,9 @@ class Connection(base.Connection):
         Returns a list of events that could not be saved in a
         (reason, event) tuple. Reasons are enumerated in
         storage.model.Event
+
+        Flush when they're all added, unless new EventTypes or
+        TraitTypes are added along the way.
         """
         session = sqlalchemy_session.get_session()
         events = []
@@ -949,10 +951,13 @@ class Connection(base.Connection):
 
             event_models_dict = {}
             if event_filter.traits:
+                sub_query = sub_query.join(models.TraitType,
+                                           models.TraitType.id ==
+                                           models.Trait.trait_type_id)
                 for key, value in event_filter.traits.iteritems():
                     if key == 'key':
-                        key = self._get_unique(session, value)
+                        sub_query = sub_query.filter(models.TraitType.desc ==
-                        sub_query = sub_query.filter(models.Trait.name == key)
+                                                     value)
                     elif key == 't_string':
                         sub_query = sub_query.filter(
                             models.Trait.t_string == value)
@@ -1000,7 +1005,8 @@ class Connection(base.Connection):
                                              generated, [])
                     event_models_dict[trait.event_id] = event
                 value = trait.get_value()
-                trait_model = api_models.Trait(trait.name.key, trait.t_type,
+                trait_model = api_models.Trait(trait.trait_type.desc,
+                                               trait.trait_type.data_type,
                                                value)
                 event.append_trait(trait_model)
 

ceilometer/storage/models.py

@@ -83,12 +83,15 @@ class Trait(Model):
     record of basic data types (int, date, float, etc).
     """
 
+    NONE_TYPE = 0
     TEXT_TYPE = 1
     INT_TYPE = 2
     FLOAT_TYPE = 3
     DATETIME_TYPE = 4
 
     def __init__(self, name, dtype, value):
+        if not dtype:
+            dtype = Trait.NONE_TYPE
         Model.__init__(self, name=name, dtype=dtype, value=value)
 
     def __repr__(self):

ceilometer/storage/sqlalchemy/migrate_repo/versions/023_add_trait_types.py

@@ -0,0 +1,148 @@
+# -*- encoding: utf-8 -*-
+#
+# 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 ceilometer.storage.sqlalchemy import migration
+from migrate import ForeignKeyConstraint
+from sqlalchemy import Column
+from sqlalchemy import Integer
+from sqlalchemy import MetaData
+from sqlalchemy import select
+from sqlalchemy import String
+from sqlalchemy import Table
+from sqlalchemy import UniqueConstraint
+
+
+def upgrade(migrate_engine):
+    meta = MetaData(migrate_engine)
+    trait_type = Table(
+        'trait_type', meta,
+        Column('id', Integer, primary_key=True),
+        Column('desc', String(255)),
+        Column('data_type', Integer),
+        UniqueConstraint('desc', 'data_type', name="tt_unique")
+    )
+    trait = Table('trait', meta, autoload=True)
+    unique_name = Table('unique_name', meta, autoload=True)
+    trait_type.create(migrate_engine)
+    # Trait type extracts data from Trait and Unique name.
+    # We take all trait names from Unique Name, and data types
+    # from Trait. We then remove dtype and name from trait, and
+    # remove the name field.
+
+    conn = migrate_engine.connect()
+    sql = ("INSERT INTO trait_type "
+           "SELECT unique_name.id, unique_name.key, trait.t_type FROM trait "
+           "INNER JOIN unique_name "
+           "ON trait.name_id = unique_name.id "
+           "GROUP BY unique_name.id, unique_name.key, trait.t_type")
+    conn.execute(sql)
+    conn.close()
+
+    # Now we need to drop the foreign key constraint, rename
+    # the trait.name column, and re-add a new foreign
+    # key constraint
+    params = {'columns': [trait.c.name_id],
+              'refcolumns': [unique_name.c.id]}
+    if migrate_engine.name == 'mysql':
+        params['name'] = "trait_ibfk_1"  # foreign key to the unique name table
+    fkey = ForeignKeyConstraint(**params)
+    fkey.drop()
+
+    Column('trait_type_id', Integer).create(trait)
+
+    # Move data from name_id column into trait_type_id column
+    query = select([trait.c.id, trait.c.name_id])
+    for key, value in migration.paged(query):
+        trait.update().where(trait.c.id == key)\
+            .values({"trait_type_id": value}).execute()
+
+    trait.c.name_id.drop()
+
+    params = {'columns': [trait.c.trait_type_id],
+              'refcolumns': [trait_type.c.id]}
+    if migrate_engine.name == 'mysql':
+        params['name'] = "_".join(('fk', 'trait_type', 'id'))
+
+    fkey = ForeignKeyConstraint(**params)
+    fkey.create()
+
+    # Drop the t_type column to data_type.
+    trait.c.t_type.drop()
+
+    # Finally, drop the unique_name table - we don't need it
+    # anymore.
+    unique_name.drop()
+
+
+def downgrade(migrate_engine):
+    meta = MetaData(migrate_engine)
+    unique_name = Table(
+        'unique_name', meta,
+        Column('id', Integer, primary_key=True),
+        Column('key', String(255), unique=True)
+    )
+
+    trait_type = Table('trait_type', meta, autoload=True)
+    trait = Table('trait', meta, autoload=True)
+
+    # Create the UniqueName table, drop the foreign key constraint
+    # to trait_type, drop the trait_type table, rename the
+    # trait.trait_type column to traitname, re-add the dtype to
+    # the trait table, and re-add the old foreign key constraint
+
+    unique_name.create(migrate_engine)
+
+    conn = migrate_engine.connect()
+    sql = ("INSERT INTO unique_name "
+           "SELECT trait_type.id, trait_type.desc "
+           "FROM trait_type")
+
+    conn.execute(sql)
+    conn.close()
+    params = {'columns': [trait.c.trait_type_id],
+              'refcolumns': [trait_type.c.id]}
+
+    if migrate_engine.name == 'mysql':
+        params['name'] = "_".join(('fk', 'trait_type', 'id'))
+    fkey = ForeignKeyConstraint(**params)
+    fkey.drop()
+
+    # Re-create the old columns in trait
+    Column("name_id", Integer).create(trait)
+    Column("t_type", Integer).create(trait)
+
+    # copy data from trait_type.data_type into trait.t_type
+    query = select([trait_type.c.id, trait_type.c.data_type])
+    for key, value in migration.paged(query):
+        trait.update().where(trait.c.trait_type_id == key)\
+            .values({"t_type": value}).execute()
+
+    # Move data from name_id column into trait_type_id column
+    query = select([trait.c.id, trait.c.trait_type_id])
+    for key, value in migration.paged(query):
+        trait.update().where(trait.c.id == key)\
+            .values({"name_id": value}).execute()
+
+    # Add a foreign key to the unique_name table
+    params = {'columns': [trait.c.name_id],
+              'refcolumns': [unique_name.c.id]}
+    if migrate_engine.name == 'mysql':
+        params['name'] = 'trait_ibfk_1'
+    fkey = ForeignKeyConstraint(**params)
+    fkey.create()
+
+    trait.c.trait_type_id.drop()
+
+    # Drop the trait_type table. It isn't needed anymore
+    trait_type.drop()

ceilometer/storage/sqlalchemy/migrate_repo/versions/023_sqlite_downgrade.sql

@@ -0,0 +1,29 @@
+ALTER TABLE trait RENAME TO trait_orig;
+
+INSERT INTO unique_name
+SELECT id, 'desc'
+FROM trait_type;
+
+CREATE TABLE trait (
+  id INTEGER PRIMARY KEY ASC,
+  t_string VARCHAR(255),
+  t_int INTEGER,
+  t_float FLOAT,
+  t_datetime FLOAT,
+  t_type INTEGER NOT NULL,
+  name_id INTEGER NOT NULL,
+  event_id INTEGER NOT NULL,
+  FOREIGN KEY (name_id) REFERENCES unique_name (id)
+  FOREIGN KEY (event_id) REFERENCES event (id)
+);
+
+
+INSERT INTO trait
+SELECT t.id, t.t_string, t.t_int, t.t_float, t.t_datetime
+    tt.data_type, t.trait_type_id, t.event_id
+FROM trait_orig t
+INNER JOIN trait_type tt
+ON tt.id = t.trait_type_id
+
+DROP TABLE trait_orig;
+DROP TABLE trait_type;

ceilometer/storage/sqlalchemy/migrate_repo/versions/023_sqlite_upgrade.sql

@@ -0,0 +1,34 @@
+ALTER TABLE trait RENAME TO trait_orig;
+
+CREATE TABLE trait_type (
+  id INTEGER PRIMARY KEY ASC,
+  'desc' STRING NOT NULL,
+  data_type INTEGER NOT NULL,
+  UNIQUE ('desc', data_type)
+);
+
+INSERT INTO trait_type
+SELECT un.id, un.key, t.t_type
+FROM unique_name un
+JOIN trait_orig t ON un.id = t.name_id
+GROUP BY un.id;
+
+CREATE TABLE trait (
+  id INTEGER PRIMARY KEY ASC,
+  t_string VARCHAR(255),
+  t_int INTEGER,
+  t_float FLOAT,
+  t_datetime FLOAT,
+  trait_type_id INTEGER NOT NULL,
+  event_id INTEGER NOT NULL,
+  FOREIGN KEY (trait_type_id) REFERENCES trait_type (id)
+  FOREIGN KEY (event_id) REFERENCES event (id)
+);
+
+INSERT INTO trait
+SELECT t.id, t.t_string, t.t_int, t.t_float, t.t_datetime, t.name_id,
+    t.event_id
+FROM trait_orig t;
+
+DROP TABLE trait_orig;
+DROP TABLE unique_name;

ceilometer/storage/sqlalchemy/models.py

@@ -289,24 +289,6 @@ class AlarmChange(Base):
     timestamp = Column(DateTime, default=timeutils.utcnow)
 
 
-class UniqueName(Base):
-    """Key names should only be stored once.
-    """
-    __tablename__ = 'unique_name'
-    __table_args__ = (
-        Index('ix_unique_name_key', 'key'),
-    )
-
-    id = Column(Integer, primary_key=True)
-    key = Column(String(255))
-
-    def __init__(self, key):
-        self.key = key
-
-    def __repr__(self):
-        return "<UniqueName: %s>" % self.key
-
-
 class EventType(Base):
     """Types of event records."""
     __tablename__ = 'event_type'
@@ -347,21 +329,45 @@ class Event(Base):
                                                               self.generated)
 
 
+class TraitType(Base):
+    """Types of event traits. A trait type includes a description
+    and a data type. Uniqueness is enforced compositely on the
+    data_type and desc fields. This is to accommodate cases, such as
+    'generated', which, depending on the corresponding event,
+    could be a date, a boolean, or a float.
+
+    """
+    __tablename__ = 'trait_type'
+    __table_args__ = (
+        UniqueConstraint('desc', 'data_type', name='tt_unique'),
+        Index('ix_trait_type', 'desc')
+    )
+
+    id = Column(Integer, primary_key=True)
+    desc = Column(String(255))
+    data_type = Column(Integer)
+
+    def __init__(self, desc, data_type):
+        self.desc = desc
+        self.data_type = data_type
+
+    def __repr__(self):
+        return "<TraitType: %s:%d>" % (self.desc, self.data_type)
+
+
 class Trait(Base):
     __tablename__ = 'trait'
     __table_args__ = (
         Index('ix_trait_t_int', 't_int'),
         Index('ix_trait_t_string', 't_string'),
         Index('ix_trait_t_datetime', 't_datetime'),
-        Index('ix_trait_t_type', 't_type'),
         Index('ix_trait_t_float', 't_float'),
     )
     id = Column(Integer, primary_key=True)
 
-    name_id = Column(Integer, ForeignKey('unique_name.id'))
+    trait_type_id = Column(Integer, ForeignKey('trait_type.id'))
-    name = relationship("UniqueName", backref=backref('name', order_by=id))
+    trait_type = relationship("TraitType", backref=backref('trait_type'))
 
-    t_type = Column(Integer)
     t_string = Column(String(255), nullable=True, default=None)
     t_float = Column(Float, nullable=True, default=None)
     t_int = Column(Integer, nullable=True, default=None)
@@ -375,10 +381,9 @@ class Trait(Base):
                   api_models.Trait.INT_TYPE: 't_int',
                   api_models.Trait.DATETIME_TYPE: 't_datetime'}
 
-    def __init__(self, name, event, t_type, t_string=None, t_float=None,
+    def __init__(self, trait_type, event, t_string=None,
-                 t_int=None, t_datetime=None):
+                 t_float=None, t_int=None, t_datetime=None):
-        self.name = name
+        self.trait_type = trait_type
-        self.t_type = t_type
         self.t_string = t_string
         self.t_float = t_float
         self.t_int = t_int
@@ -386,17 +391,31 @@ class Trait(Base):
         self.event = event
 
     def get_value(self):
-        if self.t_type == api_models.Trait.INT_TYPE:
+        if self.trait_type is None:
+            dtype = None
+        else:
+            dtype = self.trait_type.data_type
+
+        if dtype == api_models.Trait.INT_TYPE:
             return self.t_int
-        if self.t_type == api_models.Trait.FLOAT_TYPE:
+        if dtype == api_models.Trait.FLOAT_TYPE:
             return self.t_float
-        if self.t_type == api_models.Trait.DATETIME_TYPE:
+        if dtype == api_models.Trait.DATETIME_TYPE:
             return utils.decimal_to_dt(self.t_datetime)
-        if self.t_type == api_models.Trait.TEXT_TYPE:
+        if dtype == api_models.Trait.TEXT_TYPE:
             return self.t_string
+
         return None
 
     def __repr__(self):
-        return "<Trait(%s) %d=%s/%s/%s/%s on %s>" % (self.name, self.t_type,
+        name = self.trait_type.name if self.trait_type else None
-               self.t_string, self.t_float, self.t_int, self.t_datetime,
+        data_type = self.trait_type.data_type if self.trait_type\
-               self.event)
+            else api_models.Trait.NONE_TYPE
+
+        return "<Trait(%s) %d=%s/%s/%s/%s on %s>" % (name,
+                                                     data_type,
+                                                     self.t_string,
+                                                     self.t_float,
+                                                     self.t_int,
+                                                     self.t_datetime,
+                                                     self.event)

ceilometer/tests/storage/test_impl_sqlalchemy.py

@@ -47,25 +47,36 @@ class CeilometerBaseTest(EventTestBase):
         self.assertEqual(base['key'], 'value')
 
 
-class UniqueNameTest(EventTestBase):
+class TraitTypeTest(EventTestBase):
-    # UniqueName is a construct specific to sqlalchemy.
+    # TraitType is a construct specific to sqlalchemy.
     # Not applicable to other drivers.
 
-    def test_unique_exists(self):
+    def test_trait_type_exists(self):
-        u1 = self.conn._get_or_create_unique_name("foo")
+        tt1 = self.conn._get_or_create_trait_type("foo", 0)
-        self.assertTrue(u1.id >= 0)
+        self.assertTrue(tt1.id >= 0)
-        u2 = self.conn._get_or_create_unique_name("foo")
+        tt2 = self.conn._get_or_create_trait_type("foo", 0)
-        self.assertEqual(u1.id, u2.id)
+        self.assertEqual(tt1.id, tt2.id)
-        self.assertEqual(u1.key, u2.key)
+        self.assertEqual(tt1.desc, tt2.desc)
+        self.assertEqual(tt1.data_type, tt2.data_type)
 
-    def test_new_unique(self):
+    def test_new_trait_type(self):
-        u1 = self.conn._get_or_create_unique_name("foo")
+        tt1 = self.conn._get_or_create_trait_type("foo", 0)
-        self.assertTrue(u1.id >= 0)
+        self.assertTrue(tt1.id >= 0)
-        u2 = self.conn._get_or_create_unique_name("blah")
+        tt2 = self.conn._get_or_create_trait_type("blah", 0)
-        self.assertNotEqual(u1.id, u2.id)
+        self.assertNotEqual(tt1.id, tt2.id)
-        self.assertNotEqual(u1.key, u2.key)
+        self.assertNotEqual(tt1.desc, tt2.desc)
         # Test the method __repr__ returns a string
-        self.assertTrue(repr.repr(u2))
+        self.assertTrue(repr.repr(tt2))
+
+    def test_trait_different_data_type(self):
+        tt1 = self.conn._get_or_create_trait_type("foo", 0)
+        self.assertTrue(tt1.id >= 0)
+        tt2 = self.conn._get_or_create_trait_type("foo", 1)
+        self.assertNotEqual(tt1.id, tt2.id)
+        self.assertEqual(tt1.desc, tt2.desc)
+        self.assertNotEqual(tt1.data_type, tt2.data_type)
+        # Test the method __repr__ returns a string
+        self.assertTrue(repr.repr(tt2))
 
 
 class EventTypeTest(EventTestBase):
@@ -97,43 +108,44 @@ class EventTest(EventTestBase):
     def test_string_traits(self):
         model = models.Trait("Foo", models.Trait.TEXT_TYPE, "my_text")
         trait = self.conn._make_trait(model, None)
-        self.assertEqual(trait.t_type, models.Trait.TEXT_TYPE)
+        self.assertEqual(trait.trait_type.data_type, models.Trait.TEXT_TYPE)
         self.assertIsNone(trait.t_float)
         self.assertIsNone(trait.t_int)
         self.assertIsNone(trait.t_datetime)
         self.assertEqual(trait.t_string, "my_text")
-        self.assertIsNotNone(trait.name)
+        self.assertIsNotNone(trait.trait_type.desc)
 
     def test_int_traits(self):
         model = models.Trait("Foo", models.Trait.INT_TYPE, 100)
         trait = self.conn._make_trait(model, None)
-        self.assertEqual(trait.t_type, models.Trait.INT_TYPE)
+        self.assertEqual(trait.trait_type.data_type, models.Trait.INT_TYPE)
         self.assertIsNone(trait.t_float)
         self.assertIsNone(trait.t_string)
         self.assertIsNone(trait.t_datetime)
         self.assertEqual(trait.t_int, 100)
-        self.assertIsNotNone(trait.name)
+        self.assertIsNotNone(trait.trait_type.desc)
 
     def test_float_traits(self):
         model = models.Trait("Foo", models.Trait.FLOAT_TYPE, 123.456)
         trait = self.conn._make_trait(model, None)
-        self.assertEqual(trait.t_type, models.Trait.FLOAT_TYPE)
+        self.assertEqual(trait.trait_type.data_type, models.Trait.FLOAT_TYPE)
         self.assertIsNone(trait.t_int)
         self.assertIsNone(trait.t_string)
         self.assertIsNone(trait.t_datetime)
         self.assertEqual(trait.t_float, 123.456)
-        self.assertIsNotNone(trait.name)
+        self.assertIsNotNone(trait.trait_type.desc)
 
     def test_datetime_traits(self):
         now = datetime.datetime.utcnow()
         model = models.Trait("Foo", models.Trait.DATETIME_TYPE, now)
         trait = self.conn._make_trait(model, None)
-        self.assertEqual(trait.t_type, models.Trait.DATETIME_TYPE)
+        self.assertEqual(trait.trait_type.data_type,
+                         models.Trait.DATETIME_TYPE)
         self.assertIsNone(trait.t_int)
         self.assertIsNone(trait.t_string)
         self.assertIsNone(trait.t_float)
         self.assertEqual(trait.t_datetime, utils.dt_to_decimal(now))
-        self.assertIsNotNone(trait.name)
+        self.assertIsNotNone(trait.trait_type.desc)
 
     def test_bad_event(self):
         now = datetime.datetime.utcnow()