Remove MappedCollection converter; deprecate @converter

Removed the collection converter used by the :class:`.MappedCollection`
class. This converter was used only to assert that the incoming dictionary
keys matched that of their corresponding objects, and only during a bulk set
operation.  The converter can interfere with a custom validator or
:meth:`.AttributeEvents.bulk_replace` listener that wants to convert
incoming values further.  The ``TypeError`` which would be raised by this
converter when an incoming key didn't match the value is removed; incoming
values during a bulk assignment will be keyed to their value-generated key,
and not the key that's explicitly present in the dictionary.

Overall, @converter is superseded by the
:meth:`.AttributeEvents.bulk_replace` event handler added as part of
:ticket:`3896`.

Fixes: #3604
Change-Id: Id0f7bd2cec938f5975eb2ab94df9ba5754dd43c3

2 files changed, 56 insertions, 22 deletions

diff --git a/test/orm/test_collection.py b/test/orm/test_collection.py
index f0f4de8a9..58c870645 100644
--- a/test/orm/test_collection.py
+++ b/test/orm/test_collection.py
@@ -1,5 +1,4 @@
 from sqlalchemy.testing import eq_, ne_
-import sys
 from operator import and_
 
 import sqlalchemy.orm.collections as collections
@@ -13,6 +12,7 @@ from sqlalchemy.orm import create_session, mapper, relationship, \
     attributes, instrumentation
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import assert_raises, assert_raises_message
+from sqlalchemy import testing
 
 
 class Canary(sa.orm.interfaces.AttributeExtension):
@@ -1093,27 +1093,14 @@ class CollectionsTest(fixtures.ORMTest):
         # MappedCollection but is not present in basic, @converter-less
         # dict collections.
         e3 = creator()
-        if isinstance(obj.attr, collections.MappedCollection):
-            real_dict = dict(badkey=e3)
-            try:
-                obj.attr = real_dict
-                self.assert_(False)
-            except TypeError:
-                pass
-            self.assert_(obj.attr is not real_dict)
-            self.assert_('badkey' not in obj.attr)
-            eq_(set(collections.collection_adapter(obj.attr)),
-                set([e2]))
-            self.assert_(e3 not in canary.added)
-        else:
-            real_dict = dict(keyignored1=e3)
-            obj.attr = real_dict
-            self.assert_(obj.attr is not real_dict)
-            self.assert_('keyignored1' not in obj.attr)
-            eq_(set(collections.collection_adapter(obj.attr)),
-                set([e3]))
-            self.assert_(e2 in canary.removed)
-            self.assert_(e3 in canary.added)
+        real_dict = dict(keyignored1=e3)
+        obj.attr = real_dict
+        self.assert_(obj.attr is not real_dict)
+        self.assert_('keyignored1' not in obj.attr)
+        eq_(set(collections.collection_adapter(obj.attr)),
+            set([e3]))
+        self.assert_(e2 in canary.removed)
+        self.assert_(e3 in canary.added)
 
         obj.attr = typecallable()
         eq_(list(collections.collection_adapter(obj.attr)), [])
@@ -1182,6 +1169,7 @@ class CollectionsTest(fixtures.ORMTest):
         self._test_dict_bulk(MyOrdered)
         self.assert_(getattr(MyOrdered, '_sa_instrumented') == id(MyOrdered))
 
+    @testing.uses_deprecated(r".*Use the bulk_replace event handler")
     def test_dict_subclass4(self):
         # tests #2654
         class MyDict(collections.MappedCollection):
@@ -2248,6 +2236,7 @@ class InstrumentationTest(fixtures.ORMTest):
 
         collections._instrument_class(Touchy)
 
+    @testing.uses_deprecated(r".*Use the bulk_replace event handler")
     def test_name_setup(self):
 
         class Base(object):
diff --git a/test/orm/test_validators.py b/test/orm/test_validators.py
index 7ceab44dc..cbbf9f7a8 100644
--- a/test/orm/test_validators.py
+++ b/test/orm/test_validators.py
@@ -2,6 +2,7 @@ from test.orm import _fixtures
 from sqlalchemy.testing import fixtures, assert_raises, eq_, ne_, \
     assert_raises_message
 from sqlalchemy.orm import mapper, Session, validates, relationship
+from sqlalchemy.orm import collections
 from sqlalchemy.testing.mock import Mock, call
 from sqlalchemy import exc
 
@@ -184,6 +185,50 @@ class ValidatorTest(_fixtures.FixtureTest):
             [Address(email_address="e3"), Address(email_address="e4")]
         )
 
+    def test_validator_bulk_dict_set(self):
+        users, addresses, Address = (self.tables.users,
+                                     self.tables.addresses,
+                                     self.classes.Address)
+
+        class User(fixtures.ComparableEntity):
+
+            @validates('addresses', include_removes=True)
+            def validate_address(self, key, item, remove):
+                if not remove:
+                    assert isinstance(item, str)
+                else:
+                    assert isinstance(item, Address)
+                item = Address(email_address=item)
+                return item
+
+        mapper(User, users, properties={
+            'addresses': relationship(
+                Address,
+                collection_class=collections.attribute_mapped_collection(
+                    "email_address")
+            )
+        })
+        mapper(Address, addresses)
+
+        u1 = User()
+        u1.addresses["e1"] = "e1"
+        u1.addresses["e2"] = "e2"
+        eq_(
+            u1.addresses,
+            {
+                "e1": Address(email_address="e1"),
+                "e2": Address(email_address="e2")
+            }
+        )
+        u1.addresses = {"e3": "e3", "e4": "e4"}
+        eq_(
+            u1.addresses,
+            {
+                "e3": Address(email_address="e3"),
+                "e4": Address(email_address="e4")
+            }
+        )
+
     def test_validator_multi_warning(self):
         users = self.tables.users