Fix HER-1935 Many calls to File.mkdirs() and other file/dir methods don't check the return value. Also fix bug where pointless empty directories were created in scratch dir.

* BasicProfileTest.java, SelfTestBase.java, CrawlControllerTest.java, PrecedenceLoader.java, MigrateH1to3Tool.java, CrawlerLoggerModule.java, StatisticsTracker.java, CheckpointUtils.java, BdbUriUniqFilter.java, ARCWriterProcessorTest.java, WARCWriterProcessorTest.java, PersistProcessor.java, WriterPoolProcessor.java, PrefixFinderTest.java, StoredQueueTest.java, FileUtilsTest.java, ObjectIdentityBdbManualCacheTest.java, ObjectIdentityBdbCacheTest.java, ObjectPlusFilesOutputStream.java, TestUtils.java, TmpDirTestCase.java, Engine.java
    Replace calls to File.mkdirs() with FileUtils.ensureWriteableDirectory(dir). In these cases the calls were either already in a spot where the possible IOException would be handled appropriately, or the line was trivially moved into such a block.
* ActionDirectory.java, Engine.java
    replace calls to File.mkdirs() with FileUtils.ensureWriteableDirectory(dir), and throw IllegalStateException on failure
* BdbModule.java
    setup() - replace call to File.mkdirs() with FileUtils.ensureWriteableDirectory(dir) and add "throws IOException" - conveniently the place where this method is called was already in a try block that catches IOException
* Checkpoint.java
    generateFrom() - replace call to File.mkdirs() with FileUtils.ensureWriteableDirectory(dir) and add "throws IOException"
* CheckpointService.java
    move call to Checkpoint.generateFrom() inside existing try block since it now can throw IOException
* Recorder.java
    ensure(File) - replace call to File.mkdirs() with FileUtils.ensureWriteableDirectory(dir), and throw IllegalStateException on failure
    new Recorder(File,String,int,int) - call ensure() on the correct object, the containing directory; and remove redundant call to ensure()
This commit is contained in:
nlevitt committed 2011-09-12 20:12:10 +00:00
1 parent f7cc187b09
commit f20b3eee30
29 files changed
+78 -63

No files matched your search

