Add CloneBase to simplify clone() method calls which in turn cleans u…

…p the coverage reports. Fix some JavaDoc issues and lots of small coverage improvements

contrib/src/java/org/jdom2/contrib/perf/PerfDoc.java

@@ -33,7 +33,6 @@
 import org.jdom2.EntityRef;
 import org.jdom2.Namespace;
 import org.jdom2.ProcessingInstruction;
-import org.jdom2.SlimJDOMFactory;
 import org.jdom2.Text;
 import org.jdom2.UncheckedJDOMFactory;
 import org.jdom2.filter.ElementFilter;
@@ -197,7 +196,7 @@ public File getFile() {
 
 	public long saxLoad() throws Exception {
 		final long startmem = PerfTest.usedMem();
-		saxTime = PerfTest.timeRun(new SAXLoadRunnable(0) );
+		saxTime = PerfTest.timeRun(new SAXLoadRunnable(12) );
 		loadMem = PerfTest.usedMem() - startmem;
 		saxDTime = PerfTest.timeRun(new SAXLoadRunnable(8) );
 		return saxTime;
@@ -278,7 +277,8 @@ public Document subload(int type) throws Exception {
 				return null;
 			case 12:
 				SAXBuilder slimsax = new SAXBuilder();
-				slimsax.setJDOMFactory(new SlimJDOMFactory());
+				//slimsax.setJDOMFactory(new SlimJDOMFactory());
+				//slimsax.setFeature("http://xml.org/sax/features/string-interning", true);
 				return slimsax.build(car);
 		}
 		return null;

core/src/java/org/jdom2/Attribute.java

@@ -74,7 +74,8 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
  * @author  Victor Toni
  * @author  Rolf Lear
  */
-public class Attribute implements NamespaceAware, Serializable, Cloneable {
+public class Attribute extends CloneBase
+	implements NamespaceAware, Serializable, Cloneable {
 
 	// Keep the old constant names for one beta cycle to help migration
 
@@ -552,13 +553,9 @@ public String toString() {
 
 	@Override
 	public Attribute clone() {
-		try {
-			Attribute clone = (Attribute) super.clone();
-			clone.parent = null;
-			return clone;
-		} catch (CloneNotSupportedException e) {
-			throw new IllegalStateException("Clone not supported!", e);
-		}
+		final Attribute clone = (Attribute) super.clone();
+		clone.parent = null;
+		return clone;
 	}
 
 	/**

core/src/java/org/jdom2/CloneBase.java

@@ -0,0 +1,31 @@
+package org.jdom2;
+
+/**
+ * This simple class just tidies up any cloneable classes.
+ * This method deals with any CloneNotSupported exceptions.
+ * THis class is package private only.
+ * @author Rolf Lear
+ *
+ */
+class CloneBase implements Cloneable {
+
+	/**
+	 * This convenience method simply deals with the irritating
+	 * CloneNotSupportedException, and wraps it in IllegalStateException.
+	 * This should never, ever, ever happen. But, it is a pain to deal with
+	 * ugly clone code in some classes.
+	 * Subclasses of this should still call super.clone() in their clone method.
+	 */
+	@Override
+	protected Object clone() {
+		try {
+			return super.clone();
+		} catch (CloneNotSupportedException e) {
+			throw new IllegalStateException(String.format(
+					"Unable to clone class %s which should always support it.",
+					this.getClass().getName()),
+					e);
+		}
+	}
+
+}

core/src/java/org/jdom2/Content.java

@@ -73,7 +73,8 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
  * @author Bradley S. Huffman
  * @author Jason Hunter
  */
-public abstract class Content implements Cloneable, Serializable, NamespaceAware {
+public abstract class Content extends CloneBase
+		implements Cloneable, Serializable, NamespaceAware {
 
 	/**
 	 * An enumeration useful for identifying content types without
@@ -237,15 +238,9 @@ public Document getDocument() {
 	 */
 	@Override
 	public Content clone() {
-		try {
-			Content c = (Content)super.clone();
-			c.parent = null;
-			return c;
-		} catch (CloneNotSupportedException e) {
-			//Can not happen ....
-			//e.printStackTrace();
-			return null;
-		}
+		Content c = (Content)super.clone();
+		c.parent = null;
+		return c;
 	}
 
 	/**

core/src/java/org/jdom2/Document.java

@@ -67,7 +67,7 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
  * @author  Jools Enticknap
  * @author  Bradley S. Huffman
  */
-public class Document implements Parent {
+public class Document extends CloneBase implements Parent {
 
 	/**
 	 * This document's content including comments, PIs, a possible
@@ -672,13 +672,7 @@ public final int hashCode() {
 	 */
 	@Override
 	public Document clone() {
-		Document doc = null;
-
-		try {
-			doc = (Document) super.clone();
-		} catch (CloneNotSupportedException ce) {
-			// Can't happen
-		}
+		final Document doc = (Document) super.clone();
 
 		// The clone has a reference to this object's content list, so
 		// owerwrite with a empty list

core/src/java/org/jdom2/ProcessingInstruction.java

@@ -469,9 +469,7 @@ public ProcessingInstruction clone() {
 		// Object.clone()
 
 		// Create a new Map object for the clone (since Map isn't Cloneable)
-		if (mapData != null) {
-			pi.mapData = parseData(rawData);
-		}
+		pi.mapData = parseData(rawData);
 		return pi;
 	}
 

core/src/java/org/jdom2/SlimJDOMFactory.java

@@ -92,6 +92,12 @@ public SlimJDOMFactory(boolean cachetext) {
 	}
 
 
+	/**
+	 * Reset any Cached String instance data from this SlimJDOMFaxctory cache.
+	 */
+	public void clearCache() {
+		cache = new StringBin();
+	}
 
 	@Override
 	public Attribute attribute(String name, String value, Namespace namespace) {

core/src/java/org/jdom2/filter/PassThroughFilter.java

@@ -79,7 +79,7 @@ public List<Object> filter(List<?> content) {
 		for (Iterator<?> it = content.iterator(); it.hasNext(); ) {
 			ret.add(it.next());
 		}
-		return Collections.unmodifiableList(content);
+		return Collections.unmodifiableList(ret);
 	}
 
 }

core/src/java/org/jdom2/input/sax/SAXHandler.java

@@ -212,6 +212,7 @@ public final void reset() {
 		externalEntities.clear();
 		ignoringWhite = false;
 		ignoringBoundaryWhite = false;
+		resetSubCLass();
 	}
 
 	/**
@@ -295,7 +296,7 @@ public void setIgnoringElementContentWhitespace(boolean ignoringWhite) {
 	}
 
 	/**
-	 * Specifies whether or not the parser should elminate text() nodes
+	 * Specifies whether or not the parser should eliminate text() nodes
 	 * containing only whitespace when building the document. See
 	 * {@link SAXBuilder#setIgnoringBoundaryWhitespace(boolean)}.
 	 * 

test/src/java/org/jdom2/TestJDOMRuntimeExceptn.java

@@ -0,0 +1,31 @@
+package org.jdom2;
+
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+@SuppressWarnings("javadoc")
+public class TestJDOMRuntimeExceptn {
+
+	@Test
+	public void testJDOMRuntimeException() {
+		assertTrue (null != new JDOMRuntimeException().getMessage());
+		assertEquals (null, new JDOMRuntimeException().getCause());
+
+	}
+
+	@Test
+	public void testJDOMRuntimeExceptionString() {
+		assertEquals ("reason", new JDOMRuntimeException("reason").getMessage());
+
+	}
+
+	@Test
+	public void testJDOMRuntimeExceptionStringExn() {
+		Throwable t = new Throwable();
+		Exception e = new JDOMRuntimeException("reason", t);
+		assertTrue (t == e.getCause());
+		assertEquals("reason", e.getMessage());
+	}
+
+}

test/src/java/org/jdom2/test/cases/TestElement.java

@@ -2111,7 +2111,7 @@ public void testGetNamespace() {
 		assertTrue(emt.getNamespace(null) == null);
 		assertTrue(emt.getNamespace("none") == null);
 		assertTrue(emt.getNamespace("xml") == Namespace.XML_NAMESPACE);
-		assertTrue(emt.getNamespace("nsa") == nsa);
+		assertTrue(emt.getNamespace("tstada") == nsa);
 	}
 
 	@Test

test/src/java/org/jdom2/test/cases/TestSlimJDOMFactory.java

@@ -1,7 +1,11 @@
 package org.jdom2.test.cases;
 
+import static org.junit.Assert.*;
+import org.junit.Test;
+
 import org.jdom2.JDOMFactory;
 import org.jdom2.SlimJDOMFactory;
+import org.jdom2.Text;
 
 @SuppressWarnings("javadoc")
 public class TestSlimJDOMFactory extends AbstractTestJDOMFactory {
@@ -10,5 +14,27 @@ public class TestSlimJDOMFactory extends AbstractTestJDOMFactory {
 	protected JDOMFactory buildFactory() {
 		return new SlimJDOMFactory();
 	}
-
+
+	@Test
+	public void testCaching() {
+		SlimJDOMFactory fac = new SlimJDOMFactory();
+		Text ta = fac.text("hi");
+		String hi = ta.getText();
+		// we expect the StringBin to compact a string value... should no longer
+		// be the intern value.
+		assertTrue("hi" != hi);
+		assertTrue("hi" == hi.intern());
+
+		Text tb = fac.text("hi");
+		assertTrue("hi" != tb.getText());
+		assertTrue(hi == tb.getText());
+
+		fac.clearCache();
+
+		Text tc = fac.text("hi");
+		assertTrue("hi" != tc.getText());
+		assertTrue(hi != tc.getText());
+
+		assertTrue(hi.equals(tc.getText()));
+	}
 }

test/src/java/org/jdom2/test/cases/TestSlimJDOMFactoryNoText.java

@@ -0,0 +1,40 @@
+package org.jdom2.test.cases;
+
+import static org.junit.Assert.*;
+import org.junit.Test;
+
+import org.jdom2.JDOMFactory;
+import org.jdom2.SlimJDOMFactory;
+import org.jdom2.Text;
+
+@SuppressWarnings("javadoc")
+public class TestSlimJDOMFactoryNoText extends AbstractTestJDOMFactory {
+
+	@Override
+	protected JDOMFactory buildFactory() {
+		return new SlimJDOMFactory(false);
+	}
+
+	@Test
+	public void testCaching() {
+		SlimJDOMFactory fac = new SlimJDOMFactory(false);
+		Text ta = fac.text("hi");
+		String hi = ta.getText();
+		// we expect the StringBin to compact a string value... should no longer
+		// be the intern value.
+		assertTrue("hi" == hi);
+		assertTrue("hi" == hi.intern());
+
+		Text tb = fac.text("hi");
+		assertTrue("hi" == tb.getText());
+		assertTrue(hi == tb.getText());
+
+		fac.clearCache();
+
+		Text tc = fac.text("hi");
+		assertTrue("hi" == tc.getText());
+		assertTrue(hi == tc.getText());
+
+		assertTrue(hi.equals(tc.getText()));
+	}
+}

test/src/java/org/jdom2/test/cases/filter/AbstractTestFilter.java

@@ -7,7 +7,9 @@
 
 import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.LinkedList;
 import java.util.List;
+import java.util.RandomAccess;
 
 import org.jdom2.Attribute;
 import org.jdom2.CDATA;
@@ -294,22 +296,34 @@ protected <F extends Content> void exercise(Filter<F> af, Parent parent, CallBac
 	private final void exerciseCore(Filter<? extends Content> ef, Parent parent, CallBack callback) {
 		// exercise the toString()
 		assertTrue(ef.toString() != null);
+		LinkedList<Content> oc = new LinkedList<Content>();
+		ArrayList<Content> mc = new ArrayList<Content>();
 		List<Content> cont = parent.getContent();
 		for (Content c : cont) {
+			oc.add(c);
 			assertTrue(parent == c.getParent());
 			if (parent instanceof Document) {
 				assertTrue(null == c.getParentElement());
 			} else {
 				assertTrue(parent == c.getParentElement());
 			}
 			boolean mat = ef.matches(c);
+			if (mat) {
+				mc.add(c);
+			}
 			boolean cbv = callback.isValid(c);
 			if (mat != cbv) {
 				fail ("Filter " + ef + " returned " + mat 
 						+ " but isValid CallBack returned " + cbv 
 						+ " for value " + c);
 			}
 		}
+		List<?> fc = ef.filter(oc);
+		assertTrue(fc instanceof RandomAccess);
+		assertTrue(fc.size() == mc.size());
+		for (int i = 0; i < fc.size(); i++) {
+			assertTrue(fc.get(i) == mc.get(i));
+		}
 		Filter<? extends Content> cf = UnitTestUtil.deSerialize(ef);
 		assertFilterEquals(cf, ef);
 		ContentFilter xf = new ContentFilter();

test/src/java/org/jdom2/test/cases/filter/TestFilters.java

@@ -9,6 +9,7 @@
 import org.jdom2.CDATA;
 import org.jdom2.Comment;
 import org.jdom2.DocType;
+import org.jdom2.Document;
 import org.jdom2.Element;
 import org.jdom2.EntityRef;
 import org.jdom2.Namespace;
@@ -50,6 +51,11 @@ public void testContent() {
 		checkFilter(Filters.content(), new Element("tag"), null);
 	}
 
+	@Test
+	public void testDocument() {
+		checkFilter(Filters.document(), new Document(), new Object());
+	}
+
 	@Test
 	public void testAttribute() {
 		checkFilter(Filters.attribute(), new Attribute("tag", "val"), new Object());