Ticket #7392: 0002-db-distinguish-between-read-and-write-access.patch

trac/admin/console.py

@@ -167 +167 @@
     def db_query(self, sql, cursor=None, params=None):
         if not cursor:
             cnx = self.db_open()
-            cursor = cnx.cursor()
+            cursor = cnx.read.cursor()
         if params:
             cursor.execute(sql, params)
         else:
@@ -178 +178 @@
     def db_update(self, sql, cursor=None, params=None):
         if not cursor:
             cnx = self.db_open()
-            cursor = cnx.cursor()
+            cursor = cnx.write.cursor()
         else:
             cnx = None
         if params:
@@ -186 +186 @@
         else:
             cursor.execute(sql)
         if cnx:
-            cnx.commit()
+            cnx.write.commit()
 
     ##
     ## Utility methods
@@ -563 +563 @@
             # Add a few default wiki pages
             print ' Installing default wiki pages'
             cnx = self.__env.get_db_cnx()
-            cursor = cnx.cursor()
+            cursor = cnx.write.cursor()
             pages_dir = pkg_resources.resource_filename('trac.wiki', 
                                                         'default-pages') 
             self._do_wiki_load(pages_dir, cursor)
-            cnx.commit()
+            cnx.write.commit()
 
             if repository_dir:
                 try:
@@ -635 +635 @@
         from trac.versioncontrol.cache import CACHE_METADATA_KEYS
         print 'Resyncing repository history... '
         cnx = self.db_open()
-        cursor = cnx.cursor()
+        cursor = cnx.write.cursor()
         cursor.execute("DELETE FROM revision")
         cursor.execute("DELETE FROM node_change")
         cursor.executemany("DELETE FROM system WHERE name=%s",
                            [(k,) for k in CACHE_METADATA_KEYS])
         cursor.executemany("INSERT INTO system (name, value) VALUES (%s, %s)",
                            [(k, '') for k in CACHE_METADATA_KEYS])
-        cnx.commit()
+        cnx.write.commit()
         repos = env.get_repository().sync(self._resync_feedback)
+        cursor = cnx.read.cursor()
         cursor.execute("SELECT count(rev) FROM revision")
         for cnt, in cursor:
             print cnt, 'revisions cached.',
@@ -950 +951 @@
         sql = ("INSERT INTO enum(value,type,name) "
                " SELECT 1+COALESCE(max(%(cast)s),0),'%(type)s','%(name)s'"
                "   FROM enum WHERE type='%(type)s'" 
-               % {'type':type, 'name':name, 'cast': cnx.cast('value', 'int')})
-        cursor = cnx.cursor()
+               % {'type':type, 'name':name, 'cast': cnx.write.cast('value', 'int')})
+        cursor = cnx.write.cursor()
         self.db_update(sql, cursor)
-        cnx.commit()
+        cnx.write.commit()
 
     def _do_enum_change(self, type, name, newname):
         enum_cls = self._enum_map[type]
@@ -1151 +1152 @@
 
         # Bogus statement to lock the database while copying files
         cnx = self.db_open()
-        cursor = cnx.cursor()
+        cursor = cnx.write.cursor()
         cursor.execute("UPDATE system SET name=NULL WHERE name IS NULL")
 
         try:
@@ -1167 +1168 @@
             copytree(self.__env.path, dest, symlinks=1, skip=skip)
         finally:
             # Unlock database
-            cnx.rollback()
+            cnx.write.rollback()
 
         print 'Hotcopy done.'
 

trac/admin/tests/console.py

@@ -27 +27 @@
 from trac.config import Configuration
 from trac.env import Environment
 from trac.admin import console
-from trac.test import InMemoryDatabase
+from trac.test import InMemoryDatabase, DatabasePair
 from trac.util.datefmt import get_date_format_hint
 
 STRIP_TRAILING_SPACE = re.compile(r'( +)$', re.MULTILINE)
