Merge pull request #51 from bhearsum/kill-version

bug 741412: Kill version in releases table. r=nthomas

auslib/admin/views/forms.py

@@ -96,7 +96,6 @@ class PartialReleaseForm(Form):
     # for its existence in this case.
     data_version = IntegerField('data_version', widget=HiddenInput())
     product = StringField('Product', validators=[Required()])
-    version = StringField('Version', validators=[Required()])
     hashFunction = StringField('Hash Function')
     data = JSONStringField('Data', validators=[Required()])
     schema_version = IntegerField('Schema Version')
@@ -146,7 +145,6 @@ class EditRuleForm(DbEditableForm):
 
 class CompleteReleaseForm(Form):
     name = StringField('Name', validators=[Required()])
-    version = StringField('Version', validators=[Required()])
     product = StringField('Product', validators=[Required()])
     blob = JSONStringField('Data', validators=[Required()], widget=FileInput())
     data_version = IntegerField('data_version', widget=HiddenInput())

auslib/admin/views/releases.py

@@ -17,9 +17,9 @@
 __all__ = ["SingleReleaseView", "SingleLocaleView"]
 
 
-def createRelease(release, product, version, changed_by, transaction, releaseData):
+def createRelease(release, product, changed_by, transaction, releaseData):
     blob = createBlob(json.dumps(releaseData))
