Add a hasProperties method to builds

This better-defines the interface of the object passed to WithProperties
callables, including a hasProperties method; fixes MailNotifier to
render properties properly in extraHeaders; and adds a lot of tests.

Fixes #2024 and Fixes #2021.

master/buildbot/interfaces.py

@@ -1119,8 +1119,71 @@ class IRenderable(Interface):
     """An object that can be interpolated with properties from a build.
     """
 
-    def getRenderingFor(build):
+    def getRenderingFor(iprops):
         """Return the interpolation with the given properties
 
-        @param build: the L{Build} instance for which interpolation is being done.
+        @param iprops: the L{IProperties} provider supplying the properties.
+        """
+class IProperties(Interface):
+    """
+    An object providing access to build properties
+    """
+
+    def getProperty(name, default=None):
+        """Get the named property, returning the default if the property does
+        not exist.
+
+        @param name: property name
+        @type name: string
+
+        @param default: default value (default: @code{None})
+
+        @returns: property value
+        """
+
+    def hasProperty(name):
+        """Return true if the named property exists.
+
+        @param name: property name
+        @type name: string
+        @returns: boolean
+        """
+
+    def has_key(name):
+        """Deprecated name for L{hasProperty}."""
+
+    def setProperty(name, value, source, runtime=False):
+        """Set the given property, overwriting any existing value.  The source
+        describes the source of the value for human interpretation.
+
+        @param name: property name
+        @type name: string
+
+        @param value: property value
+        @type value: JSON-able value
+
+        @param source: property source
+        @type source: string
+
+        @param runtime: (optional) whether this property was set during the
+        build's runtime: usually left at its default value
+        @type runtime: boolean
+        """
+
+    def getProperties():
+        """Get the L{buildbot.process.properties.Properties} instance storing
+        these properties.  Note that the interfaec for this class is not
+        stable, so where possible the other methods of this interface should be
+        used.
+
+        @returns: L{buildbot.process.properties.Properties} instance
+        """
+
+    def render(value):
+        """Render @code{value} as an L{IRenderable}.  This essentially coerces
+        @code{value} to an L{IRenderable} and calls its @L{getRenderingFor}
+        method.
+
+        @name value: value to render
+        @returns: rendered value
         """

master/buildbot/process/build.py

@@ -17,7 +17,7 @@
 import types
 
 from zope.interface import implements
-from twisted.python import log
+from twisted.python import log, components
 from twisted.python.failure import Failure
 from twisted.internet import reactor, defer, error
 
@@ -26,10 +26,10 @@
   RETRY, SKIPPED, worst_status
 from buildbot.status.builder import Results
 from buildbot.status.progress import BuildProgress
-from buildbot.process import metrics
+from buildbot.process import metrics, properties
 
 
-class Build:
+class Build(properties.PropertiesMixin):
     """I represent a single build by a single slave. Specialized Builders can
     use subclasses of Build to hold status information unique to those build
     processes.
@@ -60,6 +60,7 @@ class Build:
     finished = False
     results = None
     stopped = False
+    set_runtime_properties = True
 
     def __init__(self, requests):
         self.requests = requests
@@ -94,25 +95,6 @@ def setSlaveEnvironment(self, env):
     def getSourceStamp(self):
         return self.source
 
-    def setProperty(self, propname, value, source, runtime=True):
-        """Set a property on this build. This may only be called after the
-        build has started, so that it has a BuildStatus object where the
-        properties can live."""
-        self.build_status.setProperty(propname, value, source, runtime=True)
-
-    def getProperties(self):
-        return self.build_status.getProperties()
-
-    def getProperty(self, propname):
-        return self.build_status.getProperty(propname)
-
-    def render(self, value):
-        """
-        Return a variant of value that has any WithProperties objects
-        substituted.  This recurses into Python's compound data types.
-        """
-        return interfaces.IRenderable(value).getRenderingFor(self)
-
     def allChanges(self):
         return self.source.changes
 
@@ -153,8 +135,6 @@ def setStepFactories(self, step_factories):
         build.allFiles() ."""
         self.stepFactories = list(step_factories)
 
-
-
     useProgress = True
 
     def getSlaveCommandVersion(self, command, oldversion=None):
@@ -546,4 +526,6 @@ def getStatus(self):
 
     # stopBuild is defined earlier
 
-
+components.registerAdapter(
+        lambda build : interfaces.IProperties(build.build_status),
+        Build, interfaces.IProperties)

master/buildbot/process/properties.py

@@ -16,7 +16,7 @@
 import re
 import weakref
 from buildbot import util
-from buildbot.interfaces import IRenderable
+from buildbot.interfaces import IRenderable, IProperties
 from twisted.python.components import registerAdapter
 from zope.interface import implements
 
@@ -37,6 +37,7 @@ class Properties(util.ComparableMixin):
     """
 
     compare_attrs = ('properties',)
+    implements(IProperties)
 
     def __init__(self, **kwargs):
         """
@@ -71,13 +72,6 @@ def __getitem__(self, name):
     def __nonzero__(self):
         return not not self.properties
 
-    def has_key(self, name):
-        return self.properties.has_key(name)
-
-    def getProperty(self, name, default=None):
-        """Get the value for the given property."""
-        return self.properties.get(name, (default,))[0]
-
     def getPropertySource(self, name):
         return self.properties[name][1]
 
@@ -96,11 +90,6 @@ def __repr__(self):
                 repr(dict((k,v[0]) for k,v in self.properties.iteritems())) +
                 ')')
 
