From 39d2f12b668bb3815547d395ad01c079fea9c6d4 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 21 Jul 2021 10:07:41 +0200 Subject: [PATCH] Return separate links for searchable types instead of single templated link (#1733) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search link of the index resource is now an array of links instead of single templated link. The array contains one link for each searchable type. Co-authored-by: René Pfeuffer --- .../java/sonia/scm/search/IndexedType.java | 9 ++ .../java/sonia/scm/search/IndexOptions.java | 2 + .../java/sonia/scm/search/SearchEngine.java | 9 ++ .../java/sonia/scm/search/SearchableType.java | 50 +++++++ scm-ui/ui-api/src/search.ts | 26 +++- scm-ui/ui-types/src/Search.ts | 4 +- .../api/v2/resources/IndexDtoGenerator.java | 23 ++- .../scm/api/v2/resources/ResourceLinks.java | 4 +- .../scm/api/v2/resources/SearchResource.java | 4 +- .../java/sonia/scm/search/LuceneIndex.java | 8 +- .../sonia/scm/search/LuceneQueryBuilder.java | 12 +- .../sonia/scm/search/LuceneSearchEngine.java | 18 ++- ...bleType.java => LuceneSearchableType.java} | 73 ++++++--- .../main/java/sonia/scm/search/Queries.java | 4 +- .../sonia/scm/search/QueryResultFactory.java | 6 +- .../scm/search/SearchableTypeResolver.java | 20 ++- .../sonia/scm/search/SearchableTypes.java | 37 +---- .../v2/resources/IndexDtoGeneratorTest.java | 36 ++++- .../api/v2/resources/IndexResourceTest.java | 6 +- .../api/v2/resources/SearchResourceTest.java | 13 +- .../scm/search/DefaultIndexQueueTest.java | 2 +- .../scm/search/LuceneQueryBuilderTest.java | 37 ++++- .../scm/search/LuceneSearchEngineTest.java | 138 ++++++++++++++++++ 23 files changed, 443 insertions(+), 98 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/search/SearchableType.java rename scm-webapp/src/main/java/sonia/scm/search/{SearchableType.java => LuceneSearchableType.java} (52%) create mode 100644 scm-webapp/src/test/java/sonia/scm/search/LuceneSearchEngineTest.java diff --git a/scm-annotations/src/main/java/sonia/scm/search/IndexedType.java b/scm-annotations/src/main/java/sonia/scm/search/IndexedType.java index 56682623ab..8c24d5948e 100644 --- a/scm-annotations/src/main/java/sonia/scm/search/IndexedType.java +++ b/scm-annotations/src/main/java/sonia/scm/search/IndexedType.java @@ -51,4 +51,13 @@ public @interface IndexedType { * @return name of the index object or an empty string which indicates that the default should be used. */ String value() default ""; + + /** + * Returns the required permission for searching this type. + * Default means that every user is able to search that type of data. + * Note: Every entry in the index has its own permission. + * + * @return required permission for searching this type. + */ + String permission() default ""; } diff --git a/scm-core/src/main/java/sonia/scm/search/IndexOptions.java b/scm-core/src/main/java/sonia/scm/search/IndexOptions.java index a528ee5628..4b99655206 100644 --- a/scm-core/src/main/java/sonia/scm/search/IndexOptions.java +++ b/scm-core/src/main/java/sonia/scm/search/IndexOptions.java @@ -25,6 +25,7 @@ package sonia.scm.search; import com.google.common.annotations.Beta; +import lombok.EqualsAndHashCode; import java.util.Locale; @@ -34,6 +35,7 @@ import java.util.Locale; * @since 2.21.0 */ @Beta +@EqualsAndHashCode public class IndexOptions { private final Type type; diff --git a/scm-core/src/main/java/sonia/scm/search/SearchEngine.java b/scm-core/src/main/java/sonia/scm/search/SearchEngine.java index 245db00c05..19cb95d7f8 100644 --- a/scm-core/src/main/java/sonia/scm/search/SearchEngine.java +++ b/scm-core/src/main/java/sonia/scm/search/SearchEngine.java @@ -26,6 +26,8 @@ package sonia.scm.search; import com.google.common.annotations.Beta; +import java.util.Collection; + /** * The {@link SearchEngine} is the main entry point for indexing and searching. * Note that this is kind of a low level api for indexing. @@ -37,6 +39,13 @@ import com.google.common.annotations.Beta; @Beta public interface SearchEngine { + /** + * Returns a list of searchable types. + * + * @return collection of searchable types + */ + Collection getSearchableTypes(); + /** * Returns the index with the given name and the given options. * The index is created if it does not exist. diff --git a/scm-core/src/main/java/sonia/scm/search/SearchableType.java b/scm-core/src/main/java/sonia/scm/search/SearchableType.java new file mode 100644 index 0000000000..25ba2c1edc --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/search/SearchableType.java @@ -0,0 +1,50 @@ +/* + * 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 com.google.common.annotations.Beta; + +/** + * A type which can be searched with the {@link SearchEngine}. + * + * @since 2.21.0 + */ +@Beta +public interface SearchableType { + + /** + * Return name of the type. + * + * @return name of type + */ + String getName(); + + /** + * Return type in form of class. + * + * @return class of type + */ + Class getType(); +} diff --git a/scm-ui/ui-api/src/search.ts b/scm-ui/ui-api/src/search.ts index 2b81871600..96368a0eb1 100644 --- a/scm-ui/ui-api/src/search.ts +++ b/scm-ui/ui-api/src/search.ts @@ -22,8 +22,8 @@ * SOFTWARE. */ -import { ApiResult, useRequiredIndexLink } from "./base"; -import { QueryResult } from "@scm-manager/ui-types"; +import { ApiResult, useIndexLinks } from "./base"; +import { Link, QueryResult } from "@scm-manager/ui-types"; import { apiClient } from "./apiclient"; import { createQueryString } from "./utils"; import { useQuery } from "react-query"; @@ -38,9 +38,29 @@ const defaultSearchOptions: SearchOptions = { type: "repository", }; +const useSearchLink = (name: string) => { + const links = useIndexLinks(); + const searchLinks = links["search"]; + if (!searchLinks) { + throw new Error("could not find search links in index"); + } + + if (!Array.isArray(searchLinks)) { + throw new Error("search links returned in wrong format, array is expected"); + } + + for (const l of searchLinks as Link[]) { + if (l.name === name) { + return l.href; + } + } + + throw new Error(`could not find search link for ${name}`); +}; + export const useSearch = (query: string, optionParam = defaultSearchOptions): ApiResult => { const options = { ...defaultSearchOptions, ...optionParam }; - const link = useRequiredIndexLink("search").replace("{type}", options.type); + const link = useSearchLink(options.type); const queryParams: Record = {}; queryParams.q = query; diff --git a/scm-ui/ui-types/src/Search.ts b/scm-ui/ui-types/src/Search.ts index da9bdced1b..ea100fb57d 100644 --- a/scm-ui/ui-types/src/Search.ts +++ b/scm-ui/ui-types/src/Search.ts @@ -29,12 +29,12 @@ export type ValueField = { value: unknown; }; -export type HighligthedField = { +export type HighlightedField = { highlighted: true; fragments: string[]; }; -export type Field = ValueField | HighligthedField; +export type Field = ValueField | HighlightedField; export type Hit = HalRepresentation & { score: number; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java index e405799960..a7151c0971 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IndexDtoGenerator.java @@ -37,6 +37,8 @@ import sonia.scm.group.GroupPermissions; import sonia.scm.initialization.InitializationFinisher; import sonia.scm.initialization.InitializationStep; import sonia.scm.plugin.PluginPermissions; +import sonia.scm.search.SearchEngine; +import sonia.scm.search.SearchableType; import sonia.scm.security.AnonymousMode; import sonia.scm.security.Authentications; import sonia.scm.security.PermissionPermissions; @@ -45,9 +47,11 @@ import sonia.scm.web.EdisonHalAppender; import javax.inject.Inject; import java.util.List; +import java.util.stream.Collectors; import static de.otto.edison.hal.Embedded.embeddedBuilder; import static de.otto.edison.hal.Link.link; +import static de.otto.edison.hal.Link.linkBuilder; public class IndexDtoGenerator extends HalAppenderMapper { @@ -55,13 +59,19 @@ public class IndexDtoGenerator extends HalAppenderMapper { private final SCMContextProvider scmContextProvider; private final ScmConfiguration configuration; private final InitializationFinisher initializationFinisher; + private final SearchEngine searchEngine; @Inject - public IndexDtoGenerator(ResourceLinks resourceLinks, SCMContextProvider scmContextProvider, ScmConfiguration configuration, InitializationFinisher initializationFinisher) { + public IndexDtoGenerator(ResourceLinks resourceLinks, + SCMContextProvider scmContextProvider, + ScmConfiguration configuration, + InitializationFinisher initializationFinisher, + SearchEngine searchEngine) { this.resourceLinks = resourceLinks; this.scmContextProvider = scmContextProvider; this.configuration = configuration; this.initializationFinisher = initializationFinisher; + this.searchEngine = searchEngine; } public IndexDto generate() { @@ -132,7 +142,7 @@ public class IndexDtoGenerator extends HalAppenderMapper { builder.single(link("repositoryRoles", resourceLinks.repositoryRoleCollection().self())); builder.single(link("importLog", resourceLinks.repository().importLog("IMPORT_LOG_ID").replace("IMPORT_LOG_ID", "{logId}"))); - builder.single(link("search", resourceLinks.search().search("INDEXED_TYPE").replace("INDEXED_TYPE", "{type}"))); + builder.array(searchLinks()); } else { builder.single(link("login", resourceLinks.authentication().jsonLogin())); } @@ -141,6 +151,15 @@ public class IndexDtoGenerator extends HalAppenderMapper { return new IndexDto(builder.build(), embeddedBuilder.build(), scmContextProvider.getVersion()); } + private List searchLinks() { + return searchEngine.getSearchableTypes().stream() + .map(SearchableType::getName) + .map(typeName -> + linkBuilder("search", resourceLinks.search().query(typeName)).withName(typeName).build() + ) + .collect(Collectors.toList()); + } + private IndexDto handleInitialization(Links.Builder builder, Embedded.Builder embeddedBuilder) { Links.Builder initializationLinkBuilder = Links.linkingTo(); Embedded.Builder initializationEmbeddedBuilder = embeddedBuilder(); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java index 7742ef4300..2a410fb764 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java @@ -1125,8 +1125,8 @@ class ResourceLinks { this.searchLinkBuilder = new LinkBuilder(pathInfo, SearchResource.class); } - public String search(String type) { - return searchLinkBuilder.method("search").parameters(type).href(); + public String query(String type) { + return searchLinkBuilder.method("query").parameters(type).href(); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SearchResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SearchResource.java index 9c18bc1737..6b338ecbfc 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SearchResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SearchResource.java @@ -61,7 +61,7 @@ public class SearchResource { } @GET - @Path("{type}") + @Path("query/{type}") @Produces(VndMediaType.QUERY_RESULT) @Operation( summary = "Query result", @@ -98,7 +98,7 @@ public class SearchResource { name = "pageSize", description = "The maximum number of results per page (defaults to 10)" ) - public QueryResultDto search(@Valid @BeanParam SearchParameters params) { + public QueryResultDto query(@Valid @BeanParam SearchParameters params) { QueryResult result = engine.search(IndexNames.DEFAULT) .start(params.getPage() * params.getPageSize()) .limit(params.getPageSize()) diff --git a/scm-webapp/src/main/java/sonia/scm/search/LuceneIndex.java b/scm-webapp/src/main/java/sonia/scm/search/LuceneIndex.java index f26cc44b71..1b05ee016b 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/LuceneIndex.java +++ b/scm-webapp/src/main/java/sonia/scm/search/LuceneIndex.java @@ -51,7 +51,7 @@ public class LuceneIndex implements Index { @Override public void store(Id id, String permission, Object object) { - SearchableType type = resolver.resolve(object); + LuceneSearchableType type = resolver.resolve(object); String uid = createUid(id, type); Document document = type.getTypeConverter().convert(object); try { @@ -68,7 +68,7 @@ public class LuceneIndex implements Index { } } - private String createUid(Id id, SearchableType type) { + private String createUid(Id id, LuceneSearchableType type) { return id.asString() + "/" + type.getName(); } @@ -78,7 +78,7 @@ public class LuceneIndex implements Index { @Override public void delete(Id id, Class type) { - SearchableType searchableType = resolver.resolve(type); + LuceneSearchableType searchableType = resolver.resolve(type); try { writer.deleteDocuments(new Term(UID, createUid(id, searchableType))); } catch (IOException e) { @@ -97,7 +97,7 @@ public class LuceneIndex implements Index { @Override public void deleteByType(Class type) { - SearchableType searchableType = resolver.resolve(type); + LuceneSearchableType searchableType = resolver.resolve(type); deleteByTypeName(searchableType.getName()); } diff --git a/scm-webapp/src/main/java/sonia/scm/search/LuceneQueryBuilder.java b/scm-webapp/src/main/java/sonia/scm/search/LuceneQueryBuilder.java index 0c00f246c2..41a8bfde7c 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/LuceneQueryBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/search/LuceneQueryBuilder.java @@ -40,6 +40,7 @@ import org.apache.lucene.search.TopDocs; import org.apache.lucene.search.TopScoreDocCollector; import org.apache.lucene.search.WildcardQuery; import org.apache.lucene.search.highlight.InvalidTokenOffsetsException; +import org.apache.shiro.SecurityUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,7 +74,10 @@ public class LuceneQueryBuilder extends QueryBuilder { protected QueryResult execute(QueryParams queryParams) { String queryString = Strings.nullToEmpty(queryParams.getQueryString()); - SearchableType searchableType = resolver.resolve(queryParams.getType()); + LuceneSearchableType searchableType = resolver.resolve(queryParams.getType()); + searchableType.getPermission().ifPresent( + permission -> SecurityUtils.getSubject().checkPermission(permission) + ); Query parsedQuery = createQuery(searchableType, queryParams, queryString); Query query = Queries.filter(parsedQuery, searchableType, queryParams); @@ -105,7 +109,7 @@ public class LuceneQueryBuilder extends QueryBuilder { return topScoreCollector.topDocs(queryParams.getStart(), queryParams.getLimit()); } - private Query createQuery(SearchableType searchableType, QueryParams queryParams, String queryString) { + private Query createQuery(LuceneSearchableType searchableType, QueryParams queryParams, String queryString) { try { if (queryString.contains(":")) { return createExpertQuery(searchableType, queryParams); @@ -116,14 +120,14 @@ public class LuceneQueryBuilder extends QueryBuilder { } } - private Query createExpertQuery(SearchableType searchableType, QueryParams queryParams) throws QueryNodeException { + private Query createExpertQuery(LuceneSearchableType searchableType, QueryParams queryParams) throws QueryNodeException { StandardQueryParser parser = new StandardQueryParser(analyzer); parser.setPointsConfigMap(searchableType.getPointsConfig()); return parser.parse(queryParams.getQueryString(), ""); } - public Query createBestGuessQuery(SearchableType searchableType, QueryBuilder.QueryParams queryParams) { + public Query createBestGuessQuery(LuceneSearchableType searchableType, QueryBuilder.QueryParams queryParams) { String[] fieldNames = searchableType.getFieldNames(); if (fieldNames == null || fieldNames.length == 0) { throw new NoDefaultQueryFieldsFoundException(searchableType.getType()); diff --git a/scm-webapp/src/main/java/sonia/scm/search/LuceneSearchEngine.java b/scm-webapp/src/main/java/sonia/scm/search/LuceneSearchEngine.java index cddbd96669..e15169ebfc 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/LuceneSearchEngine.java +++ b/scm-webapp/src/main/java/sonia/scm/search/LuceneSearchEngine.java @@ -24,19 +24,35 @@ package sonia.scm.search; +import org.apache.shiro.SecurityUtils; +import org.apache.shiro.subject.Subject; + import javax.inject.Inject; +import java.util.Collection; +import java.util.stream.Collectors; public class LuceneSearchEngine implements SearchEngine { + private final SearchableTypeResolver resolver; private final LuceneIndexFactory indexFactory; private final LuceneQueryBuilderFactory queryBuilderFactory; @Inject - public LuceneSearchEngine(LuceneIndexFactory indexFactory, LuceneQueryBuilderFactory queryBuilderFactory) { + public LuceneSearchEngine(SearchableTypeResolver resolver, LuceneIndexFactory indexFactory, LuceneQueryBuilderFactory queryBuilderFactory) { + this.resolver = resolver; this.indexFactory = indexFactory; this.queryBuilderFactory = queryBuilderFactory; } + @Override + public Collection getSearchableTypes() { + Subject subject = SecurityUtils.getSubject(); + return resolver.getSearchableTypes() + .stream() + .filter(type -> type.getPermission().map(subject::isPermitted).orElse(true)) + .collect(Collectors.toList()); + } + @Override public Index getOrCreate(String name, IndexOptions options) { return indexFactory.create(name, options); diff --git a/scm-webapp/src/main/java/sonia/scm/search/SearchableType.java b/scm-webapp/src/main/java/sonia/scm/search/LuceneSearchableType.java similarity index 52% rename from scm-webapp/src/main/java/sonia/scm/search/SearchableType.java rename to scm-webapp/src/main/java/sonia/scm/search/LuceneSearchableType.java index abbade33d7..73b2a93c55 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/SearchableType.java +++ b/scm-webapp/src/main/java/sonia/scm/search/LuceneSearchableType.java @@ -29,42 +29,41 @@ import lombok.Value; import org.apache.lucene.queryparser.flexible.standard.config.PointsConfig; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; @Value -public class SearchableType { +public class LuceneSearchableType implements SearchableType { + + private static final float DEFAULT_BOOST = 1f; Class type; String name; - String[] fieldNames; - Map boosts; - Map pointsConfig; + String permission; List fields; + String[] fieldNames; + Map boosts; + Map pointsConfig; TypeConverter typeConverter; - SearchableType(Class type, - String[] fieldNames, - Map boosts, - Map pointsConfig, - List fields, - TypeConverter typeConverter) { + LuceneSearchableType(Class type, IndexedType annotation, List fields) { this.type = type; - this.name = name(type); - this.fieldNames = fieldNames; - this.boosts = Collections.unmodifiableMap(boosts); - this.pointsConfig = Collections.unmodifiableMap(pointsConfig); - this.fields = Collections.unmodifiableList(fields); - this.typeConverter = typeConverter; + this.name = name(type, annotation); + this.permission = Strings.emptyToNull(annotation.permission()); + this.fields = fields; + this.fieldNames = fieldNames(fields); + this.boosts = boosts(fields); + this.pointsConfig = pointsConfig(fields); + this.typeConverter = TypeConverters.create(type); } - private String name(Class type) { - IndexedType annotation = type.getAnnotation(IndexedType.class); - if (annotation == null) { - throw new IllegalArgumentException( - type.getName() + " has no " + IndexedType.class.getSimpleName() + " annotation" - ); - } + public Optional getPermission() { + return Optional.ofNullable(permission); + } + + private String name(Class type, IndexedType annotation) { String nameFromAnnotation = annotation.value(); if (Strings.isNullOrEmpty(nameFromAnnotation)) { String simpleName = type.getSimpleName(); @@ -72,4 +71,32 @@ public class SearchableType { } return nameFromAnnotation; } + + private String[] fieldNames(List fields) { + return fields.stream() + .filter(SearchableField::isDefaultQuery) + .map(SearchableField::getName) + .toArray(String[]::new); + } + + private Map boosts(List fields) { + Map map = new HashMap<>(); + for (SearchableField field : fields) { + if (field.isDefaultQuery() && field.getBoost() != DEFAULT_BOOST) { + map.put(field.getName(), field.getBoost()); + } + } + return Collections.unmodifiableMap(map); + } + + private Map pointsConfig(List fields) { + Map map = new HashMap<>(); + for (SearchableField field : fields) { + PointsConfig config = field.getPointsConfig(); + if (config != null) { + map.put(field.getName(), config); + } + } + return Collections.unmodifiableMap(map); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/search/Queries.java b/scm-webapp/src/main/java/sonia/scm/search/Queries.java index 4ede1e64e0..6ec016fb65 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/Queries.java +++ b/scm-webapp/src/main/java/sonia/scm/search/Queries.java @@ -36,7 +36,7 @@ final class Queries { private Queries() { } - private static Query typeQuery(SearchableType type) { + private static Query typeQuery(LuceneSearchableType type) { return new TermQuery(new Term(FieldNames.TYPE, type.getName())); } @@ -44,7 +44,7 @@ final class Queries { return new TermQuery(new Term(FieldNames.REPOSITORY, repositoryId)); } - static Query filter(Query query, SearchableType searchableType, QueryBuilder.QueryParams params) { + static Query filter(Query query, LuceneSearchableType searchableType, QueryBuilder.QueryParams params) { BooleanQuery.Builder builder = new BooleanQuery.Builder() .add(query, MUST) .add(typeQuery(searchableType), MUST); 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 7ddfbc3b71..5444195967 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/QueryResultFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/search/QueryResultFactory.java @@ -50,9 +50,9 @@ public class QueryResultFactory { private final Analyzer analyzer; private final Highlighter highlighter; private final IndexSearcher searcher; - private final SearchableType searchableType; + private final LuceneSearchableType searchableType; - public QueryResultFactory(Analyzer analyzer, IndexSearcher searcher, SearchableType searchableType, Query query) { + public QueryResultFactory(Analyzer analyzer, IndexSearcher searcher, LuceneSearchableType searchableType, Query query) { this.analyzer = analyzer; this.searcher = searcher; this.searchableType = searchableType; @@ -61,7 +61,7 @@ public class QueryResultFactory { private Highlighter createHighlighter(Query query) { return new Highlighter( - new SimpleHTMLFormatter("**", "**"), + new SimpleHTMLFormatter("<>", ""), new QueryScorer(query) ); } diff --git a/scm-webapp/src/main/java/sonia/scm/search/SearchableTypeResolver.java b/scm-webapp/src/main/java/sonia/scm/search/SearchableTypeResolver.java index e232d27275..17d999ebaa 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/SearchableTypeResolver.java +++ b/scm-webapp/src/main/java/sonia/scm/search/SearchableTypeResolver.java @@ -31,6 +31,8 @@ import javax.annotation.Nonnull; import javax.inject.Inject; import javax.inject.Singleton; import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -44,7 +46,7 @@ import static sonia.scm.NotFoundException.notFound; @Singleton class SearchableTypeResolver { - private final Map, SearchableType> classToSearchableType = new HashMap<>(); + private final Map, LuceneSearchableType> classToSearchableType = new HashMap<>(); private final Map> nameToClass = new HashMap<>(); @Inject @@ -62,26 +64,30 @@ class SearchableTypeResolver { fillMaps(convert(indexedTypes)); } - private void fillMaps(Iterable types) { - for (SearchableType type : types) { + private void fillMaps(Iterable types) { + for (LuceneSearchableType type : types) { classToSearchableType.put(type.getType(), type); nameToClass.put(type.getName(), type.getType()); } } @Nonnull - private Set convert(Iterable> indexedTypes) { + private Set convert(Iterable> indexedTypes) { return StreamSupport.stream(indexedTypes.spliterator(), false) .map(SearchableTypes::create) .collect(Collectors.toSet()); } - public SearchableType resolve(Object object) { + public Collection getSearchableTypes() { + return Collections.unmodifiableCollection(classToSearchableType.values()); + } + + public LuceneSearchableType resolve(Object object) { return resolve(object.getClass()); } - public SearchableType resolve(Class type) { - SearchableType searchableType = classToSearchableType.get(type); + public LuceneSearchableType resolve(Class type) { + LuceneSearchableType searchableType = classToSearchableType.get(type); if (searchableType == null) { throw notFound(entity("type", type.getName())); } diff --git a/scm-webapp/src/main/java/sonia/scm/search/SearchableTypes.java b/scm-webapp/src/main/java/sonia/scm/search/SearchableTypes.java index e26f85ec4d..cd06d6b6e6 100644 --- a/scm-webapp/src/main/java/sonia/scm/search/SearchableTypes.java +++ b/scm-webapp/src/main/java/sonia/scm/search/SearchableTypes.java @@ -24,46 +24,25 @@ package sonia.scm.search; -import org.apache.lucene.queryparser.flexible.standard.config.PointsConfig; - import java.lang.reflect.Field; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; final class SearchableTypes { - private static final float DEFAULT_BOOST = 1f; - private SearchableTypes() { } - static SearchableType create(Class type) { + static LuceneSearchableType create(Class type) { List fields = new ArrayList<>(); - collectFields(type, fields); - return createSearchableType(type, fields); - } - - private static SearchableType createSearchableType(Class type, List fields) { - String[] fieldsNames = fields.stream() - .filter(SearchableField::isDefaultQuery) - .map(SearchableField::getName) - .toArray(String[]::new); - - Map boosts = new HashMap<>(); - Map pointsConfig = new HashMap<>(); - for (SearchableField field : fields) { - if (field.isDefaultQuery() && field.getBoost() != DEFAULT_BOOST) { - boosts.put(field.getName(), field.getBoost()); - } - PointsConfig config = field.getPointsConfig(); - if (config != null) { - pointsConfig.put(field.getName(), config); - } + IndexedType annotation = type.getAnnotation(IndexedType.class); + if (annotation == null) { + throw new IllegalArgumentException( + type.getName() + " has no " + IndexedType.class.getSimpleName() + " annotation" + ); } - - return new SearchableType(type, fieldsNames, boosts, pointsConfig, fields, TypeConverters.create(type)); + collectFields(type, fields); + return new LuceneSearchableType(type, annotation, fields); } private static void collectFields(Class type, List fields) { diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexDtoGeneratorTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexDtoGeneratorTest.java index ffa4ac8418..7083cfa506 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexDtoGeneratorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexDtoGeneratorTest.java @@ -25,6 +25,7 @@ package sonia.scm.api.v2.resources; import de.otto.edison.hal.Embedded; +import de.otto.edison.hal.Link; import de.otto.edison.hal.Links; import org.apache.shiro.subject.Subject; import org.apache.shiro.util.ThreadContext; @@ -41,15 +42,19 @@ import sonia.scm.config.ScmConfiguration; import sonia.scm.initialization.InitializationFinisher; import sonia.scm.initialization.InitializationStep; import sonia.scm.initialization.InitializationStepResource; +import sonia.scm.search.SearchEngine; +import sonia.scm.search.SearchableType; import sonia.scm.security.AnonymousMode; import java.net.URI; +import java.util.Arrays; import java.util.List; import static de.otto.edison.hal.Link.link; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static sonia.scm.SCMContext.USER_ANONYMOUS; @@ -66,7 +71,8 @@ class IndexDtoGeneratorTest { private ScmConfiguration configuration; @Mock private InitializationFinisher initializationFinisher; - + @Mock + private SearchEngine searchEngine; @InjectMocks private IndexDtoGenerator generator; @@ -137,6 +143,33 @@ class IndexDtoGeneratorTest { assertThat(dto.getLinks().getLinkBy("me")).isNotPresent(); } + + @Test + void shouldAppendSearchLinksForEveryType() { + List types = Arrays.asList( + searchableType("repository"), + searchableType("user"), + searchableType("group") + ); + when(searchEngine.getSearchableTypes()).thenReturn(types); + mockSubjectRelatedResourceLinks(); + when(resourceLinks.search()).thenReturn(new ResourceLinks.SearchLinks(scmPathInfo)); + + when(subject.isAuthenticated()).thenReturn(true); + + IndexDto dto = generator.generate(); + assertThat(dto.getLinks().getLinksBy("search")).contains( + Link.linkBuilder("search", "/api/v2/search/query/repository").withName("repository").build(), + Link.linkBuilder("search", "/api/v2/search/query/user").withName("user").build(), + Link.linkBuilder("search", "/api/v2/search/query/group").withName("group").build() + ); + } + } + + private SearchableType searchableType(String name) { + SearchableType searchableType = mock(SearchableType.class); + when(searchableType.getName()).thenReturn(name); + return searchableType; } @Nested @@ -195,6 +228,5 @@ class IndexDtoGeneratorTest { when(resourceLinks.namespaceCollection()).thenReturn(new ResourceLinks.NamespaceCollectionLinks(scmPathInfo)); when(resourceLinks.me()).thenReturn(new ResourceLinks.MeLinks(scmPathInfo, new ResourceLinks.UserLinks(scmPathInfo))); when(resourceLinks.repository()).thenReturn(new ResourceLinks.RepositoryLinks(scmPathInfo)); - when(resourceLinks.search()).thenReturn(new ResourceLinks.SearchLinks(scmPathInfo)); } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexResourceTest.java index b4fb98ff79..b2ce5cfc59 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/IndexResourceTest.java @@ -33,8 +33,7 @@ import org.junit.Test; import sonia.scm.SCMContextProvider; import sonia.scm.config.ScmConfiguration; import sonia.scm.initialization.InitializationFinisher; -import sonia.scm.initialization.InitializationStep; -import sonia.scm.initialization.InitializationStepResource; +import sonia.scm.search.SearchEngine; import java.net.URI; import java.util.Optional; @@ -59,11 +58,12 @@ public class IndexResourceTest { this.scmContextProvider = mock(SCMContextProvider.class); InitializationFinisher initializationFinisher = mock(InitializationFinisher.class); when(initializationFinisher.isFullyInitialized()).thenReturn(true); + SearchEngine searchEngine = mock(SearchEngine.class); IndexDtoGenerator generator = new IndexDtoGenerator( ResourceLinksMock.createMock(URI.create("/")), scmContextProvider, configuration, - initializationFinisher); + initializationFinisher, searchEngine); this.indexResource = new IndexResource(generator); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SearchResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SearchResourceTest.java index 24d049c1c5..45b072c174 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SearchResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SearchResourceTest.java @@ -37,7 +37,6 @@ import org.mapstruct.factory.Mappers; import org.mockito.Answers; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import sonia.scm.repository.Repository; import sonia.scm.search.Hit; import sonia.scm.search.IndexNames; import sonia.scm.search.QueryResult; @@ -135,11 +134,11 @@ class SearchResourceTest { JsonMockHttpResponse response = search("paging", 1, 20); JsonNode links = response.getContentAsJson().get("_links"); - assertLink(links, "self", "/v2/search/string?q=paging&page=1&pageSize=20"); - assertLink(links, "first", "/v2/search/string?q=paging&page=0&pageSize=20"); - assertLink(links, "prev", "/v2/search/string?q=paging&page=0&pageSize=20"); - assertLink(links, "next", "/v2/search/string?q=paging&page=2&pageSize=20"); - assertLink(links, "last", "/v2/search/string?q=paging&page=4&pageSize=20"); + assertLink(links, "self", "/v2/search/query/string?q=paging&page=1&pageSize=20"); + assertLink(links, "first", "/v2/search/query/string?q=paging&page=0&pageSize=20"); + assertLink(links, "prev", "/v2/search/query/string?q=paging&page=0&pageSize=20"); + assertLink(links, "next", "/v2/search/query/string?q=paging&page=2&pageSize=20"); + assertLink(links, "last", "/v2/search/query/string?q=paging&page=4&pageSize=20"); } @Test @@ -233,7 +232,7 @@ class SearchResourceTest { } private JsonMockHttpResponse search(String query, Integer page, Integer pageSize) throws URISyntaxException, UnsupportedEncodingException { - String uri = "/v2/search/string?q=" + URLEncoder.encode(query, "UTF-8"); + String uri = "/v2/search/query/string?q=" + URLEncoder.encode(query, "UTF-8"); if (page != null) { uri += "&page=" + page; } diff --git a/scm-webapp/src/test/java/sonia/scm/search/DefaultIndexQueueTest.java b/scm-webapp/src/test/java/sonia/scm/search/DefaultIndexQueueTest.java index cac7f8dc8d..89465b3a40 100644 --- a/scm-webapp/src/test/java/sonia/scm/search/DefaultIndexQueueTest.java +++ b/scm-webapp/src/test/java/sonia/scm/search/DefaultIndexQueueTest.java @@ -70,7 +70,7 @@ class DefaultIndexQueueTest { SearchableTypeResolver resolver = new SearchableTypeResolver(Account.class, IndexedNumber.class); LuceneIndexFactory indexFactory = new LuceneIndexFactory(resolver, opener); - SearchEngine engine = new LuceneSearchEngine(indexFactory, queryBuilderFactory); + SearchEngine engine = new LuceneSearchEngine(resolver, indexFactory, queryBuilderFactory); queue = new DefaultIndexQueue(engine); } diff --git a/scm-webapp/src/test/java/sonia/scm/search/LuceneQueryBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/search/LuceneQueryBuilderTest.java index efca96707a..36103a4c29 100644 --- a/scm-webapp/src/test/java/sonia/scm/search/LuceneQueryBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/search/LuceneQueryBuilderTest.java @@ -40,8 +40,10 @@ import org.apache.lucene.document.TextField; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.IndexableField; import org.apache.lucene.store.ByteBuffersDirectory; import org.apache.lucene.store.Directory; +import org.apache.shiro.authz.AuthorizationException; import org.github.sdorra.jse.ShiroExtension; import org.github.sdorra.jse.SubjectAware; import org.junit.jupiter.api.BeforeEach; @@ -378,6 +380,24 @@ class LuceneQueryBuilderTest { assertThrows(NoDefaultQueryFieldsFoundException.class, () -> query(Types.class, "something")); } + @Test + void shouldFailWithoutPermissionForTheSearchedType() throws IOException { + try (IndexWriter writer = writer()) { + writer.addDocument(denyDoc("awesome")); + } + assertThrows(AuthorizationException.class, () -> query(Deny.class, "awesome")); + } + + @Test + @SubjectAware(value = "marvin", permissions = "deny:4711") + void shouldNotFailWithRequiredPermissionForTheSearchedType() throws IOException { + try (IndexWriter writer = writer()) { + writer.addDocument(denyDoc("awesome")); + } + QueryResult result = query(Deny.class, "awesome"); + assertThat(result.getTotalHits()).isOne(); + } + @Test void shouldLimitHitsByDefaultSize() throws IOException { try (IndexWriter writer = writer()) { @@ -452,7 +472,7 @@ class LuceneQueryBuilderTest { JsonNode displayName = fields.get("displayName"); assertThat(displayName.get("highlighted").asBoolean()).isTrue(); - assertThat(displayName.get("fragments").get(0).asText()).contains("**Arthur**"); + assertThat(displayName.get("fragments").get(0).asText()).contains("<>Arthur"); } @Test @@ -556,10 +576,18 @@ class LuceneQueryBuilderTest { return document; } + private Document denyDoc(String value) { + Document document = new Document(); + document.add(new TextField("value", value, Field.Store.YES)); + document.add(new StringField(FieldNames.TYPE, "deny", Field.Store.YES)); + return document; + } + @Getter @IndexedType static class Types { + @Indexed private Integer intValue; @Indexed @@ -600,4 +628,11 @@ class LuceneQueryBuilderTest { private String content; } + @Getter + @IndexedType(permission = "deny:4711") + static class Deny { + @Indexed(defaultQuery = true) + private String value; + } + } diff --git a/scm-webapp/src/test/java/sonia/scm/search/LuceneSearchEngineTest.java b/scm-webapp/src/test/java/sonia/scm/search/LuceneSearchEngineTest.java new file mode 100644 index 0000000000..1dd43c5d5b --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/search/LuceneSearchEngineTest.java @@ -0,0 +1,138 @@ +/* + * 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 org.github.sdorra.jse.ShiroExtension; +import org.github.sdorra.jse.SubjectAware; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.Optional; + +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@SubjectAware("trillian") +@ExtendWith({MockitoExtension.class, ShiroExtension.class}) +class LuceneSearchEngineTest { + + @Mock + private SearchableTypeResolver resolver; + + @Mock + private LuceneIndexFactory indexFactory; + + @Mock + private LuceneQueryBuilderFactory queryBuilderFactory; + + @InjectMocks + private LuceneSearchEngine searchEngine; + + @Test + void shouldDelegateGetSearchableTypes() { + List mockedTypes = Collections.singletonList(searchableType("repository")); + when(resolver.getSearchableTypes()).thenReturn(mockedTypes); + + Collection searchableTypes = searchEngine.getSearchableTypes(); + + assertThat(searchableTypes).containsAll(mockedTypes); + } + + @Test + @SubjectAware(value = "dent", permissions = "user:list") + void shouldExcludeTypesWithoutPermission() { + LuceneSearchableType repository = searchableType("repository"); + LuceneSearchableType user = searchableType("user", "user:list"); + LuceneSearchableType group = searchableType("group", "group:list"); + List mockedTypes = Arrays.asList(repository, user, group); + when(resolver.getSearchableTypes()).thenReturn(mockedTypes); + + Collection searchableTypes = searchEngine.getSearchableTypes(); + + assertThat(searchableTypes).containsOnly(repository, user); + } + + private LuceneSearchableType searchableType(String name) { + return searchableType(name, null); + } + + private LuceneSearchableType searchableType(String name, String permission) { + LuceneSearchableType searchableType = mock(LuceneSearchableType.class); + lenient().when(searchableType.getName()).thenReturn(name); + when(searchableType.getPermission()).thenReturn(Optional.ofNullable(permission)); + return searchableType; + } + + @Test + void shouldDelegateGetOrCreateIndexWithDefaults() { + LuceneIndex index = mock(LuceneIndex.class); + when(indexFactory.create("idx", IndexOptions.defaults())).thenReturn(index); + + Index idx = searchEngine.getOrCreate("idx"); + assertThat(idx).isSameAs(index); + } + + @Test + void shouldDelegateGetOrCreateIndex() { + LuceneIndex index = mock(LuceneIndex.class); + IndexOptions options = IndexOptions.naturalLanguage(Locale.ENGLISH); + when(indexFactory.create("idx", options)).thenReturn(index); + + Index idx = searchEngine.getOrCreate("idx", options); + assertThat(idx).isSameAs(index); + } + + @Test + void shouldDelegateSearchWithDefaults() { + LuceneQueryBuilder mockedBuilder = mock(LuceneQueryBuilder.class); + when(queryBuilderFactory.create("idx", IndexOptions.defaults())).thenReturn(mockedBuilder); + + QueryBuilder queryBuilder = searchEngine.search("idx"); + + assertThat(queryBuilder).isSameAs(mockedBuilder); + } + + @Test + void shouldDelegateSearch() { + LuceneQueryBuilder mockedBuilder = mock(LuceneQueryBuilder.class); + IndexOptions options = IndexOptions.naturalLanguage(Locale.GERMAN); + when(queryBuilderFactory.create("idx", options)).thenReturn(mockedBuilder); + + QueryBuilder queryBuilder = searchEngine.search("idx", options); + + assertThat(queryBuilder).isSameAs(mockedBuilder); + } + +}