Skip to content

Commit

Permalink
fix: invalid version errors should use path version string
Browse files Browse the repository at this point in the history
  • Loading branch information
nosahama committed Jun 3, 2024
1 parent 1dc3eac commit 43d7ed0
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 22 deletions.
16 changes: 11 additions & 5 deletions karapace/schema_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,17 +390,17 @@ class SchemaVersion:
references: Sequence[Reference] | None


@dataclass(slots=True, frozen=True)
@dataclass(frozen=True)
class Version:
tag: str | int

LATEST_VERSION_TAG: Final | ClassVar[str] = "latest"
MINUS_1_VERSION_TAG: Final | ClassVar[str] = "-1"
LATEST_VERSION_TAG: ClassVar[str] = "latest"
MINUS_1_VERSION_TAG: ClassVar[str] = "-1"

def __post_init__(self):
def __post_init__(self) -> None:
self.validate()

def validate(self):
def validate(self) -> None:
try:
version = int(self.tag)
if (version < int(self.MINUS_1_VERSION_TAG)) or (version == 0):
Expand Down Expand Up @@ -432,3 +432,9 @@ def resolve_from_schema_versions(self, schema_versions: Mapping[int, SchemaVersi
if self.version <= max_version and self.version in schema_versions:
return self.version
return None

def __str__(self) -> str:
return str(self.tag)

def __repr__(self) -> str:
return f"Version<{self.tag}>"
29 changes: 15 additions & 14 deletions karapace/schema_registry_apis.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,12 +401,12 @@ async def compatibility_check(
try:
old = self.schema_registry.subject_version_get(subject=subject, version=version)
except InvalidVersion:
self._invalid_version(content_type, version.tag)
self._invalid_version(content_type, version)
except (VersionNotFoundException, SchemasNotFoundException, SubjectNotFoundException):
self.r(
body={
"error_code": SchemaErrorCodes.VERSION_NOT_FOUND.value,
"message": f"Version {version.tag} not found.",
"message": f"Version {version} not found.",
},
content_type=content_type,
status=HTTPStatus.NOT_FOUND,
Expand Down Expand Up @@ -802,13 +802,13 @@ async def subject_version_get(
self.r(
body={
"error_code": SchemaErrorCodes.VERSION_NOT_FOUND.value,
"message": f"Version {version.tag} not found.",
"message": f"Version {version} not found.",
},
content_type=content_type,
status=HTTPStatus.NOT_FOUND,
)
except InvalidVersion:
self._invalid_version(content_type, version.tag)
self._invalid_version(content_type, version)

async def subject_version_delete(
self, content_type: str, *, subject: str, version: str, request: HTTPRequest, user: User | None = None
Expand All @@ -835,7 +835,7 @@ async def subject_version_delete(
self.r(
body={
"error_code": SchemaErrorCodes.VERSION_NOT_FOUND.value,
"message": f"Version {version.tag} not found.",
"message": f"Version {version} not found.",
},
content_type=content_type,
status=HTTPStatus.NOT_FOUND,
Expand All @@ -845,7 +845,8 @@ async def subject_version_delete(
body={
"error_code": SchemaErrorCodes.SCHEMAVERSION_SOFT_DELETED.value,
"message": (
f"Subject '{subject}' Version 1 was soft deleted.Set permanent=true to delete permanently"
f"Subject '{subject}' Version {version} was soft deleted. "
"Set permanent=true to delete permanently"
),
},
content_type=content_type,
Expand All @@ -856,7 +857,7 @@ async def subject_version_delete(
body={
"error_code": SchemaErrorCodes.SCHEMAVERSION_NOT_SOFT_DELETED.value,
"message": (
f"Subject '{subject}' Version {version.tag} was not deleted "
f"Subject '{subject}' Version {version} was not deleted "
"first before being permanently deleted"
),
},
Expand All @@ -876,29 +877,29 @@ async def subject_version_delete(
status=HTTPStatus.UNPROCESSABLE_ENTITY,
)
except InvalidVersion:
self._invalid_version(content_type, version.tag)
self._invalid_version(content_type, version)
elif not master_url:
self.no_master_error(content_type)
else:
url = f"{master_url}/subjects/{subject}/versions/{version.tag}?permanent={permanent}"
url = f"{master_url}/subjects/{subject}/versions/{version}?permanent={permanent}"
await self._forward_request_remote(request=request, body={}, url=url, content_type=content_type, method="DELETE")

async def subject_version_schema_get(
self, content_type: str, *, subject: str, version: str, user: User | None = None
) -> None:
self._check_authorization(user, Operation.Read, f"Subject:{subject}")

version = Version(version)
try:
version = Version(version)
subject_data = self.schema_registry.subject_version_get(subject, version)
self.r(subject_data["schema"], content_type)
except InvalidVersion:
self._invalid_version(content_type, version.tag)
self._invalid_version(content_type, version)
except VersionNotFoundException:
self.r(
body={
"error_code": SchemaErrorCodes.VERSION_NOT_FOUND.value,
"message": f"Version {version.tag} not found.",
"message": f"Version {version} not found.",
},
content_type=content_type,
status=HTTPStatus.NOT_FOUND,
Expand Down Expand Up @@ -932,13 +933,13 @@ async def subject_version_referencedby_get(self, content_type, *, subject, versi
self.r(
body={
"error_code": SchemaErrorCodes.VERSION_NOT_FOUND.value,
"message": f"Version {version.tag} not found.",
"message": f"Version {version} not found.",
},
content_type=content_type,
status=HTTPStatus.NOT_FOUND,
)
except InvalidVersion:
self._invalid_version(content_type, version.tag)
self._invalid_version(content_type, version)

self.r(referenced_by, content_type, status=HTTPStatus.OK)

Expand Down
23 changes: 23 additions & 0 deletions tests/integration/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -2323,6 +2323,29 @@ async def test_version_number_validation(registry_async_client: Client) -> None:
)


async def test_get_schema_version_by_latest_tags(registry_async_client: Client) -> None:
"""
Creates a subject and schema. Tests that the endpoints
`subjects/{subject}/versions/latest` and `subjects/{subject}/versions/-1` return the latest schema.
"""
subject = create_subject_name_factory("test_subject")()
res = await registry_async_client.post(f"subjects/{subject}/versions", json={"schema": '{"type": "string"}'})
assert res.status_code == 200
schema_id = res.json()["id"]

res = await registry_async_client.get(f"subjects/{subject}/versions")
assert res.status_code == 200
schema_version = res.json()[0]

version_endpoints = {f"subjects/{subject}/versions/latest", f"subjects/{subject}/versions/-1"}
for endpoint in version_endpoints:
res = await registry_async_client.get(endpoint)
res_data = res.json()
assert res.status_code == 200
assert res_data["id"] == schema_id
assert res_data["version"] == schema_version


async def test_common_endpoints(registry_async_client: Client) -> None:
res = await registry_async_client.get("")
assert res.status_code == 200
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_schema_protobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ async def test_protobuf_schema_references(registry_async_client: Client) -> None
assert res.status_code == 200
res = await registry_async_client.delete("subjects/test_schema/versions/1")
myjson = res.json()
match_msg = "Subject 'test_schema' Version 1 was soft deleted.Set permanent=true to delete permanently"
match_msg = "Subject 'test_schema' Version 1 was soft deleted. Set permanent=true to delete permanently"

assert res.status_code == 404

Expand Down
8 changes: 6 additions & 2 deletions tests/unit/test_schema_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from karapace.errors import InvalidVersion, VersionNotFoundException
from karapace.schema_models import parse_avro_schema_definition, SchemaVersion, TypedSchema, Version
from karapace.schema_type import SchemaType
from typing import Any, Callable, Dict, Optional, Union
from typing import Any, Callable, Dict, Optional

import pytest

Expand Down Expand Up @@ -58,6 +58,10 @@ def test_tags(self, version: Version):
assert version.LATEST_VERSION_TAG == "latest"
assert version.MINUS_1_VERSION_TAG == "-1"

def test_text_formating(self, version: Version):
assert f"{version}" == "1"
assert f"{version!r}" == "Version<1>"

@pytest.mark.parametrize(
"version, is_latest",
[(Version("latest"), True), (Version("-1"), True), (Version(10), False)],
Expand Down Expand Up @@ -90,7 +94,7 @@ def test_validate_invalid(self):
(Version(10), "10"),
],
)
def test_resolved(self, version: Version, resolved_version: Union[str, int]):
def test_resolved(self, version: Version, resolved_version: str):
assert version.resolved == resolved_version

@pytest.mark.parametrize(
Expand Down

0 comments on commit 43d7ed0

Please sign in to comment.