Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

frontend: migrate API projects namespace to flask-restx #2807

Merged
merged 1 commit into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 174 additions & 42 deletions frontend/coprs_frontend/coprs/views/apiv3_ns/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,11 @@
import inspect
from functools import wraps
from werkzeug.datastructures import ImmutableMultiDict, MultiDict
from werkzeug.exceptions import HTTPException, NotFound, GatewayTimeout
from sqlalchemy.orm.attributes import InstrumentedAttribute
from flask_restx import Api, Namespace, Resource
from coprs import app
from flask_restx import Api, Namespace
from coprs.exceptions import (
AccessRestricted,
ActionInProgressException,
CoprHttpException,
InsufficientStorage,
ObjectNotFound,
BadRequest,
)
from coprs.logic.complex_logic import ComplexLogic
Expand Down Expand Up @@ -51,48 +46,68 @@ def home():
# HTTP methods
GET = ["GET"]
POST = ["POST"]
# TODO: POST != PUT nor DELETE, we should use at least use these methods according
# conventions -> POST to create new element, PUT to update element, DELETE to delete
# https://www.ibm.com/docs/en/urbancode-release/6.1.1?topic=reference-rest-api-conventions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable but if we decide to do this, we need to fix python-copr, ideally a few releases beforehand.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, this is just todo - I deprecated it in docs so users are lightly pushed to use methods according to convention but I have no intention to discuss/bring a plan to drop them... just tell users the should use something else but keep it working.

I even introduced a warning header message that the endpoint is deprecated - but yes, this will be better once python-copr is fixed accordingly because then it would make response with warning headers even using copr's official python library which isn't good - thanks for pointing to it

# fix python-copr firstly please, then put warning header to deprecated methods
PUT = ["POST", "PUT"]
DELETE = ["POST", "DELETE"]


def _convert_query_params(endpoint_method, params_to_not_look_for, **kwargs):
sig = inspect.signature(endpoint_method)
params = list(set(sig.parameters) - params_to_not_look_for)
for arg in params:
if arg not in flask.request.args:
# If parameter is present in the URL path, we can use its
# value instead of failing that it is missing in query
# parameters, e.g. let's have a view decorated with these
# two routes:
# @foo_ns.route("/foo/bar/<int:build>/<chroot>")
# @foo_ns.route("/foo/bar") accepting ?build=X&chroot=Y
# @query_params()
# Then we need the following condition to get the first
# route working
if arg in flask.request.view_args:
continue

# If parameter has a default value, it is not required
default_parameter_value = sig.parameters[arg].default
if default_parameter_value != sig.parameters[arg].empty:
kwargs[arg] = default_parameter_value
continue

raise BadRequest("Missing argument {}".format(arg))

kwargs[arg] = flask.request.args.get(arg)
return kwargs


def query_params():
params_to_not_look_for = {"args", "kwargs"}

def query_params_decorator(f):
@wraps(f)
def query_params_wrapper(*args, **kwargs):
sig = inspect.signature(f)
params = [x for x in sig.parameters]
params = list(set(params) - {"args", "kwargs"})
for arg in params:
if arg not in flask.request.args:
# If parameter is present in the URL path, we can use its
# value instead of failing that it is missing in query
# parameters, e.g. let's have a view decorated with these
# two routes:
# @foo_ns.route("/foo/bar/<int:build>/<chroot>")
# @foo_ns.route("/foo/bar") accepting ?build=X&chroot=Y
# @query_params()
# Then we need the following condition to get the first
# route working
if arg in flask.request.view_args:
continue

# If parameter has a default value, it is not required
if sig.parameters[arg].default == sig.parameters[arg].empty:
raise BadRequest("Missing argument {}".format(arg))
kwargs[arg] = flask.request.args.get(arg)
kwargs = _convert_query_params(f, params_to_not_look_for, **kwargs)
return f(*args, **kwargs)
return query_params_wrapper
return query_params_decorator


def _shared_pagination_wrapper(**kwargs):
form = PaginationForm(flask.request.args)
if not form.validate():
raise CoprHttpException(form.errors)
kwargs.update(form.data)
return kwargs


def pagination():
def pagination_decorator(f):
@wraps(f)
def pagination_wrapper(*args, **kwargs):
form = PaginationForm(flask.request.args)
if not form.validate():
raise CoprHttpException(form.errors)
kwargs.update(form.data)
kwargs = _shared_pagination_wrapper(**kwargs)
return f(*args, **kwargs)
return pagination_wrapper
return pagination_decorator
Expand Down Expand Up @@ -232,19 +247,24 @@ def get(self):
return objects[self.offset : limit]


