Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(#185): remove synchronization at all #270

Merged
merged 1 commit into from
Oct 29, 2024
Merged

fix(#185): remove synchronization at all #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 41 additions & 111 deletions src/main/java/com/jcabi/xml/XMLDocument.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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