You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@cocoon.apache.org by vg...@apache.org on 2005/04/20 04:52:10 UTC
svn commit: r162003 -
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/modules/input/XMLFileModule.java
Author: vgritsenko
Date: Tue Apr 19 19:52:09 2005
New Revision: 162003
URL: http://svn.apache.org/viewcvs?view=rev&rev=162003
Log:
fix bug: expressionValuesCache was not populated.
remove unnecessary synchronization.
Modified:
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/modules/input/XMLFileModule.java
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/modules/input/XMLFileModule.java
URL: http://svn.apache.org/viewcvs/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/modules/input/XMLFileModule.java?view=diff&r1=162002&r2=162003
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/modules/input/XMLFileModule.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/modules/input/XMLFileModule.java Tue Apr 19 19:52:09 2005
@@ -85,18 +85,24 @@
* @author <a href="mailto:haul@apache.org">Christian Haul</a>
* @version $Id$
*/
-public class XMLFileModule extends AbstractJXPathModule implements Composable, ThreadSafe {
+public class XMLFileModule extends AbstractJXPathModule
+ implements Composable, ThreadSafe {
/** Static (cocoon.xconf) configuration location, for error reporting */
String staticConfLocation;
+
/** Cached documents */
- Map documents = null;
- /** Default value for reloadability of sources */
+ Map documents;
+
+ /** Default value for reloadability of sources. Defaults to false. */
boolean reloadAll;
- /** Default value for cachability of sources */
- boolean cacheAll = true;
- /** Default value for cachability of xpath expressions. */
+
+ /** Default value for cachability of sources. Defaults to true. */
+ boolean cacheAll;
+
+ /** Default value for cachability of xpath expressions. Defaults to true. */
boolean cacheExpressions = true;
+
/** Default src */
String src;
@@ -106,23 +112,30 @@
//
// need two caches for Object and Object[]
//
+
/** XPath expression cache for single attribute values. */
private Map expressionCache;
+
/** XPath expression cache for multiple attribute values. */
private Map expressionValuesCache;
+
/**
* Takes care of (re-)loading and caching of sources.
*/
protected static class DocumentHelper {
private boolean reloadable;
private boolean cacheable;
+
/** Source location */
private String uri;
+
/** Source validity */
private SourceValidity validity;
+
/** Source content cached as DOM Document */
private Document document;
+
/** Remember who created us (and who's caching us) */
private XMLFileModule instance;
@@ -188,8 +201,7 @@
* clear everything for each configured document.
* (this is a quick fix, no time to do the whole)
*/
- this.instance.expressionCache.clear();
- this.instance.expressionValuesCache.clear();
+ this.instance.flushCache();
}
}
}
@@ -217,7 +229,6 @@
}
-
/**
* Set the current <code>ComponentManager</code> instance used by this
* <code>Composable</code>.
@@ -227,16 +238,20 @@
this.resolver = (SourceResolver) manager.lookup(SourceResolver.ROLE);
}
- /* (non-Javadoc)
- * @see org.apache.avalon.framework.activity.Disposable#dispose()
- */
+ /**
+ * Dispose this component
+ */
public void dispose() {
super.dispose();
if (this.manager != null) {
this.manager.release(this.resolver);
- this.manager = null;
this.resolver = null;
+ this.manager = null;
}
+
+ this.documents = null;
+ this.expressionCache = null;
+ this.expressionValuesCache = null;
}
/**
@@ -258,21 +273,20 @@
*/
public void configure(Configuration config)
throws ConfigurationException {
- this.staticConfLocation = config.getLocation();
super.configure(config);
- this.reloadAll = config.getChild("reloadable").getValueAsBoolean(this.reloadAll);
+ this.staticConfLocation = config.getLocation();
+ this.reloadAll = config.getChild("reloadable").getValueAsBoolean(false);
+
if (config.getChild("cachable", false) != null) {
throw new ConfigurationException("Bzzt! Wrong spelling at " +
config.getChild("cachable").getLocation() +
": please use 'cacheable', not 'cachable'");
}
- this.cacheAll = config.getChild("cacheable").getValueAsBoolean(this.cacheAll);
+ this.cacheAll = config.getChild("cacheable").getValueAsBoolean(true);
- Configuration[] files = config.getChildren("file");
- if (this.documents == null) {
- this.documents = Collections.synchronizedMap(new HashMap());
- }
+ this.documents = Collections.synchronizedMap(new HashMap());
+ Configuration[] files = config.getChildren("file");
for (int i = 0; i < files.length; i++) {
boolean reload = files[i].getAttributeAsBoolean("reloadable", this.reloadAll);
boolean cache = files[i].getAttributeAsBoolean("cacheable", this.cacheAll);
@@ -284,30 +298,15 @@
}
// init caches
- this.expressionCache = Collections.synchronizedMap(new ReferenceMap(AbstractReferenceMap.SOFT, AbstractReferenceMap.SOFT));
- this.expressionValuesCache =
- Collections.synchronizedMap(new ReferenceMap(AbstractReferenceMap.SOFT, AbstractReferenceMap.SOFT));
+ this.expressionCache = new ReferenceMap(AbstractReferenceMap.SOFT, AbstractReferenceMap.SOFT);
+ this.expressionValuesCache = new ReferenceMap(AbstractReferenceMap.SOFT, AbstractReferenceMap.SOFT);
}
/**
- * Get the DOM object that JXPath will operate on when evaluating
- * attributes. This DOM is loaded from a Source, specified in the
- * modeConf, or (if modeConf is null) from the
- * {@link #configure(Configuration)}.
- * @param modeConf The dynamic configuration for the current operation. May
- * be <code>null</code>, in which case static (cocoon.xconf) configuration
- * is used. Configuration is expected to have a <file> child node, and
- * be of the form:
- * <...>
- * <file src="..." reloadable="true|false"/>
- * </...>
- * @param objectModel Object Model for the current module operation.
+ * Retrieve document helper
*/
- protected Object getContextObject(Configuration modeConf, Map objectModel)
+ private DocumentHelper getDocumentHelper(Configuration modeConf)
throws ConfigurationException {
- String src = this.src;
- boolean reload = this.reloadAll;
- boolean cache = this.cacheAll;
boolean hasDynamicConf = false; // whether we have a <file src="..."> dynamic configuration
Configuration fileConf = null; // the nested <file>, if any
@@ -322,14 +321,11 @@
}
}
+ String src = this.src;
if (hasDynamicConf) {
src = fileConf.getAttribute("src");
}
- if (this.documents == null) {
- this.documents = Collections.synchronizedMap(new HashMap());
- }
-
if (src == null) {
throw new ConfigurationException(
"No source specified"
@@ -339,6 +335,8 @@
}
if (!this.documents.containsKey(src)) {
+ boolean reload = this.reloadAll;
+ boolean cache = this.cacheAll;
if (hasDynamicConf) {
reload = fileConf.getAttributeAsBoolean("reloadable", reload);
cache = fileConf.getAttributeAsBoolean("cacheable", cache);
@@ -348,13 +346,34 @@
+ fileConf.getLocation()
+ ": please use 'cacheable', not 'cachable'");
}
-
}
+
this.documents.put(src, new DocumentHelper(reload, cache, src, this));
}
+ return (DocumentHelper) this.documents.get(src);
+ }
+
+ /**
+ * Get the DOM object that JXPath will operate on when evaluating
+ * attributes. This DOM is loaded from a Source, specified in the
+ * modeConf, or (if modeConf is null) from the
+ * {@link #configure(Configuration)}.
+ * @param modeConf The dynamic configuration for the current operation. May
+ * be <code>null</code>, in which case static (cocoon.xconf) configuration
+ * is used. Configuration is expected to have a <file> child node, and
+ * be of the form:
+ * <...>
+ * <file src="..." reloadable="true|false"/>
+ * </...>
+ * @param objectModel Object Model for the current module operation.
+ */
+ protected Object getContextObject(Configuration modeConf, Map objectModel)
+ throws ConfigurationException {
+ DocumentHelper helper = getDocumentHelper(modeConf);
+
try {
- return ((DocumentHelper) this.documents.get(src)).getDocument(this.manager, this.resolver, getLogger());
+ return helper.getDocument(this.manager, this.resolver, getLogger());
} catch (Exception e) {
if (getLogger().isDebugEnabled()) {
getLogger().debug("Error using source " + src + "\n" + e.getMessage(), e);
@@ -365,17 +384,16 @@
public Object getAttribute(String name, Configuration modeConf, Map objectModel)
throws ConfigurationException {
- return this.getAttribute(name, modeConf, objectModel, false);
+ return getAttribute(name, modeConf, objectModel, false);
}
public Object[] getAttributeValues(String name, Configuration modeConf, Map objectModel)
throws ConfigurationException {
- Object result = this.getAttribute(name, modeConf, objectModel, true);
+ Object result = getAttribute(name, modeConf, objectModel, true);
return (result != null ? (Object[]) result : null);
}
-
- public Object getAttribute(String name, Configuration modeConf, Map objectModel, boolean getValues)
+ private Object getAttribute(String name, Configuration modeConf, Map objectModel, boolean getValues)
throws ConfigurationException {
Object contextObj = getContextObject(modeConf, objectModel);
if (modeConf != null) {
@@ -386,11 +404,7 @@
Map cache = null;
boolean hasBeenCached = false;
if (this.cacheExpressions) {
- if (getValues){
- cache = this.getValuesCache(contextObj);
- } else {
- cache = this.getCache(contextObj);
- }
+ cache = getExpressionCache(getValues? this.expressionValuesCache: this.expressionCache, contextObj);
hasBeenCached = cache.containsKey(name);
if (hasBeenCached) {
result = cache.get(name);
@@ -422,32 +436,23 @@
return result;
}
-
- private Map getCache(Object key) {
- Map cache = (Map) this.expressionCache.get(key);
- if (cache == null) {
- cache = this.initSubExpressionCache(key);
+ protected void flushCache() {
+ synchronized(this.expressionCache) {
+ this.expressionCache.clear();
}
- return cache;
- }
-
- private Map getValuesCache(Object key) {
- Map cache = (Map) this.expressionValuesCache.get(key);
- if (cache == null) {
- cache = this.initSubExpressionCache(key);
+ synchronized(this.expressionValuesCache) {
+ this.expressionValuesCache.clear();
}
- return cache;
}
- private synchronized Map initSubExpressionCache(Object key) {
- // check whether some other thread did all the work while we were waiting
- Map cache = (Map) this.expressionCache.get(key);
- if (cache == null) {
- // need to use HashMap here b/c we need to be able to store nulls
- // which are prohibited for the ReferenceMap
- cache = Collections.synchronizedMap(new HashMap());
- this.expressionCache.put(key, cache);
+ private Map getExpressionCache(Map cache, Object key) {
+ synchronized (cache) {
+ Map map = (Map) cache.get(key);
+ if (map == null) {
+ map = Collections.synchronizedMap(new HashMap());
+ cache.put(key, map);
+ }
+ return map;
}
- return cache;
}
}