TouchInstinct
/
checkstyle
307
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

100% UTs coverage for PropertyCacheFile. Refactoring of PropertyCacheFile. #1294

This commit is contained in:
Roman Ivanov 2015-07-28 10:12:18 -07:00
parent 0ce872d7cc
commit ea9ff3965d
7 changed files with 287 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pom.xml
View File

@ -1082,7 +1082,6 @@
<regex><pattern>.*.DefaultLogger</pattern><branchRate>75</branchRate><lineRate>76</lineRate></regex>
<regex><pattern>.*.Main</pattern><branchRate>80</branchRate><lineRate>90</lineRate></regex>
<regex><pattern>.*.PackageNamesLoader</pattern><branchRate>78</branchRate><lineRate>72</lineRate></regex>
<regex><pattern>.*.PropertyCacheFile</pattern><branchRate>22</branchRate><lineRate>19</lineRate></regex>
<regex><pattern>.*.TreeWalker</pattern><branchRate>94</branchRate><lineRate>92</lineRate></regex>
<regex><pattern>.*.checks.AbstractOptionCheck</pattern><branchRate>100</branchRate><lineRate>80</lineRate></regex>

116
src/main/java/com/puppycrawl/tools/checkstyle/PropertyCacheFile.java
View File

@ -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 <code>PropertyCacheFile</code> 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 <code>configuration</code>
* @param object the GlobalProperties
* @return the hashcode for <code>object</code>
*/
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);

28
src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java
View File

@ -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();
}

33
src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/CustomImportOrderCheck.java
View File

@ -117,7 +117,7 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
* For example:
* </pre>
* <p>To configure the check so that it matches default Eclipse formatter configuration
* (tested on Kepler and Luna releases):</p>
* (tested on Kepler, Luna and Mars):</p>
* <ul>
* <li>group of static imports is on the top</li>
* <li>groups of non-static imports: &quot;java&quot; and &quot;javax&quot; packages
@ -125,14 +125,6 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
* <li>imports will be sorted in the groups</li>
* <li>groups are separated by, at least, one blank line</li>
* </ul>
* <p>Notes:</p>
* <ul>
* <li>&quot;com&quot; package is not mentioned on configuration, because it is
* ignored by Eclipse Kepler and Luna (looks like Eclipse defect)</li>
* <li>configuration below doesn't work in all 100% cases due to inconsistent behavior
* prior to Mars release, but covers most scenarios</li>
* </ul>
*
* <pre>
* <code>
* &lt;module name=&quot;CustomImportOrder&quot;&gt;
@ -145,29 +137,6 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
* </code>
* </pre>
*
* <p>To configure the check so that it matches default Eclipse formatter
* configuration (tested on Mars release):</p>
* <ul>
* <li>group of static imports is on the top</li>
* <li>groups of non-static imports: &quot;java&quot; and &quot;javax&quot; packages first,
* then &quot;org&quot; and &quot;com&quot;, then all other imports as one group</li>
* <li>imports will be sorted in the groups</li>
* <li>groups are separated by, at least, one blank line</li>
* </ul>
*
* <pre>
* <code>
*&lt;module name=&quot;CustomImportOrder&quot;&gt;
* &lt;property name=&quot;customImportOrderRules&quot;
* value=&quot;STATIC###STANDARD_JAVA_PACKAGE###SPECIAL_IMPORTS
* ###THIRD_PARTY_PACKAGE&quot;/&gt;
* &lt;property name=&quot;specialImportsRegExp&quot; value=&quot;org&quot;/&gt;
* &lt;property name=&quot;thirdPartyPackageRegExp&quot; value=&quot;com&quot;/&gt;
* &lt;property name=&quot;sortImportsInGroupAlphabetically&quot; value=&quot;true&quot;/&gt;
* &lt;property name=&quot;separateLineBetweenGroups&quot; value=&quot;true&quot;/&gt;
*&lt;/module&gt;
* </code>
* </pre>
* <p>To configure the check so that it matches default IntelliJ IDEA formatter
* configuration (tested on v14):</p>
* <ul>

29
src/main/java/com/puppycrawl/tools/checkstyle/checks/imports/ImportOrderCheck.java
View File

