Post PR edits suggested by Vmora; new checks on schema name validity

test/history_creation_test.py

@@ -93,13 +93,11 @@
 ##pcur.execute("SELECT * FROM epanet_mybranch_rev_2.pipes")
 ##assert( len(pcur.fetchall()) == 1 )
 
-select_and_where_str =  versioning_base.rev_view_str( pg_conn_info, 'epanet', 'junctions','mybranch', 2)
-#print select_and_where_str[0] + " WHERE " + select_and_where_str[1]
-pcur.execute(select_and_where_str[0] + " WHERE " + select_and_where_str[1])
+select_str, where_str =  versioning_base.rev_view_str( pg_conn_info, 'epanet', 'junctions','mybranch', 2)
+pcur.execute(select_str + " WHERE " + where_str)
 assert( len(pcur.fetchall()) == 2 )
-select_and_where_str =  versioning_base.rev_view_str( pg_conn_info, 'epanet', 'pipes','mybranch', 2)
-#print select_and_where_str[0] + " WHERE " + select_and_where_str[1]
-pcur.execute(select_and_where_str[0] + " WHERE " + select_and_where_str[1])
+select_str, where_str =  versioning_base.rev_view_str( pg_conn_info, 'epanet', 'pipes','mybranch', 2)
+pcur.execute(select_str + " WHERE " + where_str)
 assert( len(pcur.fetchall()) == 1 )
 
 pcur.close()

test/versioning_base_test.py

@@ -99,7 +99,6 @@
 
 pg_conn_info = "dbname=epanet_test_db"
 select_str = versioning_base.diff_rev_view_str(pg_conn_info, 'epanet', 'junctions','trunk', 1,2)
-#print "select_str = " + select_str
 pcur.execute(select_str)
 res = pcur.fetchall()
 assert(res[0][0] == 'u')
@@ -108,42 +107,39 @@
 #('u', 3, '1', 8.0, None, None, '01010000206A0800000000000000000000000000000000F03F', 2, 2, 2, 4)]
 
 select_str = versioning_base.diff_rev_view_str(pg_conn_info, 'epanet', 'junctions','trunk', 1,3)
-#print "select_str = " + select_str
 pcur.execute(select_str)
 res = pcur.fetchall()
+assert(res[0][0] == 'u')
+assert(res[1][0] == 'i')
 #print "fetchall 1 vs 3 = " + str(res)
 #fetchall 1 vs 3 = [
 #('u', 4, '1', 22.0, None, None, '01010000206A0800000000000000000000000000000000F03F', 3, None, 3, None),
 #('i', 3, '1', 8.0, None, None, '01010000206A0800000000000000000000000000000000F03F', 2, 2, 2, 4)]
-assert(res[0][0] == 'u')
-assert(res[1][0] == 'i')
 
 select_str = versioning_base.diff_rev_view_str(pg_conn_info, 'epanet', 'junctions','trunk', 1,4)
-#print "select_str 1 vs 4 = " + select_str
 pcur.execute(select_str)
 res = pcur.fetchall()
+assert(res[0][0] == 'u')
+assert(res[1][0] == 'i')
+assert(res[2][0] == 'a')
+assert(res[3][0] == 'i') # object is in intermediate state; will be deleted in rev 5
 #print "fetchall 1 vs 4 = " + str(res)
 #fetchall 1 vs 4 = [
 #('u', 4, '1', 22.0, None, None, '01010000206A0800000000000000000000000000000000F03F', 3, None, 3, None),
 #('i', 3, '1', 8.0, None, None, '01010000206A0800000000000000000000000000000000F03F', 2, 2, 2, 4),
 #('a', 5, '10', 100.0, None, None, '01010000206A08000000000000000000400000000000000000', 4, None, None, None),
 #('i', 1, '0', 0.0, None, None, '01010000206A080000000000000000F03F0000000000000000', 1, 4, None, None)]
-assert(res[0][0] == 'u')
-assert(res[1][0] == 'i')
-assert(res[2][0] == 'a')
-assert(res[3][0] == 'i') # object is in intermediate state; will be deleted in rev 5
 
 select_str = versioning_base.diff_rev_view_str(pg_conn_info, 'epanet', 'junctions','trunk', 1,5)
-#print "select_str 1 vs 5 = " + select_str
 pcur.execute(select_str)
 res = pcur.fetchall()
