From fd317be70c33a6004dc5287f487274480133cb9c Mon Sep 17 00:00:00 2001 From: Gino Augustine Date: Sun, 10 Dec 2023 22:10:59 +0530 Subject: [PATCH 01/13] Sonar code smell fixes in web package Signed-off-by: Gino Augustine --- .../indexer/util/ErrorMessageCollector.java | 138 ++++++++ .../opengrok/indexer/web/SearchHelper.java | 299 +++++++++++------- .../java/org/opengrok/indexer/web/Util.java | 101 +++--- .../indexer/web/messages/MessagesUtils.java | 30 +- .../indexer/web/SearchHelperTest.java | 19 +- .../java/org/opengrok/web/PageConfig.java | 13 +- 6 files changed, 392 insertions(+), 208 deletions(-) create mode 100644 opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java new file mode 100644 index 00000000000..859a7901aa8 --- /dev/null +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -0,0 +1,138 @@ +/* + * 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) 2023, Oracle and/or its affiliates. + * Portions Copyright (c) 2023, Gino Augustine . + */ +package org.opengrok.indexer.util; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collections; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.StringJoiner; +import java.util.function.BiConsumer; +import java.util.function.BinaryOperator; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collector; + +/** + * Custom collector to collect error messages for list of projects and join it with comma. + * + * + * @author Gino Augustine + */ +public class ErrorMessageCollector implements Collector> { + + private final String prefix; + private final String emptyString; + private final Boolean returnNullWhenEmpty; + + /** + * Creates a collector with given prefix and + * returns optional empty for empty collection . + * @param prefix prefix before the joined string + */ + public ErrorMessageCollector(@NotNull String prefix) { + this(prefix, null); + } + + /** + * Creates a string joiner with given prefix and empty string. + * @param prefix prefix before the joined string + * @param emptyString empty string to display if collection is empty + */ + public ErrorMessageCollector(@NotNull String prefix, @Nullable String emptyString) { + this.prefix = prefix; + this.emptyString = Objects.isNull(emptyString) ? "" : emptyString; + this.returnNullWhenEmpty = Objects.isNull(emptyString); + + } + /** + * A function that creates and returns a new mutable result container. + * + * @return a function which returns a new, mutable result container + */ + @Override + public Supplier supplier() { + return () -> { + var joiner = new StringJoiner(", ", emptyString + prefix, ""); + joiner.setEmptyValue(emptyString); + return joiner; + }; + } + + /** + * A function that folds a value into a mutable result container. + * + * @return a function which folds a value into a mutable result container + */ + @Override + public BiConsumer accumulator() { + return StringJoiner::add; + } + + /** + * A function that accepts two partial results and merges them. The + * combiner function may fold state from one argument into the other and + * return that, or may return a new result container. + * + * @return a function which combines two partial results into a combined + * result + */ + @Override + public BinaryOperator combiner() { + return StringJoiner::merge; + } + + /** + * Perform the final transformation from the intermediate accumulation type + * {@code A} to the final result type {@code R}. + * + *

If the characteristic {@code IDENTITY_FINISH} is + * set, this function may be presumed to be an identity transform with an + * unchecked cast from {@code A} to {@code R}. + * + * @return a function which transforms the intermediate result to the final + * result + */ + @Override + public Function> finisher() { + return stringJoiner -> + Optional.of(stringJoiner.toString()) + .filter(msg -> !(msg.isEmpty() && returnNullWhenEmpty)); + + } + + /** + * Returns a {@code Set} of {@code Collector.Characteristics} indicating + * the characteristics of this Collector. This set should be immutable. + * + * @return an immutable set of collector characteristics + */ + @Override + public Set characteristics() { + return Collections.emptySet(); + } +} diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java index 5955265451e..e01eed29c05 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java @@ -30,26 +30,28 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; +import java.util.function.Function; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; -import java.util.stream.Collectors; + import org.apache.lucene.document.Document; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.IndexableField; -import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.ReaderUtil; import org.apache.lucene.index.Term; import org.apache.lucene.queryparser.classic.ParseException; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Matches; -import org.apache.lucene.search.MatchesIterator; import org.apache.lucene.search.MatchesUtils; import org.apache.lucene.search.Query; import org.apache.lucene.search.ScoreDoc; @@ -59,7 +61,6 @@ import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TopDocs; import org.apache.lucene.search.TopFieldDocs; -import org.apache.lucene.search.Weight; import org.apache.lucene.search.spell.DirectSpellChecker; import org.apache.lucene.search.spell.SuggestMode; import org.apache.lucene.search.spell.SuggestWord; @@ -77,7 +78,9 @@ import org.opengrok.indexer.search.Summarizer; import org.opengrok.indexer.search.context.Context; import org.opengrok.indexer.search.context.HistoryContext; +import org.opengrok.indexer.util.ErrorMessageCollector; import org.opengrok.indexer.util.ForbiddenSymlinkException; +import org.opengrok.indexer.util.WrapperIOException; /** * Working set for a search basically to factor out/separate search related @@ -85,6 +88,7 @@ * * @author Jens Elkner */ +@SuppressWarnings("java:S1135") public class SearchHelper { private static final Logger LOGGER = LoggerFactory.getLogger(SearchHelper.class); @@ -125,11 +129,11 @@ public class SearchHelper { /** * the result cursor start index, i.e. where to start displaying results */ - private final int start; + private int start; /** * max. number of result items to show */ - private final int maxItems; + private int maxItems; /** * The QueryBuilder used to create the query. */ @@ -137,18 +141,18 @@ public class SearchHelper { /** * The order used for ordering query results. */ - private final SortOrder order; + private SortOrder order; /** * Indicate whether this is search from a cross-reference. If {@code true} * {@link #executeQuery()} sets {@link #redirect} if certain conditions are * met. */ - private final boolean crossRefSearch; + private boolean crossRefSearch; /** * As with {@link #crossRefSearch}, but here indicating either a * cross-reference search or a "full blown search". */ - private final boolean guiSearch; + private boolean guiSearch; /** * if not {@code null}, the consumer should redirect the client to a * separate result page denoted by the value of this field. Automatically @@ -159,7 +163,7 @@ public class SearchHelper { * A value indicating if redirection should be short-circuited when state or * query result would have indicated otherwise. */ - private final boolean noRedirect; + private boolean noRedirect; /** * if not {@code null}, the UI should show this error message and stop * processing the search. Automatically set via @@ -222,20 +226,18 @@ public class SearchHelper { private SettingsHelper settingsHelper; - public SearchHelper(int start, SortOrder sortOrder, File dataRoot, File sourceRoot, int maxItems, - EftarFileReader eftarFileReader, QueryBuilder queryBuilder, boolean crossRefSearch, - String contextPath, boolean guiSearch, boolean noRedirect) { - this.start = start; - this.order = sortOrder; + private SearchHelper(File dataRoot, File sourceRoot, + EftarFileReader eftarFileReader, QueryBuilder queryBuilder, + String contextPath) { + this.start = 0; + this.order = SortOrder.RELEVANCY; this.dataRoot = dataRoot; this.sourceRoot = sourceRoot; - this.maxItems = maxItems; + this.maxItems = 10; this.desc = eftarFileReader; this.builder = queryBuilder; - this.crossRefSearch = crossRefSearch; this.contextPath = contextPath; - this.guiSearch = guiSearch; - this.noRedirect = noRedirect; + } public File getDataRoot() { @@ -321,7 +323,6 @@ public HistoryContext getHistoryContext() { /** * User readable description for file types. Only those listed in * fileTypeDescription will be shown to the user. - * * Returns a set of file type descriptions to be used for a search form. * * @return Set of tuples with file type and description. @@ -361,7 +362,7 @@ public SearchHelper prepareExec(SortedSet projects) { // the Query created by the QueryBuilder try { query = builder.build(); - if (projects == null) { + if (Objects.isNull(projects)) { errorMsg = "No project selected!"; return this; } @@ -374,24 +375,21 @@ public SearchHelper prepareExec(SortedSet projects) { reader = superIndexSearcher.getIndexReader(); } else { // Check list of project names first to make sure all of them are valid and indexed. - Set invalidProjects = projects.stream(). + var invalidProjects = projects.stream(). filter(proj -> (Project.getByName(proj) == null)). - collect(Collectors.toSet()); - if (!invalidProjects.isEmpty()) { - errorMsg = "Project list contains invalid projects: " + - String.join(", ", invalidProjects); - return this; - } - Set notIndexedProjects = - projects.stream(). - map(Project::getByName). - filter(proj -> !proj.isIndexed()). - collect(Collectors.toSet()); - if (!notIndexedProjects.isEmpty()) { - errorMsg = "Some of the projects to be searched are not indexed yet: " + - String.join(", ", notIndexedProjects.stream(). - map(Project::getName). - collect(Collectors.toSet())); + collect(new ErrorMessageCollector("Project list contains invalid projects: ")); + + errorMsg = invalidProjects.or(() -> + projects.stream(). + map(Project::getByName). + filter(Objects::nonNull). + filter(proj -> !proj.isIndexed()). + map(Project::getName). + collect(new ErrorMessageCollector( + "Some of the projects to be searched are not indexed yet: " + )) + ).orElse(null); + if (Objects.nonNull(errorMsg)) { return this; } @@ -401,10 +399,10 @@ public SearchHelper prepareExec(SortedSet projects) { if (reader != null) { searcher = RuntimeEnvironment.getInstance().getIndexSearcherFactory().newSearcher(reader); } else { - errorMsg = "Failed to initialize search. Check the index"; - if (!projects.isEmpty()) { - errorMsg += " for projects: " + String.join(", ", projects); - } + errorMsg = projects.stream() + .collect(new ErrorMessageCollector(" for projects: ", + "Failed to initialize search. Check the index")) + .orElse(""); return this; } } @@ -429,10 +427,11 @@ public SearchHelper prepareExec(SortedSet projects) { } catch (ParseException e) { errorMsg = PARSE_ERROR_MSG + e.getMessage(); } catch (FileNotFoundException e) { - errorMsg = "Index database not found. Check the index"; - if (!projects.isEmpty()) { - errorMsg += " for projects: " + String.join(", ", projects); - } + + errorMsg = projects.stream() + .collect(new ErrorMessageCollector(" for projects: ", + "Index database not found. Check the index")) + .orElse(""); errorMsg += "; " + e.getMessage(); } catch (IOException e) { errorMsg = e.getMessage(); @@ -533,40 +532,54 @@ private void maybeRedirectToMatchOffset(int docID, List contextFields) * must be subsequently converted to a line number and that is tractable * only from plain text. */ - Document doc = searcher.storedFields().document(docID); - String genre = doc.get(QueryBuilder.T); + var doc = searcher.storedFields().document(docID); + var genre = doc.get(QueryBuilder.T); if (!AbstractAnalyzer.Genre.PLAIN.typeName().equals(genre)) { return; } - List leaves = reader.leaves(); + var leaves = reader.leaves(); int subIndex = ReaderUtil.subIndex(docID, leaves); - LeafReaderContext leaf = leaves.get(subIndex); - - Query rewritten = query.rewrite(searcher); - Weight weight = rewritten.createWeight(searcher, ScoreMode.COMPLETE_NO_SCORES, 1); - Matches matches = weight.matches(leaf, docID - leaf.docBase); // Adjust docID - if (matches != null && matches != MatchesUtils.MATCH_WITH_NO_TERMS) { - int matchCount = 0; - int offset = -1; + var leaf = leaves.get(subIndex); + + var rewritten = query.rewrite(searcher); + var weight = rewritten.createWeight(searcher, ScoreMode.COMPLETE_NO_SCORES, 1); + var matches = weight.matches(leaf, docID - leaf.docBase); // Adjust docID + try { + Optional.ofNullable(matches) + .filter(Predicate.not(MatchesUtils.MATCH_WITH_NO_TERMS::equals)) + .map(objMatches -> calculateRedirectOffset(objMatches, contextFields)) + .filter(offset -> offset >= 0) + .ifPresent(offset -> + redirect = contextPath + Prefix.XREF_P + + Util.uriEncodePath(doc.get(QueryBuilder.PATH)) + + '?' + QueryParameters.MATCH_OFFSET_PARAM_EQ + offset + ); + + } catch (WrapperIOException ex) { + throw ex.getParentIOException(); + } + + } + private int calculateRedirectOffset(Matches matches, List contextFields) { + int offset = -1; + try { for (String field : contextFields) { - MatchesIterator matchesIterator = matches.getMatches(field); + var matchesIterator = matches.getMatches(field); while (matchesIterator.next()) { if (matchesIterator.startOffset() >= 0) { // Abort if there is more than a single match offset. - if (++matchCount > 1) { - return; + if (offset >= 0) { + return -1; } offset = matchesIterator.startOffset(); } } } - if (offset >= 0) { - redirect = contextPath + Prefix.XREF_P - + Util.uriEncodePath(doc.get(QueryBuilder.PATH)) - + '?' + QueryParameters.MATCH_OFFSET_PARAM_EQ + offset; - } + } catch (IOException ex) { + throw new WrapperIOException(ex); } + return offset; } private void redirectToFile(int docID) throws IOException { @@ -574,20 +587,23 @@ private void redirectToFile(int docID) throws IOException { redirect = contextPath + Prefix.XREF_P + Util.uriEncodePath(doc.get(QueryBuilder.PATH)); } - private void getSuggestion(Term term, IndexReader ir, - List result) throws IOException { - if (term == null) { - return; - } - String[] toks = TAB_SPACE.split(term.text(), 0); - for (String tok : toks) { - //TODO below seems to be case insensitive ... for refs/defs this is bad - SuggestWord[] words = checker.suggestSimilar(new Term(term.field(), tok), - SPELLCHECK_SUGGEST_WORD_COUNT, ir, SuggestMode.SUGGEST_ALWAYS); - for (SuggestWord w : words) { - result.add(w.string); + private String[] getSuggestion(Term term, IndexReader ir) { + //TODO below seems to be case insensitive ... for refs/defs this is bad + Function suggester = token -> { + try { + return checker.suggestSimilar(new Term(term.field(), token), + SPELLCHECK_SUGGEST_WORD_COUNT, ir, SuggestMode.SUGGEST_ALWAYS); + } catch (IOException ex) { + throw new WrapperIOException(ex); } - } + }; + return Optional.ofNullable(term) + .map(fullToken -> TAB_SPACE.split(fullToken.text(), 0)) + .stream().flatMap(Arrays::stream) + .map(suggester) + .flatMap(Arrays::stream) + .map(suggestWord -> suggestWord.string) + .toArray(String[] ::new); } /** @@ -605,60 +621,44 @@ public List getSuggestions() { if (projects == null) { return new ArrayList<>(0); } - - boolean emptyProjects = false; - String[] projectNames; - if (projects.isEmpty()) { - projectNames = new String[]{"/"}; - emptyProjects = true; - } else if (projects.size() == 1) { - projectNames = new String[]{projects.first()}; - } else { - projectNames = new String[projects.size()]; - int ii = 0; - for (String proj : projects) { - projectNames[ii++] = proj; - } - } + var projectNames = Optional.of(projects) + .filter(projectsList -> !projectsList.isEmpty()) + .map(projectsList -> projectsList.toArray(String[]::new)) + .orElseGet(() -> new String[]{"/"}); List res = new ArrayList<>(); - List dummy = new ArrayList<>(); - IndexReader ir = null; - Term t; for (String projectName : projectNames) { Suggestion suggestion = new Suggestion(projectName); try { - SuperIndexSearcher superIndexSearcher; - if (emptyProjects) { - superIndexSearcher = RuntimeEnvironment.getInstance().getSuperIndexSearcher(""); - } else { - superIndexSearcher = RuntimeEnvironment.getInstance().getSuperIndexSearcher(projectName); - } + var searcherName = projects.isEmpty() ? "" : projectName; + var superIndexSearcher = RuntimeEnvironment.getInstance().getSuperIndexSearcher(searcherName); superIndexSearchers.add(superIndexSearcher); - ir = superIndexSearcher.getIndexReader(); + var ir = superIndexSearcher.getIndexReader(); + + Optional.ofNullable(builder.getFreetext()) + .filter(Predicate.not(String::isEmpty)) + .map(freeText -> new Term(QueryBuilder.FULL, freeText)) + .map(term -> getSuggestion(term, ir)) + .ifPresent(suggestion::setFreetext); + Optional.ofNullable(builder.getRefs()) + .filter(Predicate.not(String::isEmpty)) + .map(refs -> new Term(QueryBuilder.REFS, refs)) + .map(term -> getSuggestion(term, ir)) + .ifPresent(suggestion::setDefs); + Optional.ofNullable(builder.getDefs()) + .filter(Predicate.not(String::isEmpty)) + .map(defs -> new Term(QueryBuilder.DEFS, defs)) + .map(term -> getSuggestion(term, ir)) + .ifPresent(suggestion::setDefs); - if (builder.getFreetext() != null && !builder.getFreetext().isEmpty()) { - t = new Term(QueryBuilder.FULL, builder.getFreetext()); - getSuggestion(t, ir, dummy); - suggestion.setFreetext(dummy.toArray(new String[0])); - dummy.clear(); - } - if (builder.getRefs() != null && !builder.getRefs().isEmpty()) { - t = new Term(QueryBuilder.REFS, builder.getRefs()); - getSuggestion(t, ir, dummy); - suggestion.setRefs(dummy.toArray(new String[0])); - dummy.clear(); - } - if (builder.getDefs() != null && !builder.getDefs().isEmpty()) { - t = new Term(QueryBuilder.DEFS, builder.getDefs()); - getSuggestion(t, ir, dummy); - suggestion.setDefs(dummy.toArray(new String[0])); - dummy.clear(); - } //TODO suggest also for path and history? if (suggestion.isUsable()) { res.add(suggestion); } + } catch (WrapperIOException ex) { + LOGGER.log(Level.WARNING, + String.format("Got exception while getting spelling suggestions for project %s:", projectName), + ex.getParentIOException()); } catch (IOException e) { LOGGER.log(Level.WARNING, String.format("Got exception while getting spelling suggestions for project %s:", projectName), @@ -814,4 +814,61 @@ private void ensureSettingsHelper() { settingsHelper = new SettingsHelper(reader); } } + + /** + * Search Helper Builder Class. + */ + public static final class Builder { + private final SearchHelper searchHelper; + + /** + * Search Helper Builder Class constructor. + * start parameter is defaulted to zero + * maxItems parameter is defaulted to 10 + * all boolean parameters(crossRefSearch,guiSearch,noRedirect) is defaulted to false + * order is defaulted to RELEVANCY based sorting + * @param dataRoot dataRoot + * @param sourceRoot sourceRoot + * @param eftarFileReader eftarFileReader + * @param queryBuilder queryBuilder + * @param contextPath contextPath + */ + public Builder(File dataRoot, File sourceRoot, + EftarFileReader eftarFileReader, QueryBuilder queryBuilder, + String contextPath) { + searchHelper = new SearchHelper(dataRoot, sourceRoot, eftarFileReader, queryBuilder, contextPath); + } + public Builder start(int start) { + searchHelper.start = start; + return this; + } + public Builder maxItems(int maxItems) { + searchHelper.maxItems = maxItems; + return this; + } + public Builder order(SortOrder order) { + searchHelper.order = order; + return this; + } + + public Builder crossRefSearch(boolean crossRefSearch) { + searchHelper.crossRefSearch = crossRefSearch; + return this; + } + + public Builder guiSearch(boolean guiSearch) { + searchHelper.guiSearch = guiSearch; + return this; + } + + public Builder noRedirect(boolean noRedirect) { + searchHelper.noRedirect = noRedirect; + return this; + } + + public SearchHelper build() { + return this.searchHelper; + } + + } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index d0e107fffda..4f28353c1b1 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -55,11 +55,12 @@ import java.util.Map.Entry; import java.util.Optional; import java.util.TreeMap; -import java.util.function.IntFunction; +import java.util.function.IntConsumer; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.IntStream; import java.util.zip.GZIPInputStream; import jakarta.servlet.http.HttpServletRequest; @@ -419,7 +420,7 @@ public static String breadcrumbPath(String urlPrefix, String path, */ public static String breadcrumbPath(String urlPrefix, String path, char sep, String urlPostfix, boolean compact, boolean isDir) { - if (path == null || path.length() == 0) { + if (path == null || path.isEmpty()) { return path; } String[] pnames = normalize(path.split(escapeForRegex(sep)), compact); @@ -475,7 +476,7 @@ public static String breadcrumbPath(String urlPrefix, String path, * @return always a canonical path which starts with a '/'. */ public static String getCanonicalPath(String path, char sep) { - if (path == null || path.length() == 0) { + if (path == null || path.isEmpty()) { return "/"; } String[] pnames = normalize(path.split(escapeForRegex(sep)), true); @@ -1388,62 +1389,72 @@ public static String createSlider(int offset, int limit, long size, HttpServletR int startingResult = offset - limit * (offset / limit % 10 + 1); int myFirstPage = startingResult < 0 ? 1 : startingResult / limit + 1; int myLastPage = Math.min(lastPage, myFirstPage + 10 + (myFirstPage == 1 ? 0 : 1)); - - // function taking the page number and appending the desired content into the final buffer - IntFunction generatePageLink = page -> { - int myOffset = Math.max(0, (page - 1) * limit); - if (myOffset <= offset && offset < myOffset + limit) { - // do not generate anchor for current page - buf.append("").append(page).append(SPAN_END); - } else { - buf.append(""); - // add << or >> if this link would lead to another section - if (page == myFirstPage && page != 1) { - buf.append("<<"); - } else if (page == myLastPage && myOffset + limit < size) { - buf.append(">>"); - } else { - buf.append(page); - } - buf.append(""); - } - return null; + String queryString = Optional.ofNullable(request) + .map(HttpServletRequest::getQueryString) + .map(query -> query.replaceFirst(RE_Q_E_A_A_COUNT_EQ_VAL, "")) + .map(query -> query.replaceFirst(RE_Q_E_A_A_START_EQ_VAL, "")) + .map(query -> query.replaceFirst(RE_A_ANCHOR_Q_E_A_A, "")) + .orElse(""); + IntConsumer generatePageLink = pageNumber -> { + var isFirstPage = pageNumber == myFirstPage; + var isLastPage = pageNumber == myLastPage; + buf.append( + generatePageLink(pageNumber, offset, limit, size, isFirstPage, isLastPage, queryString) + ); }; + // slider composition if (myFirstPage != 1) { - generatePageLink.apply(1); + generatePageLink.accept(1); buf.append("..."); } - for (int page = myFirstPage; page <= myLastPage; page++) { - generatePageLink.apply(page); - } + IntStream.rangeClosed(myFirstPage, myLastPage) + .forEach(generatePageLink); if (myLastPage != lastPage) { buf.append("..."); - generatePageLink.apply(lastPage); + generatePageLink.accept(lastPage); } return buf.toString(); } return slider; } + private static String generatePageLink(int page, int offset, int limit, long size, + boolean isFirstPage, boolean isLastPage, String queryString) { + final var buf = new StringBuilder(100); + var myOffset = Math.max(0, (page - 1) * limit); + if (myOffset <= offset && offset < myOffset + limit) { + // do not generate anchor for current page + buf.append("").append(page).append(SPAN_END); + } else { + buf.append(""); + // add << or >> if this link would lead to another section + if (isFirstPage && page != 1) { + buf.append("<<"); + } else if (isLastPage && myOffset + limit < size) { + buf.append(">>"); + } else { + buf.append(page); + } + buf.append(""); + } + return buf.toString(); + + } + + /** * Check if the string is a HTTP URL. * diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java index e34531d1918..34f47149878 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java @@ -170,7 +170,7 @@ private static String messagesToJson(List tags) { } /** - * Print messages for given project into JSON. These messages are + * Convert messages for given project into JSON. These messages are * tagged by project description or tagged by any of the project's group name. * * @param project the project @@ -191,27 +191,14 @@ public static String messagesToJson(Project project, String... additionalTags) { } /** - * Print messages for given project into JSON array. These messages are - * tagged by project description or tagged by any of the project's group - * name. - * - * @param project the project - * @return the json array - * @see #messagesToJson(Project, String...) - */ - public static String messagesToJson(Project project) { - return messagesToJson(project, new String[0]); - } - - /** - * Print messages for given group into JSON. + * Convert messages for given group into JSON. * * @param group the group * @param additionalTags additional list of tags * @return JSON string * @see #messagesToJson(java.util.List) */ - private static String messagesToJson(Group group, String... additionalTags) { + public static String messagesToJson(Group group, String... additionalTags) { List tags = new ArrayList<>(); tags.add(group.getName()); @@ -220,17 +207,6 @@ private static String messagesToJson(Group group, String... additionalTags) { return messagesToJson(tags); } - /** - * Convert messages for given group into JSON. - * - * @param group the group - * @return JSON string - * @see #messagesToJson(Group, String...) - */ - public static String messagesToJson(Group group) { - return messagesToJson(group, new String[0]); - } - /** * @return name of highest cssClass of messages present in the system or null. */ diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java index 82b1d2177c0..cf428a55aea 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/SearchHelperTest.java @@ -77,22 +77,19 @@ private void reindex() throws Exception { } private SearchHelper getSearchHelper(String searchTerm) { - SearchHelper sh = new SearchHelper(0, SortOrder.RELEVANCY, - env.getDataRootFile(), env.getSourceRootFile(), - env.getHitsPerPage(), null, - new QueryBuilder().setFreetext(searchTerm), false, - env.getUrlPrefix(), false, false); - + SearchHelper sh = new SearchHelper.Builder(env.getDataRootFile(), env.getSourceRootFile(), + null, new QueryBuilder().setFreetext(searchTerm), env.getUrlPrefix()) + .maxItems(env.getHitsPerPage()) + .build(); assertNotSame(0, sh.getBuilder().getSize()); return sh; } private SearchHelper getSearchHelperPath(String searchTerm) { - SearchHelper sh = new SearchHelper(0, SortOrder.RELEVANCY, - env.getDataRootFile(), env.getSourceRootFile(), - env.getHitsPerPage(), null, - new QueryBuilder().setPath(searchTerm), false, - env.getUrlPrefix(), false, false); + SearchHelper sh = new SearchHelper.Builder(env.getDataRootFile(), env.getSourceRootFile(), + null, new QueryBuilder().setFreetext(searchTerm), env.getUrlPrefix()) + .maxItems(env.getHitsPerPage()) + .build(); assertNotSame(0, sh.getBuilder().getSize()); return sh; diff --git a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java index dd7eec542b9..8c52d44e431 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -1524,10 +1524,15 @@ public SearchHelper prepareSearch() { */ public SearchHelper prepareInternalSearch(SortOrder sortOrder) { String xrValue = req.getParameter(QueryParameters.NO_REDIRECT_PARAM); - return new SearchHelper(getStartIndex(), sortOrder, getDataRoot(), new File(getSourceRootPath()), - getMaxItems(), getEftarReader(), getQueryBuilder(), getPrefix() == Prefix.SEARCH_R, - req.getContextPath(), getPrefix() == Prefix.SEARCH_R || getPrefix() == Prefix.SEARCH_P, - xrValue != null && !xrValue.isEmpty()); + return new SearchHelper.Builder(getDataRoot(), new File(getSourceRootPath()), getEftarReader(), + getQueryBuilder(), req.getContextPath()) + .start(getStartIndex()) + .order(sortOrder) + .maxItems(getMaxItems()) + .crossRefSearch(getPrefix() == Prefix.SEARCH_R) + .guiSearch(getPrefix() == Prefix.SEARCH_R || getPrefix() == Prefix.SEARCH_P) + .noRedirect(xrValue != null && !xrValue.isEmpty()) + .build(); } /** From 8c7171c8dedea4ca47664ca6fdfd6e78839b8125 Mon Sep 17 00:00:00 2001 From: Gino Augustine Date: Tue, 12 Dec 2023 09:24:23 +0530 Subject: [PATCH 02/13] Changes is is null check Signed-off-by: Gino Augustine --- .../java/org/opengrok/indexer/web/messages/MessagesUtils.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java index 34f47149878..c55eeca5176 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java @@ -39,6 +39,7 @@ import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.SortedSet; import java.util.logging.Level; @@ -179,7 +180,7 @@ private static String messagesToJson(List tags) { * @see #messagesToJson(String...) */ public static String messagesToJson(Project project, String... additionalTags) { - if (project == null) { + if (Objects.isNull(project)) { return JSONable.EMPTY; } From 02282a732e628e84e079db3f0478685d0056a3d1 Mon Sep 17 00:00:00 2001 From: Gino Augustine Date: Sat, 16 Dec 2023 10:46:14 +0530 Subject: [PATCH 03/13] Java doc changes Signed-off-by: Gino Augustine --- .../opengrok/indexer/web/SearchHelper.java | 70 ++++++++++++++++--- .../indexer/web/messages/MessagesUtils.java | 3 +- .../java/org/opengrok/web/PageConfig.java | 1 + 3 files changed, 62 insertions(+), 12 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java index e01eed29c05..d0fb41acb8f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java @@ -18,9 +18,10 @@ */ /* - * Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2011, Jens Elkner. * Portions Copyright (c) 2017, 2020, Chris Fraire . + * Portions Copyright (c) 2023, Gino Augustine . */ package org.opengrok.indexer.web; @@ -816,56 +817,103 @@ private void ensureSettingsHelper() { } /** - * Search Helper Builder Class. + * Builder Class for Search Helper. */ public static final class Builder { private final SearchHelper searchHelper; /** * Search Helper Builder Class constructor. - * start parameter is defaulted to zero - * maxItems parameter is defaulted to 10 - * all boolean parameters(crossRefSearch,guiSearch,noRedirect) is defaulted to false - * order is defaulted to RELEVANCY based sorting - * @param dataRoot dataRoot - * @param sourceRoot sourceRoot - * @param eftarFileReader eftarFileReader - * @param queryBuilder queryBuilder - * @param contextPath contextPath + *

+ * Parameters which are defaulting while using this builder + *

    + *
  • {@link #start} default value zero
    • + *
  • {@link #maxItems} default value 10
    • + *
  • {@link #crossRefSearch} default value false
    • + *
  • {@link #guiSearch} default value false
    • + *
  • {@link #noRedirect} default value false
    • + *
  • {@link #order} defaulted to RELEVANCY based sorting
    • + *
+ * @param dataRoot used to find the search index file + * @param sourceRoot the source root directory. + * @param eftarFileReader the Eftar file-reader to use. + * @param queryBuilder The QueryBuilder used to create the query. + * @param contextPath the applications' context path (usually /source) to use + * when generating a redirect URL */ public Builder(File dataRoot, File sourceRoot, EftarFileReader eftarFileReader, QueryBuilder queryBuilder, String contextPath) { searchHelper = new SearchHelper(dataRoot, sourceRoot, eftarFileReader, queryBuilder, contextPath); } + + /** + * Set result cursor start index , i.e. where to start displaying results. + * @param start result cursor start index + * @return search helper builder instance + */ public Builder start(int start) { searchHelper.start = start; return this; } + + /** + * set max. number of result items to show. + * @param maxItems maximum result items to show + * @return search helper builder instance + */ public Builder maxItems(int maxItems) { searchHelper.maxItems = maxItems; return this; } + + /** + * Sets order used for ordering query results. + * @param order query results sort order + * @return search helper builder instance + */ public Builder order(SortOrder order) { searchHelper.order = order; return this; } + /** + * Indicate whether this is search from a cross-reference. If {@code true} + * {@link #executeQuery()} sets {@link #redirect} if certain conditions are met. + * @param crossRefSearch enable or disable crossRefSearch + * @return search helper builder instance + */ public Builder crossRefSearch(boolean crossRefSearch) { searchHelper.crossRefSearch = crossRefSearch; return this; } + /** + *As with {@link #crossRefSearch}, but here indicating either a + * cross-reference search or a "full blown search". + * @param guiSearch enable or disable guiSearch + * @return search helper builder instance + */ public Builder guiSearch(boolean guiSearch) { searchHelper.guiSearch = guiSearch; return this; } + /** + * A value indicating if redirection should be short-circuited when state or + * query result would have indicated otherwise. + * @param noRedirect enable or disable redirection parameter + * @return search helper builder instance + */ public Builder noRedirect(boolean noRedirect) { searchHelper.noRedirect = noRedirect; return this; } + /** + * Create and return the final search helper instance. + * @return search helper configured to the specification of previous calls to this builder + */ public SearchHelper build() { return this.searchHelper; } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java index c55eeca5176..c672681f646 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java @@ -18,7 +18,8 @@ */ /* - * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2023, Gino Augustine . */ package org.opengrok.indexer.web.messages; diff --git a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java index 8c52d44e431..e1d16df386a 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -21,6 +21,7 @@ * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2011, Jens Elkner. * Portions Copyright (c) 2017, 2020, Chris Fraire . + * Portions Copyright (c) 2023, Gino Augustine . */ package org.opengrok.web; From b22622a1bb45200cab6211485e4eadac333974fb Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Thu, 18 Jan 2024 11:59:14 +0100 Subject: [PATCH 04/13] add missing blank line --- .../java/org/opengrok/indexer/util/ErrorMessageCollector.java | 1 + 1 file changed, 1 insertion(+) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java index 859a7901aa8..a7029e524a1 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -69,6 +69,7 @@ public ErrorMessageCollector(@NotNull String prefix, @Nullable String emptyStrin this.returnNullWhenEmpty = Objects.isNull(emptyString); } + /** * A function that creates and returns a new mutable result container. * From af2186be19275a62238d907c189eb431ed857585 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Thu, 18 Jan 2024 12:03:54 +0100 Subject: [PATCH 05/13] add blank line --- .../src/main/java/org/opengrok/indexer/web/SearchHelper.java | 1 + 1 file changed, 1 insertion(+) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java index d0fb41acb8f..31082c279ce 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java @@ -562,6 +562,7 @@ private void maybeRedirectToMatchOffset(int docID, List contextFields) } } + private int calculateRedirectOffset(Matches matches, List contextFields) { int offset = -1; try { From 20dfbe96c5a8c16e5d4ce2022851c0f2422fb977 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Thu, 18 Jan 2024 12:10:41 +0100 Subject: [PATCH 06/13] remove leading spaces --- .../src/main/java/org/opengrok/indexer/web/SearchHelper.java | 1 - 1 file changed, 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java index 31082c279ce..d0fb41acb8f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java @@ -562,7 +562,6 @@ private void maybeRedirectToMatchOffset(int docID, List contextFields) } } - private int calculateRedirectOffset(Matches matches, List contextFields) { int offset = -1; try { From 5618e5ad2662c44e267b1ecec68924955bf6454a Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Thu, 18 Jan 2024 12:12:07 +0100 Subject: [PATCH 07/13] remove leading spaces --- .../java/org/opengrok/indexer/util/ErrorMessageCollector.java | 1 - 1 file changed, 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java index a7029e524a1..859a7901aa8 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -69,7 +69,6 @@ public ErrorMessageCollector(@NotNull String prefix, @Nullable String emptyStrin this.returnNullWhenEmpty = Objects.isNull(emptyString); } - /** * A function that creates and returns a new mutable result container. * From 90c5fddece5884ca93754333397064f6d34857a3 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Thu, 18 Jan 2024 17:56:08 +0100 Subject: [PATCH 08/13] fix message --- .../java/org/opengrok/indexer/util/ErrorMessageCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java index 859a7901aa8..1d7706e12ea 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -38,7 +38,7 @@ import java.util.stream.Collector; /** - * Custom collector to collect error messages for list of projects and join it with comma. + * Custom collector to collect error messages for list of projects and join them with comma. * * * @author Gino Augustine From 2a9a232a3ea90a4eb9701a99876ec1f39e39416e Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Thu, 18 Jan 2024 18:03:48 +0100 Subject: [PATCH 09/13] remove extra space --- .../java/org/opengrok/indexer/util/ErrorMessageCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java index 1d7706e12ea..84482a631c7 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -51,7 +51,7 @@ public class ErrorMessageCollector implements Collector Date: Thu, 18 Jan 2024 18:22:35 +0100 Subject: [PATCH 10/13] adjust doc --- .../java/org/opengrok/indexer/util/ErrorMessageCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java index 84482a631c7..1edaf8395f0 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -61,7 +61,7 @@ public ErrorMessageCollector(@NotNull String prefix) { /** * Creates a string joiner with given prefix and empty string. * @param prefix prefix before the joined string - * @param emptyString empty string to display if collection is empty + * @param emptyString string to display if collection is empty, can be {@code null} */ public ErrorMessageCollector(@NotNull String prefix, @Nullable String emptyString) { this.prefix = prefix; From 4d470e26048a998269cb0531e5ad5f06f8181dce Mon Sep 17 00:00:00 2001 From: Gino Augustine Date: Mon, 19 Feb 2024 10:41:07 +0530 Subject: [PATCH 11/13] Review Comments --- .../indexer/util/ErrorMessageCollector.java | 20 +++--- .../opengrok/indexer/web/SearchHelper.java | 4 +- .../util/ErrorMessageCollectorTest.java | 61 +++++++++++++++++++ 3 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java index 1edaf8395f0..9d85b287b23 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -24,7 +24,6 @@ package org.opengrok.indexer.util; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.util.Collections; import java.util.Objects; @@ -47,7 +46,7 @@ public class ErrorMessageCollector implements Collector supplier() { return () -> { - var joiner = new StringJoiner(", ", emptyString + prefix, ""); + var joiner = new StringJoiner(", ", prefix, ""); joiner.setEmptyValue(emptyString); return joiner; }; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java index d0fb41acb8f..c1099685736 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java @@ -401,7 +401,7 @@ public SearchHelper prepareExec(SortedSet projects) { searcher = RuntimeEnvironment.getInstance().getIndexSearcherFactory().newSearcher(reader); } else { errorMsg = projects.stream() - .collect(new ErrorMessageCollector(" for projects: ", + .collect(new ErrorMessageCollector("Failed to initialize search. Check the index for projects: ", "Failed to initialize search. Check the index")) .orElse(""); return this; @@ -430,7 +430,7 @@ public SearchHelper prepareExec(SortedSet projects) { } catch (FileNotFoundException e) { errorMsg = projects.stream() - .collect(new ErrorMessageCollector(" for projects: ", + .collect(new ErrorMessageCollector("Index database not found. Check the index for projects: ", "Index database not found. Check the index")) .orElse(""); errorMsg += "; " + e.getMessage(); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java new file mode 100644 index 00000000000..88a3ff6db40 --- /dev/null +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java @@ -0,0 +1,61 @@ +/* + * 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) 2023, Oracle and/or its affiliates. + * Portions Copyright (c) 2023, Gino Augustine . + */ +package org.opengrok.indexer.util; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Set; +import java.util.stream.Stream; + +/** + * Represents a container for tests of {@link ErrorMessageCollector}. + */ +class ErrorMessageCollectorTest { + @Test + void noEmptyStringEmptyCollectionReturnOptionalEmpty() { + var collector = new ErrorMessageCollector("TestPrefix"); + var returnValue = Stream.empty().collect(collector); + Assertions.assertTrue(returnValue.isEmpty()); + } + @Test + void emptyStringWithEmptyCollectionReturnOptionalEmpty() { + var collector = new ErrorMessageCollector("TestPrefix", "TestEmptyString"); + var returnValue = Stream.empty().collect(collector); + Assertions.assertEquals("TestEmptyString", returnValue.orElse("")); + } + @Test + void noEmptyStringWithMultiElementCollectionReturnJoinedString() { + var collector = new ErrorMessageCollector("TestPrefix "); + var returnValue = Set.of("a", "b").stream().collect(collector); + Assertions.assertTrue(returnValue.orElse("").startsWith("TestPrefix ")); + } + @Test + void emptyStringWithMultiElementCollectionReturnJoinedStringWithoutEmptyString() { + var collector = new ErrorMessageCollector("TestPrefix ", "TestEmptyString"); + var returnValue = Set.of("a", "b").stream().collect(collector); + Assertions.assertTrue(returnValue.orElse("").startsWith("TestPrefix ")); + } + +} \ No newline at end of file From 8f5bf9dc4a94d8cecbc4b5e799d30477d2623244 Mon Sep 17 00:00:00 2001 From: Gino Augustine Date: Thu, 22 Feb 2024 08:58:17 +0530 Subject: [PATCH 12/13] Bumped Year in Copyright Updated test case assert based on review comments --- .../indexer/util/ErrorMessageCollector.java | 4 ++-- .../org/opengrok/indexer/web/SearchHelper.java | 4 ++-- .../main/java/org/opengrok/indexer/web/Util.java | 3 ++- .../indexer/web/messages/MessagesUtils.java | 4 ++-- .../indexer/util/ErrorMessageCollectorTest.java | 15 ++++++++------- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java index 9d85b287b23..9a53729834d 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/ErrorMessageCollector.java @@ -18,8 +18,8 @@ */ /* - * Copyright (c) 2023, Oracle and/or its affiliates. - * Portions Copyright (c) 2023, Gino Augustine . + * Copyright (c) 2024, Oracle and/or its affiliates. + * Portions Copyright (c) 2024, Gino Augustine . */ package org.opengrok.indexer.util; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java index c1099685736..f8177ef2d62 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/SearchHelper.java @@ -18,10 +18,10 @@ */ /* - * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2011, Jens Elkner. * Portions Copyright (c) 2017, 2020, Chris Fraire . - * Portions Copyright (c) 2023, Gino Augustine . + * Portions Copyright (c) 2024, Gino Augustine . */ package org.opengrok.indexer.web; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 4f28353c1b1..2764cc47e28 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -18,11 +18,12 @@ */ /* - * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2011, Jens Elkner. * Portions Copyright (c) 2017, 2020, Chris Fraire . * Portions Copyright (c) 2019, Krystof Tulinger . * Portions Copyright (c) 2023, Ric Harris . + * Portions Copyright (c) 2024, Gino Augustine . */ package org.opengrok.indexer.web; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java index c672681f646..2fe74238ae6 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/messages/MessagesUtils.java @@ -18,8 +18,8 @@ */ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2023, Gino Augustine . + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Portions Copyright (c) 2024, Gino Augustine . */ package org.opengrok.indexer.web.messages; diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java index 88a3ff6db40..61f398fcc8f 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ErrorMessageCollectorTest.java @@ -18,15 +18,14 @@ */ /* - * Copyright (c) 2023, Oracle and/or its affiliates. - * Portions Copyright (c) 2023, Gino Augustine . + * Copyright (c) 2024, Oracle and/or its affiliates. + * Portions Copyright (c) 2024, Gino Augustine . */ package org.opengrok.indexer.util; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.util.Set; import java.util.stream.Stream; /** @@ -48,14 +47,16 @@ void emptyStringWithEmptyCollectionReturnOptionalEmpty() { @Test void noEmptyStringWithMultiElementCollectionReturnJoinedString() { var collector = new ErrorMessageCollector("TestPrefix "); - var returnValue = Set.of("a", "b").stream().collect(collector); - Assertions.assertTrue(returnValue.orElse("").startsWith("TestPrefix ")); + var returnValue = Stream.of("a", "b").collect(collector); + Assertions.assertTrue(returnValue.isPresent()); + Assertions.assertEquals("TestPrefix a, b", returnValue.get()); } @Test void emptyStringWithMultiElementCollectionReturnJoinedStringWithoutEmptyString() { var collector = new ErrorMessageCollector("TestPrefix ", "TestEmptyString"); - var returnValue = Set.of("a", "b").stream().collect(collector); - Assertions.assertTrue(returnValue.orElse("").startsWith("TestPrefix ")); + var returnValue = Stream.of("a", "b").collect(collector); + Assertions.assertTrue(returnValue.isPresent()); + Assertions.assertEquals("TestPrefix a, b", returnValue.get()); } } \ No newline at end of file From 16ccfcf2ff938eae617cc5922faff9500c0ac198 Mon Sep 17 00:00:00 2001 From: Gino Augustine Date: Sun, 25 Feb 2024 15:34:38 +0530 Subject: [PATCH 13/13] Review comments --- .../java/org/opengrok/indexer/web/Util.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 2764cc47e28..389f2af7184 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -1382,9 +1382,8 @@ public static String createSlider(int offset, int limit, int size) { * @return string containing slider html */ public static String createSlider(int offset, int limit, long size, HttpServletRequest request) { - String slider = ""; if (limit < size) { - final StringBuilder buf = new StringBuilder(4096); + final StringBuilder slider = new StringBuilder(4096); int lastPage = (int) Math.ceil((double) size / limit); // startingResult is the number of a first result on the current page int startingResult = offset - limit * (offset / limit % 10 + 1); @@ -1396,10 +1395,10 @@ public static String createSlider(int offset, int limit, long size, HttpServletR .map(query -> query.replaceFirst(RE_Q_E_A_A_START_EQ_VAL, "")) .map(query -> query.replaceFirst(RE_A_ANCHOR_Q_E_A_A, "")) .orElse(""); - IntConsumer generatePageLink = pageNumber -> { + IntConsumer addToSliderPageWithPageNumber = pageNumber -> { var isFirstPage = pageNumber == myFirstPage; var isLastPage = pageNumber == myLastPage; - buf.append( + slider.append( generatePageLink(pageNumber, offset, limit, size, isFirstPage, isLastPage, queryString) ); }; @@ -1407,18 +1406,18 @@ public static String createSlider(int offset, int limit, long size, HttpServletR // slider composition if (myFirstPage != 1) { - generatePageLink.accept(1); - buf.append("..."); + addToSliderPageWithPageNumber.accept(1); + slider.append("..."); } IntStream.rangeClosed(myFirstPage, myLastPage) - .forEach(generatePageLink); + .forEach(addToSliderPageWithPageNumber); if (myLastPage != lastPage) { - buf.append("..."); - generatePageLink.accept(lastPage); + slider.append("..."); + addToSliderPageWithPageNumber.accept(lastPage); } - return buf.toString(); + return slider.toString(); } - return slider; + return ""; } private static String generatePageLink(int page, int offset, int limit, long size,