Raise an exception if a construct only property is set after construction

	* gobject/pygobject.c: (PyGProps_setattro),
	(pygobject_set_property):
	* tests/test_properties.py:
	Raise an exception if a construct only property is set after
	construction time. Refactor property setting code and add some
	more tests.

3 files changed, 96 insertions, 59 deletions

diff --git a/gobject/gobjectmodule.c b/gobject/gobjectmodule.c
index 7e393343..b6dba0d4 100644
--- a/gobject/gobjectmodule.c
+++ b/gobject/gobjectmodule.c
@@ -2558,7 +2558,9 @@ pygobject_constructv(PyGObject  *self,
     } else {
         int i;
         for (i = 0; i < n_parameters; ++i)
-            g_object_set_property(self->obj, parameters[i].name, &parameters[i].value);
+            g_object_set_property(self->obj,
+				  parameters[i].name,
+				  &parameters[i].value);
     }
     return 0;
 }
diff --git a/gobject/pygobject.c b/gobject/pygobject.c
index 1fe837d2..8d6deb2d 100644
--- a/gobject/pygobject.c
+++ b/gobject/pygobject.c
@@ -231,11 +231,44 @@ PyGProps_getattro(PyGProps *self, PyObject *attr)
     return ret;
 }
 
+static gboolean
+set_property_from_pspec(GObject *obj,
+			char *attr_name,
+			GParamSpec *pspec,
+			PyObject *pvalue)
+{
+    GValue value = { 0, };
+
+    if (pspec->flags & G_PARAM_CONSTRUCT_ONLY) {
+	PyErr_Format(PyExc_TypeError,
+		     "property '%s' can only be set in constructor",
+		     attr_name);
+	return FALSE;
+    }	
+
+    if (!(pspec->flags & G_PARAM_WRITABLE)) {
+	PyErr_Format(PyExc_TypeError,
+		     "property '%s' is not writable", attr_name);
+	return FALSE;
+    }	
+
+    g_value_init(&value, G_PARAM_SPEC_VALUE_TYPE(pspec));
+    if (pyg_param_gvalue_from_pyobject(&value, pvalue, pspec) < 0) {
+	PyErr_SetString(PyExc_TypeError,
+			"could not convert argument to correct param type");
+	return FALSE;
+    }
+
+    g_object_set_property(obj, attr_name, &value);
+    g_value_unset(&value);
+    
+    return TRUE;
+}
+
 static int
 PyGProps_setattro(PyGProps *self, PyObject *attr, PyObject *pvalue)
 {
     GParamSpec *pspec;
-    GValue value = { 0, };
     char *attr_name;
     GObject *obj;
     
@@ -263,21 +296,9 @@ PyGProps_setattro(PyGProps *self, PyObject *attr, PyObject *pvalue)
 	return PyObject_GenericSetAttr((PyObject *)self, attr, pvalue);
     }
 
-    if (!(pspec->flags & G_PARAM_WRITABLE)) {
-	PyErr_Format(PyExc_TypeError,
-		     "property '%s' is not writable", attr_name);
-	return -1;
-    }	
-
-    g_value_init(&value, G_PARAM_SPEC_VALUE_TYPE(pspec));
-    if (pyg_param_gvalue_from_pyobject(&value, pvalue, pspec) < 0) {
-	PyErr_SetString(PyExc_TypeError,
-			"could not convert argument to correct param type");
+    if (!set_property_from_pspec(obj, attr_name, pspec, pvalue))
 	return -1;
-    }
-
-    g_object_set_property(obj, attr_name, &value);
-    g_value_unset(&value);
+				  
     return 0;
 }
 
@@ -952,7 +973,7 @@ pygobject_init(PyGObject *self, PyObject *args, PyObject *kwargs)
 	}
     }
     if (pygobject_constructv(self, n_params, params))
