This patch removes unique index on the 'key' column of image_metadatum and replaces it with a compound UniqueConstraint on 'image_id' and 'key'. The 'key' column remains indexed.

Adds tests to ensure that two different images can use the same key, while preventing a single image from having two keys with the same name.

glance/common/db/sqlalchemy/session.py

@@ -29,14 +29,20 @@ FLAGS = flags.FLAGS
 _ENGINE = None
 _MAKER = None
 
+
+def get_engine(echo=False):
+    global _ENGINE
+    if not _ENGINE:
+        _ENGINE = create_engine(FLAGS.sql_connection, echo=echo)
+    return _ENGINE
+
+
 def get_session(autocommit=True, expire_on_commit=False):
     """Helper method to grab session"""
-    global _ENGINE
     global _MAKER
     if not _MAKER:
-        if not _ENGINE:
+        engine = get_engine()
-            _ENGINE = create_engine(FLAGS.sql_connection, echo=False)
+        _MAKER = sessionmaker(bind=engine,
-        _MAKER = sessionmaker(bind=_ENGINE,
                               autocommit=autocommit,
                               expire_on_commit=expire_on_commit)
     return _MAKER()

glance/parallax/db/sqlalchemy/models.py

@@ -26,9 +26,10 @@ import datetime
 from sqlalchemy.orm import relationship, backref, exc, object_mapper, validates
 from sqlalchemy import Column, Integer, String
 from sqlalchemy import ForeignKey, DateTime, Boolean, Text
+from sqlalchemy import UniqueConstraint
 from sqlalchemy.ext.declarative import declarative_base
 
-from glance.common.db.sqlalchemy.session import get_session
+from glance.common.db.sqlalchemy.session import get_session, get_engine
 
 from glance.common import exception
 from glance.common import flags
@@ -133,14 +134,15 @@ class Image(BASE, ModelBase):
     @validates('image_type')
     def validate_image_type(self, key, image_type):
         if not image_type in ('machine', 'kernel', 'ramdisk', 'raw'):
-            raise exception.Invalid("Invalid image type '%s' for image." % image_type)
+            raise exception.Invalid(
+                "Invalid image type '%s' for image." % image_type)
         return image_type
     
     @validates('status')
-    def validate_status(self, key, state):
+    def validate_status(self, key, status):
-        if not state in ('available', 'pending', 'disabled'):
+        if not status in ('available', 'pending', 'disabled'):
             raise exception.Invalid("Invalid status '%s' for image." % status)
-        return image_type
+        return status
     
     # TODO(sirp): should these be stored as metadata?
     #user_id = Column(String(255))
@@ -176,18 +178,24 @@ class ImageMetadatum(BASE, ModelBase):
     """Represents an image metadata in the datastore"""
     __tablename__ = 'image_metadata'
     __prefix__ = 'img-meta'
+    __table_args__ = (UniqueConstraint('image_id', 'key'), {})
+
     id = Column(Integer, primary_key=True)
     image_id = Column(Integer, ForeignKey('images.id'), nullable=False)
     image = relationship(Image, backref=backref('metadata'))
     
-    key = Column(String(255), index=True, unique=True)
+    key = Column(String(255), index=True)
     value = Column(Text)
 
 
 def register_models():
     """Register Models and create metadata"""
-    from sqlalchemy import create_engine
+    engine = get_engine()
-    models = (Image, ImageFile, ImageMetadatum)
+    BASE.metadata.create_all(engine)
-    engine = create_engine(FLAGS.sql_connection, echo=False)
+
-    for model in models:
+
-        model.metadata.create_all(engine)
+def unregister_models():
+    """Unregister Models, useful clearing out data before testing"""
+    engine = get_engine()
+    BASE.metadata.drop_all(engine)
+

tests/unit/test_parallax_models.py

@@ -0,0 +1,77 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 OpenStack, LLC
+# 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.
+
+import unittest
+import sqlalchemy.exceptions as sa_exc
+
+from glance.common import exception
+from glance.parallax import db
+from glance.common import flags
+from glance.parallax.db.sqlalchemy import models
+
+FLAGS = flags.FLAGS
+
+
+class TestModels(unittest.TestCase):
+    """ Test Parllax SQLAlchemy models using an in-memory sqlite DB"""
+
+    def setUp(self):
+        FLAGS.sql_connection = "sqlite://" # in-memory db
+        models.unregister_models()
+        models.register_models()
+        self.image = self._make_image(id=2, name='fake image #2')
+
+    def test_metadata_key_constraint_ok(self):
+        """Two different images are permitted to have metadata that share the
+        same key
+
+        """
+        self._make_metadatum(self.image, key="spam", value="eggs")
+        
+        second_image = self._make_image(id=3, name='fake image #3')
+        self._make_metadatum(second_image, key="spam", value="eggs")
+
+    def test_metadata_key_constraint_bad(self):
+        """The same image cannot have two distinct pieces of metadata with the
+        same key.
+
+        """
+        self._make_metadatum(self.image, key="spam", value="eggs")
+
+        self.assertRaises(sa_exc.IntegrityError,
+            self._make_metadatum, self.image, key="spam", value="eggs")
+
+    def _make_image(self, id, name):
+        """Convenience method to create an image with a given name and id"""
+        fixture = {'id': id,
+                   'name': name,
+                   'is_public': True,
+                   'image_type': 'kernel',
+                   'status': 'available'}
+        
+        context = None
+        image = db.api.image_create(context, fixture)
+        return image
+
+    def _make_metadatum(self, image, key, value):
+        """Convenience method to create metadata attached to an image"""
+        metadata = {'image_id': image['id'], 'key': key, 'value': value}
+        context = None
+        metadatum = db.api.image_metadatum_create(context, metadata)
+        return metadatum
+
+