Skip to content

Commit eeb9592

Browse files
idodeclareidodeclare
committed
Fix #423 Fix #1984 : redirect when 1 hit for any eligible query
1 parent b5cf496 commit eeb9592

File tree

4 files changed

+161
-40
lines changed

4 files changed

+161
-40
lines changed

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

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import org.opengrok.indexer.analysis.AbstractAnalyzer;
6666
import org.opengrok.indexer.analysis.AnalyzerGuru;
6767
import org.opengrok.indexer.analysis.ExpandTabsReader;
68+
import org.opengrok.indexer.analysis.StreamSource;
6869
import org.opengrok.indexer.authorization.AuthorizationFramework;
6970
import org.opengrok.indexer.configuration.Group;
7071
import org.opengrok.indexer.configuration.Project;
@@ -78,6 +79,7 @@
7879
import org.opengrok.indexer.logger.LoggerFactory;
7980
import org.opengrok.indexer.search.QueryBuilder;
8081
import org.opengrok.indexer.util.IOUtils;
82+
import org.opengrok.indexer.util.LineBreaker;
8183
import org.opengrok.indexer.util.TandemPath;
8284
import org.opengrok.indexer.web.messages.MessagesContainer.AcceptedMessage;
8385
import org.suigeneris.jrcs.diff.Diff;
@@ -131,6 +133,7 @@ public final class PageConfig {
131133
private String pageTitle;
132134
private String dtag;
133135
private String rev;
136+
private String fragmentIdentifier; // Also settable via match offset translation
134137
private Boolean hasAnnotation;
135138
private Boolean annotate;
136139
private Annotation annotation;
@@ -309,7 +312,7 @@ public DiffData getDiffData() {
309312
while ((line = br.readLine()) != null) {
310313
lines.add(line);
311314
}
312-
data.file[i] = lines.toArray(new String[lines.size()]);
315+
data.file[i] = lines.toArray(new String[0]);
313316
lines.clear();
314317
}
315318
in[i] = null;
@@ -536,7 +539,7 @@ public int getIntParam(String name, int defaultValue) {
536539
ret = x;
537540
}
538541
} catch (NumberFormatException e) {
539-
LOGGER.log(Level.INFO, "Failed to parse integer " + s, e);
542+
LOGGER.log(Level.INFO, "Failed to parse " + name + " integer " + s, e);
540543
}
541544
}
542545
return ret;
@@ -748,15 +751,6 @@ public Annotation getAnnotation() {
748751
return annotation;
749752
}
750753

751-
/**
752-
* Get the name which should be show as "Crossfile".
753-
*
754-
* @return the name of the related file or directory.
755-
*/
756-
public String getCrossFilename() {
757-
return getResourceFile().getName();
758-
}
759-
760754
/**
761755
* Get the {@code path} parameter and display value for "Search only in"
762756
* option.
@@ -1351,10 +1345,22 @@ public String getRevisionLocation(String revStr) {
13511345
sb.append("&");
13521346
sb.append(req.getQueryString());
13531347
}
1354-
String frag = req.getParameter(QueryParameters.FRAGMENT_IDENTIFIER_PARAM);
1355-
if (frag != null) {
1348+
if (fragmentIdentifier != null) {
1349+
String anchor = Util.URIEncode(fragmentIdentifier);
1350+
1351+
String reqFrag = req.getParameter(QueryParameters.FRAGMENT_IDENTIFIER_PARAM);
1352+
if (reqFrag == null || reqFrag.isEmpty()) {
1353+
/*
1354+
* We've determined that the fragmentIdentifier field must have
1355+
* been set to augment request parameters. Now include it
1356+
* explicitly in the next request parameters.
1357+
*/
1358+
sb.append("&");
1359+
sb.append(QueryParameters.FRAGMENT_IDENTIFIER_PARAM_EQ);
1360+
sb.append(anchor);
1361+
}
13561362
sb.append("#");
1357-
sb.append(Util.URIEncode(frag));
1363+
sb.append(anchor);
13581364
}
13591365

