[bugfix] fix behavior of cached_property

This commit is a fairly large chunk of code that fixes a previously
annoying and nasty bug causing cached_properties to not be cleared.
Alongside this bug fix I took the opportunity to refactor the entire
methdology behind cached properties, and bind them more strictly to the
behavior of models.

Prior to this commit, cached_properties where not be properly reset upon
a model's `update` method being called due to some invalid code. Along
with this issue, the actual behavior of cached properties landed in a
weird in-between/gray area where they had to support both non-model and
model use cases.

The new version of cached_property is strictly for use with models, and
another version `simple_cached_property` was added to support other use
cases. The new cached_property simply builds an actual `property` within
the metaclass. This is fairly efficient, and also reduces the surface
area of behavior.

When writing this I messed around with a few ideas, including allowing
the user of `cached_property` to define a linkage between the property
and fields that should invalidate its cached value, but this was both
messy and introduced massive cognitive load on understand when a
cached_property should be cleared. Although this version is slighty less
efficient, I'm very much in favor of it vs the alternatives I tried.

disco/bot/command.py

@@ -6,7 +6,7 @@ from holster.enum import Enum
 from six import integer_types
 
 from disco.bot.parser import ArgumentSet, ArgumentError
-from disco.util.functional import cached_property
+from disco.util.functional import simple_cached_property
 
 ARGS_REGEX = '(?: ((?:\n|.)*)$|$)'
 ARGS_UNGROUPED_REGEX = '(?: (?:\n|.)*$|$)'
@@ -75,28 +75,28 @@ class CommandEvent(object):
 
         return src
 
-    @cached_property
+    @simple_cached_property
     def member(self):
         """
         Guild member (if relevant) for the user that created the message.
         """
         return self.guild.get_member(self.author)
 
-    @property
+    @simple_cached_property
     def channel(self):
         """
         Channel the message was created in.
         """
         return self.msg.channel
 
-    @property
+    @simple_cached_property
     def guild(self):
         """
         Guild (if relevant) the message was created in.
         """
         return self.msg.guild
 
-    @property
+    @simple_cached_property
     def author(self):
         """
         Author of the message.
@@ -243,7 +243,7 @@ class Command(object):
             raise TypeError('Cannot resolve mention: {}'.format(raw))
         return _f
 
-    @cached_property
+    @simple_cached_property
     def compiled_regex(self):
         """
         A compiled version of this command's regex.

disco/types/base.py

@@ -8,7 +8,6 @@ from datetime import datetime as real_datetime
 
 from disco.util.chains import Chainable
 from disco.util.hashmap import HashMap
