Skip to content

Double-check for directory exists in the ensureParentDirExists(File) #978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public URLOutputStream(URL url, String method, Map<String, String> headers, int

private void ensureParentDirExists(File file) throws IOException {
if (file.getParentFile() != null && !file.getParentFile().isDirectory()) {
boolean ok = file.getParentFile().mkdirs();
boolean ok = file.getParentFile().mkdirs() || file.getParentFile().isDirectory();
if (!ok) {
throw new IOException("Failed to create directory " + file.getParentFile().getAbsolutePath());
}
Expand Down
107 changes: 88 additions & 19 deletions core/src/test/java/cucumber/runtime/io/URLOutputStreamTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,36 @@
import gherkin.util.FixJava;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.webbitserver.HttpControl;
import org.webbitserver.HttpHandler;
import org.webbitserver.HttpRequest;
import org.webbitserver.HttpResponse;
import org.webbitserver.WebServer;
import org.junit.rules.TemporaryFolder;
import org.webbitserver.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No star imports please, and please undo reordering of imports. Try to reduce the PR to significant changes only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops, it wasn't me, it was Intellij Idea. Sorry. Fixed in commits 6ad8186, cf811be

import org.webbitserver.netty.NettyWebServer;
import org.webbitserver.rest.Rest;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.io.Writer;
import java.io.*;
import java.net.InetSocketAddress;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.TimeUnit;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.*;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.junit.Assert.*;

public class URLOutputStreamTest {
private WebServer webbit;
private final int threadsCount = 100;
private final long waitTimeoutMillis = 30000L;
private final List<File> tmpFiles = new ArrayList<File>();
private final List<String> threadErrors = new ArrayList<String>();

@Rule
public TemporaryFolder tempFolder = new TemporaryFolder();

@Before
public void startWebbit() throws ExecutionException, InterruptedException {
Expand Down Expand Up @@ -127,7 +123,80 @@ public void handleHttpRequest(HttpRequest req, HttpResponse res, HttpControl ctl
}
}

@Test
public void do_not_throw_ioe_if_parent_dir_created_by_another_thread() {
final CountDownLatch countDownLatch = new CountDownLatch(1);
List<Thread> testThreads = getThreadsWithLatchForFile(countDownLatch, threadsCount);
startThreadsFromList(testThreads);
countDownLatch.countDown();
waitAllThreadsFromList(testThreads);
assertTrue("Not all parent folders were created for tmp file or tmp file was not created", isAllFilesCreated());
assertTrue("Some thread get error during work. Error list:" + threadErrors.toString(), threadErrors.isEmpty());
}

private Reader openUTF8FileReader(final File file) throws IOException {
return new InputStreamReader(new FileInputStream(file), Charset.forName("UTF-8"));
}

private List<Thread> getThreadsWithLatchForFile(final CountDownLatch countDownLatch, int threadsCount) {
List<Thread> result = new ArrayList<Thread>();
String ballast = "" + System.currentTimeMillis();
for (int i = 0; i < threadsCount; i++) {
final int curThreadNo = i;
// It useful when 2-3 threads (not more) tries to create the same directory for the report
final File tmp = (i % 3 == 0 || i % 3 == 2) ?
new File(tempFolder.getRoot().getAbsolutePath() + "/cuce" + ballast + i + "/tmpFile.tmp") :
new File(tempFolder.getRoot().getAbsolutePath() + "/cuce" + ballast + (i - 1) + "/tmpFile.tmp");
tmpFiles.add(tmp);
result.add(new Thread() {
@Override
public void run() {
try {
// Every thread should wait command to run
countDownLatch.await();
new URLOutputStream(tmp.toURI().toURL());
} catch (IOException e) {
threadErrors.add("Thread" + curThreadNo + ": parent dir not created. " + e.getMessage());
} catch (InterruptedException e) {
threadErrors.add("Thread" + curThreadNo + ": not started on time. " + e.getMessage());
}
}
});
}
return result;
}

private void startThreadsFromList(List<Thread> threads) {
for (Thread thread : threads) {
thread.start();
}
}

private void waitAllThreadsFromList(List<Thread> threads) {
long timeStart = System.currentTimeMillis();
do {
// Protection from forever loop
if (System.currentTimeMillis() - timeStart > waitTimeoutMillis) {
assertTrue("Some threads are still alive", false);
}
} while (hasListAliveThreads(threads));
}

private boolean hasListAliveThreads(List<Thread> threads) {
for (Thread thread : threads) {
if (thread.isAlive()) {
return true;
}
}
return false;
}

private boolean isAllFilesCreated() {
for (File tmpFile : tmpFiles) {
if (tmpFile.getParentFile() == null || !tmpFile.getParentFile().isDirectory() || !tmpFile.exists()) {
return false;
}
}
return true;
}
}