@ -67,7 +67,7 @@ import com.puppycrawl.tools.checkstyle.checks.AbstractOptionCheck;
* Example:
* </p>
* <p>To configure the check so that it matches default Eclipse formatter configuration
* (tested on Kepler and Luna releases):</p>
* (tested on Kepler, Luna and Mars):</p>
* <ul>
* <li>group of static imports is on the top</li>
* <li>groups of non-static imports: &quot;java&quot; then &quot;javax&quot;
@ -75,13 +75,6 @@ import com.puppycrawl.tools.checkstyle.checks.AbstractOptionCheck;
* <li>imports will be sorted in the groups</li>
* <li>groups are separated by, at least, one blank line</li>
* </ul>
* <p>Notes:</p>
* <ul>
* <li>&quot;com&quot; package is not mentioned on configuration, because it is
* ignored by Eclipse Kepler and Luna (looks like Eclipse defect)</li>
* <li>configuration below doesn't work in all 100% cases due to inconsistent behavior
* prior to Mars release, but covers most scenarios</li>
* </ul>
*
* <pre>
* &lt;module name=&quot;ImportOrder&quot;&gt;
@ -93,26 +86,6 @@ import com.puppycrawl.tools.checkstyle.checks.AbstractOptionCheck;
* &lt;/module&gt;
* </pre>
*
* <p>To configure the check so that it matches default Eclipse formatter
* configuration (tested on Mars release):</p>
* <ul>
* <li>group of static imports is on the top</li>
* <li>groups of non-static imports: &quot;java&quot; and &quot;javax&quot; packages first,
* then &quot;org&quot; and &quot;com&quot;, then all other imports as one group</li>
* <li>imports will be sorted in the groups</li>
* <li>groups are separated by, at least, one blank line</li>
* </ul>
*
* <pre>
*&lt;module name=&quot;ImportOrder&quot;&gt;
* &lt;property name=&quot;groups&quot; value=&quot;/^javax?\./,org,com&quot;/&gt;
* &lt;property name=&quot;ordered&quot; value=&quot;true&quot;/&gt;
* &lt;property name=&quot;separated&quot; value=&quot;true&quot;/&gt;
* &lt;property name=&quot;option&quot; value=&quot;above&quot;/&gt;
* &lt;property name=&quot;sortStaticImportsAlphabetically&quot; value=&quot;true&quot;/&gt;
*&lt;/module&gt;
* </pre>
*
* <p>To configure the check so that it matches default IntelliJ IDEA formatter configuration
* (tested on v14):</p>
* <ul>

120
src/test/java/com/puppycrawl/tools/checkstyle/PropertyCacheFileTest.java Normal file
View File

@ -0,0 +1,120 @@
////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code for adherence to a set of rules.
// Copyright (C) 2001-2015 the original author or authors.
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
////////////////////////////////////////////////////////////////////////////////
package com.puppycrawl.tools.checkstyle;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.powermock.api.mockito.PowerMockito.mockStatic;
import static org.powermock.api.mockito.PowerMockito.when;
import static org.powermock.api.support.membermodification.MemberMatcher.method;
import java.io.File;
import java.io.IOException;
import java.io.Serializable;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import com.puppycrawl.tools.checkstyle.api.Configuration;
@RunWith(PowerMockRunner.class)
@PrepareForTest({ PropertyCacheFile.class, PropertyCacheFileTest.class })
public class PropertyCacheFileTest {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
@Test
public void testNonAccessibleFile() throws IOException {
Configuration config = new DefaultConfiguration("myname");
final File file = temporaryFolder.newFile("file.output");
file.setReadable(true, false);
file.setWritable(false, false);
PropertyCacheFile cache = new PropertyCacheFile(config, file.getAbsolutePath());
}
@Test
public void testCtor() throws IOException {
try {
PropertyCacheFile cache = new PropertyCacheFile(null, "");
}
catch (IllegalArgumentException ex) {
assertEquals("config can not be null", ex.getMessage());
}
try {
Configuration config = new DefaultConfiguration("myname");
PropertyCacheFile cache = new PropertyCacheFile(config, null);
}
catch (IllegalArgumentException ex) {
assertEquals("fileName can not be null", ex.getMessage());
}
}
@Test
public void testInCache() throws IOException {
Configuration config = new DefaultConfiguration("myname");
final String filePath = temporaryFolder.newFile().getPath();
PropertyCacheFile cache = new PropertyCacheFile(config, filePath);
cache.put("myFile", 1);
assertTrue(cache.inCache("myFile", 1));
assertFalse(cache.inCache("myFile", 2));
assertFalse(cache.inCache("myFile1", 1));
}
@Test
public void testException_NoSuchAlgorithmException() throws Exception {
Configuration config = new DefaultConfiguration("myname");
final String filePath = temporaryFolder.newFile().getPath();
PropertyCacheFile cache = new PropertyCacheFile(config, filePath);
cache.put("myFile", 1);
mockStatic(MessageDigest.class);
when(MessageDigest.getInstance("SHA-1"))
.thenThrow(NoSuchAlgorithmException.class);
Class[] param = new Class[1];
param[0] = Serializable.class;
Method method = PropertyCacheFile.class.getDeclaredMethod("getConfigHashCode", param);
method.setAccessible(true);
try {
method.invoke(cache, config);
fail();
}
catch (InvocationTargetException e) {
assertTrue(e.getCause().getCause() instanceof NoSuchAlgorithmException);
assertEquals("Unable to calculate hashcode.", e.getCause().getMessage());
}
catch (Exception e) {
fail();
}
}
}

