Address problems with deprecated AcceptValidHeader.best_match (C4-627)

src/encoded/__init__.py

@@ -35,7 +35,7 @@
 # from webob.cookies import JSONSerializer
 
 from .loadxl import load_all
-from .util import find_other_in_pair
+# from .util import find_other_in_pair
 
 
 if sys.version_info.major < 3:

src/encoded/tests/test_renderers.py

@@ -5,7 +5,12 @@
 from dcicutils.misc_utils import filtered_warnings
 from dcicutils.qa_utils import MockResponse
 from pyramid.testing import DummyRequest
-from ..renderers import should_transform
+from unittest import mock
+from .. import renderers
+from ..renderers import (
+    best_mime_type, should_transform, MIME_TYPES_SUPPORTED, MIME_TYPE_DEFAULT,
+    MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON, MIME_TYPE_TRIAGE_MODE,
+)
 
 
 pytestmark = [pytest.mark.setone, pytest.mark.working]
@@ -20,8 +25,53 @@ def __init__(self, content_type=None, status_code: int = 200, json=None, content
         super().__init__(status_code=status_code, json=json, content=content, url=url)
 
 
+def test_mime_variables():
+
+    # Really these don't need testing but it's useful visually to remind us of their values here.
+    assert MIME_TYPE_HTML == 'text/html'
+    assert MIME_TYPE_JSON == 'application/json'
+    assert MIME_TYPE_LD_JSON == 'application/ld+json'
+
+    # The MIME_TYPES_SUPPORTED is a list whose first element has elevated importance as we've structured things.
+    # First check that it is a list, and that its contents contain the things we support. That isn't controversial.
+    assert isinstance(MIME_TYPES_SUPPORTED, list)
+    assert set(MIME_TYPES_SUPPORTED) == {MIME_TYPE_JSON, MIME_TYPE_HTML, MIME_TYPE_LD_JSON}
+    # Check that the first element is consistent with the MIME_TYPE_DEFAULT.
+    # It's an accident of history that this next relationship matters, but at this point check for consistency.
+    assert MIME_TYPE_DEFAULT == MIME_TYPES_SUPPORTED[0]
+    # Now we concern ourselves with the actual values...
+    # TODO: I think it's a bug that JSON is at the head of this list (and so the default) in cgap-portal.
+    #       cgap-portal needs to be made to match what Fourfront does to dig it out of a bug I introduced.
+    #       -kmp 29-Jan-2022
+    assert MIME_TYPES_SUPPORTED == [MIME_TYPE_HTML, MIME_TYPE_JSON, MIME_TYPE_LD_JSON]
+    assert MIME_TYPE_DEFAULT == MIME_TYPE_HTML
+
+    # Regardless of whether we're using legacy mode or modern mode, we should get the same result.
+    assert MIME_TYPE_TRIAGE_MODE in ['legacy', 'modern']
+
+
 VARIOUS_MIME_TYPES_TO_TEST = ['*/*', 'text/html', 'application/json', 'application/ld+json', 'text/xml', 'who/cares']
 
+
+def test_best_mime_type():
+
+    the_constant_answer=MIME_TYPE_DEFAULT
+
+    with filtered_warnings("ignore", category=DeprecationWarning):
+        # Suppresses this warning:
+        #     DeprecationWarning: The behavior of .best_match for the Accept classes is currently being maintained
+        #       for backward compatibility, but the method will be deprecated in the future, as its behavior is not
+        #       specified in (and currently does not conform to) RFC 7231.
+
+        for requested_mime_type in VARIOUS_MIME_TYPES_TO_TEST:
+            req = DummyRequest(headers={'Accept': requested_mime_type})
+            assert best_mime_type(req, 'legacy') == the_constant_answer
+            assert best_mime_type(req, 'modern') == the_constant_answer
+            req = DummyRequest(headers={})  # The Accept header in the request just isn't being consulted
+            assert best_mime_type(req, 'modern') == the_constant_answer
+            assert best_mime_type(req, 'modern') == the_constant_answer
+
+
 TYPICAL_URLS = [
     'http://whatever/foo',
     'http://whatever/foo/',
@@ -134,3 +184,18 @@ def test_should_transform():
             % (n_passed, n_failed, n_of(problem_area, "problem area"), ", ".join(problem_area))
     )
     print("\n", n_passed, "combinations tried. ALL PASSED")
+
+
+def test_should_transform_without_best_mime_type():
+
+    # As we call things now, we really don't need the best_mime_type function because it just returns the
+    # first element of its first argument. That probably should change. Because it should be a function
+    # of the request and its Accept offerings. Even so, we test for this now not because this makes programs
+    # right, but so we notice if/when this truth changes. -kmp 23-Mar-2021
+
+    with mock.patch.object(renderers, "best_mime_type") as mock_best_mime_type:
+
+        # Demonstrate that best_mime_type(...) could be replaced by MIME_TYPES_SUPPORTED[0]
+        mock_best_mime_type.return_value = MIME_TYPES_SUPPORTED[0]
+
+        test_should_transform()

src/encoded/tests/test_types_init_collections.py

@@ -1,7 +1,8 @@
 import pytest
 
+from dcicutils.misc_utils import utc_today_str
 from ..types.image import Image
-from ..util import utc_today_str
+# from ..util import utc_today_str
 
 
 pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema]

