Skip to content

Commit ed3b5fa

Browse files
renaud gaudinmgautierfr
authored andcommitted
safer get_hints handling
Failsafe approach to Hints usage: only fail on missing or non-dict-like values. Assume, `hints=` can be whatever and - only cherry-pick our expected Hint from there - cast any value to bool While this drifts a bit appart the zero-logic approach of the wrapper, it's a common pythonic approach for this kind of things. Main reason for this is that filtering and casting in Python allows exception to go up the stack while in-cpp casting does not and ends up ignored. Now testing: - Item without `get_hints()` - writer.Item without `get_hints()` impl. - `get_hints()` returning a non-dict value - `get_hints()` returning unexpected hints (filtered) - hints on redirection
1 parent 72688e1 commit ed3b5fa

File tree

3 files changed

+75
-5
lines changed

3 files changed

+75
-5
lines changed

libzim/wrapper.pyx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import datetime
4040
import traceback
4141

4242

43-
43+
pybool = type(True)
4444

4545
#########################
4646
# Blob #
@@ -179,8 +179,8 @@ cdef public api:
179179
cdef map[HintKeys, uint64_t] ret;
180180
try:
181181
func = getattr(obj, method.decode('UTF-8'))
182-
hintsDict = func()
183-
return convertToCppHints(func())
182+
hintsDict = {k: pybool(v) for k, v in func().items() if isinstance(k, Hint)}
183+
return convertToCppHints(hintsDict)
184184
except Exception as e:
185185
error[0] = traceback.format_exc().encode('UTF-8')
186186

libzim/writer.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ def get_contentprovider(self) -> ContentProvider:
134134
raise NotImplementedError("get_contentprovider must be implemented.")
135135

136136
def get_hints(self) -> Dict[Hint, int]:
137-
raise NotImplementedError("get_hint must be implemenent")
137+
raise NotImplementedError("get_hints must be implemented.")
138138

139139
def __repr__(self) -> str:
140140
return (

tests/test_libzim_creator.py

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def get_contentprovider(self) -> libzim.writer.ContentProvider:
4747
return StringProvider(content=getattr(self, "content", ""))
4848

4949
def get_hints(self) -> Dict[Hint, int]:
50-
return {Hint.FRONT_ARTICLE: True}
50+
return getattr(self, "hints", {Hint.FRONT_ARTICLE: True})
5151

5252

5353
@pytest.fixture(scope="function")
@@ -555,6 +555,76 @@ def get_hints(self):
555555
c.add_item(AnItem())
556556

557557

558+
def test_missing_hints(fpath):
559+
class AnItem:
560+
def get_path(self):
561+
return ""
562+
563+
def get_title(self):
564+
return ""
565+
566+
def get_mimetype(self):
567+
return ""
568+
569+
with Creator(fpath) as c:
570+
with pytest.raises(RuntimeError, match="has no attribute"):
571+
c.add_item(AnItem())
572+
573+
with pytest.raises(RuntimeError, match="must be implemented"):
574+
c.add_item(libzim.writer.Item())
575+
576+
577+
def test_nondict_hints(fpath):
578+
class AnItem:
579+
def get_path(self):
580+
return ""
581+
582+
def get_title(self):
583+
return ""
584+
585+
def get_mimetype(self):
586+
return ""
587+
588+
with Creator(fpath) as c:
589+
with pytest.raises(RuntimeError, match="has no attribute 'get_hints'"):
590+
c.add_item(AnItem())
591+
592+
with pytest.raises(RuntimeError, match="has no attribute 'items'"):
593+
c.add_item(StaticItem(path="1", title="", hints=1))
594+
595+
with pytest.raises(TypeError, match="hints"):
596+
c.add_redirection("a", "", "b", hints=1)
597+
598+
599+
def test_hints_values(fpath):
600+
with Creator(fpath) as c:
601+
# correct values
602+
c.add_item(StaticItem(path="0", title="", hints={}))
603+
c.add_item(
604+
StaticItem(
605+
path="1",
606+
title="",
607+
hints={Hint.FRONT_ARTICLE: True, Hint.COMPRESS: False},
608+
)
609+
)
610+
# non-expected Hints are ignored
611+
c.add_item(StaticItem(path="2", title="", hints={"hello": "world"}))
612+
# Hint values are casted to bool
613+
c.add_item(StaticItem(path="3", title="", hints={Hint.FRONT_ARTICLE: "world"}))
614+
c.add_redirection(
615+
path="4", title="", targetPath="0", hints={Hint.COMPRESS: True}
616+
)
617+
618+
# non-existent Hint
619+
with pytest.raises(AttributeError, match="YOLO"):
620+
c.add_item(StaticItem(path="0", title="", hints={Hint.YOLO: True}))
621+
622+
with pytest.raises(AttributeError, match="YOLO"):
623+
c.add_redirection(
624+
path="5", title="", target_path="0", hints={Hint.YOLO: True}
625+
)
626+
627+
558628
def test_reimpfeed(fpath):
559629
class AContentProvider:
560630
def __init__(self):

0 commit comments

Comments
 (0)