@@ -256,7 +256,7 @@ public class BdbModule implements Lifecycle, Checkpointable, Closeable {
}
protected void setup(File f, boolean create)
throws DatabaseException {
throws DatabaseException, IOException {
EnvironmentConfig config = new EnvironmentConfig();
config.setAllowCreate(create);
config.setLockTimeout(75, TimeUnit.MINUTES); // set to max
@@ -282,7 +282,7 @@ public class BdbModule implements Lifecycle, Checkpointable, Closeable {
config.setConfigParam("je.cleaner.expunge", "false");
} // else leave whatever other setting was already in place
f.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(f);
this.bdbEnvironment = new EnhancedEnvironment(f, config);
this.classCatalog = this.bdbEnvironment.getClassCatalog();
if(!create) {
@@ -479,7 +479,7 @@ public class BdbModule implements Lifecycle, Checkpointable, Closeable {
dbBackup.startBackup();
File envCpDir = new File(dir.getFile(),checkpointInProgress.getName());
envCpDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(envCpDir);
File logfilesList = new File(envCpDir,"jdbfiles.manifest");
String[] filedata = dbBackup.getLogFilesInBackupSet();
for (int i=0; i<filedata.length;i++) {
@@ -78,15 +78,16 @@ public class Checkpoint implements InitializingBean {
*
* @param checkpointsDir
* @param nextCheckpointNumber
* @throws IOException
*/
public void generateFrom(ConfigPath checkpointsDir, int nextCheckpointNumber) {
public void generateFrom(ConfigPath checkpointsDir, int nextCheckpointNumber) throws IOException {
getCheckpointDir().setBase(checkpointsDir);
getCheckpointDir().setPath(
"cp"
+ INDEX_FORMAT.format(nextCheckpointNumber)
+ "-"
+ ArchiveUtils.get14DigitDate());
getCheckpointDir().getFile().mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(getCheckpointDir().getFile());
afterPropertiesSet();
}
@@ -99,7 +99,7 @@ public class ObjectPlusFilesOutputStream extends ObjectOutputStream {
writeLong(file.length());
File auxDir = getAuxiliaryDirectory();
if(!auxDir.exists()) {
auxDir.mkdirs();
FileUtils.ensureWriteableDirectory(auxDir);
}
File destination = new File(auxDir,name);
hardlinkOrCopy(file, destination);
@@ -672,7 +672,10 @@ public class FileUtils {
public static File ensureWriteableDirectory(File dir)
throws IOException {
if (!dir.exists()) {
dir.mkdirs();
boolean success = dir.mkdirs();
if (!success) {
throw new IOException("Failed to create directory: " + dir);
}
} else {
if (!dir.canWrite()) {
throw new IOException("Dir " + dir.getAbsolutePath() +
@@ -120,14 +120,18 @@ public class Recorder {
*/
public Recorder(File tempDir, String backingFilenameBase,
int outBufferSize, int inBufferSize) {
this(ensure(new File(tempDir, backingFilenameBase)),
this(new File(ensure(tempDir), backingFilenameBase),
outBufferSize, inBufferSize);
tempDir.mkdirs();
}
private static File ensure(File tempDir) {
tempDir.mkdirs();
try {
org.archive.util.FileUtils.ensureWriteableDirectory(tempDir);
} catch (IOException e) {
throw new IllegalStateException(e);
}
return tempDir;
}
@@ -140,10 +140,7 @@ public class TestUtils {
String tmpDirStr = System.getProperty(TmpDirTestCase.TEST_TMP_SYSTEM_PROPERTY_NAME);
tmpDirStr = (tmpDirStr == null)? TmpDirTestCase.DEFAULT_TEST_TMP_DIR: tmpDirStr;
File tmpDir = new File(tmpDirStr);
if (!tmpDir.exists())
{
tmpDir.mkdirs();
}
FileUtils.ensureWriteableDirectory(tmpDir);
if (!tmpDir.canWrite())
{
@@ -106,10 +106,7 @@ public abstract class TmpDirTestCase extends TestCase
String tmpDirStr = System.getProperty(TEST_TMP_SYSTEM_PROPERTY_NAME);
tmpDirStr = (tmpDirStr == null)? DEFAULT_TEST_TMP_DIR: tmpDirStr;
File tmpDir = new File(tmpDirStr);
if (!tmpDir.exists())
{
tmpDir.mkdirs();
}
FileUtils.ensureWriteableDirectory(tmpDir);
if (!tmpDir.canWrite())
{
@@ -42,7 +42,7 @@ public class StoredQueueTest extends TmpDirTestCase {
protected void setUp() throws Exception {
super.setUp();
this.envDir = new File(getTmpDir(),"StoredMapTest");
this.envDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(this.envDir);
try {
EnvironmentConfig envConfig = new EnvironmentConfig();
envConfig.setTransactional(false);
@@ -82,7 +82,7 @@ public class PrefixFinderTest extends TmpDirTestCase {
File f = new File(getTmpDir(), "PrefixFinderText");
FileUtils.deleteQuietly(f);
f.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(f);
Environment bdbEnvironment = new Environment(f, config);
DatabaseConfig dbConfig = new DatabaseConfig();
dbConfig.setAllowCreate(true);
@@ -56,9 +56,9 @@ public class FileUtilsTest extends TmpDirTestCase {
protected void setUp() throws Exception {
super.setUp();
this.srcDirFile = new File(getTmpDir(), srcDirName);
this.srcDirFile.mkdirs();
FileUtils.ensureWriteableDirectory(srcDirFile);
this.tgtDirFile = new File(getTmpDir(), tgtDirName);
this.tgtDirFile.mkdirs();
FileUtils.ensureWriteableDirectory(tgtDirFile);
addFiles();
zeroLengthLinesUnix = setUpLinesFile("zeroLengthLinesUnix",0,0,400,IOUtils.LINE_SEPARATOR_UNIX);
@@ -37,10 +37,10 @@ public class ObjectIdentityBdbCacheTest extends TmpDirTestCase {
protected void setUp() throws Exception {
super.setUp();
File envDir = new File(getTmpDir(),"ObjectIdentityBdbCacheTest");
envDir.mkdirs();
File envDir = new File(getTmpDir(),"ObjectIdentityBdbCacheTest");
org.archive.util.FileUtils.ensureWriteableDirectory(envDir);
FileUtils.deleteDirectory(envDir);
envDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(envDir);
env = EnhancedEnvironment.getTestEnvironment(envDir);
this.cache = new ObjectIdentityBdbCache<IdentityCacheableWrapper<HashMap<String,String>>>();
this.cache.initialize(env,"setUpCache",IdentityCacheableWrapper.class, env.getClassCatalog());
@@ -37,10 +37,10 @@ public class ObjectIdentityBdbManualCacheTest extends TmpDirTestCase {
protected void setUp() throws Exception {
super.setUp();
File envDir = new File(getTmpDir(),"ObjectIdentityBdbCacheTest");
envDir.mkdirs();
File envDir = new File(getTmpDir(),"ObjectIdentityBdbCacheTest");
org.archive.util.FileUtils.ensureWriteableDirectory(envDir);
FileUtils.deleteDirectory(envDir);
envDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(envDir);
env = EnhancedEnvironment.getTestEnvironment(envDir);
this.cache = new ObjectIdentityBdbManualCache<IdentityCacheableWrapper<HashMap<String,String>>>();
this.cache.initialize(env,"setUpCache",IdentityCacheableWrapper.class, env.getClassCatalog());
@@ -57,7 +57,7 @@ public class BasicProfileTest extends TmpDirTestCase {
System.out.println("\nNow testing " + srcDir.getName());
File tmpDir = new File(getTmpDir(), "validatorTest");
File configDir = new File(tmpDir, srcDir.getName());
configDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(configDir);
FileUtils.copyDirectory(srcDir, configDir);
PathSharingContext ac = null;
@@ -151,9 +151,13 @@ public class ActionDirectory implements ApplicationContextAware, Lifecycle, Runn
if (isRunning()) {
return;
}
// create directories
getActionDir().getFile().mkdirs();
getDoneDir().getFile().mkdirs();
try {
// create directories
org.archive.util.FileUtils.ensureWriteableDirectory(getActionDir().getFile());
org.archive.util.FileUtils.ensureWriteableDirectory(getDoneDir().getFile());
} catch (IOException e) {
throw new IllegalStateException(e);
}
// start background executor
executor = Executors.newSingleThreadScheduledExecutor();
executor.scheduleWithFixedDelay(this, getInitialDelaySeconds(), getDelaySeconds(), TimeUnit.SECONDS);
@@ -221,12 +221,12 @@ public class CheckpointService implements Lifecycle, ApplicationContextAware, Ha
}
}
checkpointInProgress = new Checkpoint();
checkpointInProgress.generateFrom(getCheckpointsDir(),getNextCheckpointNumber());
Map<String,Checkpointable> toCheckpoint = appCtx.getBeansOfType(Checkpointable.class);
checkpointInProgress = new Checkpoint();
try {
checkpointInProgress.generateFrom(getCheckpointsDir(),getNextCheckpointNumber());
// pre (incl. acquire necessary locks)
// long startMs = System.currentTimeMillis();
for(Checkpointable c : toCheckpoint.values()) {
@@ -61,7 +61,12 @@ public class Engine {
public Engine(File jobsDir) {
this.jobsDir = jobsDir;
this.jobsDir.mkdirs();
try {
org.archive.util.FileUtils.ensureWriteableDirectory(jobsDir);
} catch (IOException e) {
throw new IllegalStateException(e);
}
findJobConfigs();
// TODO: cleanup any cruft from improperly ended jobs
@@ -178,7 +183,7 @@ public class Engine {
*/
public synchronized void copy(CrawlJob orig, File destDir, boolean asProfile)
throws IOException {
destDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(destDir);
if(destDir.list().length>0) {
throw new IOException("destination dir not empty");
}
@@ -352,7 +357,7 @@ public class Engine {
inStream.close();
// write default crawler-beans string to new job dir
newJobDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(newJobDir);
File newJobCxml = new File(newJobDir,"crawler-beans.cxml");
FileUtils.writeStringToFile(newJobCxml, defaultCxmlStr);
@@ -31,6 +31,7 @@ import java.util.Map;
import org.archive.modules.recrawl.PersistProcessor;
import org.archive.util.ArchiveUtils;
import org.archive.util.FileUtils;
import org.archive.util.bdbje.EnhancedEnvironment;
import org.archive.util.iterator.LineReadingIterator;
@@ -100,9 +101,7 @@ public class PrecedenceLoader {
FileNotFoundException, UnsupportedEncodingException, IOException {
File source = new File(args[0]);
File env = new File(args[1]);
if(!env.exists()) {
env.mkdirs();
}
FileUtils.ensureWriteableDirectory(env);
// setup target environment
EnhancedEnvironment targetEnv = PersistProcessor.setupCopyEnvironment(env);
@@ -92,7 +92,7 @@ public class MigrateH1to3Tool {
}
File destinationH3JobDir = new File(destinationH3JobDirArg);
destinationH3JobDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(destinationH3JobDir);
System.out.println("H1 source: "+sourceOrderXmlFile.getAbsolutePath());
System.out.println("H3 destination: "+destinationH3JobDir.getAbsolutePath());
@@ -46,6 +46,7 @@ import org.archive.modules.extractor.UriErrorLoggerModule;
import org.archive.net.UURI;
import org.archive.spring.ConfigPath;
import org.archive.util.ArchiveUtils;
import org.archive.util.FileUtils;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.Lifecycle;
@@ -209,9 +210,9 @@ public class CrawlerLoggerModule
if(isRunning) {
return;
}
getPath().getFile().mkdirs();
this.atg = AlertThreadGroup.current();
try {
FileUtils.ensureWriteableDirectory(getPath().getFile());
setupLogs();
} catch (IOException e) {
throw new IllegalStateException(e);
@@ -58,6 +58,7 @@ import org.archive.modules.seeds.SeedListener;
import org.archive.modules.seeds.SeedModule;
import org.archive.spring.ConfigPath;
import org.archive.util.ArchiveUtils;
import org.archive.util.FileUtils;
import org.archive.util.JSONUtils;
import org.archive.util.MimetypeUtils;
import org.archive.util.ObjectIdentityCache;
@@ -902,8 +903,8 @@ public class StatisticsTracker
return f;
}
f.getParentFile().mkdirs();
try {
FileUtils.ensureWriteableDirectory(f.getParentFile());
PrintWriter bw = new PrintWriter(new FileWriter(f));
report.write(bw, this);
bw.close();
@@ -27,6 +27,7 @@ import java.util.logging.Logger;
import org.archive.bdb.BdbModule;
import org.archive.checkpointing.Checkpoint;
import org.archive.checkpointing.Checkpointable;
import org.archive.util.FileUtils;
import org.json.JSONException;
import org.json.JSONObject;
import org.springframework.beans.factory.BeanNameAware;
@@ -165,9 +166,7 @@ implements Lifecycle, Checkpointable, BeanNameAware {
public BdbUriUniqFilter(File bdbEnv, final int cacheSizePercentage)
throws IOException {
super();
if (!bdbEnv.exists()) {
bdbEnv.mkdirs();
}
FileUtils.ensureWriteableDirectory(bdbEnv);
EnvironmentConfig envConfig = new EnvironmentConfig();
envConfig.setAllowCreate(true);
if (cacheSizePercentage > 0 && cacheSizePercentage < 100) {
@@ -27,6 +27,8 @@ import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import org.archive.util.FileUtils;
/**
* Utilities useful checkpointing.
* @author stack
@@ -76,7 +78,7 @@ public class CheckpointUtils {
public static void writeObjectToFile(final Object o, final String suffix,
final File dir)
throws IOException {
dir.mkdirs();
FileUtils.ensureWriteableDirectory(dir);
ObjectOutputStream out = new ObjectOutputStream(
new FileOutputStream(getClassCheckpointFile(dir, suffix,
o.getClass())));
@@ -30,6 +30,7 @@ import org.archive.modules.net.BdbServerCache;
import org.archive.spring.ConfigPath;
import org.archive.state.ModuleTestBase;
import org.archive.util.ArchiveUtils;
import org.archive.util.FileUtils;
/**
*
@@ -45,9 +46,7 @@ public class CrawlControllerTest extends ModuleTestBase {
tmpPath = DEFAULT_TEST_TMP_DIR;
}
File tmp = new File(tmpPath);
if (!tmp.exists()) {
tmp.mkdirs();
}
FileUtils.ensureWriteableDirectory(tmp);
FileWriter fileWriter = null;
try {
@@ -59,10 +58,10 @@ public class CrawlControllerTest extends ModuleTestBase {
}
File state = new File(tmp, "state");
state.mkdirs();
FileUtils.ensureWriteableDirectory(state);
File checkpoints = new File(tmp, "checkpoints");
checkpoints.mkdirs();
FileUtils.ensureWriteableDirectory(checkpoints);
BdbModule bdb = new BdbModule();
bdb.setDir(new ConfigPath("test",state.getAbsolutePath()));
@@ -90,7 +90,7 @@ public abstract class SelfTestBase extends TmpDirTestCase {
if(profileTemplate.exists()) {
org.apache.commons.io.FileUtils.copyDirectory(profileTemplate, tmpDefProfile);
} else {
tmpDefProfile.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(tmpDefProfile);
}
// Start up a Jetty that serves the selftest's content directory.
@@ -98,7 +98,7 @@ public abstract class SelfTestBase extends TmpDirTestCase {
// Copy configuration for eg Logging over
File tmpConfDir = new File(tmpTestDir, "conf");
tmpConfDir.mkdirs();
org.archive.util.FileUtils.ensureWriteableDirectory(tmpConfDir);
File srcConf = new File(src.getParentFile(), "conf");
FileUtils.copyDirectory(srcConf, tmpConfDir);
@@ -42,6 +42,7 @@ import org.archive.bdb.BdbModule;
import org.archive.modules.CrawlURI;
import org.archive.modules.Processor;
import org.archive.util.ArchiveUtils;
import org.archive.util.FileUtils;
import org.archive.util.OneLineSimpleLogger;
import org.archive.util.SURT;
import org.archive.util.bdbje.EnhancedEnvironment;
@@ -261,9 +262,7 @@ public abstract class PersistProcessor extends Processor {
if (envFile != null) {
// set up target environment
if (!envFile.exists()) {
envFile.mkdirs();
}
FileUtils.ensureWriteableDirectory(envFile);
targetEnv = setupCopyEnvironment(envFile);
classCatalog = targetEnv.getClassCatalog();
historyDB = targetEnv.openDatabase(null, URI_HISTORY_DBNAME,
@@ -48,6 +48,7 @@ import org.archive.modules.Processor;
import org.archive.modules.net.CrawlHost;
import org.archive.modules.net.ServerCache;
import org.archive.spring.ConfigPath;
import org.archive.util.FileUtils;
import org.springframework.beans.factory.annotation.Autowired;
/**
@@ -273,7 +274,7 @@ public class Kw3WriterProcessor extends Processor {
File dir = new File(getPath().getFile(), md5.substring(0, 2) + "/" + host +
"/current");
if (!dir.exists()) {
dir.mkdirs();
FileUtils.ensureWriteableDirectory(dir);
if (this.chmod)
chmods(dir, getPath().getFile());
}
@@ -47,6 +47,7 @@ import org.archive.modules.deciderules.recrawl.IdenticalDigestDecideRule;
import org.archive.modules.net.CrawlHost;
import org.archive.modules.net.ServerCache;
import org.archive.spring.ConfigPath;
import org.archive.util.FileUtils;
import org.json.JSONException;
import org.json.JSONObject;
import org.springframework.beans.factory.annotation.Autowired;
@@ -440,7 +441,7 @@ implements Lifecycle, Checkpointable, WriterPoolSettings {
File f = path.getFile();
if (!f.exists()) {
try {
f.mkdirs();
FileUtils.ensureWriteableDirectory(f);
} catch (Exception e) {
e.printStackTrace();
continue;
@@ -25,6 +25,7 @@ import org.archive.modules.CrawlMetadata;
import org.archive.modules.ProcessorTestBase;
import org.archive.modules.fetcher.DefaultServerCache;
import org.archive.spring.ConfigPath;
import org.archive.util.FileUtils;
import org.archive.util.TmpDirTestCase;
@@ -39,7 +40,7 @@ public class ARCWriterProcessorTest extends ProcessorTestBase {
protected Object makeModule() throws Exception {
File tmp = TmpDirTestCase.tmpDir();
tmp = new File(tmp, "ARCWriterProcessTest");
tmp.mkdirs();
FileUtils.ensureWriteableDirectory(tmp);
ARCWriterProcessor result = new ARCWriterProcessor();
result.setDirectory(new ConfigPath("test",tmp.getAbsolutePath()));
@@ -42,6 +42,7 @@ import org.archive.net.UURIFactory;
import org.archive.spring.ConfigPath;
import org.archive.uid.RecordIDGenerator;
import org.archive.uid.UUIDGenerator;
import org.archive.util.FileUtils;
import org.archive.util.TmpDirTestCase;
import org.archive.util.anvl.ANVLRecord;
@@ -57,8 +58,8 @@ public class WARCWriterProcessorTest extends ProcessorTestBase {
@Override
protected Object makeModule() throws Exception {
File tmp = TmpDirTestCase.tmpDir();
tmp = new File(tmp, "ARCWriterProcessTest");
tmp.mkdirs();
tmp = new File(tmp, "WARCWriterProcessTest");
FileUtils.ensureWriteableDirectory(tmp);
WARCWriterProcessor result = new WARCWriterProcessor();
result.setDirectory(new ConfigPath("test",tmp.getAbsolutePath()));