From 3a6a37d68e209653fddb3aaf121103e2abdb8e2c Mon Sep 17 00:00:00 2001 From: Radu Coravu Date: Fri, 4 Sep 2020 07:28:06 +0300 Subject: [PATCH 1/4] Fix for #3568, prefer public IDs when comparing descriptions for grammars stored in grammar cache. Signed-off-by: Radu Coravu --- .../dost/util/XMLGrammarPoolImplUtils.java | 14 ++++++- .../util/XMLGrammarPoolImplUtilsTest.java | 42 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java diff --git a/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java b/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java index 6f6b43cfc0..ca54b0a17c 100644 --- a/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java +++ b/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java @@ -68,7 +68,12 @@ public int hashCode(final XMLGrammarDescription desc) { // return -1 for XSD grammar hashcode because we want to disable XSD grammar caching return -1; } else { - return desc.hashCode(); + //#3568 Prefer to hash the public ID if available + if(desc.getPublicId() != null) { + return desc.getPublicId().hashCode(); + } else { + return desc.hashCode(); + } } } @@ -94,7 +99,12 @@ public boolean equals(final XMLGrammarDescription desc1, // always return false for XSD grammar to disable XSD grammar caching return false; } else { - return desc1.equals(desc2); + //#3568 Compare descriptions only by public ID if available. + if(desc1.getPublicId() != null && desc2.getPublicId() != null) { + return desc1.getPublicId().equals(desc2.getPublicId()); + } else { + return desc1.equals(desc2); + } } } diff --git a/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java b/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java new file mode 100644 index 0000000000..920996cbe4 --- /dev/null +++ b/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java @@ -0,0 +1,42 @@ +/* + * This file is part of the DITA Open Toolkit project. + * + * Copyright 2011 Jarno Elovirta + * + * See the accompanying LICENSE file for applicable license. + */ +package org.dita.dost.util; + +import static org.junit.Assert.assertEquals; + +import java.io.File; + +import org.apache.xerces.impl.dtd.DTDGrammar; +import org.apache.xerces.impl.dtd.XMLDTDDescription; +import org.apache.xerces.xni.grammars.Grammar; +import org.junit.Test; + + +public class XMLGrammarPoolImplUtilsTest { + + @Test + public void testCompareDescriptorsByPublicID() { + //#3568 We have two DITA topics located in different folders. The DOCTYPES are something like: + // + //The DTD description for each of them has a different expanded system ID because the relative system ID + //is made absolute to the XML file location + //But the grammar will still be found because we consider the public ID to have more importance when comparing the descriptions. + File base = new File("abc.xml"); + File expanded = new File("abc.dtd"); + XMLDTDDescription desc1 = new XMLDTDDescription("publicID", "topic.dtd", base.toURI().toASCIIString(), expanded.toURI().toASCIIString(), "topic"); + DTDGrammar grammar = new DTDGrammar(null, desc1); + XMLGrammarPoolImplUtils utils = new XMLGrammarPoolImplUtils(); + utils.putGrammar(grammar); + + File base2 = new File("../abc.xml"); + File expanded2 = new File("../abc.dtd"); + XMLDTDDescription desc2 = new XMLDTDDescription("publicID", "topic.dtd", base2.toURI().toASCIIString(), expanded2.toURI().toASCIIString(), "topic"); + Grammar retrieved = utils.getGrammar(desc2); + assertEquals("Should have retrieved the grammar, even though the expanded system ID differs", grammar, retrieved); + } +} From 67d069aa4ddcb60d85c3e476d6fff9885a99eb3f Mon Sep 17 00:00:00 2001 From: Jarno Elovirta Date: Tue, 8 Sep 2020 18:14:13 +0300 Subject: [PATCH 2/4] Fix indentation and remove change log comments Signed-off-by: Jarno Elovirta --- .../dost/util/XMLGrammarPoolImplUtils.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java b/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java index ca54b0a17c..fbfef28400 100644 --- a/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java +++ b/src/main/java/org/dita/dost/util/XMLGrammarPoolImplUtils.java @@ -68,12 +68,11 @@ public int hashCode(final XMLGrammarDescription desc) { // return -1 for XSD grammar hashcode because we want to disable XSD grammar caching return -1; } else { - //#3568 Prefer to hash the public ID if available - if(desc.getPublicId() != null) { - return desc.getPublicId().hashCode(); - } else { - return desc.hashCode(); - } + if (desc.getPublicId() != null) { + return desc.getPublicId().hashCode(); + } else { + return desc.hashCode(); + } } } @@ -99,12 +98,11 @@ public boolean equals(final XMLGrammarDescription desc1, // always return false for XSD grammar to disable XSD grammar caching return false; } else { - //#3568 Compare descriptions only by public ID if available. - if(desc1.getPublicId() != null && desc2.getPublicId() != null) { - return desc1.getPublicId().equals(desc2.getPublicId()); - } else { - return desc1.equals(desc2); - } + if (desc1.getPublicId() != null && desc2.getPublicId() != null) { + return desc1.getPublicId().equals(desc2.getPublicId()); + } else { + return desc1.equals(desc2); + } } } From d32699e466dc01fdccba7a56484ecaa20bdac699 Mon Sep 17 00:00:00 2001 From: Jarno Elovirta Date: Tue, 8 Sep 2020 19:35:15 +0300 Subject: [PATCH 3/4] Improve grammar pool test Signed-off-by: Jarno Elovirta --- .../util/XMLGrammarPoolImplUtilsTest.java | 81 ++++++++++++++----- 1 file changed, 61 insertions(+), 20 deletions(-) diff --git a/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java b/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java index 920996cbe4..2085489f4e 100644 --- a/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java +++ b/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java @@ -1,42 +1,83 @@ /* * This file is part of the DITA Open Toolkit project. * - * Copyright 2011 Jarno Elovirta + * Copyright 2020 Radu Coravu * * See the accompanying LICENSE file for applicable license. */ package org.dita.dost.util; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import java.io.File; import org.apache.xerces.impl.dtd.DTDGrammar; import org.apache.xerces.impl.dtd.XMLDTDDescription; import org.apache.xerces.xni.grammars.Grammar; +import org.junit.Before; import org.junit.Test; public class XMLGrammarPoolImplUtilsTest { - @Test - public void testCompareDescriptorsByPublicID() { - //#3568 We have two DITA topics located in different folders. The DOCTYPES are something like: - // - //The DTD description for each of them has a different expanded system ID because the relative system ID - //is made absolute to the XML file location - //But the grammar will still be found because we consider the public ID to have more importance when comparing the descriptions. - File base = new File("abc.xml"); - File expanded = new File("abc.dtd"); - XMLDTDDescription desc1 = new XMLDTDDescription("publicID", "topic.dtd", base.toURI().toASCIIString(), expanded.toURI().toASCIIString(), "topic"); - DTDGrammar grammar = new DTDGrammar(null, desc1); - XMLGrammarPoolImplUtils utils = new XMLGrammarPoolImplUtils(); - utils.putGrammar(grammar); - - File base2 = new File("../abc.xml"); - File expanded2 = new File("../abc.dtd"); - XMLDTDDescription desc2 = new XMLDTDDescription("publicID", "topic.dtd", base2.toURI().toASCIIString(), expanded2.toURI().toASCIIString(), "topic"); - Grammar retrieved = utils.getGrammar(desc2); - assertEquals("Should have retrieved the grammar, even though the expanded system ID differs", grammar, retrieved); + private XMLGrammarPoolImplUtils utils; + + @Before + public void setUp() { + utils = new XMLGrammarPoolImplUtils(); + } + + @Test + public void testCompareDescriptorsByPublicID_same() { + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", "publicID"); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); + + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", "publicID"); + final Grammar act = utils.getGrammar(desc2); + + assertEquals(exp, act); + } + + @Test + public void testCompareDescriptorsByPublicID_different() { + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", "publicID"); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); + + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", "differentId"); + final Grammar act = utils.getGrammar(desc2); + + assertNull(act); + } + + @Test + public void testCompareDescriptorsBySystemID_same() { + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", null); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); + + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/foo/abc.dtd", null); + final Grammar act = utils.getGrammar(desc2); + + assertEquals(exp, act); + } + + @Test + public void testCompareDescriptorsBySystemID_different() { + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", null); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); + + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", null); + final Grammar act = utils.getGrammar(desc2); + + assertNull(act); } + + private XMLDTDDescription getXmldtdDescription(String base, String systemId, String publicId) { + return new XMLDTDDescription(publicId, "topic.dtd", base, systemId, "topic"); + } + } From 6abe7e48447758f1208c04ec58c78829e25a810a Mon Sep 17 00:00:00 2001 From: Jarno Elovirta Date: Tue, 8 Sep 2020 19:36:30 +0300 Subject: [PATCH 4/4] Fix indentation Signed-off-by: Jarno Elovirta --- .../util/XMLGrammarPoolImplUtilsTest.java | 88 +++++++++---------- 1 file changed, 43 insertions(+), 45 deletions(-) diff --git a/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java b/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java index 2085489f4e..868e9ac9d3 100644 --- a/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java +++ b/src/test/java/org/dita/dost/util/XMLGrammarPoolImplUtilsTest.java @@ -7,77 +7,75 @@ */ package org.dita.dost.util; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; - -import java.io.File; - import org.apache.xerces.impl.dtd.DTDGrammar; import org.apache.xerces.impl.dtd.XMLDTDDescription; import org.apache.xerces.xni.grammars.Grammar; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + public class XMLGrammarPoolImplUtilsTest { - private XMLGrammarPoolImplUtils utils; + private XMLGrammarPoolImplUtils utils; - @Before - public void setUp() { - utils = new XMLGrammarPoolImplUtils(); - } + @Before + public void setUp() { + utils = new XMLGrammarPoolImplUtils(); + } - @Test - public void testCompareDescriptorsByPublicID_same() { - final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", "publicID"); - final DTDGrammar exp = new DTDGrammar(null, desc1); - utils.putGrammar(exp); + @Test + public void testCompareDescriptorsByPublicID_same() { + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", "publicID"); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); - final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", "publicID"); - final Grammar act = utils.getGrammar(desc2); + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", "publicID"); + final Grammar act = utils.getGrammar(desc2); - assertEquals(exp, act); - } + assertEquals(exp, act); + } - @Test + @Test public void testCompareDescriptorsByPublicID_different() { - final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", "publicID"); - final DTDGrammar exp = new DTDGrammar(null, desc1); - utils.putGrammar(exp); + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", "publicID"); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); - final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", "differentId"); - final Grammar act = utils.getGrammar(desc2); + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", "differentId"); + final Grammar act = utils.getGrammar(desc2); - assertNull(act); + assertNull(act); } - @Test - public void testCompareDescriptorsBySystemID_same() { - final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", null); - final DTDGrammar exp = new DTDGrammar(null, desc1); - utils.putGrammar(exp); + @Test + public void testCompareDescriptorsBySystemID_same() { + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", null); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); - final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/foo/abc.dtd", null); - final Grammar act = utils.getGrammar(desc2); + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/foo/abc.dtd", null); + final Grammar act = utils.getGrammar(desc2); - assertEquals(exp, act); - } + assertEquals(exp, act); + } - @Test + @Test public void testCompareDescriptorsBySystemID_different() { - final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", null); - final DTDGrammar exp = new DTDGrammar(null, desc1); - utils.putGrammar(exp); + final XMLDTDDescription desc1 = getXmldtdDescription("file:/foo/abc.xml", "file:/foo/abc.dtd", null); + final DTDGrammar exp = new DTDGrammar(null, desc1); + utils.putGrammar(exp); - final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", null); - final Grammar act = utils.getGrammar(desc2); + final XMLDTDDescription desc2 = getXmldtdDescription("file:/abc.xml", "file:/abc.dtd", null); + final Grammar act = utils.getGrammar(desc2); - assertNull(act); + assertNull(act); } - private XMLDTDDescription getXmldtdDescription(String base, String systemId, String publicId) { - return new XMLDTDDescription(publicId, "topic.dtd", base, systemId, "topic"); - } + private XMLDTDDescription getXmldtdDescription(String base, String systemId, String publicId) { + return new XMLDTDDescription(publicId, "topic.dtd", base, systemId, "topic"); + } }