From 8873f0944a27683200d228ee23986ed659c666fa Mon Sep 17 00:00:00 2001 From: Adrian Moennich Date: Tue, 1 Jul 2025 13:12:16 +0200 Subject: [PATCH 1/4] Update environment, deps, packaging, etc --- .github/workflows/docs.yml | 10 +-- .github/workflows/tests.yml | 12 ++-- pyproject.toml | 41 +++++++++++++ requirements.in | 23 +++---- requirements.txt | 107 +++++++++++++++------------------ setup.py | 44 -------------- tox.ini | 8 +-- ursh/cli/core.py | 2 +- ursh/core/app.py | 2 +- ursh/schemas.py | 4 +- ursh/testing/conftest.py | 5 +- ursh/testing/test_resources.py | 4 +- ursh/util/db.py | 19 +++++- 13 files changed, 143 insertions(+), 138 deletions(-) create mode 100644 pyproject.toml delete mode 100644 setup.py diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 24a1af9..fb3e8c0 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -9,13 +9,13 @@ jobs: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - - uses: actions/setup-python@v2 + - uses: actions/setup-python@v5 with: python-version: 3.9 - - uses: actions/setup-node@v2 + - uses: actions/setup-node@v4 with: node-version: 14.x @@ -30,7 +30,7 @@ jobs: run: echo "::set-output name=dir::$(pip cache dir)" - name: Cache pip - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ${{ steps.pip-cache.outputs.dir }} key: pip|${{ runner.os }}|3.9|${{ hashFiles('setup.*') }} @@ -45,7 +45,7 @@ jobs: npx redoc-cli bundle openapi-spec.json -o api-docs/index.html - name: Deploy to GitHub Pages - uses: crazy-max/ghaction-github-pages@v2 + uses: crazy-max/ghaction-github-pages@v4 with: target_branch: gh-pages build_dir: api-docs diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index feeefaa..295e579 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,14 +9,14 @@ on: jobs: tests: name: ${{ matrix.name }} - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest strategy: fail-fast: false matrix: include: - - {name: Style, python: '3.9', tox: style} - - {name: '3.9', python: '3.9', tox: py39, postgres: true} + - {name: Style, python: '3.12', tox: style} + - {name: '3.12', python: '3.12', tox: py312, postgres: true} services: postgres: @@ -28,9 +28,9 @@ jobs: options: --health-cmd pg_isready --health-interval 5s --health-timeout 5s --health-retries 10 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - - uses: actions/setup-python@v2 + - uses: actions/setup-python@v5 with: python-version: ${{ matrix.python }} @@ -45,7 +45,7 @@ jobs: run: echo "::set-output name=dir::$(pip cache dir)" - name: Cache pip - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ${{ steps.pip-cache.outputs.dir }} key: pip|${{ runner.os }}|${{ matrix.python }}|${{ hashFiles('setup.*') }} diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..630edd6 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,41 @@ +[project] +name = 'ursh' +dynamic = ['version', 'dependencies'] +description = 'A URL shortening microservice' +readme = 'README.md' +license = 'MIT' +authors = [{ name = 'Indico Team', email = 'indico-team@cern.ch' }] +classifiers = [ + 'Environment :: Web Environment', + 'Framework :: Flask', + 'License :: OSI Approved :: MIT License', + 'Programming Language :: Python :: 3.12', +] +requires-python = '~=3.12' + +[project.scripts] +ursh = 'ursh.cli.core:cli' + +[project.optional-dependencies] +dev = ['pytest', 'isort'] + +[project.urls] +Issues = 'https://github.com/indico/ursh/issues' +GitHub = 'https://github.com/indico/ursh' + +[build-system] +requires = ['hatchling==1.27.0', 'hatch-requirements-txt==0.4.1'] +build-backend = 'hatchling.build' + +[tool.hatch] +version = { path = 'ursh/__init__.py' } + +[tool.hatch.metadata.hooks.requirements_txt] +files = ['requirements.txt'] + +[tool.hatch.build] +packages = ['ursh'] + +[tool.uv] +# uv does not know about dynamic metadata so `pip install -e .` should always reinstall +reinstall-package = ['ursh'] diff --git a/requirements.in b/requirements.in index 1952275..7932015 100644 --- a/requirements.in +++ b/requirements.in @@ -1,14 +1,15 @@ -apispec-webframeworks==0.5.2 -flask-apispec==0.8.8 -flask-marshmallow==0.12.0 +apispec-webframeworks +click +flask +flask-apispec +flask-marshmallow flask-shell-ipython -flask-sqlalchemy==2.4.1 -flask==1.1.2 -marshmallow-sqlalchemy==0.23.0 -markupsafe<2.1 -itsdangerous<2.1 -psycopg2 +flask-sqlalchemy +ipython<9 +marshmallow-sqlalchemy +marshmallow<4 +psycopg2-binary pyyaml sqlalchemy-utc -sqlalchemy -werkzeug==1.0.1 +sqlalchemy<2 +werkzeug diff --git a/requirements.txt b/requirements.txt index 97336f1..e2bf9c1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,97 +1,93 @@ -# -# This file is autogenerated by pip-compile with python 3.9 -# To update, run: -# -# pip-compile -# -apispec[yaml]==5.1.1 +# This file was autogenerated by uv via the following command: +# uv pip compile requirements.in -o requirements.txt +apispec==6.8.2 # via # apispec-webframeworks # flask-apispec -apispec-webframeworks==0.5.2 +apispec-webframeworks==1.2.0 # via -r requirements.in -asttokens==2.0.5 +asttokens==3.0.0 # via stack-data -backcall==0.2.0 - # via ipython -click==8.0.4 +blinker==1.9.0 + # via flask +click==8.2.1 # via + # -r requirements.in # flask # flask-shell-ipython -decorator==5.1.1 +decorator==5.2.1 # via ipython -executing==0.8.3 +executing==2.2.0 # via stack-data -flask==1.1.2 +flask==3.1.1 # via # -r requirements.in # flask-apispec # flask-marshmallow # flask-shell-ipython # flask-sqlalchemy -flask-apispec==0.8.8 +flask-apispec==0.11.4 # via -r requirements.in -flask-marshmallow==0.12.0 +flask-marshmallow==1.3.0 # via -r requirements.in -flask-shell-ipython==0.4.1 +flask-shell-ipython==0.5.3 # via -r requirements.in -flask-sqlalchemy==2.4.1 +flask-sqlalchemy==3.0.5 # via -r requirements.in -greenlet==1.1.2 +greenlet==3.2.3 # via sqlalchemy -ipython==8.1.1 - # via flask-shell-ipython -itsdangerous==2.0.1 +ipython==8.37.0 # via # -r requirements.in - # flask -jedi==0.18.1 + # flask-shell-ipython +itsdangerous==2.2.0 + # via flask +jedi==0.19.2 # via ipython -jinja2==3.0.3 +jinja2==3.1.6 # via flask -markupsafe==2.0.1 +markupsafe==3.0.2 # via - # -r requirements.in + # flask # jinja2 -marshmallow==3.15.0 + # werkzeug +marshmallow==3.26.1 # via + # -r requirements.in # flask-apispec # flask-marshmallow # marshmallow-sqlalchemy # webargs -marshmallow-sqlalchemy==0.23.0 +marshmallow-sqlalchemy==1.4.2 # via -r requirements.in -matplotlib-inline==0.1.3 +matplotlib-inline==0.1.7 # via ipython -packaging==21.3 - # via marshmallow -parso==0.8.3 +packaging==25.0 + # via + # apispec + # marshmallow + # webargs +parso==0.8.4 # via jedi -pexpect==4.8.0 +pexpect==4.9.0 # via ipython -pickleshare==0.7.5 +prompt-toolkit==3.0.51 # via ipython -prompt-toolkit==3.0.28 - # via ipython -psycopg2==2.9.3 +psycopg2-binary==2.9.10 # via -r requirements.in ptyprocess==0.7.0 # via pexpect -pure-eval==0.2.2 +pure-eval==0.2.3 # via stack-data -pygments==2.11.2 +pygments==2.19.2 # via ipython -pyparsing==3.0.7 - # via packaging -pyyaml==6.0 +pyyaml==6.0.2 # via # -r requirements.in # apispec -six==1.16.0 - # via - # flask-apispec - # flask-marshmallow -sqlalchemy==1.4.32 +setuptools==80.9.0 + # via sqlalchemy-utc +sqlalchemy==1.4.54 # via # -r requirements.in # flask-sqlalchemy @@ -99,20 +95,17 @@ sqlalchemy==1.4.32 # sqlalchemy-utc sqlalchemy-utc==0.14.0 # via -r requirements.in -stack-data==0.2.0 +stack-data==0.6.3 # via ipython -traitlets==5.1.1 +traitlets==5.14.3 # via # ipython # matplotlib-inline -wcwidth==0.2.5 +wcwidth==0.2.13 # via prompt-toolkit -webargs==5.5.3 +webargs==8.7.0 # via flask-apispec -werkzeug==1.0.1 +werkzeug==3.1.3 # via # -r requirements.in # flask - -# The following packages are considered to be unsafe in a requirements file: -# setuptools diff --git a/setup.py b/setup.py deleted file mode 100644 index d1160e3..0000000 --- a/setup.py +++ /dev/null @@ -1,44 +0,0 @@ -import ast -import os -import re -import sys - -from setuptools import find_packages, setup - - -_version_re = re.compile(r'__version__\s+=\s+(.*)') - - -def read_requirements_file(fname): - with open(fname, 'r') as f: - return [dep.strip() for dep in f.readlines() if not (dep.startswith('-') or '://' in dep)] - - -def get_requirements(): - return read_requirements_file(os.path.join(os.path.dirname(__file__), 'requirements.txt')) - - -with open('ursh/__init__.py', 'rb') as f: - version = str(ast.literal_eval(_version_re.search(f.read().decode('utf-8')).group(1))) - -needs_pytest = {'pytest', 'test', 'ptr'}.intersection(sys.argv) - -setup( - name='ursh', - version=version, - description='A URL shortening microservice', - url='https://github.com/indico/ursh', - download_url='https://github.com/indico/ursh', - zip_safe=False, - include_package_data=True, - packages=find_packages(), - install_requires=get_requirements(), - entry_points={ - 'console_scripts': {'ursh = ursh.cli.core:cli'} - }, - extras_require={ - 'tests': [ - 'pytest' - ] - }, -) diff --git a/tox.ini b/tox.ini index 2f54de2..6769ac0 100644 --- a/tox.ini +++ b/tox.ini @@ -1,10 +1,10 @@ [tox] envlist = - py39 + py312 style [testenv] -extras = tests +extras = dev commands = pytest -rs -v --color=yes setenv = URSH_CONFIG=/dev/null passenv = URSH_TEST_DATABASE_URI @@ -15,5 +15,5 @@ deps = flake8 isort commands = - isort --diff --check-only setup.py ursh - flake8 setup.py ursh/ + isort --diff --check-only ursh + flake8 ursh/ diff --git a/ursh/cli/core.py b/ursh/cli/core.py index 2ba0336..9dd5221 100644 --- a/ursh/cli/core.py +++ b/ursh/cli/core.py @@ -19,7 +19,7 @@ def _get_ursh_version(ctx, param, value): ctx.exit() -def _create_app(info): +def _create_app(): from ursh.core.app import create_app return create_app() diff --git a/ursh/core/app.py b/ursh/core/app.py index fe4fde0..f0e6380 100644 --- a/ursh/core/app.py +++ b/ursh/core/app.py @@ -101,7 +101,7 @@ def _register_handlers(app): @app.shell_context_processor def _extend_shell_context(): ctx = {'db': db} - ctx.update((name, cls) for name, cls in db.Model._decl_class_registry.items() if hasattr(cls, '__table__')) + ctx.update((mapper.class_.__name__, mapper.class_) for mapper in db.Model.registry.mappers) ctx.update((x, getattr(datetime, x)) for x in ('date', 'time', 'datetime', 'timedelta')) return ctx diff --git a/ursh/schemas.py b/ursh/schemas.py index 24c81b3..11b9552 100644 --- a/ursh/schemas.py +++ b/ursh/schemas.py @@ -1,11 +1,11 @@ import posixpath +from urllib.parse import urlparse from flask import current_app from flask_marshmallow import Schema from marshmallow import ValidationError, fields, validates from werkzeug.exceptions import BadRequest, HTTPException from werkzeug.routing import RequestRedirect -from werkzeug.urls import url_parse from ursh.models import ALPHABET_MANUAL, ALPHABET_RESTRICTED @@ -69,7 +69,7 @@ def validate_shortcut(self, data): def endpoint_for_url(url): - urldata = url_parse(url) + urldata = urlparse(url) adapter = current_app.url_map.bind(urldata.netloc) try: match = adapter.match(urldata.path) diff --git a/ursh/testing/conftest.py b/ursh/testing/conftest.py index de675ae..6db046a 100644 --- a/ursh/testing/conftest.py +++ b/ursh/testing/conftest.py @@ -106,15 +106,16 @@ def database(app, postgresql): your modifications are not persistent! """ app.config['SQLALCHEMY_DATABASE_URI'] = postgresql + app.extensions.pop('sqlalchemy', None) db_.init_app(app) - if 'URSH_TEST_DATABASE_URI' in os.environ == '1': + if 'URSH_TEST_DATABASE_URI' in os.environ and os.environ.get('URSH_TEST_DATABASE_HAS_TABLES') == '1': yield db_ return with app.app_context(): db_.create_all() yield db_ - db_.session.remove() with app.app_context(): + db_.session.remove() db_.drop_all() diff --git a/ursh/testing/test_resources.py b/ursh/testing/test_resources.py index a574531..91de794 100644 --- a/ursh/testing/test_resources.py +++ b/ursh/testing/test_resources.py @@ -1,8 +1,8 @@ import posixpath +from urllib.parse import urlparse from uuid import uuid4 import pytest -from werkzeug.urls import url_parse from ursh.models import URL, Token @@ -359,7 +359,7 @@ def test_create_url(db, app, client, data, expected, status): assert data.get('shortcut') == existing_shortcut assert parsed_response.get('short_url') is not None assert parsed_response.get('owner') is not None - shortcut = url_parse(parsed_response['short_url']).path.lstrip('/') + shortcut = urlparse(parsed_response['short_url']).path.lstrip('/') url = URL.query.filter_by(shortcut=shortcut).one_or_none() assert url is not None assert url.url == data['url'] diff --git a/ursh/util/db.py b/ursh/util/db.py index 4e0a8bd..a76d71b 100644 --- a/ursh/util/db.py +++ b/ursh/util/db.py @@ -1,7 +1,21 @@ import os from importlib import import_module +from importlib.util import find_spec -import pkg_resources + +def _get_package_root_path(import_name): + """Get the root path of a package. + + Returns ``None`` if the specified import name is invalid or + points to a module instead of a package. + """ + spec = find_spec(import_name) + if spec is None or not spec.parent: + # no parent if it's not a package (PEP 451) + return None + paths = spec.submodule_search_locations + assert len(paths) == 1 + return paths[0] def import_all_models(package_name): @@ -14,8 +28,7 @@ def import_all_models(package_name): :param package_name: Top-level package name to scan for models. """ - distribution = pkg_resources.get_distribution(package_name) - package_root = os.path.join(distribution.location, package_name) + package_root = _get_package_root_path(package_name) modules = [] for root, dirs, files in os.walk(package_root): if os.path.basename(root) == 'models': From b6cfa8690815c83a07d7b1ee10f9bc2e87fa8cd6 Mon Sep 17 00:00:00 2001 From: Adrian Moennich Date: Tue, 1 Jul 2025 16:41:29 +0200 Subject: [PATCH 2/4] Fix tests passing data in wrong location --- ursh/testing/test_resources.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/ursh/testing/test_resources.py b/ursh/testing/test_resources.py index 91de794..08fec3d 100644 --- a/ursh/testing/test_resources.py +++ b/ursh/testing/test_resources.py @@ -345,11 +345,11 @@ def test_create_url(db, app, client, data, expected, status): existing_shortcut = '' if data.get('allow_reuse'): - existing = client.post('/api/urls/', query_string={'url': 'http://existing.com'}, headers=auth) + existing = client.post('/api/urls/', json={'url': 'http://existing.com'}, headers=auth) existing = existing.get_json() existing_short_url = existing.get('short_url') assert existing_short_url is not None - response = client.post('/api/urls/', query_string=data, headers=auth) + response = client.post('/api/urls/', json=data, headers=auth) parsed_response = response.get_json() for key, value in expected.items(): @@ -416,8 +416,8 @@ def test_create_url(db, app, client, data, expected, status): def test_put_url(db, client, name, data, expected, status): auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) - client.put('/api/urls/i-exist', query_string={'url': 'http://example.com'}, headers=auth) - response = client.put(f'/api/urls/{name}', query_string=data, headers=auth) + client.put('/api/urls/i-exist', json={'url': 'http://example.com'}, headers=auth) + response = client.put(f'/api/urls/{name}', json=data, headers=auth) parsed_response = response.get_json() assert response.status_code == status @@ -465,9 +465,9 @@ def test_patch_url(db, client, shortcut, data, expected, status): auth1 = make_auth(db, 'non-admin-1', is_admin=False, is_blocked=False) auth2 = make_auth(db, 'non-admin-2', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', query_string={'url': 'http://example.com'}, headers=auth1) - client.put('/api/urls/def', query_string={'url': 'http://example.com'}, headers=auth2) - response = client.patch(f'/api/urls/{shortcut}', query_string=data, headers=auth1) + client.put('/api/urls/abc', json={'url': 'http://example.com'}, headers=auth1) + client.put('/api/urls/def', json={'url': 'http://example.com'}, headers=auth2) + response = client.patch(f'/api/urls/{shortcut}', json=data, headers=auth1) parsed_response = response.get_json() assert response.status_code == status @@ -499,8 +499,8 @@ def test_patch_url(db, client, shortcut, data, expected, status): def test_delete_url(db, client, shortcut, expected, status): auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', query_string={'url': 'http://example.com'}, headers=auth) - client.put('/api/urls/def', query_string={'url': 'http://example.com'}, headers=auth) + client.put('/api/urls/abc', json={'url': 'http://example.com'}, headers=auth) + client.put('/api/urls/def', json={'url': 'http://example.com'}, headers=auth) response = client.delete(f'/api/urls/{shortcut}', headers=auth) assert response.status_code == status @@ -574,10 +574,10 @@ def test_delete_url(db, client, shortcut, expected, status): def test_get_url(db, client, url, data, expected, status): auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', query_string={'url': 'http://example.com', 'meta.author': 'me'}, headers=auth) - client.put('/api/urls/def', query_string={'url': 'http://example.com', 'meta.owner': 'all', 'meta.a': 'b'}, + client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta.author': 'me'}, headers=auth) + client.put('/api/urls/def', json={'url': 'http://example.com', 'meta.owner': 'all', 'meta.a': 'b'}, headers=auth) - client.put('/api/urls/ghi', query_string={'url': 'http://cern.ch', 'meta.author': 'me'}, headers=auth) + client.put('/api/urls/ghi', json={'url': 'http://cern.ch', 'meta.author': 'me'}, headers=auth) response = client.get(url, query_string=data, headers=auth) parsed_response = response.get_json() @@ -604,11 +604,11 @@ def test_get_admin_all(db, client): non_admin_auth1 = make_auth(db, 'non-admin-1', is_admin=False, is_blocked=False) non_admin_auth2 = make_auth(db, 'non-admin-2', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', query_string={'url': 'http://example.com', 'meta.author': 'me'}, + client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta.author': 'me'}, headers=non_admin_auth1) - client.put('/api/urls/def', query_string={'url': 'http://example.com', 'meta.owner': 'all', 'meta.a': 'b'}, + client.put('/api/urls/def', json={'url': 'http://example.com', 'meta.owner': 'all', 'meta.a': 'b'}, headers=non_admin_auth2) - client.put('/api/urls/ghi', query_string={'url': 'http://cern.ch', 'meta.author': 'me'}, headers=admin_auth) + client.put('/api/urls/ghi', json={'url': 'http://cern.ch', 'meta.author': 'me'}, headers=admin_auth) response = client.get('/api/urls/', query_string={'all': True}, headers=admin_auth) parsed_response = response.get_json() expected = [{'meta': {"author": "me"}, 'short_url': posixpath.join('http://localhost:5000/', 'abc'), @@ -633,10 +633,10 @@ def test_other_user(db, client, method): non_admin_auth1 = make_auth(db, 'non-admin-1', is_admin=False, is_blocked=False) non_admin_auth2 = make_auth(db, 'non-admin-2', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', query_string={'url': 'http://example.com', 'meta.author': 'me'}, + client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta.author': 'me'}, headers=non_admin_auth1) method = getattr(client, method) - response = method('/api/urls/abc', query_string={}, headers=non_admin_auth2) + response = method('/api/urls/abc', headers=non_admin_auth2) expected = {'error': {'code': 'insufficient-permissions', 'description': 'You are not allowed to make this request'}, 'status': 403} From 3eb73be9de750ac16f7a7bae1ae3b8bbc4b18381 Mon Sep 17 00:00:00 2001 From: Adrian Moennich Date: Tue, 1 Jul 2025 17:39:20 +0200 Subject: [PATCH 3/4] Fix tests Cleanup all the mess with field locations, stuff being in the wrong location, etc. --- ursh/blueprints/api/resources.py | 25 +++++----- ursh/schemas.py | 10 ++-- ursh/testing/test_resources.py | 78 ++++++++++++++++---------------- ursh/util/nested_query_parser.py | 5 +- 4 files changed, 62 insertions(+), 56 deletions(-) diff --git a/ursh/blueprints/api/resources.py b/ursh/blueprints/api/resources.py index ce494c3..a4b6a98 100644 --- a/ursh/blueprints/api/resources.py +++ b/ursh/blueprints/api/resources.py @@ -7,7 +7,7 @@ from ursh import db from ursh.models import URL, Token -from ursh.schemas import TokenSchema, URLSchemaManual, URLSchemaRestricted +from ursh.schemas import ShortcutSchemaManual, ShortcutSchemaRestricted, TokenSchema, URLSchema from ursh.util.decorators import admin_only, authorize_request_for_url, marshal_many_or_one @@ -214,7 +214,7 @@ def delete(self, api_key=None, **kwargs): @admin_only @marshal_many_or_one(TokenSchema, 'api_key', code=200) - @use_kwargs(TokenSchema) + @use_kwargs(TokenSchema, location='query') def get(self, api_key=None, **kwargs): """Obtain one or more tokens. --- @@ -302,8 +302,9 @@ class URLResource(MethodResource): 401: description: not authorized """ - @marshal_with(URLSchemaRestricted, code=201) - @use_kwargs(URLSchemaRestricted) + @marshal_with(URLSchema, code=201) + @use_kwargs(URLSchema) + @use_kwargs(ShortcutSchemaRestricted, location='view_args') def post(self, **kwargs): """Create a new URL object. --- @@ -369,9 +370,10 @@ def post(self, **kwargs): kwargs.get('meta', {})) return new_url, 201 - @use_kwargs(URLSchemaManual) + @use_kwargs(URLSchema) + @use_kwargs(ShortcutSchemaManual, location='view_args') @authorize_request_for_url - @marshal_with(URLSchemaManual, code=201) + @marshal_with(URLSchema, code=201) def put(self, shortcut=None, **kwargs): """Put a new URL object. --- @@ -445,9 +447,10 @@ def put(self, shortcut=None, **kwargs): kwargs.get('meta', {})) return new_url, 201 - @use_kwargs(URLSchemaManual) + @use_kwargs(URLSchema) + @use_kwargs(ShortcutSchemaManual, location='view_args') @authorize_request_for_url - @marshal_with(URLSchemaManual) + @marshal_with(URLSchema) def patch(self, shortcut=None, **kwargs): """Modify an existing URL object. --- @@ -518,7 +521,7 @@ def patch(self, shortcut=None, **kwargs): return url @authorize_request_for_url - @use_kwargs(URLSchemaManual) + @use_kwargs(ShortcutSchemaManual) def delete(self, shortcut=None, **kwargs): """Delete an existing URL object. --- @@ -561,8 +564,8 @@ def delete(self, shortcut=None, **kwargs): current_app.logger.info('URL deleted by %s: %s -> <%s>', g.token.name, url.shortcut, url.url) return Response(status=204) - @marshal_many_or_one(URLSchemaManual, 'shortcut', code=200) - @use_kwargs(URLSchemaManual) + @marshal_many_or_one(URLSchema, 'shortcut', code=200) + @use_kwargs(URLSchema, location='query') @authorize_request_for_url def get(self, shortcut=None, **kwargs): """Obtain one or more URL objects. diff --git a/ursh/schemas.py b/ursh/schemas.py index 11b9552..c70e2ce 100644 --- a/ursh/schemas.py +++ b/ursh/schemas.py @@ -39,7 +39,7 @@ class URLSchema(SchemaBase): Note: use one of the sub-classes below for validation, depending on the shortcut requirements. """ - shortcut = fields.Str(location='view_args', description='The generated or manually set URL shortcut') + shortcut = fields.Str(description='The generated or manually set URL shortcut') url = fields.URL(description='The original URL (the short URL target)') short_url = fields.Method('_get_short_url', description='The short URL') meta = fields.Dict(description='Additional metadata (provided on short URL creation)') @@ -50,18 +50,22 @@ def _get_short_url(self, obj): return posixpath.join(current_app.config['REDIRECTION_HOST'], obj.shortcut) -class URLSchemaManual(URLSchema): +class ShortcutSchemaManual(SchemaBase): """Validator for user-specified shortcuts (i.e. all requests except POST).""" + shortcut = fields.Str(description='The generated or manually set URL shortcut') + @validates('shortcut') def validate_shortcut(self, data): if not validate_shortcut(data, restricted=False): raise ValidationError('Invalid value.') -class URLSchemaRestricted(URLSchema): +class ShortcutSchemaRestricted(SchemaBase): """Validator for auto-generated shortcuts (i.e. POST requests).""" + shortcut = fields.Str(description='The generated or manually set URL shortcut') + @validates('shortcut') def validate_shortcut(self, data): if not validate_shortcut(data, restricted=True): diff --git a/ursh/testing/test_resources.py b/ursh/testing/test_resources.py index 08fec3d..60b9850 100644 --- a/ursh/testing/test_resources.py +++ b/ursh/testing/test_resources.py @@ -68,7 +68,7 @@ def test_create_token(db, client, admin, data, expected, status_code): auth = make_auth(db, 'admin', is_admin=True, is_blocked=False) else: auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) - response = client.post('/api/tokens/', data=data, headers=auth) + response = client.post('/api/tokens/', json=data, headers=auth) parsed_response = response.get_json() assert response.status_code == status_code @@ -83,8 +83,8 @@ def test_create_token(db, client, admin, data, expected, status_code): def test_create_token_name_exists(db, client): auth = make_auth(db, 'admin', is_admin=True, is_blocked=False) - client.post('/api/tokens/', data={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=auth) - response = client.post('/api/tokens/', data={'name': 'abc'}, headers=auth) + client.post('/api/tokens/', json={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=auth) + response = client.post('/api/tokens/', json={'name': 'abc'}, headers=auth) expected = {'error': {'args': ['name'], 'code': 'conflict', 'description': 'Token with name exists'}, 'status': 409} assert response.status_code == 409 @@ -156,12 +156,12 @@ def test_get_tokens(db, client, admin, url, data, expected, status): else: auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) - client.post('/api/tokens/', data={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=auth) - client.post('/api/tokens/', data={'name': 'abcd', 'callback_url': 'http://a.ch', + client.post('/api/tokens/', json={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=auth) + client.post('/api/tokens/', json={'name': 'abcd', 'callback_url': 'http://a.ch', 'is_admin': True, 'is_blocked': True}, headers=auth) - client.post('/api/tokens/', data={'name': 'abcd1', 'callback_url': 'http://a.ch', + client.post('/api/tokens/', json={'name': 'abcd1', 'callback_url': 'http://a.ch', 'is_admin': False, 'is_blocked': True}, headers=auth) - client.post('/api/tokens/', data={'name': 'abcde', 'callback_url': 'http://b.ch'}, headers=auth) + client.post('/api/tokens/', json={'name': 'abcde', 'callback_url': 'http://b.ch'}, headers=auth) response = client.get(url, query_string=data, headers=auth) parsed_response = response.get_json() @@ -234,15 +234,15 @@ def test_token_patch(db, client, admin, name, data, expected, status): else: auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) - client.post('/api/tokens/', data={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=auth) - client.post('/api/tokens/', data={'name': 'abcd', 'callback_url': 'http://a.ch', + client.post('/api/tokens/', json={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=auth) + client.post('/api/tokens/', json={'name': 'abcd', 'callback_url': 'http://a.ch', 'is_admin': True, 'is_blocked': True}, headers=auth) - client.post('/api/tokens/', data={'name': 'abcd1', 'callback_url': 'http://a.ch', + client.post('/api/tokens/', json={'name': 'abcd1', 'callback_url': 'http://a.ch', 'is_admin': False, 'is_blocked': True}, headers=auth) - client.post('/api/tokens/', data={'name': 'abcde', 'callback_url': 'http://b.ch'}, headers=auth) + client.post('/api/tokens/', json={'name': 'abcde', 'callback_url': 'http://b.ch'}, headers=auth) token = Token.query.filter_by(name=name).one_or_none() uuid = token.api_key if token else name # assume we want to use the name as an API key if it is not present - response = client.patch(f'/api/tokens/{uuid}', query_string=data, headers=auth) + response = client.patch(f'/api/tokens/{uuid}', json=data, headers=auth) parsed_response = response.get_json() assert response.status_code == status @@ -270,7 +270,7 @@ def test_token_delete(db, client, admin, expected, status): auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) admin_auth = make_auth(db, 'admin_', is_admin=True, is_blocked=False) - client.post('/api/tokens/', data={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=admin_auth) + client.post('/api/tokens/', json={'name': 'abc', 'callback_url': 'http://cern.ch'}, headers=admin_auth) api_key = Token.query.filter_by(name='abc').one_or_none().api_key response = client.delete(f'/api/tokens/{api_key}', headers=auth) @@ -317,8 +317,8 @@ def test_blocked_or_unauthorized(db, client, method, url, data, blocked): ), ( # everything works right, a new url is created with metadata - {'url': 'http://cern.ch', 'meta.author': 'me', 'meta.a': False}, - {'meta': {"author": "me", "a": "False"}, 'url': 'http://cern.ch'}, + {'url': 'http://cern.ch', 'meta': {'author': 'me', 'a': False}}, + {'meta': {'author': 'me', 'a': False}, 'url': 'http://cern.ch'}, 201 ), ( @@ -371,45 +371,45 @@ def test_create_url(db, app, client, data, expected, status): ( # everything goes right "my-short-url", - {'url': 'http://google.com', 'meta.author': 'me'}, - {'meta': {"author": "me"}, 'short_url': posixpath.join('http://localhost:5000/', 'my-short-url'), + {'url': 'http://google.com', 'meta': {'author': 'me'}}, + {'meta': {'author': 'me'}, 'short_url': posixpath.join('http://localhost:5000/', 'my-short-url'), 'url': 'http://google.com'}, 201 ), ( # invalid url "my-short-url", - {'url': 'google.com', 'meta.author': 'me'}, + {'url': 'google.com', 'meta': {'author': 'me'}}, {'error': {'code': 'validation-error', 'messages': {'url': ['Not a valid URL.']}}, 'status': 400}, 400 ), ( # empty url "my-short-url", - {'url': '', 'meta.author': 'me'}, + {'url': '', 'meta': {'author': 'me'}}, {'error': {'code': 'validation-error', 'messages': {'url': ['Not a valid URL.']}}, 'status': 400}, 400 ), ( # url with invalid characters "my-short-url*", - {'url': 'https://google.com', 'meta.author': 'me'}, + {'url': 'https://google.com', 'meta': {'author': 'me'}}, {'error': {'code': 'validation-error', 'messages': {'shortcut': ['Invalid value.']}}, 'status': 400}, 400 ), ( # url with slash "my-short-url/i-look-suspicious", - {'url': 'https://google.com', 'meta.author': 'me'}, + {'url': 'https://google.com', 'meta': {'author': 'me'}}, {}, 404 ), ( # blacklisted URL "api", - {'url': '', 'meta.author': 'me'}, + {'url': 'https://google.com', 'meta': {'author': 'me'}}, {'error': {'code': 'validation-error', - 'messages': {'shortcut': ['Invalid value.'], 'url': ['Not a valid URL.']}}, 'status': 400}, + 'messages': {'shortcut': ['Invalid value.']}}, 'status': 400}, 400 ), ]) @@ -434,7 +434,7 @@ def test_put_url(db, client, name, data, expected, status): ( # everything goes right "abc", - {'meta.author': 'me', 'url': 'http://example.com'}, + {'meta': {'author': 'me'}, 'url': 'http://example.com'}, {'meta': {"author": "me"}, 'short_url': posixpath.join('http://localhost:5000/', 'abc'), 'url': 'http://example.com'}, 200 @@ -442,21 +442,21 @@ def test_put_url(db, client, name, data, expected, status): ( # nonexistent shortcut "nonexistent", - {'meta.author': 'me', 'url': 'http://example.com'}, + {'meta': {'author': 'me'}, 'url': 'http://example.com'}, {'error': {'args': ['shortcut'], 'code': 'not-found', 'description': 'Shortcut does not exist'}, 'status': 404}, 404 ), ( # invalid url "abc", - {'meta.author': 'me', 'url': 'example.com'}, + {'meta': {'author': 'me'}, 'url': 'example.com'}, {'error': {'code': 'validation-error', 'messages': {'url': ['Not a valid URL.']}}, 'status': 400}, 400 ), ( # empty url "abc", - {'meta.author': 'me', 'url': ''}, + {'meta': {'author': 'me'}, 'url': ''}, {'error': {'code': 'validation-error', 'messages': {'url': ['Not a valid URL.']}}, 'status': 400}, 400 ), @@ -574,10 +574,10 @@ def test_delete_url(db, client, shortcut, expected, status): def test_get_url(db, client, url, data, expected, status): auth = make_auth(db, 'non-admin', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta.author': 'me'}, headers=auth) - client.put('/api/urls/def', json={'url': 'http://example.com', 'meta.owner': 'all', 'meta.a': 'b'}, + client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta': {'author': 'me'}}, headers=auth) + client.put('/api/urls/def', json={'url': 'http://example.com', 'meta': {'owner': 'all', 'a': 'b'}}, headers=auth) - client.put('/api/urls/ghi', json={'url': 'http://cern.ch', 'meta.author': 'me'}, headers=auth) + client.put('/api/urls/ghi', json={'url': 'http://cern.ch', 'meta': {'author': 'me'}}, headers=auth) response = client.get(url, query_string=data, headers=auth) parsed_response = response.get_json() @@ -585,12 +585,12 @@ def test_get_url(db, client, url, data, expected, status): if type(expected) == list: parsed_response = sorted(parsed_response, key=lambda k: k['short_url']) expected = sorted(expected, key=lambda k: k['short_url']) - for expected_token, returned_token in zip(expected, parsed_response): - for key, value in expected_token.items(): - assert value == returned_token[key] + for expected_url, returned_url in zip(expected, parsed_response): + for key, value in expected_url.items(): + assert value == returned_url[key] if status == 200: - assert returned_token.get('owner') is not None - assert returned_token.get('url') is not None + assert returned_url.get('owner') is not None + assert returned_url.get('url') is not None else: for key, value in expected.items(): assert value == parsed_response[key] @@ -604,11 +604,11 @@ def test_get_admin_all(db, client): non_admin_auth1 = make_auth(db, 'non-admin-1', is_admin=False, is_blocked=False) non_admin_auth2 = make_auth(db, 'non-admin-2', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta.author': 'me'}, + client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta': {'author': 'me'}}, headers=non_admin_auth1) - client.put('/api/urls/def', json={'url': 'http://example.com', 'meta.owner': 'all', 'meta.a': 'b'}, + client.put('/api/urls/def', json={'url': 'http://example.com', 'meta': {'owner': 'all', 'a': 'b'}}, headers=non_admin_auth2) - client.put('/api/urls/ghi', json={'url': 'http://cern.ch', 'meta.author': 'me'}, headers=admin_auth) + client.put('/api/urls/ghi', json={'url': 'http://cern.ch', 'meta': {'author': 'me'}}, headers=admin_auth) response = client.get('/api/urls/', query_string={'all': True}, headers=admin_auth) parsed_response = response.get_json() expected = [{'meta': {"author": "me"}, 'short_url': posixpath.join('http://localhost:5000/', 'abc'), @@ -633,7 +633,7 @@ def test_other_user(db, client, method): non_admin_auth1 = make_auth(db, 'non-admin-1', is_admin=False, is_blocked=False) non_admin_auth2 = make_auth(db, 'non-admin-2', is_admin=False, is_blocked=False) - client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta.author': 'me'}, + client.put('/api/urls/abc', json={'url': 'http://example.com', 'meta': {'author': 'me'}}, headers=non_admin_auth1) method = getattr(client, method) response = method('/api/urls/abc', headers=non_admin_auth2) diff --git a/ursh/util/nested_query_parser.py b/ursh/util/nested_query_parser.py index 043ff28..dd5eebb 100644 --- a/ursh/util/nested_query_parser.py +++ b/ursh/util/nested_query_parser.py @@ -4,13 +4,12 @@ import re -from webargs import core from webargs.flaskparser import FlaskParser class NestedQueryParser(FlaskParser): - def parse_querystring(self, req, name, field): - return core.get_value(_structure_dict(req.args), name, field) + def load_querystring(self, req, schema): + return _structure_dict(req.args) def _structure_dict(dict_): From 5a67f74194ae902541697f4e15d74f9bc9886587 Mon Sep 17 00:00:00 2001 From: Adrian Moennich Date: Tue, 1 Jul 2025 18:44:13 +0200 Subject: [PATCH 4/4] Make flake8 happy --- ursh/testing/test_resources.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ursh/testing/test_resources.py b/ursh/testing/test_resources.py index 60b9850..eaea096 100644 --- a/ursh/testing/test_resources.py +++ b/ursh/testing/test_resources.py @@ -166,7 +166,7 @@ def test_get_tokens(db, client, admin, url, data, expected, status): parsed_response = response.get_json() assert response.status_code == status - if type(expected) == list: + if isinstance(expected, list): parsed_response = sorted(parsed_response, key=lambda k: k['name']) expected = sorted(expected, key=lambda k: k['name']) for expected_token, returned_token in zip(expected, parsed_response): @@ -582,7 +582,7 @@ def test_get_url(db, client, url, data, expected, status): parsed_response = response.get_json() assert response.status_code == status - if type(expected) == list: + if isinstance(expected, list): parsed_response = sorted(parsed_response, key=lambda k: k['short_url']) expected = sorted(expected, key=lambda k: k['short_url']) for expected_url, returned_url in zip(expected, parsed_response):