[CMF-checkins] CVS: CMF/CMFCore - TypesTool.py:1.40

[Chris McDonough]

Update of /cvs-repository/CMF/CMFCore
In directory cvs.zope.org:/tmp/cvs-serv8190/CMFCore

Modified Files:
	TypesTool.py 
Log Message:
Changed isConstructionAllowed method to not mask construction errors.  Before this change was made, it was possible for isConstructionAllowed to raise Unauthorized, masking a situation in which the construction was not possible because a factory method didn't exist or was invalid.

We removed the raise_exc argument to isConstructionAllowed.  It was never part of the interface.  Now, isConstructionAllowed will never raise, and if there is an error during the checking of allow-ed-ness, it will be logged to the stupid log file.


=== CMF/CMFCore/TypesTool.py 1.39 => 1.40 ===
 from ActionProviderBase import ActionProviderBase
 from ActionInformation import ActionInformation
 from Expression import Expression
-from zLOG import LOG, WARNING
+from zLOG import LOG, WARNING, ERROR
+import sys
 
 from CMFCorePermissions import View, ManagePortal, AccessContentsInformation
 
@@ -445,28 +446,43 @@
     #
     #   Agent methods
     #
-    def _getFactoryMethod(self, container, raise_exc=0):
+    def _getFactoryMethod(self, container):
         if not self.product or not self.factory:
-            return None
+            raise ValueError, ('Product factory for %s was undefined' %
+                               self.getId())
+        p = container.manage_addProduct[self.product]
+        
+        m = getattr(p, self.factory, None)
+        if m is None:
+            raise ValueError, ('Product factory for %s was invalid' %
+                               self.getId())
+        if getSecurityManager().validate(p, p, self.factory, m):
+            return m
+        raise Unauthorized, ('Cannot create %s' % self.getId())
+
+    def _queryFactoryMethod(self, container, m=None):
         try:
-            p = container.manage_addProduct[self.product]
-            m = getattr(p, self.factory, None)
-            if m is not None:
-                if getSecurityManager().validate(p, p, self.factory, m):
-                    return m
-            return None
-        except: # only raise if allowed
-            if raise_exc:
-                raise
-            return None
+            if self.product and self.factory:
+                p = container.manage_addProduct[self.product]
+                t = getattr(p, self.factory, None)
+                if getSecurityManager().validate(p, p, self.factory, t):
+                    m = t
+        except:
+            LOG('Types Tool', ERROR, '_queryFactoryMethod raised an exception',
+                error=sys.exc_info())
+        return m
 
     security.declarePublic('isConstructionAllowed')
-    def isConstructionAllowed ( self, container, raise_exc=0):
+    def isConstructionAllowed ( self, container ):
         """
-        Does the current user have the permission required in
-        order to construct an instance?
+        a. Does the factory method exist?
+
+        b. Is the factory method usable?
+
+        c. Does the current user have the permission required in
+        order to invoke the factory method?
         """
-        m = self._getFactoryMethod(container, raise_exc)
+        m = self._queryFactoryMethod(container)
         return (m is not None)
 
     security.declarePublic('constructInstance')
@@ -478,10 +494,7 @@
         # Get the factory method, performing a security check
         # in the process.
 
-        m = self._getFactoryMethod(container, raise_exc=1)
-
-        if m is None:
-            raise Unauthorized, ('Cannot create %s' % self.getId())
+        m = self._getFactoryMethod(container)
 
         id = str(id)