Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use qualname to avoid key collisions #257

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions dogpile/cache/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from ..util import langhelpers


def function_key_generator(namespace, fn, to_str=str):
def function_key_generator(namespace, fn, to_str=str, use_qual_name=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about also in:

  • def function_multi_key_generator
  • def kwarg_function_key_generator

"""Return a function that generates a string
key, based on a given function as well as
arguments to the returned function itself.
Expand All @@ -16,17 +16,27 @@ def function_key_generator(namespace, fn, to_str=str):
the :paramref:`.CacheRegion.function_key_generator` argument
for :class:`.CacheRegion`.

:param namespace: A string namespace to be used in the key.
:param fn: The function which will be called.
:param to_str: A function that will convert arguments to strings.
:param use_qual_name: If True, use the qualified name of the function
instead of just the name. This is useful to avoid collision in modules
that defines more than one class containing methods with the same name.
fbidu marked this conversation as resolved.
Show resolved Hide resolved

.. seealso::

:func:`.kwarg_function_key_generator` - similar function that also
takes keyword arguments into account

"""
fn_name = fn.__name__
fbidu marked this conversation as resolved.
Show resolved Hide resolved
if use_qual_name:
fn_name = fn.__qualname__

if namespace is None:
namespace = "%s:%s" % (fn.__module__, fn.__name__)
namespace = "%s:%s" % (fn.__module__, fn_name)
else:
namespace = "%s:%s|%s" % (fn.__module__, fn.__name__, namespace)
namespace = "%s:%s|%s" % (fn.__module__, fn_name, namespace)

args = compat.inspect_getargspec(fn)
has_self = args[0] and args[0][0] in ("self", "cls")
Expand Down
69 changes: 69 additions & 0 deletions tests/cache/test_util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
from dogpile.cache import util


class A:
@classmethod
def class_method(cls):
pass

@staticmethod
def static_method():
pass

def instance_method(self):
pass

def nested_method(self):
def nested():
pass

return nested


def test_function_key_generator_qualname():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would break up each test case here and have them come in using pytest.mark.parametrize as a decorator on def test_function_key_generator_qualname. so one function, but the cases come in parameters. each parameter results in one test.

key_generator = util.function_key_generator(
None, A.class_method, use_qual_name=True
)
assert key_generator() == "tests.cache.test_util:A.class_method|"

key_generator = util.function_key_generator(
None, A.static_method, use_qual_name=True
)
assert key_generator() == "tests.cache.test_util:A.static_method|"

key_generator = util.function_key_generator(
None, A.instance_method, use_qual_name=True
)
assert key_generator() == "tests.cache.test_util:A.instance_method|"

key_generator = util.function_key_generator(
"namespace", A.class_method, use_qual_name=True
)
assert key_generator() == "tests.cache.test_util:A.class_method|namespace|"

nested = A().nested_method()
key_generator = util.function_key_generator(
None, nested, use_qual_name=True
)
assert (
key_generator()
== "tests.cache.test_util:A.nested_method.<locals>.nested|"
)


def test_function_key_generator():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, then also for the multi key and kwarg versions

key_generator = util.function_key_generator(None, A.class_method)
assert key_generator() == "tests.cache.test_util:class_method|"

key_generator = util.function_key_generator(None, A.static_method)
assert key_generator() == "tests.cache.test_util:static_method|"

key_generator = util.function_key_generator(None, A.instance_method)
assert key_generator() == "tests.cache.test_util:instance_method|"

key_generator = util.function_key_generator("namespace", A.class_method)
assert key_generator() == "tests.cache.test_util:class_method|namespace|"

nested = A().nested_method()
key_generator = util.function_key_generator(None, nested)
assert key_generator() == "tests.cache.test_util:nested|"