[glom] Do not hide the database structure if the user does not have view rights.



commit 19f4ab4dd8b0247485121ebd87e653776d815149
Author: Murray Cumming <murrayc murrayc com>
Date:   Thu Feb 23 12:58:23 2012 +0100

    Do not hide the database structure if the user does not have view rights.
    
    * glom/frame_glom.cc: update_table_in_document_from_database(): Warn if we cannot
    get the fields list from the database and do not then assume that there are no fields.
    * glom/libglom/db_utils.cc: get_fields_for_table(): Do not get the fields from the
    database because it is inefficient and we should already have updated the document.
    * glom/mode_data/box_data_list.cc: create_layout(): Do not (incredibly inefficiently)
    check that each field exists in the database. We should already have updated the
    document.
    
    This should make the UI faster too.
    Bug #669299

 ChangeLog                       |   15 +++++++++++++++
 glom/frame_glom.cc              |   39 ++++++++++++++++++++++++---------------
 glom/libglom/db_utils.cc        |   17 ++++++++++-------
 glom/mode_data/box_data_list.cc |    5 +++++
 4 files changed, 54 insertions(+), 22 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index d0d089e..fa0bce8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,20 @@
 2012-02-23  Murray Cumming  <murrayc murrayc com>
 
+	Do not hide the database structure if the user does not have view rights.
+
+	* glom/frame_glom.cc: update_table_in_document_from_database(): Warn if we cannot
+	get the fields list from the database and do not then assume that there are no fields.
+	* glom/libglom/db_utils.cc: get_fields_for_table(): Do not get the fields from the 
+	database because it is inefficient and we should already have updated the document.
+	* glom/mode_data/box_data_list.cc: create_layout(): Do not (incredibly inefficiently)
+	check that each field exists in the database. We should already have updated the 
+	document.
+
+	This should make the UI faster too.
+	Bug #669299
+
+2012-02-23  Murray Cumming  <murrayc murrayc com>
+
 	Change lots of g_warnings() to std::cerr.
 
 2012-02-23  Murray Cumming  <murrayc murrayc com>
diff --git a/glom/frame_glom.cc b/glom/frame_glom.cc
index bc99d56..66d4c63 100644
--- a/glom/frame_glom.cc
+++ b/glom/frame_glom.cc
@@ -1403,6 +1403,12 @@ void Frame_Glom::update_table_in_document_from_database()
 
   //Get the fields information from the database:
   DbUtils::type_vec_fields fieldsDatabase = DbUtils::get_fields_for_table_from_database(m_table_name);
+  if(fieldsDatabase.empty())
+  {
+    std::cerr << G_STRFUNC << ": Could not get the list of fields for table=" << m_table_name <<
+      " from the database. This might be due to insufficient database user rights." << 
+      " Falling back to the field details in the document." << std::endl;
+  }
 
   Document* pDoc = dynamic_cast<const Document*>(get_document());
   if(pDoc)
@@ -1458,25 +1464,28 @@ void Frame_Glom::update_table_in_document_from_database()
 
     //Remove fields that are no longer in the database:
     //TODO_performance: This is incredibly inefficient - but it's difficut to erase() items while iterating over them.
-    type_vec_fields fieldsActual;
-    for(type_vec_fields::const_iterator iter = fieldsDocument.begin(); iter != fieldsDocument.end(); ++iter)
+    if(!fieldsDatabase.empty()) //Do not do this if getting the fields from the database failed completely, probably due to permissions.
     {
-      sharedptr<Field> field = *iter;
-
-      //Check whether it's in the database:
-      type_vec_fields::iterator iterFindDatabase = std::find_if( fieldsDatabase.begin(), fieldsDatabase.end(), predicate_FieldHasName<Field>( field->get_name() ) );
-      if(iterFindDatabase != fieldsDatabase.end()) //If it was found
+      type_vec_fields fieldsActual;
+      for(type_vec_fields::const_iterator iter = fieldsDocument.begin(); iter != fieldsDocument.end(); ++iter)
       {
-        fieldsActual.push_back(field);
-      }
-      else
-      {
-        document_must_be_updated = true; //Something changed.
+        sharedptr<Field> field = *iter;
+
+        //Check whether it's in the database:
+        type_vec_fields::iterator iterFindDatabase = std::find_if( fieldsDatabase.begin(), fieldsDatabase.end(), predicate_FieldHasName<Field>( field->get_name() ) );
+        if(iterFindDatabase != fieldsDatabase.end()) //If it was found
+        {
+          fieldsActual.push_back(field);
+        }
+        else
+        {
+          document_must_be_updated = true; //Something changed.
+        }
       }
-    }
 
