- Use ``hash()`` rather than ``id()`` when computing the "phash" of a

  custom route/view predicate in order to allow the custom predicate
  some control over which predicates are "equal".

- Use ``response.headerlist.append`` instead of
  ``response.headers.add`` in
  ``repoze.bfg.request.add_global_response_headers`` in case the
  response is not a WebOb response.

6 files changed, 40 insertions, 22 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index ff761cc02..ccc766637 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,6 +1,9 @@
 Next release
 ============
 
+Bug Fixes
+---------
+
 - Fix a bug in ``repoze.bfg.url.static_url`` URL generation: if two
   resource specifications were used to create two separate static
   views, but they shared a common prefix, it was possible that
@@ -9,6 +12,18 @@ Next release
 - Fix another bug in ``repoze.bfg.static_url`` URL generation: too
   many slashes in generated URL.
 
+Internal
+--------
+
+- Use ``hash()`` rather than ``id()`` when computing the "phash" of a
+  custom route/view predicate in order to allow the custom predicate
+  some control over which predicates are "equal".
+
+- Use ``response.headerlist.append`` instead of
+  ``response.headers.add`` in
+  ``repoze.bfg.request.add_global_response_headers`` in case the
+  response is not a WebOb response.
+
 1.3a11 (2010-09-05)
 ===================
 
diff --git a/repoze/bfg/configuration.py b/repoze/bfg/configuration.py
index c5ae995e5..488761ec4 100644
--- a/repoze/bfg/configuration.py
+++ b/repoze/bfg/configuration.py
@@ -1982,14 +1982,14 @@ def _make_predicates(xhr=None, request_method=None, path_info=None,
             return find_interface(context, containment) is not None
         weights.append(1 << 7)
         predicates.append(containment_predicate)
-        h.update('containment:%r' % id(containment))
+        h.update('containment:%r' % hash(containment))
 
     if request_type is not None:
         def request_type_predicate(context, request):
             return request_type.providedBy(request)
         weights.append(1 << 8)
         predicates.append(request_type_predicate)
-        h.update('request_type:%r' % id(request_type))
+        h.update('request_type:%r' % hash(request_type))
 
     if traverse is not None:
         # ``traverse`` can only be used as a *route* "predicate"; it
@@ -2014,7 +2014,13 @@ def _make_predicates(xhr=None, request_method=None, path_info=None,
     if custom:
         for num, predicate in enumerate(custom):
             predicates.append(predicate)
-            h.update('custom%s:%r' % (num, id(predicate)))
+            # using hash() here rather than id() is intentional: we
+            # want to allow custom predicates that are part of
+            # frameworks to be able to define custom __hash__
+            # functions for custom predicates, so that the hash output
+            # of predicate instances which are "logically the same"
+            # may compare equal.
+            h.update('custom%s:%r' % (num, hash(predicate)))
         weights.append(1 << 10)
 
     score = 0
diff --git a/repoze/bfg/request.py b/repoze/bfg/request.py
index c9422e24d..6318faf5a 100644
--- a/repoze/bfg/request.py
+++ b/repoze/bfg/request.py
@@ -154,7 +154,7 @@ def route_request_iface(name, bases=()):
 def add_global_response_headers(request, headerlist):
     def add_headers(request, response):
         for k, v in headerlist:
-            response.headers.add(k, v)
+            response.headerlist.append((k, v))
     request.add_response_callback(add_headers)
 
 from repoze.bfg.threadlocal import get_current_request as get_request # b/c
diff --git a/repoze/bfg/tests/test_authentication.py b/repoze/bfg/tests/test_authentication.py
index bce80ca20..d020a11a6 100644
--- a/repoze/bfg/tests/test_authentication.py
+++ b/repoze/bfg/tests/test_authentication.py
@@ -419,8 +419,8 @@ class TestAuthTktCookieHelper(unittest.TestCase):
         self.assertEqual(len(request.callbacks), 1)
         response = DummyResponse()
         request.callbacks[0](None, response)
-        self.assertEqual(len(response.headers.added), 3)
-        self.assertEqual(response.headers.added[0][0], 'Set-Cookie')
+        self.assertEqual(len(response.headerlist), 3)
+        self.assertEqual(response.headerlist[0][0], 'Set-Cookie')
 
     def test_remember(self):
         plugin = self._makeOne('secret')
@@ -658,14 +658,7 @@ class DummyAuthTktModule(object):
     class BadTicket(Exception):
         pass
 
-class DummyHeaders:
-    def __init__(self):
-        self.added = []
-
-    def add(self, k, v):
-        self.added.append((k, v))
-
 class DummyResponse:
     def __init__(self):
-        self.headers = DummyHeaders()
+        self.headerlist = []
         
diff --git a/repoze/bfg/tests/test_configuration.py b/repoze/bfg/tests/test_configuration.py
index 232cbd6bb..1ee2c1018 100644
--- a/repoze/bfg/tests/test_configuration.py
+++ b/repoze/bfg/tests/test_configuration.py
@@ -3392,6 +3392,16 @@ class Test__make_predicates(unittest.TestCase):
             )
         self.failUnless(order1 > order2)
 
+    def test_different_custom_predicates_with_same_hash(self):
+        class PredicateWithHash(object):
+            def __hash__(self):
+                return 1
+        a = PredicateWithHash()
+        b = PredicateWithHash()
+        _, _, a_phash = self._callFUT(custom=(a,))
+        _, _, b_phash = self._callFUT(custom=(b,))
+        self.assertEqual(a_phash, b_phash)
+
     def test_traverse_has_remainder_already(self):
         order, predicates, phash = self._callFUT(traverse='/1/:a/:b')
         self.assertEqual(len(predicates), 1)
diff --git a/repoze/bfg/tests/test_request.py b/repoze/bfg/tests/test_request.py
index 8d23e360f..0248e10be 100644
--- a/repoze/bfg/tests/test_request.py
+++ b/repoze/bfg/tests/test_request.py
@@ -206,7 +206,7 @@ class Test_add_global_response_headers(unittest.TestCase):
         self._callFUT(request, [('c', 1)])
         self.assertEqual(len(request.response_callbacks), 1)
         request.response_callbacks[0](None, response)
-        self.assertEqual(response.headers.added,  [('c', 1)] )
+        self.assertEqual(response.headerlist,  [('c', 1)] )
 
 class DummyRequest:
     def __init__(self, environ=None):
@@ -221,14 +221,8 @@ class DummyNewRequestEvent:
     def __init__(self, request):
         self.request = request
         
-class DummyHeaders:
-    def __init__(self):
-        self.added = []
-    def add(self, k, v):
-        self.added.append((k, v))
-
 class DummyResponse:
     def __init__(self):
-        self.headers = DummyHeaders()
+        self.headerlist = []