Merge lp:~lvh/divmod.org/storeless-conform-1118498 into lp:divmod.org

[Laurens Van Houtven]

Adds a test case for __conform__ for unstored objects, and changes the implementation to pass that test.

This means that unstored objects with in-memory powerups can be adapted to their interfaces. Previously, in-memory powerups were useless for unstored objects.

[Tristan Seligmann (mithrandi)]

This basically looks good, I only have two trivial things to comment on:

8 -
9 aggregateInterfaces = {
10 IService: serviceSpecialCase,
11 - IServiceCollection: serviceSpecialCase}
12 + IServiceCollection: serviceSpecialCase
13 + }

Instead of getting into a convoluted discussion about coding style, I'll just suggest that since you're not otherwise changing this code, the formatting should be left as-is.

36 + # adapt every popwerup to IPowerupIndirector, calling this method.

Typo: "popwerup" -> "powerup"

review: Needs Fixing

[lp:~lvh/divmod.org/storeless-conform-1118498]

2704. By Laurens Van Houtven on 2013-05-02

Fix popwerup -> powerup typo

2705. By Laurens Van Houtven on 2013-05-02

Undo style change

[Laurens Van Houtven (lvh)]

Hey Tristan,

Thanks for your review, I've made the requested changes.

[Tristan Seligmann (mithrandi)]

review: Approve

=== modified file 'Axiom/axiom/item.py'
--- Axiom/axiom/item.py	2010-04-03 22:36:04 +0000
+++ Axiom/axiom/item.py	2013-05-02 12:51:26 +0000
@@ -193,7 +193,6 @@
         return value is the powerup.  These are used only by the callable
         interface adaption API, not C{powerupsFor}.
     """
-
     aggregateInterfaces = {
         IService: serviceSpecialCase,
         IServiceCollection: serviceSpecialCase}
@@ -314,19 +313,22 @@
         special rules.  The full list of such interfaces is present in the
         'aggregateInterfaces' class attribute.
         """
-        if (self.store is None  # Don't bother doing a *query* if we're not
-                                # even stored in a store yet
-            or interface is IPowerupIndirector): # you can't do a query for
-                                                 # IPowerupIndirector, that
-                                                 # would just start an infinite
-                                                 # loop.
-            return
-        pups = self.powerupsFor(interface)
-        agg = self.aggregateInterfaces
-        if interface in agg:
-            return agg[interface](self, pups)
-        for p in pups:
-            return p
+        if interface is IPowerupIndirector:
+            # This would cause an infinite loop, since powerupsFor will try to
+            # adapt every powerup to IPowerupIndirector, calling this method.
+            return
+
+        try:
+            pups = self.powerupsFor(interface)
+        except AttributeError:  # self.store is None -> self.store.query...
+            return
+
+        aggregator = self.aggregateInterfaces.get(interface, None)
+        if aggregator is not None:
+            return aggregator(self, pups)
+
+        for pup in pups:
+            return pup  # return first one, or None if no powerups
 
 
     def powerupsFor(self, interface):

=== modified file 'Axiom/axiom/test/test_powerup.py'
--- Axiom/axiom/test/test_powerup.py	2008-10-09 13:17:41 +0000
+++ Axiom/axiom/test/test_powerup.py	2013-05-02 12:51:26 +0000
@@ -289,14 +289,19 @@
     """
     Tests for the behavior of powerups which are not database-resident.
     """
+    def _createEmpowered(self, withStore=True):
+        powerup = object()
+        item = SumContributor(store=Store() if withStore else None)
+        item.inMemoryPowerUp(powerup, ISumProducer)
+        return powerup, item
+
+
     def test_powerupsFor(self):
         """
         L{Item.powerupsFor} returns a list the first element of which is the
         object previously passed to L{Item.inMemoryPowerUp}.
         """
-        powerup = object()
-        item = SumContributor(store=Store())
-        item.inMemoryPowerUp(powerup, ISumProducer)
+        powerup, item = self._createEmpowered()
         self.assertEqual(list(item.powerupsFor(ISumProducer)), [powerup])
 
 
@@ -306,8 +311,15 @@
         that item for that interface even if there are database-resident
         powerups on that item for that interface.
         """
-        powerup = object()
-        item = SumContributor(store=Store())
-        item.inMemoryPowerUp(powerup, ISumProducer)
+        powerup, item = self._createEmpowered()
         item.powerUp(item, ISumProducer)
         self.assertIdentical(ISumProducer(item), powerup)
+
+
+    def test_conformWithoutStore(self):
+        """
+        Adaptation (through L{Item.__conform__}) should be allowed even if the
+        object is not stored, as long as it has an in-memory powerup.
+        """
+        powerup, item = self._createEmpowered(withStore=False)
+        self.assertIdentical(ISumProducer(item), powerup)