Author: jonesde
Date: Thu Sep 6 21:01:25 2007
New Revision: 573442
URL: http://svn.apache.org/viewvc?rev=573442&view=rev
Log:
Fixed bugs in recently refactored FreeMarkerWorker: a Reader was always being
created even if the template was cached, also the reader was never closed which
means files stayed open until the garbage collector cleaned up these objects
which on busy production systems could lead to many thousands of open files
Modified:
ofbiz/trunk/applications/content/src/org/ofbiz/content/email/NotificationServices.java
ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/template/FreeMarkerWorker.java
Modified:
ofbiz/trunk/applications/content/src/org/ofbiz/content/email/NotificationServices.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/applications/content/src/org/ofbiz/content/email/NotificationServices.java?rev=573442&r1=573441&r2=573442&view=diff
==============================================================================
---
ofbiz/trunk/applications/content/src/org/ofbiz/content/email/NotificationServices.java
(original)
+++
ofbiz/trunk/applications/content/src/org/ofbiz/content/email/NotificationServices.java
Thu Sep 6 21:01:25 2007
@@ -205,12 +205,10 @@
return ServiceUtil.returnError("Problem finding template; see
logs");
}
- InputStreamReader templateReader = new InputStreamReader(templateUrl.openStream());
-
// process the template with the given data and write
// the email body to the String buffer
Writer writer = new StringWriter();
- FreeMarkerWorker.renderTemplate(templateName, templateReader,
templateData, writer);
+ FreeMarkerWorker.renderTemplate(templateUrl.toExternalForm(),
templateData, writer);
// extract the newly created body for the notification email
String notificationBody = writer.toString();
Modified:
ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java?rev=573442&r1=573441&r2=573442&view=diff
==============================================================================
---
ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
(original)
+++
ofbiz/trunk/framework/appserver/src/org/ofbiz/appservers/GenerateContainer.java
Thu Sep 6 21:01:25 2007
@@ -29,6 +29,7 @@
import org.ofbiz.base.container.Container;
import org.ofbiz.base.container.ContainerException;
import org.ofbiz.base.util.Debug;
+import org.ofbiz.base.util.UtilURL;
import org.ofbiz.base.util.template.FreeMarkerWorker;
import org.ofbiz.base.start.Classpath;
import org.ofbiz.base.component.ComponentConfig;
@@ -141,12 +142,6 @@
private void parseTemplate(File templateFile, Map dataMap) throws ContainerException {
Debug.log("Parsing template : " + templateFile.getAbsolutePath(),
module);
- Reader reader = null;
- try {
- reader = new InputStreamReader(new FileInputStream(templateFile));
- } catch (FileNotFoundException e) {
- throw new ContainerException(e);
- }
// create the target file/directory
String targetDirectoryName = args.length > 1 ? args[1] : null;
@@ -174,7 +169,7 @@
throw new ContainerException(e);
}
try {
- FreeMarkerWorker.renderTemplate(templateFile.getAbsolutePath(),
reader, dataMap, writer);
+
FreeMarkerWorker.renderTemplate(UtilURL.fromFilename(templateFile.getAbsolutePath()).toExternalForm(),
dataMap, writer);
} catch (Exception e) {
throw new ContainerException(e);
}
Modified:
ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/template/FreeMarkerWorker.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/template/FreeMarkerWorker.java?rev=573442&r1=573441&r2=573442&view=diff
==============================================================================
---
ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/template/FreeMarkerWorker.java
(original)
+++
ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/template/FreeMarkerWorker.java
Thu Sep 6 21:01:25 2007
@@ -19,6 +19,7 @@
package org.ofbiz.base.util.template;
import java.io.IOException;
+import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.StringReader;
@@ -128,7 +129,7 @@
* @param outWriter The Writer to render to
*/
public static void renderTemplateAtLocation(String templateLocation, Map
context, Writer outWriter) throws MalformedURLException, TemplateException,
IOException {
- renderTemplate(templateLocation, getTemplateReader(templateLocation),
context, outWriter);
+ renderTemplate(templateLocation, context, outWriter);
}
/**
@@ -138,9 +139,8 @@
* @param context The context Map
* @param outWriter The Writer to render to
*/
- public static void renderTemplate(String templateId, String
templateString, Map context, Writer outWriter) throws TemplateException,
IOException {
- Reader templateReader = new StringReader(templateString);
- renderTemplate(templateId, templateReader, context, outWriter);
+ public static void renderTemplate(String templateLocation, String
templateString, Map context, Writer outWriter) throws TemplateException,
IOException {
+ renderTemplate(templateLocation, context, outWriter);
}
/**
@@ -150,8 +150,8 @@
* @param context The context Map
* @param outWriter The Writer to render to
*/
- public static void renderTemplate(String templateId, Reader
templateReader, Map context, Writer outWriter) throws TemplateException,
IOException {
- Template template = getTemplate(templateId, templateReader);
+ public static void renderTemplate(String templateLocation, Map context,
Writer outWriter) throws TemplateException, IOException {
+ Template template = getTemplate(templateLocation);
renderTemplate(template, context, outWriter);
}
@@ -220,8 +220,9 @@
}
return defaultOfbizConfig;
}
-
- public static Reader getTemplateReader(String templateLocation) throws
IOException {
+
+ /** Make sure to close the reader when you're done! That's why this method is private, BTW. */
+ private static Reader makeReader(String templateLocation) throws
IOException {
if (UtilValidate.isEmpty(templateLocation)) {
throw new IllegalArgumentException("FreeMarker template location null
or empty");
}
@@ -229,14 +230,15 @@
URL locationUrl = null;
try {
locationUrl = FlexibleLocation.resolveLocation(templateLocation);
- }
- catch (MalformedURLException e) {
+ } catch (MalformedURLException e) {
throw new IllegalArgumentException(e.getMessage());
}
if (locationUrl == null) {
throw new IllegalArgumentException("FreeMarker file not found at
location: " + templateLocation);
}
- Reader templateReader = new
InputStreamReader(locationUrl.openStream());
+
+ InputStream locationIs = locationUrl.openStream();
+ Reader templateReader = new InputStreamReader(locationIs);
String locationProtocol = locationUrl.getProtocol();
if ("file".equals(locationProtocol) && Debug.verboseOn()) {
@@ -244,35 +246,28 @@
int lastSlash = locationFile.lastIndexOf("/");
String locationDir = locationFile.substring(0, lastSlash);
String filename = locationFile.substring(lastSlash + 1);
- Debug.logVerbose("FreeMarker render: filename=" + filename + ",
locationDir=" + locationDir, module);
+ if (Debug.verboseOn()) Debug.logVerbose("FreeMarker render: filename=" +
filename + ", locationDir=" + locationDir, module);
}
return templateReader;
}
-
+
/**
* Gets a Template instance from the template cache. If the Template
instance isn't
* found in the cache, then one will be created.
* @param templateLocation Location of the template - file path or URL
*/
public static Template getTemplate(String templateLocation) throws
TemplateException, IOException {
- return getTemplate(templateLocation,
getTemplateReader(templateLocation));
- }
-
- /**
- * Gets a Template instance from the template cache. If the Template
instance isn't
- * found in the cache, then one will be created.
- * @param templateId A unique ID for this template
- * @param templateReader The Reader that reads the template
- */
- public static Template getTemplate(String templateId, Reader
templateReader) throws TemplateException, IOException {
- Template template = (Template) cachedTemplates.get(templateId);
+ Template template = (Template) cachedTemplates.get(templateLocation);
if (template == null) {
synchronized (cachedTemplates) {
- template = (Template) cachedTemplates.get(templateId);
+ template = (Template) cachedTemplates.get(templateLocation);
if (template == null) {
- template = new Template(templateId, templateReader,
getDefaultOfbizConfig());
- cachedTemplates.put(templateId, template);
+ // only make the reader if we need it, and then close it
right after!
+ Reader templateReader = makeReader(templateLocation);
+ template = new Template(templateLocation, templateReader,
getDefaultOfbizConfig());
+ templateReader.close();
+ cachedTemplates.put(templateLocation, template);
}
}
}
@@ -612,12 +607,12 @@
return new FlexibleTemplateSource(name);
}
public long getLastModified(Object templateSource) {
- FlexibleTemplateSource fts =
(FlexibleTemplateSource)templateSource;
+ FlexibleTemplateSource fts = (FlexibleTemplateSource)
templateSource;
return fts.getLastModified();
}
public Reader getReader(Object templateSource, String encoding) throws
IOException {
- FlexibleTemplateSource fts =
(FlexibleTemplateSource)templateSource;
- return getTemplateReader(fts.getTemplateLocation());
+ FlexibleTemplateSource fts = (FlexibleTemplateSource)
templateSource;
+ return makeReader(fts.getTemplateLocation());
}
public void closeTemplateSource(Object templateSource) throws
IOException {
// do nothing
