From bb6427ea9b8e824d5eecbc9000d046af724f8168 Mon Sep 17 00:00:00 2001
From: Min RK <benjaminrk@gmail.com>
Date: Wed, 30 Mar 2022 13:48:26 +0200
Subject: [PATCH] Add FrozenDict for caching parsed_scopes dicts

Since we need them to be immutable
---
 jupyterhub/_memoize.py           | 47 ++++++++++++++++++++++++++++++++
 jupyterhub/scopes.py             | 11 +++-----
 jupyterhub/tests/test_memoize.py | 21 +++++++++++++-
 3 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/jupyterhub/_memoize.py b/jupyterhub/_memoize.py
index 1ee2861e..a8167229 100644
--- a/jupyterhub/_memoize.py
+++ b/jupyterhub/_memoize.py
@@ -88,3 +88,50 @@ def lru_cache_key(key_func, maxsize=1024):
         return cached
 
     return cache_func
+
+
+class FrozenDict(dict):
+    """A frozen dictionary subclass
+
+    Immutable and hashable, so it can be used as a cache key
+
+    Values will be frozen with `.freeze(value)`
+    and must be hashable after freezing.
+
+    Not rigorous, but enough for our purposes.
+    """
+
+    _hash = None
+
+    def __init__(self, d):
+        dict_set = dict.__setitem__
+        for key, value in d.items():
+            dict.__setitem__(self, key, self._freeze(value))
+
+    def _freeze(self, item):
+        """Make values of a dict hashable
+        - list, set -> frozenset
+        - dict -> recursive _FrozenDict
+        - anything else: assumed hashable
+        """
+        if isinstance(item, FrozenDict):
+            return item
+        elif isinstance(item, (set, list)):
+            return frozenset(item)
+        elif isinstance(item, dict):
+            return FrozenDict(item)
+        else:
+            # any other type is assumed hashable
+            return item
+
+    def __setitem__(self, key):
+        raise RuntimeError("Cannot modify frozen {type(self).__name__}")
+
+    def update(self, other):
+        raise RuntimeError("Cannot modify frozen {type(self).__name__}")
+
+    def __hash__(self):
+        """Cache hash because we are immutable"""
+        if self._hash is None:
+            self._hash = hash(tuple((key, value) for key, value in self.items()))
+        return self._hash
diff --git a/jupyterhub/scopes.py b/jupyterhub/scopes.py
index ccceec9a..e17ba38e 100644
--- a/jupyterhub/scopes.py
+++ b/jupyterhub/scopes.py
@@ -24,7 +24,7 @@ from tornado import web
 from tornado.log import app_log
 
 from . import orm, roles
-from ._memoize import DoNotCache, lru_cache_key
+from ._memoize import DoNotCache, FrozenDict, lru_cache_key
 
 """when modifying the scope definitions, make sure that `docs/source/rbac/generate-scope-table.py` is run
    so that changes are reflected in the documentation and REST API description."""
@@ -698,13 +698,10 @@ def parse_scopes(scope_list):
                 parsed_scopes[base_scope][key] = {value}
             else:
                 parsed_scopes[base_scope][key].add(value)
-    return parsed_scopes
-
-
-# Note: it doesn't make sense to cache unparse_scopes
-# because computing the cache key is as expensive as the function itself
+    return FrozenDict(parsed_scopes)
 
 
+@lru_cache_key(FrozenDict)
 def unparse_scopes(parsed_scopes):
     """Turn a parsed_scopes dictionary back into a expanded scopes set"""
     expanded_scopes = set()
@@ -722,7 +719,7 @@ def unparse_scopes(parsed_scopes):
 def reduce_scopes(expanded_scopes):
     """Reduce expanded scopes to minimal set
 
-    Eliminates redundancy, such as access:services and access:services!service=x
+    Eliminates overlapping scopes, such as access:services and access:services!service=x
     """
     return unparse_scopes(parse_scopes(expanded_scopes))
 
diff --git a/jupyterhub/tests/test_memoize.py b/jupyterhub/tests/test_memoize.py
index a53cc7d5..c7525479 100644
--- a/jupyterhub/tests/test_memoize.py
+++ b/jupyterhub/tests/test_memoize.py
@@ -1,4 +1,6 @@
-from jupyterhub._memoize import DoNotCache, LRUCache, lru_cache_key
+import pytest
+
+from jupyterhub._memoize import DoNotCache, FrozenDict, LRUCache, lru_cache_key
 
 
 def test_lru_cache():
@@ -73,3 +75,20 @@ def test_do_not_cache():
     before = call_count
     assert is_even(1) == False
     assert call_count == before + 1
+
+
+@pytest.mark.parametrize(
+    "d",
+    [
+        {"key": "value"},
+        {"key": ["list"]},
+        {"key": {"set"}},
+        {"key": ("tu", "ple")},
+        {"key": {"nested": ["dict"]}},
+    ],
+)
+def test_frozen_dict(d):
+    frozen_1 = FrozenDict(d)
+    frozen_2 = FrozenDict(d)
+    assert hash(frozen_1) == hash(frozen_2)
+    assert frozen_1 == frozen_2