Skip to content

Commit a903f80

Browse files
author
Vladimir Kotal
authored
PageConfig#getLastRevFromHistory() should work with history efficiently (#3623)
fixes #3624
1 parent a139371 commit a903f80

File tree

7 files changed

+124
-36
lines changed

7 files changed

+124
-36
lines changed

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
import org.eclipse.jgit.util.io.CountingOutputStream;
8282
import org.eclipse.jgit.util.io.NullOutputStream;
8383
import org.jetbrains.annotations.NotNull;
84+
import org.jetbrains.annotations.Nullable;
8485
import org.opengrok.indexer.configuration.CommandTimeoutType;
8586
import org.opengrok.indexer.configuration.RuntimeEnvironment;
8687
import org.opengrok.indexer.logger.LoggerFactory;
@@ -483,7 +484,24 @@ public void accept(String sinceRevision, Consumer<String> visitor) throws Histor
483484
}
484485
}
485486

487+
@Nullable
488+
@Override
489+
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
490+
History hist = getHistory(file, null, null, 1);
491+
return getLastHistoryEntry(hist);
492+
}
493+
486494
public History getHistory(File file, String sinceRevision, String tillRevision) throws HistoryException {
495+
return getHistory(file, sinceRevision, tillRevision, null);
496+
}
497+
498+
public History getHistory(File file, String sinceRevision, String tillRevision,
499+
Integer numCommits) throws HistoryException {
500+
501+
if (numCommits != null && numCommits <= 0) {
502+
return null;
503+
}
504+
487505
final List<HistoryEntry> entries = new ArrayList<>();
488506
final Set<String> renamedFiles = new HashSet<>();
489507

@@ -517,6 +535,7 @@ public History getHistory(File file, String sinceRevision, String tillRevision)
517535
}
518536
}
519537

