refactoring of objectstore to handle cleaning up after itself with less

instruction from properties/mapper
objectstore gets an assertion for appending a deleted item
mapper has fix for inheritance
mapper selectone() etc. set "limit=1"

3 files changed, 45 insertions, 51 deletions

diff --git a/lib/sqlalchemy/mapping/mapper.py b/lib/sqlalchemy/mapping/mapper.py
index d33409454..7e11f5ebe 100644
--- a/lib/sqlalchemy/mapping/mapper.py
+++ b/lib/sqlalchemy/mapping/mapper.py
@@ -73,7 +73,7 @@ class Mapper(object):
         if inherits is not None:
             self.primarytable = inherits.primarytable
             # inherit_condition is optional since the join can figure it out
-            self.table = sql.join(table, inherits.table, inherit_condition)
+            self.table = sql.join(inherits.table, table, inherit_condition)
         else:
             self.primarytable = self.table
         
@@ -83,7 +83,6 @@ class Mapper(object):
         self.table.accept_visitor(tf)
         self.tables = tf.tables
 
-
         # determine primary key columns, either passed in, or get them from our set of tables
         self.pks_by_table = {}
         if primary_key is not None:
@@ -338,7 +337,7 @@ class Mapper(object):
         
         e.g.   u = usermapper.get_by(user_name = 'fred')
         """
-        x = self.select_by(*args, **params)
+        x = self.select_whereclause(self._by_clause(*args, **params), limit=1)
         if len(x):
             return x[0]
         else:
@@ -416,6 +415,7 @@ class Mapper(object):
     def selectone(self, *args, **params):
         """works like select(), but only returns the first result by itself, or None if no 
         objects returned."""
+        params['limit'] = 1
         ret = self.select(*args, **params)
         if len(ret):
             return ret[0]
@@ -437,14 +437,14 @@ class Mapper(object):
         else:
             return self.select_whereclause(arg, **kwargs)
 
-    def select_whereclause(self, whereclause = None, params=None, **kwargs):
+    def select_whereclause(self, whereclause=None, params=None, **kwargs):
         statement = self._compile(whereclause, **kwargs)
         if params is not None:
             return self.select_statement(statement, **params)
         else:
             return self.select_statement(statement)
 
-    def count(self, whereclause = None, params=None, **kwargs):
+    def count(self, whereclause=None, params=None, **kwargs):
         s = self.table.count(whereclause)
         if params is not None:
             return s.scalar(**params)
@@ -484,6 +484,7 @@ class Mapper(object):
         list."""
           
         for table in self.tables:
+            #print "SAVE_OBJ table ", table.name
             # looping through our set of tables, which are all "real" tables, as opposed
             # to our main table which might be a select statement or something non-writeable
             
@@ -553,7 +554,6 @@ class Mapper(object):
                         update.append(params)
                 else:
                     insert.append((obj, params))
-                uow.register_saved_object(obj)
             if len(update):
                 clause = sql.and_()
                 for col in self.pks_by_table[table]:
@@ -595,7 +595,6 @@ class Mapper(object):
                     delete.append(params)
                 for col in self.pks_by_table[table]:
                     params[col.key] = self._getattrbycolumn(obj, col)
-                uow.register_deleted_object(obj)
                 self.extension.before_delete(self, obj)
             if len(delete):
                 clause = sql.and_()
diff --git a/lib/sqlalchemy/mapping/objectstore.py b/lib/sqlalchemy/mapping/objectstore.py
index 66207329c..5f48b4d60 100644
--- a/lib/sqlalchemy/mapping/objectstore.py
+++ b/lib/sqlalchemy/mapping/objectstore.py
@@ -104,7 +104,7 @@ def import_instance(instance):
         return None
     key = getattr(instance, '_instance_key', None)
     mapper = object_mapper(instance)
