Add naming validation for flow identifiers (#779) (#783)

* Add naming validation for flow identifiers

- Implement validation module for flow names, field names, and targets
- Enforce 64-char limit and alphanumeric+underscore restrictions
- Prevent double-underscore prefixes (reserved for internal use)
- Add comprehensive test coverage with 11 test cases
- Maintain backward compatibility

Fixes #779

* Address PR review feedback

- Remove validation functions from public API (internal use only)
- Simplify exception handling - let NamingError be raised directly
- Pre-compile regex patterns for performance optimization
- Add field name validation in DataScope.__setitem__
- Add target name validation in DataCollector.export

* Refactor: Move validation imports to top-level and ensure app_namespace and full_flow_name validation

* Refactor: Move flow name validation to Flow.__init__

Author: theparthgupta

python/cocoindex/flow.py

@@ -10,6 +10,13 @@
 import inspect
 import re
 
+from .validation import (
+    validate_flow_name,
+    NamingError,
+    validate_full_flow_name,
+    validate_target_name,
+)
+
 from dataclasses import dataclass
 from enum import Enum
 from threading import Lock
@@ -300,6 +307,9 @@ def __getitem__(self, field_name: str) -> DataSlice[T]:
         )
 
     def __setitem__(self, field_name: str, value: DataSlice[T]) -> None:
+        from .validation import validate_field_name
+
+        validate_field_name(field_name)
         value._state.attach_to_scope(self._engine_data_scope, field_name)
 
     def __enter__(self) -> DataScope:
