Skip to content

Commit

Permalink
Bug 1127875 - add ability to mark releases as read only
Browse files Browse the repository at this point in the history
  • Loading branch information
nurav committed Mar 3, 2016
1 parent ce2b447 commit 672db22
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 6 deletions.
3 changes: 2 additions & 1 deletion auslib/admin/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
SpecificPermissionView
from auslib.admin.views.releases import SingleLocaleView, \
SingleReleaseView, ReleaseHistoryView, \
ReleasesAPIView
ReleasesAPIView, ReleaseReadOnlyView
from auslib.admin.views.rules import RulesAPIView, \
SingleRuleView, RuleHistoryAPIView
from auslib.admin.views.history import DiffView, FieldView
Expand Down Expand Up @@ -55,6 +55,7 @@ def add_security_headers(response):
app.add_url_rule("/rules/<int:rule_id>/revisions", view_func=RuleHistoryAPIView.as_view("rules_revisions"))
app.add_url_rule("/releases", view_func=ReleasesAPIView.as_view("releases"))
app.add_url_rule("/releases/<release>", view_func=SingleReleaseView.as_view("single_release"))
app.add_url_rule("/releases/<release>/read_only", view_func=ReleaseReadOnlyView.as_view("read_only"))
app.add_url_rule("/releases/<release>/builds/<platform>/<locale>", view_func=SingleLocaleView.as_view("single_locale"))
app.add_url_rule("/releases/<release>/revisions", view_func=ReleaseHistoryView.as_view("release_revisions"))
app.add_url_rule("/history/diff/<type_>/<change_id>/<field>", view_func=DiffView.as_view("diff"))
Expand Down
33 changes: 33 additions & 0 deletions auslib/admin/views/releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,39 @@ def _delete(self, release, changed_by, transaction):
return Response(status=200)


class ReleaseReadOnlyView(AdminView):
"""/releases/:release/read_only"""

def get(self, release):
releases = dbo.releases.getReleases(name=release, limit=1)
is_release_read_only = dbo.releases.isReadOnly(name=release, limit=1)
if not releases:
return Response(status=404,
response='Requested release does not exist')
release = releases[0]
return jsonify({
'read_only': is_release_read_only
})

@requirelogin
@requirepermission('/releases/:name/read_only')
def _put(self, release, changed_by, transaction):
req_read_only = request.form.get('read_only')
is_release_read_only = dbo.releases.isReadOnly(release)
old_data_version = request.form.get('data_version')

if req_read_only and not is_release_read_only:
dbo.releases.changeReadOnly(name=release, read_only=True, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction)
elif not req_read_only and is_release_read_only:
# Only an admin user can unset the read_only field once it's set to True
if dbo.permissions.hasUrlPermission(changed_by, 'admin'):
dbo.releases.changeReadOnly(name=release, read_only=False, changed_by=changed_by, transaction=transaction)
else:
msg = "%s is not allowed to set releases as read-write" % changed_by
cef_event("Unauthorized attempt to mark release as read-write", CEF_ALERT, msg=msg)
return Response(status=201, response='read_only changed')


class ReleaseHistoryView(HistoryAdminView):
"""/releases/:release/revisions"""

Expand Down
26 changes: 25 additions & 1 deletion auslib/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import time

from sqlalchemy import Table, Column, Integer, Text, String, MetaData, \
create_engine, select, BigInteger
create_engine, select, BigInteger, Boolean
from sqlalchemy.exc import SQLAlchemyError
from sqlalchemy.sql.expression import null

Expand Down Expand Up @@ -60,6 +60,10 @@ class WrongNumberOfRowsError(SQLAlchemyError):
"""Raised when an update or delete fails because the clause matches more than one row."""


class ReadOnlyError(SQLAlchemyError):
"""Raised when a release marked as read-only is attempted to be changed."""


class AUSTransaction(object):
"""Manages a single transaction. Requires a connection object.
Expand Down Expand Up @@ -863,6 +867,7 @@ def __init__(self, metadata, dialect):
self.table = Table('releases', metadata,
Column('name', String(100), primary_key=True),
Column('product', String(15), nullable=False),
Column('read_only', Boolean, default=False),
)
if dialect == 'mysql':
from sqlalchemy.dialects.mysql import LONGTEXT
Expand Down Expand Up @@ -1013,6 +1018,7 @@ def addRelease(self, name, product, blob, changed_by, transaction=None):
return ret.inserted_primary_key[0]

def updateRelease(self, name, changed_by, old_data_version, product=None, blob=None, transaction=None):
self._proceedIfNotReadOnly(name, transaction=transaction)
what = {}
if product:
what['product'] = product
Expand All @@ -1037,6 +1043,7 @@ def addLocaleToRelease(self, name, platform, locale, data, old_data_version, cha
validated before commiting it, and a ValueError is raised if it is
invalid.
"""
self._proceedIfNotReadOnly(name, transaction=transaction)
releaseBlob = self.getReleaseBlob(name, transaction=transaction)
if 'platforms' not in releaseBlob:
releaseBlob['platforms'] = {}
Expand Down Expand Up @@ -1087,11 +1094,27 @@ def localeExists(self, name, platform, locale, transaction=None):
return False

