refactor: Removed 'required' attributes and simplify validation (DenverCoder1#43)

Author: DenverCoder1

api/index.py

@@ -28,10 +28,10 @@ def render():
         background_color = validate_color(request, "background_color", default="#0d1117")
         title_color = validate_color(request, "title_color", default="#ffffff")
         stats_color = validate_color(request, "stats_color", default="#dedede")
-        title = trim_text(validate_string(request, "title"), (width - 20) // 8)
+        title = trim_text(validate_string(request, "title", default=""), (width - 20) // 8)
         publish_timestamp = validate_int(request, "timestamp", default=0)
         duration_seconds = validate_int(request, "duration", default=0)
-        video_id = validate_video_id(request, "id", required=True)
+        video_id = validate_video_id(request, "id")
         thumbnail = data_uri_from_url(f"https://i.ytimg.com/vi/{video_id}/mqdefault.jpg")
         views = fetch_views(video_id)
         diff = (

api/validate.py

@@ -1,65 +1,37 @@
-from typing import Optional
 import re
 from flask.wrappers import Request
 
 
-def validate_int(req: Request, field: str, *, default: int = 0, required: bool = False) -> int:
-    """Validate an integer, returns the integer if valid, otherwise the default.
-
-    Raises ValueError if the field is required and the valid is not valid.
-    """
+def validate_int(req: Request, field: str, default: int = 0) -> int:
+    """Validate an integer, returns the integer if valid, otherwise the default."""
     value = req.args.get(field, "")
-    if value == "" and required:
-        raise ValueError(f"Required parameter '{field}' is missing")
     try:
         return int(value)
     except ValueError:
-        if required:
-            raise ValueError(f"{field} expects an integer but got '{value}'")
         return default
 
 
-def validate_color(
-    req: Request, field: str, *, default: str = "#ffffff", required: bool = False
-) -> str:
-    """Validate a color, returns the color if valid hex code (3, 4, 6, or 8 characters), otherwise the default.
-
-    Raises ValueError if the field is required and the valid is not valid.
-    """
+def validate_color(req: Request, field: str, default: str = "#ffffff") -> str:
+    """Validate a color, returns the color if it's a valid hex code (3, 4, 6, or 8 characters), otherwise the default."""
     value = req.args.get(field, "")
-    if value == "" and required:
-        raise ValueError(f"Required parameter '{field}' is missing")
     hex_digits = re.sub(r"[^a-fA-F0-9]", "", value)
     if len(hex_digits) not in (3, 4, 6, 8):
-        if required:
-            raise ValueError(f"{field} expects a hex color but got '{value}'")
         return default
     return f"#{hex_digits}"
 
 
-def validate_video_id(
-    req: Request, field: str, *, default: str = "", required: bool = False
-) -> str:
-    """Validate a video ID, returns the video ID if valid, otherwise the default.
+def validate_video_id(req: Request, field: str) -> str:
+    """Validate a video ID, returns the video ID if valid.
 
-    Raises ValueError if the field is required and the valid is not valid.
-    """
+    Raises ValueError if the field is not provided or fails the validation regex."""
     value = req.args.get(field, "")
-    if value == "" and required:
+    if value == "":
         raise ValueError(f"Required parameter '{field}' is missing")
     if not re.match(r"^[a-zA-Z0-9_-]+$", value):
-        if required:
-            raise ValueError(f"{field} expects a video ID but got '{value}'")
-        return default
+        raise ValueError(f"{field} expects a video ID but got '{value}'")
     return value
 
 
-def validate_string(req: Request, field: str, *, required: bool = False) -> str:
-    """Validate a string, returns the string if valid, otherwise the default.
-
-    Raises ValueError if the field is required and the valid is not valid.
-    """
-    value = req.args.get(field, "")
-    if value == "" and required:
-        raise ValueError(f"Required parameter '{field}' is missing")
-    return value
+def validate_string(req: Request, field: str, default: str = "") -> str:
+    """Validate a string, returns the string if valid, otherwise the default."""
+    return req.args.get(field, default)

tests/test_validate.py

@@ -0,0 +1,67 @@
+import pytest
+from api.validate import validate_color, validate_int, validate_string, validate_video_id
+from flask.wrappers import Request
+
+
+class MockRequest(Request):
+    def __init__(self, **kwargs):
+        self.args = kwargs  # type: ignore
+
+
+def test_validate_int():
+    # missing field
+    req = MockRequest()
+    assert validate_int(req, "width", default=250) == 250
+    # invalid field
+    req = MockRequest(width="abc")
+    assert validate_int(req, "width", default=250) == 250
+    # valid field
+    req = MockRequest(width="100")
+    assert validate_int(req, "width", default=250) == 100
+
+
+def test_validate_color():
+    # missing field
+    req = MockRequest()
+    assert validate_color(req, "background_color", default="#0d1117") == "#0d1117"
+    # invalid field characters
+    req = MockRequest(background_color="#fghijk")
+    assert validate_color(req, "background_color", default="#0d1117") == "#0d1117"
+    # invalid field length
+    req = MockRequest(background_color="#012345678")
+    assert validate_color(req, "background_color", default="#0d1117") == "#0d1117"
+    # valid field - 6 characters
+    req = MockRequest(background_color="#ffffff")
+    assert validate_color(req, "background_color", default="#0d1117") == "#ffffff"
+    # valid field - 8 characters
+    req = MockRequest(background_color="#01234567")
+    assert validate_color(req, "background_color", default="#0d1117") == "#01234567"
+    # valid field - 4 characters
+    req = MockRequest(background_color="#89ab")
+    assert validate_color(req, "background_color", default="#0d1117") == "#89ab"
+    # valid field - 3 characters
+    req = MockRequest(background_color="#cde")
+    assert validate_color(req, "background_color", default="#0d1117") == "#cde"
+
+
+def test_validate_video_id():
+    # missing field
+    req = MockRequest()
+    with pytest.raises(ValueError):
+        validate_video_id(req, "id")
+    # invalid field
+    req = MockRequest(id="<script>alert('xss')</script>")
+    with pytest.raises(ValueError):
+        validate_video_id(req, "id")
+    # valid field
+    req = MockRequest(id="abc_123-456")
+    assert validate_video_id(req, "id") == "abc_123-456"
+
+
+def test_validate_string():
+    # missing field
+    req = MockRequest()
+    assert validate_string(req, "text", default="Hello, world!") == "Hello, world!"
+    # valid field
+    req = MockRequest(text="Hello, world!")
+    assert validate_string(req, "text", default="Hello, world!") == "Hello, world!"

tox.ini

@@ -4,7 +4,7 @@ isolated_build = True
 
 [testenv]
 deps = 
-    orjson
-    feedparser
     pytest
+    feedparser
+    -rrequirements.txt
 commands = pytest tests -s