Skip to content

Commit

Permalink
Merge pull request #515 from PerchunPak/remove-type-validation-in-status
Browse files Browse the repository at this point in the history
Remove the type validation system in status responses
  • Loading branch information
kevinkjt2000 authored Apr 19, 2023
2 parents 93461e2 + abd97e4 commit 4ca7949
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 73 deletions.
2 changes: 1 addition & 1 deletion mcstatus/pinger.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def read_status(self) -> JavaStatusResponse:
raise IOError("Received invalid JSON")
try:
return JavaStatusResponse.build(raw, latency=(received - start) * 1000)
except (ValueError, TypeError) as e:
except KeyError as e:
raise IOError(f"Received invalid status response: {e}")

def test_ping(self) -> float:
Expand Down
26 changes: 0 additions & 26 deletions mcstatus/status_response.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

from abc import ABC, abstractmethod
from collections.abc import Iterable, Mapping
from dataclasses import dataclass
from typing import Any, TYPE_CHECKING

Expand Down Expand Up @@ -91,26 +90,6 @@ class RawJavaResponse(TypedDict):
}


def _validate_data(raw: Mapping[str, Any], who: str, required: Iterable[tuple[str, type]]) -> None:
"""Ensure that all required keys are present, and have the specified type.
:param raw: The raw :class:`dict` answer to check.
:param who: The name of the object that is checking the data. Example ``status``, ``player`` etc.
:param required:
An iterable of string and type. The string is the required key which must be in ``raw``, and the ``type`` is the
type that the key must be. If you want to ignore check of the type, set the type to :obj:`object`.
:raises ValueError: If the required keys are not present.
:raises TypeError: If the required keys are not of the expected type.
"""
for required_key, required_type in required:
if required_key not in raw:
raise ValueError(f"Invalid {who} object (no {required_key!r} value)")
if not isinstance(raw[required_key], required_type):
raise TypeError(
f"Invalid {who} object (expected {required_key!r} to be {required_type}, was {type(raw[required_key])})"
)


@dataclass
class BaseStatusResponse(ABC):
"""Class for storing shared data from a status response."""
Expand Down Expand Up @@ -170,7 +149,6 @@ def build(cls, raw: RawJavaResponse, latency: float = 0) -> Self:
``description`` - :class:`str`) are not of the expected type.
:return: :class:`JavaStatusResponse` object.
"""
_validate_data(raw, "status", [("players", dict), ("version", dict), ("description", str)])
return cls(
raw=raw,
players=JavaStatusPlayers.build(raw["players"]),
Expand Down Expand Up @@ -333,10 +311,8 @@ def build(cls, raw: RawJavaResponsePlayers) -> Self:
``sample`` - :class:`list`) are not of the expected type.
:return: :class:`JavaStatusPlayers` object.
"""
_validate_data(raw, "players", [("online", int), ("max", int)])
sample = None
if "sample" in raw:
_validate_data(raw, "players", [("sample", list)])
sample = [JavaStatusPlayer.build(player) for player in raw["sample"]]
return cls(
online=raw["online"],
Expand Down Expand Up @@ -374,7 +350,6 @@ def build(cls, raw: RawJavaResponsePlayer) -> Self:
are not of the expected type.
:return: :class:`JavaStatusPlayer` object.
"""
_validate_data(raw, "player", [("name", str), ("id", str)])
return cls(name=raw["name"], id=raw["id"])


Expand Down Expand Up @@ -409,7 +384,6 @@ def build(cls, raw: RawJavaResponseVersion) -> Self:
are not of the expected type.
:return: :class:`JavaStatusVersion` object.
"""
_validate_data(raw, "version", [("name", str), ("protocol", int)])
return cls(name=raw["name"], protocol=raw["protocol"])


Expand Down
14 changes: 0 additions & 14 deletions tests/status_response/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import abc
from typing import Any, TypeVar, cast

import pytest

from mcstatus.status_response import BaseStatusResponse

__all__ = ["BaseStatusResponseTest"]
Expand Down Expand Up @@ -74,18 +72,6 @@ def test_optional_field_turns_into_none(self, build: BaseStatusResponse, to_remo
del raw[to_remove]
assert getattr(type(build).build(raw), attribute_name) is None

def test_value_validating(self, build: BaseStatusResponse, exclude_field: str) -> None:
raw = cast(list, self.BUILD_METHOD_VALIDATION)[2].copy()
raw.pop(exclude_field)
with pytest.raises(ValueError):
type(build).build(raw)

def test_type_validating(self, build: BaseStatusResponse, to_change_field: str) -> None:
raw = cast(list, self.BUILD_METHOD_VALIDATION)[2].copy()
raw[to_change_field] = object()
with pytest.raises(TypeError):
type(build).build(raw)

def _dependency_table(self) -> dict[str, bool]:
# a key in the dict must be a name of a test implementation.
# and a value of the dict is a bool. if it's false - we
Expand Down
34 changes: 2 additions & 32 deletions tests/status_response/test_shared.py
Original file line number Diff line number Diff line change
@@ -1,36 +1,6 @@
from pytest import mark, raises
from pytest import raises

from mcstatus.status_response import BaseStatusResponse, _validate_data


class TestValidateDataFunction:
@mark.parametrize("raw", [{"foo": "bar"}, {"foo": 123, "bar": 1.4, "baz": True}])
@mark.parametrize("required_key", ["foo", "bar", "baz"])
@mark.parametrize("required_type", [str, int, object])
def test_invalid_data(self, raw, required_key, required_type):
if required_key in raw and isinstance(raw[required_key], required_type):
return
elif required_key in raw and not isinstance(raw[required_key], required_type):
error = TypeError
elif required_key not in raw:
error = ValueError
else:
raise ValueError("Unknown parametrize")

with raises(error) as exc:
_validate_data(raw, "test", [(required_key, required_type)])

if error == ValueError:
assert exc.match(f"no '{required_key}' value")
else:
assert exc.match(f"'{required_key}' to be {required_type}, was {type(raw[required_key])}")

def test_who_parameter(self):
who = str(object())
with raises(ValueError) as exc:
_validate_data({"foo": "bar"}, who, [("not exist", object)])

exc.match(f"^Invalid {who} object")
from mcstatus.status_response import BaseStatusResponse


class TestMCStatusResponse:
Expand Down

0 comments on commit 4ca7949

Please sign in to comment.