You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hop.apache.org by ha...@apache.org on 2020/11/07 09:53:41 UTC
[incubator-hop] branch master updated: HOP-2138 (#351)
This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-hop.git
The following commit(s) were added to refs/heads/master by this push:
new f463de9 HOP-2138 (#351)
f463de9 is described below
commit f463de988249d1b6ad19ce927dd9e142c68ad0c4
Author: Nicolas Adment <39...@users.noreply.github.com>
AuthorDate: Sat Nov 7 10:53:35 2020 +0100
HOP-2138 (#351)
- XmlHandler fix potential NPE + coode smell
- XmlParserFactoryProducer add private constructor
- XmlHandlerCache use AtomicInteger for cache hits
---
.../java/org/apache/hop/core/xml/XmlHandler.java | 68 ++++++++--------------
.../org/apache/hop/core/xml/XmlHandlerCache.java | 13 +++--
.../hop/core/xml/XmlParserFactoryProducer.java | 4 ++
3 files changed, 34 insertions(+), 51 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/xml/XmlHandler.java b/core/src/main/java/org/apache/hop/core/xml/XmlHandler.java
index b01bab0..07226bb 100644
--- a/core/src/main/java/org/apache/hop/core/xml/XmlHandler.java
+++ b/core/src/main/java/org/apache/hop/core/xml/XmlHandler.java
@@ -184,21 +184,19 @@ public class XmlHandler {
* @return The string of the subtag or null if nothing was found.
*/
public static String getTagValue( Node n, String tag, String subtag ) {
- NodeList children, tags;
- Node childnode, tagnode;
if ( n == null ) {
return null;
}
- children = n.getChildNodes();
+ NodeList children = n.getChildNodes();
for ( int i = 0; i < children.getLength(); i++ ) {
- childnode = children.item( i );
+ Node childnode = children.item( i );
if ( childnode.getNodeName().equalsIgnoreCase( tag ) ) {
// <file>
- tags = childnode.getChildNodes();
+ NodeList tags = childnode.getChildNodes();
for ( int j = 0; j < tags.getLength(); j++ ) {
- tagnode = tags.item( j );
+ Node tagnode = tags.item( j );
if ( tagnode.getNodeName().equalsIgnoreCase( subtag ) ) {
if ( tagnode.getFirstChild() != null ) {
return tagnode.getFirstChild().getNodeValue();
@@ -218,18 +216,15 @@ public class XmlHandler {
* @return The number of nodes found with a certain tag
*/
public static int countNodes( Node n, String tag ) {
- NodeList children;
- Node childnode;
-
- int count = 0;
if ( n == null ) {
return 0;
}
- children = n.getChildNodes();
+ int count = 0;
+ NodeList children = n.getChildNodes();
for ( int i = 0; i < children.getLength(); i++ ) {
- childnode = children.item( i );
+ Node childnode = children.item( i );
if ( childnode.getNodeName().equalsIgnoreCase( tag ) ) {
// <file>
count++;
@@ -246,18 +241,15 @@ public class XmlHandler {
* @return The list of nodes found with the specified tag
*/
public static List<Node> getNodes( Node n, String tag ) {
- NodeList children;
- Node childnode;
-
- List<Node> nodes = new ArrayList<Node>();
+ List<Node> nodes = new ArrayList<>();
if ( n == null ) {
return nodes;
}
- children = n.getChildNodes();
+ NodeList children = n.getChildNodes();
for ( int i = 0; i < children.getLength(); i++ ) {
- childnode = children.item( i );
+ Node childnode = children.item( i );
if ( childnode.getNodeName().equalsIgnoreCase( tag ) ) {
// <file>
nodes.add( childnode );
@@ -277,20 +269,15 @@ public class XmlHandler {
* @return The node found or null if we couldn't find anything.
*/
public static Node getNodeWithTagValue( Node n, String tag, String subtag, String subtagvalue, int nr ) {
- NodeList children;
- Node childnode, tagnode;
- String value;
-
int count = 0;
-
- children = n.getChildNodes();
+ NodeList children = n.getChildNodes();
for ( int i = 0; i < children.getLength(); i++ ) {
- childnode = children.item( i );
+ Node childnode = children.item( i );
if ( childnode.getNodeName().equalsIgnoreCase( tag ) ) {
// <hop>
- tagnode = getSubNode( childnode, subtag );
- value = getNodeValue( tagnode );
- if ( value.equalsIgnoreCase( subtagvalue ) ) {
+ Node tagnode = getSubNode( childnode, subtag );
+ String value = getNodeValue( tagnode );
+ if ( value!=null && value.equalsIgnoreCase( subtagvalue ) ) {
if ( count == nr ) {
return childnode;
}
@@ -312,12 +299,9 @@ public class XmlHandler {
*/
public static Node getNodeWithAttributeValue( Node n, String tag, String attributeName,
String attributeValue ) {
- NodeList children;
- Node childnode;
-
- children = n.getChildNodes();
+ NodeList children = n.getChildNodes();
for ( int i = 0; i < children.getLength(); i++ ) {
- childnode = children.item( i );
+ Node childnode = children.item( i );
if ( childnode.getNodeName().equalsIgnoreCase( tag ) ) {
// <hop>
Node attribute = childnode.getAttributes().getNamedItem( attributeName );
@@ -338,20 +322,17 @@ public class XmlHandler {
* @return The subnode if the tag was found, or null if nothing was found.
*/
public static Node getSubNode( Node n, String tag ) {
- int i;
- NodeList children;
- Node childnode;
- if ( n == null ) {
+ if ( n == null ) {
return null;
}
// Get the children one by one out of the node,
// compare the tags and return the first found.
//
- children = n.getChildNodes();
- for ( i = 0; i < children.getLength(); i++ ) {
- childnode = children.item( i );
+ NodeList children = n.getChildNodes();
+ for ( int i = 0; i < children.getLength(); i++ ) {
+ Node childnode = children.item( i );
if ( childnode.getNodeName().equalsIgnoreCase( tag ) ) {
return childnode;
}
@@ -423,16 +404,13 @@ public class XmlHandler {
* @return The subnode found or null in case the position was invalid.
*/
public static Node getSubNodeByNr( Node n, String tag, int nr, boolean useCache ) {
- NodeList children;
- Node childnode;
-
if ( n == null ) {
return null;
}
int count = 0;
// Find the child-nodes of this Node n:
- children = n.getChildNodes();
+ NodeList children = n.getChildNodes();
int lastChildNr = -1;
XMlHandlerCacheEntry entry = null;
@@ -449,7 +427,7 @@ public class XmlHandler {
}
for ( int i = lastChildNr; i < children.getLength(); i++ ) { // Try all children
- childnode = children.item( i );
+ Node childnode = children.item( i );
if ( childnode.getNodeName().equalsIgnoreCase( tag ) ) { // We found the right tag
if ( count == nr ) {
if ( useCache ) {
diff --git a/core/src/main/java/org/apache/hop/core/xml/XmlHandlerCache.java b/core/src/main/java/org/apache/hop/core/xml/XmlHandlerCache.java
index 0e21d4b..6191a16 100644
--- a/core/src/main/java/org/apache/hop/core/xml/XmlHandlerCache.java
+++ b/core/src/main/java/org/apache/hop/core/xml/XmlHandlerCache.java
@@ -26,6 +26,7 @@ package org.apache.hop.core.xml;
import java.util.Collections;
import java.util.Map;
import java.util.WeakHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Singleton to help speed up lookups in an XML DOM tree.<br>
@@ -51,16 +52,16 @@ public class XmlHandlerCache {
Map<XMlHandlerCacheEntry, Integer> cache;
- private volatile int cacheHits;
+ private AtomicInteger cacheHits;
private XmlHandlerCache() {
cache = Collections.synchronizedMap( new WeakHashMap<XMlHandlerCacheEntry, Integer>() );
- cacheHits = 0;
+ cacheHits = new AtomicInteger();
}
public static synchronized XmlHandlerCache getInstance() {
if ( instance == null ) {
- return instance = new XmlHandlerCache();
+ instance = new XmlHandlerCache();
}
return instance;
}
@@ -83,7 +84,7 @@ public class XmlHandlerCache {
public int getLastChildNr( XMlHandlerCacheEntry entry ) {
Integer lastChildNr = cache.get( entry );
if ( lastChildNr != null ) {
- cacheHits++;
+ cacheHits.incrementAndGet();
return lastChildNr;
}
return -1;
@@ -93,7 +94,7 @@ public class XmlHandlerCache {
* @return the number of cache hits for your statistical pleasure.
*/
public int getCacheHits() {
- return cacheHits;
+ return cacheHits.get();
}
/**
@@ -102,7 +103,7 @@ public class XmlHandlerCache {
* @param cacheHits the number of cache hits.
*/
public void setCacheHits( int cacheHits ) {
- this.cacheHits = cacheHits;
+ this.cacheHits.set(cacheHits);
}
/**
diff --git a/core/src/main/java/org/apache/hop/core/xml/XmlParserFactoryProducer.java b/core/src/main/java/org/apache/hop/core/xml/XmlParserFactoryProducer.java
index 53650b0..00febeb 100644
--- a/core/src/main/java/org/apache/hop/core/xml/XmlParserFactoryProducer.java
+++ b/core/src/main/java/org/apache/hop/core/xml/XmlParserFactoryProducer.java
@@ -40,6 +40,10 @@ public class XmlParserFactoryProducer {
private static final Log logger = LogFactory.getLog( XmlParserFactoryProducer.class );
+ private XmlParserFactoryProducer() {
+ // Static class
+ }
+
/**
* Creates an instance of {@link DocumentBuilderFactory} class with enabled {@link XMLConstants#FEATURE_SECURE_PROCESSING} property.
* Enabling this feature prevents from some XXE attacks (e.g. XML bomb)