@@ -62 +62 @@
     def get_db_cnx(self):
         if not hasattr(self, '_db'):
             self._db = InMemoryDatabase()
-        return self._db
+        return DatabasePair(self._db, self._db)
 
     def create(self, db_str=None):
         pass

trac/attachment.py

@@ -132 +132 @@
     def _fetch(self, filename, db=None):
         if not db:
             db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.read.cursor()
         cursor.execute("SELECT filename,description,size,time,author,ipnr "
                        "FROM attachment WHERE type=%s AND id=%s "
                        "AND filename=%s ORDER BY time",
@@ -172 +172 @@
         else:
             handle_ta = False
 
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.execute("DELETE FROM attachment WHERE type=%s AND id=%s "
                        "AND filename=%s", (self.parent_realm, self.parent_id,
                        self.filename))
@@ -183 +183 @@
                 self.env.log.error('Failed to delete attachment file %s',
                                    self.path, exc_info=True)
                 if handle_ta:
-                    db.rollback()
+                    db.write.rollback()
                 raise TracError(_('Could not delete attachment'))
 
         self.env.log.info('Attachment removed: %s' % self.title)
         if handle_ta:
-            db.commit()
+            db.write.commit()
 
         for listener in AttachmentModule(self.env).change_listeners:
             listener.attachment_deleted(self)
@@ -224 +224 @@
             basename = os.path.basename(path).encode('ascii')
             filename = unicode_unquote(basename)
 
-            cursor = db.cursor()
+            cursor = db.write.cursor()
             cursor.execute("INSERT INTO attachment "
                            "VALUES (%s,%s,%s,%s,%s,%s,%s,%s)",
                            (self.parent_realm, self.parent_id, filename,
@@ -237 +237 @@
                               self.author)
 
             if handle_ta:
-                db.commit()
+                db.write.commit()
 
             for listener in AttachmentModule(self.env).change_listeners:
                 listener.attachment_added(self)
@@ -248 +248 @@
     def select(cls, env, parent_realm, parent_id, db=None):
         if not db:
             db = env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.read.cursor()
         cursor.execute("SELECT filename,description,size,time,author,ipnr "
                        "FROM attachment WHERE type=%s AND id=%s ORDER BY time",
                        (parent_realm, unicode(parent_id)))
@@ -441 +441 @@
         """
         # Traverse attachment directory
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.read.cursor()
         cursor.execute("SELECT type, id, filename, time, description, author "
                        "  FROM attachment "
                        "  WHERE time > %s AND time < %s "

trac/db_default.py

@@ -180 +180 @@
   FROM ticket t
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   WHERE status <> 'closed'
-  ORDER BY """ + db.cast('p.value', 'int') + """, milestone, t.type, time
+  ORDER BY """ + db.write.cast('p.value', 'int') + """, milestone, t.type, time
 """),
 #----------------------------------------------------------------------------
  ('Active Tickets by Version',
@@ -202 +202 @@
   FROM ticket t
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   WHERE status <> 'closed'
-  ORDER BY (version IS NULL),version, """ + db.cast('p.value', 'int') +
+  ORDER BY (version IS NULL),version, """ + db.write.cast('p.value', 'int') +
   """, t.type, time
 """),
 #----------------------------------------------------------------------------
@@ -226 +226 @@
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   WHERE status <> 'closed' 
   ORDER BY (milestone IS NULL),milestone, %s, t.type, time
-""" % (db.concat("'Milestone '", 'milestone'), db.cast('p.value', 'int'))),
+""" % (db.write.concat("'Milestone '", 'milestone'), db.write.cast('p.value', 'int'))),
 #----------------------------------------------------------------------------
 ('Accepted, Active Tickets by Owner',
 """
@@ -242 +242 @@
   FROM ticket t
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   WHERE status = 'accepted'
-  ORDER BY owner, """ + db.cast('p.value', 'int') + """, t.type, time
+  ORDER BY owner, """ + db.write.cast('p.value', 'int') + """, t.type, time
 """),
 #----------------------------------------------------------------------------
 ('Accepted, Active Tickets by Owner (Full Description)',
@@ -259 +259 @@
   FROM ticket t
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   WHERE status = 'accepted'
-  ORDER BY owner, """ + db.cast('p.value', 'int') + """, t.type, time
+  ORDER BY owner, """ + db.write.cast('p.value', 'int') + """, t.type, time
 """),
 #----------------------------------------------------------------------------
 ('All Tickets By Milestone  (Including closed)',
@@ -282 +282 @@
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   ORDER BY (milestone IS NULL), milestone DESC, (status = 'closed'), 
         (CASE status WHEN 'closed' THEN changetime ELSE (-1) * %s END) DESC
-""" % db.cast('p.value', 'int')),
+""" % db.write.cast('p.value', 'int')),
 #----------------------------------------------------------------------------
 ('My Tickets',
 """