-    if(document_must_be_updated)
-      pDoc->set_table_fields(m_table_name, fieldsActual);
+      if(document_must_be_updated)
+        pDoc->set_table_fields(m_table_name, fieldsActual);
+    }
   }
 }
 #endif // !GLOM_ENABLE_CLIENT_ONLY
diff --git a/glom/libglom/db_utils.cc b/glom/libglom/db_utils.cc
index 0a51216..37d018a 100644
--- a/glom/libglom/db_utils.cc
+++ b/glom/libglom/db_utils.cc
@@ -959,23 +959,25 @@ type_vec_fields get_fields_for_table_from_database(const Glib::ustring& table_na
   return result;
 }
 
-//TODO: This is very inefficient, because it is called so often. Just update the document with whatever is really in the database:
+//TODO: This is very inefficient, because it is 
 type_vec_fields get_fields_for_table(const Document* document, const Glib::ustring& table_name, bool including_system_fields)
 {
-  //Get field definitions from the database:
-  type_vec_fields fieldsDatabase = get_fields_for_table_from_database(table_name, including_system_fields);
+  //We could also get the field definitions from the database:
+  //But that is inefficient because this method is called so often,
+  //and that meta information is not even available if the user does not have SELECT rights.
+  //Therefore we just assume that the Document has been updated from the database already.
+  //type_vec_fields fieldsDatabase = get_fields_for_table_from_database(table_name, including_system_fields);
 
   if(!document)
   {
     std::cerr << G_STRFUNC << ": document is null" << std::endl;
-    return fieldsDatabase; //This should never happen.
+    return type_vec_fields(); //This should never happen.
   }
 
-  type_vec_fields result;
-
-  type_vec_fields fieldsDocument = document->get_table_fields(table_name);
+  type_vec_fields result = document->get_table_fields(table_name);
 
   //Look at each field in the database:
+  /*
   for(type_vec_fields::iterator iter = fieldsDocument.begin(); iter != fieldsDocument.end(); ++iter)
   {
     sharedptr<Field> field = *iter;
@@ -1020,6 +1022,7 @@ type_vec_fields get_fields_for_table(const Document* document, const Glib::ustri
     if(iterFind == result.end() )
       result.push_back(*iter);
   }
+  */
 
   return result;
 }
diff --git a/glom/mode_data/box_data_list.cc b/glom/mode_data/box_data_list.cc
index b6a045b..97b6c64 100644
--- a/glom/mode_data/box_data_list.cc
+++ b/glom/mode_data/box_data_list.cc
@@ -449,6 +449,10 @@ void Box_Data_List::create_layout()
         child_item->set_editable(false);
 
       sharedptr<const LayoutItem_Field> child_field = sharedptr<const LayoutItem_Field>::cast_dynamic(child_item);
+
+      //This check has already happened in Frame_Glom::update_table_in_document_from_database().
+      //It is inefficient and unnecessary to do it here too.
+      /*
       if(child_field)
       {
         //Check that the field really exists, to avoid SQL errors.
@@ -459,6 +463,7 @@ void Box_Data_List::create_layout()
           continue;
         }
       }
+      */
 
       items_to_use.push_back(child_item);
     }



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]