From 573866feee5d423ee4ebc7e27ddc677df06118b2 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Mon, 2 Dec 2013 21:49:55 -0500 Subject: [PATCH] improved multi-threaded processing and renamed things for clarity Former-commit-id: df63ca32884130892e89533f022a5df0e79c62ad --- .../data/update/DatabaseUpdater.java | 2 +- .../data/update/ProcessTask.java | 4 +- ...ardUpdateTask.java => StandardUpdate.java} | 91 ++++++++++--------- ...ava => StandardUpdateIntegrationTest.java} | 32 +++---- 4 files changed, 68 insertions(+), 61 deletions(-) rename dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/{StandardUpdateTask.java => StandardUpdate.java} (85%) rename dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/{StandardUpdateTaskIntegrationTest.java => StandardUpdateIntegrationTest.java} (78%) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/DatabaseUpdater.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/DatabaseUpdater.java index 61778eb66..bd531f35c 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/DatabaseUpdater.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/DatabaseUpdater.java @@ -46,7 +46,7 @@ public class DatabaseUpdater implements CachedWebDataSource { @Override public void update() throws UpdateException { try { - final StandardUpdateTask task = new StandardUpdateTask(); + final StandardUpdate task = new StandardUpdate(); if (task.isUpdateNeeded()) { if (task.shouldDeleteAndRecreate()) { try { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/ProcessTask.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/ProcessTask.java index 0c3f807b2..404ba6e7c 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/ProcessTask.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/ProcessTask.java @@ -120,7 +120,7 @@ public class ProcessTask implements Callable { private void processFiles() throws UpdateException { String msg = String.format("Processing Started for NVD CVE - %s", filePair.getNvdCveInfo().getId()); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, msg); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, msg); try { importXML(filePair.getFirst(), filePair.getSecond()); cveDB.commit(); @@ -143,6 +143,6 @@ public class ProcessTask implements Callable { filePair.cleanup(); } msg = String.format("Processing Complete for NVD CVE - %s", filePair.getNvdCveInfo().getId()); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, msg); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, msg); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/StandardUpdateTask.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/StandardUpdate.java similarity index 85% rename from dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/StandardUpdateTask.java rename to dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/StandardUpdate.java index ffd8b78ad..d46e1a76f 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/StandardUpdateTask.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/StandardUpdate.java @@ -44,11 +44,11 @@ import static org.owasp.dependencycheck.data.update.DataStoreMetaInfo.MODIFIED; import org.owasp.dependencycheck.utils.FileUtils; /** - * Class responsible for updating the CPE and NVDCVE data stores. + * Class responsible for updating the NVDCVE data store. * * @author Jeremy Long (jeremy.long@owasp.org) */ -public class StandardUpdateTask { +public class StandardUpdate { /** * The max thread pool size to use when downloading files. @@ -109,7 +109,7 @@ public class StandardUpdateTask { * @throws UpdateException thrown if there is an exception generating the * update task */ - public StandardUpdateTask() throws MalformedURLException, DownloadFailedException, UpdateException { + public StandardUpdate() throws MalformedURLException, DownloadFailedException, UpdateException { properties = new DataStoreMetaInfo(); updateable = updatesNeeded(); } @@ -133,7 +133,7 @@ public class StandardUpdateTask { return; } if (maxUpdates > 3) { - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, "NVD CVE requires several updates; this could take a couple of minutes."); } if (maxUpdates > 0) { @@ -159,37 +159,33 @@ public class StandardUpdateTask { } final CallableDownloadTask call = new CallableDownloadTask(cve, file1, file2); downloadFutures.add(downloadExecutor.submit(call)); - if (ctr == 3) { - ctr = 0; - final Iterator> itr = downloadFutures.iterator(); - while (itr.hasNext()) { - final Future future = itr.next(); - while (!future.isDone()) { - try { - Thread.sleep(1000); - } catch (InterruptedException ex) { - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, null, ex); - } - } + boolean waitForFuture = ctr % 2 == 0; - final CallableDownloadTask filePair; + final Iterator> itr = downloadFutures.iterator(); + while (itr.hasNext()) { + final Future future = itr.next(); + if (waitForFuture) { //only allow two NVD/CVE files to be downloaded at a time + spinWaitForFuture(future); + } + if (future.isDone()) { //if we find something complete, add it to the process queue try { - filePair = future.get(); + final CallableDownloadTask filePair = future.get(); + itr.remove(); + final ProcessTask task = new ProcessTask(cveDB, properties, filePair); + processFutures.add(processExecutor.submit(task)); } catch (InterruptedException ex) { downloadExecutor.shutdownNow(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "Thread was interupted", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "Thread was interupted", ex); throw new UpdateException(ex); } catch (ExecutionException ex) { downloadExecutor.shutdownNow(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.SEVERE, null, ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.SEVERE, null, ex); throw new UpdateException(ex); } - itr.remove(); - final ProcessTask task = new ProcessTask(cveDB, properties, filePair); - processFutures.add(processExecutor.submit(task)); } } + } } @@ -203,11 +199,11 @@ public class StandardUpdateTask { } } catch (InterruptedException ex) { downloadExecutor.shutdownNow(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "Thread was interupted during download", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "Thread was interupted during download", ex); throw new UpdateException(ex); } catch (ExecutionException ex) { downloadExecutor.shutdownNow(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "Execution Exception during download", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "Execution Exception during download", ex); throw new UpdateException(ex); } finally { downloadExecutor.shutdown(); @@ -221,11 +217,11 @@ public class StandardUpdateTask { } } catch (InterruptedException ex) { processExecutor.shutdownNow(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "Thread was interupted during processing", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "Thread was interupted during processing", ex); throw new UpdateException(ex); } catch (ExecutionException ex) { processExecutor.shutdownNow(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "Execution Exception during process", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "Execution Exception during process", ex); throw new UpdateException(ex); } finally { processExecutor.shutdown(); @@ -253,7 +249,7 @@ public class StandardUpdateTask { } } if (maxUpdates > 3) { - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, "NVD CVE requires several updates; this could take a couple of minutes."); } if (maxUpdates > 0) { @@ -264,13 +260,13 @@ public class StandardUpdateTask { for (NvdCveInfo cve : getUpdateable()) { if (cve.getNeedsUpdate()) { count += 1; - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, "Updating NVD CVE ({0} of {1})", new Object[]{count, maxUpdates}); URL url = new URL(cve.getUrl()); File outputPath = null; File outputPath12 = null; try { - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, "Downloading {0}", cve.getUrl()); outputPath = File.createTempFile("cve" + cve.getId() + "_", ".xml"); Downloader.fetchFile(url, outputPath); @@ -279,7 +275,7 @@ public class StandardUpdateTask { outputPath12 = File.createTempFile("cve_1_2_" + cve.getId() + "_", ".xml"); Downloader.fetchFile(url, outputPath12); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, "Processing {0}", cve.getUrl()); importXML(outputPath, outputPath12); @@ -287,7 +283,7 @@ public class StandardUpdateTask { getCveDB().commit(); getProperties().save(cve); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.INFO, + Logger.getLogger(StandardUpdate.class.getName()).log(Level.INFO, "Completed update {0} of {1}", new Object[]{count, maxUpdates}); } catch (FileNotFoundException ex) { throw new UpdateException(ex); @@ -360,11 +356,11 @@ public class StandardUpdateTask { } catch (InvalidDataException ex) { final String msg = "Unable to retrieve valid timestamp from nvd cve downloads page"; Logger - .getLogger(StandardUpdateTask.class + .getLogger(StandardUpdate.class .getName()).log(Level.FINE, msg, ex); throw new DownloadFailedException(msg, ex); } catch (InvalidSettingException ex) { - Logger.getLogger(StandardUpdateTask.class + Logger.getLogger(StandardUpdate.class .getName()).log(Level.FINE, "Invalid setting found when retrieving timestamps", ex); throw new DownloadFailedException( "Invalid settings", ex); @@ -420,7 +416,7 @@ public class StandardUpdateTask { final String msg = String.format("Error parsing '%s' '%s' from nvdcve.lastupdated", DataStoreMetaInfo.LAST_UPDATED_BASE, entry.getId()); Logger - .getLogger(StandardUpdateTask.class + .getLogger(StandardUpdate.class .getName()).log(Level.FINE, msg, ex); } if (currentTimestamp == entry.getTimestamp()) { @@ -432,9 +428,9 @@ public class StandardUpdateTask { } catch (NumberFormatException ex) { final String msg = "An invalid schema version or timestamp exists in the data.properties file."; Logger - .getLogger(StandardUpdateTask.class + .getLogger(StandardUpdate.class .getName()).log(Level.WARNING, msg); - Logger.getLogger(StandardUpdateTask.class + Logger.getLogger(StandardUpdate.class .getName()).log(Level.FINE, null, ex); } } @@ -498,7 +494,7 @@ public class StandardUpdateTask { try { cveDB.close(); } catch (Exception ignore) { - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINEST, "Error closing the cveDB", ignore); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINEST, "Error closing the cveDB", ignore); } } } @@ -515,19 +511,19 @@ public class StandardUpdateTask { cveDB.open(); } catch (IOException ex) { closeDataStores(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "IO Error opening databases", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "IO Error opening databases", ex); throw new UpdateException("Error updating the CPE/CVE data, please see the log file for more details."); } catch (SQLException ex) { closeDataStores(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "SQL Exception opening databases", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "SQL Exception opening databases", ex); throw new UpdateException("Error updating the CPE/CVE data, please see the log file for more details."); } catch (DatabaseException ex) { closeDataStores(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "Database Exception opening databases", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "Database Exception opening databases", ex); throw new UpdateException("Error updating the CPE/CVE data, please see the log file for more details."); } catch (ClassNotFoundException ex) { closeDataStores(); - Logger.getLogger(StandardUpdateTask.class.getName()).log(Level.FINE, "Class not found exception opening databases", ex); + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, "Class not found exception opening databases", ex); throw new UpdateException("Error updating the CPE/CVE data, please see the log file for more details."); } } @@ -547,4 +543,15 @@ public class StandardUpdateTask { final double differenceInDays = (compareTo - date) / 1000.0 / 60.0 / 60.0 / 24.0; return differenceInDays < range; } + + private void spinWaitForFuture(final Future future) { + //then wait for downloads to finish + while (!future.isDone()) { + try { + Thread.sleep(1000); + } catch (InterruptedException ex) { + Logger.getLogger(StandardUpdate.class.getName()).log(Level.FINE, null, ex); + } + } + } } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/StandardUpdateTaskIntegrationTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/StandardUpdateIntegrationTest.java similarity index 78% rename from dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/StandardUpdateTaskIntegrationTest.java rename to dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/StandardUpdateIntegrationTest.java index 0d2e5c1c7..1870360ea 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/StandardUpdateTaskIntegrationTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/StandardUpdateIntegrationTest.java @@ -34,9 +34,9 @@ import org.owasp.dependencycheck.utils.DownloadFailedException; * * @author Jeremy Long (jeremy.long@owasp.org) */ -public class StandardUpdateTaskIntegrationTest { +public class StandardUpdateIntegrationTest { - public StandardUpdateTaskIntegrationTest() { + public StandardUpdateIntegrationTest() { } @BeforeClass @@ -55,30 +55,30 @@ public class StandardUpdateTaskIntegrationTest { public void tearDown() { } - public StandardUpdateTask getStandardUpdateTask() throws MalformedURLException, DownloadFailedException, UpdateException { - StandardUpdateTask instance = new StandardUpdateTask(); + public StandardUpdate getStandardUpdateTask() throws MalformedURLException, DownloadFailedException, UpdateException { + StandardUpdate instance = new StandardUpdate(); return instance; } /** - * Test of setDeleteAndRecreate method, of class StandardUpdateTask. + * Test of setDeleteAndRecreate method, of class StandardUpdate. */ @Test public void testSetDeleteAndRecreate() throws Exception { boolean deleteAndRecreate = false; boolean expResult = false; - StandardUpdateTask instance = getStandardUpdateTask(); + StandardUpdate instance = getStandardUpdateTask(); instance.setDeleteAndRecreate(deleteAndRecreate); boolean result = instance.shouldDeleteAndRecreate(); assertEquals(expResult, result); } /** - * Test of deleteExistingData method, of class StandardUpdateTask. + * Test of deleteExistingData method, of class StandardUpdate. */ @Test public void testDeleteExistingData() throws Exception { - StandardUpdateTask instance = getStandardUpdateTask(); + StandardUpdate instance = getStandardUpdateTask(); Exception result = null; try { instance.deleteExistingData(); @@ -89,17 +89,17 @@ public class StandardUpdateTaskIntegrationTest { } /** - * Test of openDataStores method, of class StandardUpdateTask. + * Test of openDataStores method, of class StandardUpdate. */ @Test public void testOpenDataStores() throws Exception { - StandardUpdateTask instance = getStandardUpdateTask(); + StandardUpdate instance = getStandardUpdateTask(); instance.openDataStores(); instance.closeDataStores(); } /** - * Test of withinRange method, of class StandardUpdateTask. + * Test of withinRange method, of class StandardUpdate. */ @Test public void testWithinRange() throws Exception { @@ -108,7 +108,7 @@ public class StandardUpdateTaskIntegrationTest { long current = c.getTimeInMillis(); long lastRun = c.getTimeInMillis() - (3 * (1000 * 60 * 60 * 24)); int range = 7; // 7 days - StandardUpdateTask instance = getStandardUpdateTask(); + StandardUpdate instance = getStandardUpdateTask(); boolean expResult = true; boolean result = instance.withinRange(lastRun, current, range); assertEquals(expResult, result); @@ -120,21 +120,21 @@ public class StandardUpdateTaskIntegrationTest { } /** - * Test of update method, of class StandardUpdateTask. + * Test of update method, of class StandardUpdate. */ @Test public void testUpdate() throws Exception { - StandardUpdateTask instance = getStandardUpdateTask(); + StandardUpdate instance = getStandardUpdateTask(); instance.update(); //TODO make this an actual test } /** - * Test of updatesNeeded method, of class StandardUpdateTask. + * Test of updatesNeeded method, of class StandardUpdate. */ @Test public void testUpdatesNeeded() throws Exception { - StandardUpdateTask instance = getStandardUpdateTask(); + StandardUpdate instance = getStandardUpdateTask(); Updateable result = instance.updatesNeeded(); assertNotNull(result); }