Skip to content

Commit 1f54441

Browse files
authored
Fix color tuple JSON round-trip in BackgroundLayer (#7)
1 parent 7cecf3e commit 1f54441

4 files changed

Lines changed: 66 additions & 16 deletions

File tree

quickthumb/canvas.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2747,7 +2747,10 @@ def _download_and_cache_font(self, url: str) -> str:
27472747
cached_header = f.read(4)
27482748
if any(cached_header.startswith(magic) for magic in self._VALID_FONT_MAGIC):
27492749
return cache_path
2750-
os.remove(cache_path) # stale invalid cache — re-download
2750+
try:
2751+
os.remove(cache_path) # stale invalid cache — re-download
2752+
except OSError:
2753+
pass # non-fatal: proceed to re-download; write will overwrite or fail cleanly
27512754

27522755
try:
27532756
with urlopen(url) as response:

quickthumb/models.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,18 @@ def validate_color(cls, v: str | tuple | None) -> str | tuple | None:
344344
return v
345345

346346
# HexColor validation already applied to str, just validate tuple
347-
if isinstance(v, tuple) and len(v) not in (3, 4):
348-
raise ValueError(f"invalid color tuple: {v}")
347+
if isinstance(v, tuple):
348+
if len(v) not in (3, 4) or not all(isinstance(c, int) and 0 <= c <= 255 for c in v):
349+
raise ValueError(f"invalid color tuple: {v}")
349350

350351
return v
351352

353+
@field_serializer("color")
354+
def serialize_color(self, v: str | tuple | None) -> str | None:
355+
if v is None or isinstance(v, str):
356+
return v
357+
return "#" + "".join(f"{c:02X}" for c in v)
358+
352359

353360
class TextLayer(QuickThumbModel):
354361
type: Literal["text"]

specs/TASKS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@
7676

7777
### P3 — Lower Priority
7878

79-
- [TODO] Fix color tuple JSON round-trip: `BackgroundLayer.color` accepts RGB tuples but they break `to_json()` / `from_json()`
79+
- [DONE] Fix color tuple JSON round-trip: `BackgroundLayer.color` accepts RGB tuples but they break `to_json()` / `from_json()`
8080
- [TODO] Font metadata reading: use `fonttools` to read font weight/style from file metadata instead of relying on filename parsing
8181
- [TODO] Split `canvas.py` (currently 2466 lines) into smaller modules before it becomes a maintenance burden
8282

tests/test_background_layers.py

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,12 @@ def test_should_add_radial_gradient_background_with_custom_center(self):
213213
assert gradient.center == (0.3, 0.7)
214214

215215
def test_should_serialize_background_layer_to_json(self):
216-
"""Test that canvas with background layers can be serialized to JSON"""
217-
# Given: Canvas with multiple background layers
216+
"""Test that canvas with background layers can be serialized to JSON.
217+
218+
Includes a tuple-color layer to verify that RGB/RGBA tuples are serialized as
219+
hex strings (not JSON arrays) so the output is spec-compliant and round-trippable.
220+
"""
221+
# Given: Canvas with multiple background layers, including a tuple color
218222
import json
219223

220224
from quickthumb import BlendMode, Canvas, Filter, LinearGradient
@@ -224,9 +228,12 @@ def test_should_serialize_background_layer_to_json(self):
224228
Canvas(1920, 1080)
225229
.background(color="#2c3e50", effects=[Filter(blur=5, brightness=0.8)])
226230
.background(gradient=gradient, opacity=0.5, blend_mode=BlendMode.MULTIPLY)
231+
.background(color=(255, 87, 51))
232+
.background(color=(255, 87, 51, 200))
227233
)
228234

229-
# When/Then: Serialized JSON matches full expected structure
235+
# When/Then: Serialized JSON matches full expected structure;
236+
# tuple colors appear as hex strings, not arrays.
230237
assert json.loads(canvas.to_json()) == snapshot(
231238
{
232239
"width": 1920,
@@ -264,10 +271,36 @@ def test_should_serialize_background_layer_to_json(self):
264271
"fit": None,
265272
"effects": [],
266273
},
274+
{
275+
"type": "background",
276+
"color": "#FF5733",
277+
"gradient": None,
278+
"image": None,
279+
"opacity": 1.0,
280+
"blend_mode": None,
281+
"fit": None,
282+
"effects": [],
283+
},
284+
{
285+
"type": "background",
286+
"color": "#FF5733C8",
287+
"gradient": None,
288+
"image": None,
289+
"opacity": 1.0,
290+
"blend_mode": None,
291+
"fit": None,
292+
"effects": [],
293+
},
267294
],
268295
}
269296
)
270297

298+
# Round-trip: after from_json the tuple-color layers come back as hex strings,
299+
# confirming the serializer normalises them correctly.
300+
roundtrip = Canvas.from_json(canvas.to_json())
301+
assert roundtrip.layers[2].color == "#FF5733"
302+
assert roundtrip.layers[3].color == "#FF5733C8"
303+
271304
def test_should_deserialize_background_layer_from_json(self):
272305
"""Test that canvas with background layers can be deserialized from JSON"""
273306
# Given: JSON string with background layers
@@ -374,18 +407,23 @@ def test_should_accept_8_character_hex_color(self):
374407
assert isinstance(layer, BackgroundLayer)
375408
assert layer.color == "#FF5733C8"
376409

377-
def test_should_raise_error_for_invalid_tuple_color_length(self):
378-
"""Should raise ValidationError for tuple colors with invalid length"""
379-
# Given: Canvas
410+
@pytest.mark.parametrize(
411+
"color",
412+
[
413+
(255, 87), # too short
414+
(255, 87, 51, 0, 0), # too long
415+
(256, 87, 51), # channel > 255
416+
(-1, 87, 51), # channel < 0
417+
(255, 87, 51, 256), # alpha > 255
418+
],
419+
)
420+
def test_should_raise_error_for_invalid_tuple_color(self, color):
421+
"""ValidationError for tuple colors with wrong length or out-of-range channel values"""
380422
from quickthumb import Canvas
381423
from quickthumb.errors import ValidationError
382424

383-
canvas = Canvas(200, 150)
384-
385-
# When: Adding background with invalid tuple color (wrong length)
386-
# Then: Should raise ValidationError
387425
with pytest.raises(ValidationError, match="invalid color tuple"):
388-
canvas.background(color=(255, 87))
426+
Canvas(200, 150).background(color=color)
389427

390428
def test_should_raise_error_for_invalid_fit_mode(self):
391429
"""Test that invalid fit mode raises ValidationError"""
@@ -453,7 +491,9 @@ def test_should_add_grain_effect_to_background_layer(self):
453491
"intensity": 0.12,
454492
"monochrome": True,
455493
"blend_mode": "overlay",
456-
"opacity": 1.0, 'seed': None}
494+
"opacity": 1.0,
495+
"seed": None,
496+
}
457497
)
458498
roundtrip = Canvas.from_json(json.dumps(data))
459499
assert roundtrip.layers[0].effects[0] == Grain(intensity=0.12)

0 commit comments

Comments
 (0)