-	PyErr_SetString (PyExc_RuntimeError, "could not create object");
+	PyErr_SetString(PyExc_RuntimeError, "could not create object");
 	   
  cleanup:
     for (i = 0; i < n_params; i++) {
@@ -1008,7 +1029,6 @@ pygobject_set_property(PyGObject *self, PyObject *args)
 {
     gchar *param_name;
     GParamSpec *pspec;
-    GValue value = { 0, };
     PyObject *pvalue;
 
     if (!PyArg_ParseTuple(args, "sO:GObject.set_property", &param_name,
@@ -1021,19 +1041,10 @@ pygobject_set_property(PyGObject *self, PyObject *args)
 			"the object does not support the given parameter");
 	return NULL;
     }
-    if (!(pspec->flags & G_PARAM_WRITABLE)) {
-	PyErr_Format(PyExc_TypeError, "property %s is not writable",
-		     param_name);
-	return NULL;
-    }
-    g_value_init(&value, G_PARAM_SPEC_VALUE_TYPE(pspec));
-    if (pyg_param_gvalue_from_pyobject(&value, pvalue, pspec) < 0) {
-	PyErr_SetString(PyExc_TypeError,
-			"could not convert argument to correct param type");
+    
+    if (!set_property_from_pspec(self->obj, param_name, pspec, pvalue))
 	return NULL;
-    }
-    g_object_set_property(self->obj, param_name, &value);
-    g_value_unset(&value);
+    
     Py_INCREF(Py_None);
     return Py_None;
 }
diff --git a/tests/test_properties.py b/tests/test_properties.py
index 970b6460..1c4427e8 100644
--- a/tests/test_properties.py
+++ b/tests/test_properties.py
@@ -2,55 +2,61 @@
 import unittest
 
 from common import testhelper
-from gobject import *
+from gobject import GObject, GType, new, PARAM_READWRITE, \
+     PARAM_CONSTRUCT, PARAM_CONSTRUCT_ONLY
 
 class PropertyObject(GObject):
     __gproperties__ = {
-        'property': (str, 'blurb', 'description',
-                     'default',
+        'normal': (str, 'blurb', 'description',  'default',
                      PARAM_READWRITE),
-        'construct-property': (str, 'blurb', 'description',
-                     'default',
+        'construct': (str, 'blurb', 'description', 'default',
+                     PARAM_READWRITE|PARAM_CONSTRUCT),
+        'construct-only': (str, 'blurb', 'description', 'default',
                      PARAM_READWRITE|PARAM_CONSTRUCT_ONLY),
     }
+    
     def __init__(self):
         GObject.__init__(self)
         self._value = 'default'
-        self._construct_property = None
+        self._construct_only = None
+        self._construct = None
         
     def do_get_property(self, pspec):
-        if pspec.name == 'property':
+        if pspec.name == 'normal':
             return self._value
-        elif pspec.name == 'construct-property':
-            return self._construct_property
-        raise AssertionError
-
+        elif pspec.name == 'construct':
+            return self._construct
+        elif pspec.name == 'construct-only':
+            return self._construct_only
+        else:
+            raise AssertionError
 
     def do_set_property(self, pspec, value):
-        if pspec.name == 'property':
+        if pspec.name == 'normal':
             self._value = value
-        elif pspec.name == 'construct-property':
-            self._construct_property = value
+        elif pspec.name == 'construct':
+            self._construct = value
+        elif pspec.name == 'construct-only':
+            self._construct_only = value
         else:
             raise AssertionError
-            
         
 class TestProperties(unittest.TestCase):
     def testGetSet(self):
         obj = PropertyObject()
-        obj.props.property = "value"
-        self.assertEqual(obj.props.property, "value")
+        obj.props.normal = "value"
+        self.assertEqual(obj.props.normal, "value")
 
     def testListWithInstance(self):
         obj = PropertyObject()
-        self.failUnless(hasattr(obj.props, "property"))
+        self.failUnless(hasattr(obj.props, "normal"))
 
     def testListWithoutInstance(self):
-        self.failUnless(hasattr(PropertyObject.props, "property"))
+        self.failUnless(hasattr(PropertyObject.props, "normal"))
 
     def testSetNoInstance(self):
         def set(obj):
-            obj.props.property = "foobar"
+            obj.props.normal = "foobar"
             
         self.assertRaises(TypeError, set, PropertyObject)
 
@@ -59,13 +65,31 @@ class TestProperties(unittest.TestCase):
             for pspec in obj:
                 gtype = GType(pspec)
                 self.assertEqual(gtype.parent.name, 'GParam')
-                self.assert_(pspec.name in ['property', 'construct-property'])
-
-            self.assertEqual(len(obj), 2)
+                self.failUnless(pspec.name in ['normal',
+                                               'construct',
+                                               'construct-only'])
+            self.assertEqual(len(obj), 3)
 
-    def testConstructProperty(self):
-        obj = new(PropertyObject, construct_property="123")
-        self.assertEqual(obj.props.construct_property, "123")
-        ## TODO: raise exception when setting construct-only properties
-        #obj.set_property("construct-property", "456")
-        #self.assertEqual(obj.props.construct_property, "456")
+    def testNormal(self):
+        obj = new(PropertyObject, normal="123")
+        self.assertEqual(obj.props.normal, "123")
+        obj.set_property('normal', '456')
+        self.assertEqual(obj.props.normal, "456")
+        obj.props.normal = '789'
+        self.assertEqual(obj.props.normal, "789")
+        
+    def testConstruct(self):
+        obj = new(PropertyObject, construct="123")
+        self.assertEqual(obj.props.construct, "123")
+        obj.set_property('construct', '456')
+        self.assertEqual(obj.props.construct, "456")
+        obj.props.construct = '789'
+        self.assertEqual(obj.props.construct, "789")
+                         
+    def testConstructOnly(self):
+        obj = new(PropertyObject, construct_only="123")
+        self.assertEqual(obj.props.construct_only, "123")
+        self.assertRaises(TypeError,
+                          setattr, obj.props, 'construct_only', '456')
+        self.assertRaises(TypeError,
+                          obj.set_property, 'construct-only', '456')