SOLR-133 -- found a bug in the delete XML parsing. for id's and queries with &, it did not behave correctly. Adds a fix and test. Another sideeffect that should be noted is that this parser now accepts multiple delete commands:

<delete>
 <id>1</id>
 <id>3</id>
 <id>4</id>
</delete`>


git-svn-id: https://svn.apache.org/repos/asf/lucene/solr/trunk@552385 13f79535-47bb-0310-9956-ffa450edef68

client/java/solrj/test/org/apache/solr/client/solrj/DirectXmlUpdateRequest.java

@@ -0,0 +1,63 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.client.solrj;
+
+import java.io.IOException;
+import java.util.Collection;
+
+import org.apache.solr.client.solrj.SolrServer;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.request.RequestBase;
+import org.apache.solr.client.solrj.response.SolrPingResponse;
+import org.apache.solr.client.solrj.response.UpdateResponse;
+import org.apache.solr.client.solrj.util.ClientUtils;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.ContentStream;
+
+/**
+ * 
+ * @author ryan
+ * @version $Id$
+ * @since solr 1.3
+ */
+public class DirectXmlUpdateRequest extends RequestBase
+{
+  final String xml;
+  
+  public DirectXmlUpdateRequest( String path, String body )
+  {
+    super( METHOD.POST, path );
+    xml = body;
+  }
+
+  public Collection<ContentStream> getContentStreams() {
+    return ClientUtils.toContentStreams( xml, ClientUtils.TEXT_XML );
+  }
+
+  public SolrParams getParams() {
+    return null;
+  }
+
+  public UpdateResponse process( SolrServer server ) throws SolrServerException, IOException
+  {
+    long startTime = System.currentTimeMillis();
+    UpdateResponse res = new UpdateResponse( server.request( this ) );
+    res.setElapsedTime( System.currentTimeMillis()-startTime );
+    return res;
+  }
+}

client/java/solrj/test/org/apache/solr/client/solrj/SolrExampleTestBase.java

@@ -18,6 +18,8 @@
 package org.apache.solr.client.solrj;
 
 
+import java.io.IOException;
+import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -25,7 +27,9 @@ import junit.framework.Assert;
 
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.client.solrj.response.UpdateResponse;
+import org.apache.solr.client.solrj.util.ClientUtils;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.util.XML;
 import org.apache.solr.util.AbstractSolrTestCase;
 
 /**
@@ -43,7 +47,7 @@ abstract public class SolrExampleTestBase extends AbstractSolrTestCase
   @Override public String getSolrConfigFile() { return "../../../example/solr/conf/solrconfig.xml"; }
   
   /**
-   * Subclasses need to initalize the server impl
+   * Subclasses need to initialize the server impl
    */
   protected abstract SolrServer getSolrServer();
   
@@ -148,4 +152,65 @@ abstract public class SolrExampleTestBase extends AbstractSolrTestCase
     Assert.assertEquals(2, response.getResults().getNumFound() );
     Assert.assertFalse(query.getFilterQueries() == query2.getFilterQueries());
   }
+  
+  protected void assertNumFound( String query, int num ) throws SolrServerException, IOException
+  {
+    QueryResponse rsp = getSolrServer().query( new SolrQuery( query ) );
+    if( num != rsp.getResults().getNumFound() ) {
+      fail( "expected: "+num +" but had: "+rsp.getResults().getNumFound() + " :: " + rsp.getResults() );
+    }
+  }
+
+  public void testAddDelete() throws Exception
+  {    
+    SolrServer server = getSolrServer();
+    
+    // Empty the database...
+    server.deleteByQuery( "*:*" );// delete everything!
+    
+    SolrInputDocument[] doc = new SolrInputDocument[3];
+    for( int i=0; i<3; i++ ) {
+      doc[i] = new SolrInputDocument();
+      doc[i].setField( "id", i + " & 222" );
+    }
+    String id = (String) doc[0].getFieldValue( "id" );
+    
+    server.add( doc[0] );
+    server.commit();
+    assertNumFound( "*:*", 1 ); // make sure it got in
+    
+    // make sure it got in there
+    server.deleteById( id );
+    server.commit();
+    assertNumFound( "*:*", 0 ); // make sure it got out
+    
+    // add it back 
+    server.add( doc[0] );
+    server.commit();
+    assertNumFound( "*:*", 1 ); // make sure it got in
+    server.deleteByQuery( "id:\""+ClientUtils.escapeQueryChars(id)+"\"" );
+    server.commit();
+    assertNumFound( "*:*", 0 ); // make sure it got out
+    
+    // Add two documents
+    for( SolrInputDocument d : doc ) {
+      server.add( d );
+    }
+    server.commit();
+    assertNumFound( "*:*", 3 ); // make sure it got in
+    
+    // should be able to handle multiple delete commands in a single go
+    StringWriter xml = new StringWriter();
+    xml.append( "<delete>" );
+    for( SolrInputDocument d : doc ) {
+      xml.append( "<id>" );
+      XML.escapeCharData( (String)d.getFieldValue( "id" ), xml );
+      xml.append( "</id>" );
+    }
+    xml.append( "</delete>" );
+    DirectXmlUpdateRequest up = new DirectXmlUpdateRequest( "/update", xml.toString() );
+    server.request( up );
+    server.commit();
+    assertNumFound( "*:*", 0 ); // make sure it got out
+  }
 }

src/java/org/apache/solr/handler/XmlUpdateRequestHandler.java

@@ -256,37 +256,42 @@ public class XmlUpdateRequestHandler extends RequestHandlerBase
         log.warning("unexpected attribute delete/@" + attrName);
       }
     }
-    
+
-    String val = null;
+    StringBuilder text = new StringBuilder();
-    String mode = null;
     while (true) {
       int event = parser.next();
       switch (event) {
       case XMLStreamConstants.START_ELEMENT:
-        mode = parser.getLocalName();
+        String mode = parser.getLocalName();
         if (!("id".equals(mode) || "query".equals(mode))) {
           log.warning("unexpected XML tag /delete/" + mode);
           throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, 
               "unexpected XML tag /delete/" + mode);
         }
+        text.setLength( 0 );
         break;
         
       case XMLStreamConstants.END_ELEMENT:
         String currTag = parser.getLocalName();
         if ("id".equals(currTag)) {
-          deleteCmd.id = val;
+          deleteCmd.id = text.toString();
         } else if ("query".equals(currTag)) {
-          deleteCmd.query = val;
+          deleteCmd.query = text.toString();
+        } else if( "delete".equals( currTag ) ) {
+          return;
         } else {
           log.warning("unexpected XML tag /delete/" + currTag);
           throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, 
               "unexpected XML tag /delete/" + currTag);
         }
         processor.processDelete( deleteCmd );
-        return;
+        break;
-        
+
+      // Add everything to the text
+      case XMLStreamConstants.SPACE:
+      case XMLStreamConstants.CDATA:
       case XMLStreamConstants.CHARACTERS:
-        val = parser.getText();
+        text.append( parser.getText() );
         break;
       }
     }