src/encoded/tests/test_types_protocol.py

@@ -1,7 +1,8 @@
-import datetime
+# import datetime
 import pytest
 
-from ..util import utc_today_str
+from dcicutils.misc_utils import utc_today_str
+# from ..util import utc_today_str
 
 pytestmark = [pytest.mark.setone, pytest.mark.working, pytest.mark.schema]
 

src/encoded/tests/test_util.py

@@ -0,0 +1,97 @@
+import datetime
+import pytest
+import re
+from pyramid.httpexceptions import HTTPForbidden
+
+from ..util import (
+    # compute_set_difference_one, find_other_in_pair,
+    delay_rerun,  # utc_today_str,
+    customized_delay_rerun, check_user_is_logged_in
+)
+
+
+pytestmark = pytest.mark.working
+
+
+# def test_compute_set_difference_one():
+#     """ Tests that our set difference utility function behaves correctly under various cases """
+#     s1 = {1, 2, 3, 4}
+#     s2 = {1, 2, 3}
+#     assert compute_set_difference_one(s1, s2) == 4
+#     s1 = {1, 2, 3, 4}
+#     s2 = {1, 2, 3, 4}
+#     with pytest.raises(StopIteration):
+#         compute_set_difference_one(s1, s2)
+#     with pytest.raises(StopIteration):
+#         compute_set_difference_one(s2, s1)
+#     s1 = {1, 2, 3, 4, 5, 6}
+#     s2 = {1, 2, 3, 4}
+#     with pytest.raises(RuntimeError):
+#         compute_set_difference_one(s1, s2)
+#     with pytest.raises(StopIteration):
+#         compute_set_difference_one(s2, s1)
+#     s1 = {1, 2}
+#     s2 = {1}
+#     assert compute_set_difference_one(s1, s2) == 2
+#
+#
+# def test_find_other_in_pair():
+#     """ Tests the wrapper for the above function """
+#     assert find_other_in_pair(1, [1, 2]) == 2
+#     assert find_other_in_pair(2, [1, 2]) == 1
+#     lst = [1, 2, 3]
+#     val = [1, 2]
+#     with pytest.raises(TypeError):
+#         find_other_in_pair(val, lst)  # val is 'not single valued'
+#     val = 1
+#     with pytest.raises(TypeError):
+#         find_other_in_pair(val, None)  # no pair to compare to
+#     with pytest.raises(RuntimeError):  # too many results
+#         find_other_in_pair(None, lst)
+
+
+DELAY_FUZZ_SECONDS = 0.1
+
+
+def test_delay_rerun():
+    expected_delay = 1.0
+    t0 = datetime.datetime.now()
+    delay_rerun()
+    t1 = datetime.datetime.now()
+    assert (t1 - t0).total_seconds() > expected_delay
+    assert (t1 - t0).total_seconds() < expected_delay + DELAY_FUZZ_SECONDS
+
+
+def test_customize_delay_rerun():
+    custom_delay = 0.5
+    half_delay_rerun = customized_delay_rerun(sleep_seconds=custom_delay)
+    t0 = datetime.datetime.now()
+    half_delay_rerun()
+    t1 = datetime.datetime.now()
+    assert (t1 - t0).total_seconds() > custom_delay
+    assert (t1 - t0).total_seconds() < custom_delay + DELAY_FUZZ_SECONDS
+
+
+# def test_utc_today_str():
+#     pattern = "[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]"
+#     actual = utc_today_str()
+#     assert re.match(pattern, actual), "utc_today_str() result %s did not match format: %s" % (actual, pattern)
+
+
+@pytest.mark.parametrize('principals, allow', [
+    (['role1', 'role2'], False),
+    (['role1', 'userid.uuid'], True),
+    (['role1', 'group.admin'], True),
+    (['system.Everyone'], False)
+])
+def test_check_user_is_logged_in(principals, allow):
+    """ Simple test that ensures the logged in check is working as expected """
+    class MockRequest:
+        def __init__(self, principals):
+            self.effective_principals = principals
+    req = MockRequest(principals)
+    if allow:
+        check_user_is_logged_in(req)
+    else:
+        with pytest.raises(HTTPForbidden):
+            check_user_is_logged_in(req)