+assert(res[0][0] == 'u')
+assert(res[1][0] == 'i')
+assert(res[2][0] == 'a')
+assert(res[3][0] == 'd')
 #print "fetchall 1 vs 5 = " + str(res)
 #fetchall 1 vs 5 = [
 #('u', 4, '1', 22.0, None, None, '01010000206A0800000000000000000000000000000000F03F', 3, None, 3, None),
 #('i', 3, '1', 8.0, None, None, '01010000206A0800000000000000000000000000000000F03F', 2, 2, 2, 4),
 #('a', 5, '10', 100.0, None, None, '01010000206A08000000000000000000400000000000000000', 4, None, None, None),
 #('d', 1, '0', 0.0, None, None, '01010000206A080000000000000000F03F0000000000000000', 1, 4, None, None)]
-assert(res[0][0] == 'u')
-assert(res[1][0] == 'i')
-assert(res[2][0] == 'a')
-assert(res[3][0] == 'd')

versioning.py

@@ -505,7 +505,8 @@ def view(self):
             self.q_view_dlg.tblw.itemChanged.disconnect()
             self.q_view_dlg.diffmode_chk.stateChanged.disconnect()
         except:
-            print "Failed disconnection"
+            #print "Failed disconnection"
+            pass
 
         # Make sure combobox is initalized correctly
         self.q_view_dlg.diffmode_chk.setCheckState(Qt.Unchecked)
@@ -592,7 +593,6 @@ def view(self):
                 new_uri = QgsDataSourceURI(layer.source())
                 select_str = versioning_base.diff_rev_view_str( uri.connectionInfo(),
                     schema, new_uri.table(), branches[0], rev_begin, rev_end )