-from disco.util.functional import CachedSlotProperty
 
 DATETIME_FORMATS = [
     '%Y-%m-%dT%H:%M:%S.%f',
@@ -33,6 +32,18 @@ class Unset(object):
 UNSET = Unset()
 
 
+def cached_property(method):
+    method._cached_property = set()
+    return method
+
+
+def strict_cached_property(*args):
+    def _cached_property(method):
+        method._cached_property = set(args)
+        return method
+    return _cached_property
+
+
 class ConversionError(Exception):
     def __init__(self, field, raw, e):
         super(ConversionError, self).__init__(
@@ -236,29 +247,51 @@ Model = None
 SlottedModel = None
 
 
+def _get_cached_property(name, func):
+    def _getattr(self):
+        try:
+            return getattr(self, '_' + name)
+        except AttributeError:
+            value = func(self)
+            setattr(self, '_' + name, value)
+            return value
+
+    def _setattr(self, value):
+        setattr(self, '_' + name)
+
+    def _delattr(self):
+        delattr(self, '_' + name)
+
+    prop = property(_getattr, _setattr, _delattr)
+    return prop
+
+
 class ModelMeta(type):
     def __new__(mcs, name, parents, dct):
         fields = {}
+        slots = set()
 
         for parent in parents:
             if Model and issubclass(parent, Model) and parent != Model:
                 fields.update(parent._fields)
 
         for k, v in six.iteritems(dct):
+            if hasattr(v, '_cached_property'):
+                dct[k] = _get_cached_property(k, v)
+                slots.add('_' + k)
+
             if not isinstance(v, Field):
                 continue
 
             v.name = k
             fields[k] = v
+            slots.add(k)
 
         if SlottedModel and any(map(lambda k: issubclass(k, SlottedModel), parents)):
-            bases = set(v.stored_name for v in six.itervalues(dct) if isinstance(v, CachedSlotProperty))
-
-            if '__slots__' in dct:
-                dct['__slots__'] = tuple(set(dct['__slots__']) | set(fields.keys()) | bases)
-            else:
-                dct['__slots__'] = tuple(fields.keys()) + tuple(bases)
+            # Merge our set of field slots with any other slots from the mro
+            dct['__slots__'] = tuple(set(dct.get('__slots__', [])) | slots)
 
+            # Remove all fields from the dict
             dct = {k: v for k, v in six.iteritems(dct) if k not in dct['__slots__']}
         else:
             dct = {k: v for k, v in six.iteritems(dct) if k not in fields}

disco/types/channel.py

@@ -5,9 +5,9 @@ from six.moves import map
 from holster.enum import Enum
 
 from disco.util.snowflake import to_snowflake
-from disco.util.functional import cached_property, one_or_many, chunks
+from disco.util.functional import one_or_many, chunks
 from disco.types.user import User
-from disco.types.base import SlottedModel, Field, AutoDictField, snowflake, enum, text
+from disco.types.base import SlottedModel, Field, AutoDictField, snowflake, enum, text, cached_property
 from disco.types.permissions import Permissions, Permissible, PermissionValue
 
 

disco/types/guild.py

@@ -6,9 +6,9 @@ from disco.gateway.packets import OPCode
 from disco.api.http import APIException
 from disco.util.paginator import Paginator
 from disco.util.snowflake import to_snowflake
-from disco.util.functional import cached_property
 from disco.types.base import (
-    SlottedModel, Field, ListField, AutoDictField, DictField, snowflake, text, enum, datetime
+    SlottedModel, Field, ListField, AutoDictField, DictField, snowflake, text, enum, datetime,
+    cached_property
 )
 from disco.types.user import User
 from disco.types.voice import VoiceState

disco/types/message.py

@@ -8,11 +8,10 @@ from holster.enum import Enum
 
 from disco.types.base import (
     SlottedModel, Field, ListField, AutoDictField, snowflake, text,
-    datetime, enum
+    datetime, enum, cached_property
 )
 from disco.util.paginator import Paginator
 from disco.util.snowflake import to_snowflake
-from disco.util.functional import cached_property
 from disco.types.user import User
 
 

disco/types/voice.py

@@ -1,5 +1,4 @@
-from disco.types.base import SlottedModel, Field, snowflake
-from disco.util.functional import cached_property
+from disco.types.base import SlottedModel, Field, snowflake, cached_property
 
 
 class VoiceState(SlottedModel):

disco/types/webhook.py

@@ -1,8 +1,7 @@
 import re
 
-from disco.types.base import SlottedModel, Field, snowflake
+from disco.types.base import SlottedModel, Field, snowflake, cached_property
 from disco.types.user import User
-from disco.util.functional import cached_property
 
 
 WEBHOOK_URL_RE = re.compile(r'\/api\/webhooks\/(\d+)\/(.[^/]+)')

disco/util/functional.py

@@ -48,28 +48,21 @@ def one_or_many(f):
     return _f
 
 
-class CachedSlotProperty(object):
-    __slots__ = ['stored_name', 'function', '__doc__']
-
-    def __init__(self, name, function):
-        self.stored_name = '_' + name
-        self.function = function
-        self.__doc__ = getattr(function, '__doc__')
-
-    def set(self, value):
-        setattr(self.stored_name, value)
-
-    def __get__(self, instance, owner):
-        if instance is None:
-            return self
+def simple_cached_property(method):
+    key = '_{}'.format(method.__name__)
 
+    def _getattr(inst):
         try:
-            return getattr(instance, self.stored_name)
+            return getattr(inst, key)
         except AttributeError:
-            value = self.function(instance)
-            setattr(instance, self.stored_name, value)
+            value = method(inst)
+            setattr(inst, key, value)
             return value
 
+    def _setattr(inst, value):
+        setattr(inst, key, value)
+
+    def _delattr(inst):
+        delattr(inst, key)
 
-def cached_property(f):
-    return CachedSlotProperty(f.__name__, f)
+    return property(_getattr, _setattr, _delattr)

disco/util/snowflake.py

@@ -26,10 +26,11 @@ def from_datetime(date):
 
 
 def from_timestamp(ts):
-    if six.PY3:
-        return int(ts * 1000.0 - DISCORD_EPOCH) << 22
-    else:
-        return long(ts * 1000.0 - DISCORD_EPOCH) << 22
+    return from_timestamp_ms(ts * 1000.0)
+
+
+def from_timestamp_ms(ts):
+    return int(ts - DISCORD_EPOCH) << 22
 
 
 def to_snowflake(i):

tests/types/base.py

@@ -0,0 +1,31 @@
+import pytest
+
+from disco.types.base import Model, Field, cached_property
+
+
+@pytest.fixture
+def model():
+    class TestModel(Model):
+        a = Field(int)
+        b = Field(int)
+
+        @cached_property
+        def value(self):
+            return self.a + self.b
+
+    return TestModel
+
+
+def test_cached_property(model):
+    inst = model(a=1, b=3)
+    assert inst.value == 4
+
+    inst.a = 2
+    assert inst.value == 4
+
+
+def test_cached_property_clear_on_update(model):
+    inst = model(a=1, b=3)
+    assert inst.value == 4
+    inst.update(model(a=2, b=3))
+    assert inst.value == 5

tests/util/functional.py

@@ -0,0 +1,21 @@
+from disco.util.functional import simple_cached_property
+
+
+def test_simple_cached_property():
+    class Test(object):
+        def __init__(self, a, b):
+            self.a = a
+            self.b = b
+
+        @simple_cached_property
+        def value(self):
+            return self.a + self.b
+
+    inst = Test(1, 1)
+    assert inst.value == 2
+
+    inst.a = 4
+    assert inst.value == 2
+
+    del inst.value
+    assert inst.value == 5

tests/utils.py

@@ -1,4 +1,8 @@
+import time
+import random
+
 from disco.api.client import APIClient as _APIClient
+from disco.util.snowflake import from_timestamp_ms
 
 
 class CallContainer(object):
@@ -13,3 +17,9 @@ class APIClient(_APIClient):
     def __init__(self):
         self.client = None
         self.http = CallContainer()
+
+
+def random_snowflake():
+    return from_timestamp_ms(
+        (time.time() * 1000.0) + random.randint(1, 9999)
+    )