-    def setProperty(self, name, value, source, runtime=False):
-        self.properties[name] = (value, source)
-        if runtime:
-            self.runtime.add(name)
-
     def update(self, dict, source, runtime=False):
         """Update this object from a dictionary, with an explicit source specified."""
         for k, v in dict.items():
@@ -120,6 +109,68 @@ def updateFromPropertiesNoRuntime(self, other):
             if k not in other.runtime:
                 self.properties[k] = v
 
+    # IProperties methods
+
+    def getProperty(self, name, default=None):
+        return self.properties.get(name, (default,))[0]
+
+    def hasProperty(self, name):
+        return self.properties.has_key(name)
+
+    has_key = hasProperty
+
+    def setProperty(self, name, value, source, runtime=False):
+        self.properties[name] = (value, source)
+        if runtime:
+            self.runtime.add(name)
+
+    def getProperties(self):
+        return self
+
+    def render(self, value):
+        renderable = IRenderable(value)
+        return renderable.getRenderingFor(self)
+
+
+class PropertiesMixin:
+    """
+    A mixin to add L{IProperties} methods to a class which does not implement
+    the interface, but which can be coerced to the interface via an adapter.
+
+    This is useful because L{IProperties} methods are often called on L{Build}
+    and L{BuildStatus} objects without first coercing them.
+
+    @ivar set_runtime_properties: the default value for the C{runtime}
+    parameter of L{setProperty}.
+    """
+
+    set_runtime_properties = False
+
+    def getProperty(self, propname, default=None):
+        props = IProperties(self)
+        return props.getProperty(propname, default)
+
+    def hasProperty(self, propname):
+        props = IProperties(self)
+        return props.hasProperty(propname)
+
+    has_key = hasProperty
+
+    def setProperty(self, propname, value, source, runtime=None):
+        props = IProperties(self)
+        if runtime is None:
+            runtime = self.set_runtime_properties
+        props.setProperty(propname, value, source, runtime=runtime)
+
+    def getProperties(self):
+        return IProperties(self)
+
+    def render(self, value):
+        props = IProperties(self)
+        return props.render(value)
+
+
+
 class PropertyMap:
     """
     Privately-used mapping object to implement WithProperties' substitutions,

master/buildbot/status/build.py

@@ -16,14 +16,14 @@
 import os, shutil, re
 from cPickle import dump
 from zope.interface import implements
-from twisted.python import log, runtime
+from twisted.python import log, runtime, components
 from twisted.persisted import styles
 from twisted.internet import reactor, defer
 from buildbot import interfaces, util, sourcestamp
-from buildbot.process.properties import Properties
+from buildbot.process import properties
 from buildbot.status.buildstep import BuildStepStatus
 
-class BuildStatus(styles.Versioned):
+class BuildStatus(styles.Versioned, properties.PropertiesMixin):
     implements(interfaces.IBuildStatus, interfaces.IStatusEvent)
 
     persistenceVersion = 3
@@ -41,6 +41,8 @@ class BuildStatus(styles.Versioned):
     results = None
     slavename = "???"
 
+    set_runtime_properties = True
+
     # these lists/dicts are defined here so that unserialized instances have
     # (empty) values. They are set in __init__ to new objects to make sure
     # each instance gets its own copy.
@@ -62,7 +64,7 @@ def __init__(self, parent, number):
         self.finishedWatchers = []
         self.steps = []
         self.testResults = {}
-        self.properties = Properties()
+        self.properties = properties.Properties()
 
     def __repr__(self):
         return "<%s #%s>" % (self.__class__.__name__, self.number)
@@ -75,12 +77,6 @@ def getBuilder(self):
         """
         return self.builder
 
-    def getProperty(self, propname):
-        return self.properties[propname]
-
-    def getProperties(self):
-        return self.properties
-
     def getNumber(self):
         return self.number
 
@@ -238,9 +234,6 @@ def addStepWithName(self, name):
         self.steps.append(s)
         return s
 
-    def setProperty(self, propname, value, source, runtime=True):
-        self.properties.setProperty(propname, value, source, runtime)
-
     def addTestResult(self, result):
         self.testResults[result.getName()] = result
 
@@ -387,7 +380,7 @@ def upgradeToVersion2(self):
     def upgradeToVersion3(self):
         # in version 3, self.properties became a Properties object
         propdict = self.properties
-        self.properties = Properties()
+        self.properties = properties.Properties()
         self.properties.update(propdict, "Upgrade from previous version")
         self.wasUpgraded = True
 
@@ -460,5 +453,5 @@ def asDict(self):
             result['currentStep'] = None
         return result
 
-
-
+components.registerAdapter(lambda build_status : build_status.properties,
+        BuildStatus, interfaces.IProperties)

master/buildbot/status/mail.py

@@ -544,15 +544,14 @@ def createEmail(self, msgdict, builderName, title, results, builds=None,
         if self.extraHeaders:
             for k,v in self.extraHeaders.items():
                 if len(builds) == 1:
-                    k = builds[0].render(k)
+                    k = interfaces.IProperties(builds[0]).render(k)
                 if k in m:
                     twlog.msg("Warning: Got header " + k +
                       " in self.extraHeaders "
                       "but it already exists in the Message - "
                       "not adding it.")
-                continue
-                if len(builds == 1):
-                    m[k] = builds[0].render(v)
+                if len(builds) == 1:
+                    m[k] = interfaces.IProperties(builds[0]).render(v)
                 else:
                     m[k] = v