[
https://issues.apache.org/jira/browse/ODFTOOLKIT-302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13201698#comment-13201698
]
Svante Schubert commented on ODFTOOLKIT-302:
--------------------------------------------
Hi guys,
I have been travelling end of last week, therefore I was just able to review
the patch in detail today as I needed some time to work myself into the context
of the XPath implementation (especially as JDK7 did a 'fix' in this area,
which caused error messages/confusion - more details below).
Ashok was completely right about the problem his fix was quite good, still I
fear he added a small side effect.
As JavaDoc of the fixed/changed method, the method should return an empty
iterator for prefixes, if the namespaceURI was not found in the XML file.
Frankly, spoken this was working before, but the new line
mDuplicatePrefixesByUri.put(namespaceURI, prefixes);
alerted me to change the map (state) during a get-method call.
Therefore I would have reopened the issue and added a new patch, but this is
not an available option in JIRA to me - either I am missing rights or JIRA
expects a different handling, therefore some notes inline:
Changed in OdfFileDom.java:
@@ -509,9 +509,11 @@
Set<String> prefixes =
mDuplicatePrefixesByUri.get(namespaceURI);
if (prefixes == null) {
prefixes = new HashSet<String>();
- mDuplicatePrefixesByUri.put(namespaceURI, prefixes);
}
- prefixes.add(mPrefixByUri.get(namespaceURI));
+ String givenPrefix = mPrefixByUri.get(namespaceURI);
+ if(givenPrefix != null){
+ prefixes.add(givenPrefix);
+ }
return prefixes.iterator();
I have added a test for the empty iterator and fixed a JavaDoc issue with of a
renamed test file:
Changed in XPathTest.java:
@@ -45,7 +45,7 @@
private static final String SOURCE_FILE_3 =
"XPathTest-duplicate-prefix.odt";
/**
- * 1) The first test document "slideDeckWithTwoSlides.odp" uses the
prefix "daisy" instead of "office" for ODF XML elements.
+ * 1) The first test document "XPathTest-foreignPrefix.odp" uses the
prefix "daisy" instead of "office" for ODF XML elements.
<daisy:document-content
xmlns:daisy="urn:oasis:names:tc:opendocument:xmlns:office:1.0"
xmlns:style="ur...
<daisy:scripts/>
<daisy:automatic-styles>
@@ -131,6 +131,10 @@
String alienElementValue = (String)
xpath.evaluate("//text:p/test", node, XPathConstants.STRING);
LOG.log(Level.INFO, "The value of the alien element is
{0}, expected is ''good''!", alienElementValue);
Assert.assertEquals("good", alienElementValue);
+
+ // Test if an empty iterator is being returned for a
none existing URL
+ Iterator<String> prefixes3 =
contentDom.getPrefixes("urn://this-prefix-does-not-exist-in-the-xml");
+ Assert.assertFalse("Not used prefix returned a
none-empty iterator!", prefixes3.hasNext());
} catch (Exception e) {
Finally, JDK7 changed the TreeSet implementation, see
http://www.oracle.com/technetwork/java/javase/jdk7-relnotes-418459.html
"Area: API: Utilities
Synopsis: Due to an error in java.util.TreeMap, it was previously possible to
insert invalid null elements and elements not implementing Comparable into
empty TreeMaps and TreeSets. Only a single invalid element could be inserted
into the empty TreeMaps or TreeSets; additional elements would cause the
expected NullPointerException or ClassCastException. Most other operations upon
the collection would also fail. As of JDK 7, inserting an invalid null element
or an element not implementing Comparable into an empty TreeMap or TreeSet
throws a NullPointerException.
RFE: 5045147 - http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5045147
>From my current view, we have used with OdfStyleFamily a TreeSet, which
>implements a Comparator that does allow NULL, so I can not follow their
>argumentation, as Set in general do allow NULL, but I will not pick a battle
>here and decided to do a work around for OdfStyleFamily as it happened in a
>private constructor:
private OdfStyleFamily(String name, OdfStyleProperty[] props), so I changed the
constructor and added a second one.
In addition, I followed advises of Netbeans 7.1 (e.g. added brackets, etc.).
Changed in OdfStyleFamily.java:
@@ -26,31 +26,13 @@
*/
package org.odftoolkit.odfdom.dom.style;
-import org.odftoolkit.odfdom.dom.attribute.style.StyleFamilyAttribute;
+import java.util.*;
+import org.odftoolkit.odfdom.dom.element.style.*;
import org.odftoolkit.odfdom.dom.style.props.OdfStyleProperty;
-import java.util.Set;
-import java.util.TreeSet;
-import java.util.Map;
-import java.util.HashMap;
-import org.odftoolkit.odfdom.dom.element.style.StyleChartPropertiesElement;
-import
org.odftoolkit.odfdom.dom.element.style.StyleDrawingPagePropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleGraphicPropertiesElement;
-import
org.odftoolkit.odfdom.dom.element.style.StyleHeaderFooterPropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleListLevelPropertiesElement;
-import
org.odftoolkit.odfdom.dom.element.style.StylePageLayoutPropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleParagraphPropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleRubyPropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleSectionPropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleTableCellPropertiesElement;
-import
org.odftoolkit.odfdom.dom.element.style.StyleTableColumnPropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleTablePropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleTableRowPropertiesElement;
-import org.odftoolkit.odfdom.dom.element.style.StyleTextPropertiesElement;
public class OdfStyleFamily implements Comparable<OdfStyleFamily> {
private String m_name;
-// private Class m_styleClass;
private Set<OdfStyleProperty> m_properties = new
TreeSet<OdfStyleProperty>();
private static Map<String, OdfStyleFamily> m_familyByName = new
HashMap<String, OdfStyleFamily>();
@@ -58,12 +40,14 @@
return m_familyByName.get(name);
}
- private OdfStyleFamily(String name /*, Class styleClass*/,
OdfStyleProperty[] props) {
+ private OdfStyleFamily(String name, OdfStyleProperty[] props) {
m_name = name;
-// m_styleClass = styleClass;
- for (OdfStyleProperty prop : props) {
- m_properties.add(prop);
+ m_properties.addAll(Arrays.asList(props));
+ m_familyByName.put(name, this);
}
+
+ private OdfStyleFamily(String name) {
+ m_name = name;
m_familyByName.put(name, this);
}
@@ -77,18 +61,20 @@
public static OdfStyleFamily valueOf(String name) {
OdfStyleFamily family = getByName(name);
- if (family == null)
- family = new OdfStyleFamily(name, new
OdfStyleProperty[]{ null });
-
+ if (family == null) {
+ family = new OdfStyleFamily(name);
+ }
return family;
}
public static String toString(OdfStyleFamily family) {
- if (family != null)
+ if (family != null) {
return family.toString();
- else
+ }
+ else {
return new String();
}
+ }
@Override
public String toString() {
@@ -96,7 +82,7 @@
}
public Set<OdfStyleProperty> getProperties() {
- return m_properties;
+ return Collections.unmodifiableSet(m_properties);
}
public final static OdfStyleFamily Chart = new OdfStyleFamily("chart",
Works afterwards with JDK6 and JDK7 (after the previous pom.xml fix from the
list had been applied).
My question is now, where do I add the patch for the changes? Do we have to
clone this issue first?
Thanks,
Svante
> getNamespaceURI() implementation in ODFDOM is not aware of duplicate
> namespace prefixes
> ---------------------------------------------------------------------------------------
>
> Key: ODFTOOLKIT-302
> URL: https://issues.apache.org/jira/browse/ODFTOOLKIT-302
> Project: ODF Toolkit
> Issue Type: Bug
> Components: odfdom, unittest
> Affects Versions: 0.8.8
> Environment: Ubuntu 11.04 64 bit, Intel i5, OpenJDK 1.6.0_20
> Reporter: Ashok Hariharan
> Assignee: Devin Han
> Labels: java, odf, patch, test, test-patch
> Fix For: 0.8.8
>
> Attachments: XPathTest-duplicate-prefix.odt, odffiledom-patch.txt,
> xpathtest-case-patch.txt
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> The getNamespaceURI() implementation in OdfFileDom is not aware of duplicate
> namespace prefixes. It is aware only of the default namespace prefixes,
> despite there being support for duplicate namespace prefixes in the
> implementation of OdfFileDom.
> I am submitting a patched test case "xpathtest-case-patch.txt" and the
> corresponding odt file used by the test.
> To simulate the problem --
> 1) apply the patch xpathtest-case-patch.txt on the ODFDOM source
> 2) put the XPathTest-duplicate-prefix.odt in odfdom/src/test/resources
> 3) Run the src/test/java/org/odftoolkit/odfdom/dom/XPathTest.java unit test.
> It fails.
> 4) apply the patch odffiledom-patch.txt on OdfFileDom and run the unit test
> again. It passes.
> I have added explanatory comments in the patched unit test and the patched
> getNamespaceURI().
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
