From 9a196160efb0c1fcf68b10b8c1f52941376173fd Mon Sep 17 00:00:00 2001 From: Sean N Date: Sun, 17 May 2026 05:13:36 +0200 Subject: [PATCH] Fixed regexrule and timezonerule inconsistencies --- examples/date_validation.py | 4 +- nitro_validator/__init__.py | 23 +- nitro_validator/core/rule.py | 8 +- nitro_validator/core/rule_registry.py | 16 ++ nitro_validator/core/validator.py | 44 +-- nitro_validator/utils/__init__.py | 2 +- nitro_validator/utils/rules.py | 388 ++++++++++++-------------- pyproject.toml | 2 +- tests/test_full_coverage.py | 2 +- tests/test_review_fixes.py | 321 +++++++++++++++++++++ 10 files changed, 565 insertions(+), 245 deletions(-) create mode 100644 tests/test_review_fixes.py diff --git a/examples/date_validation.py b/examples/date_validation.py index 2d72f45..588cba8 100644 --- a/examples/date_validation.py +++ b/examples/date_validation.py @@ -92,8 +92,8 @@ def example_date_format(): validator = Validator() data = { - "uk_date": "15/06/1990", # DD/MM/YYYY - "us_date": "06-15-1990", # MM-DD-YYYY + "uk_date": "15/06/1990", # DD/MM/YYYY + "us_date": "06-15-1990", # MM-DD-YYYY "timestamp": "15 Jun 1990 09:30", # DD Mon YYYY HH:MM } rules = { diff --git a/nitro_validator/__init__.py b/nitro_validator/__init__.py index 8f5971c..c618495 100644 --- a/nitro_validator/__init__.py +++ b/nitro_validator/__init__.py @@ -11,7 +11,9 @@ validated = validator.validate(data, rules) """ -__version__ = "1.0.3" +__version__ = "1.0.4" + +from typing import Optional from .core import ( NitroValidator, @@ -92,8 +94,11 @@ class NitroValidator(_OriginalNitroValidator): This is the public entry point exported at the top level of the package. It is a thin subclass of :class:`nitro_validator.core.NitroValidator` whose only job is to - default the registry to a shared instance pre-populated with all 51+ - built-in rules. Pass ``registry=`` to opt out of the defaults. + default the registry to a fresh copy of the module-level registry, + which is pre-populated with all 51+ built-in rules. Each + ``NitroValidator()`` therefore gets its own registry, so + ``register_rule()`` calls are isolated to that instance. Pass + ``registry=`` to supply your own. Example: >>> from nitro_validator import NitroValidator @@ -105,15 +110,15 @@ class NitroValidator(_OriginalNitroValidator): {'email': 'a@b.co', 'age': 21} """ - def __init__(self, registry: "NitroRuleRegistry" = None): - """Create a validator, defaulting to the shared built-in registry. + def __init__(self, registry: Optional["NitroRuleRegistry"] = None): + """Create a validator, defaulting to a fresh copy of the built-in registry. Args: - registry: A custom :class:`NitroRuleRegistry`. Omit to reuse - the module-level registry that already contains every - built-in rule. + registry: A custom :class:`NitroRuleRegistry`. Omit to get a + fresh registry seeded with every built-in rule; rules + you register on this validator will not leak to others. """ - super().__init__(registry or _default_registry) + super().__init__(registry if registry is not None else _default_registry.copy()) # Provide convenient aliases without "Nitro" prefix diff --git a/nitro_validator/core/rule.py b/nitro_validator/core/rule.py index e967db2..29203ae 100644 --- a/nitro_validator/core/rule.py +++ b/nitro_validator/core/rule.py @@ -11,13 +11,13 @@ class NitroValidationRule: Subclass this to add a new rule. Set two class attributes and implement :meth:`validate`: - * ``name`` — the string used in pipe-delimited rule syntax + * ``name`` - the string used in pipe-delimited rule syntax (e.g. ``"required"``, ``"min"``). - * ``message`` — the default error template. Placeholders ``{field}``, + * ``message`` - the default error template. Placeholders ``{field}``, ``{args}``, and positional ``{0}``, ``{1}``, ... are substituted by :meth:`get_message`. - A rule instance carries its positional arguments in ``self.args`` — + A rule instance carries its positional arguments in ``self.args`` - these come from the colon/comma-delimited tail of a rule string (``"between:1,100"`` → ``args == ("1", "100")``), always as strings when parsed from a rule string, so cast inside :meth:`validate` if @@ -58,7 +58,7 @@ def __init__(self, *args: Any, **kwargs: Any): """Store the rule's positional and keyword arguments. Rule strings like ``"between:1,100"`` are parsed into - ``*args = ("1", "100")`` — arguments therefore arrive as + ``*args = ("1", "100")`` - arguments therefore arrive as strings and must be cast inside :meth:`validate`. Pass ``message=`` to override the default template for this instance only. diff --git a/nitro_validator/core/rule_registry.py b/nitro_validator/core/rule_registry.py index 20c8a21..2b6465d 100644 --- a/nitro_validator/core/rule_registry.py +++ b/nitro_validator/core/rule_registry.py @@ -124,3 +124,19 @@ def all(self) -> Dict[str, Type[NitroValidationRule]]: def clear(self) -> None: """Remove every registered rule. Useful for building a registry from scratch.""" self._rules.clear() + + def copy(self) -> "NitroRuleRegistry": + """Return a new registry with the same rule registrations. + + The returned registry is independent: registering or unregistering + rules on it does not affect the original. Rule classes themselves + are not copied (registries store references to classes, not + instances). + + Returns: + A new :class:`NitroRuleRegistry` populated with the same + name-to-class mappings as this one. + """ + new_registry = NitroRuleRegistry() + new_registry._rules = self._rules.copy() + return new_registry diff --git a/nitro_validator/core/validator.py b/nitro_validator/core/validator.py index 68ddba9..de979da 100644 --- a/nitro_validator/core/validator.py +++ b/nitro_validator/core/validator.py @@ -16,7 +16,7 @@ class NitroValidator: either returns the validated subset or raises :class:`NitroValidationError`. - Rules can be expressed two ways — interchangeably within the same + Rules can be expressed two ways, interchangeably within the same call: * Pipe-delimited strings: ``"required|email"``, ``"min:18"``, @@ -84,7 +84,7 @@ def validate( Every rule for every field is evaluated before this method raises, so :attr:`errors` contains *all* failures, not just the first. Fields absent from ``data`` are validated against their - rules with a value of ``None`` — most rules pass on ``None``, so + rules with a value of ``None``. Most rules pass on ``None``, so pair with ``required`` to enforce presence. Args: @@ -118,15 +118,12 @@ def validate( for field, field_rules in rules.items(): value = data.get(field) - # Parse rules if they're a string if isinstance(field_rules, str): parsed_rules = self._parse_rules(field_rules) else: parsed_rules = field_rules - # Validate each rule for rule in parsed_rules: - # Create rule instance if it's a string if isinstance(rule, str): rule_instance = self._create_rule_from_string(rule) elif isinstance(rule, NitroValidationRule): @@ -134,28 +131,31 @@ def validate( else: continue - # Check if field has custom messages + override_message: Optional[str] = None field_messages = messages.get(field, {}) if isinstance(field_messages, str): - # Single message for all rules - rule_instance.custom_message = field_messages + override_message = field_messages elif isinstance(field_messages, dict): - # Specific message for this rule rule_name = rule_instance.name or rule_instance.__class__.__name__.lower() if rule_name in field_messages: - rule_instance.custom_message = field_messages[rule_name] + override_message = field_messages[rule_name] - # Run validation if not rule_instance.validate(field, value, data): if field not in self.errors: self.errors[field] = [] - self.errors[field].append(rule_instance.get_message(field)) + if override_message is not None: + prior = rule_instance.custom_message + rule_instance.custom_message = override_message + try: + self.errors[field].append(rule_instance.get_message(field)) + finally: + rule_instance.custom_message = prior + else: + self.errors[field].append(rule_instance.get_message(field)) - # Add to validated data if no errors if field not in self.errors: self.validated_data[field] = value - # Raise exception if there are errors if self.errors: raise NitroValidationError(self.errors) @@ -237,9 +237,17 @@ def _create_rule_from_string(self, rule_string: str) -> NitroValidationRule: NitroValidationRule instance Raises: - NitroRuleNotFoundError: If the rule is not found + NitroRuleNotFoundError: If the rule is not found. + NitroInvalidRuleError: If the string parses as a known rule + but with the wrong number of arguments. + + Note: + The pipe and comma are used to split rules and arguments + respectively, so they cannot appear in rule arguments. Rules + that need patterns containing ``|`` or ``,`` (notably + ``regex``) must be passed as a rule instance instead of a + string, e.g. ``RegexRule(r"^(a|b)$")``. """ - # Parse rule name and arguments if ":" in rule_string: rule_name, args_string = rule_string.split(":", 1) args = [arg.strip() for arg in args_string.split(",")] @@ -247,10 +255,8 @@ def _create_rule_from_string(self, rule_string: str) -> NitroValidationRule: rule_name = rule_string args = [] - # Get rule class from registry rule_class = self.registry.get(rule_name) - # Create and return rule instance return rule_class(*args) @classmethod @@ -264,7 +270,7 @@ def make( """Construct a validator and run :meth:`validate` in one call. Convenient when you only want a one-shot validation and the - validator instance afterwards — e.g. to read + validator instance afterwards, e.g. to read :attr:`validated_data`. Args: diff --git a/nitro_validator/utils/__init__.py b/nitro_validator/utils/__init__.py index bde91e7..4eac510 100644 --- a/nitro_validator/utils/__init__.py +++ b/nitro_validator/utils/__init__.py @@ -128,7 +128,7 @@ def register_builtin_rules(registry: NitroRuleRegistry) -> None: The default registry returned by :class:`NitroValidator() ` is already populated with these - rules — call this only when you are building a custom registry + rules; call this only when you are building a custom registry from scratch and want the built-ins available under their standard names. diff --git a/nitro_validator/utils/rules.py b/nitro_validator/utils/rules.py index 92bbecd..141b553 100644 --- a/nitro_validator/utils/rules.py +++ b/nitro_validator/utils/rules.py @@ -5,10 +5,39 @@ import re import uuid as uuid_module import json as json_module -from typing import Any +from datetime import datetime +from typing import Any, Optional, Tuple from ..core.rule import NitroValidationRule +_ISO_DATE_FORMATS: Tuple[str, ...] = ( + "%Y-%m-%d", + "%Y/%m/%d", + "%Y-%m-%d %H:%M:%S", + "%Y/%m/%d %H:%M:%S", + "%Y-%m-%dT%H:%M:%S", + "%Y-%m-%dT%H:%M:%SZ", +) + + +def _parse_iso_date(date_value: Any) -> Optional[datetime]: + """Parse a datetime or unambiguous ISO 8601 string. Returns None on failure. + + Regional formats like ``DD-MM-YYYY`` are intentionally not supported - + callers needing them must use ``DateFormatRule`` with an explicit format. + """ + if isinstance(date_value, datetime): + return date_value + if not isinstance(date_value, str): + return None + for fmt in _ISO_DATE_FORMATS: + try: + return datetime.strptime(date_value, fmt) + except ValueError: + continue + return None + + # ============================================================================ # Basic Rules # ============================================================================ @@ -111,22 +140,33 @@ def validate(self, field: str, value: Any, data: dict) -> bool: class RegexRule(NitroValidationRule): - """Validate that a field matches a regular expression.""" + """Validate that a field matches a regular expression. + + The pipe-delimited rule syntax splits on ``|`` and ``,``, so patterns + containing those characters cannot be passed as a rule string. Build + the rule directly instead, e.g. ``RegexRule(r"^(a|b)$")``. + """ name = "regex" message = "The {field} field format is invalid." + def __init__(self, *args: Any, **kwargs: Any): + super().__init__(*args, **kwargs) + self._compiled = None + if self.args: + try: + self._compiled = re.compile(self.args[0]) + except re.error: + self._compiled = None + def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True if not isinstance(value, str): return False - - pattern = self.args[0] if self.args else None - if not pattern: + if self._compiled is None: return False - - return bool(re.match(pattern, value)) + return bool(self._compiled.match(value)) class LowercaseRule(NitroValidationRule): @@ -252,33 +292,30 @@ class IpRule(NitroValidationRule): name = "ip" message = "The {field} field must be a valid IP address." + IPV4_PATTERN = re.compile( + r"^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}" + r"(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$" + ) + IPV6_PATTERN = re.compile( + r"^(([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|" + r"([0-9a-fA-F]{1,4}:){1,7}:|" + r"([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|" + r"([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|" + r"([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|" + r"([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|" + r"([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|" + r"[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|" + r":((:[0-9a-fA-F]{1,4}){1,7}|:))$" + ) + def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True if not isinstance(value, str): return False - - # Try IPv4 - ipv4_pattern = re.compile( - r"^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}" - r"(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$" - ) - if ipv4_pattern.match(value): + if self.IPV4_PATTERN.match(value): return True - - # Try IPv6 - ipv6_pattern = re.compile( - r"^(([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|" - r"([0-9a-fA-F]{1,4}:){1,7}:|" - r"([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|" - r"([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|" - r"([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|" - r"([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|" - r"([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|" - r"[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|" - r":((:[0-9a-fA-F]{1,4}){1,7}|:))$" - ) - return bool(ipv6_pattern.match(value)) + return bool(self.IPV6_PATTERN.match(value)) class Ipv4Rule(NitroValidationRule): @@ -376,6 +413,9 @@ def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True + if isinstance(value, bool): + return False + if isinstance(value, (int, float)): return True @@ -427,22 +467,21 @@ def validate(self, field: str, value: Any, data: dict) -> bool: min_value = self.args[0] if self.args else 0 - # Convert string to appropriate type if isinstance(min_value, str): if "." in min_value: min_value = float(min_value) else: min_value = int(min_value) - # Check numeric values + if isinstance(value, bool): + return False + if isinstance(value, (int, float)): return value >= min_value - # Check string length if isinstance(value, str): return len(value) >= min_value - # Check collection size if isinstance(value, (list, dict, tuple)): return len(value) >= min_value @@ -461,22 +500,21 @@ def validate(self, field: str, value: Any, data: dict) -> bool: max_value = self.args[0] if self.args else 0 - # Convert string to appropriate type if isinstance(max_value, str): if "." in max_value: max_value = float(max_value) else: max_value = int(max_value) - # Check numeric values + if isinstance(value, bool): + return False + if isinstance(value, (int, float)): return value <= max_value - # Check string length if isinstance(value, str): return len(value) <= max_value - # Check collection size if isinstance(value, (list, dict, tuple)): return len(value) <= max_value @@ -499,7 +537,6 @@ def validate(self, field: str, value: Any, data: dict) -> bool: min_value = self.args[0] max_value = self.args[1] - # Convert strings to appropriate types if isinstance(min_value, str): if "." in min_value: min_value = float(min_value) @@ -512,15 +549,15 @@ def validate(self, field: str, value: Any, data: dict) -> bool: else: max_value = int(max_value) - # Check numeric values + if isinstance(value, bool): + return False + if isinstance(value, (int, float)): return min_value <= value <= max_value - # Check string length if isinstance(value, str): return min_value <= len(value) <= max_value - # Check collection size if isinstance(value, (list, dict, tuple)): return min_value <= len(value) <= max_value @@ -537,6 +574,9 @@ def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True + if isinstance(value, bool): + return False + try: num_value = float(value) if isinstance(value, str) else value return isinstance(num_value, (int, float)) and num_value > 0 @@ -554,6 +594,9 @@ def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True + if isinstance(value, bool): + return False + try: num_value = float(value) if isinstance(value, str) else value return isinstance(num_value, (int, float)) and num_value < 0 @@ -562,11 +605,18 @@ def validate(self, field: str, value: Any, data: dict) -> bool: class DivisibleByRule(NitroValidationRule): - """Validate that a field is divisible by a given number.""" + """Validate that a field is divisible by a given number. + + For float operands the remainder is compared against a small epsilon + to avoid spurious failures from binary floating-point representation + (e.g. ``1.0 % 0.1`` is not exactly zero). + """ name = "divisible_by" message = "The {field} field must be divisible by {0}." + _FLOAT_EPSILON = 1e-9 + def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True @@ -574,6 +624,9 @@ def validate(self, field: str, value: Any, data: dict) -> bool: if not self.args: return False + if isinstance(value, bool): + return False + try: divisor = float(self.args[0]) if isinstance(self.args[0], str) else self.args[0] num_value = float(value) if isinstance(value, str) else value @@ -581,6 +634,13 @@ def validate(self, field: str, value: Any, data: dict) -> bool: if not isinstance(num_value, (int, float)) or divisor == 0: return False + if isinstance(num_value, float) or isinstance(divisor, float): + remainder = abs(num_value % divisor) + return ( + remainder < self._FLOAT_EPSILON + or abs(remainder - abs(divisor)) < self._FLOAT_EPSILON + ) + return num_value % divisor == 0 except (ValueError, TypeError): return False @@ -623,8 +683,37 @@ def validate(self, field: str, value: Any, data: dict) -> bool: return value != other_value +def _value_matches_arg(value: Any, args: tuple) -> bool: + """Return True if ``value`` matches any entry in ``args``. + + Compares directly first, then falls back to string-vs-numeric + matching so that ``1`` matches ``("1", "2", "3")`` (the form rule + arguments take when parsed from a rule string). + """ + if value in args: + return True + if isinstance(value, bool): + return False + if isinstance(value, (int, float)): + if str(value) in args: + return True + for arg in args: + if isinstance(arg, str): + try: + if float(arg) == float(value): + return True + except ValueError: + continue + return False + + class InRule(NitroValidationRule): - """Validate that a field is in a list of values.""" + """Validate that a field is in a list of values. + + When the rule is parsed from a string (``"in:1,2,3"``) every argument + arrives as a string. Numeric values are still matched correctly: an + integer ``1`` compares equal to the string ``"1"``. + """ name = "in" message = "The {field} field must be one of: {args}." @@ -633,7 +722,7 @@ def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True - return value in self.args + return _value_matches_arg(value, self.args) class NotInRule(NitroValidationRule): @@ -646,7 +735,7 @@ def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True - return value not in self.args + return not _value_matches_arg(value, self.args) # ============================================================================ @@ -676,7 +765,13 @@ def validate(self, field: str, value: Any, data: dict) -> bool: class DateRule(NitroValidationRule): - """Validate that a field is a valid date.""" + """Validate that a field is a valid date. + + Only unambiguous ISO 8601 strings (``YYYY-MM-DD``, ``YYYY/MM/DD``, + ``YYYY-MM-DDTHH:MM:SS``) are accepted. Regional formats must go + through :class:`DateFormatRule` with an explicit ``strptime`` string + to avoid locale ambiguity. + """ name = "date" message = "The {field} field must be a valid date." @@ -684,35 +779,9 @@ class DateRule(NitroValidationRule): def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True - - from datetime import datetime - - if isinstance(value, datetime): - return True - - if not isinstance(value, str): + if not isinstance(value, (str, datetime)): return False - - # Try to parse unambiguous date formats (ISO 8601) - # Ambiguous formats like %d-%m-%Y and %m-%d-%Y are excluded - # to avoid incorrect parsing. Use date_format rule for specific formats. - date_formats = [ - "%Y-%m-%d", - "%Y/%m/%d", - "%Y-%m-%d %H:%M:%S", - "%Y/%m/%d %H:%M:%S", - "%Y-%m-%dT%H:%M:%S", - "%Y-%m-%dT%H:%M:%SZ", - ] - - for fmt in date_formats: - try: - datetime.strptime(value, fmt) - return True - except ValueError: - continue - - return False + return _parse_iso_date(value) is not None class BeforeRule(NitroValidationRule): @@ -724,50 +793,14 @@ class BeforeRule(NitroValidationRule): def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True - if not self.args: return False - - # Parse the value date - value_date = self._parse_date(value) - if not value_date: + value_date = _parse_iso_date(value) + compare_date = _parse_iso_date(self.args[0]) + if value_date is None or compare_date is None: return False - - # Parse the comparison date - compare_date = self._parse_date(self.args[0]) - if not compare_date: - return False - return value_date < compare_date - def _parse_date(self, date_value): - """Helper to parse a date from string or datetime object.""" - from datetime import datetime - - if isinstance(date_value, datetime): - return date_value - - if not isinstance(date_value, str): - return None - - # Unambiguous ISO 8601 formats only - date_formats = [ - "%Y-%m-%d", - "%Y/%m/%d", - "%Y-%m-%d %H:%M:%S", - "%Y/%m/%d %H:%M:%S", - "%Y-%m-%dT%H:%M:%S", - "%Y-%m-%dT%H:%M:%SZ", - ] - - for fmt in date_formats: - try: - return datetime.strptime(date_value, fmt) - except ValueError: - continue - - return None - class AfterRule(NitroValidationRule): """Validate that a date field is after a given date.""" @@ -778,50 +811,14 @@ class AfterRule(NitroValidationRule): def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True - if not self.args: return False - - # Parse the value date - value_date = self._parse_date(value) - if not value_date: - return False - - # Parse the comparison date - compare_date = self._parse_date(self.args[0]) - if not compare_date: + value_date = _parse_iso_date(value) + compare_date = _parse_iso_date(self.args[0]) + if value_date is None or compare_date is None: return False - return value_date > compare_date - def _parse_date(self, date_value): - """Helper to parse a date from string or datetime object.""" - from datetime import datetime - - if isinstance(date_value, datetime): - return date_value - - if not isinstance(date_value, str): - return None - - # Unambiguous ISO 8601 formats only - date_formats = [ - "%Y-%m-%d", - "%Y/%m/%d", - "%Y-%m-%d %H:%M:%S", - "%Y/%m/%d %H:%M:%S", - "%Y-%m-%dT%H:%M:%S", - "%Y-%m-%dT%H:%M:%SZ", - ] - - for fmt in date_formats: - try: - return datetime.strptime(date_value, fmt) - except ValueError: - continue - - return None - class DateEqualsRule(NitroValidationRule): """Validate that a date field equals a given date.""" @@ -832,50 +829,14 @@ class DateEqualsRule(NitroValidationRule): def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True - if not self.args: return False - - # Parse the value date - value_date = self._parse_date(value) - if not value_date: - return False - - # Parse the comparison date - compare_date = self._parse_date(self.args[0]) - if not compare_date: + value_date = _parse_iso_date(value) + compare_date = _parse_iso_date(self.args[0]) + if value_date is None or compare_date is None: return False - return value_date.date() == compare_date.date() - def _parse_date(self, date_value): - """Helper to parse a date from string or datetime object.""" - from datetime import datetime - - if isinstance(date_value, datetime): - return date_value - - if not isinstance(date_value, str): - return None - - # Unambiguous ISO 8601 formats only - date_formats = [ - "%Y-%m-%d", - "%Y/%m/%d", - "%Y-%m-%d %H:%M:%S", - "%Y/%m/%d %H:%M:%S", - "%Y-%m-%dT%H:%M:%S", - "%Y-%m-%dT%H:%M:%SZ", - ] - - for fmt in date_formats: - try: - return datetime.strptime(date_value, fmt) - except ValueError: - continue - - return None - class DateFormatRule(NitroValidationRule): """Validate that a date field matches a specific format.""" @@ -886,21 +847,16 @@ class DateFormatRule(NitroValidationRule): def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True - if not self.args: return False - if not isinstance(value, str): return False - from datetime import datetime - date_format = self.args[0] - try: datetime.strptime(value, date_format) return True - except ValueError: + except (ValueError, re.error): return False @@ -910,12 +866,20 @@ def validate(self, field: str, value: Any, data: dict) -> bool: class ConfirmedRule(NitroValidationRule): - """Validate that a field matches its confirmation field (field_confirmation).""" + """Validate that a field matches its confirmation field (field_confirmation). + + Skips the check when the field itself is empty: presence is the job + of ``required``. Pair with ``required`` if a missing field should + fail confirmation. + """ name = "confirmed" message = "The {field} confirmation does not match." def validate(self, field: str, value: Any, data: dict) -> bool: + if value is None or value == "": + return True + confirmation_field = f"{field}_confirmation" confirmation_value = data.get(confirmation_field) @@ -1131,18 +1095,18 @@ def validate(self, field: str, value: Any, data: dict) -> bool: target_size = self.args[0] - # Convert string to appropriate type if isinstance(target_size, str): if "." in target_size: target_size = float(target_size) else: target_size = int(target_size) - # For numbers, check the value itself + if isinstance(value, bool): + return False + if isinstance(value, (int, float)): return value == target_size - # For strings and collections, check the length if isinstance(value, (str, list, dict, tuple)): return len(value) == target_size @@ -1181,11 +1145,23 @@ def validate(self, field: str, value: Any, data: dict) -> bool: class TimezoneRule(NitroValidationRule): - """Validate that a field is a valid timezone identifier.""" + """Validate that a field is a valid timezone identifier. + + On Python 3.9+ the canonical IANA database is consulted via + :mod:`zoneinfo`. On older interpreters (3.7, 3.8) the rule falls + back to a permissive structural regex that accepts ``UTC``, ``GMT``, + fixed offsets like ``+02:00``, and ``Area/Location`` zones including + ones with digits, mixed case, or extra path segments + (e.g. ``America/New_York``, ``Etc/GMT+5``, ``US/Eastern``). + """ name = "timezone" message = "The {field} field must be a valid timezone." + _FALLBACK_PATTERN = re.compile( + r"^(" r"UTC|GMT" r"|[+-]\d{2}:?\d{2}" r"|[A-Za-z][A-Za-z0-9_+\-]*(/[A-Za-z0-9_+\-]+)+" r")$" + ) + def validate(self, field: str, value: Any, data: dict) -> bool: if value is None or value == "": return True @@ -1195,16 +1171,12 @@ def validate(self, field: str, value: Any, data: dict) -> bool: try: import zoneinfo - # Try to get the timezone zoneinfo.ZoneInfo(value) return True + except ImportError: + return bool(self._FALLBACK_PATTERN.match(value)) except Exception: - # Fallback: check common timezone patterns - # Format: Continent/City or UTC offset - timezone_pattern = re.compile( - r"^(UTC|GMT|[A-Z][a-z]+/[A-Z][a-z_]+(/[A-Z][a-z_]+)?|[+-]\d{2}:\d{2})$" - ) - return bool(timezone_pattern.match(value)) + return False class LocaleRule(NitroValidationRule): diff --git a/pyproject.toml b/pyproject.toml index bc5f36f..500009f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "nitro-validator" -version = "1.0.3" +version = "1.0.4" description = "A powerful, standalone, dependency-free data validation library for Python with extensible rules and a clean, intuitive API." readme = "README.md" authors = [ diff --git a/tests/test_full_coverage.py b/tests/test_full_coverage.py index b31cae0..9fc5c14 100644 --- a/tests/test_full_coverage.py +++ b/tests/test_full_coverage.py @@ -609,4 +609,4 @@ def test_locale_with_invalid_pattern_but_valid_format(self): assert rule.validate("field", "en", {}) is True assert rule.validate("field", "en_US", {}) is True # Invalid pattern should fail - assert rule.validate("field", "invalid_locale_code_xyz", {}) is False \ No newline at end of file + assert rule.validate("field", "invalid_locale_code_xyz", {}) is False diff --git a/tests/test_review_fixes.py b/tests/test_review_fixes.py new file mode 100644 index 0000000..daeb73b --- /dev/null +++ b/tests/test_review_fixes.py @@ -0,0 +1,321 @@ +"""Regression tests for fixes from the 2026-05 code review. + +Each test pins a specific bug or design issue identified during review so +the fix cannot silently regress. +""" + +import pytest +from nitro_validator import ( + Validator, + NitroValidationRule, + NitroRuleRegistry, + ValidationError, +) +from nitro_validator.utils import ( + RegexRule, + DateFormatRule, + ConfirmedRule, + NumericRule, + IntegerRule, + MinRule, + MaxRule, + BetweenRule, + PositiveRule, + NegativeRule, + DivisibleByRule, + SizeRule, + InRule, + NotInRule, + EmailRule, + TimezoneRule, +) + + +class TestRegexRuleSafe: + """RegexRule must not leak re.error to callers (#1).""" + + def test_malformed_pattern_returns_false_not_exception(self): + rule = RegexRule("[unclosed") + assert rule.validate("field", "anything", {}) is False + + def test_malformed_pattern_via_validator_is_caught(self): + validator = Validator() + rule = RegexRule("(*invalid") + ok = validator.is_valid({"x": "abc"}, {"x": [rule]}) + assert ok is False + + def test_pattern_compiled_once(self): + rule = RegexRule(r"^\d+$") + assert rule._compiled is not None + first = rule._compiled + rule.validate("f", "123", {}) + rule.validate("f", "456", {}) + assert rule._compiled is first + + +class TestRegistryIsolation: + """Registering a rule on one validator must not leak to others (#3).""" + + def test_register_rule_does_not_pollute_default_registry(self): + class ScratchRule(NitroValidationRule): + name = "scratch_review" + + def validate(self, field, value, data): + return True + + v1 = Validator() + v1.register_rule(ScratchRule) + assert v1.registry.has("scratch_review") + + v2 = Validator() + assert v2.registry.has("scratch_review") is False + + def test_two_validators_have_distinct_registries(self): + a = Validator() + b = Validator() + assert a.registry is not b.registry + + def test_explicit_registry_is_used_as_is(self): + custom = NitroRuleRegistry() + v = Validator(registry=custom) + assert v.registry is custom + + def test_registry_copy_is_independent(self): + a = NitroRuleRegistry() + a.register(EmailRule) + b = a.copy() + assert b.has("email") + b.unregister("email") + assert a.has("email") + assert b.has("email") is False + + +class TestDateFormatRuleSafe: + """DateFormatRule must not leak re.error from bad format strings (#4).""" + + def test_duplicate_directive_returns_false_not_exception(self): + rule = DateFormatRule("%Y%Y") + assert rule.validate("d", "20262026", {}) is False + + def test_garbage_format_returns_false(self): + rule = DateFormatRule("not a real format") + assert rule.validate("d", "anything", {}) is False + + +class TestTimezoneRuleFallbackPath: + """Exercise the ImportError fallback that triggers on 3.7/3.8.""" + + def test_fallback_used_when_zoneinfo_missing(self, monkeypatch): + import builtins + + real_import = builtins.__import__ + + def deny_zoneinfo(name, *args, **kwargs): + if name == "zoneinfo": + raise ImportError("zoneinfo not available") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", deny_zoneinfo) + + rule = TimezoneRule() + assert rule.validate("tz", "America/New_York", {}) is True + assert rule.validate("tz", "invalid-timezone", {}) is False + + +class TestTimezoneRuleFallback: + """The 3.7/3.8 fallback regex must cover real IANA zone shapes (#5).""" + + @pytest.mark.parametrize( + "zone", + [ + "America/New_York", + "US/Eastern", + "Etc/UTC", + "Etc/GMT+5", + "Europe/London", + "America/Argentina/Buenos_Aires", + "Pacific/Honolulu", + "UTC", + "GMT", + "+02:00", + "-0500", + ], + ) + def test_fallback_accepts_real_zones(self, zone): + assert TimezoneRule._FALLBACK_PATTERN.match(zone) is not None + + @pytest.mark.parametrize( + "junk", + ["invalid-timezone", "123-456", "Not/A/Real/zone with spaces", ""], + ) + def test_fallback_rejects_junk(self, junk): + assert TimezoneRule._FALLBACK_PATTERN.match(junk) is None + + +class TestInRuleNumericCoercion: + """InRule must match numeric values against string-form args (#6).""" + + def test_int_matches_string_args(self): + rule = InRule("1", "2", "3") + assert rule.validate("n", 1, {}) is True + assert rule.validate("n", 2, {}) is True + assert rule.validate("n", 4, {}) is False + + def test_float_matches_string_args(self): + rule = InRule("1.5", "2.5") + assert rule.validate("n", 1.5, {}) is True + assert rule.validate("n", 3.5, {}) is False + + def test_via_string_form(self): + v = Validator() + v.validate({"role": 1}, {"role": "in:1,2,3"}) + + def test_not_in_rule_with_numeric(self): + rule = NotInRule("1", "2", "3") + assert rule.validate("n", 1, {}) is False + assert rule.validate("n", 4, {}) is True + + def test_bool_not_treated_as_numeric(self): + rule = InRule("1", "2") + assert rule.validate("flag", True, {}) is False + + def test_non_numeric_string_arg_is_skipped(self): + rule = InRule("abc", "2") + assert rule.validate("n", 2, {}) is True + + def test_float_arg_matches_int_value(self): + rule = InRule("abc", "2.0") + assert rule.validate("n", 2, {}) is True + + def test_no_match_when_no_arg_coerces(self): + rule = InRule("abc", "def") + assert rule.validate("n", 2, {}) is False + + +class TestConfirmedRuleEmpty: + """ConfirmedRule must skip empty values like every other non-presence rule (#9).""" + + def test_passes_on_none(self): + rule = ConfirmedRule() + assert rule.validate("password", None, {}) is True + + def test_passes_on_empty_string(self): + rule = ConfirmedRule() + assert rule.validate("password", "", {}) is True + + def test_still_fails_when_value_present_and_confirmation_missing(self): + rule = ConfirmedRule() + assert rule.validate("password", "secret", {}) is False + + def test_pair_with_required_enforces_presence(self): + v = Validator() + ok = v.is_valid({}, {"password": "required|confirmed"}) + assert ok is False + + +class TestNumericRulesExcludeBool: + """Numeric rules should not silently accept bool (which subclasses int) (#10).""" + + def test_numeric_rejects_bool(self): + rule = NumericRule() + assert rule.validate("n", True, {}) is False + assert rule.validate("n", False, {}) is False + + def test_min_rejects_bool(self): + rule = MinRule("0") + assert rule.validate("n", True, {}) is False + + def test_max_rejects_bool(self): + rule = MaxRule("10") + assert rule.validate("n", True, {}) is False + + def test_between_rejects_bool(self): + rule = BetweenRule("0", "10") + assert rule.validate("n", True, {}) is False + + def test_positive_rejects_bool(self): + rule = PositiveRule() + assert rule.validate("n", True, {}) is False + + def test_negative_rejects_bool(self): + rule = NegativeRule() + assert rule.validate("n", False, {}) is False + + def test_divisible_by_rejects_bool(self): + rule = DivisibleByRule("2") + assert rule.validate("n", True, {}) is False + + def test_size_rejects_bool(self): + rule = SizeRule("1") + assert rule.validate("n", True, {}) is False + + def test_integer_still_rejects_bool(self): + rule = IntegerRule() + assert rule.validate("n", True, {}) is False + + +class TestDivisibleByFloat: + """DivisibleByRule must handle binary float artefacts (#11).""" + + def test_one_divided_by_point_one(self): + rule = DivisibleByRule("0.1") + assert rule.validate("n", 1.0, {}) is True + + def test_three_divided_by_point_three(self): + rule = DivisibleByRule("0.3") + assert rule.validate("n", 0.9, {}) is True + + def test_genuinely_indivisible_float(self): + rule = DivisibleByRule("0.3") + assert rule.validate("n", 1.0, {}) is False + + def test_integer_path_unchanged(self): + rule = DivisibleByRule(2) + assert rule.validate("n", 10, {}) is True + assert rule.validate("n", 7, {}) is False + + +class TestCustomMessageDoesNotPersist: + """Reusing a rule instance across calls must not retain prior custom messages (#12).""" + + def test_subsequent_call_without_messages_uses_default(self): + rule = EmailRule() + v = Validator() + + try: + v.validate({"e": "junk"}, {"e": [rule]}, messages={"e": "custom oops"}) + except ValidationError as exc: + assert exc.errors["e"] == ["custom oops"] + + try: + v.validate({"e": "still junk"}, {"e": [rule]}) + except ValidationError as exc: + assert exc.errors["e"] == ["The e field must be a valid email address."] + + def test_per_rule_override_then_default(self): + rule = EmailRule() + v = Validator() + + try: + v.validate( + {"e": "junk"}, + {"e": [rule]}, + messages={"e": {"email": "special email error"}}, + ) + except ValidationError as exc: + assert exc.errors["e"] == ["special email error"] + + try: + v.validate({"e": "junk"}, {"e": [rule]}) + except ValidationError as exc: + assert exc.errors["e"] == ["The e field must be a valid email address."] + + +class TestRegexRuleDocBehaviour: + """Sanity-check the documented workaround for pipe/comma in patterns (#2).""" + + def test_pattern_with_alternation_works_via_instance(self): + rule = RegexRule(r"^(foo|bar)$") + assert rule.validate("f", "foo", {}) is True + assert rule.validate("f", "bar", {}) is True + assert rule.validate("f", "baz", {}) is False