def deleteRelease(self, changed_by, name, old_data_version, transaction=None):
self._proceedIfNotReadOnly(name, transaction=transaction)
where = [self.name == name]
self.delete(changed_by=changed_by, where=where, old_data_version=old_data_version, transaction=transaction)
cache.invalidate("blob", name)
cache.invalidate("blob_version", name)

def isReadOnly(self, name, limit=None, transaction=None):
where = [self.name == name]
column = [self.read_only]
row = self.select(where=where, columns=column, limit=limit, transaction=transaction)[0]
return row['read_only']

def _proceedIfNotReadOnly(self, name, limit=None, transaction=None):
if self.isReadOnly(name, limit, transaction):
raise ReadOnlyError("Release '%s' is read only" % name)

def changeReadOnly(self, name, read_only, changed_by, old_data_version=None, transaction=None):
where = [self.name == name]
what = dict(read_only=read_only)
self.update(where=where, what=what, changed_by=changed_by, old_data_version=old_data_version, transaction=transaction)


class Permissions(AUSTable):
"""allPermissions defines the structure and possible options for all
Expand All @@ -1108,6 +1131,7 @@ class Permissions(AUSTable):
'/releases/:name': ['method', 'product'],
'/releases/:name/revisions': ['product'],
'/releases/:name/builds/:platform/:locale': ['method', 'product'],
'/releases/:name/read_only': ['product'],
'/rules': ['method', 'product'],
'/rules/:id': ['method', 'product'],
'/rules/:id/revisions': ['product'],
Expand Down
24 changes: 24 additions & 0 deletions auslib/migrate/versions/011_add_read_only_releases.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from sqlalchemy import Column, Boolean, MetaData, Table


def upgrade(migrate_engine):
metadata = MetaData(bind=migrate_engine)

releases = Table('releases', metadata, autoload=True)
releases_history = Table('releases_history', metadata, autoload=True)

read_only = Column('read_only', Boolean)
read_only.create(releases)

history_read_only = Column('read_only', Boolean)
history_read_only.create(releases_history)


def downgrade(migrate_engine):
metadata = MetaData(bind=migrate_engine)

releases = Table('releases', metadata, autoload=True)
releases_history = Table('releases', metadata, autoload=True)

releases.c.read_only.drop()
releases_history.c.read_only.drop()
19 changes: 19 additions & 0 deletions auslib/test/admin/views/test_releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -783,3 +783,22 @@ def testSettings(self):
ret = self._get('/releases/settings')
self.assertStatusCode(ret, 200)
self.assertEqual(json.loads(ret.data), json.loads(data['blob']))


class TestReadOnlyView(ViewTest, JSONTestMixin):

def testReadOnlyGet(self):
ret = self._get('/releases/b/read_only')
is_read_only = dbo.releases.t.select(dbo.releases.name == 'b').execute().first()['read_only']
self.assertEqual(json.loads(ret.data)['read_only'], is_read_only)

def testReadOnlySetTrue(self):
data = dict(read_only=True, product='Firefox', data_version=1)
self._put('/releases/b/read_only', data=data)
read_only = dbo.releases.isReadOnly(name='b')
self.assertEqual(read_only, True)

def testReadOnlySetFalseWithoutPermissionForProduct(self):
data = dict(read_only=False, product='Firefox', data_version=1)
ret = self._put('/releases/b/read_only', username='bob', data=data)
self.assertStatusCode(ret, 401)
41 changes: 37 additions & 4 deletions auslib/test/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from auslib.global_state import cache
from auslib.db import AUSDatabase, AUSTable, AlreadySetupError, \
AUSTransaction, TransactionError, OutdatedDataError
AUSTransaction, TransactionError, OutdatedDataError, ReadOnlyError
from auslib.blobs.base import BlobValidationError
from auslib.blobs.apprelease import ReleaseBlobV1

Expand Down Expand Up @@ -1022,6 +1022,10 @@ def testDeleteRelease(self):
release = self.releases.t.select().where(self.releases.name == 'a').execute().fetchall()
self.assertEquals(release, [])

def testDeleteReleaseWhenReadOnly(self):
self.releases.changeReadOnly('a', True, changed_by='me', old_data_version=1)
self.assertRaises(ReadOnlyError, self.releases.deleteRelease, changed_by='me', name='a', old_data_version=2)

def testAddReleaseWithNameMismatch(self):
blob = ReleaseBlobV1(name="f", schema_version=1, hashFunction="sha512")
self.assertRaises(ValueError, self.releases.addRelease, "g", "g", blob, "bill")
Expand All @@ -1030,6 +1034,18 @@ def testUpdateReleaseWithNameMismatch(self):
newBlob = ReleaseBlobV1(name="c", schema_version=1, hashFunction="sha512")
self.assertRaises(ValueError, self.releases.updateRelease, "a", "bill", 1, blob=newBlob)

def testChangeReadOnly(self):
self.releases.changeReadOnly('a', True, changed_by='me', old_data_version=1)
self.assertEqual(select([self.releases.read_only]).where(self.releases.name == 'a').execute().fetchone()[0], True)

def testIsReadOnly(self):
self.releases.changeReadOnly('a', True, changed_by='me', old_data_version=1)
self.assertEqual(self.releases.isReadOnly('a'), True)

def testProceedIfNotReadOnly(self):
self.releases.changeReadOnly('a', True, changed_by='me', old_data_version=1)
self.assertRaises(ReadOnlyError, self.releases._proceedIfNotReadOnly, 'a')


class TestBlobCaching(unittest.TestCase, MemoryDatabaseMixin):

Expand Down Expand Up @@ -1283,7 +1299,7 @@ def setUp(self):
def testAddRelease(self):
blob = ReleaseBlobV1(name="d", hashFunction="sha512")
self.releases.addRelease(name='d', product='d', blob=blob, changed_by='bill')
expected = [('d', 'd', json.dumps(dict(name="d", schema_version=1, hashFunction="sha512")), 1)]
expected = [('d', 'd', False, json.dumps(dict(name="d", schema_version=1, hashFunction="sha512")), 1)]
self.assertEquals(self.releases.t.select().where(self.releases.name == 'd').execute().fetchall(), expected)

def testAddReleaseAlreadyExists(self):
Expand All @@ -1293,13 +1309,19 @@ def testAddReleaseAlreadyExists(self):
def testUpdateRelease(self):
blob = ReleaseBlobV1(name='a', hashFunction="sha512")
self.releases.updateRelease(name='a', product='z', blob=blob, changed_by='bill', old_data_version=1)
expected = [('a', 'z', json.dumps(dict(name='a', schema_version=1, hashFunction="sha512")), 2)]
expected = [('a', 'z', False, json.dumps(dict(name='a', schema_version=1, hashFunction="sha512")), 2)]
self.assertEquals(self.releases.t.select().where(self.releases.name == 'a').execute().fetchall(), expected)

def testUpdateReleaseWhenReadOnly(self):
blob = ReleaseBlobV1(name='a', hashFunction="sha512")
# set release 'a' to read-only
self.releases.changeReadOnly('a', True, changed_by='me', old_data_version=1)
self.assertRaises(ReadOnlyError, self.releases.updateRelease, name='a', product='z', blob=blob, changed_by='me', old_data_version=2)

def testUpdateReleaseWithBlob(self):
blob = ReleaseBlobV1(name='b', schema_version=1, hashFunction="sha512")
self.releases.updateRelease(name='b', product='z', changed_by='bill', blob=blob, old_data_version=1)
expected = [('b', 'z', json.dumps(dict(name='b', schema_version=1, hashFunction="sha512")), 2)]
expected = [('b', 'z', False, json.dumps(dict(name='b', schema_version=1, hashFunction="sha512")), 2)]
self.assertEquals(self.releases.t.select().where(self.releases.name == 'b').execute().fetchall(), expected)

def testUpdateReleaseInvalidBlob(self):
Expand Down Expand Up @@ -1605,6 +1627,17 @@ def testAddLocaleToReleaseResolveAlias(self):
""")
self.assertEqual(ret, expected)

def testAddLocaleWhenReadOnly(self):
data = {
"complete": {
"filesize": 1,
"from": "*",
"hashValue": "abc",
}
}
self.releases.changeReadOnly('a', True, changed_by='me', old_data_version=1)
self.assertRaises(ReadOnlyError, self.releases.addLocaleToRelease, name='a', platform='p', locale='c', data=data, old_data_version=1, changed_by='bill')


class TestPermissions(unittest.TestCase, MemoryDatabaseMixin):

Expand Down

0 comments on commit 672db22

Please sign in to comment.