def _check_if_user_can_edit_copr(ownername, projectname):
copr = get_copr(ownername, projectname)
if not flask.g.user.can_edit(copr):
raise AccessRestricted(
"User '{0}' can not see permissions for project '{1}' " \
"(missing admin rights)".format(
flask.g.user.name,
'/'.join([ownername, projectname])
)
)
return copr


def editable_copr(f):
@wraps(f)
def wrapper(ownername, projectname, **kwargs):
copr = get_copr(ownername, projectname)
if not flask.g.user.can_edit(copr):
raise AccessRestricted(
"User '{0}' can not see permissions for project '{1}' "\
"(missing admin rights)".format(
flask.g.user.name,
'/'.join([ownername, projectname])
)
)
return f(copr, **kwargs)
def wrapper(ownername, projectname):
copr = _check_if_user_can_edit_copr(ownername, projectname)
return f(copr)
return wrapper


Expand Down Expand Up @@ -374,3 +394,115 @@ def rename_fields_helper(input_dict, replace):
for value in values:
output.add(new_key, value)
return output


# Flask-restx specific helpers/decorators - don't use them with regular Flask API!
# TODO: delete/unify decorators for regular Flask and Flask-restx API once migration
# is done


def query_to_parameters(endpoint_method):
"""
Decorator passing query parameters to http method parameters

Returns:
Endpoint that has its query parameters can be used as parameters in http method
"""
params_to_not_look_for = {"self", "args", "kwargs"}

@wraps(endpoint_method)
def convert_query_parameters_of_endpoint_method(self, *args, **kwargs):
kwargs = _convert_query_params(endpoint_method, params_to_not_look_for, **kwargs)
return endpoint_method(self, *args, **kwargs)
return convert_query_parameters_of_endpoint_method


def deprecated_route_method(ns: Namespace, msg):
"""
Decorator that display a deprecation warning in headers and docs.

Usage:
class Endpoint(Resource):
...
@deprecated_route_method(foo_ns, "Message e.g. what to use instead")
...
def get():
return {"scary": "BOO!"}

Args:
ns: flask-restx Namespace
msg: Deprecation warning message.
"""
def decorate_endpoint_method(endpoint_method):
# render deprecation in API docs
ns.deprecated(endpoint_method)

@wraps(endpoint_method)
def warn_user_in_headers(self, *args, **kwargs):
custom_header = {"Warning": f"This method is deprecated: {msg}"}
resp = endpoint_method(self, *args, **kwargs)
if not isinstance(resp, tuple):
# only resp body as dict was passed
return resp, custom_header

for part_of_resp in resp[1:]:
if isinstance(part_of_resp, dict):
part_of_resp |= custom_header
return resp

return resp + (custom_header,)

return warn_user_in_headers
return decorate_endpoint_method


def deprecated_route_method_type(ns: Namespace, deprecated_method_type: str, use_instead: str):
"""
Calls deprecated_route decorator with specific message about deprecated method.

Usage:
class Endpoint(Resource):
...
@deprecated_route_method_type(foo_ns, "POST", "PUT")
...
def get():
return {"scary": "BOO!"}

Args:
ns: flask-restx Namespace
deprecated_method_type: method enum e.g. POST
use_instead: method user should use instead
"""
def call_deprecated_endpoint_method(endpoint_method):
msg = f"Use {use_instead} method instead of {deprecated_method_type}"
return deprecated_route_method(ns, msg)(endpoint_method)
return call_deprecated_endpoint_method


def restx_editable_copr(endpoint_method):
"""
Raises an exception if user don't have permissions for editing Copr repo.
Order matters! If flask.g.user is None then this will fail! If used with
@api_login_required it has to be called after it:

@api_login_required
@restx_editable_copr
...
"""
@wraps(endpoint_method)
def editable_copr_getter(self, ownername, projectname):
copr = _check_if_user_can_edit_copr(ownername, projectname)
return endpoint_method(self, copr)
return editable_copr_getter


def restx_pagination(endpoint_method):
"""
Validates pagination arguments and converts pagination parameters from query to
kwargs.
"""
@wraps(endpoint_method)
def create_pagination(self, *args, **kwargs):
kwargs = _shared_pagination_wrapper(**kwargs)
return endpoint_method(self, *args, **kwargs)
return create_pagination
10 changes: 3 additions & 7 deletions frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_builds.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
from coprs.exceptions import (BadRequest, AccessRestricted)
from coprs.views.misc import api_login_required
from coprs.views.apiv3_ns import apiv3_ns, api, rename_fields_helper
from coprs.views.apiv3_ns.schema import (
build_model,
get_build_params,
)
from coprs.views.apiv3_ns.schema.schemas import build_model
from coprs.views.apiv3_ns.schema.docs import get_build_docs
from coprs.logic.complex_logic import ComplexLogic
from coprs.logic.builds_logic import BuildsLogic
from coprs.logic.coprs_logic import CoprDirsLogic
Expand All @@ -38,8 +36,6 @@
from .json2form import get_form_compatible_data




Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A leftover change here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just syntax fix since it has 4 new lines instead of 2

