[copr] master: Start moving authorization from views to logic (8fda311)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 8fda311b272d73e41ecccb537ceee039ecf2ec67
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 28 12:09:38 2013 +0100
Start moving authorization from views to logic
- move copr updating logic
- improve the ActionInProgress exception so that we can print it as any other exception, while still making the Action accessible if needed
>---------------------------------------------------------------
coprs_frontend/coprs/exceptions.py | 9 +++++++++
coprs_frontend/coprs/logic/builds_logic.py | 3 ++-
coprs_frontend/coprs/logic/coprs_logic.py | 20 ++++++++++++++++----
.../coprs/views/coprs_ns/coprs_builds.py | 2 +-
.../coprs/views/coprs_ns/coprs_general.py | 10 +++-------
5 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/coprs_frontend/coprs/exceptions.py b/coprs_frontend/coprs/exceptions.py
index da744b2..f1954b2 100644
--- a/coprs_frontend/coprs/exceptions.py
+++ b/coprs_frontend/coprs/exceptions.py
@@ -17,3 +17,12 @@ class ActionInProgressException(BaseException):
def __init__(self, msg, action):
self.msg = msg
self.action = action
+
+ def __unicode__(self):
+ return self.formatted_msg()
+
+ def __str__(self):
+ return self.__unicode__()
+
+ def formatted_msg(self):
+ return self.msg.format(action=self.action)
diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py
index 247c01a..2a91f95 100644
--- a/coprs_frontend/coprs/logic/builds_logic.py
+++ b/coprs_frontend/coprs/logic/builds_logic.py
@@ -57,7 +57,8 @@ class BuildsLogic(object):
@classmethod
def add(cls, user, pkgs, copr):
- coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr)
+ coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr,
+ 'Can\'t build while there is an operation in progress: {action}')
build = models.Build(
pkgs=pkgs,
copr=copr,
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 1a83147..2b4b2d9 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -92,7 +92,9 @@ class CoprsLogic(object):
@classmethod
def update(cls, user, copr, check_for_duplicates = True):
- cls.raise_if_unfinished_action(user, copr)
+ cls.raise_if_unfinished_action(user, copr,
+ 'Can\'t change this Copr name, another operation is in progress: {action}')
+ cls.raise_if_cant_update(user, copr, 'Only owners and admins may update their Coprs.')
existing = cls.exists_for_user(copr.owner, copr.name).first()
if existing:
@@ -115,7 +117,8 @@ class CoprsLogic(object):
def delete(cls, user, copr, check_for_duplicates=True):
# for the time being, we authorize user to do this in view...
# TODO: do we want to dump the information somewhere, so that we can search it in future?
- cls.raise_if_unfinished_action(user, copr)
+ cls.raise_if_unfinished_action(user, copr,
+ 'Can\'t delete this Copr, another operation is in progress: {action}')
action = models.Action(action_type=helpers.ActionTypeEnum('delete'),
object_type='copr',
object_id=copr.id,
@@ -149,13 +152,22 @@ class CoprsLogic(object):
return actions
@classmethod
- def raise_if_unfinished_action(cls, user, copr):
+ def raise_if_unfinished_action(cls, user, copr, message):
"""This method raises ActionInProgressException if given copr has an unfinished
action. Returns None otherwise.
"""
unfinished_actions = cls.unfinished_actions_for(user, copr).all()
if unfinished_actions:
- raise exceptions.ActionInProgressException('Action in progress on this copr.', unfinished_actions[0])
+ raise exceptions.ActionInProgressException(message, unfinished_actions[0])
+
+ @classmethod
+ def raise_if_cant_update(cls, user, copr, message):
+ """This method raises InsufficientRightsException if given user cant update
+ given copr. Returns None otherwise.
+ """
+ # TODO: this is a bit inconsistent - shouldn't the user method be called can_update?
+ if not user.can_edit(copr):
+ raise exceptions.InsufficientRightsException(message)
class CoprPermissionsLogic(object):
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
index 1f91e49..a6871cd 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
@@ -59,7 +59,7 @@ def copr_new_build(username, coprname):
build.memory_reqs = form.memory_reqs.data
build.timeout = form.timeout.data
except exceptions.ActionInProgressException as e:
- flask.flash('Can\'t build in this Copr, there is an operation in progress: {0}'.format(e.action))
+ flask.flash(e)
db.session.rollback()
else:
flask.flash("Build was added")
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index 6da142e..adba1d0 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -164,10 +164,6 @@ def copr_edit(username, coprname, form=None):
def copr_update(username, coprname):
form = forms.CoprFormFactory.create_form_cls()()
copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
- # only owner can update a copr
- if not flask.g.user.can_edit(copr):
- flask.flash('Only owners and admins may update their Coprs.')
- return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = form.name.data))
if form.validate_on_submit():
# we don't change owner (yet)
@@ -179,8 +175,8 @@ def copr_update(username, coprname):
try:
coprs_logic.CoprsLogic.update(flask.g.user, copr, check_for_duplicates = False) # form validation checks for duplicates
- except exceptions.ActionInProgressException as e:
- flask.flash('Can\'t change this Copr name, there is another operation in progress: {0}'.format(e.action))
+ except (exceptions.ActionInProgressException, exceptions.InsufficientRightsException) as e:
+ flask.flash(e)
db.session.rollback()
else:
flask.flash('Copr was updated successfully.')
@@ -252,7 +248,7 @@ def copr_delete(username, coprname):
coprs_logic.CoprsLogic.delete(flask.g.user, copr)
except exceptions.ActionInProgressException as e:
db.session.rollback()
- flask.flash('Can\'t manipulate this Copr, there is another operation in progress: {0}'.format(e.action))
+ flask.flash(e)
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
else:
db.session.commit()
11 years
[copr] master: Allow deleting coprs (010246d)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 010246d4e7351c9113cf1c3b4cee7aa20ae26af5
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Wed Mar 27 14:46:33 2013 +0100
Allow deleting coprs
>---------------------------------------------------------------
coprs_frontend/coprs/forms.py | 3 +
coprs_frontend/coprs/logic/coprs_logic.py | 17 ++++++++
.../coprs/templates/coprs/_coprs_forms.html | 17 ++++++++
coprs_frontend/coprs/templates/coprs/detail.html | 5 ++
.../coprs/templates/coprs/detail/delete.html | 14 +++++++
.../coprs/views/coprs_ns/coprs_general.py | 24 +++++++++++
coprs_frontend/coprs/whoosheers.py | 4 ++
.../tests/test_views/test_coprs_ns/test_general.py | 41 ++++++++++++++++++++
8 files changed, 125 insertions(+), 0 deletions(-)
diff --git a/coprs_frontend/coprs/forms.py b/coprs_frontend/coprs/forms.py
index 5c80c33..7c7cf35 100644
--- a/coprs_frontend/coprs/forms.py
+++ b/coprs_frontend/coprs/forms.py
@@ -123,6 +123,9 @@ class CoprFormFactory(object):
return F
+class CoprDeleteForm(wtf.Form):
+ verify = wtf.TextField('Confirm deleting by typing "yes"',
+ validators=[wtf.Required(), wtf.Regexp(r'^yes$', message='Type "yes" - without the quotes, lowercase.')])
class BuildForm(wtf.Form):
pkgs = wtf.TextAreaField('Pkgs',
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 29aa6b0..1a83147 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -112,6 +112,23 @@ class CoprsLogic(object):
db.session.add(copr)
@classmethod
+ def delete(cls, user, copr, check_for_duplicates=True):
+ # for the time being, we authorize user to do this in view...
+ # TODO: do we want to dump the information somewhere, so that we can search it in future?
+ cls.raise_if_unfinished_action(user, copr)
+ action = models.Action(action_type=helpers.ActionTypeEnum('delete'),
+ object_type='copr',
+ object_id=copr.id,
+ old_value='{0}/{1}'.format(copr.owner.name, copr.name),
+ new_value='',
+ created_on=int(time.time()))
+
+ db.session.add(action)
+ db.session.delete(copr)
+
+ return copr
+
+ @classmethod
def exists_for_user(cls, user, coprname):
existing = models.Copr.query.filter(models.Copr.name==coprname).\
filter(models.Copr.owner_id==user.id)
diff --git a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
index b3f6742..a0f55e3 100644
--- a/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
+++ b/coprs_frontend/coprs/templates/coprs/_coprs_forms.html
@@ -31,6 +31,23 @@
</form>
{% endmacro %}
+{% macro copr_delete_form(form, copr) %}
+ <form action="{{ url_for('coprs_ns.copr_delete', username=copr.owner.name, coprname=copr.name) }}" method="post">
+ <dl>
+ {{ form.csrf_token }}
+ <dd>
+ {% if form.verify.errors %}
+ {% for error in form.verify.errors %}
+ <p class="form-error">{{ error }}</p>
+ {% endfor %}
+ {% endif %}
+ </dd>
+ {{ form.verify }}
+ <dt><input type="submit" value="Delete"></dt>
+ </dl>
+ </form>
+{% endmacro %}
+
{% macro copr_permissions_form(form, copr, permissions) %}
{% if permissions %}
<form action="{{ url_for('coprs_ns.copr_update_permissions', username=copr.owner.name, coprname=copr.name) }}" method=post>
diff --git a/coprs_frontend/coprs/templates/coprs/detail.html b/coprs_frontend/coprs/templates/coprs/detail.html
index ef0f097..749ffb6 100644
--- a/coprs_frontend/coprs/templates/coprs/detail.html
+++ b/coprs_frontend/coprs/templates/coprs/detail.html
@@ -27,6 +27,11 @@
<a href="{{ url_for('coprs_ns.copr_edit', username = copr.owner.name, coprname = copr.name) }}">Edit</a>
</li>
{% endif %}
+ {% if g.user and g.user == copr.owner %}
+ <li class="{% block delete_selected %}unselected{% endblock %}">
+ <a href="{{ url_for('coprs_ns.copr_delete', username=copr.owner.name, coprname=copr.name) }}">Delete</a>
+ </li>
+ {% endif %}
</ul>
</div>
{% block detail_body %}{% endblock %}
diff --git a/coprs_frontend/coprs/templates/coprs/detail/delete.html b/coprs_frontend/coprs/templates/coprs/detail/delete.html
new file mode 100644
index 0000000..0e65de0
--- /dev/null
+++ b/coprs_frontend/coprs/templates/coprs/detail/delete.html
@@ -0,0 +1,14 @@
+{% extends "coprs/detail.html" %}
+{% block title %}Delete {{ copr.owner.name }}/{{ copr.name }}?{% endblock %}
+{% block delete_selected %}selected{% endblock %}
+{% from "coprs/_coprs_forms.html" import copr_delete_form %}
+
+{% block detail_body %}
+ <p>If you really want to delete this Copr, you'll have to answer this riddle:</p>
+ <p>{{ range(5)|random }}.{{ range(10)|random }} hens lay {{ range(5)|random }}.{{ range(10)|random }} eggs in
+ {{ range(5)|random }}.{{ range(10)|random }} days. How many eggs do {{ range(5)|random }}.{{ range(10)|random }}
+ hens lay in {{ range(5)|random }}.{{ range(10)|random }} days?</p>
+ <p>Ok, kidding, just type "<strong>yes</strong>" into the below box.</p>
+ {{ copr_delete_form(form, copr) }}
+
+{% endblock %}
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index fc1a0ad..6da142e 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -236,3 +236,27 @@ def copr_update_permissions(username, coprname):
flask.flash('Copr permissions were updated successfully.')
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
+
+(a)coprs_ns.route('/detail/<username>/<coprname>/delete/', methods=['GET', 'POST'])
+@login_required
+def copr_delete(username, coprname):
+ form = forms.CoprDeleteForm()
+ copr = coprs_logic.CoprsLogic.get(flask.g.user, username, coprname).first()
+ # only owner can delete a copr
+ if flask.g.user != copr.owner:
+ flask.flash('Only owners may delete their Coprs.')
+ return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
+
+ if form.validate_on_submit():
+ try:
+ coprs_logic.CoprsLogic.delete(flask.g.user, copr)
+ except exceptions.ActionInProgressException as e:
+ db.session.rollback()
+ flask.flash('Can\'t manipulate this Copr, there is another operation in progress: {0}'.format(e.action))
+ return flask.redirect(flask.url_for('coprs_ns.copr_detail', username=username, coprname=coprname))
+ else:
+ db.session.commit()
+ flask.flash('Copr was deleted successfully.')
+ return flask.redirect(flask.url_for('coprs_ns.coprs_by_owner', username=username))
+ else:
+ return flask.render_template('coprs/detail/delete.html', form=form, copr=copr)
diff --git a/coprs_frontend/coprs/whoosheers.py b/coprs_frontend/coprs/whoosheers.py
index 45b5ab6..cefae0f 100644
--- a/coprs_frontend/coprs/whoosheers.py
+++ b/coprs_frontend/coprs/whoosheers.py
@@ -53,3 +53,7 @@ class CoprUserWhoosheer(AbstractWhoosheer):
coprname=copr.name,
description=copr.description,
instructions=copr.instructions)
+
+ @classmethod
+ def delete_copr(cls, writer, copr):
+ writer.delete_by_term('copr_id', copr.id)
diff --git a/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py b/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py
index 691276f..63f3a06 100644
--- a/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py
+++ b/coprs_frontend/tests/test_views/test_coprs_ns/test_general.py
@@ -370,3 +370,44 @@ class TestCoprUpdatePermissions(CoprsTestCase):
data = {'copr_builder_1': '2', 'copr_admin_3': '2'},
follow_redirects = True)
assert 'Copr permissions were updated' in r.data
+
+class TestCoprDelete(CoprsTestCase):
+ def test_delete(self, f_users, f_coprs, f_db):
+ with self.tc as c:
+ with c.session_transaction() as s:
+ s['openid'] = self.u1.openid_name
+
+ self.db.session.add_all([self.u1, self.c1])
+ r = c.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name),
+ data = {'verify': 'yes'},
+ follow_redirects = True)
+ assert 'Copr was deleted successfully' in r.data
+ assert self.models.Action.query.first().id == self.c1.id
+ assert not self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first()
+
+ def test_copr_delete_does_not_delete_if_verify_filled_wrongly(self, f_users, f_coprs, f_db):
+ with self.tc as c:
+ with c.session_transaction() as s:
+ s['openid'] = self.u1.openid_name
+
+ self.db.session.add_all([self.u1, self.c1])
+ r = c.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name),
+ data = {'verify': 'no'},
+ follow_redirects = True)
+ assert 'Copr was deleted successfully' not in r.data
+ assert not self.models.Action.query.first()
+ assert self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first()
+
+ def test_non_owner_cant_delete(self, f_users, f_coprs, f_db):
+ with self.tc as c:
+ with c.session_transaction() as s:
+ s['openid'] = self.u2.openid_name
+
+ self.db.session.add_all([self.u1, self.u2, self.c1])
+ r = c.post('/coprs/detail/{0}/{1}/delete/'.format(self.u1.name, self.c1.name),
+ data = {'verify': 'yes'},
+ follow_redirects = True)
+ self.db.session.add_all([self.c1])
+ assert 'Copr was deleted successfully' not in r.data
+ assert not self.models.Action.query.first()
+ assert self.models.Copr.query.filter(self.models.Copr.id==self.c1.id).first()
11 years
[copr] master: Only allow creating builds when the copr is not being renamed or otherwise manipulated (f4fbc29)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit f4fbc295001e114f2af0d8d13bd6f8b9590b3be6
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Wed Mar 27 10:40:16 2013 +0100
Only allow creating builds when the copr is not being renamed or otherwise manipulated
>---------------------------------------------------------------
coprs_frontend/coprs/logic/builds_logic.py | 1 +
.../coprs/views/coprs_ns/coprs_builds.py | 22 +++++++++++--------
.../tests/test_logic/test_builds_logic.py | 9 ++++++++
3 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py
index 191e95b..247c01a 100644
--- a/coprs_frontend/coprs/logic/builds_logic.py
+++ b/coprs_frontend/coprs/logic/builds_logic.py
@@ -57,6 +57,7 @@ class BuildsLogic(object):
@classmethod
def add(cls, user, pkgs, copr):
+ coprs_logic.CoprsLogic.raise_if_unfinished_action(user, copr)
build = models.Build(
pkgs=pkgs,
copr=copr,
diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
index eae3c1d..1f91e49 100644
--- a/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
+++ b/coprs_frontend/coprs/views/coprs_ns/coprs_builds.py
@@ -51,16 +51,20 @@ def copr_new_build(username, coprname):
return page_not_found('Copr with name {0} does not exist.'.format(coprname))
if form.validate_on_submit() and flask.g.user.can_build_in(copr):
- build = builds_logic.BuildsLogic.add(user=flask.g.user,
- pkgs=form.pkgs.data.replace('\n', ' '),
- copr=copr) # we're checking authorization above for now
- if flask.g.user.proven:
- build.memory_reqs = form.memory_reqs.data
- build.timeout = form.timeout.data
+ try:
+ build = builds_logic.BuildsLogic.add(user=flask.g.user,
+ pkgs=form.pkgs.data.replace('\n', ' '),
+ copr=copr) # we're checking authorization above for now
+ if flask.g.user.proven:
+ build.memory_reqs = form.memory_reqs.data
+ build.timeout = form.timeout.data
+ except exceptions.ActionInProgressException as e:
+ flask.flash('Can\'t build in this Copr, there is an operation in progress: {0}'.format(e.action))
+ db.session.rollback()
+ else:
+ flask.flash("Build was added")
+ db.session.commit()
- db.session.commit()
-
- flask.flash("Build was added")
return flask.redirect(flask.url_for('coprs_ns.copr_builds', username=username, coprname=copr.name))
else:
return copr_add_build(username=username, coprname=coprname, form=form)
diff --git a/coprs_frontend/tests/test_logic/test_builds_logic.py b/coprs_frontend/tests/test_logic/test_builds_logic.py
index 007f649..2a2468b 100644
--- a/coprs_frontend/tests/test_logic/test_builds_logic.py
+++ b/coprs_frontend/tests/test_logic/test_builds_logic.py
@@ -1,4 +1,8 @@
+import pytest
+
+from coprs.exceptions import ActionInProgressException
from coprs.logic.builds_logic import BuildsLogic
+
from tests.coprs_test_case import CoprsTestCase
class TestBuildsLogic(CoprsTestCase):
@@ -8,3 +12,8 @@ class TestBuildsLogic(CoprsTestCase):
b = BuildsLogic.add(self.u2, 'blah blah', self.c2)
self.db.session.commit()
assert b.chroots == self.mc3.chroot_name
+
+ def test_add_raises_if_copr_has_unfinished_actions(self, f_users, f_coprs, f_actions, f_db):
+ with pytest.raises(ActionInProgressException):
+ b = BuildsLogic.add(self.u1, 'blah blah', self.c1)
+ self.db.session.rollback()
11 years
[copr] master: Refactor checking for unfinished actions into a separate function (we will use it on multiple places) (d94d26a)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit d94d26ab592eba0e7eee3120aadd7b20a5f37cbc
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Wed Mar 27 10:20:04 2013 +0100
Refactor checking for unfinished actions into a separate function (we will use it on multiple places)
>---------------------------------------------------------------
coprs_frontend/coprs/logic/coprs_logic.py | 19 ++++++++++++++-----
coprs_frontend/tests/coprs_test_case.py | 2 ++
.../tests/test_logic/test_coprs_logic.py | 14 ++++++++++++++
3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index abc8f0b..29aa6b0 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -9,6 +9,7 @@ from coprs import whoosheers
class CoprsLogic(object):
"""Used for manipulating Coprs. All methods accept user object as a first argument, as this may be needed in future."""
+
@classmethod
def get(cls, user, username, coprname, **kwargs):
with_builds = kwargs.get('with_builds', False)
@@ -91,9 +92,7 @@ class CoprsLogic(object):
@classmethod
def update(cls, user, copr, check_for_duplicates = True):
- action = cls.unfinished_actions(user, copr).first()
- if action:
- raise exceptions.ActionInProgressException('Action in progress on this copr.', action)
+ cls.raise_if_unfinished_action(user, copr)
existing = cls.exists_for_user(copr.owner, copr.name).first()
if existing:
@@ -125,13 +124,23 @@ class CoprsLogic(object):
update({models.Copr.build_count: models.Copr.build_count + 1})
@classmethod
- def unfinished_actions(cls, user, copr):
+ def unfinished_actions_for(cls, user, copr):
actions = models.Action.query.filter(models.Action.object_type=='copr').\
filter(models.Action.object_id==copr.id).\
- filter(models.Action.backend_result==0)
+ filter(models.Action.backend_result==helpers.BackendResultEnum('waiting'))
return actions
+ @classmethod
+ def raise_if_unfinished_action(cls, user, copr):
+ """This method raises ActionInProgressException if given copr has an unfinished
+ action. Returns None otherwise.
+ """
+ unfinished_actions = cls.unfinished_actions_for(user, copr).all()
+ if unfinished_actions:
+ raise exceptions.ActionInProgressException('Action in progress on this copr.', unfinished_actions[0])
+
+
class CoprPermissionsLogic(object):
@classmethod
def get(cls, user, copr, searched_user):
diff --git a/coprs_frontend/tests/coprs_test_case.py b/coprs_frontend/tests/coprs_test_case.py
index dcb0cb9..4f6043f 100644
--- a/coprs_frontend/tests/coprs_test_case.py
+++ b/coprs_frontend/tests/coprs_test_case.py
@@ -100,6 +100,8 @@ class CoprsTestCase(object):
@pytest.fixture
def f_actions(self):
+ # if using actions, we need to flush coprs into db, so that we can get their ids
+ self.f_db()
self.a1 = models.Action(action_type=helpers.ActionTypeEnum('rename'),
object_type='copr',
object_id=self.c1.id,
diff --git a/coprs_frontend/tests/test_logic/test_coprs_logic.py b/coprs_frontend/tests/test_logic/test_coprs_logic.py
new file mode 100644
index 0000000..c3f365b
--- /dev/null
+++ b/coprs_frontend/tests/test_logic/test_coprs_logic.py
@@ -0,0 +1,14 @@
+import pytest
+
+from coprs.exceptions import ActionInProgressException
+from coprs.helpers import BackendResultEnum
+from coprs.logic.coprs_logic import CoprsLogic
+
+from tests.coprs_test_case import CoprsTestCase
+
+class TestCoprsLogic(CoprsTestCase):
+ def test_update_raises_if_copr_has_unfinished_actions(self, f_users, f_coprs, f_actions, f_db):
+ self.c1.name = 'foo'
+ with pytest.raises(ActionInProgressException):
+ CoprsLogic.update(self.u1, self.c1)
+ self.db.session.rollback()
11 years
[copr] master: Only display information relevant for backend in waiting builds (e5d18a7)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit e5d18a79048575b520de95a04d88445e74db69f6
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 21 11:24:06 2013 +0100
Only display information relevant for backend in waiting builds
>---------------------------------------------------------------
.../coprs/views/backend_ns/backend_general.py | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/coprs_frontend/coprs/views/backend_ns/backend_general.py b/coprs_frontend/coprs/views/backend_ns/backend_general.py
index 27baca6..7be60a2 100644
--- a/coprs_frontend/coprs/views/backend_ns/backend_general.py
+++ b/coprs_frontend/coprs/views/backend_ns/backend_general.py
@@ -15,7 +15,7 @@ def waiting_builds():
builds = query[0:10]
return flask.jsonify({'builds': [build.to_dict(options = {'copr': {'owner': {},
- '__columns_except__': ['chroots', 'repos', 'build_count'],
+ '__columns_only__': ['id', 'name'],
'__included_ids__': False},
'__included_ids__': False}) for build in builds]})
11 years
[copr] master: Document models, partially fixes #52 (6361440)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 6361440f2f0265c1369374fb457644f98c5c41dc
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 21 11:02:57 2013 +0100
Document models, partially fixes #52
>---------------------------------------------------------------
coprs_frontend/coprs/models.py | 78 ++++++++++++++++++++++++++++++++++------
1 files changed, 67 insertions(+), 11 deletions(-)
diff --git a/coprs_frontend/coprs/models.py b/coprs_frontend/coprs/models.py
index 291df11..22e8efe 100644
--- a/coprs_frontend/coprs/models.py
+++ b/coprs_frontend/coprs/models.py
@@ -55,21 +55,30 @@ class Serializer(object):
return map(lambda x: x.name, self.__table__.columns)
class User(db.Model, Serializer):
+ """Represents user of the copr frontend"""
id = db.Column(db.Integer, primary_key = True)
+ # openid_name for fas, e.g. http://bkabrda.id.fedoraproject.org/
openid_name = db.Column(db.String(100), nullable = False)
+ # just mail :)
mail = db.Column(db.String(150), nullable = False)
+ # is this user proven? proven users can modify builder memory and timeout for single builds
proven = db.Column(db.Boolean, default = False)
+ # is this user admin of the system?
admin = db.Column(db.Boolean, default = False)
+ # stuff for the cli interface
api_login = db.Column(db.String(40), nullable = False, default = 'abc')
api_token = db.Column(db.String(40), nullable = False, default = 'abc')
api_token_expiration = db.Column(db.Date, nullable = False, default = datetime.date(2000, 1, 1))
@property
def name(self):
+ """Returns the short username of the user, e.g. bkabrda"""
return self.openid_name.replace('.id.fedoraproject.org/', '').replace('http://', '')
- def permissions_for_copr(self, copr): # simple caching of permissions for given copr
- # we can't put this into class declaration because the class may be shared by multiple threads
+ def permissions_for_copr(self, copr):
+ """Get permissions of this user for the given copr.
+ Caches the permission during one request, so use this if you access them multiple times
+ """
if not hasattr(self, '_permissions_for_copr'):
self._permissions_for_copr = {}
if not copr.name in self._permissions_for_copr:
@@ -77,6 +86,7 @@ class User(db.Model, Serializer):
return self._permissions_for_copr[copr.name]
def can_build_in(self, copr):
+ """Determine if this user can build in the given copr."""
can_build = False
if copr.owner == self:
can_build = True
@@ -86,6 +96,7 @@ class User(db.Model, Serializer):
return can_build
def can_edit(self, copr):
+ """Determine if this user can edit the given copr."""
can_edit = False
if copr.owner == self:
can_edit = True
@@ -96,6 +107,11 @@ class User(db.Model, Serializer):
@classmethod
def openidize_name(cls, name):
+ """Creates proper openid_name from short name.
+
+ >>> user.openid_name == User.openidize_name(user.name)
+ True
+ """
return 'http://{0}.id.fedoraproject.org/'.format(name)
@property
@@ -105,14 +121,21 @@ class User(db.Model, Serializer):
@property
def coprs_count(self):
+ """Get number of coprs for this user."""
return Copr.query.filter_by(owner=self).count()
class Copr(db.Model, Serializer):
+ """Represents a single copr (private repo with builds, mock chroots, etc.)."""
id = db.Column(db.Integer, primary_key = True)
+ # name of the copr, no fancy chars (checked by forms)
name = db.Column(db.String(100), nullable = False)
+ # string containing urls of additional repos (separated by space)
+ # that this copr will pull dependencies from
repos = db.Column(db.Text)
+ # time of creation as returned by int(time.time())
created_on = db.Column(db.Integer)
+ # description and instructions given by copr owner
description = db.Column(db.Text)
instructions = db.Column(db.Text)
# duplicate information, but speeds up a lot and makes queries simpler
@@ -129,6 +152,7 @@ class Copr(db.Model, Serializer):
@property
def repos_list(self):
+ """Returns repos of this copr as a list of strings"""
return self.repos.split()
@property
@@ -141,12 +165,15 @@ class Copr(db.Model, Serializer):
@property
def active_mock_chroots(self):
+ """Returns list of active mock_chroots of this copr"""
return filter(lambda x: x.is_active, self.mock_chroots)
class CoprPermission(db.Model, Serializer):
- # 0 = nothing, 1 = asked for, 2 = approved
- # not using enum, as that translates to varchar on some DBs
+ """Association class for Copr<->Permission relation"""
+ ## see helpers.PermissionEnum for possible values of the fields below
+ # can this user build in the copr?
copr_builder = db.Column(db.SmallInteger, default = 0)
+ # can this user serve as an admin? (-> edit and approve permissions)
copr_admin = db.Column(db.SmallInteger, default = 0)
# relations
@@ -156,19 +183,31 @@ class CoprPermission(db.Model, Serializer):
copr = db.relationship('Copr', backref = db.backref('copr_permissions'))
class Build(db.Model, Serializer):
+ """Representation of one build in one copr"""
id = db.Column(db.Integer, primary_key = True)
+ # list of space separated urls of packages to build
pkgs = db.Column(db.Text)
+ # was this build canceled by user?
canceled = db.Column(db.Boolean, default = False)
- # These two are present for every build, as they might change in Copr
- # between submitting and starting build => we want to keep them as submitted
+ ## These two are present for every build, as they might change in Copr
+ ## between submitting and starting build => we want to keep them as submitted
+ # list of space separated mock chroot names
chroots = db.Column(db.Text, nullable = False)
+ # list of space separated additional repos
repos = db.Column(db.Text)
+ ## the three below represent time of important events for this build
+ ## as returned by int(time.time())
submitted_on = db.Column(db.Integer, nullable = False)
started_on = db.Column(db.Integer)
ended_on = db.Column(db.Integer)
+ # url of the build results
results = db.Column(db.Text)
+ # status as returned by backend, see build.state for value explanation
+ # (TODO: this would deserve an enum)
status = db.Column(db.Integer)
+ # memory requirements for backend builder
memory_reqs = db.Column(db.Integer, default = constants.DEFAULT_BUILD_MEMORY)
+ # maximum allowed time of build, build will fail if exceeded
timeout = db.Column(db.Integer, default = constants.DEFAULT_BUILD_TIMEOUT)
# relations
@@ -179,6 +218,7 @@ class Build(db.Model, Serializer):
@property
def state(self):
+ """Return text representation of status of this build"""
if self.status == 1:
return 'succeeded'
elif self.status == 0:
@@ -191,17 +231,26 @@ class Build(db.Model, Serializer):
@property
def cancelable(self):
+ """Find out if this build is cancelable.
+
+ ATM, build is cancelable only if it wasn't grabbed by backend.
+ """
return self.state == 'pending'
class MockChroot(db.Model, Serializer):
+ """Representation of mock chroot"""
id = db.Column(db.Integer, primary_key = True)
- os_release = db.Column(db.String(50), nullable = False) # fedora/epel/...
- os_version = db.Column(db.String(50), nullable = False) # 18/rawhide/...
- arch = db.Column(db.String(50), nullable = False) # x86_64/i686/...
+ # fedora/epel/..., mandatory
+ os_release = db.Column(db.String(50), nullable = False)
+ # 18/rawhide/..., optional (mock chroot doesn't need to have this)
+ os_version = db.Column(db.String(50), nullable = False)
+ # x86_64/i686/..., mandatory
+ arch = db.Column(db.String(50), nullable = False)
is_active = db.Column(db.Boolean, default = True)
@property
def chroot_name(self):
+ """Textual representation of name of this chroot"""
if self.os_version:
format_string = '{rel}-{ver}-{arch}'
else:
@@ -211,6 +260,7 @@ class MockChroot(db.Model, Serializer):
arch=self.arch)
class CoprChroot(db.Model, Serializer):
+ """Representation of Copr<->MockChroot relation"""
mock_chroot_id = db.Column(db.Integer, db.ForeignKey('mock_chroot.id'), primary_key = True)
mock_chroot = db.relationship('MockChroot', backref = db.backref('copr_chroots'))
copr_id = db.Column(db.Integer, db.ForeignKey('copr.id'), primary_key = True)
@@ -219,16 +269,22 @@ class CoprChroot(db.Model, Serializer):
cascade='all,delete,delete-orphan'))
class Action(db.Model, Serializer):
+ """Representation of a custom action that needs backends cooperation"""
id = db.Column(db.Integer, primary_key=True)
- # delete, rename,
+ # delete, rename, ...; see helpers.ActionTypeEnum
action_type = db.Column(db.Integer, nullable=False)
- # copr,
+ # copr, ...; downcase name of class of modified object
object_type = db.Column(db.String(20))
+ # id of the modified object
object_id = db.Column(db.Integer)
+ # old and new values of the changed property
old_value = db.Column(db.String(255))
new_value = db.Column(db.String(255))
+ # backend result, see helpers.BackendResultEnum
backend_result = db.Column(db.Integer, default=helpers.BackendResultEnum('waiting'))
+ # optional message from the backend
backend_message = db.Column(db.Text)
+ # time created as returned by int(time.time())
created_on = db.Column(db.Integer)
def __str__(self):
11 years, 1 month
[copr] master: Properly deal with inactive mock chroots, closes #50 (2fdf8a7)
by bkabrda@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 2fdf8a7328687f59a4a2f508fc775202f46dbe42
Author: Bohuslav Kabrda <bkabrda(a)redhat.com>
Date: Thu Mar 21 10:27:42 2013 +0100
Properly deal with inactive mock chroots, closes #50
>---------------------------------------------------------------
coprs_frontend/coprs/logic/builds_logic.py | 4 +---
coprs_frontend/coprs/logic/coprs_logic.py | 11 +++++++++--
coprs_frontend/coprs/models.py | 4 ++++
.../coprs/templates/coprs/detail/overview.html | 12 ++++++------
coprs_frontend/coprs/templates/coprs/show.html | 2 +-
5 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/coprs_frontend/coprs/logic/builds_logic.py b/coprs_frontend/coprs/logic/builds_logic.py
index 1ef7252..191e95b 100644
--- a/coprs_frontend/coprs/logic/builds_logic.py
+++ b/coprs_frontend/coprs/logic/builds_logic.py
@@ -61,9 +61,7 @@ class BuildsLogic(object):
pkgs=pkgs,
copr=copr,
repos=copr.repos,
- chroots=' '.join(map(lambda x: x.chroot_name,
- filter(lambda b: b.is_active, #only add active chroots
- copr.mock_chroots))),
+ chroots=' '.join(map(lambda x: x.chroot_name, copr.active_mock_chroots)),
user=user,
submitted_on=int(time.time()))
# no need to check for authorization here
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py
index 9418090..abc8f0b 100644
--- a/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/coprs_frontend/coprs/logic/coprs_logic.py
@@ -41,7 +41,9 @@ class CoprsLogic(object):
query = db.session.query(models.Copr).\
join(models.Copr.owner).\
- options(db.contains_eager(models.Copr.owner))
+ options(db.contains_eager(models.Copr.owner)).\
+ order_by(models.Copr.id.desc())
+
if user_relation == 'owned':
query = query.filter(models.User.openid_name == models.User.openidize_name(username))
elif user_relation == 'allowed':
@@ -51,7 +53,12 @@ class CoprsLogic(object):
join(aliased_user, models.CoprPermission.user).\
filter(aliased_user.openid_name == models.User.openidize_name(username))
if with_mock_chroots:
- query = query.options(db.subqueryload(*models.Copr.mock_chroots.attr))
+ query = query.outerjoin(*models.Copr.mock_chroots.attr).\
+ options(db.contains_eager(*models.Copr.mock_chroots.attr)).\
+ order_by(models.MockChroot.os_release.asc()).\
+ order_by(models.MockChroot.os_version.asc()).\
+ order_by(models.MockChroot.arch.asc())
+
return query
@classmethod
diff --git a/coprs_frontend/coprs/models.py b/coprs_frontend/coprs/models.py
index 957b80e..291df11 100644
--- a/coprs_frontend/coprs/models.py
+++ b/coprs_frontend/coprs/models.py
@@ -139,6 +139,10 @@ class Copr(db.Model, Serializer):
def instructions_or_not_filled(self):
return self.instructions or 'Instructions not filled in by author.'
+ @property
+ def active_mock_chroots(self):
+ return filter(lambda x: x.is_active, self.mock_chroots)
+
class CoprPermission(db.Model, Serializer):
# 0 = nothing, 1 = asked for, 2 = approved
# not using enum, as that translates to varchar on some DBs
diff --git a/coprs_frontend/coprs/templates/coprs/detail/overview.html b/coprs_frontend/coprs/templates/coprs/detail/overview.html
index 0d74561..6152ddf 100644
--- a/coprs_frontend/coprs/templates/coprs/detail/overview.html
+++ b/coprs_frontend/coprs/templates/coprs/detail/overview.html
@@ -12,10 +12,10 @@
<th class="leftmost">Release</th>
<th class="rightmost">Architecture</th>
</tr>
- {% for mock_chroot in copr.mock_chroots %}
- {% if loop.index < copr.mock_chroots|length %}
- {% if mock_chroot.os_release != copr.mock_chroots[loop.index].os_release or
- mock_chroot.os_version != copr.mock_chroots[loop.index].os_version %}
+ {% for mock_chroot in copr.active_mock_chroots %}
+ {% if loop.index < copr.active_mock_chroots|length %}
+ {% if mock_chroot.os_release != copr.active_mock_chroots[loop.index].os_release or
+ mock_chroot.os_version != copr.active_mock_chroots[loop.index].os_version %}
{# next release is different => release-end #}
<tr class="release-end">
{% else %}
@@ -24,8 +24,8 @@
{% else %}{# last line => release-end for sure #}
<tr class="release-end">
{% endif %}
- {% if mock_chroot.os_release != copr.mock_chroots[loop.index0 - 1].os_release or
- mock_chroot.os_version != copr.mock_chroots[loop.index0 - 1].os_version or
+ {% if mock_chroot.os_release != copr.active_mock_chroots[loop.index0 - 1].os_release or
+ mock_chroot.os_version != copr.active_mock_chroots[loop.index0 - 1].os_version or
loop.index0 == 0 %}
{# previous os_release-os_version were different or this is the first one #}
<td>{{ mock_chroot.os_release|capitalize }} {{ mock_chroot.os_version }}</td>
diff --git a/coprs_frontend/coprs/templates/coprs/show.html b/coprs_frontend/coprs/templates/coprs/show.html
index c28ebcc..5ccde32 100644
--- a/coprs_frontend/coprs/templates/coprs/show.html
+++ b/coprs_frontend/coprs/templates/coprs/show.html
@@ -25,7 +25,7 @@
<a href="{{ url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name) }}">{{ copr.owner.name }}/{{ copr.name }}</a>
<p>{{ copr.description_or_not_filled }}</p>
<p class="repos">
- {% for mock_chroot in copr.mock_chroots %}
+ {% for mock_chroot in copr.active_mock_chroots %}
{{ mock_chroot.os_release|os_name_short(mock_chroot.os_version) }}.{{ mock_chroot.arch }}{% if not loop.last %}, {% endif %}
{% endfor %}
</p>
11 years, 1 month