Skip to content

Commit

Permalink
fix(#185): remove synchronization at all
Browse files Browse the repository at this point in the history
  • Loading branch information
@maxonfjvipon
maxonfjvipon committed Oct 29, 2024
1 parent 48854d6 commit cb8870b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 126 deletions.
152 changes: 41 additions & 111 deletions src/main/java/com/jcabi/xml/XMLDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;
import net.sf.saxon.xpath.XPathFactoryImpl;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
Expand All @@ -80,25 +81,6 @@
}
)
public final class XMLDocument implements XML {

/**
* XPath factory.
*/
private static final XPathFactory XFACTORY =
XPathFactory.newInstance();

/**
* Transformer factory.
*/
private static final TransformerFactory TFACTORY =
TransformerFactory.newInstance();

/**
* DOM document builder factory.
*/
private static final DocumentBuilderFactory DFACTORY =
DocumentBuilderFactory.newInstance();

/**
* Namespace context to use for {@link #xpath(String)}
* and {@link #nodes(String)} methods.
Expand All @@ -115,25 +97,6 @@ public final class XMLDocument implements XML {
*/
private final transient Node cache;

/**
* Transformer factory to use for {@link #toString()}.
*/
private final transient TransformerFactory tfactory;

static {
if (XMLDocument.DFACTORY.getClass().getName().contains("xerces")) {
try {
XMLDocument.DFACTORY.setFeature(
"http://apache.org/xml/features/nonvalidating/load-external-dtd",
false
);
} catch (final ParserConfigurationException ex) {
throw new IllegalStateException(ex);
}
}
XMLDocument.DFACTORY.setNamespaceAware(true);
}

/**
* Public ctor, from XML as a text.
*
Expand All @@ -154,40 +117,12 @@ public final class XMLDocument implements XML {
*/
public XMLDocument(final String text) {
this(
new DomParser(XMLDocument.DFACTORY, text).document(),
new DomParser(XMLDocument.configuredDFactory(), text).document(),
new XPathContext(),
false
);
}

/**
* Public ctor, from XML as a text.
*
* <p>The object is created with a default implementation of
* {@link NamespaceContext}, which already defines a
* number of namespaces, for convenience, including:
*
* <pre> xhtml: http://www.w3.org/1999/xhtml
* xs: http://www.w3.org/2001/XMLSchema
* xsi: http://www.w3.org/2001/XMLSchema-instance
* xsl: http://www.w3.org/1999/XSL/Transform
* svg: http://www.w3.org/2000/svg</pre>
*
* <p>In future versions we will add more namespaces (submit a ticket if
* you need more of them defined here).
*
* @param text XML document body
* @param factory Transformer factory
*/
public XMLDocument(final String text, final TransformerFactory factory) {
this(
new DomParser(XMLDocument.DFACTORY, text).document(),
new XPathContext(),
false,
factory
);
}

/**
* Public ctor, from XML as a text.
*
Expand All @@ -208,7 +143,7 @@ public XMLDocument(final String text, final TransformerFactory factory) {
*/
public XMLDocument(final byte[] data) {
this(
new DomParser(XMLDocument.DFACTORY, data).document(),
new DomParser(XMLDocument.configuredDFactory(), data).document(),
new XPathContext(),
false
);
Expand Down Expand Up @@ -334,43 +269,26 @@ public XMLDocument(final InputStream stream) throws IOException {
stream.close();
}

/**
* Private ctor.
* @param node The source
* @param ctx Namespace context
* @param lfe Is it a leaf node?
*/
private XMLDocument(
final Node node,
final XPathContext ctx,
final boolean lfe
) {
this(node, ctx, lfe, XMLDocument.TFACTORY);
}

/**
* Private ctor.
* @param cache The source
* @param context Namespace context
* @param leaf Is it a leaf node?
* @param tfactory Transformer factory
* @checkstyle ParameterNumberCheck (5 lines)
*/
public XMLDocument(
private XMLDocument(
final Node cache,
final XPathContext context,
final boolean leaf,
final TransformerFactory tfactory
final boolean leaf
) {
this.context = context;
this.leaf = leaf;
this.cache = cache;
this.tfactory = tfactory;
}

@Override
public String toString() {
return this.asString(this.cache);
return XMLDocument.asString(this.cache);
}

@Override
Expand Down Expand Up @@ -438,7 +356,7 @@ public List<String> xpath(final String query) {
throw new IllegalArgumentException(
String.format(
"Invalid XPath query '%s' at %s: %s",
query, XMLDocument.XFACTORY.getClass().getName(),
query, XPathFactoryImpl.class.getName(),
ex.getLocalizedMessage()
),
exp
Expand Down Expand Up @@ -475,7 +393,7 @@ public List<XML> nodes(final String query) {
throw new IllegalArgumentException(
String.format(
"Invalid XPath query '%s' by %s",
query, XMLDocument.XFACTORY.getClass().getName()
query, XPathFactoryImpl.class.getName()
), ex
);
}
Expand All @@ -497,14 +415,15 @@ public XML merge(final NamespaceContext ctx) {
* @return A cloned node imported in a dedicated document.
*/
private static Node createImportedNode(final Node node) {
final DocumentBuilderFactory factory = XMLDocument.configuredDFactory();
final DocumentBuilder builder;
try {
builder = XMLDocument.DFACTORY.newDocumentBuilder();
builder = factory.newDocumentBuilder();
} catch (final ParserConfigurationException ex) {
throw new IllegalArgumentException(
String.format(
"Failed to create document builder by %s",
XMLDocument.DFACTORY.getClass().getName()
factory.getClass().getName()
),
ex
);
Expand All @@ -530,12 +449,9 @@ private static Node createImportedNode(final Node node) {
* @throws XPathExpressionException If an error occurs when evaluating XPath
*/
@SuppressWarnings("unchecked")
private <T> T fetch(final String query, final Class<T> type)
throws XPathExpressionException {
final XPath xpath;
synchronized (XMLDocument.class) {
xpath = XMLDocument.XFACTORY.newXPath();
}
private <T> T fetch(final String query, final Class<T> type) throws XPathExpressionException {
final XPathFactory factory = XPathFactory.newInstance();
final XPath xpath = factory.newXPath();
xpath.setNamespaceContext(this.context);
final QName qname;
if (type.equals(String.class)) {
Expand All @@ -559,18 +475,16 @@ private <T> T fetch(final String query, final Class<T> type)
* @param node The DOM node.
* @return String representation
*/
private String asString(final Node node) {
final StringWriter writer = new StringWriter();
private static String asString(final Node node) {
final TransformerFactory factory = TransformerFactory.newInstance();
final Transformer trans;
try {
synchronized (XMLDocument.class) {
trans = this.tfactory.newTransformer();
}
trans = factory.newTransformer();
} catch (final TransformerConfigurationException ex) {
throw new IllegalArgumentException(
String.format(
"Failed to create transformer by %s",
this.tfactory.getClass().getName()
XPathFactoryImpl.class.getName()
),
ex
);
Expand All @@ -581,11 +495,10 @@ private String asString(final Node node) {
trans.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
}
final Source source = new DOMSource(node);
final StringWriter writer = new StringWriter();
final Result result = new StreamResult(writer);
try {
synchronized (node) {
trans.transform(source, result);
}
trans.transform(source, result);
} catch (final TransformerException ex) {
throw new IllegalArgumentException(
String.format(
Expand All @@ -606,11 +519,9 @@ private String asString(final Node node) {
*/
private static Node transform(final Source source) {
final DOMResult result = new DOMResult();
final TransformerFactory factory = TransformerFactory.newInstance();
try {
final Transformer trans;
synchronized (XMLDocument.class) {
trans = XMLDocument.TFACTORY.newTransformer();
}
final Transformer trans = factory.newTransformer();
trans.transform(source, result);
} catch (final TransformerException ex) {
throw new IllegalArgumentException(
Expand All @@ -625,4 +536,23 @@ private static Node transform(final Source source) {
return result.getNode();
}

/**
* Create new {@link DocumentBuilderFactory} and configure it.
* @return Configured factory
*/
private static DocumentBuilderFactory configuredDFactory() {
final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
if (factory.getClass().getName().contains("xerces")) {
try {
factory.setFeature(
"http://apache.org/xml/features/nonvalidating/load-external-dtd",
false
);
} catch (final ParserConfigurationException ex) {
throw new IllegalStateException(ex);
}
}
factory.setNamespaceAware(true);
return factory;
}
}
12 changes: 4 additions & 8 deletions src/main/java/com/jcabi/xml/XSDDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,9 @@ public String toString() {
public Collection<SAXParseException> validate(final Source xml) {
final Schema schema;
try {
synchronized (XSDDocument.class) {
schema = SchemaFactory
.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)
.newSchema(new StreamSource(new StringReader(this.xsd)));
}
schema = SchemaFactory
.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)
.newSchema(new StreamSource(new StringReader(this.xsd)));
} catch (final SAXException ex) {
throw new IllegalStateException(
String.format("Failed to create XSD schema from %s", this.xsd),
Expand All @@ -193,9 +191,7 @@ public Collection<SAXParseException> validate(final Source xml) {
final Validator validator = schema.newValidator();
validator.setErrorHandler(new XSDDocument.ValidationHandler(errors));
try {
synchronized (XSDDocument.class) {
validator.validate(xml);
}
validator.validate(xml);
} catch (final SAXException | IOException ex) {
throw new IllegalStateException(ex);
}
Expand Down
9 changes: 2 additions & 7 deletions src/test/java/com/jcabi/xml/XMLDocumentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import javax.xml.transform.TransformerFactory;
import org.apache.commons.lang3.StringUtils;
import org.cactoos.io.ResourceOf;
import org.cactoos.io.TeeInput;
Expand Down Expand Up @@ -391,15 +390,11 @@ void comparesDocumentsWithDifferentIndentations() {
// The current implementation of XMLDocument does not ignore different indentations
// when comparing two XML documents. We need to implement a comparison that ignores
// different indentations. Don't forget to remove the @Disabled annotation from this test.
final TransformerFactory factory = TransformerFactory.newInstance(
"com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl",
Thread.currentThread().getContextClassLoader()
);
MatcherAssert.assertThat(
"Different indentations should be ignored",
new XMLDocument("<program>\n <indentation/>\n</program>", factory),
new XMLDocument("<program>\n <indentation/>\n</program>"),
Matchers.equalTo(
new XMLDocument("<program>\n <indentation/>\n</program>\n", factory)
new XMLDocument("<program>\n <indentation/>\n</program>\n")
)
);
}
Expand Down

0 comments on commit cb8870b

Please sign in to comment.