diff --git a/dev/checkstyle/suppressions.xml b/dev/checkstyle/suppressions.xml index 4b6bd720329..b4da4777c11 100644 --- a/dev/checkstyle/suppressions.xml +++ b/dev/checkstyle/suppressions.xml @@ -49,6 +49,8 @@ Portions Copyright (c) 2018-2020, Chris Fraire . + + diff --git a/opengrok-indexer/pom.xml b/opengrok-indexer/pom.xml index b47f5fd2fec..a05b91d1d59 100644 --- a/opengrok-indexer/pom.xml +++ b/opengrok-indexer/pom.xml @@ -210,8 +210,26 @@ Portions Copyright (c) 2020-2020, Lubos Kosco . micrometer-registry-prometheus ${micrometer.version} + + com.oath.halodb + halodb + 0.5.3 + + + org.slf4j + slf4j-nop + 1.7.26 + + + + yahoo-bintray + yahoo-bintray + https://yahoo.bintray.com/maven + + + diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevHistoryParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevHistoryParser.java index 97087ed051f..bf1cc13391f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevHistoryParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevHistoryParser.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -49,9 +50,8 @@ public class AccuRevHistoryParser implements Executor.StreamHandler { * @param file the file to parse history for * @param repos Pointer to the {@code AccuRevRepository} * @return object representing the file's history - * @throws HistoryException if a problem occurs while executing p4 command */ - History parse(File file, Repository repos) throws HistoryException { + History parse(File file, Repository repos) { repository = (AccuRevRepository) repos; @@ -68,7 +68,7 @@ History parse(File file, Repository repos) throws HistoryException { if (relPath.equals(rootRelativePath)) { - List entries = new ArrayList(); + List entries = new ArrayList<>(); entries.add(new HistoryEntry( "", new Date(), "OpenGrok", null, "Workspace Root", true)); @@ -76,18 +76,11 @@ History parse(File file, Repository repos) throws HistoryException { history = new History(entries); } else { - try { - /* - * Errors will be logged, so not bothering to add to the output. - */ - Executor executor = repository.getHistoryLogExecutor(file); - executor.exec(true, this); - - } catch (IOException e) { - throw new HistoryException( - "Failed to get history for: \"" - + file.getAbsolutePath() + "\"" + e); - } + /* + * Errors will be logged, so not bothering to add to the output. + */ + Executor executor = repository.getHistoryLogExecutor(file); + executor.exec(true, this); } return history; @@ -96,7 +89,7 @@ History parse(File file, Repository repos) throws HistoryException { public void processStream(InputStream input) throws IOException { BufferedReader in = new BufferedReader(new InputStreamReader(input)); - List entries = new ArrayList(); + List entries = new ArrayList<>(); String line; String user; Date date; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java index a89853a01db..4bcd66567f2 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AccuRevRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2018, Chris Fraire . + * Portions Copyright (c) 2018-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -108,7 +108,7 @@ public AccuRevRepository() { } @Override - public Annotation annotate(File file, String rev) throws IOException { + public Annotation annotate(File file, String rev) { ArrayList cmd = new ArrayList<>(); @@ -141,7 +141,7 @@ public Annotation annotate(File file, String rev) throws IOException { * @param file file for which history is to be retrieved. * @return An Executor ready to be started */ - Executor getHistoryLogExecutor(File file) throws IOException { + Executor getHistoryLogExecutor(File file) { // Do not use absolute paths because symbolic links will cause havoc. String path = getDepotRelativePath( file ); @@ -455,12 +455,12 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { - return new AccuRevHistoryParser().parse(file, this); + HistoryEnumeration getHistory(File file) throws HistoryException { + return new SingleHistory(new AccuRevHistoryParser().parse(file, this)); } @Override - String determineParent(CommandTimeoutType cmdType) throws IOException { + String determineParent(CommandTimeoutType cmdType) { getAccuRevInfo(new File(getDirectoryName()), cmdType); return parentInfo; } @@ -471,7 +471,7 @@ String determineBranch(CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarHistoryParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarHistoryParser.java index a76149ba189..a6cae07b8d3 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarHistoryParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarHistoryParser.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017, 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -50,8 +50,8 @@ class BazaarHistoryParser implements Executor.StreamHandler { private static final Logger LOGGER = LoggerFactory.getLogger(BazaarHistoryParser.class); private String myDir; - private List entries = new ArrayList<>(); //NOPMD - private BazaarRepository repository = new BazaarRepository(); //NOPMD + private final List entries = new ArrayList<>(); //NOPMD + private final BazaarRepository repository; BazaarHistoryParser(BazaarRepository repository) { this.repository = repository; @@ -93,12 +93,11 @@ History parse(File file, String sinceRevision) throws HistoryException { @Override public void processStream(InputStream input) throws IOException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); - BufferedReader in = new BufferedReader(new InputStreamReader(input)); - String s; - HistoryEntry entry = null; int state = 0; + + String s; while ((s = in.readLine()) != null) { if ("------------------------------------------------------------".equals(s)) { if (entry != null && state > 2) { @@ -169,8 +168,8 @@ public void processStream(InputStream input) throws IOException { File f = new File(myDir, s); try { - String name = env.getPathRelativeToSourceRoot(f); - entry.addFile(name.intern()); + String path = env.getPathRelativeToSourceRoot(f); + entry.addFile(path); } catch (ForbiddenSymlinkException e) { LOGGER.log(Level.FINER, e.getMessage()); // ignored diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java index c444984af9b..2cd0072cc27 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017-2018, Chris Fraire . + * Portions Copyright (c) 2017-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -77,7 +77,7 @@ Executor getHistoryLogExecutor(final File file, final String sinceRevision) throws IOException { String filename = getRepoRelativePath(file); - List cmd = new ArrayList(); + List cmd = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); cmd.add(RepoCommand); cmd.add("log"); @@ -202,7 +202,7 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file, String sinceRevision) throws HistoryException { + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); History result = new BazaarHistoryParser(this).parse(file, sinceRevision); // Assign tags to changesets they represent @@ -210,11 +210,11 @@ History getHistory(File file, String sinceRevision) throws HistoryException { if (env.isTagsEnabled()) { assignTagsInHistory(result); } - return result; + return new SingleHistory(result); } @Override - History getHistory(File file) throws HistoryException { + HistoryEnumeration getHistory(File file) throws HistoryException { return getHistory(file, null); } @@ -271,7 +271,7 @@ String determineBranch(CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarTagEntry.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarTagEntry.java index 899a1b114dd..b3604a75a3a 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarTagEntry.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BazaarTagEntry.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -37,6 +38,6 @@ public BazaarTagEntry(int revision, String tag) { @Override public int compareTo(HistoryEntry that) { assert this.revision != NOREV : "BazaarTagEntry created without tag specified.specified"; - return ((Integer) this.revision).compareTo(Integer.parseInt(that.getRevision())); + return Integer.compare(this.revision, Integer.parseInt(that.getRevision())); } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java index 6fd3b96dbc5..537078e543c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/BitKeeperRepository.java @@ -20,7 +20,7 @@ /* * Author: James Service * Portions by: Oracle and/or its affiliates. - * Portions Copyright (c) 2018, Chris Fraire . + * Portions Copyright (c) 2018-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -168,7 +168,7 @@ public Version getVersion() { * @return null */ @Override - String determineBranch(CommandTimeoutType cmdType) throws IOException { + String determineBranch(CommandTimeoutType cmdType) { return null; } @@ -181,7 +181,7 @@ String determineBranch(CommandTimeoutType cmdType) throws IOException { String determineParent(CommandTimeoutType cmdType) throws IOException { final File directory = new File(getDirectoryName()); - final ArrayList argv = new ArrayList(); + final ArrayList argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); argv.add("parent"); @@ -232,7 +232,7 @@ public boolean fileHasHistory(File file) { final File directory = absolute.getParentFile(); final String basename = absolute.getName(); - final ArrayList argv = new ArrayList(); + final ArrayList argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); argv.add("files"); @@ -251,10 +251,10 @@ public boolean fileHasHistory(File file) { * Construct a History for a file in this repository. * * @param file a file in the repository - * @return history a history object + * @return history a history sequence */ @Override - History getHistory(File file) throws HistoryException { + HistoryEnumeration getHistory(File file) throws HistoryException { return getHistory(file, null); } @@ -263,15 +263,15 @@ History getHistory(File file) throws HistoryException { * * @param file a file in the repository * @param sinceRevision omit history from before, and including, this revision - * @return history a history object + * @return history a history sequence */ @Override - History getHistory(File file, String sinceRevision) throws HistoryException { + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { final File absolute = file.getAbsoluteFile(); final File directory = absolute.getParentFile(); final String basename = absolute.getName(); - final ArrayList argv = new ArrayList(); + final ArrayList argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); argv.add("log"); @@ -297,7 +297,7 @@ History getHistory(File file, String sinceRevision) throws HistoryException { assignTagsInHistory(history); } - return history; + return new SingleHistory(history); } @Override @@ -305,7 +305,7 @@ boolean getHistoryGet( BufferSink sink, String parent, String basename, String revision) { final File directory = new File(parent).getAbsoluteFile(); - final ArrayList argv = new ArrayList(); + final ArrayList argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); argv.add("get"); @@ -436,7 +436,7 @@ public void buildTagList(File directory, CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/CVSRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/CVSRepository.java index 52b2ec3dcd0..cbda22cb0a1 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/CVSRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/CVSRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, 2020, Chris Fraire . + * Portions Copyright (c) 2017, 2019-2020, Chris Fraire . */ package org.opengrok.indexer.history; @@ -144,7 +144,7 @@ public boolean isRepositoryFor(File file, CommandTimeoutType cmdType) { } @Override - String determineBranch(CommandTimeoutType cmdType) throws IOException { + String determineBranch(CommandTimeoutType cmdType) { String branch = null; File tagFile = new File(getDirectoryName(), "CVS/Tag"); @@ -250,12 +250,12 @@ public boolean fileHasHistory(File file) { } @Override - History getHistory(File file) throws HistoryException { - return new CVSHistoryParser().parse(file, this); + HistoryEnumeration getHistory(File file) throws HistoryException { + return new SingleHistory(new CVSHistoryParser().parse(file, this)); } @Override - Annotation annotate(File file, String revision) throws IOException { + Annotation annotate(File file, String revision) { ArrayList cmd = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); cmd.add(RepoCommand); @@ -284,7 +284,7 @@ Annotation annotate(File file, String revision) throws IOException { } @Override - String determineParent(CommandTimeoutType cmdType) throws IOException { + String determineParent(CommandTimeoutType cmdType) { File rootFile = new File(getDirectoryName() + File.separator + "CVS" + File.separator + "Root"); String parent = null; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java index 550d64fed46..97eed6aee6c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/ClearCaseRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017-2018, 2020, Chris Fraire . + * Portions Copyright (c) 2017-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -143,10 +143,9 @@ boolean getHistoryGet( * @param file file to annotate * @param revision revision to annotate * @return file annotation - * @throws java.io.IOException if I/O exception occurred */ @Override - public Annotation annotate(File file, String revision) throws IOException { + public Annotation annotate(File file, String revision) { ArrayList argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); @@ -223,7 +222,7 @@ boolean isRepositoryFor(File file, CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } @@ -269,12 +268,12 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { - return new ClearCaseHistoryParser().parse(file, this); + HistoryEnumeration getHistory(File file) throws HistoryException { + return new SingleHistory(new ClearCaseHistoryParser().parse(file, this)); } @Override - String determineParent(CommandTimeoutType cmdType) throws IOException { + String determineParent(CommandTimeoutType cmdType) { return null; } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java index 831e90bf030..599690bfccb 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java @@ -24,37 +24,33 @@ package org.opengrok.indexer.history; -import java.beans.Encoder; -import java.beans.Expression; -import java.beans.PersistenceDelegate; -import java.beans.XMLDecoder; -import java.beans.XMLEncoder; -import java.io.BufferedInputStream; -import java.io.BufferedOutputStream; import java.io.BufferedReader; import java.io.BufferedWriter; import java.io.File; -import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.FileReader; import java.io.IOException; import java.io.OutputStreamWriter; import java.io.Writer; +import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Collections; import java.util.Date; +import java.util.Enumeration; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.ListIterator; import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.zip.GZIPInputStream; -import java.util.zip.GZIPOutputStream; import org.opengrok.indexer.configuration.PathAccepter; import org.opengrok.indexer.configuration.RuntimeEnvironment; @@ -78,7 +74,7 @@ class FileHistoryCache implements HistoryCache { private static final String DIRECTORY_FILE_PREFIX = "OpenGrokDirHist"; private final PathAccepter pathAccepter = env.getPathAccepter(); - private boolean historyIndexDone = false; + private boolean historyIndexDone; @Override public void setHistoryIndexDone() { @@ -95,63 +91,40 @@ public boolean isHistoryIndexDone() { * @param filename name of the file * @param historyEntries list of HistoryEntry objects forming the (incremental) history of the file * @param repository repository object in which the file belongs - * @param srcFile file object - * @param root root of the source repository - * @param renamed true if the file was renamed in the past + * @param forceOverwrite a value indicating whether to overwrite the cache + * file if it exists or if {@code false} then to try to merge + * {@code historyEntries} with any existing cache file */ private void doFileHistory(String filename, List historyEntries, - Repository repository, File srcFile, File root, boolean renamed) - throws HistoryException { - - History hist = null; - - /* - * If the file was renamed (in the changesets that are being indexed), - * its history is not stored in the historyEntries so it needs to be acquired - * directly from the repository. - * This ensures that complete history of the file (across renames) - * will be saved. - */ - if (renamed) { - hist = repository.getHistory(srcFile); - } - - File file = new File(root, filename); - - if (hist == null) { - hist = new History(); + Repository repository, boolean forceOverwrite) throws HistoryException { - // File based history cache does not store files for individual - // changesets so strip them unless it is history for the repository. - for (HistoryEntry ent : historyEntries) { - if (file.isDirectory() && filename.equals(repository.getDirectoryName())) { - ent.stripTags(); - } else { - ent.strip(); - } - } + File file = new File(filename); + boolean isDirectory = file.isDirectory(); - // add all history entries - hist.setHistoryEntries(historyEntries); - } else { - for (HistoryEntry ent : hist.getHistoryEntries()) { + // File based history cache does not store files for individual + // changesets so strip them unless it is history for the repository. + for (HistoryEntry ent : historyEntries) { + if (isDirectory && filename.equals(repository.getDirectoryName())) { + ent.stripTags(); + } else { ent.strip(); } } - // Assign tags to changesets they represent. - if (env.isTagsEnabled() && repository.hasFileBasedTags()) { - repository.assignTagsInHistory(hist); - } + History hist = new History(); + // add all history entries + hist.setHistoryEntries(historyEntries); - // Only store directory history for the top-level. - if (!file.isDirectory() || filename.equals(repository.getDirectoryName())) { - storeFile(hist, file, repository, !renamed); + // Store history for file -- or for the top-level directory. + if (file.isFile() || (isDirectory && filename.equals(repository.getDirectoryName()))) { + storeFile(hist, file, repository, forceOverwrite); + } else { + LOGGER.log(Level.FINE, "Skipping ineligible {0}", file); } } - private boolean isRenamedFile(String filename, Repository repository, History history) - throws IOException { + private boolean isRenamedFile(String filename, Repository repository, + StoreAssociations associations) throws IOException { String repodir; try { @@ -163,26 +136,27 @@ private boolean isRenamedFile(String filename, Repository repository, History hi } String shortestfile = filename.substring(repodir.length() + 1); - return (history.isRenamed(shortestfile)); - } - - static class FilePersistenceDelegate extends PersistenceDelegate { - @Override - protected Expression instantiate(Object oldInstance, Encoder out) { - File f = (File) oldInstance; - return new Expression(oldInstance, f.getClass(), "new", - new Object[] {f.toString()}); - } + return associations.renamedFiles.contains(shortestfile); } + /** + * Does nothing. + */ @Override public void initialize() { - // nothing to do } + /** + * Does nothing. + */ @Override public void optimize() { - // nothing to do + } + + /** + * Does nothing. + */ + public void close() { } @Override @@ -192,12 +166,12 @@ public boolean supportsRepository(Repository repository) { } /** - * Get a File object describing the cache file. + * Gets a {@link File} object describing the cache file. * * @param file the file to find the cache for - * @return file that might contain cached history for file + * @return file that might contain cached history for {@code file} */ - private static File getCachedFile(File file) throws HistoryException, + private File getCachedFile(File file) throws HistoryException, ForbiddenSymlinkException { StringBuilder sb = new StringBuilder(); @@ -223,17 +197,6 @@ private static File getCachedFile(File file) throws HistoryException, return new File(TandemPath.join(sb.toString(), ".gz")); } - /** - * Read history from a file. - */ - private static History readCache(File file) throws IOException { - try (FileInputStream in = new FileInputStream(file); - XMLDecoder d = new XMLDecoder(new GZIPInputStream( - new BufferedInputStream(in)))) { - return (History) d.readObject(); - } - } - /** * Store history in file on disk. * @param dir directory where the file will be saved @@ -245,39 +208,24 @@ private void writeHistoryToFile(File dir, History history, File cacheFile) throw // at the same time. Since I would like to avoid read-locking, I just // serialize the write access to the cache file. The generation of the // cache file would most likely be executed during index generation, and - // that happens sequencial anyway.... + // that happens sequentially anyway.... // Generate the file with a temporary name and move it into place when // I'm done so I don't have to protect the readers for partially updated // files... - final File output; + File output = null; try { output = File.createTempFile("oghist", null, dir); - try (FileOutputStream out = new FileOutputStream(output); - XMLEncoder e = new XMLEncoder(new GZIPOutputStream( - new BufferedOutputStream(out)))) { - e.setPersistenceDelegate(File.class, - new FilePersistenceDelegate()); - e.writeObject(history); + history.writeGZIP(output); + synchronized (lock) { + Files.move(output.toPath(), cacheFile.toPath(), + StandardCopyOption.REPLACE_EXISTING); } } catch (IOException ioe) { - throw new HistoryException("Failed to write history", ioe); - } - synchronized (lock) { - if (!cacheFile.delete() && cacheFile.exists()) { - if (!output.delete()) { - LOGGER.log(Level.WARNING, - "Failed to remove temporary history cache file"); - } - throw new HistoryException( - "Cachefile exists, and I could not delete it."); - } - if (!output.renameTo(cacheFile)) { - if (!output.delete()) { - LOGGER.log(Level.WARNING, - "Failed to remove temporary history cache file"); - } - throw new HistoryException("Failed to rename cache tmpfile."); + if (output != null) { + //noinspection ResultOfMethodCallIgnored + output.delete(); } + throw new HistoryException("Failed to write history", ioe); } } @@ -295,7 +243,7 @@ private History mergeOldAndNewHistory(File cacheFile, History histNew, Repositor History history = null; try { - histOld = readCache(cacheFile); + histOld = History.readGZIP(cacheFile); // Merge old history with the new history. List listOld = histOld.getHistoryEntries(); if (!listOld.isEmpty()) { @@ -306,9 +254,9 @@ private History mergeOldAndNewHistory(File cacheFile, History histNew, Repositor } history = new History(listOld); - // Retag the last changesets in case there have been some new + // Un-tag the last changesets in case there have been some new // tags added to the repository. Technically we should just - // retag the last revision from the listOld however this + // re-tag the last revision from the listOld however this // does not solve the problem when listNew contains new tags // retroactively tagging changesets from listOld so we resort // to this somewhat crude solution. @@ -316,7 +264,6 @@ private History mergeOldAndNewHistory(File cacheFile, History histNew, Repositor for (HistoryEntry ent : history.getHistoryEntries()) { ent.setTags(null); } - repo.assignTagsInHistory(history); } } } catch (IOException ex) { @@ -332,12 +279,12 @@ private History mergeOldAndNewHistory(File cacheFile, History histNew, Repositor * * @param histNew history object to store * @param file file to store the history object into - * @param repo repository for the file - * @param mergeHistory whether to merge the history with existing or - * store the histNew as is + * @param forceOverwrite a value indicating whether to overwrite the + * cache file if it exists or if {@code false} then to try to append + * {@code histNext} with any existing cache file */ private void storeFile(History histNew, File file, Repository repo, - boolean mergeHistory) throws HistoryException { + boolean forceOverwrite) throws HistoryException { File cacheFile; try { @@ -346,30 +293,42 @@ private void storeFile(History histNew, File file, Repository repo, LOGGER.log(Level.FINER, e.getMessage()); return; } - History history = histNew; + + boolean cachedExists = cacheFile.exists(); File dir = cacheFile.getParentFile(); - if (!dir.isDirectory() && !dir.mkdirs()) { + /* + * The double check-exists in the following conditional is necessary + * because during a race when two threads are simultaneously linking + * for a not-yet-existent `dir`, the first check-exists will be false + * for both threads, but then only one will see true from mkdirs -- so + * the other needs a fallback again to check-exists. + */ + if (!cachedExists && !dir.isDirectory() && !dir.mkdirs() && + !dir.isDirectory()) { throw new HistoryException( "Unable to create cache directory '" + dir + "'."); } - if (mergeHistory && cacheFile.exists()) { + History history = null; + if (!forceOverwrite && cachedExists) { history = mergeOldAndNewHistory(cacheFile, histNew, repo); } - // If the merge failed, null history will be returned. - // In such case store at least new history as a best effort. + // If the merge failed, null history will be returned. In such case, or + // if merge was not done, store at least new history as a best effort. if (history == null) { history = histNew; } - writeHistoryToFile(dir, history, cacheFile); - } + // Assign tags to changesets they represent. + if (env.isTagsEnabled() && repo.hasFileBasedTags()) { + repo.assignTagsInHistory(history); + } - private void storeFile(History histNew, File file, Repository repo) - throws HistoryException { - storeFile(histNew, file, repo, false); + repo.deduplicateRevisions(history); + + writeHistoryToFile(dir, history, cacheFile); } private void finishStore(Repository repository, String latestRev) { @@ -390,36 +349,102 @@ private void finishStore(Repository repository, String latestRev) { } /** - * Store history for the whole repository in directory hierarchy resembling - * the original repository structure. History of individual files will be - * stored under this hierarchy, each file containing history of - * corresponding source file. + * Calls {@link #store(Enumeration, Repository, boolean)} with + * {@code historySequence}, {@code repository}, and {@code false}. * - * @param history history object to process into per-file histories - * @param repository repository object + * @param historySequence a defined history series to store + * @param repository a defined repository whose history to store + * @throws HistoryException if the history cannot be stored */ - @Override - public void store(History history, Repository repository) + public void store(Enumeration historySequence, Repository repository) throws HistoryException { - final boolean handleRenamedFiles = repository.isHandleRenamedFiles(); + store(historySequence, repository, false); + } - String latestRev = null; + /** + * Stores the history enumeration for a repository, where + * {@code historyElements} must be ordered from most recent to earlier + * between each element and within each element, in directory hierarchy + * resembling the original repository structure. History of individual + * files will be stored under this hierarchy, each file containing history + * of corresponding source file. + * + * @param historySequence a defined history series to store + * @param repository a defined repository whose history to store + * @param forceOverwrite a value indicating whether to overwrite existing + * stored history for the files in {@code historySequence} + * @throws HistoryException if the history cannot be stored + */ + public void store(Enumeration historySequence, Repository repository, + boolean forceOverwrite) throws HistoryException { // Return immediately when there is nothing to do. - List entries = history.getHistoryEntries(); - if (entries.isEmpty()) { + if (!historySequence.hasMoreElements()) { return; } - LOGGER.log(Level.FINE, - "Storing history for repository {0}", - new Object[] {repository.getDirectoryName()}); + StoreAssociations associations = new StoreAssociations(); + try { + associations.histTemp.open(); + } catch (IOException e) { + throw new HistoryException("Failed FileHistoryTemp open()", e); + } - // Firstly store the history for the top-level directory. - doFileHistory(repository.getDirectoryName(), history.getHistoryEntries(), - repository, env.getSourceRootFile(), null, false); + try { + storeInner(historySequence, repository, associations, forceOverwrite); + } finally { + try { + associations.histTemp.close(); + } catch (IOException e) { + LOGGER.log(Level.SEVERE, "Failed FileHistoryTemp close()", e); + } + } + } + + /** + * Called by {@link #store(Enumeration, Repository, boolean)} in try block. + */ + private void storeInner(Enumeration historySequence, Repository repository, + StoreAssociations associations, boolean forceOverwrite) { + + String latestRev = null; + boolean didLogIntro = false; - HashMap> map = new HashMap<>(); + while (historySequence.hasMoreElements()) { + if (!didLogIntro) { + LOGGER.log(Level.FINE, "Storing history for repository {0}", + repository.getDirectoryName()); + didLogIntro = true; + } + + History hist = historySequence.nextElement(); + if (latestRev == null && hist.count() > 0) { + latestRev = hist.getHistoryEntry(0).getRevision(); + } + + if (associations.renamedFiles == null) { + associations.renamedFiles = new HashSet<>(hist.getRenamedFiles()); + } + + storeTemp(hist, repository, associations); + } + + if (env.isHandleHistoryOfRenamedFiles()) { + storeRenamesTemp(repository, associations); + } + + if (latestRev != null) { + storePending(repository, associations, forceOverwrite); + finishStore(repository, latestRev); + } + } + + private void storeTemp(History history, Repository repository, StoreAssociations associations) { + + final File root = env.getSourceRootFile(); + final boolean handleRenamedFiles = repository.isHandleRenamedFiles(); + + Map> map = new HashMap<>(); HashMap acceptanceCache = new HashMap<>(); /* @@ -431,12 +456,9 @@ public void store(History history, Repository repository) */ for (HistoryEntry e : history.getHistoryEntries()) { // The history entries are sorted from newest to oldest. - if (latestRev == null) { - latestRev = e.getRevision(); - } for (String s : e.getFiles()) { /* - * We do not want to generate history cache for files which + * We do not want to generate historycache for files that * do not currently exist in the repository. * * Also we cache the result of this evaluation to boost @@ -458,16 +480,12 @@ public void store(History history, Repository repository) } } - List list = map.get(s); - if (list == null) { - list = new ArrayList<>(); - map.put(s, list); - } - /* - * We need to do deep copy in order to have different tags - * per each commit. - */ + List list = map.computeIfAbsent(s, k -> new ArrayList<>()); if (env.isTagsEnabled() && repository.hasFileBasedTags()) { + /* + * We need to do deep copy in order to have different tags + * per each commit. + */ list.add(new HistoryEntry(e)); } else { list.add(e); @@ -475,119 +493,183 @@ public void store(History history, Repository repository) } } + LOGGER.log(Level.FINEST, "Processing temp history for {0} files", map.size()); + /* - * Now traverse the list of files from the hash map built above - * and for each file store its history (saved in the value of the - * hash map entry for the file) in a file. Skip renamed files - * which will be handled separately below. + * Now traverse the list of files from the map built above and for each + * file append its history (saved in the value of the map entry for the + * file) to the HashDB, skipping renamed files which will be handled + * separately. */ - final File root = env.getSourceRootFile(); - int fileHistoryCount = 0; + final CountDownLatch latch = new CountDownLatch(map.size()); + final AtomicInteger fileTempCount = new AtomicInteger(); for (Map.Entry> map_entry : map.entrySet()) { + final String filename = map_entry.getKey(); + final List fileHistory = map_entry.getValue(); + try { - if (handleRenamedFiles && - isRenamedFile(map_entry.getKey(), repository, history)) { + if (handleRenamedFiles && isRenamedFile(filename, repository, associations)) { + List mappedFileHistory = associations.historyRenamedFiles. + computeIfAbsent(filename, k -> new ArrayList<>()); + mappedFileHistory.addAll(fileHistory); + latch.countDown(); continue; } } catch (IOException ex) { - LOGGER.log(Level.WARNING, "isRenamedFile() got exception", ex); + LOGGER.log(Level.WARNING, "Error with isRenamedFile()", ex); } - doFileHistory(map_entry.getKey(), map_entry.getValue(), - repository, null, root, false); - fileHistoryCount++; + final File keyFile = new File(root, filename); + // Using the historyRenamedExecutor here too. + env.getIndexerParallelizer().getHistoryRenamedExecutor().submit(() -> { + try { + associations.histTemp.append(keyFile.toString(), fileHistory); + fileTempCount.incrementAndGet(); + } catch (Throwable ex) { + // Catch any exception so that one file does not derail. + LOGGER.log(Level.SEVERE, "Error appending FileHistoryTemp", ex); + } finally { + latch.countDown(); + } + }); } - LOGGER.log(Level.FINE, "Stored history for {0} files", fileHistoryCount); - - if (!handleRenamedFiles) { - finishStore(repository, latestRev); - return; + try { + // Wait for the executors to finish. + latch.await(); + } catch (InterruptedException ex) { + LOGGER.log(Level.SEVERE, "Failed to await latch", ex); } + LOGGER.log(Level.FINEST, "Stored temp history for {0} files", + fileTempCount.intValue()); + } - /* - * Now handle renamed files (in parallel). - */ - HashMap> renamed_map = - new HashMap<>(); - for (final Map.Entry> map_entry : map.entrySet()) { - try { - if (isRenamedFile(map_entry.getKey(), repository, history)) { - renamed_map.put(map_entry.getKey(), map_entry.getValue()); + /** + * Handles renames in parallel. If a file was renamed (in the changesets + * that are being indexed), its history is not stored in the historyEntries + * so it needs to be acquired directly from the repository. This ensures + * that complete history of the file (across renames) will be saved. + */ + private void storeRenamesTemp(Repository repository, StoreAssociations associations) { + + final File root = env.getSourceRootFile(); + + final int renamedSize = associations.historyRenamedFiles.size(); + LOGGER.log(Level.FINEST, "Processing temp history for {0} renamed files", + renamedSize); + + final CountDownLatch latch = new CountDownLatch(renamedSize); + final AtomicInteger renamedFileHistoryCount = new AtomicInteger(); + for (final Map.Entry> map_entry : + associations.historyRenamedFiles.entrySet()) { + env.getIndexerParallelizer().getHistoryRenamedExecutor().submit(() -> { + try { + final String renamedFile = map_entry.getKey(); + + History hist = HistoryUtil.union(repository.getHistory( + new File(env.getSourceRootPath() + renamedFile))); + + final List fileHistory; + boolean forceOverwrite = false; + if (hist.count() < 1) { + fileHistory = map_entry.getValue(); + } else { + fileHistory = hist.getHistoryEntries(); + forceOverwrite = true; + } + + for (HistoryEntry ent : hist.getHistoryEntries()) { + ent.strip(); + } + + final File keyFile = new File(root, renamedFile); + + // FileHistoryTemp by design is not thread-safe. + if (forceOverwrite) { + associations.histTemp.set(keyFile.toString(), fileHistory); + } else { + associations.histTemp.append(keyFile.toString(), fileHistory); + } + renamedFileHistoryCount.getAndIncrement(); + } catch (Throwable ex) { + // Catch any exception so that one file does not derail. + LOGGER.log(Level.WARNING, "Error writing history", ex); + } finally { + latch.countDown(); } - } catch (IOException ex) { - LOGGER.log(Level.WARNING, - "isRenamedFile() got exception ", ex); - } + }); } - // The directories for the renamed files have to be created before - // the actual files otherwise storeFile() might be racing for - // mkdirs() if there are multiple renamed files from single directory - // handled in parallel. - for (final String file : renamed_map.keySet()) { - File cache; - try { - cache = getCachedFile(new File(env.getSourceRootPath() + file)); - } catch (ForbiddenSymlinkException ex) { - LOGGER.log(Level.FINER, ex.getMessage()); - continue; - } - File dir = cache.getParentFile(); - if (!dir.isDirectory() && !dir.mkdirs()) { - LOGGER.log(Level.WARNING, - "Unable to create cache directory ' {0} '.", dir); - } + try { + // Wait for the executors to finish. + latch.await(); + } catch (InterruptedException ex) { + LOGGER.log(Level.SEVERE, "Failed to await latch", ex); } - final Repository repositoryF = repository; - final CountDownLatch latch = new CountDownLatch(renamed_map.size()); - AtomicInteger renamedFileHistoryCount = new AtomicInteger(); - for (final Map.Entry> map_entry : renamed_map.entrySet()) { + LOGGER.log(Level.FINE, "Stored temp history for {0} renamed files", + renamedFileHistoryCount.intValue()); + } + + private void storePending(Repository repository, StoreAssociations associations, + boolean forceOverwrite) { + + final int fileCount = associations.histTemp.fileCount(); + LOGGER.log(Level.FINEST, "Processing pending history for {0} files", fileCount); + + /* + * Now traverse the list of files from the HashDB built above and for + * each file store its history. Skip renamed files which will be + * handled separately. + */ + final CountDownLatch latch = new CountDownLatch(fileCount); + final AtomicInteger fileHistoryCount = new AtomicInteger(); + Enumeration keyedEnumeration = associations.histTemp.getEnumerator(); + for (int i = 0; i < fileCount; ++i) { + // Using the historyRenamedExecutor here too. env.getIndexerParallelizer().getHistoryRenamedExecutor().submit(() -> { - try { - doFileHistory(map_entry.getKey(), map_entry.getValue(), - repositoryF, - new File(env.getSourceRootPath() + map_entry.getKey()), - root, true); - renamedFileHistoryCount.getAndIncrement(); - } catch (Exception ex) { - // We want to catch any exception since we are in thread. - LOGGER.log(Level.WARNING, - "doFileHistory() got exception ", ex); - } finally { - latch.countDown(); + try { + KeyedHistory keyedHistory; + synchronized (lock) { + keyedHistory = keyedEnumeration.nextElement(); } + + doFileHistory(keyedHistory.getFile(), keyedHistory.getEntries(), repository, + forceOverwrite || keyedHistory.isForceOverwrite()); + fileHistoryCount.incrementAndGet(); + } catch (Throwable ex) { + // Catch any exception so that one file does not derail. + LOGGER.log(Level.SEVERE, "Error writing history", ex); + } finally { + latch.countDown(); + } }); } - // Wait for the executors to finish. try { // Wait for the executors to finish. latch.await(); } catch (InterruptedException ex) { - LOGGER.log(Level.SEVERE, "latch exception", ex); + LOGGER.log(Level.SEVERE, "Failed to await latch", ex); } - LOGGER.log(Level.FINE, "Stored history for {0} renamed files", - renamedFileHistoryCount.intValue()); - finishStore(repository, latestRev); + LOGGER.log(Level.FINEST, "Stored pending history for {0} files", + fileHistoryCount.intValue()); } @Override public History get(File file, Repository repository, boolean withFiles) throws HistoryException, ForbiddenSymlinkException { - File cache = getCachedFile(file); - if (isUpToDate(file, cache)) { + File cacheFile = getCachedFile(file); + if (isUpToDate(file, cacheFile)) { try { - return readCache(cache); + return History.readGZIP(cacheFile); } catch (Exception e) { - LOGGER.log(Level.WARNING, - "Error when reading cache file '" + cache, e); + LOGGER.log(Level.WARNING, "Error reading " + cacheFile, e); } } /* * Some mirrors of repositories which are capable of fetching history - * for directories may contain lots of files untracked by given SCM. + * for directories may contain lots of files un-tracked by given SCM. * For these it would be waste of time to get their history * since the history of all files in this repository should have been * fetched in the first phase of indexing. @@ -606,7 +688,7 @@ public History get(File file, Repository repository, boolean withFiles) long time; try { time = System.currentTimeMillis(); - history = repository.getHistory(file); + history = HistoryUtil.union(repository.getHistory(file)); time = System.currentTimeMillis() - time; } catch (UnsupportedOperationException e) { // In this case, we've found a file for which the SCM has no history @@ -621,11 +703,9 @@ public History get(File file, Repository repository, boolean withFiles) // a sub-directory change. This will cause us to present a stale // history log until a the current directory is updated and // invalidates the cache entry. - if ((cache != null) && - (cache.exists() || - (time > env.getHistoryReaderTimeLimit()))) { + if (cacheFile.exists() || time > env.getHistoryReaderTimeLimit()) { // retrieving the history takes too long, cache it! - storeFile(history, file, repository); + storeFile(history, file, repository, true); } } @@ -653,21 +733,20 @@ private boolean isUpToDate(File file, File cachedFile) { public boolean hasCacheForDirectory(File directory, Repository repository) throws HistoryException { assert directory.isDirectory(); - Repository repos = HistoryGuru.getInstance().getRepository(directory); - if (repos == null) { + Repository repo = HistoryGuru.getInstance().getRepository(directory); + if (repo == null) { return true; } File dir = env.getDataRootFile(); dir = new File(dir, FileHistoryCache.HISTORY_CACHE_DIR_NAME); try { - dir = new File(dir, env.getPathRelativeToSourceRoot( - new File(repos.getDirectoryName()))); + dir = new File(dir, env.getPathRelativeToSourceRoot(new File(repo.getDirectoryName()))); } catch (ForbiddenSymlinkException e) { LOGGER.log(Level.FINER, e.getMessage()); return false; } catch (IOException e) { - throw new HistoryException("Could not resolve " + - repos.getDirectoryName() + " relative to source root", e); + throw new HistoryException("Could not resolve " + repo.getDirectoryName() + + " relative to source root", e); } return dir.exists(); } @@ -682,7 +761,7 @@ public boolean hasCacheForFile(File file) throws HistoryException { } } - public String getRepositoryHistDataDirname(Repository repository) { + String getRepositoryHistDataDirname(Repository repository) { String repoDirBasename; try { @@ -716,23 +795,19 @@ private String getRepositoryCachedRevPath(Repository repository) { * @param rev latest revision which has been just indexed */ private void storeLatestCachedRevision(Repository repository, String rev) { - Writer writer = null; - try { - writer = new BufferedWriter(new OutputStreamWriter( - new FileOutputStream(getRepositoryCachedRevPath(repository)))); + String repositoryCachedRevPath = getRepositoryCachedRevPath(repository); + if (repositoryCachedRevPath == null) { + // getRepositoryHistDataDirname() already logged the WARNING. + return; + } + + try (FileOutputStream oss = new FileOutputStream(repositoryCachedRevPath); + Writer writer = new BufferedWriter(new OutputStreamWriter(oss))) { writer.write(rev); } catch (IOException ex) { LOGGER.log(Level.WARNING, "Cannot write latest cached revision to file for " + repository.getDirectoryName(), ex); - } finally { - try { - if (writer != null) { - writer.close(); - } - } catch (IOException ex) { - LOGGER.log(Level.WARNING, "Cannot close file", ex); - } } } @@ -787,7 +862,9 @@ public void clear(Repository repository) { // remove the file cached last revision (done separately in case // it gets ever moved outside of the hierarchy) File cachedRevFile = new File(revPath); - cachedRevFile.delete(); + if (!cachedRevFile.delete() && cachedRevFile.exists()) { + LOGGER.log(Level.WARNING, "Error deleting {0}", cachedRevFile); + } } String histDir = getRepositoryHistDataDirname(repository); @@ -818,14 +895,12 @@ public void clearFile(String path) { File parent = historyFile.getParentFile(); if (!historyFile.delete() && historyFile.exists()) { - LOGGER.log(Level.WARNING, - "Failed to remove obsolete history cache-file: {0}", - historyFile.getAbsolutePath()); + LOGGER.log(Level.WARNING, "Error removing obsolete {0}", + historyFile.getAbsolutePath()); } if (parent.delete()) { - LOGGER.log(Level.FINE, "Removed empty history cache dir:{0}", - parent.getAbsolutePath()); + LOGGER.log(Level.FINE, "Removed empty {0}", parent.getAbsolutePath()); } } @@ -833,4 +908,13 @@ public void clearFile(String path) { public String getInfo() { return getClass().getSimpleName(); } + + private static class StoreAssociations { + final FileHistoryTemp histTemp = new FileHistoryTemp(); + + final Map> historyRenamedFiles = + new ConcurrentHashMap<>(); + + Set renamedFiles; + } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryTemp.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryTemp.java new file mode 100644 index 00000000000..6c4f8e0a626 --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryTemp.java @@ -0,0 +1,351 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.history; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.primitives.Longs; +import com.oath.halodb.HaloDB; +import com.oath.halodb.HaloDBException; +import com.oath.halodb.HaloDBOptions; +import org.opengrok.indexer.logger.LoggerFactory; +import org.opengrok.indexer.util.IOUtils; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.Closeable; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; + +/** + * Represents a logger of batches of file {@link HistoryEntry} arrays for + * transient storage to be pieced together later file by file. + */ +class FileHistoryTemp implements Closeable { + + private static final Logger LOGGER = LoggerFactory.getLogger(FileHistoryTemp.class); + + private static final ObjectMapper MAPPER; + private static final TypeReference TYPE_H_E_F_ARRAY; + + private final AtomicLong counter = new AtomicLong(0); + private Path tempDir; + private Map batchPointers; + private HaloDB batchDb; + + static { + MAPPER = new ObjectMapper(); + MAPPER.setSerializationInclusion(JsonInclude.Include.NON_EMPTY); + + TYPE_H_E_F_ARRAY = new TypeReference() { }; + } + + /** + * Gets the number of appended files. (Not thread safe.) + * @return a non-negative number + */ + int fileCount() { + return batchDb == null ? 0 : batchPointers.size(); + } + + /** + * Creates a temporary directory, and opens a HashDb for temporary storage + * of history. (Not thread safe.) + */ + public void open() throws IOException { + if (tempDir != null) { + throw new IllegalStateException("already open"); + } + + tempDir = Files.createTempDirectory("org_opengrok-file_history_log"); + Path batchDbPath = tempDir.resolve("batch.db"); + + HaloDBOptions options = new HaloDBOptions(); + options.setMaxFileSize(64 * 1024 * 1024); // 64 MB + options.setFlushDataSizeBytes(8 * 1024 * 1024); // 8 MB + options.setCompactionThresholdPerFile(1.0); + options.setCompactionJobRate(64 * 1024 * 1024); // 64 MB + options.setNumberOfRecords(10_000_000); + options.setCleanUpTombstonesDuringOpen(false); + options.setCleanUpInMemoryIndexOnClose(true); + options.setUseMemoryPool(true); + options.setMemoryPoolChunkSize(2 * 1024 * 1024); // 2 MB + options.setFixedKeySize(Longs.BYTES); + + try { + batchDb = HaloDB.open(batchDbPath.toString(), options); + } catch (HaloDBException e) { + throw new IOException("opening temp db", e); + } + + counter.set(0); + batchPointers = new ConcurrentHashMap<>(); + } + + /** + * Cleans up temporary directory. (Not thread safe.) + */ + @Override + public void close() throws IOException { + if (batchPointers != null) { + batchPointers.clear(); + } + + try { + if (batchDb != null) { + batchDb.close(); + } + } catch (HaloDBException e) { + throw new IOException("closing temp db", e); + } finally { + batchDb = null; + if (tempDir != null) { + IOUtils.removeRecursive(tempDir); + } + } + } + + /** + * Sets the specified {@code entries} as the list of entries for + * {@code file}, which will indicate a full overwrite later. (Thread safe + * but not guaranteeing batch order for simultaneous calls for the same + * {@code file}.) + * @throws IOException if an I/O error occurs writing to temp + */ + public void set(String file, List entries) throws IOException { + incorporate(file, entries, true); + } + + /** + * Appends the specified batch of {@code entries} to the previous list of + * entries for {@code file}. (Thread safe but not guaranteeing batch order + * for simultaneous calls for the same {@code file}.) + * @throws IOException if an I/O error occurs writing to temp + */ + public void append(String file, List entries) throws IOException { + incorporate(file, entries, false); + } + + private void incorporate(String file, List entries, boolean forceOverwrite) + throws IOException { + + if (batchDb == null) { + throw new IllegalStateException("not open"); + } + + HistoryEntryFixed[] fixedEntries = fix(entries); + ByteArrayOutputStream bytesOut = new ByteArrayOutputStream(); + MAPPER.writeValue(bytesOut, fixedEntries); + byte[] serialized = bytesOut.toByteArray(); + byte[] packed = BinPacker.pack(serialized); + + long i = counter.incrementAndGet(); + try { + batchDb.put(Longs.toByteArray(i), packed); + } catch (HaloDBException e) { + throw new IOException("writing temp db", e); + } + + final PointedMeta pointed; + if (forceOverwrite) { + pointed = new PointedMeta(true); + batchPointers.put(file, pointed); + } else { + pointed = batchPointers.computeIfAbsent(file, k -> new PointedMeta(false)); + } + + synchronized (pointed.lock) { + pointed.counters.add(i); + } + } + + /** + * Gets an enumerator to recompose batches of entries by file into + * {@link KeyedHistory} instances. (Not thread safe.) + * @return a defined enumeration + */ + Enumeration getEnumerator() { + + if (batchDb == null) { + throw new IllegalStateException("not open"); + } + + final Iterator iterator = batchPointers.keySet().iterator(); + + return new Enumeration() { + @Override + public boolean hasMoreElements() { + return iterator.hasNext(); + } + + @Override + public KeyedHistory nextElement() { + String file = iterator.next(); + PointedMeta pointed = batchPointers.get(file); + List entries; + try { + entries = readEntries(pointed.counters); + } catch (IOException e) { + throw new RuntimeException("readEntries()", e); + } + return new KeyHistoryImpl(file, entries, pointed.forceOverwrite); + } + }; + } + + private List readEntries(List pointers) throws IOException { + + List res = new ArrayList<>(); + for (long nextCounter : pointers) { + byte[] packed; + try { + packed = batchDb.get(Longs.toByteArray(nextCounter)); + } catch (HaloDBException e) { + throw new IOException("reading temp db", e); + } + byte[] serialized = BinPacker.unpack(packed); + Object obj = MAPPER.readValue(serialized, TYPE_H_E_F_ARRAY); + + if (obj == null) { + LOGGER.log(Level.SEVERE, "Unexpected null"); + } else if (obj instanceof HistoryEntryFixed[]) { + for (HistoryEntryFixed element : (HistoryEntryFixed[]) obj) { + res.add(element.toEntry()); + } + } else { + LOGGER.log(Level.SEVERE, "Unexpected serialized type, {0}", obj.getClass()); + } + } + return res; + } + + private static HistoryEntryFixed[] fix(List entries) { + HistoryEntryFixed[] res = new HistoryEntryFixed[entries.size()]; + + int i = 0; + for (HistoryEntry entry : entries) { + res[i++] = new HistoryEntryFixed(entry); + } + return res; + } + + private static class PointedMeta { + final Object lock = new Object(); + final List counters = new ArrayList<>(); + final boolean forceOverwrite; + + PointedMeta(boolean forceOverwrite) { + this.forceOverwrite = forceOverwrite; + } + } + + private static class KeyHistoryImpl implements KeyedHistory { + private final String file; + private final List entries; + private final boolean forceOverwrite; + + KeyHistoryImpl(String file, List entries, boolean forceOverwrite) { + this.file = file; + this.entries = entries; + this.forceOverwrite = forceOverwrite; + } + + @Override + public String getFile() { + return file; + } + + @Override + public List getEntries() { + return Collections.unmodifiableList(entries); + } + + @Override + public boolean isForceOverwrite() { + return forceOverwrite; + } + } + + private static class BinPacker { + static final int MINIMUM_LENGTH_TO_COMPRESS = 100; + + static byte[] pack(byte[] value) { + if (value.length < MINIMUM_LENGTH_TO_COMPRESS) { + final byte[] packed = new byte[value.length + 1]; + packed[0] = 0; // Set 0 to indicate uncompressed. + System.arraycopy(value, 0, packed, 1, value.length); + return packed; + } else { + ByteArrayOutputStream bytesOut = new ByteArrayOutputStream(); + bytesOut.write(1); // Set 1 to indicate compressed. + + try (GZIPOutputStream gz = new GZIPOutputStream(bytesOut)) { + gz.write(value); + } catch (IOException e) { + // Not expected to happen + throw new RuntimeException(e); + } + return bytesOut.toByteArray(); + } + } + + static byte[] unpack(byte[] packed) { + if (packed[0] == 0) { + byte[] res = new byte[packed.length - 1]; + System.arraycopy(packed, 1, res, 0, packed.length - 1); + return res; + } else { + ByteArrayInputStream bytesIn = new ByteArrayInputStream(packed); + //noinspection ResultOfMethodCallIgnored + bytesIn.read(); + + ByteArrayOutputStream res = new ByteArrayOutputStream(); + try (GZIPInputStream gz = new GZIPInputStream(bytesIn)) { + int b; + while ((b = gz.read()) != -1) { + res.write(b); + } + } catch (IOException e) { + // Not expected to happen + throw new RuntimeException(e); + } + return res.toByteArray(); + } + } + } +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FilteredHistoryEnumeration.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FilteredHistoryEnumeration.java new file mode 100644 index 00000000000..211d560028c --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/FilteredHistoryEnumeration.java @@ -0,0 +1,126 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.history; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.NoSuchElementException; + +/** + * Represents a filtering of a history sequence up to but not including a + * specified revision identifier. For {@link Repository} implementations that + * do not yet support a partial history, then a simple, but expensive, + * filtering of the full history is done. + *