88
src/test/java/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java
View File

@ -25,7 +25,9 @@ import static org.junit.Assert.fail;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.io.Writer;
import java.util.Locale;
import org.junit.Assert;
import org.junit.Rule;
@ -122,13 +124,95 @@ public class TreeWalkerTest extends BaseCheckTestSupport {
public void testSettersForParameters() throws Exception {
final TreeWalker treeWalker = new TreeWalker();
treeWalker.setTabWidth(1);
treeWalker.configure(new DefaultConfiguration("default config"));
treeWalker.setCacheFile(temporaryFolder.newFile().getPath());
}
@Test
public void testNonExistingCacheFileDoesNotThrowException() {
public void testNonExistingCacheFileDoesNotThrowException() throws Exception {
final TreeWalker treeWalker = new TreeWalker();
treeWalker.configure(new DefaultConfiguration("default config"));
treeWalker.setCacheFile("/invalid");
treeWalker.destroy();
treeWalker.finishLocalSetup();
try {
treeWalker.destroy();
fail();
}
catch (IllegalStateException ex) {
assertTrue(ex.getCause() instanceof IOException);
}
}
@Test
public void testCacheFile() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(HiddenFieldCheck.class);
final DefaultConfiguration treeWalkerConfig = createCheckConfig(TreeWalker.class);
treeWalkerConfig.addAttribute("cacheFile", temporaryFolder.newFile().getPath());
treeWalkerConfig.addChild(checkConfig);
final DefaultConfiguration checkerConfig = new DefaultConfiguration("configuration");
checkerConfig.addAttribute("charset", "UTF-8");
checkerConfig.addChild(treeWalkerConfig);
final Checker checker = new Checker();
final Locale locale = Locale.ROOT;
checker.setLocaleCountry(locale.getCountry());
checker.setLocaleLanguage(locale.getLanguage());
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.configure(checkerConfig);
checker.addListener(new BriefLogger(stream));
final String pathToEmptyFile = temporaryFolder.newFile("file.java").getPath();
final String[] expected = {
};
verify(checker, pathToEmptyFile, pathToEmptyFile, expected);
// one more time to reuse cache
verify(checker, pathToEmptyFile, pathToEmptyFile, expected);
}
@Test
public void testCacheFile_changeInConfig() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(HiddenFieldCheck.class);
final DefaultConfiguration treeWalkerConfig = createCheckConfig(TreeWalker.class);
treeWalkerConfig.addAttribute("cacheFile", temporaryFolder.newFile().getPath());
treeWalkerConfig.addChild(checkConfig);
final DefaultConfiguration checkerConfig = new DefaultConfiguration("configuration");
checkerConfig.addAttribute("charset", "UTF-8");
checkerConfig.addChild(treeWalkerConfig);
Checker checker = new Checker();
final Locale locale = Locale.ROOT;
checker.setLocaleCountry(locale.getCountry());
checker.setLocaleLanguage(locale.getLanguage());
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.configure(checkerConfig);
checker.addListener(new BriefLogger(stream));
final String pathToEmptyFile = temporaryFolder.newFile("file.java").getPath();
final String[] expected = {
};
verify(checker, pathToEmptyFile, pathToEmptyFile, expected);
// update Checker config
//checker.destroy();
//checker.configure(checkerConfig);
checker = new Checker();
checker.setLocaleCountry(locale.getCountry());
checker.setLocaleLanguage(locale.getLanguage());
checker.setModuleClassLoader(Thread.currentThread().getContextClassLoader());
checker.configure(checkerConfig);
checker.addListener(new BriefLogger(stream));
// here is diff with previous checker
checkerConfig.addAttribute("fileExtensions", "java,javax");
// one more time on updated config
verify(checker, pathToEmptyFile, pathToEmptyFile, expected);
}
}