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

Allow to use __caching_id__ rather than __repr__ as an object caching key #123

Merged
merged 1 commit into from
Oct 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 14 additions & 5 deletions flask_caching/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ def get_arg_default(f, position):
return arg_def if arg_def != inspect.Parameter.empty else None


def get_id_func(obj):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this separate function is needed here? As i see you only use this value within get_id.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For future reuse and ease readability?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I simply canʼt think of a use case when you need the id generating function but not the ID. And this way you add an unnecessary function call to a oneliner with no significant improvement on readability. How about something like this?

def get_id(obj):
    id_func = getattr(obj, '__caching_id__', repr)

    return id_func(obj)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂

return getattr(obj, "__caching_id__", repr)


def get_id(obj):
return get_id_func(obj)(obj)


def function_namespace(f, args=None):
"""Attempts to returns unique namespace for function"""
m_args = get_arg_names(f)
Expand All @@ -79,10 +87,11 @@ def function_namespace(f, args=None):

instance_self = getattr(f, "__self__", None)


if instance_self and not inspect.isclass(instance_self):
instance_token = repr(f.__self__)
instance_token = get_id(f.__self__)
elif m_args and m_args[0] == "self" and args:
instance_token = repr(args[0])
instance_token = get_id(args[0])

module = f.__module__

Expand Down Expand Up @@ -595,11 +604,11 @@ def _memoize_kwargs_to_args(self, f, *args, **kwargs):
for i in range(args_len):
arg_default = get_arg_default(f, i)
if i == 0 and arg_names[i] in ("self", "cls"):
#: use the repr of the class instance
#: use the id func of the class instance
#: this supports instance methods for
#: the memoized functions, giving more
#: flexibility to developers
arg = repr(args[0])
arg = get_id(args[0])
arg_num += 1
elif arg_names[i] in kwargs:
arg = kwargs[arg_names[i]]
Expand All @@ -622,7 +631,7 @@ def _memoize_kwargs_to_args(self, f, *args, **kwargs):
# try:
# arg = hash(arg)
# except:
# arg = repr(arg)
# arg = get_id(arg)

#: Or what about a special __cacherepr__ function
#: on an object, this allows objects to act normal
Expand Down
24 changes: 24 additions & 0 deletions tests/test_memoize.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,30 @@ def add(self, b):
assert adder1.add(3) != adder2.add(3)


def test_memoize_classfunc_repr(app, cache):
class Adder(object):
def __init__(self, initial):
self.initial = initial

@cache.memoize()
def add(self, b):
return self.initial + b

def __repr__(self):
return "42"

def __caching_id__(self):
return self.initial

adder1 = Adder(1)
adder2 = Adder(2)

x = adder1.add(3)
assert adder1.add(3) == x
assert adder1.add(4) != x
assert adder1.add(3) != adder2.add(3)


def test_memoize_classfunc_delete(app, cache):
with app.test_request_context():
class Adder(object):
Expand Down