@@ -367,7 +377,7 @@ def collect(self, **kwargs: Any) -> None:
 
     def export(
         self,
-        name: str,
+        target_name: str,
         target_spec: op.TargetSpec,
         /,
         *,
@@ -381,6 +391,8 @@ def export(
 
         `vector_index` is for backward compatibility only. Please use `vector_indexes` instead.
         """
+
+        validate_target_name(target_name)
         if not isinstance(target_spec, op.TargetSpec):
             raise ValueError(
                 "export() can only be called on a CocoIndex target storage"
@@ -398,7 +410,7 @@ def export(
             vector_indexes=vector_indexes,
         )
         self._flow_builder_state.engine_flow_builder.export(
-            name,
+            target_name,
             _spec_kind(target_spec),
             dump_engine_object(target_spec),
             dump_engine_object(index_options),
@@ -660,6 +672,8 @@ class Flow:
     def __init__(
         self, name: str, full_name: str, engine_flow_creator: Callable[[], _engine.Flow]
     ):
+        validate_flow_name(name)
+        validate_full_flow_name(full_name)
         self._name = name
         self._full_name = full_name
         engine_flow = None
@@ -831,11 +845,6 @@ def get_flow_full_name(name: str) -> str:
 
 
 def add_flow_def(name: str, fl_def: Callable[[FlowBuilder, DataScope], None]) -> Flow:
-    """Add a flow definition to the cocoindex library."""
-    if not all(c.isalnum() or c == "_" for c in name):
-        raise ValueError(
-            f"Flow name '{name}' contains invalid characters. Only alphanumeric characters and underscores are allowed."
-        )
     with _flows_lock:
         if name in _flows:
             raise KeyError(f"Flow with name {name} already exists")

python/cocoindex/setting.py

@@ -6,6 +6,7 @@
 
 from typing import Callable, Self, Any, overload
 from dataclasses import dataclass
+from .validation import validate_app_namespace_name
 
 _app_namespace: str = ""
 
@@ -27,6 +28,8 @@ def split_app_namespace(full_name: str, delimiter: str) -> tuple[str, str]:
 
 def set_app_namespace(app_namespace: str) -> None:
     """Set the application namespace."""
+    if app_namespace:
+        validate_app_namespace_name(app_namespace)
     global _app_namespace  # pylint: disable=global-statement
     _app_namespace = app_namespace
 

python/cocoindex/tests/test_validation.py

@@ -0,0 +1,134 @@
+"""Tests for naming validation functionality."""
+
+import pytest
+from cocoindex.validation import (
+    validate_field_name,
+    validate_flow_name,
+    validate_full_flow_name,
+    validate_app_namespace_name,
+    validate_target_name,
+    NamingError,
+    validate_identifier_name,
+)
+
+
+class TestValidateIdentifierName:
+    """Test the core validation function."""
+
+    def test_valid_names(self) -> None:
+        """Test that valid names pass validation."""
+        valid_names = [
+            "field1",
+            "field_name",
+            "_private",
+            "a",
+            "field123",
+            "FIELD_NAME",
+            "MyField",
+            "field_123_test",
+        ]
+
+        for name in valid_names:
+            result = validate_identifier_name(name)
+            assert result is None, f"Valid name '{name}' failed validation: {result}"
+
+    def test_valid_names_with_dots(self) -> None:
+        """Test that valid names with dots pass validation when allowed."""
+        valid_names = ["app.flow", "my_app.my_flow", "namespace.sub.flow", "a.b.c.d"]
+
+        for name in valid_names:
+            result = validate_identifier_name(name, allow_dots=True)
+            assert result is None, (
+                f"Valid dotted name '{name}' failed validation: {result}"
+            )
+
+    def test_invalid_starting_characters(self) -> None:
+        """Test names with invalid starting characters."""
+        invalid_names = [
+            "123field",  # starts with digit
+            ".field",  # starts with dot
+            "-field",  # starts with dash
+            " field",  # starts with space
+        ]
+
+        for name in invalid_names:
+            result = validate_identifier_name(name)
+            assert result is not None, (
+                f"Invalid name '{name}' should have failed validation"
+            )
+
+    def test_double_underscore_restriction(self) -> None:
+        """Test double underscore restriction."""
+        invalid_names = ["__reserved", "__internal", "__test"]
+
+        for name in invalid_names:
+            result = validate_identifier_name(name)
+            assert result is not None
+            assert "double underscores" in result.lower()
+
+    def test_length_restriction(self) -> None:
+        """Test maximum length restriction."""
+        long_name = "a" * 65
+        result = validate_identifier_name(long_name, max_length=64)
+        assert result is not None
+        assert "maximum length" in result.lower()
+
+
+class TestSpecificValidators:
+    """Test the specific validation functions."""
+
+    def test_valid_field_names(self) -> None:
+        """Test valid field names."""
+        valid_names = ["field1", "field_name", "_private", "FIELD"]
+        for name in valid_names:
+            validate_field_name(name)  # Should not raise
+
+    def test_invalid_field_names(self) -> None:
+        """Test invalid field names raise NamingError."""
+        invalid_names = ["123field", "field-name", "__reserved", "a" * 65]
+
+        for name in invalid_names:
+            with pytest.raises(NamingError):
+                validate_field_name(name)
+
+    def test_flow_validation(self) -> None:
+        """Test flow name validation."""
+        # Valid flow names
+        validate_flow_name("MyFlow")
+        validate_flow_name("my_flow_123")
+
+        # Invalid flow names
+        with pytest.raises(NamingError):
+            validate_flow_name("123flow")
+
+        with pytest.raises(NamingError):
+            validate_flow_name("__reserved_flow")
+
+    def test_full_flow_name_allows_dots(self) -> None:
+        """Test that full flow names allow dots."""
+        validate_full_flow_name("app.my_flow")
+        validate_full_flow_name("namespace.subnamespace.flow")
+
+        # But still reject invalid patterns
+        with pytest.raises(NamingError):
+            validate_full_flow_name("123.invalid")
+
+    def test_target_validation(self) -> None:
+        """Test target name validation."""
+        validate_target_name("my_target")
+        validate_target_name("output_table")
+
+        with pytest.raises(NamingError):
+            validate_target_name("123target")
+
+    def test_app_namespace_validation(self) -> None:
+        """Test app namespace validation."""
+        validate_app_namespace_name("myapp")
+        validate_app_namespace_name("my_app_123")
+
+        # Should not allow dots in app namespace
+        with pytest.raises(NamingError):
+            validate_app_namespace_name("my.app")
+
+        with pytest.raises(NamingError):
+            validate_app_namespace_name("123app")

python/cocoindex/validation.py

@@ -0,0 +1,104 @@
+"""
+Naming validation for CocoIndex identifiers.
+
+This module enforces naming conventions for flow names, field names,
+target names, and app namespace names as specified in issue #779.
+"""
+
+import re
+from typing import Optional
+
+_IDENTIFIER_PATTERN = re.compile(r"^[a-zA-Z_][a-zA-Z0-9_]*$")
+_IDENTIFIER_WITH_DOTS_PATTERN = re.compile(r"^[a-zA-Z_][a-zA-Z0-9_.]*$")
+
+
+class NamingError(ValueError):
+    """Exception raised for naming convention violations."""
+
+    pass
+
+
+def validate_identifier_name(
+    name: str,
+    max_length: int = 64,
+    allow_dots: bool = False,
+    identifier_type: str = "identifier",
+) -> Optional[str]:
+    """
+    Validate identifier names according to CocoIndex naming rules.
+
+    Args:
+        name: The name to validate
+        max_length: Maximum allowed length (default 64)
+        allow_dots: Whether to allow dots in the name (for full flow names)
+        identifier_type: Type of identifier for error messages
+
+    Returns:
+        None if valid, error message string if invalid
+    """
+    if not name:
+        return f"{identifier_type} name cannot be empty"
+
+    if len(name) > max_length:
+        return f"{identifier_type} name '{name}' exceeds maximum length of {max_length} characters"
+
+    if name.startswith("__"):
+        return f"{identifier_type} name '{name}' cannot start with double underscores (reserved for internal usage)"
+
+    # Define allowed pattern
+    if allow_dots:
+        pattern = _IDENTIFIER_WITH_DOTS_PATTERN
+        allowed_chars = "letters, digits, underscores, and dots"
+    else:
+        pattern = _IDENTIFIER_PATTERN
+        allowed_chars = "letters, digits, and underscores"
+
+    if not pattern.match(name):
+        return f"{identifier_type} name '{name}' must start with a letter or underscore and contain only {allowed_chars}"
+
+    return None
+
+
+def validate_field_name(name: str) -> None:
+    """Validate field names."""
+    error = validate_identifier_name(
+        name, max_length=64, allow_dots=False, identifier_type="Field"
+    )
+    if error:
+        raise NamingError(error)
+
+
+def validate_flow_name(name: str) -> None:
+    """Validate flow names."""
+    error = validate_identifier_name(
+        name, max_length=64, allow_dots=False, identifier_type="Flow"
+    )
+    if error:
+        raise NamingError(error)
+
+
+def validate_full_flow_name(name: str) -> None:
+    """Validate full flow names (can contain dots for namespacing)."""
+    error = validate_identifier_name(
+        name, max_length=64, allow_dots=True, identifier_type="Full flow"
+    )
+    if error:
+        raise NamingError(error)
+
+
+def validate_app_namespace_name(name: str) -> None:
+    """Validate app namespace names."""
+    error = validate_identifier_name(
+        name, max_length=64, allow_dots=False, identifier_type="App namespace"
+    )
+    if error:
+        raise NamingError(error)
+
+
+def validate_target_name(name: str) -> None:
+    """Validate target names."""
+    error = validate_identifier_name(
+        name, max_length=64, allow_dots=False, identifier_type="Target"
+    )
+    if error:
+        raise NamingError(error)