[gxml] * actually implement syncing of attributes from hashtable to libxml2 structures (needs more testing)

[Richard Hans Schwarting <rschwart src gnome org>]

commit 9c3ad5b9609b2b52f0285b28ff2a465c9a70e010
Author: Richard Schwarting <aquarichy gmail com>
Date:   Tue Aug 9 03:34:51 2011 +0200

    * actually implement syncing of attributes from hashtable to libxml2 structures (needs more testing)

 gxml/BackedNode.vala |    1 +
 gxml/Document.vala   |   83 ++++++++++++++++++++++++++++++++++-------------
 gxml/Element.vala    |   87 ++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 138 insertions(+), 33 deletions(-)
---
diff --git a/gxml/BackedNode.vala b/gxml/BackedNode.vala
index 10a8d5d..2313fd6 100644
--- a/gxml/BackedNode.vala
+++ b/gxml/BackedNode.vala
@@ -246,6 +246,7 @@ namespace GXml.Dom {
 			Xml.Buffer *buffer;
 			string str;
 
+			this.owner_document.sync_dirty_elements ();
 			buffer = new Xml.Buffer ();
 			buffer->node_dump (this.owner_document.xmldoc, this.node, level, format ? 1 : 0);
 			str = buffer->content ();
diff --git a/gxml/Document.vala b/gxml/Document.vala
index dc60856..c311599 100644
--- a/gxml/Document.vala
+++ b/gxml/Document.vala
@@ -46,34 +46,39 @@ namespace GXml.Dom {
 	 * [[http://www.w3.org/TR/DOM-Level-1/level-one-core.html#i-Document]]
 	 */
 	public class Document : XNode {
-		/** Private properties */
+		/* *** Private properties *** */
+
+		/**
+		 * This contains a map of Xml.Nodes that have been
+		 * accessed and the GXml XNode we created to represent
+		 * them on-demand.  That way, we don't create an XNode
+		 * for EVERY node, even if the user never actually
+		 * accesses it.  
+		 */ 
 		internal HashTable<Xml.Node*, XNode> node_dict = new HashTable<Xml.Node*, XNode> (GLib.direct_hash, GLib.direct_equal);
 		// We don't want want to use XNode's Xml.Node or its dict
 		// internal HashTable<Xml.Attr*, Attr> attr_dict = new HashTable<Xml.Attr*, Attr> (null, null);
 
+		/**
+		 * This contains a list of elements whose attributes
+		 * may have been modified within GXml, and whose modified
+		 * attributes need to be saved back to the underlying
+		 * libxml2 structure when we save.  (Necessary because
+		 * the user can obtain a HashTable and modify that in a
+		 * way that we can't follow unless we check ourselves.)
+		 * Perhaps I really should implement a NamedNodeMap :|
+		 * TODO: do that
+		 */
+		internal List<Element> dirty_elements = new List<Element> ();
+
 		/* TODO: for future reference, find out if internals
-		   are only accessible by children when they're compiled
-		   together */
+		   are only accessible by children when they're
+		   compiled together.  I have a test that had a
+		   separately compiled TestDocument : Document class,
+		   and it couldn't access the internal xmldoc. */
 		internal Xml.Doc *xmldoc;
 
-		/** Private methods */
-		// internal unowned Attr? lookup_attr (Xml.Attr *xmlattr) {
-		// 	unowned Attr attrnode;
-
-		// 	if (xmlattr == null) {
-		// 		return null; // TODO: consider throwing an error instead
-		// 	}
-
-		// 	attrnode = this.attr_dict.lookup (xmlattr);
-		// 	if (attrnode == null) {
-		// 		// TODO: threadsafety
-		// 		this.attr_dict.insert (xmlattr, new Attr (xmlattr, this));
-		// 		attrnode = this.attr_dict.lookup (xmlattr);
-		// 	}
-
-		// 	return attrnode;
-		// }
-
+		/* *** Private methods *** */
 		internal unowned XNode? lookup_node (Xml.Node *xmlnode) {
 			unowned XNode domnode;
 
@@ -342,10 +347,36 @@ namespace GXml.Dom {
 		}
 
 		/**
+		 * This should be called by any function that wants to
+		 * look at libxml2 data structures, particularly the
+		 * attributes of elements.  Such as: saving an Xml.Doc
+		 * to disk, or stringifying an Xml.Node.  GXml
+		 * developer, if you grep for ".node" and ".xmldoc",
+		 * you can help identify potential points where you
+		 * should sync.
+		 */
+		internal void sync_dirty_elements () {
+			Xml.Node * tmp_node;
+
+			// TODO: test that adding attributes works with stringification and saving
+			if (this.dirty_elements.length () > 0) {
+				// tmp_node for generating Xml.Ns* objects when saving attributes
+				tmp_node = new Xml.Node (null, "tmp"); 
+				foreach (Element elem in this.dirty_elements) {
+					elem.save_attributes (tmp_node);
+				}
+				this.dirty_elements = new List<Element> (); // clear the old list
+			}
+		}
+		
+
+		/**
 		 * Saves a Document to the file at path file_path
 		 */
 		// TODO: is this a simple Unix file path, or does libxml2 do networks, too?
 		public void save_to_path (string file_path) {
+			sync_dirty_elements ();
+
 			this.xmldoc->save_file (file_path);
 		}
 
@@ -357,6 +388,8 @@ namespace GXml.Dom {
 			Cancellable can = new Cancellable ();
 			OutputStreamBox box = { outstream, can };
 
+			sync_dirty_elements ();
+
 			// TODO: make sure libxml2's vapi gets patched
 			Xml.SaveCtxt *ctxt = new Xml.SaveCtxt.to_io ((Xml.OutputWriteCallback)_iowrite,
 								     (Xml.OutputCloseCallback)_iooutclose,
@@ -375,8 +408,11 @@ namespace GXml.Dom {
 			   for DOM Level 1 Core wants us to. Handle ourselves? */
 			// TODO: what does libxml2 do with Elements?  should we just use nodes? probably
 			// TODO: what should we be passing for ns other than old_ns?  Figure it out
-			Xml.Node *xmlelem = this.xmldoc->new_node (null, tag_name, null);
-			Element new_elem = new Element (xmlelem, this);
+			Xml.Node *xmlelem;
+			Element new_elem;
+
+			xmlelem = this.xmldoc->new_node (null, tag_name, null);
+			new_elem = new Element (xmlelem, this);
 			return new_elem;
 		}
 		/**
@@ -480,6 +516,7 @@ namespace GXml.Dom {
 			string str;
 			int len;
 
+			sync_dirty_elements ();
 			this.xmldoc->dump_memory_format (out str, out len, format);
 
 			return str;
diff --git a/gxml/Element.vala b/gxml/Element.vala
index 38fd7c8..39a43aa 100644
--- a/gxml/Element.vala
+++ b/gxml/Element.vala
@@ -51,7 +51,7 @@ namespace GXml.Dom {
 
 		/* HashTable used for XML NamedNodeMap */
 		// TODO: note that NamedNodeMap is 'live' so changes to the Node should be seen in the NamedNodeMap (already retrieved), no duplicating it: http://www.w3.org/TR/DOM-Level-1/level-one-core.html
-		private HashTable<string,Attr> _attributes = new HashTable<string,Attr> (GLib.str_hash, GLib.str_equal); // TODO: make sure other HashTables have appropriate hash, equal functions
+		private HashTable<string,Attr> _attributes = null;
 
 		/**
 		 * Contains a HashTable of Attr attributes associated with this element.
@@ -72,21 +72,89 @@ namespace GXml.Dom {
 		 * document.
 		 */
 		public override HashTable<string,Attr>? attributes {
-			// TODO: make sure we want the user to be able to manipulate attributes using this HashTable. // Yes, we do, it should be a live reflection
-			// TODO: remember that this table needs to be synced with libxml2 structures; perhaps use a flag that indicates whether it was even accessed, and only then sync it later on
+			/* TODO: make sure we want the user to be able
+			 * to manipulate attributes using this
+			 * HashTable. Yes, we do, it should be a live
+			 * reflection.  That's OK though, as long as
+			 * we save dirty attributes tables also when
+			 * we save the the ones in our hashtable back
+			 * into the xml.Doc before writing that to
+			 * whatever disk.
+			 */
+			/* TODO: remember that this table needs to be
+			 * synced with libxml2 structures; perhaps use
+			 * a flag that indicates whether it was even
+			 * accessed, and only then sync it later on
+			 */
 			get {
+				Attr attr;
+
+				try {
+					if (this._attributes == null) {
+						this.owner_document.dirty_elements.append (this);
+						this._attributes = new HashTable<string,Attr> (GLib.str_hash, GLib.str_equal);
+						// TODO: make sure other HashTables have appropriate hash, equal functions
+						
+						for (Xml.Attr *prop = base.node->properties; prop != null; prop = prop->next) {
+							attr = this.owner_document.create_attribute (prop->name);
+							this.attributes.replace (prop->name, attr);
+						}
+					}
+				} catch (DomError e) {
+					// TODO: handle this case, results from create_attribute 
+				}
+
 				return this._attributes;
-				// switch (this.node_type) {
-				// case NodeType.ELEMENT:
-				// 	// TODO: what other nodes have attrs?
-				// default:
-				// 	return null;
-				// }
 			}
 			internal set {
 			}
 		}
 
+		/**
+		 * This should be called before saving a GXml Document
+		 * to a libxml2 Xml.Doc*, or else any changes made to
+		 * attributes in the Element will only exist within
+		 * the hash table proxy and will not be recorded.
+		 */
+		internal void save_attributes (Xml.Node *tmp_node) {
+			Attr attr;
+			Xml.Ns *ns;
+
+			/* First, check if anyone has tried to access attributes, which
+			   means it could have changed.  Do this by checking whether our
+			   underlying local hashtable is still null. */
+			/* TODO: make sure that in normal operation
+			   where attributes aren't _explicitly_ referenced, that we don't
+			   internally induce this._attributes from being created. */
+			if (this._attributes != null) {
+				// First we have to clear the old properties, so we don't create duplicates
+				for (Xml.Attr *xmlattr = this.node->properties; xmlattr != null; xmlattr = xmlattr->next) {
+					// TODO: make sure that this actually works, and that I don't lose my attr->next for the next step by unsetting attr
+					// TODO: need a good test case that makes sure that the properties do not get duplicated, that removed ones stay removed, and new ones appear when recorded to back to a file
+					if (xmlattr->ns == null) {
+						// Attr has no namespace
+						this.node->unset_prop (xmlattr->name);
+					} else {
+						// Attr has a namespace
+						this.node->unset_ns_prop (xmlattr->ns, xmlattr->name);
+					}
+				}
+				
+				// Go through the GXml table of attributes for this element and add corresponding libxml2 ones
+				foreach (string propname in this.attributes.get_keys ()) {
+					attr = this.attributes.lookup (propname);
+
+					if (attr.namespace_uri != null || attr.prefix != null) {
+						// I hate namespace handling between libxml2 and DOM Level 2/3 Core!
+						ns = tmp_node->new_ns (attr.namespace_uri, attr.prefix);
+						this.node->set_ns_prop (ns, propname, attr.node_value);
+					} else {
+						this.node->set_prop (propname, attr.node_value);
+					}
+				}
+			}
+		}
+
 
 		/* Constructors */
 		internal Element (Xml.Node *node, Document doc) {
@@ -156,7 +224,6 @@ namespace GXml.Dom {
 		 * @return The Attr node named by name for this element.
 		 */
 		public Attr? get_attribute_node (string name) {
-
 			// TODO: verify that attributes returns null with unknown name
 			return this.attributes.lookup (name);
 		}