diff --git a/AUTHORS.rst b/AUTHORS.rst index 90401390..23819dd3 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -63,4 +63,5 @@ Patches and suggestions - Ville Skyttä - Hugo van Kemenade - Mark Vasilkov +- fly1d diff --git a/CHANGES.rst b/CHANGES.rst index 47dcda3a..762f6b74 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -18,6 +18,7 @@ Features: Bug fixes: +* Keep cached tree builder modules when another configuration is requested. (#585) * The sanitizer now permits ```` tags. It used to allow ``
`` already. (#423) diff --git a/html5lib/_utils.py b/html5lib/_utils.py index 7e23ee57..346783c8 100644 --- a/html5lib/_utils.py +++ b/html5lib/_utils.py @@ -135,12 +135,10 @@ def moduleFactory(baseModule, *args, **kwargs): mod = ModuleType(name) objs = factory(baseModule, *args, **kwargs) mod.__dict__.update(objs) - if "name" not in moduleCache: + if name not in moduleCache: moduleCache[name] = {} - if "args" not in moduleCache[name]: + if args not in moduleCache[name]: moduleCache[name][args] = {} - if "kwargs" not in moduleCache[name][args]: - moduleCache[name][args][kwargs_tuple] = {} moduleCache[name][args][kwargs_tuple] = mod return mod diff --git a/html5lib/tests/test_module_factory.py b/html5lib/tests/test_module_factory.py new file mode 100644 index 00000000..f02caaf5 --- /dev/null +++ b/html5lib/tests/test_module_factory.py @@ -0,0 +1,87 @@ +from __future__ import absolute_import, division, unicode_literals + +from types import ModuleType +import xml.etree.ElementTree as ElementTree + +import pytest + +from html5lib import HTMLParser, getTreeBuilder +from html5lib._utils import moduleFactoryFactory + + +def _module_factory(): + def factory(backend, *args, **kwargs): + return {"backend": backend, "arguments": args, "options": kwargs} + + return moduleFactoryFactory(factory) + + +@pytest.mark.parametrize("args_a, options_a, args_b, options_b", [ + (("first",), {}, ("second",), {}), + ((), {"enabled": False}, (), {"enabled": True}), + (("first",), {"enabled": False}, ("first",), {"enabled": True}), + (("first",), {"enabled": False}, ("second",), {"enabled": False}), +], ids=["positional", "keyword", "shared-arguments", "shared-options"]) +def test_module_factory_reuses_each_configuration(args_a, options_a, args_b, options_b): + backend = ModuleType(str("factory_backend")) + build = _module_factory() + + first = build(backend, *args_a, **options_a) + second = build(backend, *args_b, **options_b) + first_again = build(backend, *args_a, **options_a) + second_again = build(backend, *args_b, **options_b) + + assert first.backend is backend + assert second.backend is backend + assert first.arguments == args_a + assert second.arguments == args_b + assert first.options == options_a + assert second.options == options_b + assert first_again.arguments == args_a + assert first_again.options == options_a + assert second_again.arguments == args_b + assert second_again.options == options_b + assert first is not second + assert first_again is first + assert second_again is second + + +def test_module_factory_keeps_distinct_backends_cached(): + backend_a = ModuleType(str("factory_backend_a")) + backend_b = ModuleType(str("factory_backend_b")) + build = _module_factory() + + first = build(backend_a, enabled=True) + second = build(backend_b, enabled=True) + + assert first.backend is backend_a + assert second.backend is backend_b + assert first is not second + assert build(backend_a, enabled=True) is first + assert build(backend_b, enabled=True) is second + + +@pytest.mark.parametrize("full_tree", [False, True]) +def test_etree_builder_identity_survives_alternating_options(full_tree): + first = getTreeBuilder("etree", ElementTree, fullTree=full_tree) + first_parser = HTMLParser(tree=first, namespaceHTMLElements=False) + second = getTreeBuilder("etree", ElementTree, fullTree=not full_tree) + second_parser = HTMLParser(tree=second, namespaceHTMLElements=False) + first_again = getTreeBuilder("etree", ElementTree, fullTree=full_tree) + second_again = getTreeBuilder("etree", ElementTree, fullTree=not full_tree) + + assert first.implementation is ElementTree + assert second.implementation is ElementTree + assert first is not second + assert first_again is first + assert second_again is second + assert isinstance(first_parser.tree, first_again) + assert isinstance(second_parser.tree, second_again) + + # Parsing is an existing-behavior control, not the cache regression itself. + parsers = [(first_parser, full_tree), (second_parser, not full_tree)] + for parser, includes_document in parsers: + document = parser.parse("

content

") + root = document.find("html") if includes_document else document + assert root.tag == "html" + assert root.find("body/p").text == "content"