Repository :
http://git.fedorahosted.org/cgit/copr.git
On branch : master
---------------------------------------------------------------
commit 29b9391ecbcca2b3e0646d995bacb1f2d771d090
Author: Valentin Gologuzov <vgologuz(a)redhat.com>
Date: Tue Sep 1 14:47:22 2015 +0200
[frontend][api] Updated and tested PUT method to /projects/<id> resource
---------------------------------------------------------------
frontend/coprs_frontend/coprs/logic/coprs_logic.py | 33 +----
.../coprs/rest_api/resources/project.py | 61 ++++------
frontend/coprs_frontend/coprs/rest_api/schemas.py | 58 +++++++--
.../coprs/views/coprs_ns/coprs_general.py | 3 +-
.../tests/test_api/test_project_r.py | 128 ++++++++++++++++++--
5 files changed, 197 insertions(+), 86 deletions(-)
diff --git a/frontend/coprs_frontend/coprs/logic/coprs_logic.py
b/frontend/coprs_frontend/coprs/logic/coprs_logic.py
index 0957462..91206cc 100644
--- a/frontend/coprs_frontend/coprs/logic/coprs_logic.py
+++ b/frontend/coprs_frontend/coprs/logic/coprs_logic.py
@@ -4,6 +4,7 @@ from sqlalchemy import and_
from sqlalchemy.event import listen
from sqlalchemy.orm.attributes import NEVER_SET
from sqlalchemy.orm.exc import NoResultFound
+from sqlalchemy.orm.attributes import get_history
from coprs import db
from coprs import exceptions
@@ -164,37 +165,15 @@ class CoprsLogic(object):
db.session.add(copr)
@classmethod
- def update(cls, user, copr, check_for_duplicates=True):
- cls.raise_if_unfinished_blocking_action(
- copr, "Can't change this project name,"
- " another operation is in progress: {action}")
+ def update(cls, user, copr):
+ # we should call get_history before other requests, otherwise
+ # the changes would be forgotten
+ if get_history(copr, "name").has_changes():
+ raise MalformedArgumentException("Change name of the project is
forbidden")
users_logic.UsersLogic.raise_if_cant_update_copr(
user, copr, "Only owners and admins may update their projects.")
- existing = cls.exists_for_user(copr.owner, copr.name).first()
- if existing:
- if check_for_duplicates and existing.id != copr.id:
- raise exceptions.DuplicateException(
- "Project: '{0}' already exists".format(copr.name))
-
- else: # we're renaming
- # if we fire a models.Copr.query, it will use the modified copr in session
- # -> workaround this by just getting the name
- old_copr_name = (db.session.query(models.Copr.name)
- .filter(models.Copr.id == copr.id)
- .filter(models.Copr.deleted == False)
- .first()[0])
-
- action =
models.Action(action_type=helpers.ActionTypeEnum("rename"),
- object_type="copr",
- object_id=copr.id,
- old_value="{0}/{1}".format(copr.owner.name,
- old_copr_name),
- new_value="{0}/{1}".format(copr.owner.name,
- copr.name),
- created_on=int(time.time()))
- db.session.add(action)
db.session.add(copr)
@classmethod
diff --git a/frontend/coprs_frontend/coprs/rest_api/resources/project.py
b/frontend/coprs_frontend/coprs/rest_api/resources/project.py
index 6c2cbd7..17dc94b 100644
--- a/frontend/coprs_frontend/coprs/rest_api/resources/project.py
+++ b/frontend/coprs_frontend/coprs/rest_api/resources/project.py
@@ -19,12 +19,13 @@ from coprs.logic.helpers import slice_query
from coprs.logic.users_logic import UsersLogic
from coprs.logic.coprs_logic import CoprsLogic
from coprs.exceptions import ActionInProgressException, InsufficientRightsException
-from coprs.rest_api.schemas import ProjectSchema
+from coprs.rest_api.schemas import ProjectSchema, ProjectCreateSchema
from ..exceptions import ObjectAlreadyExists, AuthFailed, CannotProcessRequest,
AccessForbidden
from ..util import get_one_safe, json_loads_safe, mm_deserialize, render_allowed_method,
mm_serialize_one
def rest_api_auth_required(f):
+ # todo: move to common.py and test this
@functools.wraps(f)
def decorated_function(*args, **kwargs):
token = None
@@ -69,10 +70,6 @@ def render_project(copr, self_params=None):
"builds": {"href": url_for(".buildlistr",
project_id=copr.id)},
"chroots": {"href":
url_for(".projectchrootlistr", project_id=copr.id)}
},
- # "allowed_methods": [
- # render_allowed_method("GET", "Get single copr",
require_auth=False),
- # render_allowed_method("DELETE", "Delete current copr",
require_auth=True),
- # ]
}
@@ -85,30 +82,19 @@ class ProjectListR(Resource):
"""
owner = flask.g.user
- result = mm_deserialize(ProjectSchema(), flask.request.data)
+ result = mm_deserialize(ProjectCreateSchema(), flask.request.data)
# todo check that chroots are available
req = result.data
-
- extra = {
- k: req[k]
- for k in [
- "description",
- "instructions",
- "contact",
- "homepage",
- "auto_createrepo",
- "build_enable_net",
- "repos"
- ] if k in req
- }
+ name = req.pop("name")
+ selected_chroots = req.pop("chroots")
try:
project = CoprsLogic.add(
user=owner, check_for_duplicates=True,
- name=req["name"],
- selected_chroots=req["chroots"],
- **extra
+ name=name,
+ selected_chroots=selected_chroots,
+ **req
)
db.session.commit()
except DuplicateException as error:
@@ -154,19 +140,6 @@ class ProjectListR(Resource):
"coprs": [render_project(copr) for copr in coprs_list],
}
- # TODO: show only if user provided ?help=true param
- #
- # if req_args.get("help"):
- # result_dict["allowed_methods"] = [
- # render_allowed_method("GET", "Get list of coprs",
require_auth=False,
- # params=[
- # "username: filter coprs owned by the
user",
- # "limit: show only the given number of
coprs",
- # "offset: skip given number of
coprs",
- # ]),
- # render_allowed_method("POST", "Creates new copr, send
dict with copr fields"),
- # ]
-
return result_dict
@@ -220,5 +193,21 @@ class ProjectR(Resource):
"""
Modifies project by replacement of provided fields
"""
- pass
+ project = get_one_safe(CoprsLogic.get_by_id(int(project_id)))
+
+ project_dict = mm_deserialize(ProjectSchema(), flask.request.data).data
+ # pprint(project_dict)
+
+ for k, v in project_dict.items():
+ setattr(project, k, v)
+
+ try:
+ CoprsLogic.update(flask.g.user, project)
+ db.session.commit()
+ except InsufficientRightsException as err:
+ raise AccessForbidden(str(err))
+
+ resp = make_response("", 201)
+ resp.headers["Location"] = url_for(".projectr",
project_id=project.id)
+ return resp
diff --git a/frontend/coprs_frontend/coprs/rest_api/schemas.py
b/frontend/coprs_frontend/coprs/rest_api/schemas.py
index 7325e3e..216a880 100644
--- a/frontend/coprs_frontend/coprs/rest_api/schemas.py
+++ b/frontend/coprs_frontend/coprs/rest_api/schemas.py
@@ -2,6 +2,30 @@
from collections import Iterable
from marshmallow import Schema, fields
+from marshmallow import Schema, fields, validates_schema, ValidationError, validate
+
+
+def validate_any(fn_list):
+ """
+ :param fn_list: list of callable functions, each takes one param
+ :return: None if at least one validation function exists without exceptions
+ :raises ValidationError: otherwise
+ """
+ def func(value):
+ # import ipdb; ipdb.set_trace()
+ errors = []
+ for fn in fn_list:
+ try:
+ fn(value)
+ except ValidationError as err:
+ errors.append(str(err))
+ else:
+ return
+ else:
+ errors.insert(0, u"At least one validator should accept given
value:")
+ raise ValidationError(errors)
+
+ return func
class SpaceSeparatedList(fields.Field):
@@ -11,8 +35,11 @@ class SpaceSeparatedList(fields.Field):
return value.split()
def _deserialize(self, value):
- if value is None or not isinstance(value, Iterable):
+ if value is None:
return ""
+ elif not isinstance(value, Iterable) or isinstance(value, basestring):
+ raise ValidationError("Value `{}` is not a list of strings"
+ .format(value))
else:
return " ".join(value)
@@ -31,23 +58,28 @@ class MockChrootSchema(Schema):
class ProjectSchema(Schema):
- id = fields.Int()
- name = fields.Str(required=True)
+ id = fields.Int(dump_only=True)
+ name = fields.Str(dump_only=True)
owner = fields.Str(attribute="owner_name", dump_only=True)
- description = fields.Str()
- instructions = fields.Str()
- homepage = fields.Str()
- contact = fields.Str()
+ description = fields.Str(allow_none=True)
+ instructions = fields.Str(allow_none=True)
+ homepage = fields.Url(allow_none=True)
+ contact = fields.Str(validate=validate_any([
+ validate.URL(),
+ validate.Email(),
+ validate.OneOf(["", None]),
+ ]), allow_none=True)
- auto_createrepo = fields.Bool()
- build_enable_net = fields.Bool()
- last_modified = fields.DateTime()
+ disable_createrepo = fields.Bool(allow_none=True)
+ build_enable_net = fields.Bool(allow_none=True)
+ last_modified = fields.DateTime(dump_only=True)
- #additional_repos = fields.List(fields.Str(), attribute="repos_list")
- repos = SpaceSeparatedList()
+ repos = SpaceSeparatedList(allow_none=True)
- # used only for creation
+
+class ProjectCreateSchema(ProjectSchema):
+ name = fields.Str(required=True)
chroots = fields.List(fields.Str(), load_only=True)
diff --git a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
index 5a85b87..6ff6221 100644
--- a/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
+++ b/frontend/coprs_frontend/coprs/views/coprs_ns/coprs_general.py
@@ -347,8 +347,7 @@ def copr_update(username, coprname):
try:
# form validation checks for duplicates
- coprs_logic.CoprsLogic.update(
- flask.g.user, copr, check_for_duplicates=False)
+ coprs_logic.CoprsLogic.update(flask.g.user, copr)
except (exceptions.ActionInProgressException,
exceptions.InsufficientRightsException) as e:
diff --git a/frontend/coprs_frontend/tests/test_api/test_project_r.py
b/frontend/coprs_frontend/tests/test_api/test_project_r.py
index 6fae757..c6bb38d 100644
--- a/frontend/coprs_frontend/tests/test_api/test_project_r.py
+++ b/frontend/coprs_frontend/tests/test_api/test_project_r.py
@@ -1,6 +1,7 @@
# coding: utf-8
import base64
+import copy
import json
from marshmallow import pprint
@@ -15,6 +16,19 @@ from tests.coprs_test_case import CoprsTestCase, TransactionDecorator
class TestProjectResource(CoprsTestCase):
+ put_update_dict = {
+ "description": "foo bar",
+ "instructions": "cthulhu fhtagn",
+ "repos": [
+ "http://example.com/repo",
+ "copr://foo/bar"
+ "copr://g/foo/bar"
+ ],
+ "disable_createrepo": True,
+ "build_enable_net": False,
+ "homepage": "http://example.com/foobar",
+ "contact": "foo(a)example.com",
+ }
def test_self(self):
href = "/api_2/projects"
@@ -77,8 +91,9 @@ class TestProjectResource(CoprsTestCase):
project = CoprsLogic.get_by_id(p_id).one()
assert len(obj["project_chroots"]) == len(project.copr_chroots)
- def test_get_one_with_builds(self, f_users, f_mock_chroots,
- f_coprs, f_builds, f_db):
+ def test_get_one_with_builds(
+ self, f_users, f_mock_chroots,
+ f_coprs, f_builds, f_db):
p_id_list = [p.id for p in self.basic_coprs_list]
for p_id in p_id_list:
@@ -93,8 +108,9 @@ class TestProjectResource(CoprsTestCase):
builds = BuildsLogic.get_multiple_by_copr(project).all()
assert len(obj["project_builds"]) == len(builds)
- def test_delete_not_found(self, f_users, f_mock_chroots,
- f_users_api, f_db):
+ def test_delete_not_found(
+ self, f_users, f_mock_chroots,
+ f_users_api, f_db):
href = "/api_2/projects/{}".format("1")
@@ -104,8 +120,9 @@ class TestProjectResource(CoprsTestCase):
)
assert r0.status_code == 404
- def test_delete_ok(self, f_users, f_mock_chroots,
- f_coprs, f_users_api, f_db):
+ def test_delete_ok(
+ self, f_users, f_mock_chroots,
+ f_coprs, f_users_api, f_db):
href = "/api_2/projects/{}".format(self.c1.id)
@@ -116,8 +133,9 @@ class TestProjectResource(CoprsTestCase):
assert r0.status_code == 204
assert self.tc.get(href).status_code == 404
- def test_delete_fail_unfinished_build(self, f_users, f_mock_chroots,
- f_coprs, f_builds, f_users_api, f_db):
+ def test_delete_fail_unfinished_build(
+ self, f_users, f_mock_chroots,
+ f_coprs, f_builds, f_users_api, f_db):
href = "/api_2/projects/{}".format(self.c1.id)
@@ -156,3 +174,97 @@ class TestProjectResource(CoprsTestCase):
)
assert r0.status_code == 403
+ def test_project_put_ok(
+ self, f_users, f_mock_chroots,
+ f_coprs, f_users_api, f_db):
+
+ href = "/api_2/projects/{}".format(self.c1.id)
+ r0 = self.request_rest_api_with_auth(
+ href,
+ method="put",
+ content=self.put_update_dict
+ )
+ assert r0.status_code == 201
+
+ r1 = self.tc.get(r0.headers["Location"])
+ obj = json.loads(r1.data)
+ updated_project = obj["project"]
+ for k, v in self.put_update_dict.items():
+ assert updated_project[k] == v
+
+ def test_project_put_wrong_user(
+ self, f_users, f_mock_chroots,
+ f_coprs, f_users_api, f_db):
+ login = self.u2.api_login
+ token = self.u2.api_token
+
+ href = "/api_2/projects/{}".format(self.c1.id)
+ r0 = self.request_rest_api_with_auth(
+ href,
+ method="put",
+ content=self.put_update_dict,
+ login=login, token=token,
+ )
+ assert r0.status_code == 403
+
+ def test_project_put_not_found(
+ self, f_users, f_mock_chroots, f_users_api, f_db):
+
+ href = "/api_2/projects/1"
+ r0 = self.request_rest_api_with_auth(
+ href,
+ method="put",
+ content=self.put_update_dict,
+ )
+ assert r0.status_code == 404
+
+ def test_project_put_bad_values(
+ self, f_users, f_mock_chroots,
+ f_coprs, f_users_api, f_db):
+ href = "/api_2/projects/{}".format(self.c1.id)
+ cases = []
+
+ def add_case(field, value):
+ t = copy.deepcopy(self.put_update_dict)
+ t[field] = value
+ cases.append(t)
+
+ add_case("repos", "foobar")
+ add_case("repos", 1)
+ add_case("contact", "adsg")
+ add_case("contact", "1")
+ add_case("homepage", "sdg")
+
+ for test_case in cases:
+ r0 = self.request_rest_api_with_auth(
+ href,
+ method="put",
+ content=test_case
+ )
+ assert r0.status_code == 400
+
+ def test_project_put_ok_values(
+ self, f_users, f_mock_chroots,
+ f_coprs, f_users_api, f_db):
+ href = "/api_2/projects/{}".format(self.c1.id)
+ cases = []
+
+ def add_case(field, value):
+ t = copy.deepcopy(self.put_update_dict)
+ t[field] = value
+ cases.append(t)
+
+ add_case("repos", [])
+ add_case("repos", None)
+ add_case("contact", "")
+ add_case("contact", None)
+ add_case("contact", "foo(a)bar.com")
+ add_case("homepage", "http://foo.org/bar/xdeeg?sdfg")
+
+ for test_case in cases:
+ r0 = self.request_rest_api_with_auth(
+ href,
+ method="put",
+ content=test_case
+ )
+ assert r0.status_code == 201