From 93b6aeb455b7b992ac41cb0f8c872e851cf8f69d Mon Sep 17 00:00:00 2001
From: Leonardo Rochael Almeida <leonardo@nexedi.com>
Date: Fri, 16 Apr 2010 06:00:35 +0000
Subject: [PATCH] remove __getitem__ (dictionary) access of ZSQLCatalog

git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@34607 20353a03-c40f-0410-a6d1-a30d3c3de9de
---
 product/ZSQLCatalog/SQLCatalog.py            | 10 +++++++++-
 product/ZSQLCatalog/tests/testZSQLCatalog.py | 11 +++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/product/ZSQLCatalog/SQLCatalog.py b/product/ZSQLCatalog/SQLCatalog.py
index b1de5de276..274a15ce2e 100644
--- a/product/ZSQLCatalog/SQLCatalog.py
+++ b/product/ZSQLCatalog/SQLCatalog.py
@@ -856,11 +856,19 @@ class Catalog(Folder,
       raise
     self._last_clear_reserved_time += 1
 
-  def __getitem__(self, uid):
+  def getRecordForUid(self, uid):
     """
     Get an object by UID
     Note: brain is defined in Z SQL Method object
     """
+    # this method used to be __getitem__(self, uid) but was found to hurt more
+    # than it helped: It would be inadvertently called by 
+    # (un)restrictedTraverse and if there was any error in rendering the SQL
+    # expression or contacting the database, an error different from KeyError
+    # would be raised, causing confusion.
+    # It could also have a performance impact for traversals to objects in
+    # the acquisition context on Zope 2.12 even when it didn't raise a weird
+    # error.
     method = getattr(self,  self.sql_getitem_by_uid)
     search_result = method(uid = uid)
     if len(search_result) > 0:
diff --git a/product/ZSQLCatalog/tests/testZSQLCatalog.py b/product/ZSQLCatalog/tests/testZSQLCatalog.py
index 848960d467..3b72c4b266 100644
--- a/product/ZSQLCatalog/tests/testZSQLCatalog.py
+++ b/product/ZSQLCatalog/tests/testZSQLCatalog.py
@@ -89,6 +89,17 @@ class TestSQLCatalog(unittest.TestCase):
     self.assertFalse(
         self._catalog.isPortalTypeSelected('not_exists', 'Selected'))
 
+  def test_getRecordByUid(self):
+    class MyError(RuntimeError):
+      pass
+    # test that our method actually gets called while looking records up by
+    # uid by raising our own exception
+    self._catalog.sql_getitem_by_uid = 'z_dummy_lookup_method'
+    def z_dummy_lookup_method(uid):
+      raise MyError('foo')
+    self._catalog.z_dummy_lookup_method = z_dummy_lookup_method
+    self.assertRaises(MyError, self._catalog.getRecordForUid, 1)
+
 def test_suite():
   suite = unittest.TestSuite()
   suite.addTest(unittest.makeSuite(TestSQLCatalog))
-- 
2.30.9