From 11673e6d079465b81086349ccfd2fc19d69e8304 Mon Sep 17 00:00:00 2001 From: Matthias Thieroff Date: Wed, 15 Dec 2021 15:07:46 +0100 Subject: [PATCH] Fix display of ellipsis in search fragments (#1896) Display ellipsis as an indicator that there is more content before or behind a search result fragment only if there really is more content. --- gradle/changelog/search_ellipsis.yaml | 2 + .../src/main/java/sonia/scm/search/Hit.java | 25 +++++++-- .../src/__resources__/ContentSearchHit.ts | 12 +++-- .../src/__snapshots__/storyshots.test.ts.snap | 8 --- .../ui-components/src/search/TextHitField.tsx | 4 +- scm-ui/ui-scripts/src/webpack.config.js | 1 + scm-ui/ui-types/src/Search.ts | 2 + .../sonia/scm/search/ContentFragment.java | 44 +++++++++++++++ .../sonia/scm/search/LuceneHighlighter.java | 24 ++++++--- .../sonia/scm/search/QueryResultFactory.java | 9 ++-- .../scm/search/LuceneHighlighterTest.java | 54 +++++++++++-------- 11 files changed, 135 insertions(+), 50 deletions(-) create mode 100644 gradle/changelog/search_ellipsis.yaml create mode 100644 scm-webapp/src/main/java/sonia/scm/search/ContentFragment.java diff --git a/gradle/changelog/search_ellipsis.yaml b/gradle/changelog/search_ellipsis.yaml new file mode 100644 index 0000000000..b682bc6ae6 --- /dev/null +++ b/gradle/changelog/search_ellipsis.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Do not display ellipsis if search result matches start or end of content ([#1896](https://github.com/scm-manager/scm-manager/pull/1896)) diff --git a/scm-core/src/main/java/sonia/scm/search/Hit.java b/scm-core/src/main/java/sonia/scm/search/Hit.java index 6779fff141..aff09678df 100644 --- a/scm-core/src/main/java/sonia/scm/search/Hit.java +++ b/scm-core/src/main/java/sonia/scm/search/Hit.java @@ -81,7 +81,7 @@ public class Hit { @Getter @AllArgsConstructor(access = AccessLevel.PRIVATE) public abstract static class Field { - boolean highlighted; + private final boolean highlighted; } /** @@ -89,7 +89,7 @@ public class Hit { */ @Getter public static class ValueField extends Field { - Object value; + private final Object value; public ValueField(Object value) { super(false); @@ -102,11 +102,30 @@ public class Hit { */ @Getter public static class HighlightedField extends Field { - String[] fragments; + private final String[] fragments; + + /** + * @since 2.28.0 + */ + private final boolean matchesContentStart; + + /** + * @since 2.28.0 + */ + private final boolean matchesContentEnd; public HighlightedField(String[] fragments) { + this(fragments, false, false); + } + + /** + * @since 2.28.0 + */ + public HighlightedField(String[] fragments, boolean matchesContentStart, boolean matchesContentEnd) { super(true); this.fragments = fragments; + this.matchesContentStart = matchesContentStart; + this.matchesContentEnd = matchesContentEnd; } } diff --git a/scm-ui/ui-components/src/__resources__/ContentSearchHit.ts b/scm-ui/ui-components/src/__resources__/ContentSearchHit.ts index bfb07e3a66..b55c370c59 100644 --- a/scm-ui/ui-components/src/__resources__/ContentSearchHit.ts +++ b/scm-ui/ui-components/src/__resources__/ContentSearchHit.ts @@ -33,7 +33,9 @@ export const javaHit: Hit = { "import org.slf4j.LoggerFactory;\n\nimport java.util.Date;\nimport java.util.HashMap;\nimport java.util.Map;\nimport java.util.concurrent.TimeUnit;\n\n/**\n * Jwt implementation of {@link <|[[--AccessTokenBuilder--]]|>}.\n * \n * @author Sebastian Sdorra\n * @since 2.0.0\n */\npublic final class <|[[--JwtAccessTokenBuilder--]]|> implements <|[[--AccessTokenBuilder--]]|> {\n\n /**\n * the logger for <|[[--JwtAccessTokenBuilder--]]|>\n */\n private static final Logger LOG = LoggerFactory.getLogger(<|[[--JwtAccessTokenBuilder.class--]]|>);\n \n private final KeyGenerator keyGenerator; \n private final SecureKeyResolver keyResolver; \n \n private String subject;\n private String issuer;\n", " private final Map custom = Maps.newHashMap();\n \n <|[[--JwtAccessTokenBuilder--]]|>(KeyGenerator keyGenerator, SecureKeyResolver keyResolver) {\n this.keyGenerator = keyGenerator;\n this.keyResolver = keyResolver;\n }\n\n @Override\n public <|[[--JwtAccessTokenBuilder--]]|> subject(String subject) {\n", ' public <|[[--JwtAccessTokenBuilder--]]|> custom(String key, Object value) {\n Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "null or empty value not allowed");\n Preconditions.checkArgument(value != null, "null or empty value not allowed");\n' - ] + ], + matchesContentStart: false, + matchesContentEnd: false } }, _links: {} @@ -46,7 +48,9 @@ export const bashHit: Hit = { highlighted: true, fragments: [ '# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\n# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\n# SOFTWARE.\n#\n\n<|[[--getent--]]|> group scm >/dev/null || groupadd -r scm\n<|[[--getent--]]|> passwd scm >/dev/null || \\\n useradd -r -g scm -M \\\n -s /sbin/nologin -d /var/lib/scm \\\n -c "user for the scm-server process" scm\nexit 0\n\n' - ] + ], + matchesContentStart: false, + matchesContentEnd: true } }, _links: {} @@ -60,7 +64,9 @@ export const markdownHit: Hit = { fragments: [ "---\ntitle: SCM-Manager v2 Test <|[[--Cases--]]|>\n---\n\nDescribes the expected behaviour for SCMM v2 REST Resources using manual tests.\n\nThe following states general test <|[[--cases--]]|> per HTTP Method and en expected return code as well as exemplary curl calls.\nResource-specifics are stated \n\n## Test <|[[--Cases--]]|>\n\n### GET\n\n- Collection Resource (e.g. `/users`)\n - Without parameters -> 200\n - Parameters\n - `?pageSize=1` -> Only one embedded element, pageTotal reflects the correct number of pages, `last` link points to last page.\n - `?pageSize=1&page=1` -> `next` link points to page 0 ; `prev` link points to page 2\n - `?sortBy=admin` -> Sorted by `admin` field of embedded objects\n - `?sortBy=admin&desc=true` -> Invert sorting\n- Individual Resource (e.g. `/users/scmadmin`)\n - Exists -> 200\n", "\n### DELETE\n\n- existing -> 204\n- not existing -> 204\n- without permission -> 401\n\n## Exemplary calls & Resource specific test <|[[--cases--]]|>\n\nIn order to extend those tests to other Resources, have a look at the rest docs. Note that the Content Type is specific to each resource as well.\n" - ] + ], + matchesContentStart: true, + matchesContentEnd: false } }, _links: {} diff --git a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap index 7c7930b3cd..ff36b7ad27 100644 --- a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap +++ b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap @@ -90029,8 +90029,6 @@ exports[`Storyshots TextHitField Bash SyntaxHighlighting 1`] = ` exit 0 - ... - `; @@ -90202,8 +90200,6 @@ public final class exports[`Storyshots TextHitField Markdown SyntaxHighlighting 1`] = ` 
-  ...
-
   ---
 title: SCM-Manager v2 Test 
   
@@ -90239,8 +90235,6 @@ Resource-specifics are stated
 - Individual Resource (e.g. \`/users/scmadmin\`)
     - Exists  -> 200
 
-  ...
-
   
 ### DELETE
 
@@ -90286,8 +90280,6 @@ exports[`Storyshots TextHitField Unknown SyntaxHighlighting 1`] = `
 exit 0
 
 
-  ...
-
`; diff --git a/scm-ui/ui-components/src/search/TextHitField.tsx b/scm-ui/ui-components/src/search/TextHitField.tsx index 73fdca7bb8..ca2ce89e98 100644 --- a/scm-ui/ui-components/src/search/TextHitField.tsx +++ b/scm-ui/ui-components/src/search/TextHitField.tsx @@ -39,13 +39,13 @@ const HighlightedTextField: FC = ({ field, syntaxHigh <> {field.fragments.map((fragment, i) => ( - {separator} + {field.matchesContentStart ? null : separator} {syntaxHighlightingLanguage ? ( ) : ( )} - {i + 1 >= field.fragments.length ? separator : null} + {i + 1 >= field.fragments.length && !field.matchesContentEnd ? separator : null} ))} diff --git a/scm-ui/ui-scripts/src/webpack.config.js b/scm-ui/ui-scripts/src/webpack.config.js index f00774cb17..028949a349 100644 --- a/scm-ui/ui-scripts/src/webpack.config.js +++ b/scm-ui/ui-scripts/src/webpack.config.js @@ -140,6 +140,7 @@ module.exports = [ } }, historyApiFallback: true, + host: "127.0.0.1", port: 3000, hot: true, devMiddleware: { diff --git a/scm-ui/ui-types/src/Search.ts b/scm-ui/ui-types/src/Search.ts index 56c3a8cef0..b8890f4e0f 100644 --- a/scm-ui/ui-types/src/Search.ts +++ b/scm-ui/ui-types/src/Search.ts @@ -33,6 +33,8 @@ export type ValueHitField = { export type HighlightedHitField = { highlighted: true; fragments: string[]; + matchesContentStart: boolean; + matchesContentEnd: boolean; }; export type HitField = ValueHitField | HighlightedHitField; diff --git a/scm-webapp/src/main/java/sonia/scm/search/ContentFragment.java b/scm-webapp/src/main/java/sonia/scm/search/ContentFragment.java new file mode 100644 index 0000000000..1f613716b1 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/search/ContentFragment.java @@ -0,0 +1,44 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.search; + +import lombok.Getter; + +@Getter +public class ContentFragment { + private final String fragment; + private final boolean matchesContentStart; + private final boolean matchesContentEnd; + + ContentFragment(String fragment) { + this(fragment, false, false); + } + + ContentFragment(String fragment, boolean matchesContentStart, boolean matchesContentEnd) { + this.fragment = fragment; + this.matchesContentStart = matchesContentStart; + this.matchesContentEnd = matchesContentEnd; + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/search/LuceneHighlighter.java b/scm-webapp/src/main/java/sonia/scm/search/LuceneHighlighter.java index cdcfe81617..5f7bcd989f 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/LuceneHighlighter.java +++ b/scm-webapp/src/main/java/sonia/scm/search/LuceneHighlighter.java @@ -70,25 +70,32 @@ public final class LuceneHighlighter { return field.isHighlighted() && queriedFields.contains(field.getName()); } - public String[] highlight(String fieldName, Indexed.Analyzer fieldAnalyzer, String value) throws InvalidTokenOffsetsException, IOException { + public ContentFragment[] highlight(String fieldName, Indexed.Analyzer fieldAnalyzer, String value) throws InvalidTokenOffsetsException, IOException { String[] fragments = highlighter.getBestFragments(analyzer, fieldName, value, MAX_NUM_FRAGMENTS); if (fieldAnalyzer == Indexed.Analyzer.CODE) { - fragments = keepWholeLine(value, fragments); + return keepWholeLine(value, fragments); } - return Arrays.stream(fragments) - .toArray(String[]::new); + return Arrays.stream(fragments).map(ContentFragment::new) + .toArray(ContentFragment[]::new); } - private String[] keepWholeLine(String content, String[] fragments) { + private ContentFragment[] keepWholeLine(String content, String[] fragments) { return Arrays.stream(fragments) .map(fragment -> keepWholeLine(content, fragment)) - .toArray(String[]::new); + .toArray(ContentFragment[]::new); } - private String keepWholeLine(String content, String fragment) { + private ContentFragment keepWholeLine(String content, String fragment) { + boolean matchesContentStart = false; + boolean matchesContentEnd = false; + String raw = fragment.replace(PRE_TAG, "").replace(POST_TAG, ""); int index = content.indexOf(raw); + if (index == 0) { + matchesContentStart = true; + } + int start = content.lastIndexOf('\n', index); String snippet; @@ -109,9 +116,10 @@ public final class LuceneHighlighter { int end = content.indexOf('\n', index + raw.length()); if (end < 0) { end = content.length(); + matchesContentEnd = true; } - return snippet + content.substring(index + raw.length(), end) + "\n"; + return new ContentFragment(snippet + content.substring(index + raw.length(), end) + (matchesContentEnd ? "" : "\n"), matchesContentStart, matchesContentEnd); } } diff --git a/scm-webapp/src/main/java/sonia/scm/search/QueryResultFactory.java b/scm-webapp/src/main/java/sonia/scm/search/QueryResultFactory.java index 429b91c196..301ab7b095 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/QueryResultFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/search/QueryResultFactory.java @@ -34,6 +34,7 @@ import org.apache.lucene.search.highlight.InvalidTokenOffsetsException; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -75,9 +76,11 @@ public class QueryResultFactory { Object value = field.value(document); if (value != null) { if (highlighter.isHighlightable(field)) { - String[] fragments = createFragments(field, value.toString()); + ContentFragment[] fragments = createFragments(field, value.toString()); if (fragments.length > 0) { - return of(new Hit.HighlightedField(fragments)); + boolean firstFragmentMatchesContentStart = fragments[0].isMatchesContentStart(); + boolean lastFragmentMatchesContentEnd = fragments[fragments.length - 1].isMatchesContentEnd(); + return of(new Hit.HighlightedField(Arrays.stream(fragments).map(ContentFragment::getFragment).toArray(String[]::new), firstFragmentMatchesContentStart, lastFragmentMatchesContentEnd)); } } return of(new Hit.ValueField(value)); @@ -85,7 +88,7 @@ public class QueryResultFactory { return empty(); } - private String[] createFragments(LuceneSearchableField field, String value) throws InvalidTokenOffsetsException, IOException { + private ContentFragment[] createFragments(LuceneSearchableField field, String value) throws InvalidTokenOffsetsException, IOException { return highlighter.highlight(field.getName(), field.getAnalyzer(), value); } diff --git a/scm-webapp/src/test/java/sonia/scm/search/LuceneHighlighterTest.java b/scm-webapp/src/test/java/sonia/scm/search/LuceneHighlighterTest.java index b9f2576079..7e8c7d2843 100644 --- a/scm-webapp/src/test/java/sonia/scm/search/LuceneHighlighterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/search/LuceneHighlighterTest.java @@ -56,43 +56,47 @@ class LuceneHighlighterTest { String content = content("content"); LuceneHighlighter highlighter = new LuceneHighlighter(analyzer, query); - String[] snippets = highlighter.highlight("content", Indexed.Analyzer.DEFAULT, content); + ContentFragment[] contentFragments = highlighter.highlight("content", Indexed.Analyzer.DEFAULT, content); - assertThat(snippets).hasSize(1).allSatisfy( - snippet -> assertThat(snippet).contains("<|[[--Golgafrinchan--]]|>") + assertThat(contentFragments).hasSize(1).allSatisfy( + contentFragment -> assertThat(contentFragment.getFragment()).contains("<|[[--Golgafrinchan--]]|>") ); } @Test void shouldHighlightCodeAndKeepLines() throws IOException, InvalidTokenOffsetsException { - String[] snippets = highlightCode("GameOfLife.java", "die"); + ContentFragment[] contentFragments = highlightCode("GameOfLife.java", "die"); - assertThat(snippets).hasSize(1).allSatisfy( - snippet -> assertThat(snippet.split("\n")).contains( - "\t\t\t\tint neighbors= getNeighbors(above, same, below);", - "\t\t\t\tif(neighbors < 2 || neighbors > 3){", - "\t\t\t\t\tnewGen[row]+= \"_\";//<2 or >3 neighbors -> <|[[--die--]]|>", - "\t\t\t\t}else if(neighbors == 3){", - "\t\t\t\t\tnewGen[row]+= \"#\";//3 neighbors -> spawn/live" - ) + assertThat(contentFragments).hasSize(1).allSatisfy( + contentFragment -> { + assertThat(contentFragment.getFragment().split("\n")).contains( + "\t\t\t\tint neighbors= getNeighbors(above, same, below);", + "\t\t\t\tif(neighbors < 2 || neighbors > 3){", + "\t\t\t\t\tnewGen[row]+= \"_\";//<2 or >3 neighbors -> <|[[--die--]]|>", + "\t\t\t\t}else if(neighbors == 3){", + "\t\t\t\t\tnewGen[row]+= \"#\";//3 neighbors -> spawn/live" + ); + assertThat(contentFragment.isMatchesContentEnd()).isFalse(); + assertThat(contentFragment.isMatchesContentEnd()).isFalse(); + } ); } @Test void shouldNotStartHighlightedFragmentWithLineBreak() throws IOException, InvalidTokenOffsetsException { - String[] snippets = highlightCode("GameOfLife.java", "die"); + ContentFragment[] contentFragments = highlightCode("GameOfLife.java", "die"); - assertThat(snippets).hasSize(1).allSatisfy( - snippet -> assertThat(snippet).doesNotStartWith("\n") + assertThat(contentFragments).hasSize(1).allSatisfy( + contentFragment -> assertThat(contentFragment.getFragment()).doesNotStartWith("\n") ); } @Test void shouldHighlightCodeInTsx() throws IOException, InvalidTokenOffsetsException { - String[] snippets = highlightCode("Button.tsx", "inherit"); + ContentFragment[] contentFragments = highlightCode("Button.tsx", "inherit"); - assertThat(snippets).hasSize(1).allSatisfy( - snippet -> assertThat(snippet.split("\n")).contains( + assertThat(contentFragments).hasSize(1).allSatisfy( + contentFragment -> assertThat(contentFragment.getFragment().split("\n")).contains( "}) => {", " const renderIcon = () => {", " return <>{icon ? \" className=\"is-medium pr-1\" /> : null};", @@ -103,16 +107,20 @@ class LuceneHighlighterTest { @Test void shouldHighlightFirstCodeLine() throws InvalidTokenOffsetsException, IOException { - String[] snippets = highlightCode("GameOfLife.java", "gameoflife"); + ContentFragment[] contentFragments = highlightCode("GameOfLife.java", "gameoflife"); - assertThat(snippets).hasSize(1); + assertThat(contentFragments).hasSize(1); + assertThat(contentFragments[0].isMatchesContentStart()).isTrue(); + assertThat(contentFragments[0].isMatchesContentEnd()).isFalse(); } @Test void shouldHighlightLastCodeLine() throws InvalidTokenOffsetsException, IOException { - String[] snippets = highlightCode("Button.tsx", "default"); + ContentFragment[] contentFragments = highlightCode("Button.tsx", "default"); - assertThat(snippets).hasSize(1); + assertThat(contentFragments).hasSize(1); + assertThat(contentFragments[0].isMatchesContentStart()).isFalse(); + assertThat(contentFragments[0].isMatchesContentEnd()).isTrue(); } @Nested @@ -154,7 +162,7 @@ class LuceneHighlighterTest { } - private String[] highlightCode(String resource, String search) throws IOException, InvalidTokenOffsetsException { + private ContentFragment[] highlightCode(String resource, String search) throws IOException, InvalidTokenOffsetsException { NonNaturalLanguageAnalyzer analyzer = new NonNaturalLanguageAnalyzer(); Query query = new TermQuery(new Term("content", search));