From ea9ff3965d9c657b5b7bd2d969e075f430635c47 Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Tue, 28 Jul 2015 10:12:18 -0700 Subject: [PATCH] 100% UTs coverage for PropertyCacheFile. Refactoring of PropertyCacheFile. #1294 --- pom.xml | 1 - .../tools/checkstyle/PropertyCacheFile.java | 116 +++++++++-------- .../tools/checkstyle/TreeWalker.java | 28 +++- .../imports/CustomImportOrderCheck.java | 33 +---- .../checks/imports/ImportOrderCheck.java | 29 +---- .../checkstyle/PropertyCacheFileTest.java | 120 ++++++++++++++++++ .../tools/checkstyle/TreeWalkerTest.java | 88 ++++++++++++- 7 files changed, 287 insertions(+), 128 deletions(-) create mode 100644 src/test/java/com/puppycrawl/tools/checkstyle/PropertyCacheFileTest.java diff --git a/pom.xml b/pom.xml index cd8304247..936791cce 100644 --- a/pom.xml +++ b/pom.xml @@ -1082,7 +1082,6 @@ .*.DefaultLogger7576 .*.Main8090 .*.PackageNamesLoader7872 - .*.PropertyCacheFile2219 .*.TreeWalker9492 .*.checks.AbstractOptionCheck10080 diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java b/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java index 696e92e2a..809315bde 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java @@ -20,8 +20,8 @@ package com.puppycrawl.tools.checkstyle; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.ObjectOutputStream; @@ -31,9 +31,6 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.Properties; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import com.google.common.io.Closeables; import com.google.common.io.Flushables; import com.puppycrawl.tools.checkstyle.api.Configuration; @@ -51,8 +48,6 @@ import com.puppycrawl.tools.checkstyle.api.Configuration; * @author Oliver Burn */ final class PropertyCacheFile { - /** Logger for PropertyCacheFile */ - private static final Log LOG = LogFactory.getLog(PropertyCacheFile.class); /** * The property key to use for storing the hashcode of the @@ -74,84 +69,87 @@ final class PropertyCacheFile { /** bit shift */ private static final int SHIFT_4 = 4; - /** name of file to store details **/ - private final String detailsFile; /** the details on files **/ private final Properties details = new Properties(); + /** configuration object **/ + private Configuration config; + + /** file name of cache **/ + private String fileName; + /** * Creates a new PropertyCacheFile instance. * - * @param currentConfig the current configuration, not null + * @param config the current configuration, not null * @param fileName the cache file */ - PropertyCacheFile(Configuration currentConfig, String fileName) { - boolean setInActive = true; - if (fileName != null) { - FileInputStream inStream = null; - // get the current config so if the file isn't found - // the first time the hash will be added to output file - final String currentConfigHash = getConfigHashCode(currentConfig); + PropertyCacheFile(Configuration config, String fileName) { + if (config == null) { + throw new IllegalArgumentException("config can not be null"); + } + if (fileName == null) { + throw new IllegalArgumentException("fileName can not be null"); + } + this.config = config; + this.fileName = fileName; + } + + /** + * load cached values from file + * @throws IOException when there is a problems with file read + */ + void load() throws IOException { + FileInputStream inStream = null; + // get the current config so if the file isn't found + // the first time the hash will be added to output file + final String currentConfigHash = getConfigHashCode(config); + if (new File(fileName).exists()) { try { inStream = new FileInputStream(fileName); details.load(inStream); - final String cachedConfigHash = - details.getProperty(CONFIG_HASH_KEY); - setInActive = false; - if (cachedConfigHash == null - || !cachedConfigHash.equals(currentConfigHash)) { + final String cachedConfigHash = details.getProperty(CONFIG_HASH_KEY); + if (!currentConfigHash.equals(cachedConfigHash)) { // Detected configuration change - clear cache details.clear(); details.put(CONFIG_HASH_KEY, currentConfigHash); } } - catch (final FileNotFoundException e) { - // Ignore, the cache does not exist - setInActive = false; - // put the hash in the file if the file is going to be created - details.put(CONFIG_HASH_KEY, currentConfigHash); - } - catch (final IOException e) { - LOG.debug("Unable to open cache file, ignoring.", e); - } finally { Closeables.closeQuietly(inStream); } } - detailsFile = setInActive ? null : fileName; + else { + // put the hash in the file if the file is going to be created + details.put(CONFIG_HASH_KEY, currentConfigHash); + } } - /** Cleans up the object and updates the cache file. **/ - void destroy() { - if (detailsFile != null) { - FileOutputStream out = null; - try { - out = new FileOutputStream(detailsFile); - details.store(out, null); - } - catch (final IOException e) { - LOG.debug("Unable to save cache file.", e); - } - finally { - if (out != null) { - flushAndCloseOutStream(out); - } - } + /** + * Cleans up the object and updates the cache file. + * @throws IOException when there is a problems with file save + */ + void persist() throws IOException { + FileOutputStream out = null; + try { + out = new FileOutputStream(fileName); + details.store(out, null); + } + finally { + flushAndCloseOutStream(out); } } /** * Flushes and closes output stream. * @param stream the output stream + * @throws IOException when there is a problems with file flush and close */ - private static void flushAndCloseOutStream(OutputStream stream) { - try { + private static void flushAndCloseOutStream(OutputStream stream) throws IOException { + if (stream != null) { Flushables.flush(stream, false); - Closeables.close(stream, false); - } - catch (final IOException ex) { - LOG.debug("Unable to flush and close output stream.", ex); } + Closeables.close(stream, false); } /** @@ -159,7 +157,7 @@ final class PropertyCacheFile { * @param timestamp the timestamp of the file to check * @return whether the specified file has already been checked ok */ - boolean alreadyChecked(String fileName, long timestamp) { + boolean inCache(String fileName, long timestamp) { final String lastChecked = details.getProperty(fileName); return lastChecked != null && lastChecked.equals(Long.toString(timestamp)); @@ -170,17 +168,17 @@ final class PropertyCacheFile { * @param fileName name of the file that checked ok * @param timestamp the timestamp of the file */ - void checkedOk(String fileName, long timestamp) { + void put(String fileName, long timestamp) { details.put(fileName, Long.toString(timestamp)); } /** * Calculates the hashcode for a GlobalProperties. * - * @param configuration the GlobalProperties - * @return the hashcode for configuration + * @param object the GlobalProperties + * @return the hashcode for object */ - private static String getConfigHashCode(Serializable configuration) { + private static String getConfigHashCode(Serializable object) { try { // im-memory serialization of Configuration @@ -188,7 +186,7 @@ final class PropertyCacheFile { ObjectOutputStream oos = null; try { oos = new ObjectOutputStream(baos); - oos.writeObject(configuration); + oos.writeObject(object); } finally { flushAndCloseOutStream(oos); diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java b/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java index 05a581fbf..fb7c418ef 100755 --- a/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java @@ -20,6 +20,7 @@ package com.puppycrawl.tools.checkstyle; import java.io.File; +import java.io.IOException; import java.io.Reader; import java.io.StringReader; import java.util.AbstractMap.SimpleEntry; @@ -103,7 +104,7 @@ public final class TreeWalker private int tabWidth = DEFAULT_TAB_WIDTH; /** cache file **/ - private PropertyCacheFile cache = new PropertyCacheFile(null, null); + private PropertyCacheFile cache; /** class loader to resolve classes with. **/ private ClassLoader classLoader; @@ -130,6 +131,13 @@ public final class TreeWalker public void setCacheFile(String fileName) { final Configuration configuration = getConfiguration(); cache = new PropertyCacheFile(configuration, fileName); + + try { + cache.load(); + } + catch (IOException e) { + throw new IllegalStateException("cache file load is failed", e); + } } /** @param classLoader class loader to resolve classes with. */ @@ -178,8 +186,9 @@ public final class TreeWalker // check if already checked and passed the file final String fileName = file.getPath(); final long timestamp = file.lastModified(); - if (cache.alreadyChecked(fileName, timestamp) - || !Utils.fileExtensionMatches(file, getFileExtensions())) { + if (cache != null + && (cache.inCache(fileName, timestamp) + || !Utils.fileExtensionMatches(file, getFileExtensions()))) { return; } @@ -216,8 +225,8 @@ public final class TreeWalker getMessageCollector().add(createLocalizedMessage(ex.getMessage())); } - if (getMessageCollector().size() == 0) { - cache.checkedOk(fileName, timestamp); + if (cache != null && getMessageCollector().size() == 0) { + cache.put(fileName, timestamp); } } @@ -466,7 +475,14 @@ public final class TreeWalker for (Check c : commentChecks) { c.destroy(); } - cache.destroy(); + if (cache != null) { + try { + cache.persist(); + } + catch (IOException e) { + throw new IllegalStateException("Unable to persist cache file", e); + } + } super.destroy(); } diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java index 6e1359946..312fb4598 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java @@ -117,7 +117,7 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; * For example: * *

To configure the check so that it matches default Eclipse formatter configuration - * (tested on Kepler and Luna releases):

+ * (tested on Kepler, Luna and Mars):

* - *

Notes:

- * - * * 
  *        
  * <module name="CustomImportOrder">
@@ -145,29 +137,6 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
  *        
  *
* - *

To configure the check so that it matches default Eclipse formatter - * configuration (tested on Mars release):

- * - * - * 
- *        
- *<module name="CustomImportOrder">
- *    <property name="customImportOrderRules"
- *        value="STATIC###STANDARD_JAVA_PACKAGE###SPECIAL_IMPORTS
- *        ###THIRD_PARTY_PACKAGE"/>
- *    <property name="specialImportsRegExp" value="org"/>
- *    <property name="thirdPartyPackageRegExp" value="com"/>
- *    <property name="sortImportsInGroupAlphabetically" value="true"/>
- *    <property name="separateLineBetweenGroups" value="true"/>
- *</module>
- *        
- *
*

To configure the check so that it matches default IntelliJ IDEA formatter * configuration (tested on v14):

*