-                #print "select_str = " + select_str
                 # change data source uri to point to select sql
                 # schema needs to be set to empty
                 new_uri.setDataSource("",
@@ -674,18 +674,12 @@ def view(self):
                 for layer_id in reversed(self.current_layers):
                     layer = QgsMapLayerRegistry.instance().mapLayer(layer_id)
                     new_uri = QgsDataSourceURI(layer.source())
-                    select_and_where_str =  versioning_base.rev_view_str(
-                        self.pg_conn_info(), schema, new_uri.table(), branches[0], rev)
-                    select_str = select_and_where_str[0]
-                    where_str = select_and_where_str[1]
-                    #select_str_orig = "SELECT * FROM "+schema+"."+new_uri.table()
-                    #print "select_str_orig = " + "     " + select_str_orig
-                    #print "select_str from base = " + select_str
-                    #where_str_orig = ("("+branches[0] + "_rev_end IS NULL "
-                    #    "OR "+branches[0]+"_rev_end >= "+str(rev) + ") "
-                    #     "AND "+branches[0]+"_rev_begin <= "+str(rev) )
-                    #print "where_str_orig = "  + "     " + where_str_orig
-                    #print "where_str from base = " + where_str
+                    select_str, where_str =  versioning_base.rev_view_str(
+                        self.pg_conn_info(),
+                        schema,
+                        new_uri.table(),
+                        branches[0],
+                        rev)
                     new_uri.setSql(where_str)
                     new_uri.setDataSource("",
                         "("+select_str+")",
@@ -967,9 +961,26 @@ def checkout_pg(self):
             return
         working_copy_schema = line_edit.text()
         if not working_copy_schema:
-            print "aborted"
+            print "Name not provided; aborted"
+            self.iface.messageBar().pushMessage("Warning",
+            "Please provide a schema name.", duration=5)
+            return
+        # Check if name is valid for a PG object; only characters and max length
+        # are checked; use of reserved words is not checked
+        if len(working_copy_schema) > 63:
+            print "Name too long; aborted"
+            self.iface.messageBar().pushMessage("Warning",
+            "\""+working_copy_schema+"\" is "+str(len(working_copy_schema))+
+            " characters long;  maximum is 63.", duration=5)
+            return
+        valid_name = re.match('^[a-z_][a-z_0-9$]*$', str(working_copy_schema))
+        if not(valid_name):
+            print "Not a valid name"
+            self.iface.messageBar().pushMessage("Warning",
+            "\""+working_copy_schema+"\" is not valid; first character must be "
+            "<b>lowercase</b> letter or underscore; other characters may be "
+            "<b>lowercase</b> letters, underscore or digits.", duration=10)
             return
-
         print "checking out ", tables_for_conninfo, " from ", uri.connectionInfo()
         versioning_base.pg_checkout( self.pg_conn_info(),
                 tables_for_conninfo, working_copy_schema )

versioning_base.py

@@ -1125,6 +1125,9 @@ def diff_rev_view_str(pg_conn_info, schema, table, branch, rev_begin, rev_end):
     """DIFFerence_REVision_VIEW_STRing
     Create the SQL view string of the specified revision difference (comparison).
     """
+    rev_begin = str(rev_begin)
+    rev_end = str(rev_end)
+
     pcur = Db(psycopg2.connect(pg_conn_info))
 
     pcur.execute("SELECT * FROM "+schema+".revisions "
@@ -1136,30 +1139,30 @@ def diff_rev_view_str(pg_conn_info, schema, table, branch, rev_begin, rev_end):
     [max_rev] = pcur.fetchone()
     if int(rev_begin) > max_rev or int(rev_begin) <= 0:
         pcur.close()
-        raise RuntimeError("Revision 1 (begin) "+str(rev_begin)+" doesn't exist")
+        raise RuntimeError("Revision 1 (begin) "+rev_begin+" doesn't exist")
     if int(rev_end) > max_rev or int(rev_end) <= 0:
         pcur.close()
-        raise RuntimeError("Revision 2 (end) "+str(rev_end)+" doesn't exist")
+        raise RuntimeError("Revision 2 (end) "+rev_end+" doesn't exist")
 
     select_str = ("SELECT "
     "CASE WHEN "
-        +schema+"."+table+"."+branch+"_rev_begin > "+str(rev_begin)+ " "
-        "AND " +schema+"."+table+"."+branch+"_rev_begin <= "+str(rev_end)+ " "
+        +schema+"."+table+"."+branch+"_rev_begin > "+rev_begin+ " "
+        "AND " +schema+"."+table+"."+branch+"_rev_begin <= "+rev_end+ " "
         "AND " +schema+"."+table+"."+branch+"_parent IS NULL THEN 'a' "
-    "WHEN (" +schema+"."+table+"."+branch+"_rev_begin > "+str(rev_begin)+ " "
+    "WHEN (" +schema+"."+table+"."+branch+"_rev_begin > "+rev_begin+ " "
         "AND " +schema+"."+table+"."+branch+"_rev_end IS NULL "
         "AND " +schema+"."+table+"."+branch+"_parent IS NOT NULL) "
-        "OR ("+schema+"."+table+"."+branch+"_rev_end >= "+str(rev_end)+" "
+        "OR ("+schema+"."+table+"."+branch+"_rev_end >= "+rev_end+" "
         "AND "+schema+"."+table+"."+branch+"_child IS NOT NULL) "
         "THEN 'u' "
-    "WHEN " +schema+"."+table+"."+branch+"_rev_end > "+str(rev_begin)+ " "
-        "AND " +schema+"."+table+"."+branch+"_rev_end < "+str(rev_end)+ " "
+    "WHEN " +schema+"."+table+"."+branch+"_rev_end > "+rev_begin+ " "
+        "AND " +schema+"."+table+"."+branch+"_rev_end < "+rev_end+ " "
         "AND " +schema+"."+table+"."+branch+"_child IS NULL THEN 'd' ELSE 'i' END "
     "as diff_status, * FROM "+schema+"."+table+ " "
-    "WHERE (" +schema+"."+table+"."+branch+"_rev_begin > "+str(rev_begin)+ " "
-        "AND " +schema+"."+table+"."+branch+"_rev_begin <= "+str(rev_end)+") "
-        "OR (" +schema+"."+table+"."+branch+"_rev_end > "+str(rev_begin)+ " "
-        "AND " +schema+"."+table+"."+branch+"_rev_end <= "+str(rev_end)+ " )")
+    "WHERE (" +schema+"."+table+"."+branch+"_rev_begin > "+rev_begin+ " "
+        "AND " +schema+"."+table+"."+branch+"_rev_begin <= "+rev_end+") "
+        "OR (" +schema+"."+table+"."+branch+"_rev_end > "+rev_begin+ " "
+        "AND " +schema+"."+table+"."+branch+"_rev_end <= "+rev_end+ " )")
 
     pcur.close()
     return select_str
@@ -1189,7 +1192,7 @@ def rev_view_str(pg_conn_info, schema, table, branch, rev):
          "AND "+branch+"_rev_begin <= "+str(rev) )
 
     pcur.close()
-    return [select_str, where_str]
+    return select_str, where_str
 
 def add_revision_view(pg_conn_info, schema, branch, rev):
     """Create schema with views of the specified revision.