Merge lp:~methanal-developers/methanal/844872-lazy-expander-content-updates into lp:methanal

[Jonathan Jacobs]

[Tristan Seligmann (mithrandi)]

getHeaderContent and getExpanderContent should have unit tests; these shouldn't be too hard to set up.

review: Needs Fixing

[lp:~methanal-developers/methanal/844872-lazy-expander-content-updates]

192. By Jonathan Jacobs on 2011-09-09

Unit tests.

[Jonathan Jacobs (jjacobs)]

> getHeaderContent and getExpanderContent should have unit tests; these
> shouldn't be too hard to set up.

Done.

[Tristan Seligmann (mithrandi)]

review: Approve

=== modified file 'methanal/test/test_widgets.py'
--- methanal/test/test_widgets.py	2010-04-25 13:48:14 +0000
+++ methanal/test/test_widgets.py	2011-09-09 14:38:49 +0000
@@ -1,10 +1,14 @@
 """
 Unit tests for L{methanal.widgets}.
 """
+from functools import partial
+
 from twisted.trial import unittest
 
 from nevow import inevow
 
+from xmantissa.webtheme import ThemedElement
+
 from methanal import widgets, errors
 
 
@@ -250,3 +254,86 @@
         self.assertEquals(
             repr(tabGroup),
             "<TabGroup id=u'id' title=u'Title' tabs=%r>" % tabs)
+
+
+
+class ComparableLiveElement(ThemedElement):
+    """
+    C{LiveElement} implementing C{__eq__} comparing C{self.value}.
+    """
+    def __init__(self, value):
+        self.value = value
+
+
+    def __eq__(self, other):
+        try:
+            return self.value == other.value
+        except AttributeError:
+            return False
+
+
+
+class ExpanderTests(unittest.TestCase):
+    """
+    Tests for L{methanal.widgets.Expander}.
+    """
+    def test_getContent(self):
+        """
+        L{Expander.getContent} returns the content for the specified node ID,
+        C{KeyError} is raised if an unknown node ID is specified.
+        """
+        expander = widgets.Expander(
+            headerFactory=partial(ComparableLiveElement, u'foo'),
+            contentFactory=partial(ComparableLiveElement, u'bar'))
+        self.assertEqual(
+            expander.getContent(u'header').value, u'foo')
+        self.assertEqual(
+            expander.getContent(u'content').value, u'bar')
+        self.assertRaises(
+            KeyError, expander.getContent, u'not_a_thing')
+
+
+    def test_lazyHeaderContent(self):
+        """
+        L{Expander.getHeaderContent} returns the result of
+        L{Expander.headerFactory} if C{currentContent == newContent} returns
+        C{False}, otherwise C{None} is returned.
+        """
+        expander = widgets.Expander(
+            headerFactory=partial(ComparableLiveElement, u'content'),
+            contentFactory=None)
+
+        # Fresh.
+        content = expander.getHeaderContent()
+        self.assertNotIdentical(None, content)
+        self.assertEqual(u'content', content.value)
+        # Same as last time
+        self.assertIdentical(None, expander.getHeaderContent())
+        # New value.
+        expander.headerFactory = partial(ComparableLiveElement, u'other')
+        content = expander.getHeaderContent()
+        self.assertNotIdentical(None, content)
+        self.assertEqual(u'other', content.value)
+
+
+    def test_lazyExpanderContent(self):
+        """
+        L{Expander.getExpanderContent} returns the result of
+        L{Expander.contentFactory} if C{currentContent == newContent} returns
+        C{False}, otherwise C{None} is returned.
+        """
+        expander = widgets.Expander(
+            headerFactory=None,
+            contentFactory=partial(ComparableLiveElement, u'content'))
+
+        # Fresh.
+        content = expander.getExpanderContent()
+        self.assertNotIdentical(None, content)
+        self.assertEqual(u'content', content.value)
+        # Same as last time
+        self.assertIdentical(None, expander.getExpanderContent())
+        # New value.
+        expander.contentFactory = partial(ComparableLiveElement, u'other')
+        content = expander.getExpanderContent()
+        self.assertNotIdentical(None, content)
+        self.assertEqual(u'other', content.value)

=== modified file 'methanal/widgets.py'
--- methanal/widgets.py	2011-07-11 11:00:11 +0000
+++ methanal/widgets.py	2011-09-09 14:38:49 +0000
@@ -1083,6 +1083,10 @@
 
     @type expanded: C{bool}
     @ivar expanded: Is the content visible?
+
+    @ivar currentHeaderContent: Current header renderable.
+
+    @ivar currentExpanderContent: Current header renderable.
     """
     fragmentName = 'methanal-expander'
     jsClass = u'Methanal.Widgets.Expander'
@@ -1093,6 +1097,8 @@
         self.headerFactory = headerFactory
         self.contentFactory = contentFactory
         self.expanded = expanded
+        self.currentHeaderContent = None
+        self.currentExpanderContent = None
 
 
     def getInitialArguments(self):
@@ -1101,31 +1107,74 @@
 
     @expose
     def getContent(self, nodeID):
+        """
+        Get the content for C{nodeID}.
+
+        @rtype: I{renderable}
+        """
         return {
             'header':  self.getHeaderContent,
             'content': self.getExpanderContent}[nodeID]()
 
 
     def getHeaderContent(self):
+        """
+        Get the content for the expander header.
+
+        The content factory is invoked and its result compared to
+        L{currentHeaderContent}, with C{__eq__}, if the result is C{True}
+        then C{None} is returned.
+
+        @return: Renderable or C{None} if the content hasn't changed.
+        """
         content = self.headerFactory()
+        if content == self.currentHeaderContent:
+            return None
         content.setFragmentParent(self)
+        self.currentHeaderContent = content
         return content
 
 
     def getExpanderContent(self):
+        """
+        Get the content for the expander body.
+
+        The content factory is invoked and its result compared to
+        L{currentExpanderContent}, with C{__eq__}, if the result is C{True}
+        then C{None} is returned.
+
+        @return: Renderable or C{None} if the content hasn't changed.
+        """
         content = self.contentFactory()
+        if content == self.currentExpanderContent:
+            return None
         content.setFragmentParent(self)
+        self.currentExpanderContent = content
         return content
 
 
     def updateRemoteHeaderContent(self):
-        return self.callRemote(
-            'setContentFromWidgetInfo', self.getHeaderContent(), u'header')
+        """
+        Update the client-side header content.
+
+        If L{getHeaderContent} returns C{None} no update is performed.
+        """
+        content = self.getHeaderContent()
+        if content is not None:
+            return self.callRemote(
+                'setContentFromWidgetInfo', content, u'header')
 
 
     def updateRemoteContent(self):
-        return self.callRemote(
-            'setContentFromWidgetInfo', self.getExpanderContent(), u'content')
+        """
+        Update the client-side body content.
+
+        If L{getExpanderContent} returns C{None} no update is performed.
+        """
+        content = self.getExpanderContent()
+        if content is not None:
+            return self.callRemote(
+                'setContentFromWidgetInfo', content, u'content')
 
 
     @renderer