Skip to content

Commit

Permalink
logging when we attempt to create the empty bundle files and increasing
Browse files Browse the repository at this point in the history 
the thread sleep time
  • Loading branch information
Andy Berry committed Jun 15, 2015
1 parent 033c759 commit e20f475
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
15 changes: 11 additions & 4 deletions ...legacy/src/main/java/org/bladerunnerjs/legacy/command/test/testrunner/BundlerHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

import org.apache.commons.lang3.StringUtils;
import org.bladerunnerjs.api.App;
import org.bladerunnerjs.api.BRJS;
import org.bladerunnerjs.api.logging.Logger;
import org.bladerunnerjs.api.model.exception.ModelOperationException;
import org.bladerunnerjs.api.model.exception.request.ContentProcessingException;
import org.bladerunnerjs.api.model.exception.request.MalformedRequestException;
Expand Down Expand Up @@ -47,7 +49,7 @@ public BundlerHandler(BundlableNode bundlableNode)
}


public void createBundleFile(File bundleFile, String bundlePath, String version) throws IOException, MalformedRequestException, ResourceNotFoundException, ContentProcessingException, ModelOperationException
public void createBundleFile(BRJS brjs, File bundleFile, String bundlePath, String version) throws IOException, MalformedRequestException, ResourceNotFoundException, ContentProcessingException, ModelOperationException
{
if (bundlePath.contains("\\"))
{
Expand All @@ -56,7 +58,7 @@ public void createBundleFile(File bundleFile, String bundlePath, String version)

String modelRequestPath = getModelRequestPath(bundlePath);

repeatedlyAttemptToCreateBundleFile(bundleFile);
repeatedlyAttemptToCreateBundleFile(brjs, bundleFile);

try (OutputStream bundleFileOutputStream = new FileOutputStream(bundleFile, false);
ResponseContent content = BundleSetRequestHandler.handle(new JsTestDriverBundleSet(bundlableNode.getBundleSet()), modelRequestPath, new StaticContentAccessor(app), version); )
Expand All @@ -69,19 +71,24 @@ public void createBundleFile(File bundleFile, String bundlePath, String version)

// this is a workaround for the scenario where Windows indexing service or virus scanners etc can lock the file when we try to create it
// see http://stackoverflow.com/a/10516563/2634854 for more info
private void repeatedlyAttemptToCreateBundleFile(File bundleFile) throws IOException
private void repeatedlyAttemptToCreateBundleFile(BRJS brjs, File bundleFile) throws IOException
{
if (bundleFile.exists()) {
throw new IOException( String.format("The bundle file '%s' already exists and should not. It should have previously been deleted so new content can be written to it", bundleFile.getAbsolutePath()) );
}
bundleFile.getParentFile().mkdirs();

Logger logger = brjs.logger(this.getClass());
for (int i = 0; i < 100; i++) {
try {
if (i % 10 == 0) { // print log line every 10 iterations

This comment has been minimized.

Sign in to view

Copy link
@dchambers

dchambers Jun 15, 2015

Contributor

@andyberry88, would it be better to just log to the user that it took N ms before we were able to write the file?

This comment has been minimized.

Sign in to view

Copy link
@andy-berry-dev

andy-berry-dev Jun 15, 2015

Member

Would that add any value? The reasoning behind the log line was just so a user who wonders why theres a pause for a second or so can see why. But logging at every attempt would be overkill and create too much noise.

Since we're now using the API properly and ensuring the file doesn't exist before this point I expect we can eventually remove this loop and just have bundleFile.createNewFile(). I think the only reason we've needed it was to fix a bug that we'd caused by using the API incorrectly.

This comment has been minimized.

Sign in to view

Copy link
@dchambers

dchambers Jun 15, 2015

Contributor

I suppose the thing that worried me about it is that it seems dodgy that we should ever have to wait at all, yet we only log if we had to wait for more than 1 second.

This comment has been minimized.

Sign in to view

Copy link
@andy-berry-dev

andy-berry-dev Jun 15, 2015

Member

I've removed the repeated attempts to create the bundle file since I don't think its needed at all now. I spotted a rather large bug in our deleteDirectoryFromBottomUp code which wasn't deleting populated directories properly which may well be the root cause of this and is probably the reason why we had FileUtils.deleteQuietly() everywhere.
logger.debug("Attempting to create an empty bundle file at '%s'.", bundleFile.getAbsolutePath());
}
bundleFile.createNewFile();
if (bundleFile.isFile()) {
return;
}
Thread.sleep(10);
Thread.sleep(100);
} catch (IOException | InterruptedException ex) {
// ignore the exception from creating the file or thread interupted
}
Expand Down
2 changes: 1 addition & 1 deletion ...main/java/org/bladerunnerjs/legacy/command/test/testrunner/JsTestDriverBundleCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public static void createRequiredBundles(BRJS brjs, MemoizedFile jsTestDriverCon
{
String bundlePath = StringUtils.substringAfterLast( requestedFile.getAbsolutePath(), BUNDLES_DIR_NAME+File.separator);
bundlePath = StringUtils.replace(bundlePath, "\\", "/");
bundlerHandler.createBundleFile(requestedFile, bundlePath, brjs.getAppVersionGenerator().getVersion());
bundlerHandler.createBundleFile(brjs, requestedFile, bundlePath, brjs.getAppVersionGenerator().getVersion());
}
}
MemoizedFile testsDir = jsTestDriverConf.getParentFile().file("tests");
Expand Down
2 changes: 1 addition & 1 deletion .../org/bladerunnerjs/legacy/command/test/testrunner/specutility/BundlerHandlerSpecTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void runWithPaths(String... requestPaths) throws Exception
File bundleFile = testPack.file(requestPath);
String bundlePath = StringUtils.substringAfterLast(bundleFile.getAbsolutePath(), JsTestDriverBundleCreator.BUNDLES_DIR_NAME + File.separator);
bundlePath = StringUtils.replace(bundlePath, "\\", "/");
new BundlerHandler(testPack).createBundleFile(bundleFile, bundlePath, brjs.getAppVersionGenerator().getVersion());
new BundlerHandler(testPack).createBundleFile(brjs, bundleFile, bundlePath, brjs.getAppVersionGenerator().getVersion());
}
}
}
Expand Down

0 comments on commit e20f475

Please sign in to comment.