apiv3_builds_ns = Namespace("build", description="Builds")
api.add_namespace(apiv3_builds_ns)

Expand Down Expand Up @@ -95,7 +91,7 @@ def render_build(build):
@apiv3_builds_ns.route("/<int:build_id>")
class GetBuild(Resource):

@apiv3_builds_ns.doc(params=get_build_params)
@apiv3_builds_ns.doc(params=get_build_docs)
@apiv3_builds_ns.marshal_with(build_model)
def get(self, build_id):
"""
Expand Down
49 changes: 20 additions & 29 deletions frontend/coprs_frontend/coprs/views/apiv3_ns/apiv3_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,16 @@
UnknownSourceTypeException,
InvalidForm,
)
from coprs.views.misc import api_login_required
from coprs.views.misc import api_login_required, restx_api_login_required
from coprs import db, models, forms, helpers
from coprs.views.apiv3_ns import apiv3_ns, api, rename_fields_helper
from coprs.views.apiv3_ns.schema import (
from coprs.views.apiv3_ns import apiv3_ns, api, rename_fields_helper, query_to_parameters
from coprs.views.apiv3_ns.schema.schemas import (
package_model,
add_package_params,
edit_package_params,
get_package_parser,
add_package_parser,
edit_package_parser,
package_get_params,
package_add_input_model,
package_edit_input_model,
)
from coprs.views.apiv3_ns.schema.docs import add_package_docs, edit_package_docs
from coprs.logic.packages_logic import PackagesLogic

# @TODO if we need to do this on several places, we should figure a better way to do it
Expand Down Expand Up @@ -110,25 +109,21 @@ def get_arg_to_bool(argument):

@apiv3_packages_ns.route("/")
class GetPackage(Resource):
parser = get_package_parser()

@apiv3_packages_ns.expect(parser)
@query_to_parameters
@apiv3_packages_ns.doc(params=package_get_params)
@apiv3_packages_ns.marshal_with(package_model)
def get(self):
def get(self, ownername, projectname, packagename, with_latest_build=False,
with_latest_succeeded_build=False):
"""
Get a package
Get a single package from a Copr project.
"""
args = self.parser.parse_args()
with_latest_build = args.with_latest_build
with_latest_succeeded_build = args.with_latest_succeeded_build

copr = get_copr(args.ownername, args.projectname)
copr = get_copr(ownername, projectname)
try:
package = PackagesLogic.get(copr.id, args.packagename)[0]
package = PackagesLogic.get(copr.id, packagename)[0]
except IndexError as ex:
msg = ("No package with name {name} in copr {copr}"
.format(name=args.packagename, copr=copr.name))
.format(name=packagename, copr=copr.name))
raise ObjectNotFound(msg) from ex
return to_dict(package, with_latest_build, with_latest_succeeded_build)

Expand Down Expand Up @@ -171,11 +166,9 @@ def get_package_list(ownername, projectname, with_latest_build=False,

@apiv3_packages_ns.route("/add/<ownername>/<projectname>/<package_name>/<source_type_text>")
class PackageAdd(Resource):
parser = add_package_parser()

@api_login_required
@apiv3_packages_ns.doc(params=add_package_params)
@apiv3_packages_ns.expect(parser)
@restx_api_login_required
@apiv3_packages_ns.doc(params=add_package_docs)
@apiv3_packages_ns.expect(package_add_input_model)
@apiv3_packages_ns.marshal_with(package_model)
def post(self, ownername, projectname, package_name, source_type_text):
"""
Expand All @@ -195,11 +188,9 @@ def post(self, ownername, projectname, package_name, source_type_text):
@apiv3_packages_ns.route("/edit/<ownername>/<projectname>/<package_name>/")
@apiv3_packages_ns.route("/edit/<ownername>/<projectname>/<package_name>/<source_type_text>")
class PackageEdit(Resource):
parser = edit_package_parser()

@api_login_required
@apiv3_packages_ns.doc(params=edit_package_params)
@apiv3_packages_ns.expect(parser)
@restx_api_login_required
@apiv3_packages_ns.doc(params=edit_package_docs)
@apiv3_packages_ns.expect(package_edit_input_model)
@apiv3_packages_ns.marshal_with(package_model)
def post(self, ownername, projectname, package_name, source_type_text=None):
"""
Expand Down
Loading