-    dbo.releases.addRelease(name=release, product=product, version=version,
+    dbo.releases.addRelease(name=release, product=product,
                             blob=blob, changed_by=changed_by, transaction=transaction)
     return dbo.releases.getReleases(name=release, transaction=transaction)[0]
 
@@ -72,7 +72,6 @@ def changeRelease(release, changed_by, transaction, existsCallback, commitCallba
         cef_event("Bad input", CEF_WARN, errors=form.errors)
         return Response(status=400, response=json.dumps(form.errors))
     product = form.product.data
-    version = form.version.data
     incomingData = form.data.data
     copyTo = form.copyTo.data
     alias = form.alias.data
@@ -101,7 +100,7 @@ def changeRelease(release, changed_by, transaction, existsCallback, commitCallba
     for rel in allReleases:
         try:
             releaseInfo = dbo.releases.getReleases(name=rel, transaction=transaction)[0]
-            if existsCallback(rel, product, version):
+            if existsCallback(rel, product):
                 new = False
             # "release" is the one named in the URL (as opposed to the
             # ones that can be provided in copyTo), and we treat it as
@@ -133,7 +132,7 @@ def changeRelease(release, changed_by, transaction, existsCallback, commitCallba
             if hashFunction:
                 newReleaseData['hashFunction'] = hashFunction
             try:
-                releaseInfo = createRelease(rel, product, version, changed_by, transaction, newReleaseData)
+                releaseInfo = createRelease(rel, product, changed_by, transaction, newReleaseData)
             except BlobValidationError as e:
                 msg = "Couldn't create release: %s" % e
                 cef_event("Bad input", CEF_WARN, errors=msg, release=rel)
@@ -144,29 +143,11 @@ def changeRelease(release, changed_by, transaction, existsCallback, commitCallba
                 return Response(status=400, response=json.dumps({"data": e.args}))
             old_data_version = 1
 
-        # If the version doesn't match, just update it. This will be the case for nightlies
-        # every time there's a version bump.
-        if version != releaseInfo['version']:
-            log.debug("database version for %s is %s, updating it to %s", rel, releaseInfo['version'], version)
-            try:
-                dbo.releases.updateRelease(name=rel, version=version,
-                                           changed_by=changed_by, old_data_version=old_data_version,
-                                           transaction=transaction)
-            except BlobValidationError as e:
-                msg = "Couldn't update release: %s" % e
-                cef_event("Bad input", CEF_WARN, errors=msg, release=rel)
-                return Response(status=400, response=json.dumps({"data": e.errors}))
-            except ValueError as e:
-                msg = "Couldn't update release: %s" % e
-                cef_event("Bad input", CEF_WARN, errors=msg, release=rel)
-                return Response(status=400, response=json.dumps({"data": e.args}))
-            old_data_version += 1
-
         extraArgs = {}
         if alias:
             extraArgs['alias'] = alias
         try:
-            commitCallback(rel, product, version, incomingData, releaseInfo['data'], old_data_version, extraArgs)
+            commitCallback(rel, product, incomingData, releaseInfo['data'], old_data_version, extraArgs)
         except BlobValidationError as e:
             msg = "Couldn't update release: %s" % e
             cef_event("Bad input", CEF_WARN, errors=msg, release=rel)
@@ -207,13 +188,13 @@ def _put(self, release, platform, locale, changed_by, transaction):
            what to set the status code to, and what data_version applies to.
            In an ideal world we would probably require a data_version for the
            releases named in copyTo as well."""
-        def exists(rel, product, version):
+        def exists(rel, product):
             if rel == release:
                 return dbo.releases.localeExists(name=rel, platform=platform,
                                                  locale=locale, transaction=transaction)
             return False
 
-        def commit(rel, product, version, localeData, releaseData, old_data_version, extraArgs):
+        def commit(rel, product, localeData, releaseData, old_data_version, extraArgs):
             return dbo.releases.addLocaleToRelease(name=rel, platform=platform,
                                                    locale=locale, data=localeData, alias=extraArgs.get('alias'),
                                                    old_data_version=old_data_version,
@@ -249,7 +230,7 @@ def _put(self, release, changed_by, transaction):
         if dbo.releases.getReleases(name=release, limit=1):
             data_version = form.data_version.data
             try:
-                dbo.releases.updateRelease(name=release, blob=blob, version=form.version.data,
+                dbo.releases.updateRelease(name=release, blob=blob,
                                            product=form.product.data, changed_by=changed_by,
                                            old_data_version=data_version, transaction=transaction)
             except BlobValidationError as e:
@@ -265,7 +246,7 @@ def _put(self, release, changed_by, transaction):
         else:
             try:
                 dbo.releases.addRelease(name=release, product=form.product.data,
-                                        version=form.version.data, blob=blob,
+                                        blob=blob,
                                         changed_by=changed_by, transaction=transaction)
             except BlobValidationError as e:
                 msg = "Couldn't update release: %s" % e
@@ -280,12 +261,12 @@ def _put(self, release, changed_by, transaction):
     @requirelogin
     @requirepermission('/releases/:name')
     def _post(self, release, changed_by, transaction):
-        def exists(rel, product, version):
+        def exists(rel, product):
             if rel == release:
                 return True
             return False
 
-        def commit(rel, product, version, newReleaseData, releaseData, old_data_version, extraArgs):
+        def commit(rel, product, newReleaseData, releaseData, old_data_version, extraArgs):
             releaseData.update(newReleaseData)
             blob = createBlob(releaseData)
             return dbo.releases.updateRelease(name=rel, blob=blob,
@@ -392,7 +373,7 @@ def _post(self, release, transaction, changed_by):
 
         try:
             dbo.releases.updateRelease(changed_by=changed_by, name=change['name'],
-                                       version=change['version'], blob=blob,
+                                       blob=blob,
                                        old_data_version=old_data_version, transaction=transaction)
         except BlobValidationError as e:
             cef_event("Bad input", CEF_WARN, errors=e.args)
@@ -411,8 +392,6 @@ def get(self, **kwargs):
         kwargs = {}
         if request.args.get('product'):
             kwargs['product'] = request.args.get('product')
-        if request.args.get('version'):
-            kwargs['version'] = request.args.get('version')
         if request.args.get('name_prefix'):
             kwargs['name_prefix'] = request.args.get('name_prefix')
         if request.args.get('names_only'):
@@ -429,7 +408,6 @@ def get(self, **kwargs):
                 # return : db name
                 'name': 'name',
                 'product': 'product',
-                'version': 'version',
                 'data_version': 'data_version',
             }
             for release in releases:
@@ -455,7 +433,7 @@ def _post(self, changed_by, transaction):
             blob = createBlob(form.blob.data)
             name = dbo.releases.addRelease(
                 name=form.name.data, product=form.product.data,
-                version=form.version.data, blob=blob,
+                blob=blob,
                 changed_by=changed_by, transaction=transaction
             )
         except BlobValidationError as e:

auslib/db.py

@@ -863,7 +863,6 @@ def __init__(self, metadata, dialect):
         self.table = Table('releases', metadata,
                            Column('name', String(100), primary_key=True),
                            Column('product', String(15), nullable=False),
-                           Column('version', String(25), nullable=False),
                            )
         if dialect == 'mysql':
             from sqlalchemy.dialects.mysql import LONGTEXT
@@ -909,22 +908,19 @@ def containsForbiddenDomain(self, data):
 
         return False
 
-    def getReleases(self, name=None, product=None, version=None, limit=None, transaction=None):
+    def getReleases(self, name=None, product=None, limit=None, transaction=None):
         self.log.debug("Looking for releases with:")
         self.log.debug("name: %s", name)
         self.log.debug("product: %s", product)
-        self.log.debug("version: %s", version)
         where = []
         if name:
             where.append(self.name == name)
         if product:
             where.append(self.product == product)
-        if version:
-            where.append(self.version == version)
         # We could get the "data" column here too, but getReleaseBlob knows how
         # to grab cached versions of that, so it's better to let it take care
         # of it.
-        rows = self.select(columns=[self.name, self.product, self.version, self.data_version],
+        rows = self.select(columns=[self.name, self.product, self.data_version],
                            where=where, limit=limit, transaction=transaction)
         for row in rows:
             row["data"] = self.getReleaseBlob(row["name"], transaction)
@@ -935,19 +931,17 @@ def countReleases(self, transaction=None):
         count, = self.t.count().execute().fetchone()
         return count
 
-    def getReleaseInfo(self, product=None, version=None, limit=None,
+    def getReleaseInfo(self, product=None, limit=None,
                        transaction=None, nameOnly=False, name_prefix=None):
         where = []
         if product:
             where.append(self.product == product)
-        if version:
-            where.append(self.version == version)
         if name_prefix:
             where.append(self.name.startswith(name_prefix))
         if nameOnly:
             column = [self.name]
         else:
-            column = [self.name, self.product, self.version, self.data_version]
+            column = [self.name, self.product, self.data_version]
         rows = self.select(where=where, columns=column, limit=limit, transaction=transaction)
         return rows
 
@@ -1001,7 +995,7 @@ def getBlob():
 
         return blob
 
-    def addRelease(self, name, product, version, blob, changed_by, transaction=None):
+    def addRelease(self, name, product, blob, changed_by, transaction=None):
         blob.validate()
         # Generally blobs have names, but there's no requirement that they have to.
         if blob.get("name"):
@@ -1011,19 +1005,17 @@ def addRelease(self, name, product, version, blob, changed_by, transaction=None)
         if self.containsForbiddenDomain(blob):
             raise ValueError("Release blob contains forbidden domain.")
 
-        columns = dict(name=name, product=product, version=version, data=blob.getJSON())
+        columns = dict(name=name, product=product, data=blob.getJSON())
         # Raises DuplicateDataError if the release already exists.
         ret = self.insert(changed_by=changed_by, transaction=transaction, **columns)
         cache.put("blob", name, {"data_version": 1, "blob": blob})
         cache.put("blob_version", name, 1)
         return ret.inserted_primary_key[0]
 
-    def updateRelease(self, name, changed_by, old_data_version, product=None, version=None, blob=None, transaction=None):
+    def updateRelease(self, name, changed_by, old_data_version, product=None, blob=None, transaction=None):
         what = {}
         if product:
             what['product'] = product
-        if version:
-            what['version'] = version
         if blob:
             blob.validate()
             # Generally blobs have names, but there's no requirement that they have to.

auslib/migrate/versions/010_delete_releases_version.py

@@ -0,0 +1,17 @@
+from sqlalchemy import Column, String, MetaData, Table
+
+
+def upgrade(migrate_engine):
+    metadata = MetaData(bind=migrate_engine)
+    Table('releases', metadata, autoload=True).c.version.drop()
+    Table('releases_history', metadata, autoload=True).c.version.drop()
+
+
+def downgrade(migrate_engine):
+    metadata = MetaData(bind=migrate_engine)
+
+    version = Column("version", String(25), unique=True)
+    version.create(Table("releases", metadata, autoload=True), unique_name="version_unique")
+
+    history_version = Column("version", String(25))
+    history_version.create(Table('releases_history', metadata, autoload=True))

auslib/test/admin/views/base.py

@@ -29,14 +29,14 @@ def setUp(self):
         dbo.permissions.t.insert().execute(permission='/releases/:name', username='bob', options=json.dumps(dict(product=['fake'])), data_version=1)
         dbo.permissions.t.insert().execute(permission='/rules/:id', username='bob', options=json.dumps(dict(product=['fake'])), data_version=1)
         dbo.releases.t.insert().execute(
-            name='a', product='a', version='a', data=json.dumps(dict(name='a', hashFunction="sha512", schema_version=1)), data_version=1)
+            name='a', product='a', data=json.dumps(dict(name='a', hashFunction="sha512", schema_version=1)), data_version=1)
         dbo.releases.t.insert().execute(
-            name='ab', product='a', version='a', data=json.dumps(dict(name='ab', hashFunction="sha512", schema_version=1)), data_version=1)
+            name='ab', product='a', data=json.dumps(dict(name='ab', hashFunction="sha512", schema_version=1)), data_version=1)
         dbo.releases.t.insert().execute(
-            name='b', product='b', version='b', data=json.dumps(dict(name='b', hashFunction="sha512", schema_version=1)), data_version=1)
+            name='b', product='b', data=json.dumps(dict(name='b', hashFunction="sha512", schema_version=1)), data_version=1)
         dbo.releases.t.insert().execute(
-            name='c', product='c', version='c', data=json.dumps(dict(name='c', hashFunction="sha512", schema_version=1)), data_version=1)
-        dbo.releases.t.insert().execute(name='d', product='d', version='d', data_version=1, data="""
+            name='c', product='c', data=json.dumps(dict(name='c', hashFunction="sha512", schema_version=1)), data_version=1)
+        dbo.releases.t.insert().execute(name='d', product='d', data_version=1, data="""
 {
     "name": "d",
     "schema_version": 1,

auslib/test/admin/views/test_history.py

@@ -36,7 +36,7 @@ def testFieldViewRelease(self):
         data = json.dumps(dict(detailsUrl='blah', fakePartials=True, schema_version=1, name="d", hashFunction="sha512"))
         ret = self._post(
             '/releases/d',
-            data=dict(data=data, product='d', version='d', data_version=1)
+            data=dict(data=data, product='d', data_version=1)
         )
         self.assertStatusCode(ret, 200)
 
@@ -77,7 +77,7 @@ def testFieldViewRelease(self):
         data = json.dumps(dict(detailsUrl='blah', fakePartials=False, schema_version=1, name="d", hashFunction="sha512"))
         ret = self._post(
             '/releases/d',
-            data=dict(data=data, product='d', version='d', data_version=2)
+            data=dict(data=data, product='d', data_version=2)
         )
         self.assertStatusCode(ret, 200)