538+
int num = 0;
520539
for (RevCommit commit : walk) {
521540
if (commit.getParentCount() > 1 && !isMergeCommitsEnabled()) {
522541
continue;
@@ -535,6 +554,10 @@ public History getHistory(File file, String sinceRevision, String tillRevision)
535554
}
536555

537556
entries.add(historyEntry);
557+
558+
if (numCommits != null && ++num >= numCommits) {
559+
break;
560+
}
538561
}
539562
} catch (IOException | ForbiddenSymlinkException e) {
540563
throw new HistoryException(String.format("failed to get history for ''%s''", file), e);

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,11 @@ public History getHistoryUI(File file) throws HistoryException {
235235
return getHistory(file, true, true);
236236
}
237237

238+
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
239+
final Repository repo = getRepository(file);
240+
return repo.getLastHistoryEntry(file, ui);
241+
}
242+
238243
/**
239244
* Get the history for the specified file.
240245
*

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialHistoryParser.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,15 @@ class MercurialHistoryParser implements Executor.StreamHandler {
7474
* @param sinceRevision the changeset right before the first one to fetch, or
7575
* {@code null} if all changesets should be fetched
7676
* @param tillRevision end revision or {@code null}
77+
* @param numCommits number of revisions to get
7778
* @return history for the specified file or directory
7879
* @throws HistoryException if an error happens when parsing the history
7980
*/
80-
History parse(File file, String sinceRevision, String tillRevision) throws HistoryException {
81+
History parse(File file, String sinceRevision, String tillRevision, Integer numCommits) throws HistoryException {
8182
isDir = file.isDirectory();
8283
try {
83-
Executor executor = repository.getHistoryLogExecutor(file, sinceRevision, tillRevision, false);
84+
Executor executor = repository.getHistoryLogExecutor(file, sinceRevision, tillRevision, false,
85+
numCommits);
8486
int status = executor.exec(true, this);
8587

8688
if (status != 0) {

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.regex.Matcher;
4141
import java.util.regex.Pattern;
4242

43+
import org.jetbrains.annotations.Nullable;
4344
import org.opengrok.indexer.configuration.CommandTimeoutType;
4445
import org.opengrok.indexer.configuration.RuntimeEnvironment;
4546
import org.opengrok.indexer.logger.LoggerFactory;
@@ -160,6 +161,11 @@ private String getRevisionNum(String changeset) throws HistoryException {
160161
}
161162
}
162163

164+
Executor getHistoryLogExecutor(File file, String sinceRevision, String tillRevision, boolean revisionsOnly)
165+
throws HistoryException, IOException {
166+
return getHistoryLogExecutor(file, sinceRevision, tillRevision, revisionsOnly, null);
167+
}
168+
163169
/**
164170
* Get an executor to be used for retrieving the history log for the named
165171
* file or directory.
@@ -170,9 +176,11 @@ private String getRevisionNum(String changeset) throws HistoryException {
170176
* For files this does not apply and full history is returned.
171177
* @param tillRevision end revision
172178
* @param revisionsOnly get only revision numbers
179+
* @param numRevisions number of revisions to get
173180
* @return An Executor ready to be started
174181
*/
175-
Executor getHistoryLogExecutor(File file, String sinceRevision, String tillRevision, boolean revisionsOnly)
182+
Executor getHistoryLogExecutor(File file, String sinceRevision, String tillRevision, boolean revisionsOnly,
183+
Integer numRevisions)
176184
throws HistoryException, IOException {
177185

178186
String filename = getRepoRelativePath(file);
@@ -243,6 +251,11 @@ Executor getHistoryLogExecutor(File file, String sinceRevision, String tillRevis
243251
}
244252
}
245253

254+
if (numRevisions != null && numRevisions > 0) {
255+
cmd.add("-l");
256+
cmd.add(numRevisions.toString());
257+
}
258+
246259
if (!filename.isEmpty()) {
247260
cmd.add("--");
248261
cmd.add(filename);
@@ -261,8 +274,7 @@ Executor getHistoryLogExecutor(File file, String sinceRevision, String tillRevis
261274
* error occurred and with non-zero {@code iterations} if some data was
262275
* transferred
263276
*/
264-
private HistoryRevResult getHistoryRev(
265-
BufferSink sink, String fullpath, String rev) {
277+
private HistoryRevResult getHistoryRev(BufferSink sink, String fullpath, String rev) {
266278

267279
HistoryRevResult result = new HistoryRevResult();
268280
File directory = new File(getDirectoryName());
@@ -271,9 +283,9 @@ private HistoryRevResult getHistoryRev(
271283
if (rev.indexOf(':') != -1) {
272284
revision = rev.substring(0, rev.indexOf(':'));
273285
}
286+
274287
try {
275-
String filename
276-
= fullpath.substring(getDirectoryName().length() + 1);
288+
String filename = fullpath.substring(getDirectoryName().length() + 1);
277289
ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK);
278290
String[] argv = {RepoCommand, "cat", "-r", revision, filename};
279291
Executor executor = new Executor(Arrays.asList(argv), directory,
@@ -289,6 +301,7 @@ private HistoryRevResult getHistoryRev(
289301
} catch (Exception exp) {
290302
LOGGER.log(Level.SEVERE, "Failed to get history", exp);
291303
}
304+
292305
return result;
293306
}
294307

@@ -556,12 +569,29 @@ public void accept(String sinceRevision, Consumer<String> visitor) throws Histor
556569
parse(new File(getDirectoryName()), sinceRevision);
557570
}
558571

572+
@Nullable
573+
@Override
574+
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
575+
History hist = getHistory(file, null, null, 1);
576+
return getLastHistoryEntry(hist);
577+
}
578+
559579
History getHistory(File file, String sinceRevision) throws HistoryException {
560580
return getHistory(file, sinceRevision, null);
561581
}
562582

563583
@Override
564584
History getHistory(File file, String sinceRevision, String tillRevision) throws HistoryException {
585+
return getHistory(file, sinceRevision, tillRevision, null);
586+
}
587+
588+
History getHistory(File file, String sinceRevision, String tillRevision,
589+
Integer numCommits) throws HistoryException {
590+
591+
if (numCommits != null && numCommits <= 0) {
592+
return null;
593+
}
594+
565595
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
566596
// Note that the filtering of revisions based on sinceRevision is done
567597
// in the history log executor by passing appropriate options to
@@ -570,7 +600,8 @@ History getHistory(File file, String sinceRevision, String tillRevision) throws
570600
// for file, the file is renamed and its complete history is fetched
571601
// so no sinceRevision filter is needed.
572602
// See findOriginalName() code for more details.
573-
History result = new MercurialHistoryParser(this).parse(file, sinceRevision, tillRevision);
603+
History result = new MercurialHistoryParser(this).
604+
parse(file, sinceRevision, tillRevision, numCommits);
574605

575606
// Assign tags to changesets they represent.
576607
// We don't need to check if this repository supports tags,
@@ -633,7 +664,6 @@ String determineParent(CommandTimeoutType cmdType) throws IOException {
633664

634665
@Override
635666
public String determineCurrentVersion(CommandTimeoutType cmdType) throws IOException {
636-
String line = null;
637667
File directory = new File(getDirectoryName());
638668

639669
List<String> cmd = new ArrayList<>();

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/Repository.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,39 @@ public abstract class Repository extends RepositoryInfo {
111111
*/
112112
abstract History getHistory(File file) throws HistoryException;
113113

114+
HistoryEntry getLastHistoryEntry(History hist) {
115+
if (hist == null) {
116+
return null;
117+
}
118+
119+
List<HistoryEntry> hlist = hist.getHistoryEntries();
120+
if (hlist == null || hlist.isEmpty()) {
121+
return null;
122+
}
123+
124+
return hlist.get(0);
125+
}
126+
127+
/**
128+
* This is generic implementation that retrieves the full history of given file
129+
* and returns the latest history entry. This is obviously very inefficient, both in terms of memory and I/O.
130+
* The extending classes are encouraged to implement their own version.
131+
* @param file file
132+
* @return last history entry or null
133+
* @throws HistoryException on error
134+
*/
135+
public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryException {
136+
History hist;
137+
try {
138+
hist = HistoryGuru.getInstance().getHistory(file, false, ui);
139+
} catch (HistoryException ex) {
140+
LOGGER.log(Level.WARNING, "failed to get history for {0}", file);
141+
return null;
142+
}
143+
144+
return getLastHistoryEntry(hist);
145+
}
146+
114147
public Repository() {
115148
super();
116149
ignoredFiles = new ArrayList<>();

‎opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,4 +383,15 @@ void testGetHistoryRenamedFileTillRev() throws Exception {
383383
collect(Collectors.toList());
384384
assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 2, 7)), revisions);
385385
}
386+
387+
@Test
388+
void testGetLastHistoryEntry() throws Exception {
389+
File root = new File(repository.getSourceRoot(), "mercurial");
390+
File file = new File(root, "novel.txt");
391+
assertTrue(file.exists() && file.isFile());
392+
MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(root);
393+
HistoryEntry historyEntry = hgRepo.getLastHistoryEntry(file, true);
394+
assertNotNull(historyEntry);
395+
assertEquals("8:6a8c423f5624", historyEntry.getRevision());
396+
}
386397
}

‎opengrok-web/src/main/java/org/opengrok/web/PageConfig.java

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@
7676
import org.opengrok.indexer.configuration.Project;
7777
import org.opengrok.indexer.configuration.RuntimeEnvironment;
7878
import org.opengrok.indexer.history.Annotation;
79-
import org.opengrok.indexer.history.History;
8079
import org.opengrok.indexer.history.HistoryEntry;
8180
import org.opengrok.indexer.history.HistoryException;
8281
import org.opengrok.indexer.history.HistoryGuru;
@@ -1285,38 +1284,23 @@ public String getLatestRevision() {
12851284
}
12861285

12871286
// fallback
1288-
return getLastRevFromHistory();
1289-
}
1290-
1291-
@Nullable
1292-
private String getLastRevFromHistory() {
1293-
History hist;
12941287
try {
1295-
hist = HistoryGuru.getInstance().
1296-
getHistory(new File(getEnv().getSourceRootFile(), getPath()), false, true);
1297-
} catch (HistoryException ex) {
1298-
return null;
1299-
}
1300-
1301-
if (hist == null) {
1302-
return null;
1303-
}
1304-
1305-
List<HistoryEntry> hlist = hist.getHistoryEntries();
1306-
if (hlist == null) {
1307-
return null;
1308-
}
1309-
1310-
if (hlist.size() == 0) {
1288+
return getLastRevFromHistory();
1289+
} catch (HistoryException e) {
1290+
LOGGER.log(Level.WARNING, "cannot get latest revision for {0}", getPath());
13111291
return null;
13121292
}
1293+
}
13131294

1314-
HistoryEntry he = hlist.get(0);
1315-
if (he == null) {
1316-
return null;
1295+
@Nullable
1296+
private String getLastRevFromHistory() throws HistoryException {
1297+
HistoryEntry he = HistoryGuru.getInstance().
1298+
getLastHistoryEntry(new File(getEnv().getSourceRootFile(), getPath()), true);
1299+
if (he != null) {
1300+
return he.getRevision();
13171301
}
13181302

1319-
return he.getRevision();
1303+
return null;
13201304
}
13211305

13221306
/**

0 commit comments

Comments
 (0)