-    key = (key[0], repr(mapper.table), key[2])
+    key = (key[0], mapper.table.hash_key(), key[2])
     u = uow()
     if key is not None:
         if u.identity_map.has_key(key):
@@ -121,6 +121,8 @@ class UOWListElement(attributes.ListElement):
         attributes.ListElement.__init__(self, obj, key, data=data, **kwargs)
         self.deleteremoved = deleteremoved
     def list_value_changed(self, obj, key, item, listval, isdelete):
+        if not isdelete and uow().deleted.contains(item):
+            raise "re-inserting a deleted value into a list"
         uow().modified_lists.append(self)
         if self.deleteremoved and isdelete:
             uow().register_deleted(item)
@@ -159,7 +161,7 @@ class UnitOfWork(object):
         self.parent = parent
 
     def get(self, class_, *id):
-        return sqlalchemy.mapper.object_mapper(class_).get(*id)
+        return object_mapper(class_).get(*id)
 
     def _get(self, key):
         return self.identity_map[key]
@@ -209,6 +211,9 @@ class UnitOfWork(object):
             del self.new[obj]
         except KeyError:
             pass
+        if not hasattr(obj, '_instance_key'):
+            mapper = object_mapper(obj)
+            obj._instance_key = mapper.instance_key(obj)
         self._put(obj._instance_key, obj)
         self.attributes.commit(obj)
         
@@ -263,11 +268,13 @@ class UnitOfWork(object):
             if self.deleted.contains(obj):
                 continue
             commit_context.register_object(obj, listonly = True)
-            commit_context.register_saved_list(item)
-            for o in item.added_items() + item.deleted_items():
-                if self.deleted.contains(o):
-                    continue
-                commit_context.register_object(o, listonly=True)
+            commit_context.register_saved_history(item)
+
+#            for o in item.added_items() + item.deleted_items():
+#                if self.deleted.contains(o):
+#                    continue
+#                commit_context.register_object(o, listonly=True)
+                     
         for obj in self.deleted:
             if objset is not None and not objset.contains(obj):
                 continue
@@ -320,10 +327,7 @@ class UOWTransaction(object):
         self.mappers = util.HashSet()
         self.dependencies = {}
         self.tasks = {}
-        self.saved_objects = util.HashSet()
-        self.saved_lists = util.HashSet()
-        self.deleted_objects = util.HashSet()
-        self.deleted_lists = util.HashSet()
+        self.saved_histories = util.HashSet()
 
     def register_object(self, obj, isdelete = False, listonly = False, **kwargs):
         """adds an object to this UOWTransaction to be updated in the database.
@@ -378,18 +382,9 @@ class UOWTransaction(object):
         targettask = self.get_task_by_mapper(mapperfrom)
         task.dependencies.append(UOWDependencyProcessor(processor, targettask, isdeletefrom))
 
-    def register_saved_object(self, obj):
-        self.saved_objects.append(obj)
-
-    def register_saved_list(self, listobj):
-        self.saved_lists.append(listobj)
+    def register_saved_history(self, listobj):
+        self.saved_histories.append(listobj)
 
-    def register_deleted_list(self, listobj):
-        self.deleted_lists.append(listobj)
-        
-    def register_deleted_object(self, obj):
-        self.deleted_objects.append(obj)
-        
     def execute(self, echo=False):
         for task in self.tasks.values():
             task.mapper.register_dependencies(self)
@@ -399,32 +394,32 @@ class UOWTransaction(object):
             print "Task dump:\n" + head.dump()
         if head is not None:
             head.execute(self)
+        if LOG or echo:
+            print "\nAfter Execute:\n" + head.dump()
             
     def post_exec(self):
         """after an execute/commit is completed, all of the objects and lists that have
         been committed are updated in the parent UnitOfWork object to mark them as clean."""
-        for obj in self.saved_objects:
-            mapper = object_mapper(obj)
-            obj._instance_key = mapper.instance_key(obj)
-            self.uow.register_clean(obj)
-
-        for obj in self.saved_lists:
-            try:
-                obj.commit()
-                del self.uow.modified_lists[obj]
-            except KeyError:
-                pass
-
-        for obj in self.deleted_objects:
-            self.uow._remove_deleted(obj)
         
-        for obj in self.deleted_lists:
+        for task in self.tasks.values():
+            for elem in task.objects.values():
+                if elem.isdelete:
+                    self.uow._remove_deleted(elem.obj)
+                else:
+                    self.uow.register_clean(elem.obj)
+                
+        for obj in self.saved_histories:
             try:
                 obj.commit()
                 del self.uow.modified_lists[obj]
             except KeyError:
                 pass
 
+	# this assertion only applies to a full commit(), not a
+	# partial one
+        #if len(self.uow.new) > 0 or len(self.uow.dirty) >0 or len(self.uow.modified_lists) > 0:
+        #    raise "assertion failed"
+
     def _sort_dependencies(self):
         """creates a hierarchical tree of dependent tasks.  the root node is returned.
         when the root node is executed, it also executes its child tasks recursively."""
@@ -530,7 +525,10 @@ class UOWTask(object):
             rec.isdelete = True
 
     def delete(self, obj):
-        del self.objects[obj]
+        try:
+            del self.objects[obj]
+        except KeyError:
+            pass
         
     def execute(self, trans):
         """executes this UOWTask.  saves objects to be saved, processes all dependencies
