Merge pull request #44 from spekr/master

Close streams in finally clause
dfabulich · Jul 17, 2018 · 7553eba · 7553eba
2 parents 168ac4f + 12c38f6
commit 7553eba
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 52 deletions.
diff --git a/src/main/java/com/redfin/sitemapgenerator/SitemapGenerator.java b/src/main/java/com/redfin/sitemapgenerator/SitemapGenerator.java
@@ -1,18 +1,17 @@
 package com.redfin.sitemapgenerator;
 
+import org.xml.sax.SAXException;
+
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
-import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.zip.GZIPOutputStream;
 
-import org.xml.sax.SAXException;
-
 abstract class SitemapGenerator<U extends ISitemapUrl, THIS extends SitemapGenerator<U,THIS>> {
 	/** 50000 URLs per sitemap maximum */
 	public static final int MAX_URLS_PER_SITEMAP = 50000;
@@ -69,7 +68,11 @@ public THIS addUrl(U url) {
 			if (!allowMultipleSitemaps) throw new RuntimeException("More than " + maxUrls + " urls, but allowMultipleSitemaps is false.  Enable allowMultipleSitemaps to split the sitemap into multiple files with a sitemap index.");
 			if (baseDir != null) {
 				if (mapCount == 0) mapCount++;
-				writeSiteMap();
+				try {
+					writeSiteMap();
+				} catch(IOException ex) {
+					throw new RuntimeException("Closing of stream failed.", ex);
+				}
 				mapCount++;
 				urls.clear();
 			}
@@ -105,9 +108,8 @@ public THIS addUrls(U... urls) {
 	 * or write out one sitemap immediately.
 	 * @param urls the URLs to add to this sitemap
 	 * @return this
-	 * @throws MalformedURLException
 	 */
-	public THIS addUrls(String... urls) throws MalformedURLException {
+	public THIS addUrls(String... urls) {
 		for (String url : urls) addUrl(url);
 		return getThis();
 	}
@@ -117,16 +119,15 @@ public THIS addUrls(String... urls) throws MalformedURLException {
 	 * or else write out one sitemap immediately.
 	 * @param url the URL to add to this sitemap
 	 * @return this
-	 * @throws MalformedURLException
 	 */
-	public THIS addUrl(String url) throws MalformedURLException {
+	public THIS addUrl(String url) {
 		U sitemapUrl;
 		try {
 			sitemapUrl = renderer.getUrlClass().getConstructor(String.class).newInstance(url);
+			return addUrl(sitemapUrl);
 		} catch (Exception e) {
 			throw new RuntimeException(e);
 		}
-		return addUrl(sitemapUrl);
 	}
 
 	/** Add multiple URLs of the appropriate type to this sitemap, one at a time.
@@ -150,10 +151,10 @@ public THIS addUrl(URL url) {
 		U sitemapUrl;
 		try {
 			sitemapUrl = renderer.getUrlClass().getConstructor(URL.class).newInstance(url);
+			return addUrl(sitemapUrl);
 		} catch (Exception e) {
 			throw new RuntimeException(e);
 		}
-		return addUrl(sitemapUrl);
 	}
 
 	@SuppressWarnings("unchecked")
@@ -168,7 +169,11 @@ THIS getThis() {
 	public List<File> write() {
 		if (finished) throw new RuntimeException("Sitemap already printed; you must create a new generator to make more sitemaps");
 		if (!allowEmptySitemap && urls.isEmpty() && mapCount == 0) throw new RuntimeException("No URLs added, sitemap would be empty; you must add some URLs with addUrls");
-		writeSiteMap();
+		try {
+			writeSiteMap();
+		} catch (IOException ex) {
+			throw new RuntimeException("Closing of streams has failed at some point.", ex);
+		}
 		finished = true;
 		return outFiles;
 	}
@@ -231,7 +236,7 @@ public File writeSitemapsWithIndex(File outFile) {
 		return outFile;
 	}
 
-	private void writeSiteMap() {
+	private void writeSiteMap() throws IOException {
 		if (baseDir == null) {
 			throw new NullPointerException("To write to files, baseDir must not be null");
 		}
@@ -244,30 +249,36 @@ private void writeSiteMap() {
 		}
 		File outFile = new File(baseDir, fileNamePrefix+fileNameSuffix);
 		outFiles.add(outFile);
+
+		OutputStreamWriter out = null;
 		try {
-			OutputStreamWriter out;
 			if (gzip) {
 				FileOutputStream fileStream = new FileOutputStream(outFile);
 				GZIPOutputStream gzipStream = new GZIPOutputStream(fileStream);
 				out = new OutputStreamWriter(gzipStream, Charset.forName("UTF-8").newEncoder());
 			} else {
 				out = new OutputStreamWriter(new FileOutputStream(outFile), Charset.forName("UTF-8").newEncoder());
 			}
-			
+
 			writeSiteMap(out);
+			out.flush();
+
 			if (autoValidate) SitemapValidator.validateWebSitemap(outFile);
 		} catch (IOException e) {
 			throw new RuntimeException("Problem writing sitemap file " + outFile, e);
 		} catch (SAXException e) {
 			throw new RuntimeException("Sitemap file failed to validate (bug?)", e);
+		} finally {
+			if(out != null) {
+				out.close();
+			}
 		}
 	}
 
 	private void writeSiteMap(OutputStreamWriter out) throws IOException {
 		StringBuilder sb = new StringBuilder();
 		writeSiteMapAsString(sb, urls);
 		out.write(sb.toString());
-		out.close();
 	}
 
 }
diff --git a/src/main/java/com/redfin/sitemapgenerator/SitemapIndexGenerator.java b/src/main/java/com/redfin/sitemapgenerator/SitemapIndexGenerator.java
@@ -1,5 +1,7 @@
 package com.redfin.sitemapgenerator;
 
+import org.xml.sax.SAXException;
+
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
@@ -9,8 +11,6 @@
 import java.util.ArrayList;
 import java.util.Date;
 
-import org.xml.sax.SAXException;
-
 /**
  * Builds a sitemap index, which points only to other sitemaps.
  * @author Dan Fabulich
@@ -223,15 +223,27 @@ public SitemapIndexGenerator addUrls(String prefix, String suffix, int count) {
 	public void write() {
 		if (!allowEmptyIndex && urls.isEmpty()) throw new RuntimeException("No URLs added, sitemap index would be empty; you must add some URLs with addUrls");
 		try {
-			// TODO gzip? is that legal for a sitemap index?
-			FileWriter out = new FileWriter(outFile);
-			writeSiteMap(out);
-			if (autoValidate) SitemapValidator.validateSitemapIndex(outFile);
-		} catch (IOException e) {
-			throw new RuntimeException("Problem writing sitemap index file " + outFile, e);
-		} catch (SAXException e) {
-			throw new RuntimeException("Problem validating sitemap index file (bug?)", e);
+			FileWriter out = null;
+			try {
+				// TODO gzip? is that legal for a sitemap index?
+				out = new FileWriter(outFile);
+				writeSiteMap(out);
+				out.flush();
+
+				if (autoValidate) SitemapValidator.validateSitemapIndex(outFile);
+			} catch (IOException e) {
+				throw new RuntimeException("Problem writing sitemap index file " + outFile, e);
+			} catch (SAXException e) {
+				throw new RuntimeException("Problem validating sitemap index file (bug?)", e);
+			} finally {
+				if(out != null) {
+					out.close();
+				}
+			}
+		} catch (IOException ex) {
+			throw new RuntimeException("Closing of stream has failed.", ex);
 		}
+
 	}
 
 	private void writeSiteMap(OutputStreamWriter out) throws IOException {
@@ -254,7 +266,6 @@ private void writeSiteMap(OutputStreamWriter out) throws IOException {
 			out.write("  </sitemap>\n");
 		}
 		out.write("</sitemapindex>");
-		out.close();
 	}
 
 }
diff --git a/src/main/java/com/redfin/sitemapgenerator/SitemapValidator.java b/src/main/java/com/redfin/sitemapgenerator/SitemapValidator.java
@@ -1,19 +1,18 @@
 package com.redfin.sitemapgenerator;
 
-import java.io.File;
-import java.io.FileReader;
-import java.io.IOException;
-import java.io.InputStream;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
 
 import javax.xml.XMLConstants;
 import javax.xml.transform.sax.SAXSource;
 import javax.xml.transform.stream.StreamSource;
 import javax.xml.validation.Schema;
 import javax.xml.validation.SchemaFactory;
 import javax.xml.validation.Validator;
-
-import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStream;
 
 /** Validates sitemaps and sitemap indexes
  * 
@@ -39,23 +38,30 @@ public class SitemapValidator {
 	private synchronized static void lazyLoad() {
 		if (sitemapSchema != null)  return;
 		SchemaFactory factory =
-            SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+			SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
 		try {
-			InputStream stream = SitemapValidator.class.getResourceAsStream("sitemap.xsd");
-			if (stream == null) throw new RuntimeException("BUG Couldn't load sitemap.xsd");
-			StreamSource source = new StreamSource(stream);
-			sitemapSchema = factory.newSchema(source);
-			stream.close();
-
-			stream = SitemapValidator.class.getResourceAsStream("siteindex.xsd");
-			if (stream == null) throw new RuntimeException("BUG Couldn't load siteindex.xsd");
-			source = new StreamSource(stream);
-			sitemapIndexSchema = factory.newSchema(source);
-			stream.close();
+			sitemapSchema = lazyLoad(factory, "sitemap.xsd");
+			sitemapIndexSchema = lazyLoad(factory, "siteindex.xsd");
 		} catch (Exception e) {
 			throw new RuntimeException("BUG", e);
 		}
 	}
+
+	private synchronized static Schema lazyLoad(SchemaFactory factory, String resource) throws IOException, SAXException {
+		InputStream stream = null;
+
+		try {
+			stream = SitemapValidator.class.getResourceAsStream(resource);
+			if (stream == null) throw new RuntimeException("BUG Couldn't load " + resource);
+			StreamSource source = new StreamSource(stream);
+			return factory.newSchema(source);
+		} finally {
+			if(stream != null) {
+				stream.close();
+			}
+		}
+
+	}
 
 	/** Validates an ordinary web sitemap file (NOT a Google-specific sitemap) */
 	public static void validateWebSitemap(File sitemap) throws SAXException {
@@ -70,15 +76,24 @@ public static void validateSitemapIndex(File sitemap) throws SAXException {
 	}
 
 	private static void validateXml(File sitemap, Schema schema) throws SAXException {
-		Validator validator = schema.newValidator();
 		try {
-			FileReader reader = new FileReader(sitemap);
-			SAXSource source = new SAXSource(new InputSource(reader));
-			validator.validate(source);
-			reader.close();
-		} catch (IOException e) {
-			throw new RuntimeException(e);
+			Validator validator = schema.newValidator();
+			FileReader reader = null;
+			try {
+				reader = new FileReader(sitemap);
+				SAXSource source = new SAXSource(new InputSource(reader));
+				validator.validate(source);
+			} catch (IOException e) {
+				throw new RuntimeException(e);
+			} finally {
+				if(reader != null) {
+					reader.close();
+				}
+			}
+		} catch (IOException ex) {
+			throw new RuntimeException("Unable to close stream.", ex);
 		}
+
 	}
 
 }