From 242708bc71078a6250507838b9543aae9e5c9411 Mon Sep 17 00:00:00 2001 From: Mike McGarr Date: Wed, 9 Jan 2019 08:55:01 -0800 Subject: [PATCH 1/2] Incremental asset baking while serving This is the first step to improving bake performance. The scope of this change is focused solely on copying newly changed asset files while serving content. This should work per asset file as it is changed. It should also copy asset files that exist in the content directory. --- .../src/main/java/org/jbake/app/Asset.java | 89 +++++++++++++++---- .../src/main/java/org/jbake/app/FileUtil.java | 15 ++++ .../src/main/java/org/jbake/app/Oven.java | 17 +++- .../launcher/CustomFSChangeListener.java | 14 +-- .../test/java/org/jbake/app/AssetTest.java | 57 +++++++++++- .../test/java/org/jbake/app/FileUtilTest.java | 15 ++++ 6 files changed, 180 insertions(+), 27 deletions(-) diff --git a/jbake-core/src/main/java/org/jbake/app/Asset.java b/jbake-core/src/main/java/org/jbake/app/Asset.java index 5d4f7c7e1..e45efc8ca 100644 --- a/jbake-core/src/main/java/org/jbake/app/Asset.java +++ b/jbake-core/src/main/java/org/jbake/app/Asset.java @@ -71,21 +71,78 @@ public boolean accept(File file) { copy(path, config.getDestinationFolder(), filter); } - private void copy(File sourceFolder, File targetFolder, final FileFilter filter) { + /** + * Copy one asset file at a time. + * + * @param asset The asset file to copy + */ + public void copySingleFile(File asset) { + try { + String targetPath = config.getDestinationFolder().getCanonicalPath() + File.separatorChar + assetSubPath(asset); + LOGGER.info("Copying single asset file to [" + targetPath + "]"); + copyFile(asset, new File(targetPath)); + } catch (IOException io) { + LOGGER.error("Failed to copy the asset file.", io); + } + } + + /** + * Determine if a given file is an asset file. + * @param path to the file to validate. + * @return true if the path provided points to a file in the asset folder. + */ + public boolean isAssetFile(File path) { + boolean isAsset = false; + + try { + if(FileUtil.directoryOnlyIfNotIgnored(path.getParentFile())) { + if (FileUtil.isFileInDirectory(path, config.getAssetFolder())) { + isAsset = true; + } else if (FileUtil.isFileInDirectory(path, config.getContentFolder()) + && FileUtil.getNotContentFileFilter().accept(path)) { + isAsset = true; + } + } + } catch (IOException ioe) { + LOGGER.error("Unable to determine the path to asset file {}", path.getPath(), ioe); + } + return isAsset; + } + + /** + * Responsible for copying any asset files that exist within the content directory. + * + * @param path of the content directory + */ + public void copyAssetsFromContent(File path) { + copy(path, config.getDestinationFolder(), FileUtil.getNotContentFileFilter()); + } + + /** + * Accessor method to the collection of errors generated during the bake + * + * @return a list of errors. + */ + public List getErrors() { + return new ArrayList<>(errors); + } + + private String assetSubPath(File asset) throws IOException { + // First, strip asset folder from file path + String targetFolder = asset.getCanonicalPath().replace(config.getAssetFolder().getCanonicalPath() + File.separatorChar, ""); + // And just to be sure, let's also remove the content folder, as some assets are copied from here. + targetFolder = targetFolder.replace(config.getContentFolder().getCanonicalPath() + File.separatorChar, ""); + return targetFolder; + } + private void copy(File sourceFolder, File targetFolder, final FileFilter filter) { final File[] assets = sourceFolder.listFiles(filter); if (assets != null) { Arrays.sort(assets); for (File asset : assets) { final File target = new File(targetFolder, asset.getName()); if (asset.isFile()) { - try { - FileUtils.copyFile(asset, target); - LOGGER.info("Copying [{}]... done!", asset.getPath()); - } catch (IOException e) { - LOGGER.error("Copying [{}]... failed!", asset.getPath(), e); - errors.add(e); - } + copyFile(asset, target); } else if (asset.isDirectory()) { copy(asset, target, filter); } @@ -93,13 +150,13 @@ private void copy(File sourceFolder, File targetFolder, final FileFilter filter) } } - public void copyAssetsFromContent(File path) { - copy(path, config.getDestinationFolder(), FileUtil.getNotContentFileFilter()); - } - - - public List getErrors() { - return new ArrayList<>(errors); + private void copyFile(File asset, File targetFolder) { + try { + FileUtils.copyFile(asset, targetFolder); + LOGGER.info("Copying [{}]... done!", asset.getPath()); + } catch (IOException e) { + LOGGER.error("Copying [{}]... failed!", asset.getPath(), e); + errors.add(e); + } } - } diff --git a/jbake-core/src/main/java/org/jbake/app/FileUtil.java b/jbake-core/src/main/java/org/jbake/app/FileUtil.java index 049474d60..cbbf2933a 100644 --- a/jbake-core/src/main/java/org/jbake/app/FileUtil.java +++ b/jbake-core/src/main/java/org/jbake/app/FileUtil.java @@ -181,4 +181,19 @@ public static String asPath(String path) { } return path.replace(File.separator, "/"); } + + /** + * Utility method to determine if a given file is located somewhere in the directory provided. + * + * @param file used to check if it is located in the provided directory. + * @param directory to validate whether or not the provided file resides. + * @return true if the file is somewhere in the provided directory, false if it is not. + * @throws IOException if the canonical path for either of the input directories can't be determined. + */ + public static boolean isFileInDirectory(File file, File directory) throws IOException { + return (file.exists() + && !file.isHidden() + && directory.isDirectory() + && file.getCanonicalPath().startsWith(directory.getCanonicalPath())); + } } diff --git a/jbake-core/src/main/java/org/jbake/app/Oven.java b/jbake-core/src/main/java/org/jbake/app/Oven.java index 9eb9a6781..07b391491 100644 --- a/jbake-core/src/main/java/org/jbake/app/Oven.java +++ b/jbake-core/src/main/java/org/jbake/app/Oven.java @@ -114,6 +114,22 @@ private void checkConfiguration(JBakeConfiguration configuration) { inspector.inspect(); } + /** + * Responsible for incremental baking, typically a single file at a time. + * + * @param fileToBake The file to bake + */ + public void bake(File fileToBake) { + Asset asset = utensils.getAsset(); + if(asset.isAssetFile(fileToBake)) { + LOGGER.info("Baking a change to an asset [" + fileToBake.getPath() + "]"); + asset.copySingleFile(fileToBake); + } else { + LOGGER.info("Playing it safe and running a full bake..."); + bake(); + } + } + /** * All the good stuff happens in here... */ @@ -195,7 +211,6 @@ private void renderContent() { } } - public List getErrors() { return new ArrayList<>(errors); } diff --git a/jbake-core/src/main/java/org/jbake/launcher/CustomFSChangeListener.java b/jbake-core/src/main/java/org/jbake/launcher/CustomFSChangeListener.java index 20065cd8c..bd8135b54 100644 --- a/jbake-core/src/main/java/org/jbake/launcher/CustomFSChangeListener.java +++ b/jbake-core/src/main/java/org/jbake/launcher/CustomFSChangeListener.java @@ -2,11 +2,14 @@ import org.apache.commons.vfs2.FileChangeEvent; import org.apache.commons.vfs2.FileListener; +import org.apache.commons.vfs2.FileObject; import org.jbake.app.Oven; import org.jbake.app.configuration.JBakeConfiguration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; + public class CustomFSChangeListener implements FileListener { private final static Logger LOGGER = LoggerFactory.getLogger(CustomFSChangeListener.class); @@ -20,24 +23,23 @@ public CustomFSChangeListener(JBakeConfiguration config) { @Override public void fileCreated(FileChangeEvent event) throws Exception { LOGGER.info("File created event detected: {}", event.getFile().getURL()); - exec(); + exec(event.getFile()); } @Override public void fileDeleted(FileChangeEvent event) throws Exception { LOGGER.info("File deleted event detected: {}", event.getFile().getURL()); - exec(); + exec(event.getFile()); } @Override public void fileChanged(FileChangeEvent event) throws Exception { LOGGER.info("File changed event detected: {}", event.getFile().getURL()); - exec(); + exec(event.getFile()); } - private void exec() { + private void exec(FileObject file) { final Oven oven = new Oven(config); - oven.bake(); + oven.bake(new File(file.getName().getPath())); } - } diff --git a/jbake-core/src/test/java/org/jbake/app/AssetTest.java b/jbake-core/src/test/java/org/jbake/app/AssetTest.java index 7de51ed93..5784dc3f7 100644 --- a/jbake-core/src/test/java/org/jbake/app/AssetTest.java +++ b/jbake-core/src/test/java/org/jbake/app/AssetTest.java @@ -18,10 +18,12 @@ public class AssetTest { private DefaultJBakeConfiguration config; + private File fixtureDir; @Before public void setup() throws Exception { - config = (DefaultJBakeConfiguration) new ConfigUtil().loadConfig(new File(this.getClass().getResource("/fixture").getFile())); + fixtureDir = new File(this.getClass().getResource("/fixture").getFile()); + config = (DefaultJBakeConfiguration) new ConfigUtil().loadConfig(fixtureDir); config.setDestinationFolder(folder.getRoot()); Assert.assertEquals(".html", config.getOutputExtension()); } @@ -30,7 +32,7 @@ public void setup() throws Exception { public TemporaryFolder folder = new TemporaryFolder(); @Test - public void copy() throws Exception { + public void testCopy() throws Exception { Asset asset = new Asset(config); asset.copy(); @@ -45,7 +47,29 @@ public void copy() throws Exception { } @Test - public void copyCustomFolder() throws Exception { + public void testCopySingleFile() throws Exception { + Asset asset = new Asset(config); + String cssSubPath = File.separatorChar + "css" + File.separatorChar + "bootstrap.min.css"; + String contentImgPath = File.separatorChar + "blog" + File.separatorChar + "2013" + File.separatorChar + + "images" + File.separatorChar + "custom-image.jpg"; + + // Copy single Asset File + File expected = new File(folder.getRoot().getPath() + cssSubPath); + Assert.assertFalse("cssFile should not exist before running the test; avoids false positives", expected.exists()); + File cssFile = new File(fixtureDir.getPath() + File.separatorChar + "assets" + cssSubPath); + asset.copySingleFile(cssFile); + Assert.assertTrue("Css asset file did not copy", expected.exists()); + + // Copy single Content file + expected = new File(folder.getRoot().getPath() + contentImgPath); + Assert.assertFalse("content image file should not exist before running the test", expected.exists()); + File imgFile = new File(fixtureDir.getPath() + File.separatorChar + "content" + contentImgPath); + asset.copySingleFile(imgFile); + Assert.assertTrue("Content img file did not copy", expected.exists()); + } + + @Test + public void testCopyCustomFolder() throws Exception { config.setAssetFolder(new File(config.getSourceFolder(),"/media")); Asset asset = new Asset(config); asset.copy(); @@ -57,7 +81,7 @@ public void copyCustomFolder() throws Exception { } @Test - public void copyIgnore() throws Exception { + public void testCopyIgnore() throws Exception { File assetFolder = folder.newFolder("ignoredAssets"); FileUtils.copyDirectory(new File(this.getClass().getResource("/fixture/ignorables").getFile()), assetFolder); config.setAssetFolder(assetFolder); @@ -152,6 +176,31 @@ public void testCopyAssetsFromContent(){ Assert.assertTrue("Errors during asset copying", asset.getErrors().isEmpty()); } + @Test + public void testIsFileAsset() { + File cssAsset = new File(config.getAssetFolder().getAbsolutePath() + File.separatorChar + "css" + File.separatorChar + "bootstrap.min.css"); + Assert.assertTrue(cssAsset.exists()); + File contentFile = new File(config.getContentFolder().getAbsolutePath() + File.separatorChar + "about.html"); + Assert.assertTrue(contentFile.exists()); + Asset asset = new Asset(config); + + Assert.assertTrue(asset.isAssetFile(cssAsset)); + Assert.assertFalse(asset.isAssetFile(contentFile)); + } + + /* + @Test + public void testAssetTargetFolder() throws Exception { + Asset asset = new Asset(config); + + File cssAsset = new File(config.getAssetFolder().getAbsolutePath() + File.separatorChar + "css" + File.separatorChar + "bootstrap.min.css"); + Assert.assertEquals("css", asset.assetTargetFolder(cssAsset)); + + cssAsset = new File(config.getAssetFolder().getAbsolutePath() + File.separatorChar + "css" + File.separatorChar + "foobar" + File.separatorChar + "bootstrap.min.css"); + Assert.assertEquals("css/foobar", asset.assetTargetFolder(cssAsset)); + } + */ + private Integer countFiles(File path){ int total = 0; FileFilter filesOnly = FileFilterUtils.fileFileFilter(); diff --git a/jbake-core/src/test/java/org/jbake/app/FileUtilTest.java b/jbake-core/src/test/java/org/jbake/app/FileUtilTest.java index c91bba49f..e1e654db4 100644 --- a/jbake-core/src/test/java/org/jbake/app/FileUtilTest.java +++ b/jbake-core/src/test/java/org/jbake/app/FileUtilTest.java @@ -4,7 +4,9 @@ import java.io.File; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; /** * Created by frank on 28.03.16. @@ -17,4 +19,17 @@ public void testGetRunningLocation() throws Exception { File path = FileUtil.getRunningLocation(); assertEquals(new File("build/classes").getAbsolutePath(), path.getPath()); } + + @Test + public void testIsFileInDirectory() throws Exception { + File fixtureDir = new File(this.getClass().getResource("/fixture").getFile()); + File jbakeFile = new File(fixtureDir.getCanonicalPath() + File.separatorChar + "jbake.properties"); + assertTrue("jbake.properties expected to be in /fixture directory", FileUtil.isFileInDirectory(jbakeFile, fixtureDir)); + + File contentFile = new File(fixtureDir.getCanonicalPath() + File.separatorChar + "content" + File.separatorChar + "projects.html"); + assertTrue("projects.html expected to be nested in the /fixture directory", FileUtil.isFileInDirectory(contentFile, fixtureDir)); + + File contentDir = contentFile.getParentFile(); + assertFalse("jbake.properties file should not be in the /fixture/content directory", FileUtil.isFileInDirectory(jbakeFile, contentDir)); + } } From 58a6f9fe18128a296d6f75f9e987ee6f49031095 Mon Sep 17 00:00:00 2001 From: Mike McGarr Date: Thu, 17 Jan 2019 18:13:06 -0800 Subject: [PATCH 2/2] Removing commented out test method --- .../src/test/java/org/jbake/app/AssetTest.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/jbake-core/src/test/java/org/jbake/app/AssetTest.java b/jbake-core/src/test/java/org/jbake/app/AssetTest.java index 5784dc3f7..975625036 100644 --- a/jbake-core/src/test/java/org/jbake/app/AssetTest.java +++ b/jbake-core/src/test/java/org/jbake/app/AssetTest.java @@ -188,19 +188,6 @@ public void testIsFileAsset() { Assert.assertFalse(asset.isAssetFile(contentFile)); } - /* - @Test - public void testAssetTargetFolder() throws Exception { - Asset asset = new Asset(config); - - File cssAsset = new File(config.getAssetFolder().getAbsolutePath() + File.separatorChar + "css" + File.separatorChar + "bootstrap.min.css"); - Assert.assertEquals("css", asset.assetTargetFolder(cssAsset)); - - cssAsset = new File(config.getAssetFolder().getAbsolutePath() + File.separatorChar + "css" + File.separatorChar + "foobar" + File.separatorChar + "bootstrap.min.css"); - Assert.assertEquals("css/foobar", asset.assetTargetFolder(cssAsset)); - } - */ - private Integer countFiles(File path){ int total = 0; FileFilter filesOnly = FileFilterUtils.fileFileFilter();