diff --git a/lib/sqlalchemy/mapping/properties.py b/lib/sqlalchemy/mapping/properties.py
index eb575d1dc..f1278b9e7 100644
--- a/lib/sqlalchemy/mapping/properties.py
+++ b/lib/sqlalchemy/mapping/properties.py
@@ -421,7 +421,9 @@ class PropertyLoader(MapperProperty):
         #print self.mapper.table.name + " " + self.key + " " + repr(len(deplist)) + " process_dep isdelete " + repr(delete) + " direction " + repr(self.direction)
 
         def getlist(obj, passive=True):
-            return self.get_object_dependencies(obj, uowcommit, passive)
+            l = self.get_object_dependencies(obj, uowcommit, passive)
+            uowcommit.register_saved_history(l)
+            return l
 
         # plugin point
         
@@ -435,7 +437,6 @@ class PropertyLoader(MapperProperty):
                         associationrow = {}
                         self._synchronize(obj, child, associationrow, False)
                         secondary_delete.append(associationrow)
-                    uowcommit.register_deleted_list(childlist)
             else:
                 for obj in deplist:
                     childlist = getlist(obj)
@@ -448,7 +449,6 @@ class PropertyLoader(MapperProperty):
                         associationrow = {}
                         self._synchronize(obj, child, associationrow, False)
                         secondary_delete.append(associationrow)
-                    uowcommit.register_saved_list(childlist)
             if len(secondary_delete):
                 # TODO: precompile the delete/insert queries and store them as instance variables
                 # on the PropertyLoader
@@ -473,13 +473,11 @@ class PropertyLoader(MapperProperty):
                 for child in childlist.deleted_items() + childlist.unchanged_items():
                     self._synchronize(obj, child, None, True)
                     uowcommit.register_object(child)
-                uowcommit.register_deleted_list(childlist)
         elif self.association is not None:
             # manage association objects.
             for obj in deplist:
                 childlist = getlist(obj, passive=True)
                 if childlist is None: continue
-                uowcommit.register_saved_list(childlist)
                 
                 #print "DIRECTION", self.direction
                 d = {}
@@ -517,7 +515,6 @@ class PropertyLoader(MapperProperty):
                     uowcommit.register_object(obj)
                 childlist = getlist(obj, passive=True)
                 if childlist is None: continue
-                uowcommit.register_saved_list(childlist)
                 for child in childlist.added_items():
                     self._synchronize(obj, child, None, False)
                     if self.direction == PropertyLoader.LEFT: