Skip to content

Commit 7b5f14f

Browse files
authored
avoid global isHistoryEnabled() checks in favor of per repository check (#4746)
fixes #4063
1 parent c76efba commit 7b5f14f

File tree

20 files changed

+200
-62
lines changed

20 files changed

+200
-62
lines changed

opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, 2021, Chris Fraire <[email protected]>.
2323
*/
2424
package org.opengrok.indexer.analysis;
@@ -596,7 +596,7 @@ public void populateDocument(Document doc, File file, String path, AbstractAnaly
596596
doc.add(new SortedDocValuesField(QueryBuilder.FULLPATH,
597597
new BytesRef(file.getAbsolutePath())));
598598

599-
if (RuntimeEnvironment.getInstance().isHistoryEnabled()) {
599+
if (HistoryGuru.getInstance().repositorySupportsHistory(file)) {
600600
populateDocumentHistory(doc, file);
601601
}
602602
doc.add(new Field(QueryBuilder.DATE, date, string_ft_stored_nanalyzed_norms));

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2007, 2024, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2007, 2025, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, 2020, Chris Fraire <[email protected]>.
2323
* Portions Copyright (c) 2020, Aleksandr Kirillov <[email protected]>.
2424
*/
@@ -580,7 +580,7 @@ public Configuration() {
580580
setHandleHistoryOfRenamedFiles(false);
581581
setHistoryBasedReindex(true);
582582
setHistoryCache(true);
583-
setHistoryEnabled(true);
583+
setHistoryEnabled(false);
584584
setHitsPerPage(25);
585585
setIgnoredNames(new IgnoredNames());
586586
setIncludedNames(new Filter());

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

+10-3
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, 2020, Chris Fraire <[email protected]>.
2323
*/
2424
package org.opengrok.indexer.history;
@@ -54,7 +54,6 @@
5454
import org.opengrok.indexer.analysis.AnalyzerFactory;
5555
import org.opengrok.indexer.analysis.AnalyzerGuru;
5656
import org.opengrok.indexer.configuration.CommandTimeoutType;
57-
import org.opengrok.indexer.configuration.Configuration;
5857
import org.opengrok.indexer.configuration.Configuration.RemoteSCM;
5958
import org.opengrok.indexer.configuration.OpenGrokThreadFactory;
6059
import org.opengrok.indexer.configuration.PathAccepter;
@@ -630,6 +629,14 @@ public boolean hasHistory(File file) {
630629
}
631630
}
632631

632+
return repositorySupportsHistory(file);
633+
}
634+
635+
/**
636+
* @param file {@link File} object for a file under source root
637+
* @return whether related {@link Repository} and settings allow for history retrieval
638+
*/
639+
public boolean repositorySupportsHistory(File file) {
633640
Repository repo = getRepository(file);
634641
if (repo == null) {
635642
LOGGER.finest(() -> String.format("cannot find repository for '%s' to check history presence",
@@ -642,7 +649,7 @@ public boolean hasHistory(File file) {
642649
}
643650

644651
// This should return true for Annotate view.
645-
Configuration.RemoteSCM globalRemoteSupport = env.getRemoteScmSupported();
652+
RemoteSCM globalRemoteSupport = env.getRemoteScmSupported();
646653
boolean remoteSupported = ((globalRemoteSupport == RemoteSCM.ON)
647654
|| (globalRemoteSupport == RemoteSCM.UIONLY)
648655
|| (globalRemoteSupport == RemoteSCM.DIRBASED)

opengrok-indexer/src/main/java/org/opengrok/indexer/history/LatestRevisionUtil.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
2222
*/
2323
package org.opengrok.indexer.history;
2424

2525
import org.apache.lucene.document.DateTools;
2626
import org.apache.lucene.document.Document;
2727
import org.jetbrains.annotations.Nullable;
2828
import org.jetbrains.annotations.VisibleForTesting;
29-
import org.opengrok.indexer.configuration.RuntimeEnvironment;
3029
import org.opengrok.indexer.index.IndexDatabase;
3130
import org.opengrok.indexer.logger.LoggerFactory;
3231
import org.opengrok.indexer.search.QueryBuilder;
@@ -52,11 +51,11 @@ private LatestRevisionUtil() {
5251

5352
/**
5453
* @param file file object corresponding to a file under source root
55-
* @return last revision string for {@code file} or null
54+
* @return last revision string for {@code file} or {@code null}
5655
*/
5756
@Nullable
5857
public static String getLatestRevision(File file) {
59-
if (!RuntimeEnvironment.getInstance().isHistoryEnabled()) {
58+
if (!HistoryGuru.getInstance().repositorySupportsHistory(file)) {
6059
return null;
6160
}
6261

opengrok-indexer/src/main/java/org/opengrok/indexer/index/IndexDatabase.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -1283,10 +1283,9 @@ private static void createAnnotationCache(File file, Document doc) {
12831283
}
12841284
}
12851285

1286-
private AbstractAnalyzer getAnalyzerFor(File file, String path)
1287-
throws IOException {
1288-
try (InputStream in = new BufferedInputStream(
1289-
new FileInputStream(file))) {
1286+
@VisibleForTesting
1287+
static AbstractAnalyzer getAnalyzerFor(File file, String path) throws IOException {
1288+
try (InputStream in = new BufferedInputStream(new FileInputStream(file))) {
12901289
return AnalyzerGuru.getAnalyzer(in, path);
12911290
}
12921291
}

opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java

-2
Original file line numberDiff line numberDiff line change
@@ -1026,8 +1026,6 @@ public static String[] parseOptions(String[] argv) throws ParseException {
10261026
cfg = new Configuration();
10271027
}
10281028

1029-
cfg.setHistoryEnabled(false); // force user to turn on history capture
1030-
10311029
argv = optParser.parse(argv);
10321030

10331031
return argv;

opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileAnnotationCacheTest.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved.
2222
*/
2323
package org.opengrok.indexer.history;
2424

@@ -61,6 +61,8 @@ void setUp() throws Exception {
6161
repositories = new TestRepository();
6262
repositories.create(getClass().getResource("/repositories"));
6363

64+
env.setHistoryEnabled(true);
65+
6466
// This needs to be set before the call to env.setRepositories() below as it instantiates HistoryGuru.
6567
env.setAnnotationCacheEnabled(true);
6668

opengrok-indexer/src/test/java/org/opengrok/indexer/history/FileHistoryCacheTest.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2018, 2020, Chris Fraire <[email protected]>.
2323
* Portions Copyright (c) 2020, 2023, Ric Harris <[email protected]>.
2424
*/
@@ -55,6 +55,7 @@
5555
import org.apache.commons.lang3.time.DateUtils;
5656
import org.eclipse.jgit.api.Git;
5757
import org.junit.jupiter.api.AfterEach;
58+
import org.junit.jupiter.api.BeforeAll;
5859
import org.junit.jupiter.api.BeforeEach;
5960
import org.junit.jupiter.api.Test;
6061
import org.junit.jupiter.api.condition.EnabledOnOs;
@@ -90,6 +91,12 @@ class FileHistoryCacheTest {
9091
private boolean savedIsHandleHistoryOfRenamedFiles;
9192
private boolean savedIsTagsEnabled;
9293

94+
@BeforeAll
95+
static void setUpClass() throws Exception {
96+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
97+
env.setHistoryEnabled(true);
98+
}
99+
93100
/**
94101
* Set up the test environment with repositories and a cache instance.
95102
*/

opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruTest.java

+1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class HistoryGuruTest {
9090
@BeforeAll
9191
static void setUpClass() throws Exception {
9292
env = RuntimeEnvironment.getInstance();
93+
env.setHistoryEnabled(true);
9394
env.setAnnotationCacheEnabled(true);
9495
savedNestingMaximum = env.getNestingMaximum();
9596

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

+7
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import org.apache.commons.lang3.tuple.Pair;
2727
import org.junit.jupiter.api.AfterEach;
28+
import org.junit.jupiter.api.BeforeAll;
2829
import org.junit.jupiter.api.BeforeEach;
2930
import org.junit.jupiter.api.Test;
3031
import org.junit.jupiter.params.ParameterizedTest;
@@ -108,6 +109,12 @@ private void setUpTestRepository() throws IOException, URISyntaxException {
108109
repositoryRoot = new File(repository.getSourceRoot(), "mercurial");
109110
}
110111

112+
@BeforeAll
113+
static void setUpClass() throws Exception {
114+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
115+
env.setHistoryEnabled(true);
116+
}
117+
111118
@BeforeEach
112119
void setup() throws IOException, URISyntaxException {
113120
setUpTestRepository();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* CDDL HEADER START
3+
*
4+
* The contents of this file are subject to the terms of the
5+
* Common Development and Distribution License (the "License").
6+
* You may not use this file except in compliance with the License.
7+
*
8+
* See LICENSE.txt included in this distribution for the specific
9+
* language governing permissions and limitations under the License.
10+
*
11+
* When distributing Covered Code, include this CDDL HEADER in each
12+
* file and include the License file at LICENSE.txt.
13+
* If applicable, add the following below this CDDL HEADER, with the
14+
* fields enclosed by brackets "[]" replaced with your own identifying
15+
* information: Portions Copyright [yyyy] [name of copyright owner]
16+
*
17+
* CDDL HEADER END
18+
*/
19+
20+
/*
21+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
22+
*/
23+
package org.opengrok.indexer.index;
24+
25+
import org.apache.lucene.document.Document;
26+
import org.apache.lucene.document.TextField;
27+
import org.junit.jupiter.api.AfterEach;
28+
import org.junit.jupiter.api.BeforeEach;
29+
import org.junit.jupiter.params.ParameterizedTest;
30+
import org.junit.jupiter.params.provider.ValueSource;
31+
import org.mockito.ArgumentCaptor;
32+
import org.mockito.Mockito;
33+
import org.opengrok.indexer.analysis.AbstractAnalyzer;
34+
import org.opengrok.indexer.analysis.AnalyzerGuru;
35+
import org.opengrok.indexer.configuration.CommandTimeoutType;
36+
import org.opengrok.indexer.configuration.RuntimeEnvironment;
37+
import org.opengrok.indexer.history.HistoryGuru;
38+
import org.opengrok.indexer.history.Repository;
39+
import org.opengrok.indexer.history.RepositoryFactory;
40+
import org.opengrok.indexer.search.QueryBuilder;
41+
import org.opengrok.indexer.util.NullWriter;
42+
import org.opengrok.indexer.util.TestRepository;
43+
44+
import java.io.File;
45+
import java.io.Writer;
46+
import java.net.URL;
47+
import java.nio.file.Path;
48+
import java.util.HashMap;
49+
50+
import static org.junit.jupiter.api.Assertions.assertEquals;
51+
import static org.junit.jupiter.api.Assertions.assertNotNull;
52+
import static org.junit.jupiter.api.Assertions.assertTrue;
53+
import static org.mockito.Mockito.atLeast;
54+
import static org.mockito.Mockito.verify;
55+
56+
class AnalyzerGuruDocumentTest {
57+
private RuntimeEnvironment env;
58+
59+
private static TestRepository testRepository;
60+
61+
@BeforeEach
62+
void setUpClass() throws Exception {
63+
env = RuntimeEnvironment.getInstance();
64+
65+
testRepository = new TestRepository();
66+
URL resourceURL = HistoryGuru.class.getResource("/repositories");
67+
assertNotNull(resourceURL);
68+
testRepository.create(resourceURL);
69+
70+
env.setSourceRoot(testRepository.getSourceRoot());
71+
env.setDataRoot(testRepository.getDataRoot());
72+
env.setHistoryEnabled(true);
73+
env.setProjectsEnabled(true);
74+
RepositoryFactory.initializeIgnoredNames(env);
75+
76+
// Restore the project and repository information.
77+
env.setProjects(new HashMap<>());
78+
env.setRepositories(testRepository.getSourceRoot());
79+
HistoryGuru.getInstance().invalidateRepositories(env.getRepositories(), CommandTimeoutType.INDEXER);
80+
env.generateProjectRepositoriesMap();
81+
}
82+
83+
@AfterEach
84+
void tearDownClass() throws Exception {
85+
testRepository.destroy();
86+
}
87+
88+
/**
89+
* {@link AnalyzerGuru#populateDocument(Document, File, String, AbstractAnalyzer, Writer)} should populate
90+
* the history of the document only if the repository related to the file allows for it.
91+
*/
92+
@ParameterizedTest
93+
@ValueSource(booleans = {false, true})
94+
void testPopulateDocumentHistory(boolean historyEnabled) throws Exception {
95+
AnalyzerGuru analyzerGuru = new AnalyzerGuru();
96+
Document doc = Mockito.mock(Document.class);
97+
Path filePath = Path.of(env.getSourceRootPath(), "git", "main.c");
98+
File file = filePath.toFile();
99+
assertTrue(file.exists());
100+
HistoryGuru histGuru = HistoryGuru.getInstance();
101+
Repository repository = histGuru.getRepository(file);
102+
assertNotNull(repository);
103+
repository.setHistoryEnabled(historyEnabled);
104+
String relativePath = env.getPathRelativeToSourceRoot(file);
105+
analyzerGuru.populateDocument(doc, file, relativePath,
106+
IndexDatabase.getAnalyzerFor(file, relativePath), new NullWriter());
107+
ArgumentCaptor<TextField> argument = ArgumentCaptor.forClass(TextField.class);
108+
verify(doc, atLeast(1)).add(argument.capture());
109+
assertEquals(historyEnabled,
110+
argument.getAllValues().stream().anyMatch(e -> e.name().equals(QueryBuilder.HIST)));
111+
}
112+
}

opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexerVsDeletedDocumentsTest.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.
2222
*/
2323
package org.opengrok.indexer.index;
2424

@@ -36,6 +36,7 @@
3636
import org.eclipse.jgit.treewalk.TreeWalk;
3737
import org.eclipse.jgit.treewalk.filter.PathFilter;
3838
import org.junit.jupiter.api.AfterEach;
39+
import org.junit.jupiter.api.BeforeAll;
3940
import org.junit.jupiter.api.BeforeEach;
4041
import org.junit.jupiter.params.ParameterizedTest;
4142
import org.junit.jupiter.params.provider.Arguments;
@@ -81,6 +82,12 @@
8182
class IndexerVsDeletedDocumentsTest {
8283
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
8384

85+
@BeforeAll
86+
static void setUpClass() {
87+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
88+
env.setHistoryEnabled(true);
89+
}
90+
8491
private static String getRandomString(int numChars) {
8592
Random random = new Random();
8693
final StringBuilder sb = new StringBuilder();

opengrok-indexer/src/test/java/org/opengrok/indexer/search/context/HistoryContextTest.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2017, Chris Fraire <[email protected]>.
2323
*/
2424
package org.opengrok.indexer.search.context;
@@ -64,9 +64,12 @@ class HistoryContextTest {
6464

6565
@BeforeAll
6666
static void setUpClass() throws Exception {
67+
RuntimeEnvironment env = RuntimeEnvironment.getInstance();
68+
env.setHistoryEnabled(true);
69+
6770
repositories = new TestRepository();
6871
repositories.create(HistoryContextTest.class.getResource("/repositories"));
69-
RuntimeEnvironment.getInstance().setRepositories(repositories.getSourceRoot());
72+
env.setRepositories(repositories.getSourceRoot());
7073
}
7174

7275
@AfterAll

0 commit comments

Comments
 (0)