GDataXMLNode doesn't support the NSXMLNode 'parent' method (Patch and Test included)

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Any code that relies on the NSXMLNode parent attribute won't work, as 
GDataXMLNode doesn't supply it.  Since GDataXMLNode and GDataXMLElement attempt 
to mirror the NSXML equivalents, this signature missing causes problems.

What is the expected output? What do you see instead?
Expected behavior is that NSXMLNode#parent should return the parent object, or 
nil if it's not available.  Since NSXMLNode is replaced with GDataXMLNode, it 
should return the parent object for the parent method call.

What version of the product are you using? On what operating system?
I checked out trunk today, June 25, 2010; revision 538.  I'm using it in an 
iPhone project, but it's a problem in the code for any platform.

Please provide any additional information below.
Attached is a patch to fix it, along with a simple assertion to test it.

--  Morgan Schweers

Original issue reported on code.google.com by cyber...@gmail.com on 25 Jun 2010 at 11:30

Attachments:

• add_parent_to_gdataxmlnode_signature.diff
• test_gdataxmlnode_parent.diff

[GoogleCodeExporter]

We can't add pointers from children to parents at the GDataXMLNode level 
because that would create retain loops. 

Also complicating things is that "addChild:" doesn't add a pointer from parent 
to child, but rather makes a copy of the child. libxml isn't reference-counted 
so each GDataXMLNode must point to a unique copy of a libxml node, or else 
"borrow" it weakly.

A parent method like this would work, but might not give you the behavior you 
expect:

- (GDataXMLNode *)parent {
  if (xmlNode_ != NULL && xmlNode_->parent != NULL) {
    GDataXMLNode *parent = [GDataXMLNode nodeBorrowingXMLNode:xmlNode_->parent];
    return parent;
  }
  return nil;
}

It creates a new GDataXMLNode referencing the parent libxml node. There's no 
way to go from parent A to child B, then back to the parent A. It has to be a 
new GDataXMLNode that also points to the same parent data as A. But that new 
parent node can't guarantee the life of the underlying libxml tree data without 
copying it yet again.

Original comment by gregrobbins on 26 Jun 2010 at 2:39

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Greetings,
Ah; yeah, I'm not sure that would do the right thing.  I'm relatively new to 
Objective C; is the problem with retain loops a garbage collection issue?

With your comment in mind I looked at -addChild anew.  Ignoring the fundamental 
problem of retain cycles, it's definitely the right thing to do to remove the 
child.parent=self's there, as they're strictly wrong, modifying the passed-in 
object, not the actual child object.

With that removed, because -addChild releases the cached values at the top of 
the -addChild method, the next reference to -children will recreate all the 
GDataXMLNode's, and since that code sets parent, it would do the right thing 
for my use case.

I'm less sure about the -addAttribute call; I think that I'd have to modify the 
-attributes method also, to set parent appropriately, again with the cache 
clearing that occurs in mind.

Poking around to try and understand, it seems like retain cycles shouldn't be 
an issue for non-GC'ed environments if the child never tries to release its 
parent.  Under GC, I could imagine it holding memory that should otherwise be 
released.

I understand that the patch, or probably any similar patch, isn't going to be 
helpful to the framework.  At this point I'm mainly trying to figure out how to 
make it work for my own code, which is an iPhone app, thus not facing GC 
issues.  Any suggestions or corrections are very welcome, as it'll help me 
learn ObjC better.  :)

--  Morgan Schweers

Original comment by cyber...@gmail.com on 26 Jun 2010 at 8:44

[GoogleCodeExporter]

The -parent method I showed above is probably the most correct implementation 
given the underlying use of libxml, but I don't know if it's sufficient for 
your needs. 

Fundamentally, Apple's NSXML class only works perfectly with Apple's 
implementation (though they don't consider it perfect enough to put on the 
iPhone, clearly.)

Retain loops are a problem whenever a child points to a parent and the parent 
also points to the child, even indirectly. In this case, it's compounded by 
their being the real underlying libxml tree and the incomplete tree of GDataXML 
objects to try to maintain the NSXML interface.

Trying to keep track of parents and children in the GDataXMLNode is fraught 
with risk. The current implementation caches a bit for efficiency, but the 
actual tree is always the underlying libxml tree, never the GDataXML object 
tree.

Original comment by gregrobbins on 29 Jun 2010 at 1:25