refactor: validate version on initialization

karapace/schema_models.py

@@ -397,6 +397,9 @@ class Version:
     LATEST_VERSION_TAG: Final | ClassVar[str] = "latest"
     MINUS_1_VERSION_TAG: Final | ClassVar[str] = "-1"
 
+    def __post_init__(self):
+        self.validate()
+
     def validate(self):
         try:
             version = int(self.tag)

karapace/schema_registry.py

@@ -266,7 +266,6 @@ def subject_version_get(self, subject: Subject, version: Version, *, include_del
     async def subject_version_referencedby_get(
         self, subject: Subject, version: Version, *, include_deleted: bool = False
     ) -> list:
-        version.validate()
         schema_versions = self.subject_get(subject, include_deleted=include_deleted)
         if not schema_versions:
             raise SubjectNotFoundException()

karapace/schema_registry_apis.py

@@ -377,12 +377,11 @@ async def compatibility_check(
 
         self._check_authorization(user, Operation.Read, f"Subject:{subject}")
 
-        version = Version(version)
-        version.validate()
         body = request.json
         schema_type = self._validate_schema_type(content_type=content_type, data=body)
         references = self._validate_references(content_type, schema_type, body)
         try:
+            version = Version(version)
             references, new_schema_dependencies = self.schema_registry.resolve_references(references)
             new_schema = ValidatedTypedSchema.parse(
                 schema_type=schema_type,
@@ -783,10 +782,9 @@ async def subject_version_get(
     ) -> None:
         self._check_authorization(user, Operation.Read, f"Subject:{subject}")
 
-        version = Version(version)
-        version.validate()
         deleted = request.query.get("deleted", "false").lower() == "true"
         try:
+            version = Version(version)
             subject_data = self.schema_registry.subject_version_get(subject, version, include_deleted=deleted)
             if "compatibility" in subject_data:
                 del subject_data["compatibility"]
@@ -816,13 +814,12 @@ async def subject_version_delete(
         self, content_type: str, *, subject: str, version: str, request: HTTPRequest, user: User | None = None
     ) -> None:
         self._check_authorization(user, Operation.Write, f"Subject:{subject}")
-        version = Version(version)
-        version.validate()
         permanent = request.query.get("permanent", "false").lower() == "true"
 
         are_we_master, master_url = await self.schema_registry.get_master()
         if are_we_master:
             try:
+                version = Version(version)
                 resolved_version = await self.schema_registry.subject_version_delete_local(subject, version, permanent)
                 self.r(str(resolved_version), content_type, status=HTTPStatus.OK)
             except (SubjectNotFoundException, SchemasNotFoundException):
@@ -878,6 +875,8 @@ async def subject_version_delete(
                     content_type=content_type,
                     status=HTTPStatus.UNPROCESSABLE_ENTITY,
                 )
+            except InvalidVersion:
+                self._invalid_version(content_type, version.tag)
         elif not master_url:
             self.no_master_error(content_type)
         else:
@@ -917,8 +916,8 @@ async def subject_version_schema_get(
     async def subject_version_referencedby_get(self, content_type, *, subject, version, user: User | None = None):
         self._check_authorization(user, Operation.Read, f"Subject:{subject}")
 
-        version = Version(version)
         try:
+            version = Version(version)
             referenced_by = await self.schema_registry.subject_version_referencedby_get(subject, version)
         except (SubjectNotFoundException, SchemasNotFoundException):
             self.r(

tests/unit/test_schema_models.py

@@ -60,19 +60,26 @@ def test_tags(self, version: Version):
 
     @pytest.mark.parametrize(
         "version, is_latest",
-        [(Version("latest"), True), (Version("-1"), True), (Version("-20"), False), (Version(10), False)],
+        [(Version("latest"), True), (Version("-1"), True), (Version(10), False)],
     )
     def test_is_latest(self, version: Version, is_latest: bool):
         assert version.is_latest is is_latest
 
     @pytest.mark.parametrize("version", [Version("latest"), Version(10), Version(-1)])
     def test_validate(self, version: Version):
-        version.validate()
+        assert version
 
-    @pytest.mark.parametrize("invalid_version", [Version("invalid_version"), Version(0), Version(-20)])
-    def test_validate_invalid(self, invalid_version: Version):
+    def test_validate_invalid(self):
         with pytest.raises(InvalidVersion):
-            assert not invalid_version.validate()
+            Version("invalid_version")
+        with pytest.raises(InvalidVersion):
+            Version(0)
+        with pytest.raises(InvalidVersion):
+            Version("0")
+        with pytest.raises(InvalidVersion):
+            Version(-20)
+        with pytest.raises(InvalidVersion):
+            Version("-10")
 
     @pytest.mark.parametrize(
         "version, resolved_version",
@@ -108,23 +115,13 @@ def test_resolve_from_schema_versions(
         schema_versions.update(schema_versions_factory(10))
         assert version.resolve_from_schema_versions(schema_versions) == resolved_version
 
-    @pytest.mark.parametrize(
-        "invalid_version",
-        [
-            Version("invalid_version"),
-            Version(0),
-            Version(-20),
-            Version("-10"),
-            Version("100"),
-            Version(2000),
-        ],
-    )
+    @pytest.mark.parametrize("nonexisting_version", [Version("100"), Version(2000)])
     def test_resolve_from_schema_versions_invalid(
         self,
-        invalid_version: Version,
+        nonexisting_version: Version,
         schema_versions_factory: SVFCallable,
     ):
         schema_versions = dict()
         schema_versions.update(schema_versions_factory(1))
         with pytest.raises(VersionNotFoundException):
-            invalid_version.resolve_from_schema_versions(schema_versions)
+            nonexisting_version.resolve_from_schema_versions(schema_versions)