Repository: falconry/falcon
Pull request #1459 (2019-02-24 01:46:19)

feat(etag): Improve etag performance and docs, plus usability of the ETag class

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=h1) Report
> Merging [#1459](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/1f49bd857c77a983cdae10466310a5f072d16eeb?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1459/graphs/tree.svg?width=650&token=zOtkfNQ3h6&height=150&src=pr)](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master   #1459   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          39      39           
  Lines        2613    2609    -4     
  Branches      414     413    -1     
======================================
- Hits         2613    2609    -4
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/util/structures.py](https://codecov.io/gh/falconry/falcon/pull/1459/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvc3RydWN0dXJlcy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/request\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1459/diff?src=pr&el=tree#diff-ZmFsY29uL3JlcXVlc3RfaGVscGVycy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/request.py](https://codecov.io/gh/falconry/falcon/pull/1459/diff?src=pr&el=tree#diff-ZmFsY29uL3JlcXVlc3QucHk=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=footer). Last update [1f49bd8...7191fd3](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=h1) Report
> Merging [#1459](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/1f49bd857c77a983cdae10466310a5f072d16eeb?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1459/graphs/tree.svg?width=650&token=zOtkfNQ3h6&height=150&src=pr)](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master   #1459   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          39      39           
  Lines        2613    2609    -4     
  Branches      414     413    -1     
======================================
- Hits         2613    2609    -4
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/util/structures.py](https://codecov.io/gh/falconry/falcon/pull/1459/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvc3RydWN0dXJlcy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/request\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1459/diff?src=pr&el=tree#diff-ZmFsY29uL3JlcXVlc3RfaGVscGVycy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [falcon/request.py](https://codecov.io/gh/falconry/falcon/pull/1459/diff?src=pr&el=tree#diff-ZmFsY29uL3JlcXVlc3QucHk=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=footer). Last update [1f49bd8...7191fd3](https://codecov.io/gh/falconry/falcon/pull/1459?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

kgriffs: Updated per feedback.

Patch:
diff --git a/falcon/request.py b/falcon/request.py
index 669bdd7..4ddcf54 100644
--- a/falcon/request.py
+++ b/falcon/request.py
@@ -355,32 +355,24 @@ class Request(object):
         range_unit (str): Unit of the range parsed from the value of the
             Range header, or ``None`` if the header is missing
         if_match (list): Value of the If-Match header, as a parsed list of
-            :class:`falcon.ETag` objects or ``None`` if the header is missing.
+            :class:`falcon.ETag` objects or ``None`` if the header is missing
+            or its value is blank.
 
-            This property is determined by the value of ``HTTP_IF_MATCH``
-            in the WSGI environment dict.
+            This property provides a list of all ``entity-tags`` in the
+            header, both strong and weak, in the same order as listed in
+            the header.
 
-            Note:
-                This property includes strong and weak entity-tags. Per
-                `RFC 7239`_, two entity-tags are equivalent if both are not
-                weak and their opaque-tags match character-by-character.
-
-            (See also: RFC 7239, Section 3.1)
+            (See also: RFC 7232, Section 3.1)
 
         if_none_match (list): Value of the If-None-Match header, as a parsed
             list of :class:`falcon.ETag` objects or ``None`` if the header is
-            missing.
+            missing or its value is blank.
 
-            This property is determined by the value of ``HTTP_IF_NONE_MATCH``
-            in the WSGI environment dict.
-
-            Note:
-                This property includes strong and weak entity-tags. Per
-                `RFC 7239`_, two entity-tags are equivalent if their
-                opaque-tags match character-by-character, regardless of
-                either or both being tagged as weak.
+            This property provides a list of all ``entity-tags`` in the
+            header, both strong and weak, in the same order as listed in
+            the header.
 
-            (See also: RFC 7239, Section 3.2)
+            (See also: RFC 7232, Section 3.2)
 
         if_modified_since (datetime): Value of the If-Modified-Since header,
             or ``None`` if the header is missing.
@@ -433,6 +425,8 @@ class Request(object):
 
     _cookies = None
     _cookies_collapsed = None
+    _cached_if_match = None
+    _cached_if_none_match = None
 
     # Child classes may override this
     context_type = type('RequestContext', (dict,), {})
@@ -619,11 +613,30 @@ class Request(object):
 
     @property
     def if_match(self):
-        return helpers.parse_etags(self.env.get('HTTP_IF_MATCH'))
+        # TODO(kgriffs): It may make sense at some point to create a
+        #   header property generator that DRY's up the memoization
+        #   pattern for us.
+        # PERF(kgriffs): It probably isn't worth it to set
+        #   self._cached_if_match to a special type/object to distinguish
+        #   between the variable being unset and the header not being
+        #   present in the request. The reason is that if the app
+        #   gets a None back on the first reference to property, it
+        #   probably isn't going to access the property again (TBD).
+        if self._cached_if_match is None:
+            header_value = self.env.get('HTTP_IF_MATCH')
+            if header_value:
+                self._cached_if_match = helpers._parse_etags(header_value)
+
+        return self._cached_if_match
 
     @property
     def if_none_match(self):
-        return helpers.parse_etags(self.env.get('HTTP_IF_NONE_MATCH'))
+        if self._cached_if_none_match is None:
+            header_value = self.env.get('HTTP_IF_NONE_MATCH')
+            if header_value:
+                self._cached_if_none_match = helpers._parse_etags(header_value)
+
+        return self._cached_if_none_match
 
     @property
     def if_modified_since(self):
diff --git a/falcon/request_helpers.py b/falcon/request_helpers.py
index db7ca02..2769390 100644
--- a/falcon/request_helpers.py
+++ b/falcon/request_helpers.py
@@ -28,7 +28,12 @@ from falcon.util.compat import http_cookies
 #
 _COOKIE_NAME_RESERVED_CHARS = re.compile('[\x00-\x1F\x7F-\xFF()<>@,;:\\\\"/[\\]?={} \x09]')
 
-_ETAG_PATTERN = re.compile(r'([Ww]/)?(?:"(.*?)"|(.*?))(?:\s*,\s*|$)')
+# NOTE(kgriffs): strictly speaking, the weakness indicator is
+#   case-sensitive, but this wasn't explicit until RFC 7232
+#   so we allow for both. We also require quotes because that's
+#   been standardized since 1999, and it makes the regex simpler
+#   and more performant.
+_ENTITY_TAG_PATTERN = re.compile(r'([Ww]/)?"([^"]*)"')
 
 
 def parse_cookie_header(header_value):
@@ -113,26 +118,18 @@ def header_property(wsgi_name):
     return property(fget)
 
 
-def make_etag(value, is_weak=False):
-    """Creates and returns a ETag object.
+# NOTE(kgriffs): Going forward we should privatize helpers, as done here. We
+#   can always move this over to falcon.util if we decide it would be
+#   more generally useful to app developers.
+def _parse_etags(etag_str):
+    """Parse a string containing one or more HTTP entity-tags.
 
-    Args:
-        value (str): Unquated entity tag value
-        is_weak (bool): The weakness indicator
-
-    Returns:
-        A ``str``-like Etag instance with weakness indicator.
+    The string is assumed to be formatted as defined for a precondition
+    header, and may contain either a single ETag, or multiple comma-separated
+    ETags. The string may also contain a '*' character, in order to indicate
+    that any ETag should match the precondition.
 
-    """
-    etag = ETag(value)
-    etag.is_weak = is_weak
-    return etag
-
-
-def parse_etags(etag_str):
-    """
-    Parse a string of ETags given in the If-Match or If-None-Match header as
-    defined by RFC 7232.
+    (See also: RFC 7232, Section 3)
 
     Args:
         etag_str (str): An ASCII header value to parse ETags from. ETag values
@@ -140,42 +137,38 @@ def parse_etags(etag_str):
             function should be used.
 
     Returns:
-        A list of unquoted ETags or ``['*']`` if all ETags should be matched.
+        list: A list of unquoted ETags or ``['*']`` if all ETags should be
+        matched. If the string to be parse is empty, or contains only
+        whitespace, ``None`` will be returned instead.
 
     """
-    if etag_str is None:
-        return None
 
-    etags = []
     etag_str = etag_str.strip()
     if not etag_str:
-        return etags
+        return None
 
     if etag_str == '*':
-        etags.append(etag_str)
-        return etags
+        return [etag_str]
 
     if ',' not in etag_str:
-        value = etag_str
-        is_weak = False
-        if value.startswith(('W/', 'w/')):
-            is_weak = True
-            value = value[2:]
-        if value[:1] == value[-1:] == '"':
-            value = value[1:-1]
-        etags.append(make_etag(value, is_weak))
-    else:
-        pos = 0
-        end = len(etag_str)
-        while pos < end:
-            match = _ETAG_PATTERN.match(etag_str, pos)
-            is_weak, quoted, raw = match.groups()
-            value = quoted or raw
-            if value:
-                etags.append(make_etag(value, bool(is_weak)))
-            pos = match.end()
-
-    return etags
+        return [ETag.loads(etag_str)]
+
+    etags = []
+
+    # PERF(kgriffs): Parsing out the weak string like this turns out to be more
+    #   performant than grabbing the entire entity-tag and passing it to
+    #   ETag.loads(). This is also faster than parsing etag_str manually via
+    #   str.find() and slicing.
+    for weak, value in _ENTITY_TAG_PATTERN.findall(etag_str):
+        t = ETag(value)
+        t.is_weak = bool(weak)
+        etags.append(t)
+
+    # NOTE(kgriffs): Normalize a string with only whitespace and commas
+    #   to None, since it is like a list of individual ETag headers that
+    #   are all set to nothing, and so therefore basically should be
+    #   treated as not having been set in the first place.
+    return etags or None
 
 
 class BoundedStream(io.IOBase):
diff --git a/falcon/util/structures.py b/falcon/util/structures.py
index 26d6e7b..cba3e77 100644
--- a/falcon/util/structures.py
+++ b/falcon/util/structures.py
@@ -108,22 +108,114 @@ class CaseInsensitiveDict(MutableMapping):  # pragma: no cover
 
 
 class ETag(str):
-    """An entity-tag ``str``-like object with the weakness indicator.
+    """Convenience class to represent a parsed HTTP entity-tag.
+
+    This class is simply a subclass of ``str`` with a few helper methods and
+    an extra attribute to indicate whether the entity-tag is weak or strong. The
+    value of the string is equivalent to what RFC 7232 calls an "opaque-tag",
+    i.e. an entity-tag sans quotes and the weakness indicator.
+
+    Note:
+
+        Given that a weak entity-tag comparison can be performed by
+        using the ``==`` operator (per the example below), only a
+        :meth:`~.strong_compare` method is provided.
+
+    Here is an example ``on_get()`` method that demonstrates how to use instances
+    of this class::
+
+        def on_get(self, req, resp):
+            content_etag = self._get_content_etag()
+            for etag in (req.if_none_match or []):
+                if etag == '*' or etag == content_etag:
+                    resp.status = falcon.HTTP_304
+                    return
+
+            # ...
+
+            resp.etag = content_etag
+            resp.status = falcon.HTTP_200
+
+    (See also: RFC 7232)
 
     Attributes:
-        is_weak (bool): The weakness indicator.
+        is_weak (bool): ``True`` if the entity-tag is weak, otherwise ``False``.
 
     """
+
     is_weak = False
 
-    def to_header(self):
-        """Convert the ETag into a HTTP header string.
+    def strong_compare(self, other):
+        """Performs a strong entity-tag comparison.
+
+        Two entity-tags are equivalent if both are not weak and their
+        opaque-tags match character-by-character.
+
+        (See also: RFC 7232, Section 2.3.2)
+
+        Arguments:
+            other (ETag): The other :class:`~.ETag` to which you are comparing
+            this one.
+
+        Returns:
+            bool: ``True`` if the two entity-tags match, otherwise ``False``.
+
+        """
+
+        return self == other and not (self.is_weak or other.is_weak)
+
+    def dumps(self):
+        """Serialize the ETag to a string suitable for use in a precondition header.
+
+        (See also: RFC 7232, Section 2.3)
 
         Returns:
             str: An opaque quoted string, possibly prefixed by a weakness
             indicator ``W/``.
-
         """
+
         if self.is_weak:
-            return 'W/"%s"' % self
-        return '"%s"' % self
+            # PERF(kgriffs): Simple concatenation like this is slightly faster
+            #   than %s string formatting.
+            return 'W/"' + self + '"'
+
+        return '"' + self + '"'
+
+    @classmethod
+    def loads(cls, etag_str):
+        """Class method that deserializes a single entity-tag string from a precondition header.
+
+        Note:
+
+            This method is meant to be used only for parsing a single
+            entity-tag. It can not be used to parse a comma-separated list of
+            values.
+
+        (See also: RFC 7232, Section 2.3)
+
+        Arguments:
+            etag_str (str): An ASCII string representing a single entity-tag,
+                as defined by RFC 7232.
+
+        Returns:
+            ETag: An instance of `~.ETag` representing the parsed entity-tag.
+
+        """
+
+        value = etag_str
+
+        is_weak = False
+        if value.startswith(('W/', 'w/')):
+            is_weak = True
+            value = value[2:]
+
+        # NOTE(kgriffs): We allow for an unquoted entity-tag just in case,
+        #   although it has been non-standard to do so since at least 1999
+        #   with the advent of RFC 2616.
+        if value[:1] == value[-1:] == '"':
+            value = value[1:-1]
+
+        t = cls(value)
+        t.is_weak = is_weak
+
+        return t


Test patch:
diff --git a/tests/test_request_attrs.py b/tests/test_request_attrs.py
index 2e710ff..a7f7b63 100644
--- a/tests/test_request_attrs.py
+++ b/tests/test_request_attrs.py
@@ -5,14 +5,32 @@ import pytest
 
 import falcon
 from falcon.request import Request, RequestOptions
-from falcon.request_helpers import make_etag
+from falcon.request_helpers import _parse_etags
 import falcon.testing as testing
 import falcon.uri
 from falcon.util import compat
+from falcon.util.structures import ETag
 
 _PROTOCOLS = ['HTTP/1.0', 'HTTP/1.1']
 
 
+def _make_etag(value, is_weak=False):
+    """Creates and returns an ETag object.
+
+    Args:
+        value (str): Unquated entity tag value
+        is_weak (bool): The weakness indicator
+
+    Returns:
+        A ``str``-like Etag instance with weakness indicator.
+
+    """
+    etag = ETag(value)
+
+    etag.is_weak = is_weak
+    return etag
+
+
 class TestRequestAttributes(object):
 
     def setup_method(self, method):
@@ -785,39 +803,121 @@ class TestRequestAttributes(object):
         assert req.app == ''
 
     @pytest.mark.parametrize('etag,expected', [
-        ('', []),
-        (',', []),
+        ('', None),
+        (' ', None),
+        ('   ', None),
+        ('\t', None),
+        (' \t', None),
+        (',', None),
+        (',,', None),
+        (',, ', None),
+        (', , ', None),
         ('*', ['*']),
         (
             'W/"67ab43"',
-            [make_etag('67ab43', is_weak=True)]
+            [_make_etag('67ab43', is_weak=True)]
         ),
         (
             'w/"67ab43"',
-            [make_etag('67ab43', is_weak=True)]
+            [_make_etag('67ab43', is_weak=True)]
+        ),
+        (
+            ' w/"67ab43"',
+            [_make_etag('67ab43', is_weak=True)]
+        ),
+        (
+            'w/"67ab43" ',
+            [_make_etag('67ab43', is_weak=True)]
+        ),
+        (
+            'w/"67ab43 " ',
+            [_make_etag('67ab43 ', is_weak=True)]
         ),
         (
             '"67ab43"',
-            [make_etag('67ab43', is_weak=True)]
+            [_make_etag('67ab43')]
+        ),
+        (
+            ' "67ab43"',
+            [_make_etag('67ab43')]
+        ),
+        (
+            ' "67ab43" ',
+            [_make_etag('67ab43')]
+        ),
+        (
+            '"67ab43" ',
+            [_make_etag('67ab43')]
+        ),
+        (
+            '" 67ab43" ',
+            [_make_etag(' 67ab43')]
+        ),
+        (
+            '67ab43"',
+            [_make_etag('67ab43"')]
+        ),
+        (
+            '"67ab43',
+            [_make_etag('"67ab43')]
         ),
         (
             '67ab43',
-            [make_etag('67ab43', is_weak=False)]
+            [_make_etag('67ab43')]
+        ),
+        (
+            '67ab43 ',
+            [_make_etag('67ab43')]
+        ),
+        (
+            '  67ab43 ',
+            [_make_etag('67ab43')]
+        ),
+        (
+            '  67ab43',
+            [_make_etag('67ab43')]
         ),
         (
-            'W/"67ab43", "54ed21", 42we85',
-            [make_etag('67ab43', is_weak=True),
-             make_etag('54ed21', is_weak=False),
-             make_etag('42we85', is_weak=False)]
+            # NOTE(kgriffs): To simplify parsing and improve performance, we
+            #   do not attempt to handle unquoted entity-tags when there is
+            #   a list; it is non-standard anyway, and has been since 1999.
+            'W/"67ab43", "54ed21", junk"F9,22", junk "41, 7F", unquoted, w/"22, 41, 7F", "", W/""',
+            [
+                _make_etag('67ab43', is_weak=True),
+                _make_etag('54ed21'),
+
+                # NOTE(kgriffs): Test that the ETag initializer defaults to
+                #   is_weak == False
+                ETag('F9,22'),
+
+                _make_etag('41, 7F'),
+                _make_etag('22, 41, 7F', is_weak=True),
+
+                # NOTE(kgriffs): According to the grammar in RFC 7232, zero
+                #  etagc's is acceptable.
+                _make_etag(''),
+                _make_etag('', is_weak=True),
+            ]
         ),
     ])
     def test_etag(self, etag, expected):
-        self._test_header_expected_value('If-Match', etag, 'if_match', expected)
-        self._test_header_expected_value('If-None-Match', etag, 'if_none_match', expected)
+        self._test_header_etag('If-Match', etag, 'if_match', expected)
+        self._test_header_etag('If-None-Match', etag, 'if_none_match', expected)
 
     def test_etag_is_missing(self):
-        assert self.req.if_match is None
-        assert self.req.if_none_match is None
+        # NOTE(kgriffs): Loop in order to test caching
+        for __ in range(3):
+            assert self.req.if_match is None
+            assert self.req.if_none_match is None
+
+    @pytest.mark.parametrize('header_value', ['', ' ', '  '])
+    def test_etag_parsing_helper(self, header_value):
+        # NOTE(kgriffs): Test a couple of cases that are not directly covered
+        #   elsewhere (but that we want the helper to still support
+        #   for the sake of avoiding suprises if they are ever called without
+        #   preflighting the header value).
+
+        assert _parse_etags(header_value) is None
 
     # -------------------------------------------------------------------------
     # Helpers
@@ -836,6 +936,25 @@ class TestRequestAttributes(object):
         req = Request(testing.create_environ(headers=headers))
         assert getattr(req, attr) == expected_value
 
+    def _test_header_etag(self, name, value, attr, expected_value):
+        headers = {name: value}
+        req = Request(testing.create_environ(headers=headers))
+
+        # NOTE(kgriffs): Loop in order to test caching
+        for __ in range(3):
+            value = getattr(req, attr)
+
+            if expected_value is None:
+                assert value is None
+                return
+
+            assert value is not None
+
+            for element, expected_element in zip(value, expected_value):
+                assert element == expected_element
+                if isinstance(expected_element, ETag):
+                    assert element.is_weak == expected_element.is_weak
+
     def _test_error_details(self, headers, attr_name,
                             error_type, title, description):
         req = Request(testing.create_environ(headers=headers))
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 4890e35..2626a2c 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -332,11 +332,47 @@ class TestFalconUtils(object):
             falcon.get_http_status('-404.3')
         assert falcon.get_http_status(123, 'Go Away') == '123 Go Away'
 
-    def test_etag_to_header(self):
+    def test_etag_dumps_to_header_format(self):
         etag = structures.ETag('67ab43')
-        assert etag.to_header() == '"67ab43"'
+
+        assert etag.dumps() == '"67ab43"'
+
         etag.is_weak = True
-        assert etag.to_header() == 'W/"67ab43"'
+        assert etag.dumps() == 'W/"67ab43"'
+
+        assert structures.ETag('67a b43').dumps() == '"67a b43"'
+
+    def test_etag_strong_vs_weak_comparison(self):
+        strong_67ab43_one = structures.ETag.loads('"67ab43"')
+        strong_67ab43_too = structures.ETag.loads('"67ab43"')
+        strong_67aB43 = structures.ETag.loads('"67aB43"')
+        weak_67ab43_one = structures.ETag.loads('W/"67ab43"')
+        weak_67ab43_two = structures.ETag.loads('W/"67ab43"')
+        weak_67aB43 = structures.ETag.loads('W/"67aB43"')
+
+        assert strong_67aB43 == strong_67aB43
+        assert weak_67aB43 == weak_67aB43
+        assert strong_67aB43 == weak_67aB43
+        assert weak_67aB43 == strong_67aB43
+        assert strong_67ab43_one == strong_67ab43_too
+        assert weak_67ab43_one == weak_67ab43_two
+
+        assert strong_67aB43 != strong_67ab43_one
+        assert strong_67ab43_one != strong_67aB43
+
+        assert strong_67aB43.strong_compare(strong_67aB43)
+        assert strong_67ab43_one.strong_compare(strong_67ab43_too)
+        assert not strong_67aB43.strong_compare(strong_67ab43_one)
+        assert not strong_67ab43_one.strong_compare(strong_67aB43)
+
+        assert not strong_67ab43_one.strong_compare(weak_67ab43_one)
+        assert not weak_67ab43_one.strong_compare(strong_67ab43_one)
+
+        assert not weak_67aB43.strong_compare(weak_67aB43)
+        assert not weak_67ab43_one.strong_compare(weak_67ab43_two)
+
+        assert not weak_67ab43_one.strong_compare(weak_67aB43)
+        assert not weak_67aB43.strong_compare(weak_67ab43_one)
 
 
 @pytest.mark.parametrize(