modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java | 159 ++++++---- 1 file changed, 98 insertions(+), 61 deletions(-)
New commits: commit 83197699d41880beb6266fc47d38cbf201bce7a4 Author: Jay Shaughnessy jshaughn@redhat.com Date: Mon Dec 5 15:55:31 2011 -0500
[Bug 760145 - Drift snapshot displays wrong directory name on windows] This is fairly bad as the paths get corrupted for all drift entries when the basedir is a windows file system root (e.g. c:/ or d:/). There is no workaround other than not using the file system root as the basedir. I'm not sure, this may also happen using the a linux root "/" basedir.
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java index 0ba5795..71ddf44 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java @@ -538,7 +538,13 @@ public class DriftDetector implements Runnable { if (basedir.equals(file)) { return "."; } - return file.getAbsolutePath().substring(basedir.getAbsolutePath().length() + 1); + String filePath = file.getAbsolutePath(); + String basedirPath = basedir.getAbsolutePath(); + int basedirLen = basedirPath.length(); + if (!basedirPath.endsWith(File.separator)) { + ++basedirLen; + } + return filePath.substring(basedirLen); }
private String sha256(File file) throws IOException {
commit 6f3d99d160c4910bffe16ada89b625fe251bea44 Author: Jay Shaughnessy jshaughn@redhat.com Date: Mon Dec 5 15:51:07 2011 -0500
[Bug 760289 - Excessive file scanning in drift detection when using includes filters] Now, when using includes file paths limit the directory scanning to only those included directories.
Note that using a "." as an includes path basically translates to using the base directory, in which case the scan will be as it is now. A future enhancement may be to analyze the pattern and decide whether a recursive scan is necessary. Currently. So, using includes patterns to just look for certain files in the base directory will expose you to the full scan.
diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java index 3f32f1f..0ba5795 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/drift/DriftDetector.java @@ -45,6 +45,7 @@ import org.rhq.common.drift.FileEntry; import org.rhq.common.drift.Headers; import org.rhq.core.domain.drift.DriftChangeSetCategory; import org.rhq.core.domain.drift.DriftDefinition; +import org.rhq.core.domain.drift.Filter; import org.rhq.core.util.MessageDigestGenerator; import org.rhq.core.util.file.FileVisitor;
@@ -167,13 +168,12 @@ public class DriftDetector implements Runnable {
log.debug("Generating drift change set for " + schedule);
- File currentSnapshot = changeSetMgr.findChangeSet(schedule.getResourceId(), - schedule.getDriftDefinition().getName(), COVERAGE); + File currentSnapshot = changeSetMgr.findChangeSet(schedule.getResourceId(), schedule.getDriftDefinition() + .getName(), COVERAGE); File snapshotFile = currentSnapshot;
if (schedule.getDriftDefinition().isPinned()) { snapshotFile = new File(snapshotFile.getParentFile(), "snapshot.pinned"); - }
final File basedir = new File(basedir(schedule.getResourceId(), schedule.getDriftDefinition())); @@ -203,43 +203,50 @@ public class DriftDetector implements Runnable { }
// First look for files that have either been modified or deleted - scanForModifiedOrDeletedFiles(schedule, basedir, processedFiles, snapshotEntries, deltaEntries, coverageReader); + scanForModifiedOrDeletedFiles(schedule, basedir, processedFiles, snapshotEntries, deltaEntries, + coverageReader); } finally { coverageReader.close(); }
// If the basedir is still valid we need to do a directory tree scan to look for newly added files if (basedir.isDirectory()) { - forEachFile(basedir, new FilterFileVisitor(basedir, schedule.getDriftDefinition().getIncludes(), schedule - .getDriftDefinition().getExcludes(), new FileVisitor() { - @Override - public void visit(File file) { - try { - if (processedFiles.contains(file)) { - return; - } + DriftDefinition driftDef = schedule.getDriftDefinition(); + List<Filter> includes = driftDef.getIncludes(); + List<Filter> excludes = driftDef.getExcludes(); + + for (File dir : getScanDirectories(basedir, includes)) { + forEachFile(dir, new FilterFileVisitor(basedir, includes, excludes, new FileVisitor() { + @Override + public void visit(File file) { + try { + if (processedFiles.contains(file)) { + return; + }
- if (!file.canRead()) { - if (log.isDebugEnabled()) { - log.debug("Skipping " + file.getPath() + " since it is not readable."); + if (!file.canRead()) { + if (log.isDebugEnabled()) { + log.debug("Skipping " + file.getPath() + " since it is not readable."); + } + return; } - return; - }
- if (log.isInfoEnabled()) { - log.info("Detected added file for " + schedule + " --> " + file.getAbsolutePath()); - } + if (log.isInfoEnabled()) { + log.info("Detected added file for " + schedule + " --> " + file.getAbsolutePath()); + }
- FileEntry newEntry = addedFileEntry(relativePath(basedir, file), sha256(file)); - deltaEntries.add(newEntry); - snapshotEntries.add(newEntry); - } catch (IOException e) { - log.error("An error occurred while generating a drift change set for " + schedule + ": " - + e.getMessage()); - throw new DriftDetectionException("An error occurred while generating a drift change set", e); + FileEntry newEntry = addedFileEntry(relativePath(basedir, file), sha256(file)); + deltaEntries.add(newEntry); + snapshotEntries.add(newEntry); + } catch (IOException e) { + log.error("An error occurred while generating a drift change set for " + schedule + ": " + + e.getMessage()); + throw new DriftDetectionException("An error occurred while generating a drift change set", + e); + } } - } - })); + })); + } }
if (deltaEntries.isEmpty()) { @@ -250,8 +257,8 @@ public class DriftDetector implements Runnable { // the current snapshot to match the pinned snapshot. Note though // that we increment the snapshot version in order to let the server // know about the state change. - if (schedule.getDriftDefinition().isPinned() && newVersion > 1 && !isPreviousChangeSetEmpty( - schedule.getResourceId(), schedule.getDriftDefinition())) { + if (schedule.getDriftDefinition().isPinned() && newVersion > 1 + && !isPreviousChangeSetEmpty(schedule.getResourceId(), schedule.getDriftDefinition())) { currentSnapshot.delete(); File newSnapshot = updateCurrentSnapshot(schedule, snapshotEntries, newVersion);
@@ -259,8 +266,8 @@ public class DriftDetector implements Runnable { // TODO report back to the server that we are back in compliance } } else { - if (schedule.getDriftDefinition().isPinned() && newVersion > 1 && - isSameAsPreviousChangeSet(deltaEntries, currentSnapshot)) { + if (schedule.getDriftDefinition().isPinned() && newVersion > 1 + && isSameAsPreviousChangeSet(deltaEntries, currentSnapshot)) { // if we are still out of compliance just report, we report a // repeat change set to indicate no changes but also still out // of compliance. @@ -276,6 +283,26 @@ public class DriftDetector implements Runnable { } }
+ private Set<File> getScanDirectories(final File basedir, List<Filter> includes) { + + Set<File> directories = new HashSet<File>(); + + if (null == includes || includes.isEmpty()) { + directories.add(basedir); + } else { + for (Filter filter : includes) { + String path = filter.getPath(); + if (".".equals(path)) { + directories.add(basedir); + } else { + directories.add(new File(basedir, path)); + } + } + } + + return directories; + } + private void scanForModifiedOrDeletedFiles(DriftDetectionSchedule schedule, File basedir, Set<File> processedFiles, List<FileEntry> snapshotEntries, List<FileEntry> deltaEntries, ChangeSetReader coverageReader) throws IOException { @@ -312,6 +339,7 @@ public class DriftDetector implements Runnable { } }
+ @SuppressWarnings("unused") private boolean isPreviousChangeSetEmpty(int resourceId, DriftDefinition definition) throws IOException { File changeSet = changeSetMgr.findChangeSet(resourceId, definition.getName(), DRIFT); if (!changeSet.exists()) { @@ -337,15 +365,14 @@ public class DriftDetector implements Runnable { }
private void updateDeltaSnapshot(DriftDetectionSummary summary, DriftDetectionSchedule schedule, - List<FileEntry> deltaEntries, int newVersion, File oldSnapshot, File newSnapshot) - throws IOException { + List<FileEntry> deltaEntries, int newVersion, File oldSnapshot, File newSnapshot) throws IOException {
ChangeSetWriter deltaWriter = null;
try { Headers deltaHeaders = createHeaders(schedule, DRIFT, newVersion); - File driftChangeSet = changeSetMgr.findChangeSet(schedule.getResourceId(), - schedule.getDriftDefinition().getName(), DRIFT); + File driftChangeSet = changeSetMgr.findChangeSet(schedule.getResourceId(), schedule.getDriftDefinition() + .getName(), DRIFT); deltaWriter = changeSetMgr.getChangeSetWriter(driftChangeSet, deltaHeaders);
summary.setDriftChangeSet(driftChangeSet); @@ -376,10 +403,9 @@ public class DriftDetector implements Runnable {
try { Headers snapshotHeaders = createHeaders(schedule, COVERAGE, newVersion); - File newSnapshot = changeSetMgr.findChangeSet(schedule.getResourceId(), - schedule.getDriftDefinition().getName(), COVERAGE); - newSnapshotWriter = changeSetMgr.getChangeSetWriter(schedule.getResourceId(), - snapshotHeaders); + File newSnapshot = changeSetMgr.findChangeSet(schedule.getResourceId(), schedule.getDriftDefinition() + .getName(), COVERAGE); + newSnapshotWriter = changeSetMgr.getChangeSetWriter(schedule.getResourceId(), snapshotHeaders);
for (FileEntry entry : snapshotEntries) { newSnapshotWriter.write(entry); @@ -411,19 +437,19 @@ public class DriftDetector implements Runnable { return false; } switch (entry.getType()) { - case FILE_ADDED: - if (!entry.getNewSHA().equals(newEntry.getNewSHA())) { - return false; - } - case FILE_CHANGED: - if (!entry.getNewSHA().equals(newEntry.getNewSHA()) || - !entry.getOldSHA().equals(newEntry.getOldSHA())) { - return false; - } - default: // FILE_REMOVED - if (!entry.getOldSHA().equals(newEntry.getOldSHA())) { - return false; - } + case FILE_ADDED: + if (!entry.getNewSHA().equals(newEntry.getNewSHA())) { + return false; + } + case FILE_CHANGED: + if (!entry.getNewSHA().equals(newEntry.getNewSHA()) + || !entry.getOldSHA().equals(newEntry.getOldSHA())) { + return false; + } + default: // FILE_REMOVED + if (!entry.getOldSHA().equals(newEntry.getOldSHA())) { + return false; + } } numEntries++; } @@ -443,9 +469,9 @@ public class DriftDetector implements Runnable {
if (!basedir.exists()) { if (log.isWarnEnabled()) { - log.warn("The base directory [" + basedir.getAbsolutePath() + "] for " + schedule + " does not " + - "exist. You may want review the drift definition and verify that the value of the base " + - "directory is in fact correct."); + log.warn("The base directory [" + basedir.getAbsolutePath() + "] for " + schedule + " does not " + + "exist. You may want review the drift definition and verify that the value of the base " + + "directory is in fact correct."); } summary.setBaseDirExists(false); return; @@ -477,9 +503,13 @@ public class DriftDetector implements Runnable { }
private void doDirectoryScan(final DriftDetectionSchedule schedule, DriftDefinition driftDef, final File basedir, - final ChangeSetWriter writer) { - forEachFile(basedir, new FilterFileVisitor(basedir, driftDef.getIncludes(), driftDef.getExcludes(), - new FileVisitor() { + final ChangeSetWriter writer) { + + List<Filter> includes = driftDef.getIncludes(); + List<Filter> excludes = driftDef.getExcludes(); + + for (File dir : getScanDirectories(basedir, includes)) { + forEachFile(dir, new FilterFileVisitor(basedir, includes, excludes, new FileVisitor() { @Override public void visit(File file) { try { @@ -501,6 +531,7 @@ public class DriftDetector implements Runnable { } } })); + } }
private String relativePath(File basedir, File file) {
rhq-commits@lists.fedorahosted.org