N.b. the underlying sequence is fully exhausted during the filtering. + */ +class FilteredHistoryEnumeration implements HistoryEnumeration { + + private final HistoryEnumeration underlying; + private final String sinceRevision; + private boolean didMatchRevision; + + /** + * Initializes an instance from the specified sequence. + * @param historySequence a defined sequence ordered from most recent to + * earlier between each element and within each element + * @param sinceRevision the revision at which to stop (non-inclusive) + * returning data, or {@code null} to return the full sequence + */ + FilteredHistoryEnumeration(HistoryEnumeration historySequence, String sinceRevision) { + this.underlying = historySequence; + this.sinceRevision = sinceRevision; + } + + @Override + public void close() throws IOException { + underlying.close(); + } + + /** + * Tests if this enumeration contains more elements. + * @return {@code true} if and only if a defined {@code sinceRevision} has + * not yet matched and if the underlying sequence contains at least one + * more element to provide; {@code false} otherwise. + */ + @Override + public boolean hasMoreElements() { + if (didMatchRevision) { + return false; + } else { + return underlying.hasMoreElements(); + } + } + + /** + * Returns the next element of this enumeration if a defined + * {@code sinceRevision} has not yet matched and if the underlying sequence + * has at least one more element to provide. + * @return the next element of this enumeration. + * @throws NoSuchElementException if a defined {@code sinceRevision} has + * matched or if no more elements exist. + */ + @Override + public History nextElement() { + if (didMatchRevision) { + throw new NoSuchElementException(); + } + + History next = underlying.nextElement(); + + if (sinceRevision != null) { + // Iterate to try to match sinceRevision within the element. + int i = 0; + for (; i < next.count(); ++i) { + HistoryEntry historyEntry = next.getHistoryEntry(i); + if (sinceRevision.equals(historyEntry.getRevision())) { + break; + } + } + + if (i < next.count()) { + didMatchRevision = true; + + // non-intuitive order BTW + List filtered = new ArrayList<>(next.getHistoryEntries(i, 0)); + next = new History(filtered, next.getRenamedFiles()); + + // Exhaust the underlying sequence + while (underlying.hasMoreElements()) { + underlying.nextElement(); + } + } + } + + return next; + } + + /** + * {@inheritDoc} + */ + @Override + public int exitValue() { + return underlying.exitValue(); + } +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitHistoryParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitHistoryParser.java index 8da3cd02fd0..d25872bc7f4 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitHistoryParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitHistoryParser.java @@ -24,17 +24,18 @@ package org.opengrok.indexer.history; import java.io.BufferedReader; -import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.nio.charset.StandardCharsets; +import java.io.StringReader; import java.nio.file.InvalidPathException; import java.text.ParseException; import java.util.ArrayList; import java.util.List; +import java.util.NoSuchElementException; +import java.util.function.Consumer; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -42,28 +43,41 @@ import org.opengrok.indexer.logger.LoggerFactory; import org.opengrok.indexer.util.Executor; import org.opengrok.indexer.util.ForbiddenSymlinkException; +import org.opengrok.indexer.util.ObjectCloseableEnumeration; +import org.opengrok.indexer.util.ObjectStreamHandler; import org.opengrok.indexer.util.StringUtils; /** * Parse a stream of Git log comments. */ -class GitHistoryParser implements Executor.StreamHandler { +class GitHistoryParser implements Executor.StreamHandler, ObjectStreamHandler { private static final Logger LOGGER = LoggerFactory.getLogger(GitHistoryParser.class); - private enum ParseState { + private static final int HISTORY_ENTRY_BATCH_SIZE = 512; + private enum ParseState { HEADER, MESSAGE, FILES } + + private final boolean handleRenamedFiles; + private final RuntimeEnvironment env; + private String myDir; private GitRepository repository = new GitRepository(); private List entries = new ArrayList<>(); private History history; - private final boolean handleRenamedFiles; - - GitHistoryParser(boolean flag) { - handleRenamedFiles = flag; + private BufferedReader reader; + private String savedLine; + + /** + * Initializes an instance, with the user specifying whether renamed-files + * handling should be done. + */ + GitHistoryParser(boolean handleRenamedFiles) { + this.handleRenamedFiles = handleRenamedFiles; + this.env = RuntimeEnvironment.getInstance(); } /** @@ -84,21 +98,59 @@ public History getHistory() { @Override public void processStream(InputStream input) throws IOException { try (BufferedReader in = new BufferedReader(GitRepository.newLogReader(input))) { - process(in); + processStream(in); } history = new History(entries); } - - private void process(BufferedReader in) throws IOException { - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); - entries = new ArrayList<>(); + + /** + * Process the output from the log command and insert the HistoryEntries + * into the {@link #getHistory()} property. + * + * @param input The output from the process + * @throws java.io.IOException If an error occurs while reading the stream + */ + public void processStream(BufferedReader input) throws IOException { + process(input); + history = new History(entries); + } + + /** + * Initializes the handler to read from the specified input. + */ + public void initializeObjectStream(InputStream in) { + reader = null; + reader = new BufferedReader(GitRepository.newLogReader(in)); + } + + /** + * Reads a {@link HistoryEntry} from the initialized input unless the + * stream has been exhausted. + * @return a defined instance or {@code null} if the stream has been + * exhausted + */ + public Object readObject() throws IOException { HistoryEntry entry = null; ParseState state = ParseState.HEADER; - String s = in.readLine(); + + String s; + if (savedLine == null) { + s = reader.readLine(); + } else { + s = savedLine; + savedLine = null; + } + while (s != null) { if (state == ParseState.HEADER) { if (s.startsWith("commit")) { + if (entry != null) { + savedLine = s; + return entry; + } + entry = new HistoryEntry(); + entry.setActive(true); String commit = s.substring("commit".length()).trim(); /* @@ -113,25 +165,6 @@ private void process(BufferedReader in) throws IOException { commit = commit.substring(0, offset); } - if (entry != null) { - /* - * With -m, a commit hash repeats for each merged head. - * For OpenGrok's purposes, the same HistoryEntry would - * get reused for each head. If the commit hash differs, - * then add the entry to the list, and prepare to start - * a new entry. - */ - if (commit.equals(entry.getRevision())) { - entry.setMessage(""); // Avoid message repetition. - } else { - entries.add(entry); - entry = null; - } - } - if (entry == null) { - entry = new HistoryEntry(); - } - entry.setActive(true); entry.setRevision(commit); } else if (s.startsWith("Author:") && entry != null) { entry.setAuthor(s.substring("Author:".length()).trim()); @@ -147,12 +180,15 @@ private void process(BufferedReader in) throws IOException { // throw new IOException("Failed to parse author date: " + s, pe); } + } else //noinspection StatementWithEmptyBody + if (s.startsWith("Merge:") && entry != null) { + ; // ignore for now } else if (StringUtils.isOnlyWhitespace(s)) { // We are done reading the heading, start to read the message state = ParseState.MESSAGE; // The current line is empty - the message starts on the next line (to be parsed below). - s = in.readLine(); + s = reader.readLine(); } } @@ -175,7 +211,7 @@ private void process(BufferedReader in) throws IOException { try { File f = new File(myDir, s); String path = env.getPathRelativeToSourceRoot(f); - entry.addFile(path.intern()); + entry.addFile(path); } catch (ForbiddenSymlinkException e) { LOGGER.log(Level.FINER, e.getMessage()); // ignore @@ -187,41 +223,45 @@ private void process(BufferedReader in) throws IOException { } } } - s = in.readLine(); + s = reader.readLine(); } - if (entry != null) { + return entry; + } + + private void process(BufferedReader in) throws IOException { + entries = new ArrayList<>(); + reader = in; + + HistoryEntry entry; + while ((entry = (HistoryEntry) readObject()) != null) { entries.add(entry); } } /** - * Parse the history for the specified file. + * Starts a parse of history for the specified file. * * @param file the file to parse history for - * @param repos Pointer to the GitRepository + * @param repo a defined instance * @param sinceRevision the oldest changeset to return from the executor, or * {@code null} if all changesets should be returned - * @return object representing the file's history + * @param tagger an optional function to tag changesets + * @return a defined sequence representing the file's history */ - History parse(File file, Repository repos, String sinceRevision) throws HistoryException { - myDir = repos.getDirectoryName() + File.separator; - repository = (GitRepository) repos; + HistoryEnumeration startParse(File file, GitRepository repo, String sinceRevision, + Consumer tagger) throws HistoryException { + + myDir = repo.getDirectoryName() + File.separator; + repository = repo; RenamedFilesParser parser = new RenamedFilesParser(); try { - Executor executor = repository.getHistoryLogExecutor(file, sinceRevision); - int status = executor.exec(true, this); - - if (status != 0) { - throw new HistoryException( - String.format("Failed to get history for: \"%s\" Exit code: %d", - file.getAbsolutePath(), - status)); - } + Executor executor; + // Process renames first so they are on the first in sequence. if (handleRenamedFiles) { - executor = repository.getRenamedFilesExecutor(file, sinceRevision); - status = executor.exec(true, parser); + executor = repo.getRenamedFilesExecutor(file, sinceRevision); + int status = executor.exec(true, parser); if (status != 0) { throw new HistoryException( @@ -230,13 +270,16 @@ History parse(File file, Repository repos, String sinceRevision) throws HistoryE status)); } } + + executor = repo.getHistoryLogExecutor(file, sinceRevision); + ObjectCloseableEnumeration entriesSequence = executor.startExec(true, this); + List renamedFiles = parser.getRenamedFiles(); + return newHistoryEnumeration(entriesSequence, renamedFiles, tagger); } catch (IOException e) { throw new HistoryException( String.format("Failed to get history for: \"%s\"", file.getAbsolutePath()), e); } - - return new History(entries, parser.getRenamedFiles()); } /** @@ -247,9 +290,96 @@ History parse(File file, Repository repos, String sinceRevision) throws HistoryE * @throws IOException if we fail to parse the buffer */ History parse(String buffer) throws IOException { - myDir = RuntimeEnvironment.getInstance().getSourceRootPath(); - processStream(new ByteArrayInputStream(buffer.getBytes(StandardCharsets.UTF_8))); - return new History(entries); + myDir = env.getSourceRootPath(); + processStream(new BufferedReader(new StringReader(buffer))); + return history; + } + + /** + * Transforms a specified sequence of {@link HistoryEntry} instances into a + * batching sequence of {@link History} instances. + * @return a defined, wrapping sequence + */ + private static HistoryEnumeration newHistoryEnumeration( + final ObjectCloseableEnumeration entriesSequence, + final List renamedFiles, + final Consumer tagger) { + + // Renamed files are published on the first element. + History firstHistory = nextHistory(entriesSequence, renamedFiles, tagger); + + return new HistoryEnumeration() { + History nextHistory = firstHistory; + + @Override + public void close() throws IOException { + nextHistory = null; + entriesSequence.close(); + } + + @Override + public boolean hasMoreElements() { + return nextHistory != null; + } + + @Override + public History nextElement() { + if (nextHistory == null) { + throw new NoSuchElementException(); + } + History res = nextHistory; + nextHistory = null; + // Renamed files are only published on the first element. + nextHistory = nextHistory(entriesSequence, null, tagger); + return res; + } + + @Override + public int exitValue() { + return entriesSequence.exitValue(); + } + }; + } + + /** + * Reads a next batch if available. + * @return a defined instance or {@code null} if the sequence is exhausted + */ + private static History nextHistory( + final ObjectCloseableEnumeration entriesSequence, + final List renamedFiles, + final Consumer tagger) { + + List entries = null; + while (entriesSequence.hasMoreElements()) { + HistoryEntry entry = (HistoryEntry) entriesSequence.nextElement(); + if (entries == null) { + entries = new ArrayList<>(); + } + entries.add(entry); + if (entries.size() >= HISTORY_ENTRY_BATCH_SIZE) { + break; + } + } + + if (entries == null && renamedFiles == null ) { + return null; + } + + History res; + if (renamedFiles != null) { + if (entries == null) { + entries = new ArrayList<>(); + } + res = new History(entries, renamedFiles); + } else { + res = new History(entries); + } + + if (tagger != null) { + tagger.accept(res); + } + return res; } /** diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java index fd56df56214..398ed6bf800 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java @@ -38,6 +38,7 @@ import java.util.Date; import java.util.List; import java.util.TreeSet; +import java.util.function.Consumer; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; @@ -84,7 +85,7 @@ public class GitRepository extends Repository { /** * All git commands that emit date that needs to be parsed by - * {@code getDateFormat()} should use this option. + * {@link #getDateFormat()} should use this option. */ private static final String GIT_DATE_OPT = "--date=iso8601-strict"; @@ -244,7 +245,7 @@ boolean getHistoryGet( return false; } - HistoryRevResult result = getHistoryRev(sink::write, fullpath, rev); + HistoryRevResult result = getHistoryRev(sink, fullpath, rev); if (!result.success && result.iterations < 1) { /* * If we failed to get the contents it might be that the file was @@ -547,22 +548,46 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { + HistoryEnumeration getHistory(File file) throws HistoryException { return getHistory(file, null); } @Override - History getHistory(File file, String sinceRevision) + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); - History result = new GitHistoryParser(isHandleRenamedFiles()).parse(file, this, sinceRevision); // Assign tags to changesets they represent // We don't need to check if this repository supports tags, // because we know it :-) + Consumer tagger = null; if (env.isTagsEnabled()) { - assignTagsInHistory(result); + tagger = this::assignTagsInHistory; + } + + GitHistoryParser parser = new GitHistoryParser(isHandleRenamedFiles()); + return parser.startParse(file, this, sinceRevision, tagger); + } + + /** + * Expunge entries which have revisions (i.e. commit hashes) identical to + * their immediately preceding neighbor's to accommodate `git log -m` + * returning an entry for each commit of a merge (possibly with distinct + * file lists). + */ + @Override + void deduplicateRevisions(History history) { + HistoryEntry last = null; + for (int i = 0; i < history.count(); ++i) { + HistoryEntry entry = history.getHistoryEntry(i); + if (last == null || last.getRevision() == null || + !last.getRevision().equals(entry.getRevision())) { + last = entry; + } else { + last.merge(entry); + history.removeHistoryEntry(i); + --i; + } } - return result; } @Override diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java index 421f5a922a3..c5efd8a325c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java @@ -24,35 +24,59 @@ package org.opengrok.indexer.history; +import java.beans.Encoder; +import java.beans.Expression; +import java.beans.PersistenceDelegate; +import java.beans.XMLDecoder; +import java.beans.XMLEncoder; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.util.ArrayList; import java.util.List; +import java.util.zip.GZIPInputStream; +import java.util.zip.GZIPOutputStream; /** * Class representing the history of a file. */ public class History { /** Entries in the log. The first entry is the most recent one. */ - private List entries; + private final ArrayList entries; /** * track renamed files so they can be treated in special way (for some * SCMs) during cache creation. * These are relative to repository root. */ - private List renamedFiles = new ArrayList(); + private final ArrayList renamedFiles; public History() { - this(new ArrayList()); + this(new ArrayList<>()); } History(List entries) { - this.entries = entries; + this.entries = new ArrayList<>(entries); + this.renamedFiles = new ArrayList<>(); } History(List entries, List renamed) { - this.entries = entries; - this.renamedFiles = renamed; + this.entries = new ArrayList<>(entries); + this.renamedFiles = new ArrayList<>(renamed); } - + + /** + * Gets the number of history entries. + * @return a non-negative number + */ + public int count() { + return entries == null ? 0 : entries.size(); + } + /** * Set the list of log entries for the file. The first entry is the most * recent one. @@ -60,7 +84,12 @@ public History() { * @param entries The entries to add to the list */ public void setHistoryEntries(List entries) { - this.entries = entries; + if (entries == null) { + this.entries.clear(); + } else if (this.entries != entries) { + this.entries.clear(); + this.entries.addAll(entries); + } } /** @@ -72,6 +101,28 @@ public List getHistoryEntries() { return entries; } + /** + * Returns the entry at the specified position. + * @param index index of the entry to return + * @return the element at the specified position + * @throws IndexOutOfBoundsException if the index is out of range + * ({@code index < 0 || index >= size()}) + */ + HistoryEntry getHistoryEntry(int index) { + return entries.get(index); + } + + /** + * Removes the entry at the specified position. + * @param index the index of the entry to be removed + * @return the entry previously at the specified position + * @throws IndexOutOfBoundsException if the index is out of range + * ({@code index < 0 || index > count()}) + */ + HistoryEntry removeHistoryEntry(int index) { + return entries.remove(index); + } + /** * Get the list of log entries, most recent first. * With parameters @@ -82,7 +133,7 @@ public List getHistoryEntries() { */ public List getHistoryEntries(int limit, int offset) { offset = offset < 0 ? 0 : offset; - limit = offset + limit > entries.size() ? limit = entries.size() - offset : limit; + limit = offset + limit > entries.size() ? entries.size() - offset : limit; return entries.subList(offset, offset + limit); } @@ -121,11 +172,60 @@ public boolean hasTags() { * Gets a value indicating if {@code file} is in the list of renamed files. * TODO: Warning -- this does a slow {@link List} search. */ - public boolean isRenamed(String file) { + public boolean isRenamedBySlowLookup(String file) { return renamedFiles.contains(file); } public List getRenamedFiles() { return renamedFiles; } + + /** + * Reads a GZIP file to decode a {@link History} instance. + * @param file the source file + * @return a defined instance + * @throws IOException if an I/O error occurs + */ + static History readGZIP(File file) throws IOException { + try (FileInputStream in = new FileInputStream(file); + BufferedInputStream bufIn = new BufferedInputStream(in); + GZIPInputStream gzIn = new GZIPInputStream(bufIn)) { + return decodeObject(gzIn); + } + } + + static History decodeObject(InputStream in) { + try (XMLDecoder d = new XMLDecoder(in)) { + return (History) d.readObject(); + } + } + + /** + * Writes the current instance to a file with GZIP compression. + * @param file the target file + * @throws IOException if an I/O error occurs + */ + void writeGZIP(File file) throws IOException { + try (FileOutputStream out = new FileOutputStream(file); + BufferedOutputStream bufOut = new BufferedOutputStream(out); + GZIPOutputStream gzOut = new GZIPOutputStream(bufOut)) { + encodeObject(gzOut); + } + } + + void encodeObject(OutputStream out) { + try (XMLEncoder e = new XMLEncoder(out)) { + e.setPersistenceDelegate(File.class, new FilePersistenceDelegate()); + e.writeObject(this); + } + } + + static class FilePersistenceDelegate extends PersistenceDelegate { + @Override + protected Expression instantiate(Object oldInstance, Encoder out) { + File f = (File) oldInstance; + return new Expression(oldInstance, f.getClass(), "new", + new Object[] {f.toString()}); + } + } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryCache.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryCache.java index d466a64c49f..79b5b1f4a28 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryCache.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryCache.java @@ -19,11 +19,13 @@ /* * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.File; import java.util.Date; +import java.util.Enumeration; import java.util.Map; import org.opengrok.indexer.util.ForbiddenSymlinkException; @@ -65,14 +67,17 @@ History get(File file, Repository repository, boolean withFiles) throws HistoryException, ForbiddenSymlinkException; /** - * Store the history for a repository. - * - * @param history The history to store + * Stores the history enumeration for a repository, where + * {@code historyElements} must be ordered from most recent to earlier + * between each element and within each element. + * @param historySequence The history series to store * @param repository The repository whose history to store + * @param forceOverwrite a value indicating whether to overwrite existing + * stored history for the files in {@code historySequence} * @throws HistoryException if the history cannot be stored */ - void store(History history, Repository repository) - throws HistoryException; + void store(Enumeration historySequence, Repository repository, + boolean forceOverwrite) throws HistoryException; /** * Optimize how the history is stored on disk. This method is typically @@ -86,6 +91,12 @@ void store(History history, Repository repository) */ void optimize() throws HistoryException; + /** + * Closes the history cache, releasing any resources. {@link #initialize()} + * must be called before accessing the cache again. + */ + void close() throws HistoryException; + /** * Check if the specified directory is present in the cache. * @param directory the directory to check diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntry.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntry.java index c918c737da3..a42a8939775 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntry.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntry.java @@ -19,18 +19,19 @@ /* * Copyright (c) 2006, 2018 Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2019, Chris Fraire . - */ -/* * Copyright 2006 Trond Norbye. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; +import java.util.Arrays; import java.util.Date; +import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Pattern; import org.opengrok.indexer.logger.LoggerFactory; @@ -44,21 +45,21 @@ public class HistoryEntry { static final String TAGS_SEPARATOR = ", "; private static final Logger LOGGER = LoggerFactory.getLogger(HistoryEntry.class); + private static final String TAGS_SEP_ESC = Pattern.quote(TAGS_SEPARATOR); private String revision; private Date date; private String author; private String tags; - @SuppressWarnings("PMD.AvoidStringBufferField") - private final StringBuffer message; + private final StringBuilder message; private boolean active; private SortedSet files; /** Creates a new instance of HistoryEntry. */ public HistoryEntry() { - message = new StringBuffer(); + message = new StringBuilder(); files = new TreeSet<>(); } @@ -82,7 +83,7 @@ public HistoryEntry(String revision, Date date, String author, setDate(date); this.author = author; this.tags = tags; - this.message = new StringBuffer(message); + this.message = new StringBuilder(message); this.active = active; this.files = new TreeSet<>(); } @@ -210,4 +211,43 @@ public void stripFiles() { public void stripTags() { tags = null; } + + public void merge(HistoryEntry other) { + // Merge files if necessary. + if (other.files != null) { + if (files == null) { + files = other.files; + other.files = null; + } else { + files.addAll(other.files); + } + } + // Merge tags if necessary. + if (other.tags != null) { + if (tags == null) { + tags = other.tags; + other.tags = null; + } else { + tags = mergeTags(tags, other.tags); + } + } + } + + private static String mergeTags(String l, String r) { + Set distinct = new TreeSet<>(); + distinct.addAll(Arrays.asList(l.split(TAGS_SEP_ESC))); + distinct.addAll(Arrays.asList(r.split(TAGS_SEP_ESC))); + + StringBuilder merged = new StringBuilder(); + for (String rev : distinct) { + merged.append(rev); + merged.append(TAGS_SEPARATOR); + } + if (merged.length() > 0) { + // Remove last trailing separator. + merged.setLength(merged.length() - TAGS_SEPARATOR.length()); + } + + return merged.toString(); + } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntryFixed.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntryFixed.java new file mode 100644 index 00000000000..b85d17f827f --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntryFixed.java @@ -0,0 +1,140 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2006, 2018 Oracle and/or its affiliates. All rights reserved. + * Copyright 2006 Trond Norbye. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . + */ +package org.opengrok.indexer.history; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +import java.time.Instant; +import java.util.Arrays; +import java.util.Date; + +/** + * Represents an immutable (i.e. "fixed") version of {@link HistoryEntry} for + * serialization. + * @author Trond Norbye + */ +public class HistoryEntryFixed { + + private final String revision; + private final Long date; // Date Instant toEpochMilli() + private final String author; + private final String tags; + private final String message; + private final boolean active; + private final String[] files; + + /** + * Copy constructor to fix the specified {@link HistoryEntry} fully. + */ + HistoryEntryFixed(HistoryEntry that) { + this(that, false); + } + + /** + * Copy constructor to fix the specified {@link HistoryEntry} with a + * parameter indicating whether to trim files from the entry. + */ + HistoryEntryFixed(HistoryEntry that, boolean trim) { + this.revision = that.getRevision(); + + Date thatDate = that.getDate(); + this.date = thatDate == null ? null : thatDate.toInstant().toEpochMilli(); + + this.author = that.getAuthor(); + this.tags = that.getTags(); + this.message = that.getMessage(); + this.active = that.isActive(); + this.files = trim ? new String[0] : that.getFiles().toArray(new String[0]); + } + + @JsonCreator + public HistoryEntryFixed(@JsonProperty("revision") String revision, + @JsonProperty("date") Long date, + @JsonProperty("author") String author, + @JsonProperty("tags") String tags, + @JsonProperty("message") String message, + @JsonProperty("active") boolean active, + @JsonProperty("files") String[] files) { + + this.revision = revision; + this.date = date; + this.author = author; + this.tags = tags; + this.message = message; + this.active = active; + this.files = files; + } + + public String getAuthor() { + return author; + } + + public String getTags() { + return tags; + } + + public Long getDate() { + return date; + } + + public String getMessage() { + return message; + } + + public String getRevision() { + return revision; + } + + public boolean isActive() { + return active; + } + + public String[] getFiles() { + return Arrays.copyOf(files, files.length); + } + + HistoryEntry toEntry() { + HistoryEntry res = new HistoryEntry(); + + res.setRevision(this.revision); + + if (this.date != null) { + res.setDate(Date.from(Instant.ofEpochMilli(this.date))); + } + + res.setAuthor(this.author); + res.setTags(this.tags); + res.appendMessage(this.message); + res.setActive(this.active); + + if (this.files != null) { + for (String file : this.files) { + res.addFile(file); + } + } + return res; + } +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEnumeration.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEnumeration.java new file mode 100644 index 00000000000..0c35056838e --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEnumeration.java @@ -0,0 +1,44 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.history; + +import java.io.Closeable; +import java.util.Enumeration; + +/** + * Represents an API for a sequence of {@link History} instances where the + * sequence is {@link Closeable} to release resources. + */ +public interface HistoryEnumeration extends Enumeration, Closeable { + + /** + * Returns the exit value for the subprocess. + * + * @return the exit value of the subprocess represented by this instance. + * By convention, the value {@code 0} indicates normal termination. + * @throws IllegalThreadStateException if the subprocess represented by + * this instance has not yet terminated + */ + int exitValue(); +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java index e7508ffbe3b..3844c45325c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java @@ -151,22 +151,30 @@ public String getCacheInfo() throws HistoryException { * @throws IOException if I/O exception occurs */ public Annotation annotate(File file, String rev) throws IOException { - Annotation ret = null; + Annotation ret = null; Repository repo = getRepository(file); if (repo != null) { ret = repo.annotate(file, rev); - History hist = null; - try { - hist = repo.getHistory(file); - } catch (HistoryException ex) { - LOGGER.log(Level.FINEST, - "Cannot get messages for tooltip: ", ex); + HistoryEnumeration historySequence = null; + if (ret != null) { + try { + historySequence = repo.getHistory(file); + } catch (HistoryException ex) { + LOGGER.log(Level.FINEST, "Cannot get history for " + file, ex); + } } - if (hist != null && ret != null) { + if (historySequence != null) { + History hist = null; + try { + hist = HistoryUtil.union(historySequence); + } catch (HistoryException ex) { + LOGGER.log(Level.FINEST, "Error reading entire history for " + file, ex); + throw new IOException(ex); // Wrap the history exception + } Set revs = ret.getRevisions(); int revsMatched = 0; - // !!! cannot do this because of not matching rev ids (keys) + // !!! cannot do this because of not matching rev ids (keys) // first is the most recent one, so we need the position of "rev" // until the end of the list //if (hent.indexOf(rev)>0) { @@ -262,7 +270,7 @@ public History getHistory(File file, boolean withFiles, boolean ui) return null; } } - return repo.getHistory(file); + return HistoryUtil.union(repo.getHistory(file)); } return null; @@ -556,12 +564,8 @@ private void createCache(Repository repository, String sinceRevision) { private void createCacheReal(Collection repositories) { Statistics elapsed = new Statistics(); ExecutorService executor = env.getIndexerParallelizer().getHistoryExecutor(); - // Since we know each repository object from the repositories - // collection is unique, we can abuse HashMap to create a list of - // repository,revision tuples with repository as key (as the revision - // string does not have to be unique - surely it is not unique - // for the initial index case). - HashMap repos2process = new HashMap<>(); + + List repos2process = new ArrayList<>(); // Collect the list of pairs first so that we // do not have to deal with latch decrementing in the cycle below. @@ -570,7 +574,7 @@ private void createCacheReal(Collection repositories) { try { latestRev = historyCache.getLatestCachedRevision(repo); - repos2process.put(repo, latestRev); + repos2process.add(new CreateCacheWork(repo, latestRev)); } catch (HistoryException he) { LOGGER.log(Level.WARNING, String.format( @@ -582,20 +586,22 @@ private void createCacheReal(Collection repositories) { LOGGER.log(Level.INFO, "Creating historycache for {0} repositories", repos2process.size()); final CountDownLatch latch = new CountDownLatch(repos2process.size()); - for (final Map.Entry entry : repos2process.entrySet()) { - executor.submit(new Runnable() { + for (final CreateCacheWork entry : repos2process) { + Future submission = executor.submit(new Runnable() { @Override public void run() { try { - createCache(entry.getKey(), entry.getValue()); + createCache(entry.repo, entry.latestRev); } catch (Exception ex) { // We want to catch any exception since we are in thread. - LOGGER.log(Level.WARNING, "createCacheReal() got exception", ex); + LOGGER.log(Level.SEVERE, "Failed historycache for " + + entry.repo.getDirectoryName(), ex); } finally { latch.countDown(); } } }); + entry.submission = submission; } /* @@ -606,10 +612,19 @@ public void run() { try { latch.await(); } catch (InterruptedException ex) { - LOGGER.log(Level.SEVERE, "latch exception", ex); + LOGGER.log(Level.SEVERE, "Failed to await latch", ex); return; } + for (CreateCacheWork entry : repos2process) { + try { + entry.submission.get(); + } catch (Exception ex) { + LOGGER.log(Level.SEVERE, "Failed historycache for " + + entry.repo.getDirectoryName(), ex); + } + } + // The cache has been populated. Now, optimize how it is stored on // disk to enhance performance and save space. try { @@ -876,7 +891,7 @@ public void run() { try { latch.await(); } catch (InterruptedException ex) { - LOGGER.log(Level.SEVERE, "latch exception", ex); + LOGGER.log(Level.SEVERE, "Failed to await latch", ex); } executor.shutdown(); @@ -899,4 +914,15 @@ private void putRepository(Repository repository) { repositoryRoots.put(repoDirParent, ""); repositories.put(repoDirectoryName, repository); } + + private static class CreateCacheWork { + final Repository repo; + final String latestRev; + Future submission; + + CreateCacheWork(Repository repo, String latestRev) { + this.repo = repo; + this.latestRev = latestRev; + } + } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryUtil.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryUtil.java new file mode 100644 index 00000000000..6b0d9584e34 --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryUtil.java @@ -0,0 +1,69 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.history; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +/** + * Represents a container for {@link History}-related utility methods. + */ +public class HistoryUtil { + + /** + * Initialize an instance as the union of a sequence and closes the + * sequence to affirm a zero exit value. + * @param sequence a defined sequence + * @throws HistoryException if the sequence fails to enumerate or the + * subprocess returns non-zero + */ + public static History union(HistoryEnumeration sequence) throws HistoryException { + + List entries = new ArrayList<>(); + List renamedFiles = new ArrayList<>(); + + while (sequence.hasMoreElements()) { + History that = sequence.nextElement(); + entries.addAll(that.getHistoryEntries()); + renamedFiles.addAll(that.getRenamedFiles()); + } + + try { + sequence.close(); + } catch (IOException e) { + throw new HistoryException("Error closing sequence", e); + } + + if (sequence.exitValue() != 0) { + throw new HistoryException("HistoryEnumeration exit value=" + sequence.exitValue()); + } + + return new History(entries, renamedFiles); + } + + /* private to enforce static */ + private HistoryUtil() { + } +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/KeyedHistory.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/KeyedHistory.java new file mode 100644 index 00000000000..e60e60aa3a0 --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/KeyedHistory.java @@ -0,0 +1,36 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.history; + +import java.util.List; + +/** + * Represents an API for a file name and its associated {@link HistoryEntry} + * collection. + */ +interface KeyedHistory { + String getFile(); + List getEntries(); + boolean isForceOverwrite(); +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialHistoryParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialHistoryParser.java index 793597d0f45..82c26be2cda 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialHistoryParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialHistoryParser.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2006, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017, 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -111,9 +111,10 @@ History parse(File file, String changeset) throws HistoryException { public void processStream(InputStream input) throws IOException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); BufferedReader in = new BufferedReader(new InputStreamReader(input)); - entries = new ArrayList(); - String s; + entries = new ArrayList<>(); HistoryEntry entry = null; + + String s; while ((s = in.readLine()) != null) { if (s.startsWith(MercurialRepository.CHANGESET)) { entry = new HistoryEntry(); @@ -141,7 +142,7 @@ public void processStream(InputStream input) throws IOException { File f = new File(mydir, strings[ii]); try { String path = env.getPathRelativeToSourceRoot(f); - entry.addFile(path.intern()); + entry.addFile(path); } catch (ForbiddenSymlinkException e) { LOGGER.log(Level.FINER, e.getMessage()); // ignore diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index a9754416849..a426dc2901d 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -66,13 +66,13 @@ public class MercurialRepository extends Repository { * The boolean property and environment variable name to indicate whether * forest-extension in Mercurial adds repositories inside the repositories. */ - public static final String NOFOREST_PROPERTY_KEY + private static final String NOFOREST_PROPERTY_KEY = "org.opengrok.indexer.history.mercurial.disableForest"; static final String CHANGESET = "changeset: "; static final String USER = "user: "; static final String DATE = "date: "; - static final String DESCRIPTION = "description: "; + private static final String DESCRIPTION = "description: "; static final String FILE_COPIES = "file_copies: "; static final String FILES = "files: "; static final String END_OF_ENTRY @@ -147,7 +147,6 @@ String determineBranch(CommandTimeoutType cmdType) throws IOException { Executor getHistoryLogExecutor(File file, String sinceRevision) throws HistoryException, IOException { String filename = getRepoRelativePath(file); - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); List cmd = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); @@ -250,7 +249,7 @@ private HistoryRevResult getHistoryRev( * @param fullpath file path * @param full_rev_to_find revision number (in the form of * {rev}:{node|short}) - * @returns original filename + * @return original filename */ private String findOriginalName(String fullpath, String full_rev_to_find) throws IOException { @@ -354,7 +353,7 @@ boolean getHistoryGet( return false; } - HistoryRevResult result = getHistoryRev(sink::write, fullpath, rev); + HistoryRevResult result = getHistoryRev(sink, fullpath, rev); if (!result.success && result.iterations < 1) { /* * If we failed to get the contents it might be that the file was @@ -384,10 +383,9 @@ boolean getHistoryGet( * @param file file to annotate * @param revision revision to annotate * @return file annotation - * @throws java.io.IOException if I/O exception occurred */ @Override - public Annotation annotate(File file, String revision) throws IOException { + public Annotation annotate(File file, String revision) { ArrayList argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); @@ -494,12 +492,12 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { + HistoryEnumeration getHistory(File file) throws HistoryException { return getHistory(file, null); } @Override - History getHistory(File file, String sinceRevision) + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); // Note that the filtering of revisions based on sinceRevision is done @@ -518,7 +516,7 @@ History getHistory(File file, String sinceRevision) if (env.isTagsEnabled()) { assignTagsInHistory(result); } - return result; + return new SingleHistory(result); } /** @@ -573,7 +571,6 @@ String determineParent(CommandTimeoutType cmdType) throws IOException { @Override public String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { - String line = null; File directory = new File(getDirectoryName()); List cmd = new ArrayList<>(); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialTagEntry.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialTagEntry.java index 2abd15746e0..b581f934430 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialTagEntry.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialTagEntry.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2012, 2019 Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -39,6 +40,6 @@ public int compareTo(HistoryEntry that) { assert this.revision != NOREV : "Mercurial TagEntry created without revision specified"; String[] revs = that.getRevision().split(":"); assert revs.length == 2 : "Unable to parse revision format"; - return ((Integer) this.revision).compareTo(Integer.parseInt(revs[0])); + return Integer.compare(this.revision, Integer.parseInt(revs[0])); } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneHistoryParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneHistoryParser.java index 34299395f95..4276993cc8f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneHistoryParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneHistoryParser.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2009, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017, 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -98,10 +98,10 @@ History parse(File file, String changeset) throws HistoryException { public void processStream(InputStream input) throws IOException { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); BufferedReader in = new BufferedReader(new InputStreamReader(input)); - String s; - HistoryEntry entry = null; int state = 0; + + String s; while ((s = in.readLine()) != null) { s = s.trim(); // Later versions of monotone (such as 1.0) output even more dashes so lets require @@ -164,9 +164,8 @@ public void processStream(InputStream input) throws IOException { for (String f : files) { File file = new File(mydir, f); try { - String path = env.getPathRelativeToSourceRoot( - file); - entry.addFile(path.intern()); + String path = env.getPathRelativeToSourceRoot(file); + entry.addFile(path); } catch (ForbiddenSymlinkException e) { LOGGER.log(Level.FINER, e.getMessage()); // ignore diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java index 1299669ab48..544e747564e 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MonotoneRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2009, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017-2018, Chris Fraire . + * Portions Copyright (c) 2017-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -184,14 +184,14 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { + HistoryEnumeration getHistory(File file) throws HistoryException { return getHistory(file, null); } @Override - History getHistory(File file, String sinceRevision) + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { - return new MonotoneHistoryParser(this).parse(file, sinceRevision); + return new SingleHistory(new MonotoneHistoryParser(this).parse(file, sinceRevision)); } private String getQuietOption() { @@ -202,7 +202,7 @@ private String getQuietOption() { } } - public static final String DEPRECATED_KEY + private static final String DEPRECATED_KEY = "org.opengrok.indexer.history.monotone.deprecated"; private boolean useDeprecated() { @@ -248,7 +248,7 @@ String determineBranch(CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java index 21ded704769..586f092584d 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/PerforceRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2018, 2020, Chris Fraire . + * Portions Copyright (c) 2018-2020, Chris Fraire . * Portions Copyright (c) 2019, Chris Ross . */ package org.opengrok.indexer.history; @@ -216,15 +216,15 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { + HistoryEnumeration getHistory(File file) throws HistoryException { PerforceHistoryParser parser = new PerforceHistoryParser(this); - return parser.parse(file); + return new SingleHistory(parser.parse(file)); } @Override - History getHistory(File file, String sinceRevision) throws HistoryException { + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { PerforceHistoryParser parser = new PerforceHistoryParser(this); - return parser.parse(file, sinceRevision); + return new SingleHistory(parser.parse(file, sinceRevision)); } @Override diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java index ba217bd0b70..e1c7e9d5275 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RCSRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2018, Chris Fraire . + * Portions Copyright (c) 2018-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -93,7 +93,7 @@ boolean fileHasAnnotation(File file) { } @Override - Annotation annotate(File file, String revision) throws IOException { + Annotation annotate(File file, String revision) { List argv = new ArrayList<>(); ensureCommand(CMD_BLAME_PROPERTY_KEY, CMD_BLAME_FALLBACK); @@ -118,9 +118,7 @@ Annotation annotate(File file, String revision) throws IOException { */ static IOException wrapInIOException(String message, Throwable t) { // IOException's constructor takes a Throwable, but only in JDK 6 - IOException ioe = new IOException(message + ": " + t.getMessage()); - ioe.initCause(t); - return ioe; + return new IOException(message + ": " + t.getMessage(), t); } @Override @@ -166,22 +164,22 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { - return new RCSHistoryParser().parse(file, this); + HistoryEnumeration getHistory(File file) throws HistoryException { + return new SingleHistory(new RCSHistoryParser().parse(file, this)); } @Override - String determineParent(CommandTimeoutType cmdType) throws IOException { + String determineParent(CommandTimeoutType cmdType) { return null; } @Override - String determineBranch(CommandTimeoutType cmdType) throws IOException { + String determineBranch(CommandTimeoutType cmdType) { return null; } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java index 22143828c0b..6ba330e5eac 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RazorRepository.java @@ -20,7 +20,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. * Portions Copyright 2008 Peter Bray - * Portions Copyright (c) 2017-2018, Chris Fraire . + * Portions Copyright (c) 2017-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -166,31 +166,6 @@ public void setDirectoryName(File directory) { = new File(directory, RAZOR_DIR).getAbsolutePath(); } - public String getOpengrokSourceRootDirectoryPath() { - return opengrokSourceRootDirectoryPath; - } - - public void setOpengrokSourceRootDirectoryPath(String opengrokSourceRootDirectoryPath) { - this.opengrokSourceRootDirectoryPath = opengrokSourceRootDirectoryPath; - } - - public String getRazorGroupBaseDirectoryPath() { - return razorGroupBaseDirectoryPath; - } - - public void setRazorGroupBaseDirectoryPath(String razorGroupBaseDirectoryPath) { - this.razorGroupBaseDirectoryPath = razorGroupBaseDirectoryPath; - } - - String getOpenGrokFileNameFor(File file) { - return file.getAbsolutePath() - .substring(opengrokSourceRootDirectoryPath.length()); - } - - File getSourceNameForOpenGrokName(String path) { - return new File(opengrokSourceRootDirectoryPath + path); - } - File getRazorHistoryFileFor(File file) throws IOException { return pathTranslation(file, "/History/", "", ""); } @@ -344,12 +319,12 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { - return new RazorHistoryParser().parse(file, this); + HistoryEnumeration getHistory(File file) throws HistoryException { + return new SingleHistory(new RazorHistoryParser().parse(file, this)); } @Override - String determineParent(CommandTimeoutType cmdType) throws IOException { + String determineParent(CommandTimeoutType cmdType) { return null; } @@ -359,7 +334,7 @@ String determineBranch(CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java index b89fcb6f03d..838bc661408 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepoRepository.java @@ -20,12 +20,11 @@ /* * Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2010, Trond Norbye . All rights reserved. - * Portions Copyright (c) 2018, Chris Fraire . + * Portions Copyright (c) 2018-2019, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.File; -import java.io.IOException; import org.opengrok.indexer.configuration.CommandTimeoutType; import org.opengrok.indexer.util.BufferSink; @@ -86,7 +85,7 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) { + HistoryEnumeration getHistory(File file) { throw new UnsupportedOperationException("Should never be called!"); } @@ -107,7 +106,7 @@ Annotation annotate(File file, String revision) { } @Override - String determineParent(CommandTimeoutType cmdType) throws IOException { + String determineParent(CommandTimeoutType cmdType) { return null; } @@ -117,7 +116,7 @@ String determineBranch(CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java index a353c5cc089..79ac8a58230 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java @@ -90,7 +90,7 @@ public abstract class Repository extends RepositoryInfo { abstract boolean fileHasHistory(File file); /** - * Check if the repository supports {@code getHistory()} requests for whole + * Check if the repository supports {@link #getHistory(File)} requests for whole * directories at once. * * @return {@code true} if the repository can get history for directories @@ -101,10 +101,13 @@ public abstract class Repository extends RepositoryInfo { * Get the history log for the specified file or directory. * * @param file the file to get the history for - * @return history log for file + * @return history log for file or directory as an enumeration ordered from + * most recent to earlier between each element and within each element to + * allow for efficiency where possible to avoid bringing the entire history + * into program memory simultaneously. * @throws HistoryException on error accessing the history */ - abstract History getHistory(File file) throws HistoryException; + abstract HistoryEnumeration getHistory(File file) throws HistoryException; public Repository() { super(); @@ -114,7 +117,7 @@ public Repository() { /** * Gets the instance's repository command, primarily for testing purposes. - * @return null if not {@link isWorking}, or otherwise a defined command + * @return null if not {@link #isWorking()}, or otherwise a defined command */ public String getRepoCommand() { isWorking(); @@ -137,40 +140,37 @@ public String getRepoCommand() { * @param file the file to get the history for * @param sinceRevision the revision right before the first one to return, * or {@code null} to return the full history - * @return partial history for file + * @return partial history log for file or directory as an enumeration + * ordered from most recent to earlier between each element and within each + * element to allow for efficiency where possible to avoid bringing the + * entire history into program memory simultaneously. * @throws HistoryException on error accessing the history */ - History getHistory(File file, String sinceRevision) + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { // If we want an incremental history update and get here, warn that // it may be slow. if (sinceRevision != null) { + String repoSimpleName = getClass().getSimpleName(); LOGGER.log(Level.WARNING, "Incremental history retrieval is not implemented for {0}.", - getClass().getSimpleName()); + repoSimpleName); LOGGER.log(Level.WARNING, - "Falling back to slower full history retrieval."); + "Falling back to slower full history retrieval for {0}.", + repoSimpleName); } - History history = getHistory(file); - - if (sinceRevision == null) { - return history; - } - - List partial = new ArrayList<>(); - for (HistoryEntry entry : history.getHistoryEntries()) { - partial.add(entry); - if (sinceRevision.equals(entry.getRevision())) { - // Found revision right before the first one to return. - break; - } - } + return new FilteredHistoryEnumeration(getHistory(file), sinceRevision); + } - removeAndVerifyOldestChangeset(partial, sinceRevision); - history.setHistoryEntries(partial); - return history; + /** + * Does nothing, but subclasses can override to remove duplicates that may + * exist in a list of history assumed to be sorted with most recent + * changeset first. + */ + void deduplicateRevisions(History history) { + // nothing } /** @@ -178,7 +178,7 @@ History getHistory(File file, String sinceRevision) * changeset first) and verify that it is the changeset we expected to find * there. * - * @param entries a list of {@code HistoryEntry} objects + * @param entries a list of {@link HistoryEntry} objects * @param revision the revision we expect the oldest entry to have * @throws HistoryException if the oldest entry was not the one we expected */ @@ -274,12 +274,12 @@ TreeSet getTagList() { } /** - * Assign tags to changesets they represent The complete list of tags must - * be pre-built using {@code getTagList()}. Then this function squeeze all + * Assign tags to changesets they represent. The complete list of tags must + * be pre-built using {@link #buildTagList(File, boolean)}, and then this function squeezes all * tags to changesets which actually exist in the history of given file. - * Must be implemented repository-specific. + * The {@link TagEntry} instances must be implemented repository-specific. * - * @see getTagList + * @see #buildTagList(File, boolean) * @param hist History we want to assign tags to. */ void assignTagsInHistory(History hist) { @@ -296,20 +296,20 @@ void assignTagsInHistory(History hist) { // Assign all tags created since the last revision // Revision in this HistoryEntry must be already specified! // TODO is there better way to do this? We need to "repeat" - // last element returned by call to next() + // last element returned by call to next() while (lastTagEntry != null || it.hasNext()) { if (lastTagEntry == null) { lastTagEntry = it.next(); } - if (lastTagEntry.compareTo(ent) >= 0) { - if (ent.getTags() == null) { - ent.setTags(lastTagEntry.getTags()); - } else { - ent.setTags(ent.getTags() + TAGS_SEPARATOR + lastTagEntry.getTags()); - } - } else { + if (lastTagEntry.compareTo(ent) < 0) { break; } + + if (ent.getTags() == null) { + ent.setTags(lastTagEntry.getTags()); + } else { + ent.setTags(ent.getTags() + TAGS_SEPARATOR + lastTagEntry.getTags()); + } if (it.hasNext()) { lastTagEntry = it.next(); } else { @@ -352,8 +352,8 @@ protected String getRevisionForAnnotate(String history_revision) { /** * Create a history log cache for all files in this repository. - * {@code getHistory()} is used to fetch the history for the entire - * repository. If {@code hasHistoryForDirectories()} returns {@code false}, + * {@link #getHistory(File, String)} is used to fetch the history for the entire + * repository. If {@link #hasHistoryForDirectories()} returns {@code false}, * this method is a no-op. * * @param cache the cache instance in which to store the history log @@ -382,9 +382,9 @@ final void createCache(HistoryCache cache, String sinceRevision) File directory = new File(getDirectoryName()); - History history; + HistoryEnumeration historySequence; try { - history = getHistory(directory, sinceRevision); + historySequence = getHistory(directory, sinceRevision); } catch (HistoryException he) { if (sinceRevision == null) { // Failed to get full history, so fail. @@ -397,12 +397,13 @@ final void createCache(HistoryCache cache, String sinceRevision) LOGGER.log(Level.WARNING, "Failed to get partial history. Attempting to " + "recreate the history cache from scratch.", he); - history = null; + historySequence = null; } - if (sinceRevision != null && history == null) { + if (sinceRevision != null && historySequence == null) { + sinceRevision = null; // Failed to get partial history, now get full history instead. - history = getHistory(directory); + historySequence = getHistory(directory); // Got full history successfully. Clear the history cache so that // we can recreate it from scratch. cache.clear(this); @@ -410,12 +411,24 @@ final void createCache(HistoryCache cache, String sinceRevision) // We need to refresh list of tags for incremental reindex. RuntimeEnvironment env = RuntimeEnvironment.getInstance(); - if (env.isTagsEnabled() && this.hasFileBasedTags()) { + if (env.isTagsEnabled() && hasFileBasedTags()) { this.buildTagList(new File(this.getDirectoryName()), CommandTimeoutType.INDEXER); } - if (history != null) { - cache.store(history, this); + if (historySequence != null) { + cache.store(historySequence, this, sinceRevision == null); + + try { + historySequence.close(); + } catch (IOException e) { + throw new HistoryException(String.format("Error closing history of %s", + getDirectoryName()), e); + } + + if (historySequence.exitValue() != 0) { + throw new HistoryException(String.format("HistoryEnumeration exit value %d for %s", + historySequence.exitValue(), getDirectoryName())); + } } } @@ -511,7 +524,7 @@ boolean isNestable() { return false; } - private DateFormat getDateFormat() { + protected DateFormat getDateFormat() { return new RepositoryDateFormat(); } @@ -676,4 +689,5 @@ public Date parse(String source, ParsePosition pos) { throw new UnsupportedOperationException("not implemented"); } } + } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java index 8bcbce52161..c4a8ff41a64 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2018, Chris Fraire . + * Portions Copyright (c) 2018-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -184,8 +184,8 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { - return new SCCSHistoryParser(this).parse(file); + HistoryEnumeration getHistory(File file) throws HistoryException { + return new SingleHistory(new SCCSHistoryParser(this).parse(file)); } @Override @@ -224,7 +224,7 @@ String determineBranch(CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMHistoryParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMHistoryParser.java index f7ef3cf7d05..4e9598500d9 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMHistoryParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMHistoryParser.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2013, 2020, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -146,17 +147,12 @@ public void processStream(InputStream input) throws IOException { } History parse(File file, String sinceRevision) throws HistoryException { - try { - Executor executor = repository.getHistoryLogExecutor(file, sinceRevision); - int status = executor.exec(true, this); + Executor executor = repository.getHistoryLogExecutor(file, sinceRevision); + int status = executor.exec(true, this); - if (status != 0) { - throw new HistoryException("Failed to get history for: \"" - + file.getAbsolutePath() + "\" Exit code: " + status); - } - } catch (IOException e) { + if (status != 0) { throw new HistoryException("Failed to get history for: \"" - + file.getAbsolutePath() + "\"", e); + + file.getAbsolutePath() + "\" Exit code: " + status); } return history; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java index 5b2d9e05982..9a0adb1e997 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SSCMRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2018, Chris Fraire . + * Portions Copyright (c) 2018-2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -129,7 +129,7 @@ private Properties getProperties(File file) { * {@code null} if all changesets should be returned * @return An Executor ready to be started */ - Executor getHistoryLogExecutor(final File file, String sinceRevision) throws IOException { + Executor getHistoryLogExecutor(final File file, String sinceRevision) { List argv = new ArrayList<>(); ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); @@ -159,14 +159,14 @@ Executor getHistoryLogExecutor(final File file, String sinceRevision) throws IOE } @Override - History getHistory(File file) throws HistoryException { + HistoryEnumeration getHistory(File file) throws HistoryException { return getHistory(file, null); } @Override - History getHistory(File file, String sinceRevision) + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { - return new SSCMHistoryParser(this).parse(file, sinceRevision); + return new SingleHistory(new SSCMHistoryParser(this).parse(file, sinceRevision)); } @Override @@ -306,7 +306,7 @@ Annotation annotate(File file, String revision) throws IOException { return parseAnnotation(exec.getOutputReader(), file.getName()); } - protected Annotation parseAnnotation(Reader input, String fileName) + private Annotation parseAnnotation(Reader input, String fileName) throws IOException { BufferedReader in = new BufferedReader(input); Annotation ret = new Annotation(fileName); @@ -345,7 +345,7 @@ boolean isRepositoryFor(File file, CommandTimeoutType cmdType) { } @Override - String determineParent(CommandTimeoutType cmdType) throws IOException { + String determineParent(CommandTimeoutType cmdType) { return null; } @@ -355,7 +355,7 @@ String determineBranch(CommandTimeoutType cmdType) { } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SingleHistory.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SingleHistory.java new file mode 100644 index 00000000000..94715db60dc --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SingleHistory.java @@ -0,0 +1,72 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.history; + +import java.util.NoSuchElementException; + +/** + * Represents a history sequence for a single item. This is used for older + * repository implementations that read all history at once with no + * sequencing. + */ +class SingleHistory implements HistoryEnumeration { + private final History element; + private boolean didYield; + + SingleHistory(History element) { + if (element == null) { + throw new IllegalArgumentException("element is null"); + } + this.element = element; + } + + /** + * Does nothing. + */ + @Override + public void close() { + } + + @Override + public boolean hasMoreElements() { + return !didYield; + } + + @Override + public History nextElement() { + if (didYield) { + throw new NoSuchElementException(); + } + didYield = true; + return element; + } + + /** + * @return 0 to indicate success + */ + @Override + public int exitValue() { + return 0; + } +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java index 54e38ffd0d7..fc22cae9038 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SubversionRepository.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017-2018, 2020, Chris Fraire . + * Portions Copyright (c) 2017-2020, Chris Fraire . */ package org.opengrok.indexer.history; @@ -71,7 +71,7 @@ public class SubversionRepository extends Repository { private static final String URLattr = "url"; - protected String reposPath; + String reposPath; public SubversionRepository() { type = "Subversion"; @@ -275,13 +275,13 @@ boolean hasHistoryForDirectories() { } @Override - History getHistory(File file) throws HistoryException { - return getHistory(file, null, 0, CommandTimeoutType.INDEXER); + HistoryEnumeration getHistory(File file) throws HistoryException { + return new SingleHistory(getHistory(file, null, 0, CommandTimeoutType.INDEXER)); } @Override - History getHistory(File file, String sinceRevision) throws HistoryException { - return getHistory(file, sinceRevision, 0, CommandTimeoutType.INDEXER); + HistoryEnumeration getHistory(File file, String sinceRevision) throws HistoryException { + return new SingleHistory(getHistory(file, sinceRevision, 0, CommandTimeoutType.INDEXER)); } private History getHistory(File file, String sinceRevision, int numEntries, @@ -386,7 +386,7 @@ String determineParent(CommandTimeoutType cmdType) { } @Override - String determineBranch(CommandTimeoutType cmdType) throws IOException { + String determineBranch(CommandTimeoutType cmdType) { String branch = null; Document document = getInfoDocument(); @@ -403,7 +403,7 @@ String determineBranch(CommandTimeoutType cmdType) throws IOException { } @Override - public String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + public String determineCurrentVersion(CommandTimeoutType cmdType) { String curVersion = null; try { diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/TagEntry.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/TagEntry.java index 1e3669b1b94..e1704382721 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/TagEntry.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/TagEntry.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2012, 2018 Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, 2020, Chris Fraire . + * Portions Copyright (c) 2017, 2019-2020, Chris Fraire . */ package org.opengrok.indexer.history; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java index 948f527fa2d..0dd21adf064 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java @@ -1703,8 +1703,7 @@ private void finishWriting() throws IOException { if (hasPendingCommit) { writer.rollback(); } - LOGGER.log(Level.WARNING, - "An error occurred while finishing writer and completer", e); + LOGGER.log(Level.WARNING, "Error while finishing writer and completer", e); throw e; } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java index 9e8e2ac41bd..00479d1f615 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java @@ -809,7 +809,8 @@ public static String[] parseOptions(String[] argv) throws ParseException { // so that options may be overwritten later. configure.parse(argv); - LOGGER.log(Level.INFO, "Indexer options: {0}", Arrays.toString(argv)); + LOGGER.log(Level.INFO, "Indexer options {0}", Executor.escapeForShell(Arrays.asList(argv), + false, PlatformUtils.isWindows())); if (cfg == null) { cfg = new Configuration(); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileCompleter.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileCompleter.java index aebb3ff70b8..5217d343618 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileCompleter.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileCompleter.java @@ -52,11 +52,11 @@ import org.opengrok.indexer.util.TandemPath; /** - * Represents a tracker of pending file deletions and renamings that can later - * be executed. + * Represents a tracker of pending file deletions, renamings, and linkages that + * can later be executed. *

* {@link PendingFileCompleter} is not generally thread-safe, as only - * {@link #add(org.opengrok.indexer.index.PendingFileRenaming)} is expected + * {@link #add(PendingFileRenaming)} is expected * to be run in parallel; that method is thread-safe -- but only among other * callers of the same method. *

@@ -65,13 +65,13 @@ * additions of {@link PendingSymlinkage}s, {@link PendingFileDeletion}s, and * {@link PendingFileRenaming}s are indicated. *

- * {@link #add(org.opengrok.indexer.index.PendingSymlinkage)} should only + * {@link #add(PendingSymlinkage)} should only * be called in serial from a single thread in an isolated stage. *

- * {@link #add(org.opengrok.indexer.index.PendingFileDeletion)} should only + * {@link #add(PendingFileDeletion)} should only * be called in serial from a single thread in an isolated stage. *

- * {@link #add(org.opengrok.indexer.index.PendingFileRenaming)}, as noted, + * {@link #add(PendingFileRenaming)}, as noted, * can be called in parallel in an isolated stage. */ class PendingFileCompleter { @@ -81,7 +81,7 @@ class PendingFileCompleter { * {@link PendingFileRenaming} actions. *

Value is {@code ".org_opengrok"}. */ - public static final String PENDING_EXTENSION = ".org_opengrok"; + static final String PENDING_EXTENSION = ".org_opengrok"; private static final Logger LOGGER = LoggerFactory.getLogger(PendingFileCompleter.class); @@ -622,8 +622,8 @@ private static class PendingSymlinkageExec { * deleted for cleanliness. */ private static class SkeletonDirs { - boolean ineligible; // a flag used during recursion final Set childDirs = new TreeSet<>(DESC_PATHLEN_COMPARATOR); + boolean ineligible; // a flag used during recursion void reset() { ineligible = false; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileDeletion.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileDeletion.java index bea9e0f22e0..186210cc556 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileDeletion.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileDeletion.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2017, Chris Fraire . + * Copyright (c) 2017, 2019, Chris Fraire . */ package org.opengrok.indexer.index; @@ -26,10 +26,10 @@ /** * Represents the metadata for a pending file deletion. */ -public final class PendingFileDeletion { +final class PendingFileDeletion { private final String absolutePath; - public PendingFileDeletion(String absolutePath) { + PendingFileDeletion(String absolutePath) { this.absolutePath = absolutePath; } @@ -55,6 +55,9 @@ public boolean equals(Object o) { return this.absolutePath.equals(other.absolutePath); } + /** + * Gets the hash code of the absolute path. + */ @Override public int hashCode() { return this.absolutePath.hashCode(); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileRenaming.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileRenaming.java index c849570a222..2625dddfe76 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileRenaming.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingFileRenaming.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2017-2018, Chris Fraire . + * Copyright (c) 2017-2019, Chris Fraire . */ package org.opengrok.indexer.index; @@ -26,11 +26,11 @@ /** * Represents the metadata for a pending file renaming. */ -public final class PendingFileRenaming { +final class PendingFileRenaming { private final String absolutePath; private final String transientPath; - public PendingFileRenaming(String absolutePath, String transientPath) { + PendingFileRenaming(String absolutePath, String transientPath) { this.absolutePath = absolutePath; this.transientPath = transientPath; } @@ -45,7 +45,7 @@ public String getAbsolutePath() { /** * @return the transient, absolute path */ - public String getTransientPath() { + String getTransientPath() { return transientPath; } @@ -64,6 +64,9 @@ public boolean equals(Object o) { return this.absolutePath.equals(other.absolutePath); } + /** + * Gets the hash code of the final, absolute path. + */ @Override public int hashCode() { return this.absolutePath.hashCode(); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingSymlinkage.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingSymlinkage.java index 2ed4b1c1752..efb96bddece 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingSymlinkage.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/PendingSymlinkage.java @@ -18,18 +18,18 @@ */ /* - * Copyright (c) 2017-2018, Chris Fraire . + * Copyright (c) 2017-2019, Chris Fraire . */ package org.opengrok.indexer.index; /** * Represents the metadata for a pending symbolic linkage. */ -public final class PendingSymlinkage { +final class PendingSymlinkage { private final String sourcePath; private final String targetRelPath; - public PendingSymlinkage(String sourcePath, String targetRelPath) { + PendingSymlinkage(String sourcePath, String targetRelPath) { this.sourcePath = sourcePath; this.targetRelPath = targetRelPath; } @@ -37,14 +37,14 @@ public PendingSymlinkage(String sourcePath, String targetRelPath) { /** * @return the source, absolute path */ - public String getSourcePath() { + String getSourcePath() { return sourcePath; } /** * @return the target, relative path */ - public String getTargetRelPath() { + String getTargetRelPath() { return targetRelPath; } @@ -63,6 +63,9 @@ public boolean equals(Object o) { return this.sourcePath.equals(other.sourcePath); } + /** + * Gets the hash code of the source, absolute path. + */ @Override public int hashCode() { return this.sourcePath.hashCode(); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java index 93338216fc4..1badf26a37e 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java @@ -36,6 +36,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Timer; import java.util.TimerTask; import java.util.logging.Level; @@ -59,8 +60,8 @@ public class Executor { private static final Pattern ARG_UNIX_QUOTING = Pattern.compile("[^-:.+=%a-zA-Z0-9_/]"); private static final Pattern ARG_GNU_STYLE_EQ = Pattern.compile("^--[-.a-zA-Z0-9_]+="); - private List cmdList; - private File workingDirectory; + private final List cmdList; + private final File workingDirectory; private byte[] stdout; private byte[] stderr; private int timeout; // in seconds, 0 means no timeout @@ -154,140 +155,222 @@ public int exec(boolean reportExceptions) { */ public int exec(final boolean reportExceptions, StreamHandler handler) { int ret = -1; - ProcessBuilder processBuilder = new ProcessBuilder(cmdList); - final String cmd_str = escapeForShell(processBuilder.command(), false, - PlatformUtils.isWindows()); - final String dir_str; - Timer timer = null; // timer for timing out the process + stdout = null; + stderr = null; + ExecutorProcess ep = null; + byte[] errBytes = null; + try { + Statistics stat = new Statistics(); + ep = startExec(reportExceptions); + handler.processStream(ep.process.getInputStream()); + ret = ep.process.waitFor(); - if (workingDirectory != null) { - processBuilder.directory(workingDirectory); - if (processBuilder.environment().containsKey("PWD")) { - processBuilder.environment().put("PWD", - workingDirectory.getAbsolutePath()); + stat.report(LOGGER, Level.FINE, + String.format("Finished command [%s] in directory %s with exit code %d", + ep.cmd_str, ep.dir_str, ret)); + + // Wait for the stderr read-out thread to finish the processing and + // only after that read the data. + ep.err_thread.join(); + errBytes = ep.err.getBytes(); + stderr = errBytes; + } catch (IOException e) { + if (reportExceptions) { + LOGGER.log(Level.SEVERE, "Failed to read from process: " + + cmdList.get(0), e); + } + if (ep == null) { + return ret; + } + } catch (InterruptedException e) { + if (reportExceptions) { + LOGGER.log(Level.SEVERE, "Waiting for process interrupted: " + + cmdList.get(0), e); + } + } finally { + if (ep != null) { + ret = ep.finish(); } } - File cwd = processBuilder.directory(); - if (cwd == null) { - dir_str = System.getProperty("user.dir"); - } else { - dir_str = cwd.toString(); + if (ret != 0 && reportExceptions) { + logErrBytes(ret, ep.cmd_str, ep.dir_str, errBytes); } - String env_str = ""; - if (LOGGER.isLoggable(Level.FINER)) { - Map env_map = processBuilder.environment(); - env_str = " with environment: " + env_map.toString(); + return ret; + } + + /** + * Starts a command execution, and produces an iterable. + * + * @param reportExceptions Should exceptions be added to the log or not + * @param handler The handler to handle data from standard output + * @return a defined instance to wrap the execution + * @throws IOException if an execution cannot be started + */ + public ObjectCloseableEnumeration startExec(boolean reportExceptions, + ObjectStreamHandler handler) throws IOException { + + if (handler == null) { + throw new IllegalArgumentException("handler is null"); } - LOGGER.log(Level.FINE, - "Executing command [{0}] in directory {1}{2}", - new Object[] {cmd_str, dir_str, env_str}); - Process process = null; + stdout = null; + stderr = null; + final ExecutorProcess ep; try { - Statistics stat = new Statistics(); - process = processBuilder.start(); - final Process proc = process; + ep = startExec(reportExceptions); + } catch (IOException e) { + if (reportExceptions) { + LOGGER.log(Level.SEVERE, "Failed to read from process: " + cmdList.get(0), e); + } + throw e; + } - final InputStream errorStream = process.getErrorStream(); - final SpoolHandler err = new SpoolHandler(); - Thread thread = new Thread(new Runnable() { + final String arg0 = cmdList.get(0); + handler.initializeObjectStream(ep.process.getInputStream()); + Object firstObject = handler.readObject(); - @Override - public void run() { + return new ObjectCloseableEnumeration() { + + Object nextObject = firstObject; + boolean hadProcessError; + int exitValue = -1; + + @Override + public int exitValue() { + return exitValue; + } + + @Override + public void close() throws IOException { + nextObject = null; + + exitValue = ep.finish(); + if (hadProcessError) { + throw new IOException("Failed to execute: " + arg0); + } + } + + @Override + public boolean hasMoreElements() { + return nextObject != null; + } + + @Override + public Object nextElement() { + if (nextObject == null) { + throw new NoSuchElementException(); + } + Object res = nextObject; + nextObject = null; + try { + nextObject = handler.readObject(); + } catch (IOException e) { + hadProcessError = true; + if (reportExceptions) { + LOGGER.log(Level.SEVERE, "Failed to read from process: " + arg0, e); + } + } + + if (!hadProcessError && nextObject == null) { try { - err.processStream(errorStream); - } catch (IOException ex) { + exitValue = ep.process.waitFor(); + ep.err_thread.join(); + } catch (InterruptedException e) { + hadProcessError = true; if (reportExceptions) { - LOGGER.log(Level.SEVERE, - "Error while executing command [{0}] in directory {1}", - new Object[] {cmd_str, dir_str}); - LOGGER.log(Level.SEVERE, "Error during process pipe listening", ex); + LOGGER.log(Level.SEVERE, "Waiting for process interrupted: " + + arg0, e); } } - } - }); - thread.start(); - - int timeout = this.timeout; - /* - * Setup timer so if the process get stuck we can terminate it and - * make progress instead of hanging the whole operation. - */ - if (timeout != 0) { - // invoking the constructor starts the background thread - timer = new Timer(); - timer.schedule(new TimerTask() { - @Override public void run() { - LOGGER.log(Level.WARNING, - String.format("Terminating process of command [%s] in directory %s " + - "due to timeout %d seconds", cmd_str, dir_str, timeout / 1000)); - proc.destroy(); + + LOGGER.log(Level.FINE, "Finished command {0} in directory {1}", + new Object[] {ep.cmd_str, ep.dir_str}); + + final byte[] errBytes = ep.err.getBytes(); + stderr = errBytes; + if (exitValue != 0 && reportExceptions) { + // Limit to avoid flooding the logs. + logErrBytes(exitValue, ep.cmd_str, ep.dir_str, errBytes); } - }, timeout); + } + return res; } + }; + } - handler.processStream(process.getInputStream()); - - ret = process.waitFor(); + /** + * Executes the command for its further processing as an active program. + * + * @param reportExceptions Should exceptions be added to the log or not + * @return a defined, started instance + */ + private ExecutorProcess startExec(final boolean reportExceptions) throws IOException { - stat.report(LOGGER, Level.FINE, String.format("Finished command [%s] in directory %s with exit code %d", - cmd_str, dir_str, ret)); - LOGGER.log(Level.FINE, - "Finished command [{0}] in directory {1} with exit code {2}", - new Object[] {cmd_str, dir_str, ret}); + final ExecutorProcess ep = new ExecutorProcess(); + ep.process_builder = new ProcessBuilder(cmdList); + ep.cmd_str = escapeForShell(ep.process_builder.command(), false, + PlatformUtils.isWindows()); - // Wait for the stderr read-out thread to finish the processing and - // only after that read the data. - thread.join(); - stderr = err.getBytes(); - } catch (IOException e) { - if (reportExceptions) { - LOGGER.log(Level.SEVERE, - "Failed to read from process: " + cmdList.get(0), e); - } - } catch (InterruptedException e) { - if (reportExceptions) { - LOGGER.log(Level.SEVERE, - "Waiting for process interrupted: " + cmdList.get(0), e); - } - } finally { - // Stop timer thread if the instance exists. - if (timer != null) { - timer.cancel(); + if (workingDirectory != null) { + ep.process_builder.directory(workingDirectory); + if (ep.process_builder.environment().containsKey("PWD")) { + ep.process_builder.environment().put("PWD", workingDirectory.getAbsolutePath()); } + } + + File cwd = ep.process_builder.directory(); + if (cwd == null) { + ep.dir_str = System.getProperty("user.dir"); + } else { + ep.dir_str = cwd.toString(); + } + + String env_str = ""; + if (LOGGER.isLoggable(Level.FINER)) { + Map env_map = ep.process_builder.environment(); + env_str = " with environment: " + env_map.toString(); + } + LOGGER.log(Level.FINE, "Executing command [{0}] in directory {1}{2}", + new Object[] {ep.cmd_str, ep.dir_str, env_str}); + + final Process proc = ep.process_builder.start(); + ep.process = proc; + + final InputStream errorStream = proc.getErrorStream(); + ep.err_thread = new Thread(() -> { try { - if (process != null) { - IOUtils.close(process.getOutputStream()); - IOUtils.close(process.getInputStream()); - IOUtils.close(process.getErrorStream()); - ret = process.exitValue(); + ep.err.processStream(errorStream); + } catch (IOException ex) { + if (reportExceptions) { + LOGGER.log(Level.SEVERE, + "Error while executing command [{0}] in directory {1}", + new Object[] {ep.cmd_str, ep.dir_str}); + LOGGER.log(Level.SEVERE, "Error during process pipe listening", ex); } - } catch (IllegalThreadStateException e) { - process.destroy(); } - } + }); + ep.err_thread.start(); - if (ret != 0 && reportExceptions) { - int MAX_MSG_SZ = 512; /* limit to avoid flooding the logs */ - StringBuilder msg = new StringBuilder("Non-zero exit status ") - .append(ret).append(" from command [") - .append(cmd_str) - .append("] in directory ") - .append(dir_str); - if (stderr != null && stderr.length > 0) { - msg.append(": "); - if (stderr.length > MAX_MSG_SZ) { - msg.append(new String(stderr, 0, MAX_MSG_SZ)).append("..."); - } else { - msg.append(new String(stderr)); - } - } - LOGGER.log(Level.WARNING, msg.toString()); + final int timeout = this.timeout; + /* + * Setup timer so if the process get stuck we can terminate it and + * make progress instead of hanging the whole operation. + */ + if (timeout != 0) { + // invoking the constructor starts the background thread + ep.timer = new Timer(); + ep.timer.schedule(new TimerTask() { + @Override public void run() { + LOGGER.log(Level.WARNING, + String.format("Terminating process of command [%s] in directory %s " + + "due to timeout %d seconds", ep.cmd_str, ep.dir_str, timeout / 1000)); + proc.destroy(); + } + }, timeout); } - - return ret; + return ep; } /** @@ -393,7 +476,24 @@ public void processStream(InputStream input) throws IOException { } } } - + + private static void logErrBytes(int exitValue, String cmdStr, String dirStr, byte[] errBytes) { + final int MAX_MSG_SZ = 256; + + StringBuilder msg = new StringBuilder().append("Non-zero exit status ").append(exitValue). + append(" from command ").append(cmdStr).append(" in directory ").append(dirStr); + + if (errBytes != null && errBytes.length > 0) { + msg.append(": "); + if (errBytes.length > MAX_MSG_SZ) { + msg.append(new String(errBytes, 0, MAX_MSG_SZ)).append("..."); + } else { + msg.append(new String(errBytes)); + } + } + LOGGER.log(Level.WARNING, msg.toString()); + } + public static void registerErrorHandler() { UncaughtExceptionHandler dueh = Thread.getDefaultUncaughtExceptionHandler(); @@ -484,4 +584,32 @@ private static String escapeForPowerShell(String value) { replace("\u0011", "`v"). replace("\t", "`t"); } + + private static class ExecutorProcess { + final SpoolHandler err = new SpoolHandler(); + String cmd_str; + String dir_str; + ProcessBuilder process_builder; + Process process; + Timer timer; + Thread err_thread; + + int finish() { + if (timer != null) { + timer.cancel(); + timer = null; + } + if (process != null) { + try { + IOUtils.close(process.getOutputStream()); + IOUtils.close(process.getInputStream()); + IOUtils.close(process.getErrorStream()); + return process.exitValue(); + } catch (IllegalThreadStateException e) { + process.destroy(); + } + } + return -1; + } + } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ObjectCloseableEnumeration.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ObjectCloseableEnumeration.java new file mode 100644 index 00000000000..9c0f97428c0 --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ObjectCloseableEnumeration.java @@ -0,0 +1,44 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.util; + +import java.io.Closeable; +import java.util.Enumeration; + +/** + * Represents an API for a sequence of {@link Object} instances where + * the sequence is {@link Closeable} to release resources. + */ +public interface ObjectCloseableEnumeration extends Enumeration, Closeable { + + /** + * Returns the exit value for the subprocess. + * + * @return the exit value of the subprocess represented by this instance. + * By convention, the value {@code 0} indicates normal termination. + * @throws IllegalThreadStateException if the subprocess represented by + * this instance has not yet terminated + */ + int exitValue(); +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ObjectStreamHandler.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ObjectStreamHandler.java new file mode 100644 index 00000000000..4f8dda3fb3e --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ObjectStreamHandler.java @@ -0,0 +1,46 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.util; + +import java.io.IOException; +import java.io.InputStream; + +/** + * Represents an API for parsing objects from a stream. + */ +public interface ObjectStreamHandler { + + /** + * Initializes the handler to read from the specified input. + */ + void initializeObjectStream(InputStream in); + + /** + * Reads an object from the initialized input unless the stream has been + * exhausted. + * @return a defined instance or {@code null} if the stream has been + * exhausted + */ + Object readObject() throws IOException; +} diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BazaarHistoryParserTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BazaarHistoryParserTest.java index ce57fa8dc45..68648a2ebeb 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BazaarHistoryParserTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BazaarHistoryParserTest.java @@ -24,20 +24,18 @@ package org.opengrok.indexer.history; -import java.nio.file.Paths; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + import java.util.Arrays; import java.util.HashSet; import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import org.opengrok.indexer.configuration.RuntimeEnvironment; import org.opengrok.indexer.util.PlatformUtils; -import org.opengrok.indexer.web.Util; - -import static org.junit.Assert.*; /** * @@ -47,17 +45,6 @@ public class BazaarHistoryParserTest { private BazaarHistoryParser instance; - public BazaarHistoryParserTest() { - } - - @BeforeClass - public static void setUpClass() throws Exception { - } - - @AfterClass - public static void tearDownClass() throws Exception { - } - @Before public void setUp() { if (RuntimeEnvironment.getInstance().getSourceRootPath() == null) { @@ -206,5 +193,4 @@ public void parseLogDirectory() throws Exception { assertEquals(author1, e1.getAuthor()); assertEquals(new HashSet<>(Arrays.asList(files)), e1.getFiles()); } - } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BitKeeperRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BitKeeperRepositoryTest.java index abc739ccefc..ead7eaf9b2a 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BitKeeperRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/BitKeeperRepositoryTest.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2018 Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -95,7 +96,7 @@ public void tearDown() { bkFiles = null; } - static private void validateHistory(History history) throws Exception { + static private void validateHistory(History history) { final List entries = history.getHistoryEntries(); final List renames = history.getRenamedFiles(); @@ -113,12 +114,12 @@ static private void validateHistory(History history) throws Exception { // Validate that the renamed files list corresponds // to all the file names we know of for this file. - final TreeSet fileNames = new TreeSet(); + final TreeSet fileNames = new TreeSet<>(); for (final HistoryEntry entry : entries) { fileNames.addAll(entry.getFiles()); } final String currentName = entries.get(0).getFiles().first(); - final TreeSet pastNames = new TreeSet(renames); + final TreeSet pastNames = new TreeSet<>(renames); pastNames.add(currentName); assertEquals("File history has incorrect rename list.", fileNames, pastNames); } @@ -129,8 +130,8 @@ public void testGetHistory() throws Exception { for (final String bkFile : bkFiles) { final File file = new File(bkRepo.getDirectoryName(), bkFile); - final History fullHistory = bkRepo.getHistory(file); - final History partHistory = bkRepo.getHistory(file, "1.2"); + final History fullHistory = HistoryUtil.union(bkRepo.getHistory(file)); + final History partHistory = HistoryUtil.union(bkRepo.getHistory(file, "1.2")); // I made sure that each file had a 1.2 validateHistory(fullHistory); @@ -144,14 +145,14 @@ public void testGetHistory() throws Exception { } @Test - public void testGetHistoryInvalid() throws Exception { + public void testGetHistoryInvalid() { assertNotNull("Couldn't read bitkeeper test repository.", bkRepo); final File file = new File(bkRepo.getDirectoryName(), "fakename.cpp"); boolean caughtFull = false; try { - final History fullHistory = bkRepo.getHistory(file); + HistoryUtil.union(bkRepo.getHistory(file)); } catch (final HistoryException e) { caughtFull = true; } @@ -159,20 +160,20 @@ public void testGetHistoryInvalid() throws Exception { boolean caughtPart = false; try { - final History partHistory = bkRepo.getHistory(file, "1.2"); + HistoryUtil.union(bkRepo.getHistory(file, "1.2")); } catch (final HistoryException e) { caughtPart = true; } assertTrue("No exception thrown by getHistory with fake file", caughtPart); } - static private String readStream(InputStream stream) throws IOException { + static private String readStream(InputStream stream) { final Scanner scanner = new Scanner(stream).useDelimiter("\\A"); return scanner.hasNext() ? scanner.next() : ""; } @Test - public void testGetHistoryGet() throws Exception { + public void testGetHistoryGet() { assertNotNull("Couldn't read bitkeeper test repository.", bkRepo); for (final String bkFile : bkFiles) { @@ -186,7 +187,7 @@ public void testGetHistoryGet() throws Exception { } @Test - public void testGetHistoryGetInvalid() throws Exception { + public void testGetHistoryGetInvalid() { assertNotNull("Couldn't read bitkeeper test repository.", bkRepo); assertNull("Something returned by getHistoryGet with fake file", @@ -207,19 +208,19 @@ public void testAnnotation() throws Exception { assertEquals("Wrong file returned by annotate.", currentVersion.getFilename(), file.getName()); assertEquals("Wrong file returned by annotate.", firstVersion.getFilename(), file.getName()); assertTrue("Incorrect revisions returned by annotate.", currentVersion.getRevisions().size() > 1); - assertTrue("Incorrect revisions returned by annotate.", firstVersion.getRevisions().size() == 1); + assertEquals("Incorrect revisions returned by annotate.", 1, firstVersion.getRevisions().size()); } } @Test - public void testAnnotationInvalid() throws Exception { + public void testAnnotationInvalid() { assertNotNull("Couldn't read bitkeeper test repository.", bkRepo); final File file = new File(bkRepo.getDirectoryName(), "fakename.cpp"); boolean caughtCurrent = false; try { - final Annotation currentVersion = bkRepo.annotate(file, "+"); + bkRepo.annotate(file, "+"); } catch (final IOException e) { caughtCurrent = true; } @@ -227,7 +228,7 @@ public void testAnnotationInvalid() throws Exception { boolean caughtFirst = false; try { - final Annotation firstVersion = bkRepo.annotate(file, "1.1"); + bkRepo.annotate(file, "1.1"); } catch (final IOException e) { caughtFirst = true; } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/CVSRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/CVSRepositoryTest.java index e518403f858..1ca9fc895b2 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/CVSRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/CVSRepositoryTest.java @@ -28,14 +28,14 @@ import java.io.FileOutputStream; import java.io.FileWriter; import java.io.IOException; -import java.io.Reader; -import java.io.StringReader; import java.nio.channels.FileChannel; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.junit.After; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.junit.Before; @@ -106,9 +106,7 @@ public static void runCvsCommand(File reposRoot, String ... args) { List cmdargs = new ArrayList<>(); CVSRepository repo = new CVSRepository(); cmdargs.add(repo.getRepoCommand()); - for (String arg: args) { - cmdargs.add(arg); - } + Collections.addAll(cmdargs, args); Executor exec = new Executor(cmdargs, reposRoot); int exitCode = exec.exec(); if (exitCode != 0) { @@ -122,7 +120,6 @@ public static void runCvsCommand(File reposRoot, String ... args) { /** * Get the CVS repository, test that getBranch() returns null if there is * no branch. - * @throws Exception */ @Test public void testGetBranchNoBranch() throws Exception { @@ -130,7 +127,7 @@ public void testGetBranchNoBranch() throws Exception { File root = new File(repository.getSourceRoot(), "cvs_test/cvsrepo"); CVSRepository cvsrepo = (CVSRepository) RepositoryFactory.getRepository(root); - assertEquals(null, cvsrepo.getBranch()); + assertNull("getBranch()", cvsrepo.getBranch()); } /** @@ -139,7 +136,6 @@ public void testGetBranchNoBranch() throws Exception { * with branch revision numbers. * Last, check that history entries of the file follow through before the * branch was created. - * @throws Exception */ @Test public void testNewBranch() throws Exception { @@ -172,7 +168,7 @@ public void testNewBranch() throws Exception { Annotation annotation = cvsrepo.annotate(mainC, null); assertEquals("1.2.2.1", annotation.getRevision(1)); - History mainCHistory = cvsrepo.getHistory(mainC); + History mainCHistory = HistoryUtil.union(cvsrepo.getHistory(mainC)); assertEquals(3, mainCHistory.getHistoryEntries().size()); assertEquals("1.2.2.1", mainCHistory.getHistoryEntries().get(0).getRevision()); assertEquals("1.2", mainCHistory.getHistoryEntries().get(1).getRevision()); @@ -199,7 +195,6 @@ public void testFileHasHistory() { /** * Test of parseAnnotation method, of class CVSRepository. - * @throws java.lang.Exception */ @Test public void testParseAnnotation() throws Exception { @@ -222,7 +217,7 @@ public void testParseAnnotation() throws Exception { assertNotNull(result); assertEquals(3, result.size()); for (int i = 1; i <= 3; i++) { - assertEquals(true, result.isEnabled(i)); + assertTrue("isEnabled()", result.isEnabled(i)); } assertEquals(revId1, result.getRevision(1)); assertEquals(revId2, result.getRevision(2)); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheOctopusTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheOctopusTest.java index 656ec23662e..5ce2d0648da 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheOctopusTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheOctopusTest.java @@ -54,7 +54,6 @@ public class FileHistoryCacheOctopusTest { public void setUp() throws Exception { repositories = new TestRepository(); repositories.create(getClass().getResourceAsStream("/history/git-octopus.zip")); - cache = new FileHistoryCache(); cache.initialize(); } @@ -72,9 +71,10 @@ public void tearDown() { public void testStoreAndGet() throws Exception { File reposRoot = new File(repositories.getSourceRoot(), "git-octopus"); Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); cache.store(historyToStore, repo); + historyToStore.close(); assertEquals("latest git-octopus commit", "206f862b", cache.getLatestCachedRevision(repo)); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheTest.java index 32edf3cf4ad..0fd35537ad6 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheTest.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2018, 2020, Chris Fraire . + * Portions Copyright (c) 2018-2020, Chris Fraire . * Portions Copyright (c) 2020, Ric Harris . */ package org.opengrok.indexer.history; @@ -41,6 +41,7 @@ import org.apache.commons.lang3.time.DateUtils; import org.junit.After; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.opengrok.indexer.condition.ConditionalRun; @@ -61,8 +62,8 @@ public class FileHistoryCacheTest { private static final String SVN_DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; + private static RuntimeEnvironment env; - private final RuntimeEnvironment env = RuntimeEnvironment.getInstance(); private TestRepository repositories; private FileHistoryCache cache; @@ -74,6 +75,11 @@ public class FileHistoryCacheTest { @Rule public ConditionalRunRule rule = new ConditionalRunRule(); + @BeforeClass + public static void setUpClass() { + env = RuntimeEnvironment.getInstance(); + } + /** * Set up the test environment with repositories and a cache instance. */ @@ -157,16 +163,17 @@ private void assertSameEntry(HistoryEntry expected, HistoryEntry actual, boolean public void testStoreAndGetNotRenamed() throws Exception { File reposRoot = new File(repositories.getSourceRoot(), "mercurial"); Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); cache.store(historyToStore, repo); + historyToStore.close(); // This makes sure that the file which contains the latest revision // has indeed been created. assertEquals("9:8b340409b3a8", cache.getLatestCachedRevision(repo)); // test reindex - History historyNull = new History(); + HistoryEnumeration historyNull = new SingleHistory(new History()); cache.store(historyNull, repo); assertEquals("9:8b340409b3a8", cache.getLatestCachedRevision(repo)); @@ -186,10 +193,11 @@ public void testStoreAndGetIncrementalTags() throws Exception { File reposRoot = new File(repositories.getSourceRoot(), "mercurial"); Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); // Store the history. cache.store(historyToStore, repo); + historyToStore.close(); // Add bunch of changesets with file based changes and tags. MercurialRepositoryTest.runHgCommand(reposRoot, "import", @@ -206,7 +214,7 @@ public void testStoreAndGetIncrementalTags() throws Exception { // Verify tags in fileHistory for main.c which is the most interesting // file from the repository from the perspective of tags. File main = new File(reposRoot, "main.c"); - assertTrue(main.exists()); + assertTrue("main.c should exist", main.exists()); History retrievedHistoryMainC = cache.get(main, repo, true); List entries = retrievedHistoryMainC.getHistoryEntries(); assertEquals("Unexpected number of entries for main.c", @@ -223,7 +231,7 @@ public void testStoreAndGetIncrementalTags() throws Exception { HistoryEntry e2 = entries.get(2); assertEquals("Unexpected revision for entry 2", "1:f24a5fd7a85d", e2.getRevision()); - assertEquals("Invalid tag list for revision 1", null, e2.getTags()); + assertNull("Invalid tag list for revision 1", e2.getTags()); // Reindex from scratch. File dir = new File(cache.getRepositoryHistDataDirname(repo)); @@ -232,8 +240,8 @@ public void testStoreAndGetIncrementalTags() throws Exception { // We cannot call cache.get() here since it would read the history anew. // Instead check that the data directory does not exist anymore. assertFalse(dir.exists()); - History freshHistory = repo.getHistory(reposRoot); - cache.store(freshHistory, repo); + History freshHistory = HistoryUtil.union(repo.getHistory(reposRoot)); + cache.store(new SingleHistory(freshHistory), repo); History updatedHistoryFromScratch = cache.get(reposRoot, repo, true); assertEquals("Unexpected number of history entries", freshHistory.getHistoryEntries().size(), @@ -262,12 +270,12 @@ public void testStoreAndGet() throws Exception { env.setHandleHistoryOfRenamedFiles(true); Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + History historyToStore = HistoryUtil.union(repo.getHistory(reposRoot)); - cache.store(historyToStore, repo); + cache.store(new SingleHistory(historyToStore), repo); // test reindex - History historyNull = new History(); + HistoryEnumeration historyNull = new SingleHistory(new History()); cache.store(historyNull, repo); // test get history for single file @@ -309,10 +317,10 @@ public void testStoreAndGet() throws Exception { // test get history for directory // Need to refresh history to store since the file lists were stripped // from it in the call to cache.store() above. - historyToStore = repo.getHistory(reposRoot); + History fullHistory = HistoryUtil.union(repo.getHistory(reposRoot)); History dirHistory = cache.get(reposRoot, repo, true); assertSameEntries( - historyToStore.getHistoryEntries(), + fullHistory.getHistoryEntries(), dirHistory.getHistoryEntries(), true); // test incremental update @@ -346,7 +354,7 @@ public void testStoreAndGet() throws Exception { // lists of files so we need to set isdir to true. assertSameEntry(newEntry2, updatedEntries.removeFirst(), true); assertSameEntry(newEntry1, updatedEntries.removeFirst(), true); - assertSameEntries(historyToStore.getHistoryEntries(), updatedEntries, true); + assertSameEntries(fullHistory.getHistoryEntries(), updatedEntries, true); // test clearing of cache File dir = new File(cache.getRepositoryHistDataDirname(repo)); @@ -356,9 +364,9 @@ public void testStoreAndGet() throws Exception { // Instead check that the data directory does not exist anymore. assertFalse(dir.exists()); - cache.store(historyToStore, repo); + cache.store(new SingleHistory(historyToStore), repo); // check that the data directory is non-empty - assertEquals(true, dir.list().length > 0); + assertTrue("dir.list().length is positive", dir.list().length > 0); updatedHistory = cache.get(reposRoot, repo, true); assertSameEntries(updatedHistory.getHistoryEntries(), cache.get(reposRoot, repo, true).getHistoryEntries(), true); @@ -393,8 +401,9 @@ public void testRenameFileThenDoIncrementalReindex() throws Exception { // It is necessary to call getRepository() only after tags were enabled // to produce list of tags. Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); cache.store(historyToStore, repo); + historyToStore.close(); // Import changesets which rename one of the files in the repository. MercurialRepositoryTest.runHgCommand(reposRoot, "import", @@ -532,8 +541,9 @@ public void testRenamedFilePlusChangesBranched() throws Exception { // It is necessary to call getRepository() only after tags were enabled // to produce list of tags. Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); cache.store(historyToStore, repo); + historyToStore.close(); /* quick sanity check */ updatedHistory = cache.get(reposRoot, repo, true); @@ -641,7 +651,7 @@ public void testMultipleRenamedFiles() throws Exception { // Generate history index. Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); cache.store(historyToStore, repo); /* quick sanity check */ @@ -700,7 +710,7 @@ public void testRenamedFile() throws Exception { // Generate history index. Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); cache.store(historyToStore, repo); /* quick sanity check */ @@ -815,12 +825,11 @@ public void testStoreAndTryToGetIgnored() throws Exception { File reposRoot = new File(repositories.getSourceRoot(), "mercurial"); Repository repo = RepositoryFactory.getRepository(reposRoot); - History historyToStore = repo.getHistory(reposRoot); - + HistoryEnumeration historyToStore = repo.getHistory(reposRoot); cache.store(historyToStore, repo); // test reindex history - History historyNull = new History(); + HistoryEnumeration historyNull = new SingleHistory(new History()); cache.store(historyNull, repo); // test get history for single file diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitHistoryParserTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitHistoryParserTest.java index 62747a8e769..ca1547d08e8 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitHistoryParserTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitHistoryParserTest.java @@ -42,7 +42,6 @@ import org.junit.Test; import org.opengrok.indexer.util.TestRepository; import org.opengrok.indexer.web.Util; - /** * @author austvik */ @@ -107,12 +106,19 @@ public void shouldHandleMergeCommits() throws Exception { History gitHistory = instance.getHistory(); assertNotNull("should parse git-log-merged-file.txt", gitHistory); List entries = gitHistory.getHistoryEntries(); - assertEquals("git-log-merged-file.txt entries", 1, entries.size()); + assertEquals("git-log-merged-file.txt entries", 2, entries.size()); final String MERGE_REV = "4c3d5e8e"; + HistoryEntry e1 = entries.get(1); + assertEquals("entries[1] revision", MERGE_REV, e1.getRevision()); + HistoryEntry e0 = entries.get(0); assertEquals("entries[0] revision", MERGE_REV, e0.getRevision()); + SortedSet f1 = e1.getFiles(); + assertEquals("e[1] files size", 1, f1.size()); + assertEquals("e[1] files[0]", "/contrib/serf/STATUS", f1.first()); + SortedSet f0 = e0.getFiles(); assertEquals("e[0] files size", 1, f0.size()); assertEquals("e[0] files[0]", "/contrib/serf/STATUS", Util.fixPathIfWindows(f0.first())); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryOctopusTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryOctopusTest.java index b6909072b19..fd4328ab2d6 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryOctopusTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryOctopusTest.java @@ -167,16 +167,17 @@ public void testOctopusHistory() throws Exception { File root = new File(repository.getSourceRoot(), "git-octopus"); GitRepository gitRepo = (GitRepository) RepositoryFactory.getRepository(root); - History history = gitRepo.getHistory(root); + History history = HistoryUtil.union(gitRepo.getHistory(root)); assertNotNull("git-octopus getHistory()", history); List entries = history.getHistoryEntries(); assertNotNull("git-octopus getHistoryEntries()", entries); /* - * git-octopus has four-way merge, but GitHistoryParser condenses. + * git-octopus has four-way merge, so there are three more history + * entries with `git log -m` */ - assertEquals("git-octopus log entries", 4, entries.size()); + assertEquals("git-octopus log entries", 7, entries.size()); SortedSet allFiles = new TreeSet<>(); for (HistoryEntry entry : entries) { diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryTest.java index 774e547f806..92d2adc298f 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/GitRepositoryTest.java @@ -202,7 +202,6 @@ public void testRenamedFiles() throws Exception { = (GitRepository) RepositoryFactory.getRepository(root); gitrepo.setHandleRenamedFiles(true); - int i = 0; for (String[] test : tests) { String file = Paths.get(root.getCanonicalPath(), test[0]).toString(); String changeset = test[1]; @@ -213,7 +212,6 @@ public void testRenamedFiles() throws Exception { } catch (IOException ex) { Assert.fail(String.format("Looking for original name of %s in %s shouldn't fail", file, changeset)); } - i++; } } @@ -440,7 +438,7 @@ public void testRenamedHistory() throws Exception { GitRepository gitrepo = (GitRepository) RepositoryFactory.getRepository(root); - History history = gitrepo.getHistory(root); + History history = HistoryUtil.union(gitrepo.getHistory(root)); Assert.assertNotNull(history); Assert.assertNotNull(history.getHistoryEntries()); Assert.assertEquals(8, history.getHistoryEntries().size()); @@ -448,11 +446,11 @@ public void testRenamedHistory() throws Exception { Assert.assertNotNull(history.getRenamedFiles()); Assert.assertEquals(3, history.getRenamedFiles().size()); - Assert.assertTrue(history.isRenamed("moved/renamed2.c")); - Assert.assertTrue(history.isRenamed("moved2/renamed2.c")); - Assert.assertTrue(history.isRenamed("moved/renamed.c")); - Assert.assertFalse(history.isRenamed("non-existent.c")); - Assert.assertFalse(history.isRenamed("renamed.c")); + Assert.assertTrue(history.isRenamedBySlowLookup("moved/renamed2.c")); + Assert.assertTrue(history.isRenamedBySlowLookup("moved2/renamed2.c")); + Assert.assertTrue(history.isRenamedBySlowLookup("moved/renamed.c")); + Assert.assertFalse(history.isRenamedBySlowLookup("non-existent.c")); + Assert.assertFalse(history.isRenamedBySlowLookup("renamed.c")); Assert.assertEquals("84599b3c", history.getHistoryEntries().get(0).getRevision()); Assert.assertEquals("67dfbe26", history.getHistoryEntries().get(1).getRevision()); @@ -471,7 +469,7 @@ public void testRenamedSingleHistory() throws Exception { GitRepository gitrepo = (GitRepository) RepositoryFactory.getRepository(root); - History history = gitrepo.getHistory(new File(root.getAbsolutePath(), "moved2/renamed2.c")); + History history = HistoryUtil.union(gitrepo.getHistory(new File(root.getAbsolutePath(), "moved2/renamed2.c"))); Assert.assertNotNull(history); Assert.assertNotNull(history.getHistoryEntries()); Assert.assertEquals(5, history.getHistoryEntries().size()); @@ -479,11 +477,11 @@ public void testRenamedSingleHistory() throws Exception { Assert.assertNotNull(history.getRenamedFiles()); Assert.assertEquals(3, history.getRenamedFiles().size()); - Assert.assertTrue(history.isRenamed("moved/renamed2.c")); - Assert.assertTrue(history.isRenamed("moved2/renamed2.c")); - Assert.assertTrue(history.isRenamed("moved/renamed.c")); - Assert.assertFalse(history.isRenamed("non-existent.c")); - Assert.assertFalse(history.isRenamed("renamed.c")); + Assert.assertTrue(history.isRenamedBySlowLookup("moved/renamed2.c")); + Assert.assertTrue(history.isRenamedBySlowLookup("moved2/renamed2.c")); + Assert.assertTrue(history.isRenamedBySlowLookup("moved/renamed.c")); + Assert.assertFalse(history.isRenamedBySlowLookup("non-existent.c")); + Assert.assertFalse(history.isRenamedBySlowLookup("renamed.c")); Assert.assertEquals("84599b3c", history.getHistoryEntries().get(0).getRevision()); Assert.assertEquals("67dfbe26", history.getHistoryEntries().get(1).getRevision()); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryEntryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryEntryTest.java index dae82bbb583..885cbacf14d 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryEntryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryEntryTest.java @@ -19,21 +19,24 @@ /* * Copyright (c) 2008, 2018 Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; import java.util.Collections; import java.util.Date; +import java.util.SortedSet; import java.util.TreeSet; -import org.junit.After; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; -import static org.junit.Assert.*; /** - * * @author austvik */ public class HistoryEntryTest { @@ -44,27 +47,12 @@ public class HistoryEntryTest { private String historyAuthor = "test author"; private String historyMessage = "history entry message"; - public HistoryEntryTest() { - } - - @BeforeClass - public static void setUpClass() throws Exception { - } - - @AfterClass - public static void tearDownClass() throws Exception { - } - @Before public void setUp() { instance = new HistoryEntry(historyRevision, historyDate, historyAuthor, null, historyMessage, true); } - @After - public void tearDown() { - } - /** * Test of getLine method, of class HistoryEntry. */ @@ -256,4 +244,44 @@ public void strip() { assertEquals(null, instance.getTags()); } + @Test + public void shouldMergeSimplest() { + HistoryEntry a = new HistoryEntry(); + HistoryEntry b = new HistoryEntry(); + + a.merge(b); + assertEquals("a getFiles() size",0, a.getFiles().size()); + assertNull("a getTags()", a.getTags()); + } + + @Test + public void shouldMergeFiles() { + HistoryEntry a = new HistoryEntry(); + a.setFiles(new TreeSet<>(Arrays.asList("a,b".split(",")))); + + HistoryEntry b = new HistoryEntry(); + b.setFiles(new TreeSet<>(Arrays.asList("b,c".split(",")))); + + a.merge(b); + SortedSet aFiles = a.getFiles(); + assertEquals("a getFiles() size",3, aFiles.size()); + assertTrue("contains 'a'", aFiles.contains("a")); + assertTrue("contains 'b'", aFiles.contains("b")); + assertTrue("contains 'c'", aFiles.contains("c")); + + assertNull("a getTags()", a.getTags()); + } + + @Test + public void shouldMergeTags() { + HistoryEntry a = new HistoryEntry(); + a.setTags("a, b"); + + HistoryEntry b = new HistoryEntry(); + b.setTags("c, b"); + + a.merge(b); + assertEquals("a getFiles() size",0, a.getFiles().size()); + assertEquals("a getTags()", "a, b, c", a.getTags()); + } } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryTest.java new file mode 100644 index 00000000000..484850d84ab --- /dev/null +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryTest.java @@ -0,0 +1,69 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2019, Chris Fraire . + */ + +package org.opengrok.indexer.history; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import java.io.ByteArrayOutputStream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; + +/** + * Represents a container for tests of {@link History}. + */ +public class HistoryTest { + + @Test + public void testEncode() { + final History hist = new History( + new ArrayList<>(Arrays.asList( + new HistoryEntry( + "2", + new Date(1554648411000L), + "fred", + "b", + "second commit", + true), + new HistoryEntry( + "1", + new Date(1554648401000L), + "barney", + "a, b", + "first commit", + true))), + new ArrayList<>(Arrays.asList("a/b", "b/c"))); + + ByteArrayOutputStream bytesOut = new ByteArrayOutputStream(); + hist.encodeObject(bytesOut); + String xml = new String(bytesOut.toByteArray()); + + assertNotNull("String from encodeObject()", xml); + assertTrue("has Date #2", xml.contains("1554648411000")); + assertTrue("has Date #1", xml.contains("1554648401000")); + } +} diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index 014047224e7..c921b125889 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, Chris Fraire . + * Portions Copyright (c) 2017, 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -104,7 +104,7 @@ public void testGetHistory() throws Exception { File root = new File(repository.getSourceRoot(), "mercurial"); MercurialRepository mr = (MercurialRepository) RepositoryFactory.getRepository(root); - History hist = mr.getHistory(root); + History hist = HistoryUtil.union(mr.getHistory(root)); List entries = hist.getHistoryEntries(); assertEquals(REVISIONS.length, entries.size()); for (int i = 0; i < entries.size(); i++) { @@ -128,7 +128,7 @@ public void testGetHistorySubdir() throws Exception { MercurialRepository mr = (MercurialRepository) RepositoryFactory.getRepository(root); - History hist = mr.getHistory(new File(root, "subdir")); + History hist = HistoryUtil.union(mr.getHistory(new File(root, "subdir"))); List entries = hist.getHistoryEntries(); assertEquals(1, entries.size()); } @@ -136,8 +136,6 @@ public void testGetHistorySubdir() throws Exception { /** * Test that subset of changesets can be extracted based on penultimate * revision number. This works for directories only. - * - * @throws Exception */ @Test public void testGetHistoryPartial() throws Exception { @@ -146,7 +144,7 @@ public void testGetHistoryPartial() throws Exception { MercurialRepository mr = (MercurialRepository) RepositoryFactory.getRepository(root); // Get all but the oldest revision. - History hist = mr.getHistory(root, REVISIONS[REVISIONS.length - 1]); + History hist = HistoryUtil.union(mr.getHistory(root, REVISIONS[REVISIONS.length - 1])); List entries = hist.getHistoryEntries(); assertEquals(REVISIONS.length - 1, entries.size()); for (int i = 0; i < entries.size(); i++) { @@ -185,8 +183,6 @@ static public void runHgCommand(File reposRoot, String ... args) { /** * Test that history of branched repository contains changesets of the * default branch as well. - * - * @throws Exception */ @Test public void testGetHistoryBranch() throws Exception { @@ -205,13 +201,13 @@ public void testGetHistoryBranch() throws Exception { = (MercurialRepository) RepositoryFactory.getRepository(root); // Get all revisions. - History hist = mr.getHistory(root); + History hist = HistoryUtil.union(mr.getHistory(root)); List entries = hist.getHistoryEntries(); List both = new ArrayList<>(REVISIONS.length + REVISIONS_extra_branch.length); Collections.addAll(both, REVISIONS_extra_branch); Collections.addAll(both, REVISIONS); - String revs[] = both.toArray(new String[both.size()]); + String[] revs = both.toArray(new String[0]); assertEquals(revs.length, entries.size()); // Ideally we should check that the last revision is branched but // there is currently no provision for that in HistoryEntry object. @@ -225,7 +221,7 @@ public void testGetHistoryBranch() throws Exception { } // Get revisions starting with given changeset before the repo was branched. - hist = mr.getHistory(root, "8:6a8c423f5624"); + hist = HistoryUtil.union(mr.getHistory(root, "8:6a8c423f5624")); entries = hist.getHistoryEntries(); assertEquals(2, entries.size()); assertEquals(REVISIONS_extra_branch[0], entries.get(0).getRevision()); @@ -234,8 +230,6 @@ public void testGetHistoryBranch() throws Exception { /** * Test that contents of last revision of a text file match expected content. - * - * @throws java.lang.Exception */ @Test public void testGetHistoryGet() throws Exception { @@ -267,8 +261,6 @@ public void testGetHistoryGet() throws Exception { /** * Test that it is possible to get contents of multiple revisions of a file. - * - * @throws java.lang.Exception */ @Test public void testgetHistoryGetForAll() throws Exception { @@ -287,8 +279,6 @@ public void testgetHistoryGetForAll() throws Exception { /** * Test that {@code getHistoryGet()} returns historical contents of renamed * file. - * - * @throws java.lang.Exception */ @Test public void testGetHistoryGetRenamed() throws Exception { @@ -315,8 +305,6 @@ public void testGetHistoryGetRenamed() throws Exception { /** * Test that {@code getHistory()} throws an exception if the revision * argument doesn't match any of the revisions in the history. - * - * @throws java.lang.Exception */ @Test public void testGetHistoryWithNoSuchRevision() throws Exception { diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/PerforceRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/PerforceRepositoryTest.java index a0c6a08f5c2..9b5fb1d193d 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/PerforceRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/PerforceRepositoryTest.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017, 2020, Chris Fraire . + * Portions Copyright (c) 2017, 2019-2020, Chris Fraire . * Portions Copyright (c) 2019, Chris Ross . */ package org.opengrok.indexer.history; @@ -122,7 +122,7 @@ public void testHistoryAndAnnotations() throws Exception { for (File f : files) { if (instance.fileHasHistory(f)) { - History history = instance.getHistory(f); + History history = HistoryUtil.union(instance.getHistory(f)); assertNotNull("Failed to get history for: " + f.getAbsolutePath(), history); for (HistoryEntry entry : history.getHistoryEntries()) { diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RCSRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RCSRepositoryTest.java index be88b5a403b..bc0f74c7ffa 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RCSRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RCSRepositoryTest.java @@ -19,6 +19,7 @@ /* * Copyright (c) 2017, 2018 Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.history; @@ -93,7 +94,7 @@ public void testAnnotation() throws Exception { public void testGetHistory() throws Exception { File root = new File(repository.getSourceRoot(), "rcs_test"); RCSRepository repo = (RCSRepository) RepositoryFactory.getRepository(root); - History hist = repo.getHistory(new File(root, "Makefile")); + History hist = HistoryUtil.union(repo.getHistory(new File(root, "Makefile"))); List entries = hist.getHistoryEntries(); assertEquals(REVISIONS.length, entries.size()); for (int i = 0; i < entries.size(); i++) { diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java index 8069fa3f330..fd0e58d0863 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/RepositoryTest.java @@ -19,12 +19,11 @@ /* * Copyright (c) 2017, 2018 Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2018, Chris Fraire . + * Portions Copyright (c) 2018-2019, Chris Fraire . */ package org.opengrok.indexer.history; import java.io.File; -import java.io.IOException; import java.text.ParseException; import java.util.Arrays; import org.junit.Assert; @@ -154,7 +153,7 @@ public boolean hasHistoryForDirectories() { } @Override - public History getHistory(File file) throws HistoryException { + public HistoryEnumeration getHistory(File file) { return null; } @@ -170,7 +169,7 @@ public boolean fileHasAnnotation(File file) { } @Override - public Annotation annotate(File file, String revision) throws IOException { + public Annotation annotate(File file, String revision) { return null; } @@ -180,17 +179,17 @@ public boolean isRepositoryFor(File file, CommandTimeoutType cmdType) { } @Override - public String determineParent(CommandTimeoutType cmdType) throws IOException { + public String determineParent(CommandTimeoutType cmdType) { return ""; } @Override - public String determineBranch(CommandTimeoutType cmdType) throws IOException { + public String determineBranch(CommandTimeoutType cmdType) { return ""; } @Override - String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException { + String determineCurrentVersion(CommandTimeoutType cmdType) { return null; } } diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java index f01bc63a9d4..c3bf60e6f15 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerTest.java @@ -294,7 +294,7 @@ public void fileRemoved(String path) { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); File f = new File(env.getDataRootPath(), TandemPath.join("historycache" + path, ".gz")); - Assert.assertTrue("history cache file should be preserved", f.exists()); + Assert.assertTrue("historycache file should be preserved", f.exists()); } removedFiles.add(path); } diff --git a/suggester/pom.xml b/suggester/pom.xml index ce3a5c2da56..eb15cc695ae 100644 --- a/suggester/pom.xml +++ b/suggester/pom.xml @@ -85,7 +85,7 @@ org.slf4j slf4j-nop - 1.7.25 + 1.7.26