src/encoded/util.py

@@ -296,31 +296,31 @@ def beanstalk_env_from_registry(registry):
     return registry.settings.get('env.name')
 
 
-def compute_set_difference_one(s1, s2):
-    """ Computes the set difference between s1 and s2 (ie: in s1 but not in s2)
-        PRE: s1 and s2 differ by one element and thus their set
-        difference is a single element
-
-        :arg s1 (set(T)): super set
-        :arg s2 (set(T)): subset
-        :returns (T): the single differing element between s1 and s2.
-        :raises: exception if more than on element is found
-    """
-    res = s1 - s2
-    if len(res) > 1:
-        raise RuntimeError('Got more than one result for set difference')
-    return next(iter(res))
-
-
-def find_other_in_pair(element, pair):
-    """ Wrapper for compute_set_difference_one
-
-        :arg element (T): item to look for in pair
-        :arg pair (2-tuple of T): pair of things 'element' is in
-        :returns (T): item in pair that is not element
-        :raises: exception if types do not match or in compute_set_diferrence_one
-    """
-    return compute_set_difference_one(set(pair), {element})
+# def compute_set_difference_one(s1, s2):
+#     """ Computes the set difference between s1 and s2 (ie: in s1 but not in s2)
+#         PRE: s1 and s2 differ by one element and thus their set
+#         difference is a single element
+#
+#         :arg s1 (set(T)): super set
+#         :arg s2 (set(T)): subset
+#         :returns (T): the single differing element between s1 and s2.
+#         :raises: exception if more than on element is found
+#     """
+#     res = s1 - s2
+#     if len(res) > 1:
+#         raise RuntimeError('Got more than one result for set difference')
+#     return next(iter(res))
+#
+#
+# def find_other_in_pair(element, pair):
+#     """ Wrapper for compute_set_difference_one
+#
+#         :arg element (T): item to look for in pair
+#         :arg pair (2-tuple of T): pair of things 'element' is in
+#         :returns (T): item in pair that is not element
+#         :raises: exception if types do not match or in compute_set_diferrence_one
+#     """
+#     return compute_set_difference_one(set(pair), {element})
 
 
 def customized_delay_rerun(sleep_seconds=1):
@@ -335,8 +335,8 @@ def parameterized_delay_rerun(*args):
 delay_rerun = customized_delay_rerun(sleep_seconds=1)
 
 
-def utc_today_str():
-    return datetime.datetime.strftime(datetime.datetime.utcnow(), "%Y-%m-%d")
+# def utc_today_str():
+#     return datetime.datetime.strftime(datetime.datetime.utcnow(), "%Y-%m-%d")
 
 
 def check_user_is_logged_in(request):