13601366
return sb.toString();
@@ -1518,6 +1524,7 @@ public SearchHelper prepareInternalSearch() {
15181524
// jel: this should be IMHO a config param since not only core dependend
15191525
sh.parallel = Runtime.getRuntime().availableProcessors() > 1;
15201526
sh.isCrossRefSearch = getPrefix() == Prefix.SEARCH_R;
1527+
sh.isGuiSearch = sh.isCrossRefSearch || getPrefix() == Prefix.SEARCH_P;
15211528
sh.compressed = env.isCompressXref();
15221529
sh.desc = getEftarReader();
15231530
sh.sourceRoot = new File(getSourceRootPath());
@@ -1547,6 +1554,7 @@ private PageConfig(HttpServletRequest req) {
15471554
this.req = req;
15481555
this.authFramework = RuntimeEnvironment.getInstance().getAuthorizationFramework();
15491556
this.executor = RuntimeEnvironment.getInstance().getRevisionExecutor();
1557+
this.fragmentIdentifier = req.getParameter(QueryParameters.FRAGMENT_IDENTIFIER_PARAM);
15501558
}
15511559

15521560
/**
@@ -1782,4 +1790,30 @@ public boolean isNotModified(HttpServletRequest request, HttpServletResponse res
17821790
public static String getRelativePath(String root, String path) {
17831791
return Paths.get(root).relativize(Paths.get(path)).toString();
17841792
}
1793+
1794+
public boolean evaluateMatchOffset() {
1795+
if (fragmentIdentifier == null) {
1796+
int matchOffset = getIntParam(QueryParameters.MATCH_OFFSET_PARAM, -1);
1797+
if (matchOffset >= 0) {
1798+
File resourceFile = getResourceFile();
1799+
if (resourceFile.isFile()) {
1800+
LineBreaker breaker = new LineBreaker();
1801+
StreamSource streamSource = StreamSource.fromFile(resourceFile);
1802+
try {
1803+
breaker.reset(streamSource);
1804+
int matchLine = breaker.findLineIndex(matchOffset);
1805+
if (matchLine >= 0) {
1806+
// Convert to 1-based offset to accord with OpenGrok line number.
1807+
fragmentIdentifier = String.valueOf(matchLine + 1);
1808+
return true;
1809+
}
1810+
} catch (IOException e) {
1811+
LOGGER.log(Level.WARNING, "Failed to evaluate match offset for " +
1812+
resourceFile, e);
1813+
}
1814+
}
1815+
}
1816+
}
1817+
return false;
1818+
}
17851819
}

‎opengrok-indexer/src/main/java/org/opengrok/indexer/web/QueryParameters.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,16 @@ public class QueryParameters {
107107
*/
108108
public static final String HIST_SEARCH_PARAM_EQ = HIST_SEARCH_PARAM + "=";
109109

110+
/**
111+
* Parameter name to specify a match offset.
112+
*/
113+
public static final String MATCH_OFFSET_PARAM = "mo";
114+
115+
/**
116+
* {@link #MATCH_OFFSET_PARAM} concatenated with {@code "=" }.
117+
*/
118+
public static final String MATCH_OFFSET_PARAM_EQ = MATCH_OFFSET_PARAM + "=";
119+
110120
/**
111121
* Parameter name to specify an OpenGrok search of paths.
112122
*/

‎opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java

Lines changed: 90 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,28 @@
4646
import org.apache.lucene.index.DirectoryReader;
4747
import org.apache.lucene.index.IndexReader;
4848
import org.apache.lucene.index.IndexableField;
49+
import org.apache.lucene.index.LeafReaderContext;
50+
import org.apache.lucene.index.ReaderUtil;
4951
import org.apache.lucene.index.Term;
5052
import org.apache.lucene.queryparser.classic.ParseException;
5153
import org.apache.lucene.search.IndexSearcher;
54+
import org.apache.lucene.search.Matches;
55+
import org.apache.lucene.search.MatchesIterator;
56+
import org.apache.lucene.search.MatchesUtils;
5257
import org.apache.lucene.search.Query;
5358
import org.apache.lucene.search.ScoreDoc;
59+
import org.apache.lucene.search.ScoreMode;
5460
import org.apache.lucene.search.Sort;
5561
import org.apache.lucene.search.SortField;
5662
import org.apache.lucene.search.TermQuery;
5763
import org.apache.lucene.search.TopDocs;
5864
import org.apache.lucene.search.TopFieldDocs;
65+
import org.apache.lucene.search.Weight;
5966
import org.apache.lucene.search.spell.DirectSpellChecker;
6067
import org.apache.lucene.search.spell.SuggestMode;
6168
import org.apache.lucene.search.spell.SuggestWord;
6269
import org.apache.lucene.store.FSDirectory;
70+
import org.opengrok.indexer.analysis.AbstractAnalyzer;
6371
import org.opengrok.indexer.analysis.AnalyzerGuru;
6472
import org.opengrok.indexer.analysis.CompatibleAnalyser;
6573
import org.opengrok.indexer.analysis.Definitions;
@@ -142,6 +150,11 @@ public class SearchHelper {
142150
* met.
143151
*/
144152
public boolean isCrossRefSearch;
153+
/**
154+
* As with {@link #isCrossRefSearch}, but here indicating either a
155+
* cross-reference search or a "full blown search".
156+
*/
157+
public boolean isGuiSearch;
145158
/**
146159
* if not {@code null}, the consumer should redirect the client to a
147160
* separate result page denoted by the value of this field. Automatically
@@ -391,42 +404,93 @@ public SearchHelper executeQuery() {
391404
TopFieldDocs fdocs = searcher.search(query, start + maxItems, sort);
392405
totalHits = fdocs.totalHits.value;
393406
hits = fdocs.scoreDocs;
394-
// Bug #3900: Check if this is a search for a single term, and that
395-
// term is a definition. If that's the case, and we only have one match,
396-
// we'll generate a direct link instead of a listing.
397-
boolean isSingleDefinitionSearch
398-
= (query instanceof TermQuery) && (builder.getDefs() != null);
399-
400-
// Attempt to create a direct link to the definition if we search for
401-
// one single definition term AND we have exactly one match AND there
402-
// is only one definition of that symbol in the document that matches.
403-
boolean uniqueDefinition = false;
404-
Document doc = null;
405-
String symbol = null;
406-
if (isCrossRefSearch && isSingleDefinitionSearch && hits != null && hits.length == 1) {
407-
doc = searcher.doc(hits[0].doc);
408-
IndexableField tagsField = doc.getField(QueryBuilder.TAGS);
409-
if (tagsField != null) {
410-
byte[] rawTags = tagsField.binaryValue().bytes;
411-
Definitions tags = Definitions.deserialize(rawTags);
412-
symbol = ((TermQuery) query).getTerm().text();
413-
if (tags.occurrences(symbol) == 1) {
414-
uniqueDefinition = true;
415-
}
407+
408+
/*
409+
* Determine if possibly a single-result redirect to xref is
410+
* eligible and applicable. If history query is active, then nope.
411+
*/
412+
if (hits != null && hits.length == 1 && builder.getHist() == null) {
413+
int docID = hits[0].doc;
414+
if (isCrossRefSearch && query instanceof TermQuery && builder.getDefs() != null) {
415+
maybeRedirectToDefinition(docID, (TermQuery) query);
416+
} else if (isGuiSearch) {
417+
maybeRedirectToMatchOffset(docID, builder.getContextFields());
416418
}
417419
}
418-
if (uniqueDefinition) {
420+
} catch (IOException | ClassNotFoundException e) {
421+
errorMsg = e.getMessage();
422+
}
423+
return this;
424+
}
425+
426+
private void maybeRedirectToDefinition(int docID, TermQuery termQuery)
427+
throws IOException, ClassNotFoundException {
428+
// Bug #3900: Check if this is a search for a single term, and that
429+
// term is a definition. If that's the case, and we only have one match,
430+
// we'll generate a direct link instead of a listing.
431+
//
432+
// Attempt to create a direct link to the definition if we search for
433+
// one single definition term AND we have exactly one match AND there
434+
// is only one definition of that symbol in the document that matches.
435+
Document doc = searcher.doc(docID);
436+
IndexableField tagsField = doc.getField(QueryBuilder.TAGS);
437+
if (tagsField != null) {
438+
byte[] rawTags = tagsField.binaryValue().bytes;
439+
Definitions tags = Definitions.deserialize(rawTags);
440+
String symbol = termQuery.getTerm().text();
441+
if (tags.occurrences(symbol) == 1) {
419442
String anchor = Util.URIEncode(symbol);
420443
redirect = contextPath + Prefix.XREF_P
421444
+ Util.URIEncodePath(doc.get(QueryBuilder.PATH))
422445
+ '?' + QueryParameters.FRAGMENT_IDENTIFIER_PARAM_EQ + anchor
423446
+ '#' + anchor;
424447
}
425-
} catch (IOException | ClassNotFoundException e) {
426-
errorMsg = e.getMessage();
427448
}
428-
return this;
429449
}
450+
451+
private void maybeRedirectToMatchOffset(int docID, List<String> contextFields)
452+
throws IOException {
453+
/*
454+
* Only PLAIN files might redirect to a file offset, since an offset
455+
* must be subsequently converted to a line number and that is tractable
456+
* only from plain text.
457+
*/
458+
Document doc = searcher.doc(docID);
459+
String genre = doc.get(QueryBuilder.T);
460+
if (!AbstractAnalyzer.Genre.PLAIN.typeName().equals(genre)) {
461+
return;
462+
}
463+
464+
List<LeafReaderContext> leaves = reader.leaves();
465+
int subIndex = ReaderUtil.subIndex(docID, leaves);
466+
LeafReaderContext leaf = leaves.get(subIndex);
467+
468+
Query rewritten = query.rewrite(reader);
469+
Weight weight = rewritten.createWeight(searcher, ScoreMode.COMPLETE_NO_SCORES, 1);
470+
Matches matches = weight.matches(leaf, docID);
471+
if (matches != MatchesUtils.MATCH_WITH_NO_TERMS) {
472+
int matchCount = 0;
473+
int offset = -1;
474+
for (String field : contextFields) {
475+
MatchesIterator matchesIterator = matches.getMatches(field);
476+
while (matchesIterator.next()) {
477+
if (matchesIterator.startOffset() >= 0) {
478+
// Abort if there is more than a single match offset.
479+
if (++matchCount > 1) {
480+
return;
481+
}
482+
offset = matchesIterator.startOffset();
483+
}
484+
}
485+
}
486+
if (offset >= 0) {
487+
redirect = contextPath + Prefix.XREF_P
488+
+ Util.URIEncodePath(doc.get(QueryBuilder.PATH))
489+
+ '?' + QueryParameters.MATCH_OFFSET_PARAM_EQ + offset;
490+
}
491+
}
492+
}
493+
430494
private static final Pattern TABSPACE = Pattern.compile("[\t ]+");
431495

432496
private void getSuggestion(Term term, IndexReader ir,

‎opengrok-web/src/main/webapp/list.jsp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,13 @@ final String DUMMY_REVISION = "unknown";
7878
*/
7979
String latestRevision = cfg.getLatestRevision();
8080
if (latestRevision != null) {
81+
cfg.evaluateMatchOffset();
8182
String location = cfg.getRevisionLocation(latestRevision);
8283
response.sendRedirect(location);
8384
return;
8485
}
8586
if (!cfg.getEnv().isGenerateHtml()) {
87+
cfg.evaluateMatchOffset();
8688
/*
8789
* Economy mode is on and failed to get the last revision
8890
* (presumably running with history turned off). Use dummy
@@ -93,6 +95,17 @@ final String DUMMY_REVISION = "unknown";
9395
response.sendRedirect(location);
9496
return;
9597
}
98+
99+
if (cfg.evaluateMatchOffset()) {
100+
/*
101+
* If after calling, a match offset has been translated to a
102+
* fragment identifier (specifying a line#), then redirect to self.
103+
* This block will not be activated again the second time.
104+
*/
105+
String location = cfg.getRevisionLocation(""); // empty
106+
response.sendRedirect(location);
107+
return;
108+
}
96109
}
97110
98111
Annotation annotation = cfg.getAnnotation();

0 commit comments

Comments
 (0)