@@ -300 +300 @@
   FROM ticket t
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   WHERE t.status <> 'closed' AND owner = $USER
-  ORDER BY (status = 'accepted') DESC, """ + db.cast('p.value', 'int') + 
+  ORDER BY (status = 'accepted') DESC, """ + db.write.cast('p.value', 'int') +
   """, milestone, t.type, time
 """),
 #----------------------------------------------------------------------------
@@ -323 +323 @@
   FROM ticket t
   LEFT JOIN enum p ON p.name = t.priority AND p.type = 'priority'
   WHERE status <> 'closed' 
-  ORDER BY (owner = $USER) DESC, """ + db.cast('p.value', 'int') + 
+  ORDER BY (owner = $USER) DESC, """ + db.write.cast('p.value', 'int') +
   """, milestone, t.type, time
 """))
 

trac/env.py

@@ -322 +322 @@
         """
         if not db:
             db = self.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.read.cursor()
         cursor.execute("SELECT value FROM system "
                        "WHERE name='%sdatabase_version'" %
                        (initial and 'initial_' or ''))
@@ -380 +380 @@
         """
         if not cnx:
             cnx = self.get_db_cnx()
-        cursor = cnx.cursor()
+        cursor = cnx.read.cursor()
         cursor.execute("SELECT DISTINCT s.sid, n.value, e.value "
                        "FROM session AS s "
                        " LEFT JOIN session_attribute AS n ON (n.sid=s.sid "
@@ -440 +440 @@
             self.backup(backup_dest)
         for participant in upgraders:
             participant.upgrade_environment(db)
-        db.commit()
+        db.write.commit()
 
         # Database schema may have changed, so close all connections
         self.shutdown()
@@ -473 +473 @@
     def environment_created(self):
         """Insert default data into the database."""
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         for table, cols, vals in db_default.get_data(db):
             cursor.executemany("INSERT INTO %s (%s) VALUES (%s)" % (table,
                                ','.join(cols), ','.join(['%s' for c in cols])),
                                vals)
-        db.commit()
+        db.write.commit()
         self._update_sample_config()
 
     def environment_needs_upgrade(self, db):
@@ -490 +490 @@
         return True
 
     def upgrade_environment(self, db):
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         dbver = self.env.get_version()
         for i in range(dbver + 1, db_default.db_version + 1):
             name  = 'db%i' % i

trac/perm.py

@@ -171 +171 @@
 
         actions = set([])
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.read.cursor()
         cursor.execute("SELECT username,action FROM permission")
         rows = cursor.fetchall()
         while True:
@@ -198 +198 @@
         # The optimized loop we had before didn't.  This is very inefficient,
         # but it works.
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.read.cursor()
         result = set()
         users = set([u[0] for u in self.env.get_known_users()])
         for user in users:
@@ -214 +214 @@
         The permissions are returned as a list of (subject, action)
         formatted tuples."""
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.read.cursor()
         cursor.execute("SELECT username,action FROM permission")
         return [(row[0], row[1]) for row in cursor]
 
     def grant_permission(self, username, action):
         """Grants a user the permission to perform the specified action."""
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.execute("INSERT INTO permission VALUES (%s, %s)",
                        (username, action))
         self.log.info('Granted permission for %s to %s' % (action, username))
-        db.commit()
+        db.write.commit()
 
     def revoke_permission(self, username, action):
         """Revokes a users' permission to perform the specified action."""
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.execute("DELETE FROM permission WHERE username=%s AND action=%s",
                        (username, action))
         self.log.info('Revoked permission for %s to %s' % (action, username))
-        db.commit()
+        db.write.commit()
 
 
 class DefaultPermissionGroupProvider(Component):

trac/search/api.py

@@ -47 +47 @@
     """
     assert columns and terms
 
-    likes = ['%s %s' % (i, db.like()) for i in columns]
+    likes = ['%s %s' % (i, db.read.like()) for i in columns]
     c = ' OR '.join(likes)
     sql = '(' + ') AND ('.join([c] * len(terms)) + ')'
     args = []
     for t in terms:
-        args.extend(['%' + db.like_escape(t) + '%'] * len(columns))
+        args.extend(['%' + db.read.like_escape(t) + '%'] * len(columns))
     return sql, tuple(args)
 
 def shorten_result(text='', keywords=[], maxlen=240, fuzz=60):

trac/test.py

@@ -36 +36 @@
 from trac.util import translation
 
 
+class DatabasePair(object):
+    """
+    Holds two database connections, one for reading and one for writing.
+    """
+    def __init__(self, db, dbwr):
+        self.read = db
+        self.write = dbwr
+
 def Mock(bases=(), *initargs, **kw):
     """
     Simple factory for dummy classes that can be used as replacement for the 
@@ -172 +180 @@
         ComponentManager.__init__(self)
         Component.__init__(self)
         self.enabled_components = enable or ['trac.*']
-        self.db = InMemoryDatabase()
+        ndb = InMemoryDatabase()
+        self.db = DatabasePair(ndb, ndb)
         self.systeminfo = [('Python', sys.version)]
 
         import trac
@@ -195 +204 @@
 
         from trac import db_default
         if default_data:
-            cursor = self.db.cursor()
+            cursor = self.db.write.cursor()
             for table, cols, vals in db_default.get_data(self.db):
                 cursor.executemany("INSERT INTO %s (%s) VALUES (%s)"
                                    % (table, ','.join(cols),
                                       ','.join(['%s' for c in cols])),
                                    vals)
-            self.db.commit()
+            self.db.write.commit()
             
         self.known_users = []
         translation.activate(Locale and Locale('en', 'US'))

trac/tests/env.py

@@ -26 +26 @@
 
     def test_get_known_users(self):
         """Testing env.get_known_users"""
-        cursor = self.db.cursor()
+        cursor = self.db.write.cursor()
         cursor.executemany("INSERT INTO session VALUES (%s,%s,0)",
                            [('123', 0),('tom', 1), ('joe', 1), ('jane', 1)])
         cursor.executemany("INSERT INTO session_attribute VALUES (%s,%s,%s,%s)",

trac/tests/perm.py

@@ -15 +15 @@
 
     def test_simple_actions(self):
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.executemany("INSERT INTO permission VALUES (%s,%s)", [
                            ('john', 'WIKI_MODIFY'), ('john', 'REPORT_ADMIN'),
                            ('kate', 'TICKET_CREATE')])
@@ -25 +25 @@
 
     def test_simple_group(self):
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.executemany("INSERT INTO permission VALUES (%s,%s)", [
                            ('dev', 'WIKI_MODIFY'), ('dev', 'REPORT_ADMIN'),
                            ('john', 'dev')])
@@ -34 +34 @@
 
     def test_nested_groups(self):
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.executemany("INSERT INTO permission VALUES (%s,%s)", [
                            ('dev', 'WIKI_MODIFY'), ('dev', 'REPORT_ADMIN'),
                            ('admin', 'dev'), ('john', 'admin')])
@@ -43 +43 @@
 
     def test_mixed_case_group(self):
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.executemany("INSERT INTO permission VALUES (%s,%s)", [
                            ('Dev', 'WIKI_MODIFY'), ('Dev', 'REPORT_ADMIN'),
                            ('Admin', 'Dev'), ('john', 'Admin')])
@@ -52 +52 @@
 
     def test_builtin_groups(self):
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.executemany("INSERT INTO permission VALUES (%s,%s)", [
                            ('authenticated', 'WIKI_MODIFY'),
                            ('authenticated', 'REPORT_ADMIN'),
@@ -64 +64 @@
 
     def test_get_all_permissions(self):
         db = self.env.get_db_cnx()
-        cursor = db.cursor()
+        cursor = db.write.cursor()
         cursor.executemany("INSERT INTO permission VALUES (%s,%s)", [
                            ('dev', 'WIKI_MODIFY'), ('dev', 'REPORT_ADMIN'),
                            ('john', 'dev')])

trac/ticket/admin.py

@@ -98 +98 @@
                     for name in sel:
                         comp = model.Component(self.env, name, db=db)
                         comp.delete(db=db)
-                    db.commit()
+                    db.write.commit()
                     req.redirect(req.href.admin(cat, page))
 
                 # Set default component
@@ -196 +196 @@
                     for name in sel:
                         mil = model.Milestone(self.env, name, db=db)
                         mil.delete(db=db)
-                    db.commit()
+                    db.write.commit()
                     req.redirect(req.href.admin(cat, page))
 
                 # Set default milestone
@@ -278 +278 @@
                     for name in sel:
                         ver = model.Version(self.env, name, db=db)
                         ver.delete(db=db)
-                    db.commit()
+                    db.write.commit()
                     req.redirect(req.href.admin(cat, page))
 
                 # Set default version
@@ -358 +358 @@
                     for name in sel:
                         enum = self._enum_cls(self.env, name, db=db)
                         enum.delete(db=db)
-                    db.commit()
+                    db.write.commit()
                     req.redirect(req.href.admin(cat, page))
 
                 # Appy changes
@@ -387 +387 @@
                         if new_value != enum.value:
                             enum.value = new_value
                             enum.update(db=db)
-                    db.commit()
+                    db.write.commit()
 
                     req.redirect(req.href.admin(cat, page))
 

trac/ticket/api.py

@@ -321 +321 @@
                 if Ticket.id_is_valid(num):
                     # TODO: watch #6436 and when done, attempt to retrieve 
                     #       ticket directly (try: Ticket(self.env, num) ...)
-                    cursor = formatter.db.cursor() 
+                    cursor = formatter.db.read.cursor()
                     cursor.execute("SELECT type,summary,status,resolution "
                                    "FROM ticket WHERE id=%s", (str(num),)) 
                     for type, summary, status, resolution in cursor:

trac/ticket/model.py

@@ -50 +50 @@
             self.id = self.time_created = self.time_changed = None
         self._old = {}
 
-    def _get_db(self, db):
-        return db or self.env.get_db_cnx()
-
-    def _get_db_for_write(self, db):
-        if db:
-            return (db, False)
-        else:
-            return (self.env.get_db_cnx(), True)
-
     exists = property(fget=lambda self: self.id is not None)
 
     def _init_defaults(self, db=None):
@@ -86 +77 @@
     def _fetch_ticket(self, tkt_id, db=None):
         row = None
         if self.id_is_valid(tkt_id):
-            db = self._get_db(db)
+            if db is None:
+                db = self.env.get_db_cnx()
 
             # Fetch the standard ticket fields
             std_fields = [f['name'] for f in self.fields if not f.get('custom')]
-            cursor = db.cursor()
+