From 77c8031d1b2a87ed7eb9e6525eb490cd2a2da6a9 Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Thu, 28 Sep 2023 16:54:28 +0200 Subject: [PATCH 1/2] Reset repo to pre-pydantic-v2 state. --- CHANGELOG.md | 46 ++++- environment.yml | 3 +- pyproject.toml | 2 +- quetz/_version.py | 2 +- quetz/authentication/oauth2.py | 8 +- quetz/db_models.py | 2 +- quetz/jobs/api.py | 4 +- quetz/jobs/rest_models.py | 24 +-- quetz/main.py | 31 ++-- quetz/metrics/rest_models.py | 6 +- ...3fb7d_update_scoped_api_key_uploader_id.py | 61 +++++++ quetz/rest_models.py | 139 ++++++---------- quetz/tests/api/test_api_keys.py | 6 +- quetz/tests/api/test_channels.py | 2 +- quetz/tests/api/test_main_packages.py | 157 ++++++++++++------ quetz/tests/test_docs.py | 16 ++ quetz/tests/test_jobs.py | 12 +- quetz/tests/test_mirror.py | 12 +- setup.cfg | 1 + 19 files changed, 348 insertions(+), 186 deletions(-) create mode 100644 quetz/migrations/versions/3ba25f23fb7d_update_scoped_api_key_uploader_id.py create mode 100644 quetz/tests/test_docs.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4174b4cf..9f8e83d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,50 @@ +## 0.10.1 + +([Full Changelog](https://github.com/mamba-org/quetz/compare/v0.10.0...ef6836a7c887dc97a89d8b5cce4472a03740b692)) + +### Bugs fixed + +- Fix oauth2 revoke functionality [#665](https://github.com/mamba-org/quetz/pull/665) ([@wolfv](https://github.com/wolfv)) +- Fix docs rest model [#661](https://github.com/mamba-org/quetz/pull/661) ([@mbestipa](https://github.com/mbestipa)) + +### Contributors to this release + +([GitHub contributors page for this release](https://github.com/mamba-org/quetz/graphs/contributors?from=2023-09-11&to=2023-09-28&type=c)) + +[@codecov-commenter](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Acodecov-commenter+updated%3A2023-09-11..2023-09-28&type=Issues) | [@mbestipa](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Ambestipa+updated%3A2023-09-11..2023-09-28&type=Issues) | [@wolfv](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Awolfv+updated%3A2023-09-11..2023-09-28&type=Issues) + + + +## 0.10.0 + +([Full Changelog](https://github.com/mamba-org/quetz/compare/v0.9.2...0854d442d1b20b7eb90e023eef27527b722fbcc6)) + +### Enhancements made + +- Fix postgres pool size [#657](https://github.com/mamba-org/quetz/pull/657) ([@beenje](https://github.com/beenje)) +- Migrate to Pydantic v2 [#656](https://github.com/mamba-org/quetz/pull/656) ([@beenje](https://github.com/beenje)) + +### Bugs fixed + +- Fix double upload bug and improve test [#663](https://github.com/mamba-org/quetz/pull/663) ([@AndreasAlbertQC](https://github.com/AndreasAlbertQC)) +- Add migration script for scoped API keys [#655](https://github.com/mamba-org/quetz/pull/655) ([@beenje](https://github.com/beenje)) +- Fix crash when uploading a package through scoped API key [#647](https://github.com/mamba-org/quetz/pull/647) ([@gabm](https://github.com/gabm)) +- Consider packages.conda for index update and channel mirroring [#638](https://github.com/mamba-org/quetz/pull/638) ([@YYYasin19](https://github.com/YYYasin19)) + +### Maintenance and upkeep improvements + +- Pin pydantic\<2 [#653](https://github.com/mamba-org/quetz/pull/653) ([@AndreasAlbertQC](https://github.com/AndreasAlbertQC)) +- Make upload routes consistent with each other [#635](https://github.com/mamba-org/quetz/pull/635) ([@AndreasAlbertQC](https://github.com/AndreasAlbertQC)) + +### Contributors to this release + +([GitHub contributors page for this release](https://github.com/mamba-org/quetz/graphs/contributors?from=2023-06-21&to=2023-09-11&type=c)) + +[@AndreasAlbertQC](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3AAndreasAlbertQC+updated%3A2023-06-21..2023-09-11&type=Issues) | [@beenje](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Abeenje+updated%3A2023-06-21..2023-09-11&type=Issues) | [@codecov-commenter](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Acodecov-commenter+updated%3A2023-06-21..2023-09-11&type=Issues) | [@gabm](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Agabm+updated%3A2023-06-21..2023-09-11&type=Issues) | [@janjagusch](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Ajanjagusch+updated%3A2023-06-21..2023-09-11&type=Issues) | [@wolfv](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Awolfv+updated%3A2023-06-21..2023-09-11&type=Issues) | [@YYYasin19](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3AYYYasin19+updated%3A2023-06-21..2023-09-11&type=Issues) + ## 0.9.2 ([Full Changelog](https://github.com/mamba-org/quetz/compare/v0.9.1...efd519fd840304fb73fe6fd31ee4ae7f010ab1a2)) @@ -22,8 +66,6 @@ [@AndreasAlbertQC](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3AAndreasAlbertQC+updated%3A2023-06-20..2023-06-21&type=Issues) | [@codecov-commenter](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Acodecov-commenter+updated%3A2023-06-20..2023-06-21&type=Issues) | [@rominf](https://github.com/search?q=repo%3Amamba-org%2Fquetz+involves%3Arominf+updated%3A2023-06-20..2023-06-21&type=Issues) - - ## 0.9.1 ([Full Changelog](https://github.com/mamba-org/quetz/compare/v0.9.0...13f588cc16560c78927e4b2406a77958a62d5ca6)) diff --git a/environment.yml b/environment.yml index 083b5a56..438ecc55 100644 --- a/environment.yml +++ b/environment.yml @@ -50,6 +50,7 @@ dependencies: - conda-content-trust - pyinstrument - pytest-asyncio - - pydantic <2 + - pytest-timeout + - pydantic >=2 - pip: - git+https://github.com/jupyter-server/jupyter_releaser.git@v2 diff --git a/pyproject.toml b/pyproject.toml index 95bd793c..729c22d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ check-imports = ["quetz"] ignore = ["W004"] [tool.tbump.version] -current = "0.9.2" +current = "0.10.1" regex = ''' (?P\d+)\.(?P\d+)\.(?P\d+) ((?Pa|b|rc|.dev)(?P\d+))? diff --git a/quetz/_version.py b/quetz/_version.py index 7e2da70f..5feb7a00 100644 --- a/quetz/_version.py +++ b/quetz/_version.py @@ -1,2 +1,2 @@ -version_info = (0, 9, 2, "", "") +version_info = (0, 10, 1, "", "") __version__ = '.'.join(filter(lambda s: len(s) > 0, map(str, version_info))) diff --git a/quetz/authentication/oauth2.py b/quetz/authentication/oauth2.py index 31b702cb..a1702d3a 100644 --- a/quetz/authentication/oauth2.py +++ b/quetz/authentication/oauth2.py @@ -29,9 +29,11 @@ async def login(self, request: Request): request, str(redirect_uri) ) - async def revoke(self, request): - client_id = self.client.client_id - return RedirectResponse(self.revoke_url.format(client_id=client_id)) + async def revoke(self): + client_id = self.authenticator.client_id + return RedirectResponse( + self.authenticator.revoke_url.format(client_id=client_id) + ) class OAuthAuthenticator(BaseAuthenticator): diff --git a/quetz/db_models.py b/quetz/db_models.py index 91c5db49..10ece686 100644 --- a/quetz/db_models.py +++ b/quetz/db_models.py @@ -55,7 +55,7 @@ class User(Base): 'Profile', uselist=False, back_populates='user', cascade="all,delete-orphan" ) - role = Column(String) + role = Column(String, nullable=True) @classmethod def find(cls, db, name): diff --git a/quetz/jobs/api.py b/quetz/jobs/api.py index 79cf087c..38579b2f 100644 --- a/quetz/jobs/api.py +++ b/quetz/jobs/api.py @@ -17,7 +17,7 @@ from quetz.rest_models import PaginatedResponse from .models import JobStatus, TaskStatus -from .rest_models import Job, JobBase, JobUpdateModel, Task +from .rest_models import Job, JobCreate, JobUpdateModel, Task api_router = APIRouter() @@ -44,7 +44,7 @@ def get_jobs( @api_router.post("/api/jobs", tags=["Jobs"], status_code=201, response_model=Job) def create_job( - job: JobBase, + job: JobCreate, dao: Dao = Depends(get_dao), auth: authorization.Rules = Depends(get_rules), ): diff --git a/quetz/jobs/rest_models.py b/quetz/jobs/rest_models.py index 64607b2d..c518a587 100644 --- a/quetz/jobs/rest_models.py +++ b/quetz/jobs/rest_models.py @@ -5,7 +5,7 @@ from typing import Optional from importlib_metadata import entry_points as get_entry_points -from pydantic import BaseModel, Field, validator +from pydantic import BaseModel, ConfigDict, Field, field_validator from . import handlers from .models import JobStatus, TaskStatus @@ -83,7 +83,6 @@ def parse_job_name(v): class JobBase(BaseModel): """New job spec""" - items_spec: str = Field(..., title='Item selector spec') manifest: str = Field(None, title='Name of the function') start_at: Optional[datetime] = Field( @@ -97,7 +96,8 @@ class JobBase(BaseModel): ), ) - @validator("manifest", pre=True) + @field_validator("manifest", mode="before") + @classmethod def validate_job_name(cls, function_name): if isinstance(function_name, bytes): return parse_job_name(function_name) @@ -107,6 +107,12 @@ def validate_job_name(cls, function_name): return function_name.encode('ascii') +class JobCreate(JobBase): + """Create job spec""" + + items_spec: str = Field(..., title='Item selector spec') + + class JobUpdateModel(BaseModel): """Modify job spec items (status and items_spec)""" @@ -123,10 +129,8 @@ class Job(JobBase): status: JobStatus = Field(None, title='Status of the job (running, paused, ...)') - items_spec: str = Field(None, title='Item selector spec') - - class Config: - orm_mode = True + items_spec: Optional[str] = Field(None, title='Item selector spec') + model_config = ConfigDict(from_attributes=True) class Task(BaseModel): @@ -136,12 +140,12 @@ class Task(BaseModel): created: datetime = Field(None, title='Created at') status: TaskStatus = Field(None, title='Status of the task (running, paused, ...)') - @validator("package_version", pre=True) + @field_validator("package_version", mode="before") + @classmethod def convert_package_version(cls, v): if v: return {'filename': v.filename, 'id': uuid.UUID(bytes=v.id).hex} else: return {} - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) diff --git a/quetz/main.py b/quetz/main.py index 88bd8fe6..e6f074c7 100644 --- a/quetz/main.py +++ b/quetz/main.py @@ -17,7 +17,6 @@ from typing import Awaitable, Callable, List, Optional, Tuple, Type import pydantic -import pydantic.error_wrappers import requests from fastapi import ( APIRouter, @@ -477,7 +476,7 @@ def delete_user( @api_router.get( "/users/{username}/role", - response_model=rest_models.UserRole, + response_model=rest_models.UserOptionalRole, tags=["users"], ) def get_user_role( @@ -732,7 +731,7 @@ def post_channel( detail="Cannot use both `includelist` and `excludelist` together.", ) - user_attrs = new_channel.dict(exclude_unset=True) + user_attrs = new_channel.model_dump(exclude_unset=True) if "size_limit" in user_attrs: auth.assert_set_channel_size_limit() @@ -789,7 +788,7 @@ def patch_channel( ): auth.assert_update_channel_info(channel.name) - user_attrs = channel_data.dict(exclude_unset=True) + user_attrs = channel_data.model_dump(exclude_unset=True) if "size_limit" in user_attrs: auth.assert_set_channel_size_limit() @@ -1064,7 +1063,7 @@ def get_package_versions( version_list = [] for version, profile, api_key_profile in version_profile_list: - version_data = rest_models.PackageVersion.from_orm(version) + version_data = rest_models.PackageVersion.model_validate(version) version_list.append(version_data) return version_list @@ -1089,7 +1088,7 @@ def get_paginated_package_versions( version_list = [] for version, profile, api_key_profile in version_profile_list['result']: - version_data = rest_models.PackageVersion.from_orm(version) + version_data = rest_models.PackageVersion.model_validate(version) version_list.append(version_data) return { @@ -1396,9 +1395,6 @@ async def post_upload( dest = os.path.join(condainfo.info["subdir"], filename) - body.seek(0) - await pkgstore.add_package_async(body, channel_name, dest) - package_name = str(condainfo.info.get("name")) package_data = rest_models.Package( name=package_name, @@ -1435,6 +1431,9 @@ async def post_upload( logger.debug(f"duplicate package '{package_name}' in channel '{channel_name}'") raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Duplicate") + body.seek(0) + await pkgstore.add_package_async(body, channel_name, dest) + pm.hook.post_add_package_version(version=version, condainfo=condainfo) wrapped_bg_task = background_task_wrapper(indexing.update_indexes, logger) @@ -1479,7 +1478,7 @@ def _assert_filename_package_name_consistent(file_name: str, package_name: str): wait=wait_exponential(multiplier=1, min=4, max=10), after=after_log(logger, logging.WARNING), ) -def _extract_and_upload_package(file, channel_name, channel_proxylist): +def _extract_and_upload_package(file, channel_name, channel_proxylist, force: bool): try: conda_info = CondaInfo(file.file, file.filename) except Exception as e: @@ -1497,6 +1496,10 @@ def _extract_and_upload_package(file, channel_name, channel_proxylist): logger.info(f"Skip upload of proxied file {file.filename}") return conda_info + # Make sure that we do not upload over existing files + if pkgstore.file_exists(channel_name, dest) and not force: + raise FileExistsError(f"{file.filename}") + try: file.file.seek(0) logger.debug( @@ -1574,8 +1577,14 @@ def handle_package_files( files, (channel.name,) * len(files), (channel_proxylist,) * len(files), + (force,) * len(files), ) ] + except FileExistsError as e: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Duplicate {str(e)}", + ) except exceptions.PackageError as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=e.detail @@ -1650,7 +1659,7 @@ def _delete_file(condainfo, filename): summary=str(condainfo.about.get("summary", "n/a")), description=str(condainfo.about.get("description", "n/a")), ) - except pydantic.error_wrappers.ValidationError as err: + except pydantic.ValidationError as err: _delete_file(condainfo, file.filename) raise errors.ValidationError( "Validation Error for package: " diff --git a/quetz/metrics/rest_models.py b/quetz/metrics/rest_models.py index bce4b063..c6df59ce 100644 --- a/quetz/metrics/rest_models.py +++ b/quetz/metrics/rest_models.py @@ -1,7 +1,7 @@ from datetime import datetime from typing import Dict, List -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field from quetz.metrics.db_models import IntervalType @@ -9,9 +9,7 @@ class PackageVersionMetricItem(BaseModel): timestamp: datetime count: int - - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class PackageVersionMetricSeries(BaseModel): diff --git a/quetz/migrations/versions/3ba25f23fb7d_update_scoped_api_key_uploader_id.py b/quetz/migrations/versions/3ba25f23fb7d_update_scoped_api_key_uploader_id.py new file mode 100644 index 00000000..46fd0ffd --- /dev/null +++ b/quetz/migrations/versions/3ba25f23fb7d_update_scoped_api_key_uploader_id.py @@ -0,0 +1,61 @@ +"""Update scoped API key uploader id + +Revision ID: 3ba25f23fb7d +Revises: d212023a8e0b +Create Date: 2023-08-02 08:03:09.961559 + +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = '3ba25f23fb7d' +down_revision = 'd212023a8e0b' +branch_labels = None +depends_on = None + + +def upgrade(): + package_versions = sa.sql.table("package_versions", sa.sql.column("uploader_id")) + conn = op.get_bind() + # Get all user_id/owner_id from channel scoped API keys + # (user is anonymous - username is null) + res = conn.execute( + sa.text( + """SELECT api_keys.user_id, api_keys.owner_id FROM api_keys + INNER JOIN users ON users.id = api_keys.user_id + WHERE users.username is NULL; + """ + ) + ) + results = res.fetchall() + # Replace the uploader with the key owner (real user instead of the anonymous one) + for result in results: + op.execute( + package_versions.update() + .where(package_versions.c.uploader_id == result[0]) + .values(uploader_id=result[1]) + ) + + +def downgrade(): + package_versions = sa.sql.table("package_versions", sa.sql.column("uploader_id")) + conn = op.get_bind() + # Get all user_id/owner_id from channel scoped API keys + # (user is anonymous - username is null) + res = conn.execute( + sa.text( + """SELECT api_keys.user_id, api_keys.owner_id FROM api_keys + INNER JOIN users ON users.id = api_keys.user_id + WHERE users.username is NULL; + """ + ) + ) + results = res.fetchall() + # Replace the uploader with the key anonymous user + for result in results: + op.execute( + package_versions.update() + .where(package_versions.c.uploader_id == result[1]) + .values(uploader_id=result[0]) + ) diff --git a/quetz/rest_models.py b/quetz/rest_models.py index a9b4700d..926ff4b9 100644 --- a/quetz/rest_models.py +++ b/quetz/rest_models.py @@ -9,18 +9,15 @@ from enum import Enum from typing import Generic, List, Optional, TypeVar -from pydantic import BaseModel, Field, root_validator, validator -from pydantic.generics import GenericModel +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator T = TypeVar('T') class BaseProfile(BaseModel): - name: Optional[str] = Field(None, nullable=True) + name: Optional[str] = Field(None) avatar_url: str - - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class Profile(BaseProfile): @@ -30,27 +27,23 @@ class Profile(BaseProfile): class BaseUser(BaseModel): id: uuid.UUID username: str - - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class User(BaseUser): profile: BaseProfile -Profile.update_forward_refs() +Profile.model_rebuild() -Role = Field(None, regex='owner|maintainer|member') +Role = Field(None, pattern='owner|maintainer|member') class Member(BaseModel): role: str = Role user: User - - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class Pagination(BaseModel): @@ -67,26 +60,22 @@ class MirrorMode(str, Enum): class ChannelBase(BaseModel): name: str = Field(None, title='The name of the channel', max_length=50) description: Optional[str] = Field( - None, title='The description of the channel', max_length=300, nullable=True + None, title='The description of the channel', max_length=300 ) private: bool = Field(True, title="channel should be private") - size_limit: Optional[int] = Field( - None, title="size limit of the channel", nullable=True - ) + size_limit: Optional[int] = Field(None, title="size limit of the channel") ttl: int = Field(36000, title="ttl of the channel") - mirror_channel_url: Optional[str] = Field( - None, regex="^(http|https)://.+", nullable=True - ) - mirror_mode: Optional[MirrorMode] = Field(None, nullable=True) + mirror_channel_url: Optional[str] = Field(None, pattern="^(http|https)://.+") + mirror_mode: Optional[MirrorMode] = Field(None) - @validator("size_limit") + @field_validator("size_limit") + @classmethod def check_positive(cls, v): if v is not None and v < 0: return ValueError("must be positive value") return v - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class ChannelExtra(ChannelBase): @@ -97,9 +86,7 @@ class ChannelExtra(ChannelBase): class ChannelRole(BaseModel): name: str = Field(title="channel name") role: str = Field(title="user role") - - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class ChannelActionEnum(str, Enum): @@ -133,37 +120,33 @@ class ChannelMetadata(BaseModel): includelist: Optional[List[str]] = Field( None, title="list of packages to include while creating a channel", - nullable=True, ) excludelist: Optional[List[str]] = Field( None, title="list of packages to exclude while creating a channel", - nullable=True, ) proxylist: Optional[List[str]] = Field( None, title="list of packages that should only be proxied (not copied, " "stored and redistributed)", - nullable=True, ) class Channel(ChannelBase): metadata: ChannelMetadata = Field( - default_factory=ChannelMetadata, title="channel metadata", example={} + default_factory=ChannelMetadata, title="channel metadata", examples=[] ) actions: Optional[List[ChannelActionEnum]] = Field( None, title="list of actions to run after channel creation " "(see /channels/{}/actions for description)", - nullable=True, ) - @root_validator - def check_mirror_params(cls, values): - mirror_url = values.get("mirror_channel_url") - mirror_mode = values.get("mirror_mode") + @model_validator(mode='after') + def check_mirror_params(self) -> "Channel": + mirror_url = self.mirror_channel_url + mirror_mode = self.mirror_mode if mirror_url and not mirror_mode: raise ValueError( @@ -174,18 +157,14 @@ def check_mirror_params(cls, values): "'mirror_mode' provided but 'mirror_channel_url' is undefined" ) - return values + return self class ChannelMirrorBase(BaseModel): - url: str = Field(None, regex="^(http|https)://.+") - api_endpoint: Optional[str] = Field(None, regex="^(http|https)://.+", nullable=True) - metrics_endpoint: Optional[str] = Field( - None, regex="^(http|https)://.+", nullable=True - ) - - class Config: - orm_mode = True + url: str = Field(None, pattern="^(http|https)://.+") + api_endpoint: Optional[str] = Field(None, pattern="^(http|https)://.+") + metrics_endpoint: Optional[str] = Field(None, pattern="^(http|https)://.+") + model_config = ConfigDict(from_attributes=True) class ChannelMirror(ChannelMirrorBase): @@ -194,41 +173,31 @@ class ChannelMirror(ChannelMirrorBase): class Package(BaseModel): name: str = Field( - None, title='The name of package', max_length=1500, regex=r'^[a-z0-9-_\.]*$' + None, title='The name of package', max_length=1500, pattern=r'^[a-z0-9-_\.]*$' ) - summary: Optional[str] = Field( - None, title='The summary of the package', nullable=True - ) - description: Optional[str] = Field( - None, title='The description of the package', nullable=True - ) - url: Optional[str] = Field(None, title="project url", nullable=True) - platforms: List[str] = Field(None, title="supported platforms", nullable=True) - current_version: Optional[str] = Field( - None, title="latest version of any platform", nullable=True - ) - latest_change: Optional[datetime] = Field( - None, title="date of latest change", nullable=True - ) - - @validator("platforms", pre=True) + summary: Optional[str] = Field(None, title='The summary of the package') + description: Optional[str] = Field(None, title='The description of the package') + url: Optional[str] = Field(None, title="project url") + platforms: Optional[List[str]] = Field(None, title="supported platforms") + current_version: Optional[str] = Field(None, title="latest version of any platform") + latest_change: Optional[datetime] = Field(None, title="date of latest change") + + @field_validator("platforms", mode="before") + @classmethod def parse_list_of_platforms(cls, v): if isinstance(v, str): return v.split(":") else: return v - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class PackageRole(BaseModel): name: str = Field(title='The name of package') channel_name: str = Field(title='The channel this package belongs to') role: str = Field(title="user role for this package") - - class Config: - orm_mode = True + model_config = ConfigDict(from_attributes=True) class PackageSearch(Package): @@ -237,14 +206,12 @@ class PackageSearch(Package): class ChannelSearch(BaseModel): name: str = Field(None, title='The name of the channel', max_length=1500) - description: str = Field(None, title='The description of the channel') + description: Optional[str] = Field(None, title='The description of the channel') private: bool = Field(None, title='The visibility of the channel') + model_config = ConfigDict(from_attributes=True) - class Config: - orm_mode = True - -class PaginatedResponse(GenericModel, Generic[T]): +class PaginatedResponse(BaseModel, Generic[T]): pagination: Pagination = Field(None, title="Pagination object") result: List[T] = Field([], title="Result objects") @@ -254,21 +221,25 @@ class PostMember(BaseModel): role: str = Role +class UserOptionalRole(BaseModel): + role: Optional[str] = Role + + class UserRole(BaseModel): role: str = Role class CPRole(BaseModel): channel: str - package: Optional[str] = Field(None, nullable=True) + package: Optional[str] = Field(None) role: str = Role class BaseApiKey(BaseModel): description: str - time_created: Optional[date] = Field(None, nullable=True) - expire_at: Optional[date] = Field(None, nullable=True) - roles: Optional[List[CPRole]] = Field(None, nullable=True) + time_created: Optional[date] = Field(None) + expire_at: Optional[date] = Field(None) + roles: Optional[List[CPRole]] = Field(None) class ApiKey(BaseApiKey): @@ -289,18 +260,18 @@ class PackageVersion(BaseModel): uploader: BaseProfile time_created: datetime download_count: int + model_config = ConfigDict(from_attributes=True) - class Config: - orm_mode = True - - @validator("uploader", pre=True) + @field_validator("uploader", mode="before") + @classmethod def convert_uploader(cls, v): if hasattr(v, "profile"): return v.profile else: return v - @validator("info", pre=True) + @field_validator("info", mode="before") + @classmethod def load_json(cls, v): if isinstance(v, str): return json.loads(v) @@ -310,5 +281,5 @@ def load_json(cls, v): class ChannelAction(BaseModel): action: ChannelActionEnum - start_at: Optional[datetime] = Field(None, nullable=True) - repeat_every_seconds: Optional[int] = Field(None, nullable=True) + start_at: Optional[datetime] = Field(None) + repeat_every_seconds: Optional[int] = Field(None) diff --git a/quetz/tests/api/test_api_keys.py b/quetz/tests/api/test_api_keys.py index a78a86a6..85b4e83b 100644 --- a/quetz/tests/api/test_api_keys.py +++ b/quetz/tests/api/test_api_keys.py @@ -11,7 +11,7 @@ def api_keys(other_user, user, db, dao: Dao): def key_factory(key_user, descr, expire_at, roles): return dao.create_api_key( key_user.id, - BaseApiKey.parse_obj( + BaseApiKey.model_validate( dict(description=descr, expire_at=expire_at, roles=roles) ), descr, @@ -111,7 +111,7 @@ def test_list_keys_with_package_roles( def test_list_keys_subrole(auth_client, dao, user, private_channel): dao.create_api_key( user.id, - BaseApiKey.parse_obj( + BaseApiKey.model_validate( dict( description="user-key", roles=[ @@ -134,7 +134,7 @@ def test_list_keys_subrole(auth_client, dao, user, private_channel): def test_list_keys_without_roles(auth_client, dao, user): dao.create_api_key( user.id, - BaseApiKey.parse_obj(dict(description="user-key", roles=[])), + BaseApiKey.model_validate(dict(description="user-key", roles=[])), "user-key", ) diff --git a/quetz/tests/api/test_channels.py b/quetz/tests/api/test_channels.py index 534352c4..3976feab 100644 --- a/quetz/tests/api/test_channels.py +++ b/quetz/tests/api/test_channels.py @@ -689,7 +689,7 @@ def test_url_with_slash(auth_client, public_channel, db, remote_session): response = auth_client.post( f"/api/channels/{public_channel.name}/mirrors/", json={"url": mirror_url}, - allow_redirects=False, + follow_redirects=False, ) assert response.status_code == 307 diff --git a/quetz/tests/api/test_main_packages.py b/quetz/tests/api/test_main_packages.py index 3678c80b..1723df2a 100644 --- a/quetz/tests/api/test_main_packages.py +++ b/quetz/tests/api/test_main_packages.py @@ -1,9 +1,10 @@ import hashlib import json import os +import shutil import time from pathlib import Path -from typing import BinaryIO, Callable, Union +from typing import BinaryIO, Callable, Tuple, Union import pytest @@ -333,12 +334,12 @@ def _upload_file_1( auth_client, public_channel, public_package, - package_filename: str, + filepath: Path, force: bool = False, ): """Upload a file using /channels/{channel_name}/packages/{package_name}/files/""" - with open(package_filename, "rb") as fid: - files = {"files": (package_filename, fid)} + with open(filepath, "rb") as fid: + files = {"files": (filepath.name, fid)} response = auth_client.post( f"/api/channels/{public_channel.name}/packages/" f"{public_package.name}/files/", @@ -352,15 +353,15 @@ def _upload_file_2( auth_client, public_channel, public_package, - package_filename: str, + filepath: Path, force: bool = False, ): """Upload a file using /channels/{channel_name}/upload/{file_name}""" - with open(package_filename, "rb") as fid: + with open(filepath, "rb") as fid: body_bytes = fid.read() response = auth_client.post( - f"/api/channels/{public_channel.name}/upload/{package_filename}", + f"/api/channels/{public_channel.name}/upload/{filepath.name}", content=body_bytes, params={"force": force, "sha256": hashlib.sha256(body_bytes).hexdigest()}, ) @@ -384,7 +385,7 @@ def test_upload_package_version_wrong_filename( os.rename("test-package-0.1-0.tar.bz2", package_filename) response = upload_function( - auth_client, public_channel, public_package, package_filename + auth_client, public_channel, public_package, Path(package_filename) ) package_dir = Path(pkgstore.channels_dir) / public_channel.name / 'linux-64' @@ -397,12 +398,14 @@ def test_upload_package_version_wrong_filename( assert not os.path.exists(package_dir) -def sha256(path: Union[Path, str]) -> str: +def sha_and_md5(path: Union[Path, str]) -> Tuple[str, str]: sha = hashlib.sha256() + md5 = hashlib.md5() with open(path, "rb") as f: for chunk in iter(lambda: f.read(2**16), b""): sha.update(chunk) - return sha.hexdigest() + md5.update(chunk) + return sha.hexdigest(), md5.hexdigest() @pytest.mark.parametrize("upload_function", [_upload_file_1, _upload_file_2]) @@ -416,78 +419,128 @@ def test_upload_duplicate_package_version( config, remove_package_versions, upload_function: Callable, + tmp_path, ): pkgstore = config.get_package_store() - package_filename = "test-package-0.1-0.tar.bz2" - package_filename_copy = "test-package-0.1-0_copy.tar.bz2" + def get_repodata(): + """Helper function to read repo data""" + package_dir = Path(pkgstore.channels_dir) / public_channel.name / 'linux-64' + return json.loads((package_dir / 'repodata.json').read_text()) + + # Test setup: path1 is a package we will upload + path1 = Path(__file__).parent.parent / "data" / "test-package-0.1-0.tar.bz2" + + # To test duplicate uploads, we have a second file `_copy`, which is the same + # package and version but has a different content, so different hashes + # We must move this file to a temporary directory so that we can give it the same + # name as the first file. + path2 = tmp_path / path1.name + shutil.copyfile( + Path(__file__).parent.parent / "data" / "test-package-0.1-0_copy.tar.bz2", path2 + ) - file_sha = sha256(package_filename) - file_copy_sha = sha256(package_filename_copy) + # Sanity checks + sha1, md51 = sha_and_md5(path1) + sha2, md52 = sha_and_md5(path2) + assert (sha1 != sha2) and ( + md51 != md52 + ), "Sanity check failure: Test files have same hash" assert ( - file_sha != file_copy_sha - ), "Sanity check: Test files must have different hashes for this test." - file_size = os.path.getsize(package_filename) - file_copy_size = os.path.getsize(package_filename_copy) + path1.name == path2.name + ), "Sanity check failure: Test files have different name" + + size1 = os.path.getsize(path1) + size2 = os.path.getsize(path2) assert ( - file_size != file_copy_size - ), "Sanity check: Test files must have different sizes for this test." + size1 != size2 + ), "Sanity check failure: Test files must have different sizes for this test." - upload_function(auth_client, public_channel, public_package, package_filename) + # First upload + # File should not exist + assert not pkgstore.file_exists(public_channel.name, f"linux-64/{path1.name}") + response = upload_function(auth_client, public_channel, public_package, path1) - def get_repodata(): - """Helper function to read repo data""" - package_dir = Path(pkgstore.channels_dir) / public_channel.name / 'linux-64' - return json.loads((package_dir / 'repodata.json').read_text()) + # Expect success + assert response.status_code == 201 - repodata_init = get_repodata() - assert repodata_init["packages"][package_filename]["sha256"] == file_sha + # Check meta data is OK + repodata_after_first = get_repodata() + assert repodata_after_first["packages"][path1.name]["sha256"] == sha1 + assert repodata_after_first["packages"][path1.name]["md5"] == md51 + assert repodata_after_first["packages"][path1.name]["size"] == size1 - # Try submitting the same package without 'force' flag - response = upload_function( - auth_client, public_channel, public_package, package_filename + # Check that the file in the store is OK + file_in_store = ( + Path(pkgstore.channels_dir) / public_channel.name / 'linux-64' / path1.name ) + assert size1 == os.path.getsize(file_in_store) + assert (sha1, md51) == sha_and_md5(file_in_store) + + # Second upload: File with same name but different content, without force + response = upload_function(auth_client, public_channel, public_package, path2) - # Expect 409 here + # Expect 409 since the file already exists assert response.status_code == 409 detail = response.json()['detail'] assert "Duplicate" in detail - # Change the archive to test force update - os.remove(package_filename) - os.rename(package_filename_copy, package_filename) + # Check meta data is OK: It should not have changed with respect to before + repodata_after_second = get_repodata() + assert repodata_after_second == repodata_after_first + # Check that the file in the store is OK + file_in_store = ( + Path(pkgstore.channels_dir) / public_channel.name / 'linux-64' / path1.name + ) + + # File in store should not be the second file + assert not ( + (sha2, md52) == sha_and_md5(file_in_store) + ), "Duplicate upload without force updated stored file." + assert not ( + size2 == os.path.getsize(file_in_store) + ), "Duplicate upload without force updated stored file." + + # File in store should be the first file + assert size1 == os.path.getsize( + file_in_store + ), "Duplicate upload without force updated stored file." + assert (sha1, md51) == sha_and_md5( + file_in_store + ), "Duplicate upload without force updated stored file." + + # Third upload: Same as second but now with force # Ensure the 'time_modified' value changes in repodata.json time.sleep(1) # Submit the same package with 'force' flag response = upload_function( - auth_client, public_channel, public_package, package_filename, force=True + auth_client, public_channel, public_package, path2, force=True ) assert response.status_code == 201 # Check that repodata content has been updated - repodata = get_repodata() + repodata_after_force = get_repodata() # Info should match - assert repodata_init["info"] == repodata["info"] + assert repodata_after_first["info"] == repodata_after_force["info"] # Package keys should match - assert repodata_init["packages"].keys() == repodata["packages"].keys() - repodata_init_pkg = repodata_init["packages"][package_filename] - repodata_pkg = repodata["packages"][package_filename] - - # Hashes should match pre-upload expectation - assert repodata_init_pkg["sha256"] == file_sha - assert repodata_pkg["sha256"] == file_copy_sha + assert ( + repodata_after_first["packages"].keys() + == repodata_after_force["packages"].keys() + ) - # Sizes should match pre-upload expectation - assert repodata_init_pkg["size"] == file_size - assert repodata_pkg["size"] == file_copy_size + # Hashes should now have changed to the second file + assert repodata_after_force["packages"][path1.name]["sha256"] == sha2 + assert repodata_after_force["packages"][path1.name]["md5"] == md52 + assert repodata_after_force["packages"][path1.name]["size"] == size2 - # Upload-related metadata should not match - for key in "time_modified", "md5", "sha256": - assert repodata_init_pkg[key] != repodata_pkg[key] + assert ( + repodata_after_force["packages"][path1.name]["time_modified"] + > repodata_after_first["packages"][path1.name]["time_modified"] + ) @pytest.mark.parametrize("package_name", ["test-package"]) @@ -609,7 +662,7 @@ def test_validate_package_names(auth_client, public_channel, remove_package_vers @pytest.mark.parametrize( "package_name,msg", [ - ("TestPackage", "string does not match"), + ("TestPackage", "String should match"), ("test-package", None), ], ) @@ -835,7 +888,7 @@ def api_key(db, dao: Dao, owner, private_channel): # create an api key with restriction key = dao.create_api_key( owner.id, - BaseApiKey.parse_obj( + BaseApiKey.model_validate( dict( description="test api key", expire_at="2099-12-31", diff --git a/quetz/tests/test_docs.py b/quetz/tests/test_docs.py new file mode 100644 index 00000000..cae6fe88 --- /dev/null +++ b/quetz/tests/test_docs.py @@ -0,0 +1,16 @@ +def test_docs_endpoint(client): + response = client.get("/docs") + assert response.status_code == 200 + assert "text/html" in response.headers['Content-Type'] + + +def test_openapi_endpoint(client): + response = client.get("/openapi.json") + assert response.status_code == 200 + assert "application/json" in response.headers['Content-Type'] + + +def test_redoc_endpoint(client): + response = client.get("/redoc") + assert response.status_code == 200 + assert "text/html" in response.headers['Content-Type'] diff --git a/quetz/tests/test_jobs.py b/quetz/tests/test_jobs.py index 32163a0c..5680c4dc 100644 --- a/quetz/tests/test_jobs.py +++ b/quetz/tests/test_jobs.py @@ -244,7 +244,7 @@ async def test_running_task(db, user, package_version, supervisor): assert task.status == TaskStatus.pending # wait for task status to change - for i in range(50): + for i in range(100): time.sleep(0.05) db.refresh(task) @@ -283,7 +283,7 @@ async def test_restart_worker_process( assert task.status == TaskStatus.pending # wait for task status to change - for i in range(50): + for i in range(100): time.sleep(0.05) db.refresh(task) @@ -598,16 +598,20 @@ def test_post_new_job_manifest_validation( @pytest.mark.parametrize("user_role", ["owner"]) -def test_post_new_job_invalid_items_spec(auth_client, user, db, dummy_job_plugin): +def test_post_new_job_invalid_items_spec( + auth_client, user, db, dummy_job_plugin, mocker +): # items_spec=None is not allowed for jobs # (but it works with actions) manifest = "quetz-dummyplugin:dummy_func" + dummy_func = mocker.Mock() + mocker.patch("quetz_dummyplugin.jobs.dummy_func", dummy_func, create=True) response = auth_client.post( "/api/jobs", json={"items_spec": None, "manifest": manifest} ) assert response.status_code == 422 msg = response.json()['detail'] - assert "not an allowed value" in msg[0]['msg'] + assert "Input should be a valid string" in msg[0]['msg'] @pytest.mark.parametrize("user_role", ["owner"]) diff --git a/quetz/tests/test_mirror.py b/quetz/tests/test_mirror.py index ee59efb9..e715a253 100644 --- a/quetz/tests/test_mirror.py +++ b/quetz/tests/test_mirror.py @@ -860,11 +860,11 @@ def test_wrong_package_format(client, dummy_repo, owner, job_supervisor): [ ("proxy", None, "'mirror_channel_url' is undefined"), (None, "http://my-host", "'mirror_mode' is undefined"), - ("undefined", "http://my-host", "not a valid enumeration member"), - ("proxy", "my-host", "does not match"), - ("proxy", "http://", "does not match"), - ("proxy", "http:my-host", "does not match"), - ("proxy", "hosthttp://my-host", "does not match"), + ("undefined", "http://my-host", "Input should be 'proxy' or 'mirror'"), + ("proxy", "my-host", "String should match pattern"), + ("proxy", "http://", "String should match pattern"), + ("proxy", "http:my-host", "String should match pattern"), + ("proxy", "hosthttp://my-host", "String should match pattern"), (None, None, None), # non-mirror channel ("proxy", "http://my-host", None), ("proxy", "https://my-host", None), @@ -1070,7 +1070,7 @@ def test_proxylist_mirror_channel(owner, client, mirror_mode): response = client.get( "/get/mirror-channel-btel/linux-64/nrnpython-0.1-0.tar.bz2", - allow_redirects=False, + follow_redirects=False, ) assert response.status_code == 307 assert ( diff --git a/setup.cfg b/setup.cfg index 91215c78..f5b2737d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -34,6 +34,7 @@ install_requires = pluggy prometheus_client python-multipart + pydantic>=2.0.0 pyyaml requests sqlalchemy From 938462bb8a89b81784ae1a720a99277813accd51 Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Thu, 28 Sep 2023 16:54:59 +0200 Subject: [PATCH 2/2] Add test case where target user role is None. --- quetz/tests/api/test_users.py | 1 + 1 file changed, 1 insertion(+) diff --git a/quetz/tests/api/test_users.py b/quetz/tests/api/test_users.py index 21a38394..4f3473bd 100644 --- a/quetz/tests/api/test_users.py +++ b/quetz/tests/api/test_users.py @@ -27,6 +27,7 @@ def test_validate_user_role_names(user, client, other_user, db): ("other", "owner", "member", 200), ("other", "owner", "maintainer", 200), ("other", "owner", "owner", 200), + ("other", "owner", None, 200), ("missing_user", "owner", "member", 404), ], )