From 25df9ca4206ebaef2498d2748b186c30c7b48b26 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Tue, 24 May 2022 06:35:23 -0400 Subject: [PATCH] refactor: make safe_lxml an ordinary folder in openedx/core/lib (#25689) --- cms/celery.py | 2 +- cms/conftest.py | 2 +- cms/wsgi.py | 2 +- common/lib/conftest.py | 2 +- common/lib/safe_lxml/setup.py | 16 ----------- common/test/conftest.py | 4 ++- docs/guides/conf.py | 2 -- docs/guides/docstrings/common_lib.rst | 1 - lms/celery.py | 2 +- lms/wsgi.py | 2 +- lms/wsgi_apache_lms.py | 2 +- manage.py | 2 +- .../core/lib}/safe_lxml/__init__.py | 0 openedx/core/lib/safe_lxml/conftest.py | 7 +++++ .../core/lib}/safe_lxml/etree.py | 2 ++ .../core/lib}/safe_lxml/tests.py | 10 ++++++- .../xblock_builtin/xblock_discussion/tests.py | 2 +- requirements/edx/base.txt | 2 -- requirements/edx/development.txt | 2 -- requirements/edx/local.in | 1 - requirements/edx/testing.txt | 2 -- safe_lxml/__init__.py | 27 +++++++++++++++++++ scripts/verify-dunder-init.sh | 2 +- setup.cfg | 1 - 24 files changed, 58 insertions(+), 39 deletions(-) delete mode 100644 common/lib/safe_lxml/setup.py rename {common/lib/safe_lxml => openedx/core/lib}/safe_lxml/__init__.py (100%) create mode 100644 openedx/core/lib/safe_lxml/conftest.py rename {common/lib/safe_lxml => openedx/core/lib}/safe_lxml/etree.py (98%) rename {common/lib/safe_lxml => openedx/core/lib}/safe_lxml/tests.py (66%) create mode 100644 safe_lxml/__init__.py diff --git a/cms/celery.py b/cms/celery.py index 88f38e5b1fb9..a7166feb1c27 100644 --- a/cms/celery.py +++ b/cms/celery.py @@ -8,7 +8,7 @@ import os # Patch the xml libs before anything else. -from safe_lxml import defuse_xml_libs +from openedx.core.lib.safe_lxml import defuse_xml_libs defuse_xml_libs() diff --git a/cms/conftest.py b/cms/conftest.py index 1aa0fab95fd6..dc360e3df14b 100644 --- a/cms/conftest.py +++ b/cms/conftest.py @@ -16,7 +16,7 @@ from openedx.core.pytest_hooks import DeferPlugin # Patch the xml libs before anything else. -from safe_lxml import defuse_xml_libs # isort:skip # lint-amnesty, pylint: disable=wrong-import-order +from openedx.core.lib.safe_lxml import defuse_xml_libs # isort:skip defuse_xml_libs() diff --git a/cms/wsgi.py b/cms/wsgi.py index d69eb8a7b9c1..0ccd953d98dd 100644 --- a/cms/wsgi.py +++ b/cms/wsgi.py @@ -12,7 +12,7 @@ """ # Patch the xml libs before anything else. -from safe_lxml import defuse_xml_libs +from openedx.core.lib.safe_lxml import defuse_xml_libs defuse_xml_libs() import os # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position diff --git a/common/lib/conftest.py b/common/lib/conftest.py index c3af3f60914e..8b5b151a255a 100644 --- a/common/lib/conftest.py +++ b/common/lib/conftest.py @@ -5,7 +5,7 @@ import pytest -from safe_lxml import defuse_xml_libs +from openedx.core.lib.safe_lxml import defuse_xml_libs # This import is needed for pytest plugin configuration, so please avoid deleting this during refactoring from openedx.core.pytest_hooks import pytest_configure # pylint: disable=unused-import diff --git a/common/lib/safe_lxml/setup.py b/common/lib/safe_lxml/setup.py deleted file mode 100644 index 9a4a54c8d4ae..000000000000 --- a/common/lib/safe_lxml/setup.py +++ /dev/null @@ -1,16 +0,0 @@ -""" -Setup.py for safe_lxml. -""" - - -from setuptools import setup - -setup( - name="safe_lxml", - version="1.0", - packages=["safe_lxml"], - install_requires=[ - "lxml", - "defusedxml" - ], -) diff --git a/common/test/conftest.py b/common/test/conftest.py index 32b122bbca3b..ceb71f6230e8 100644 --- a/common/test/conftest.py +++ b/common/test/conftest.py @@ -1,4 +1,6 @@ """Code run by pylint before running any tests.""" -from safe_lxml import defuse_xml_libs + +# Patch the xml libs before anything else. +from openedx.core.lib.safe_lxml import defuse_xml_libs defuse_xml_libs() diff --git a/docs/guides/conf.py b/docs/guides/conf.py index 213d131ce3e6..e61fcd5f17b7 100644 --- a/docs/guides/conf.py +++ b/docs/guides/conf.py @@ -21,7 +21,6 @@ sys.path.insert(0, root) sys.path.append(root / "docs/guides") sys.path.append(root / "common/lib/capa") -sys.path.append(root / "common/lib/safe_lxml") sys.path.append(root / "common/lib/xmodule") # Use a settings module that allows all LMS and Studio code to be imported @@ -223,7 +222,6 @@ modules = { 'cms': 'cms', 'common/lib/capa/capa': 'common/lib/capa', - 'common/lib/safe_lxml/safe_lxml': 'common/lib/safe_lxml', 'common/lib/xmodule/xmodule': 'common/lib/xmodule', 'lms': 'lms', 'openedx': 'openedx', diff --git a/docs/guides/docstrings/common_lib.rst b/docs/guides/docstrings/common_lib.rst index 4a4a1d40d638..b87467016612 100644 --- a/docs/guides/docstrings/common_lib.rst +++ b/docs/guides/docstrings/common_lib.rst @@ -9,5 +9,4 @@ out from edx-platform into separate packages at some point. :maxdepth: 2 common/lib/capa/modules - common/lib/safe_lxml/modules common/lib/xmodule/modules diff --git a/lms/celery.py b/lms/celery.py index 2ca97de3c62a..eca05a64e192 100644 --- a/lms/celery.py +++ b/lms/celery.py @@ -8,7 +8,7 @@ import os # Patch the xml libs before anything else. -from safe_lxml import defuse_xml_libs +from openedx.core.lib.safe_lxml import defuse_xml_libs defuse_xml_libs() diff --git a/lms/wsgi.py b/lms/wsgi.py index f0cf08305adc..bb0d0f7ede8c 100644 --- a/lms/wsgi.py +++ b/lms/wsgi.py @@ -9,7 +9,7 @@ """ # Patch the xml libs -from safe_lxml import defuse_xml_libs +from openedx.core.lib.safe_lxml import defuse_xml_libs defuse_xml_libs() import os # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position diff --git a/lms/wsgi_apache_lms.py b/lms/wsgi_apache_lms.py index 194dbb887870..e40be055d493 100644 --- a/lms/wsgi_apache_lms.py +++ b/lms/wsgi_apache_lms.py @@ -6,7 +6,7 @@ """ # Patch the xml libs before anything else. -from safe_lxml import defuse_xml_libs +from openedx.core.lib.safe_lxml import defuse_xml_libs defuse_xml_libs() import os # lint-amnesty, pylint: disable=wrong-import-order, wrong-import-position diff --git a/manage.py b/manage.py index f8ca29b1d301..98bcc4071799 100755 --- a/manage.py +++ b/manage.py @@ -17,7 +17,7 @@ log_python_warnings() # Patch the xml libs before anything else. -from safe_lxml import defuse_xml_libs +from openedx.core.lib.safe_lxml import defuse_xml_libs # isort:skip defuse_xml_libs() import importlib diff --git a/common/lib/safe_lxml/safe_lxml/__init__.py b/openedx/core/lib/safe_lxml/__init__.py similarity index 100% rename from common/lib/safe_lxml/safe_lxml/__init__.py rename to openedx/core/lib/safe_lxml/__init__.py diff --git a/openedx/core/lib/safe_lxml/conftest.py b/openedx/core/lib/safe_lxml/conftest.py new file mode 100644 index 000000000000..afddae8356b8 --- /dev/null +++ b/openedx/core/lib/safe_lxml/conftest.py @@ -0,0 +1,7 @@ +""" +Code run by pytest before running any tests in the safe_lxml directory. +""" +from openedx.core.lib.safe_lxml import defuse_xml_libs + + +defuse_xml_libs() diff --git a/common/lib/safe_lxml/safe_lxml/etree.py b/openedx/core/lib/safe_lxml/etree.py similarity index 98% rename from common/lib/safe_lxml/safe_lxml/etree.py rename to openedx/core/lib/safe_lxml/etree.py index 8bd35afd8567..21593f06ef4c 100644 --- a/common/lib/safe_lxml/safe_lxml/etree.py +++ b/openedx/core/lib/safe_lxml/etree.py @@ -5,6 +5,8 @@ It also includes a safer XMLParser. For processing xml always prefer this over using lxml.etree directly. + +isort:skip_file """ # Names are imported into this module so that it can be a stand-in for diff --git a/common/lib/safe_lxml/safe_lxml/tests.py b/openedx/core/lib/safe_lxml/tests.py similarity index 66% rename from common/lib/safe_lxml/safe_lxml/tests.py rename to openedx/core/lib/safe_lxml/tests.py index 01ced5e163b3..3608d43bfa93 100644 --- a/common/lib/safe_lxml/safe_lxml/tests.py +++ b/openedx/core/lib/safe_lxml/tests.py @@ -1,4 +1,12 @@ -"""Test that we have defused XML.""" +""" +Test that we have defused XML. + +For these tests, the defusing will happen in one or more of the `conftest.py` +files that runs at pytest startup calls `defuse_xml_libs()`. + +In production, the defusing happens when the LMS or Studio `wsgi.py` files +call `defuse_xml_libs()`. +""" import defusedxml diff --git a/openedx/core/lib/xblock_builtin/xblock_discussion/tests.py b/openedx/core/lib/xblock_builtin/xblock_discussion/tests.py index 5337c2795d11..3e0b58b55665 100644 --- a/openedx/core/lib/xblock_builtin/xblock_discussion/tests.py +++ b/openedx/core/lib/xblock_builtin/xblock_discussion/tests.py @@ -14,7 +14,7 @@ from xblock.runtime import Runtime from openedx.core.lib.xblock_builtin.xblock_discussion.xblock_discussion import DiscussionXBlock -from safe_lxml import etree # lint-amnesty, pylint: disable=wrong-import-order +from openedx.core.lib.safe_lxml import etree def attribute_pair_repr(self): diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 10c9d931db0a..c58c4ddc66cd 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -22,8 +22,6 @@ # via -r requirements/edx/local.in -e git+https://github.com/edx/RateXBlock.git@2.0.1#egg=rate-xblock # via -r requirements/edx/github.in --e common/lib/safe_lxml - # via -r requirements/edx/local.in -e common/lib/sandbox-packages # via -r requirements/edx/local.in -e openedx/core/lib/xblock_builtin/xblock_discussion diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 87ce26eba155..21b4952b0301 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -22,8 +22,6 @@ # via -r requirements/edx/testing.txt -e git+https://github.com/edx/RateXBlock.git@2.0.1#egg=rate-xblock # via -r requirements/edx/testing.txt --e common/lib/safe_lxml - # via -r requirements/edx/testing.txt -e common/lib/sandbox-packages # via -r requirements/edx/testing.txt -e openedx/core/lib/xblock_builtin/xblock_discussion diff --git a/requirements/edx/local.in b/requirements/edx/local.in index d60eab5e641f..e68bbadf11ff 100644 --- a/requirements/edx/local.in +++ b/requirements/edx/local.in @@ -1,7 +1,6 @@ # Python libraries to install that are local to the edx-platform repo -e . -e common/lib/capa --e common/lib/safe_lxml -e common/lib/sandbox-packages -e common/lib/xmodule diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 12cc17fe0e3f..2108fc824840 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -22,8 +22,6 @@ # via -r requirements/edx/base.txt -e git+https://github.com/edx/RateXBlock.git@2.0.1#egg=rate-xblock # via -r requirements/edx/base.txt --e common/lib/safe_lxml - # via -r requirements/edx/base.txt -e common/lib/sandbox-packages # via -r requirements/edx/base.txt -e openedx/core/lib/xblock_builtin/xblock_discussion diff --git a/safe_lxml/__init__.py b/safe_lxml/__init__.py new file mode 100644 index 000000000000..dbbc0ed49970 --- /dev/null +++ b/safe_lxml/__init__.py @@ -0,0 +1,27 @@ +""" +Temporary import path shim module. + +Previously, the safe_lxml package was housed in common/lib/safe_lxml. +It was installed as its own Python project, so instead of its import path +being, as one would expect: + + import common.lib.safe_lxml.safe_lxml + +it was instead just: + + import safe_lxml + +To increase the sanity of edx-platform and simplify its tooling, we are +moving the safe_lxml package to openedx/core/lib (in tihs same repo) and +changing its import path to: + + import openedx.core.lib.safe_lxml + +In order to maintain backwards-compatibility with code using the +old import path for one release, we expose this compatibility module. + +Jira ticket (public, but requires account): https://openedx.atlassian.net/browse/BOM-2583 +Target removal for this shim module: by Olive. +""" + +from openedx.core.lib.safe_lxml import * # pylint: disable=unused-wildcard-import,wrong-import-order diff --git a/scripts/verify-dunder-init.sh b/scripts/verify-dunder-init.sh index bc5da5860ffe..c6cf3134bb72 100755 --- a/scripts/verify-dunder-init.sh +++ b/scripts/verify-dunder-init.sh @@ -38,7 +38,7 @@ exclude+='|^common/test/data/?.*$' # * common/lib/xmodule -> EXCLUDE from check. # * common/lib/xmodule/xmodule/modulestore -> INCLUDE in check. exclude+='|^common/lib$' -exclude+='|^common/lib/(capa|safe_lxml|sandbox-packages|xmodule)$' +exclude+='|^common/lib/(capa|sandbox-packages|xmodule)$' # Docs, scripts. exclude+='|^docs/.*$' diff --git a/setup.cfg b/setup.cfg index b923ea1357c3..a626677f7a88 100644 --- a/setup.cfg +++ b/setup.cfg @@ -47,4 +47,3 @@ multi_line_output=3 skip= envs migrations - common/lib/safe_lxml/safe_lxml/etree.py