Skip to content

Commit d3716ee

Browse files
authored
fix: resolve migration crashes from null values and malformed regex patterns (#233)
Corrects issues when parsing empty migrations and other edge cases for migrations.
1 parent cd88b4a commit d3716ee

File tree

5 files changed

+153
-27
lines changed

5 files changed

+153
-27
lines changed

sqlspec/migrations/fix.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ def apply_renames(self, renames: "list[MigrationRename]", dry_run: bool = False)
140140

141141
rename.old_path.rename(rename.new_path)
142142

143-
def update_file_content(self, file_path: Path, old_version: str, new_version: str) -> None:
143+
def update_file_content(self, file_path: Path, old_version: "str | None", new_version: "str | None") -> None:
144144
"""Update SQL query names and version comments in file content.
145145
146146
Transforms query names and version metadata from old version to new version:
@@ -153,10 +153,14 @@ def update_file_content(self, file_path: Path, old_version: str, new_version: st
153153
154154
Args:
155155
file_path: Path to file to update.
156-
old_version: Old version string.
157-
new_version: New version string.
156+
old_version: Old version string (None values skipped gracefully).
157+
new_version: New version string (None values skipped gracefully).
158158
159159
"""
160+
if not old_version or not new_version:
161+
logger.warning("Skipping content update - missing version information")
162+
return
163+
160164
content = file_path.read_text(encoding="utf-8")
161165

162166
up_pattern = re.compile(rf"(-- name:\s+migrate-){re.escape(old_version)}(-up)")

sqlspec/migrations/validation.py

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
from sqlspec.utils.version import parse_version
1515

1616
if TYPE_CHECKING:
17+
from collections.abc import Sequence
18+
1719
from sqlspec.utils.version import MigrationVersion
1820

1921
__all__ = ("MigrationGap", "detect_out_of_order_migrations", "format_out_of_order_warning")
@@ -39,7 +41,7 @@ class MigrationGap:
3941

4042

4143
def detect_out_of_order_migrations(
42-
pending_versions: "list[str]", applied_versions: "list[str]"
44+
pending_versions: "Sequence[str | None]", applied_versions: "Sequence[str | None]"
4345
) -> "list[MigrationGap]":
4446
"""Detect migrations created before already-applied migrations.
4547
@@ -51,29 +53,26 @@ def detect_out_of_order_migrations(
5153
independent sequences within their own namespaces.
5254
5355
Args:
54-
pending_versions: List of migration versions not yet applied.
55-
applied_versions: List of migration versions already applied.
56+
pending_versions: List of migration versions not yet applied (may contain None).
57+
applied_versions: List of migration versions already applied (may contain None).
5658
5759
Returns:
58-
List of migration gaps representing out-of-order migrations.
59-
Empty list if no out-of-order migrations detected.
60-
61-
Example:
62-
Applied: [20251011120000, 20251012140000]
63-
Pending: [20251011130000, 20251013090000]
64-
Result: Gap for 20251011130000 (created between applied migrations)
65-
66-
Applied: [ext_litestar_0001, 0001, 0002]
67-
Pending: [ext_adk_0001]
68-
Result: [] (extensions excluded from out-of-order detection)
60+
List of migration gaps where pending versions are older than applied.
6961
"""
7062
if not applied_versions or not pending_versions:
7163
return []
7264

7365
gaps: list[MigrationGap] = []
7466

75-
parsed_applied = [parse_version(v) for v in applied_versions]
76-
parsed_pending = [parse_version(v) for v in pending_versions]
67+
# Filter out None values, empty strings, and whitespace-only strings
68+
valid_applied = [v for v in applied_versions if v is not None and v.strip()]
69+
valid_pending = [v for v in pending_versions if v is not None and v.strip()]
70+
71+
if not valid_applied or not valid_pending:
72+
return []
73+
74+
parsed_applied = [parse_version(v) for v in valid_applied]
75+
parsed_pending = [parse_version(v) for v in valid_pending]
7776

7877
core_applied = [v for v in parsed_applied if v.extension is None]
7978
core_pending = [v for v in parsed_pending if v.extension is None]

sqlspec/utils/version.py

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
logger = logging.getLogger(__name__)
2727

28-
SEQUENTIAL_PATTERN = re.compile(r"^(?!\d{14}$)(\d+)$")
28+
SEQUENTIAL_PATTERN = re.compile(r"^(?!\d{14}$)\d+$")
2929
TIMESTAMP_PATTERN = re.compile(r"^(\d{14})$")
3030
EXTENSION_PATTERN = re.compile(r"^ext_(\w+)_(.+)$")
3131

@@ -135,7 +135,7 @@ def __repr__(self) -> str:
135135
return f"MigrationVersion({self.type.value}={self.raw})"
136136

137137

138-
def is_sequential_version(version_str: str) -> bool:
138+
def is_sequential_version(version_str: "str | None") -> bool:
139139
"""Check if version string is sequential format.
140140
141141
Sequential format: Any sequence of digits (0001, 42, 9999, 10000+).
@@ -144,7 +144,7 @@ def is_sequential_version(version_str: str) -> bool:
144144
version_str: Version string to check.
145145
146146
Returns:
147-
True if sequential format.
147+
True if sequential format, False if None or whitespace.
148148
149149
Examples:
150150
>>> is_sequential_version("0001")
@@ -155,11 +155,15 @@ def is_sequential_version(version_str: str) -> bool:
155155
True
156156
>>> is_sequential_version("20251011120000")
157157
False
158+
>>> is_sequential_version(None)
159+
False
158160
"""
161+
if version_str is None or not version_str.strip():
162+
return False
159163
return bool(SEQUENTIAL_PATTERN.match(version_str))
160164

161165

162-
def is_timestamp_version(version_str: str) -> bool:
166+
def is_timestamp_version(version_str: "str | None") -> bool:
163167
"""Check if version string is timestamp format.
164168
165169
Timestamp format: 14-digit YYYYMMDDHHmmss (20251011120000).
@@ -168,14 +172,18 @@ def is_timestamp_version(version_str: str) -> bool:
168172
version_str: Version string to check.
169173
170174
Returns:
171-
True if timestamp format.
175+
True if timestamp format, False if None or whitespace.
172176
173177
Examples:
174178
>>> is_timestamp_version("20251011120000")
175179
True
176180
>>> is_timestamp_version("0001")
177181
False
182+
>>> is_timestamp_version(None)
183+
False
178184
"""
185+
if version_str is None or not version_str.strip():
186+
return False
179187
if not TIMESTAMP_PATTERN.match(version_str):
180188
return False
181189

@@ -187,7 +195,7 @@ def is_timestamp_version(version_str: str) -> bool:
187195
return True
188196

189197

190-
def parse_version(version_str: str) -> MigrationVersion:
198+
def parse_version(version_str: "str | None") -> MigrationVersion:
191199
"""Parse version string into structured format.
192200
193201
Supports:
@@ -202,7 +210,7 @@ def parse_version(version_str: str) -> MigrationVersion:
202210
Parsed migration version.
203211
204212
Raises:
205-
ValueError: If version format is invalid.
213+
ValueError: If version format is invalid, None, or whitespace-only.
206214
207215
Examples:
208216
>>> v = parse_version("0001")
@@ -219,6 +227,10 @@ def parse_version(version_str: str) -> MigrationVersion:
219227
>>> v.extension
220228
'litestar'
221229
"""
230+
if version_str is None or not version_str.strip():
231+
msg = "Invalid migration version: version string is None or empty"
232+
raise ValueError(msg)
233+
222234
extension_match = EXTENSION_PATTERN.match(version_str)
223235
if extension_match:
224236
extension_name = extension_match.group(1)
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
"""Test cases for null handling fixes in migration system."""
2+
3+
import tempfile
4+
from pathlib import Path
5+
6+
import pytest
7+
8+
from sqlspec.migrations.fix import MigrationFixer
9+
from sqlspec.migrations.validation import detect_out_of_order_migrations
10+
from sqlspec.utils.version import is_sequential_version, is_timestamp_version, parse_version
11+
12+
13+
class TestNullHandlingFixes:
14+
"""Test fixes for None value handling in migrations."""
15+
16+
def test_parse_version_with_none(self):
17+
"""Test parse_version handles None gracefully."""
18+
with pytest.raises(ValueError, match="Invalid migration version: version string is None or empty"):
19+
parse_version(None)
20+
21+
def test_parse_version_with_empty_string(self):
22+
"""Test parse_version handles empty string gracefully."""
23+
with pytest.raises(ValueError, match="Invalid migration version: version string is None or empty"):
24+
parse_version("")
25+
26+
def test_parse_version_with_whitespace_only(self):
27+
"""Test parse_version handles whitespace-only strings."""
28+
with pytest.raises(ValueError, match="Invalid migration version: version string is None or empty"):
29+
parse_version(" ")
30+
31+
def test_parse_version_valid_formats_still_work(self):
32+
"""Test that valid version formats still work after fixes."""
33+
# Sequential versions
34+
result = parse_version("0001")
35+
assert result.type.value == "sequential"
36+
assert result.sequence == 1
37+
38+
result = parse_version("9999")
39+
assert result.type.value == "sequential"
40+
assert result.sequence == 9999
41+
42+
# Timestamp versions
43+
result = parse_version("20251011120000")
44+
assert result.type.value == "timestamp"
45+
assert result.timestamp is not None
46+
47+
# Extension versions
48+
result = parse_version("ext_litestar_0001")
49+
assert result.type.value == "sequential" # Base is sequential
50+
assert result.extension == "litestar"
51+
52+
def test_migration_fixer_handles_none_gracefully(self):
53+
"""Test MigrationFixer.update_file_content handles None values."""
54+
with tempfile.TemporaryDirectory() as temp_dir:
55+
migrations_path = Path(temp_dir)
56+
fixer = MigrationFixer(migrations_path)
57+
58+
test_file = migrations_path / "test.sql"
59+
test_file.write_text("-- Test content")
60+
61+
# Should not crash with None values
62+
fixer.update_file_content(test_file, None, "0001")
63+
fixer.update_file_content(test_file, "0001", None)
64+
fixer.update_file_content(test_file, None, None)
65+
66+
# File should remain unchanged
67+
content = test_file.read_text()
68+
assert content == "-- Test content"
69+
70+
def test_validation_filters_none_values(self):
71+
"""Test migration validation filters None values properly."""
72+
# Should not crash with None values in lists
73+
gaps = detect_out_of_order_migrations(
74+
pending_versions=["0001", None, "0003", ""], applied_versions=[None, "0002", " ", "0004"]
75+
)
76+
77+
# Should only process valid versions
78+
assert len(gaps) >= 0 # Should not crash
79+
80+
def test_sequential_pattern_edge_cases(self):
81+
"""Test sequential pattern handles edge cases."""
82+
assert is_sequential_version("0001")
83+
assert is_sequential_version("9999")
84+
assert is_sequential_version("10000")
85+
assert not is_sequential_version("20251011120000") # Timestamp
86+
assert not is_sequential_version("abc")
87+
assert not is_sequential_version("")
88+
assert not is_sequential_version(None)
89+
90+
def test_timestamp_pattern_edge_cases(self):
91+
"""Test timestamp pattern handles edge cases."""
92+
assert is_timestamp_version("20251011120000")
93+
assert is_timestamp_version("20250101000000")
94+
assert is_timestamp_version("20251231235959")
95+
assert not is_timestamp_version("0001") # Sequential
96+
assert not is_timestamp_version("2025101112000") # Too short
97+
assert not is_timestamp_version("202510111200000") # Too long
98+
assert not is_timestamp_version("")
99+
assert not is_timestamp_version(None)
100+
101+
def test_error_messages_are_descriptive(self):
102+
"""Test that error messages are helpful for debugging."""
103+
try:
104+
parse_version(None)
105+
except ValueError as e:
106+
assert "version string is None or empty" in str(e)
107+
108+
try:
109+
parse_version("")
110+
except ValueError as e:
111+
assert "version string is None or empty" in str(e)

tests/unit/test_migrations/test_version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ def test_parse_invalid_version() -> None:
9595
with pytest.raises(ValueError, match="Invalid migration version format"):
9696
parse_version("abc")
9797

98-
with pytest.raises(ValueError, match="Invalid migration version format"):
98+
with pytest.raises(ValueError, match="Invalid migration version"):
9999
parse_version("")
100100

101101
with pytest.raises(ValueError, match="Invalid migration version format"):

0 commit comments

Comments
 (0)