PHP and XML - Deleting nodes

Question

I have created a PHP script which builds a table from an XML document via AJAX. For example:

<bookstore>
  <book>
    <title>Everyday Italian</title>
    <author>Giada De Laurentiis</author>
    <year>2005</year>
    <price>30.00</price>
  </book>
  <book>
    <title>Harry Potter</title>
    <author>J K.  Rowling</author>
    <year>2005</year>    
    <price>29.99</price>
  </book>
</bookstore>

Would create a table with title, author, year and price columns plus an additional delete colum. When parsing the XML I have set the tr id to be that of the current XML element (0 and 1) in the case above.

When I click delete I fire off an AJAX request with the ID of the row I want to delete. The delete script is receiving the current row number without problem but I am having strange results when trying to delete it. The current code I'm trying is below (taken from http://quest4knowledge.wordpress.com/2010/09/04/php-xml-create-add-edit-modify-using-dom-simplexml-xpath/ 7.2)

if (isset($_POST['rowNumber'])) {

    $rowNumber = $_POST['rowNumber'];
    $file = $_POST['file'];

    $dom = new DOMDocument();
    $dom->load("../XML/".$file);      
    $xml = $dom->documentElement;
    //PROBLEM HERE
    $xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item($rowNumber));

    $handle = fopen("../XML/".$file, 'w');
    fwrite($handle, $dom->saveXML()); 

}

I build the table on page load and then on every deletion. Trouble is that the incorrect rows are being deleted and I can't figure out why.

ADDITIONAL TESTING...

Deletes the first node on 50% of clicks:

$xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item(0));

Always deletes the first node:

$xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item(1));

Deletes the second node on 50% of clicks:

$xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item(2));

Always deletes the second node:

$xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item(3));

Deletes the third node on 50% of clicks:

$xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item(4));

Always deletes the third node:

$xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item(5));

ADDED MY AJAX CODE

$('#generatedTable a.delete').live('click', function (e) {
    e.preventDefault();

//TABLE ROW ID TO BE DELETED. CAN ALERT THIS FINE.
    var trID = $(this).closest('tr').attr('id'); 

    $.ajax({
        url: "functions/xmlDelete.php", 
        type: "POST",
        dataType: "json",
        data: "rowNumber="+ trID + "&fileName=" + fileName,
        success: function(data) {

            $.ajax({
                url: "functions/xmlParser.php", 
                type: "POST",
                dataType: "json",
                data: "fileName="+ fileName,
                success: function(data) {

                    $('#xmlTable').html(data.table);
                    $('#xmlTable').fadeIn('fast');

                    oTable = $('#generatedTable').dataTable({
                        "bJQueryUI": true,
                        "bPaginate": false,
                        "bLengthChange": false,
                        "bFilter": false,
                        "bSort": false,
                        "bInfo": true
                    }); 

                }      
            }); 

        }      
    });         
} );

This is what my table rows look like and I can alert the trID without problem.

<tr id="0" class="odd">
  <td id="0">1999 Grammy Nominees</td>
  <td id="1">Many</td>
  <td id="2">USA</td>
  <td id="3">Grammy</td>
  <td id="4">10.20</td>
  <td id="5">1999</td>
  <td align="center"><a class="edit" href="">Edit</a></td>
  <td align="center"><a class="delete" href="">Delete</a></td>
</tr>

Can anyone help explain what I'm seeing here. Thanks!

Answer 1

You are not taking into account that childNodes() includes all nodes, not just elements.

For the xml document that you provide, $dom->documentElement->childNodes->item(0) is the whitespace between the end of <bookstore> and the beginning of <book>, not the first <book> node.

Now you know why the DOM is so irritating.

I suggest you use either DOMXPath or SimpleXML rather than loop through the childNodes to collect Element indexes.

DOMXPath solution

if (isset($_POST['rowNumber'], $_POST['file']) and ctype_digit($_POST['rowNumber'])) {
    $rowNumber = $_POST['rowNumber'];
    $file = '../XML/'.$_POST['file'];

    $dom = new DOMDocument();
    $dom->load($file);

    $root = $dom->documentElement;

    $xp = new DOMXPath($dom);
    $books = $xp->query('*', $root);
    if ($books->item($rowNumber)) {
        $root->removeChild($books->item($rowNumber));
        // Note that "$root->childNodes->item(0)->parentNode" is completely unnecessary.
        // You already have the parent node ($root), so just use it directly!
    } else {
        echo "Row does not exist";
    }

    $dom->save($file);
}

SimpleXML solution

if (isset($_POST['rowNumber'], $_POST['file']) and ctype_digit($_POST['rowNumber'])) {
    $rowNumber = (int) $_POST['rowNumber']; // casting to int is necessary!!
    $file = '../XML/'.$_POST['file'];

    $sxe = simplexml_load_file($file);
    unset($sxe->book[$rowNumber]);

    // or, if you don't want to make element name assumptions:
    // $children = $sxe->children();
    // unset($children[$rowNumber]);

    $sxe->asXML($file);
}

Problems

However, there are two serious problems with your whole approach.

1. You are accepting untrusted input through the file post variable. There is nothing a user from reading and writing to any XML file on your drive. You should have a predefined list of valid file values that $_POST['file'] must match before you will process the request.

2. You do not account for concurrency. This manifests itself in two ways:

   1. If two users are editing at the same time and one deletes book 0, then when the second deletes book 0 he will be deleting a different book than he intended, i.e., what appears to him to be book 1! The solution to this is that each book needs an explicit, unique identifier which you use in the XML file and your html interface. You cannot use index numbering to identify nodes.
   2. Your reads and writes to and from the XML file are not atomic, nor do they use any file locking. Thus it is possible for someone to read the XML while another PHP process is in the middle of writing to the file, giving the reader an incomplete XML file. You must either do an atomic write (on *nix systems, write to a unique temporary file and then link() to the real file name), or you must open the file with flock() or some other locking mechanism.

Because managing concurrent reads and writes (especially in a speedy way) is so difficult, I recommend you abandon this file-as-a-database approach and use a real database instead.

(And by the way, your HTML is invalid. id attributes must be unique within a document, but you have two id="0".)

Answer 2

If I understand the documentation you linked correctly,

 $library->childNodes->item(0)->parentNode

simply is a complicated and irritating way of casting A DOMDocument to a DOMElement. You will spot that pattern in some more examples.

Now that we have a DOMElement, we can invoke removeChild() on it, which gets passed a DOMElement. This element is

$library->childNodes->item(1)

for the second element.

So, your code possibly should look like this:

$xml->childNodes->item(0)->parentNode->removeChild($xml->childNodes->item($rowNumber));

Given that $rowNumber is 0-based.