diff --git a/app/lib/frontend/handlers/listing.dart b/app/lib/frontend/handlers/listing.dart index eda837b2ba..0315b98434 100644 --- a/app/lib/frontend/handlers/listing.dart +++ b/app/lib/frontend/handlers/listing.dart @@ -81,8 +81,7 @@ Future _packagesHandlerHtmlCore(shelf.Request request) async { ); final int totalCount = searchResult.totalCount; final errorMessage = searchResult.errorMessage; - final statusCode = - searchResult.statusCode ?? (errorMessage == null ? 200 : 500); + final statusCode = searchResult.statusCode; if (errorMessage != null && statusCode >= 500) { _logger.severe('[pub-search-not-working] ${searchResult.errorMessage}'); } @@ -93,7 +92,6 @@ Future _packagesHandlerHtmlCore(shelf.Request request) async { searchResult, links, searchForm: searchForm, - messageFromBackend: searchResult.errorMessage, openSections: openSections, ), status: statusCode, diff --git a/app/lib/frontend/templates/listing.dart b/app/lib/frontend/templates/listing.dart index dc72258c71..bf45c234d5 100644 --- a/app/lib/frontend/templates/listing.dart +++ b/app/lib/frontend/templates/listing.dart @@ -5,9 +5,11 @@ import 'dart:math'; import 'package:_pub_shared/search/search_form.dart'; +import 'package:collection/collection.dart'; import '../../package/search_adapter.dart'; import '../../search/search_service.dart'; +import '../../shared/urls.dart' as urls; import '../dom/dom.dart' as d; import '_consts.dart'; @@ -35,7 +37,6 @@ String renderPkgIndexPage( SearchResultPage searchResultPage, PageLinks links, { required SearchForm searchForm, - String? messageFromBackend, Set? openSections, }) { final topPackages = getSdkDict(null).topSdkPackages; @@ -47,8 +48,9 @@ String renderPkgIndexPage( searchForm: searchForm, totalCount: searchResultPage.totalCount, title: topPackages, - messageFromBackend: messageFromBackend, + messageFromBackend: searchResultPage.errorMessage, ), + nameMatches: _nameMatches(searchForm, searchResultPage.nameMatches), packageList: packageList(searchResultPage), pagination: searchResultPage.hasHit ? paginationNode(links) : null, openSections: openSections, @@ -121,3 +123,24 @@ class PageLinks { return min(fromSymmetry, max(currentPage!, fromCount)); } } + +d.Node? _nameMatches(SearchForm form, List? matches) { + if (matches == null || matches.isEmpty) { + return null; + } + final singular = matches.length == 1; + final isExactNameMatch = singular && form.parsedQuery.text == matches.single; + final nameMatchLabel = isExactNameMatch + ? 'Exact package name match: ' + : 'Matching package ${singular ? 'name' : 'names'}: '; + + return d.p(children: [ + d.b(text: nameMatchLabel), + ...matches.expandIndexed((i, name) { + return [ + if (i > 0) d.text(', '), + d.code(child: d.a(href: urls.pkgPageUrl(name), text: name)), + ]; + }), + ]); +} diff --git a/app/lib/frontend/templates/views/pkg/index.dart b/app/lib/frontend/templates/views/pkg/index.dart index f74fae78b6..98e802f4f2 100644 --- a/app/lib/frontend/templates/views/pkg/index.dart +++ b/app/lib/frontend/templates/views/pkg/index.dart @@ -13,12 +13,14 @@ import '../../../static_files.dart'; d.Node packageListingNode({ required SearchForm searchForm, required d.Node listingInfo, + required d.Node? nameMatches, required d.Node packageList, required d.Node? pagination, required Set? openSections, }) { final innerContent = d.fragment([ listingInfo, + if (nameMatches != null) nameMatches, packageList, if (pagination != null) pagination, d.markdown('Check our help page for details on ' diff --git a/app/lib/package/search_adapter.dart b/app/lib/package/search_adapter.dart index 79438ec54b..a4d019dc7b 100644 --- a/app/lib/package/search_adapter.dart +++ b/app/lib/package/search_adapter.dart @@ -42,11 +42,12 @@ class SearchAdapter { return SearchResultPage( form, result.totalCount, + nameMatches: result.nameMatches, sdkLibraryHits: result.sdkLibraryHits, packageHits: result.packageHits.map((h) => views[h.package]).nonNulls.toList(), errorMessage: result.errorMessage, - statusCode: result.statusCode, + statusCode: result.statusCode ?? 200, ); } @@ -84,8 +85,10 @@ class SearchAdapter { Future _fallbackSearch(SearchForm form) async { // Some search queries must not be served with the fallback search. if (form.parsedQuery.tagsPredicate.isNotEmpty) { - return PackageSearchResult.empty( - errorMessage: 'Search is temporarily unavailable.'); + return PackageSearchResult.error( + errorMessage: 'Search is temporarily unavailable.', + statusCode: 503, + ); } final names = await nameTracker @@ -108,11 +111,13 @@ class SearchAdapter { packageHits = packageHits.skip(form.offset).take(form.pageSize ?? 10).toList(); return PackageSearchResult( - timestamp: clock.now().toUtc(), - packageHits: packageHits, - totalCount: totalCount, - errorMessage: - 'Search is temporarily impaired, filtering and ranking may be incorrect.'); + timestamp: clock.now().toUtc(), + packageHits: packageHits, + totalCount: totalCount, + errorMessage: + 'Search is temporarily impaired, filtering and ranking may be incorrect.', + statusCode: 503, + ); } Future> _getPackageViewsFromHits( @@ -137,6 +142,11 @@ class SearchResultPage { /// The total number of results available for the search. final int totalCount; + /// Package names that are exact name matches or close to (e.g. names that + /// would be considered as blocker for publishing). + final List? nameMatches; + + /// The hits from the SDK libraries. final List sdkLibraryHits; /// The current list of packages on the page. @@ -146,24 +156,20 @@ class SearchResultPage { /// the query was not processed entirely. final String? errorMessage; - /// The non-200 status code that will be used to render the [errorMessage]. - final int? statusCode; + /// The code that will be used to render the page. + final int statusCode; SearchResultPage( this.form, this.totalCount, { + this.nameMatches, List? sdkLibraryHits, List? packageHits, this.errorMessage, - this.statusCode, + this.statusCode = 200, }) : sdkLibraryHits = sdkLibraryHits ?? [], packageHits = packageHits ?? []; - SearchResultPage.empty(this.form, {this.errorMessage, this.statusCode}) - : totalCount = 0, - sdkLibraryHits = [], - packageHits = []; - bool get hasNoHit => sdkLibraryHits.isEmpty && packageHits.isEmpty; bool get hasHit => !hasNoHit; } diff --git a/app/lib/search/mem_index.dart b/app/lib/search/mem_index.dart index fa36ff724d..5ccf43a11f 100644 --- a/app/lib/search/mem_index.dart +++ b/app/lib/search/mem_index.dart @@ -147,6 +147,7 @@ class InMemoryPackageIndex { packages.removeWhere((x) => !keys.contains(x)); } + List? nameMatches; late List packageHits; switch (query.effectiveOrder ?? SearchOrder.top) { case SearchOrder.top: @@ -162,12 +163,10 @@ class InMemoryPackageIndex { .map((key, value) => value * _adjustedOverallScores[key]!); // If the search hits have an exact name match, we move it to the front of the result list. final parsedQueryText = query.parsedQuery.text; - final priorityPackageName = - packages.contains(parsedQueryText ?? '') ? parsedQueryText : null; - packageHits = _rankWithValues( - overallScore.getValues(), - priorityPackageName: priorityPackageName, - ); + if (parsedQueryText != null && _packages.containsKey(parsedQueryText)) { + nameMatches = [parsedQueryText]; + } + packageHits = _rankWithValues(overallScore.getValues()); break; case SearchOrder.text: final score = textResults?.pkgScore ?? Score.empty(); @@ -209,6 +208,7 @@ class InMemoryPackageIndex { return PackageSearchResult( timestamp: clock.now().toUtc(), totalCount: totalCount, + nameMatches: nameMatches, packageHits: packageHits, ); } @@ -333,16 +333,11 @@ class InMemoryPackageIndex { return null; } - List _rankWithValues( - Map values, { - String? priorityPackageName, - }) { + List _rankWithValues(Map values) { final list = values.entries .map((e) => PackageHit(package: e.key, score: e.value)) .toList(); list.sort((a, b) { - if (a.package == priorityPackageName) return -1; - if (b.package == priorityPackageName) return 1; final int scoreCompare = -a.score!.compareTo(b.score!); if (scoreCompare != 0) return scoreCompare; // if two packages got the same score, order by last updated diff --git a/app/lib/search/result_combiner.dart b/app/lib/search/result_combiner.dart index 501fccd20c..e6b94c3c9e 100644 --- a/app/lib/search/result_combiner.dart +++ b/app/lib/search/result_combiner.dart @@ -48,10 +48,7 @@ class SearchResultCombiner { sdkLibraryHits.sort((a, b) => -a.score.compareTo(b.score)); } - return PackageSearchResult( - timestamp: primaryResult.timestamp, - totalCount: primaryResult.totalCount, - packageHits: primaryResult.packageHits, + return primaryResult.change( sdkLibraryHits: sdkLibraryHits.take(3).toList(), ); } diff --git a/app/lib/search/search_client.dart b/app/lib/search/search_client.dart index 7064460b1c..36c5bd25bb 100644 --- a/app/lib/search/search_client.dart +++ b/app/lib/search/search_client.dart @@ -52,7 +52,7 @@ class SearchClient { // check validity first final validity = query.evaluateValidity(); if (validity.isRejected) { - return PackageSearchResult.empty( + return PackageSearchResult.error( errorMessage: 'Search query rejected. ${validity.rejectReason}', statusCode: 400, ); @@ -87,8 +87,10 @@ class SearchClient { } } if (response == null) { - return PackageSearchResult.empty( - errorMessage: 'Search is temporarily unavailable.'); + return PackageSearchResult.error( + errorMessage: 'Search is temporarily unavailable.', + statusCode: 503, + ); } if (response.statusCode == 200) { return PackageSearchResult.fromJson( @@ -97,12 +99,16 @@ class SearchClient { } // Search request before the service initialization completed. if (response.statusCode == searchIndexNotReadyCode) { - return PackageSearchResult.empty( - errorMessage: 'Search is temporarily unavailable.'); + return PackageSearchResult.error( + errorMessage: 'Search is temporarily unavailable.', + statusCode: 503, + ); } // There has been a generic issue with the service. - return PackageSearchResult.empty( - errorMessage: 'Service returned status code ${response.statusCode}.'); + return PackageSearchResult.error( + errorMessage: 'Service returned status code ${response.statusCode}.', + statusCode: response.statusCode, + ); } if (sourceIp != null) { diff --git a/app/lib/search/search_service.dart b/app/lib/search/search_service.dart index 0b166e627f..9c21b97720 100644 --- a/app/lib/search/search_service.dart +++ b/app/lib/search/search_service.dart @@ -303,8 +303,12 @@ class QueryValidity { @JsonSerializable(includeIfNull: false, explicitToJson: true) class PackageSearchResult { - final DateTime? timestamp; + final DateTime timestamp; final int totalCount; + + /// Package names that are exact name matches or close to (e.g. names that + /// would be considered as blocker for publishing). + final List? nameMatches; final List sdkLibraryHits; final List packageHits; @@ -317,32 +321,46 @@ class PackageSearchResult { PackageSearchResult({ required this.timestamp, required this.totalCount, + this.nameMatches, List? sdkLibraryHits, List? packageHits, this.errorMessage, - this.statusCode, - }) : sdkLibraryHits = sdkLibraryHits ?? [], - packageHits = packageHits ?? []; - - PackageSearchResult.empty({this.errorMessage, this.statusCode}) - : timestamp = clock.now().toUtc(), + int? statusCode, + }) : packageHits = packageHits ?? [], + sdkLibraryHits = sdkLibraryHits ?? [], + statusCode = statusCode; + + PackageSearchResult.error({ + required this.errorMessage, + required this.statusCode, + }) : timestamp = clock.now().toUtc(), totalCount = 0, + nameMatches = null, sdkLibraryHits = [], packageHits = []; - factory PackageSearchResult.fromJson(Map json) { - return _$PackageSearchResultFromJson({ - // TODO: remove fallback in the next release - 'errorMessage': json['message'], - ...json, - }); - } + factory PackageSearchResult.fromJson(Map json) => + _$PackageSearchResultFromJson(json); - Duration get age => clock.now().difference(timestamp!); + Duration get age => clock.now().difference(timestamp); Map toJson() => _$PackageSearchResultToJson(this); bool get isEmpty => packageHits.isEmpty && sdkLibraryHits.isEmpty; + + PackageSearchResult change({ + List? sdkLibraryHits, + }) { + return PackageSearchResult( + timestamp: timestamp, + totalCount: totalCount, + nameMatches: nameMatches, + sdkLibraryHits: sdkLibraryHits ?? this.sdkLibraryHits, + packageHits: packageHits, + errorMessage: errorMessage, + statusCode: statusCode, + ); + } } @JsonSerializable(includeIfNull: false, explicitToJson: true) diff --git a/app/lib/search/search_service.g.dart b/app/lib/search/search_service.g.dart index 36b5e41b8e..3281e789f3 100644 --- a/app/lib/search/search_service.g.dart +++ b/app/lib/search/search_service.g.dart @@ -75,10 +75,11 @@ Map _$ApiDocPageToJson(ApiDocPage instance) => PackageSearchResult _$PackageSearchResultFromJson(Map json) => PackageSearchResult( - timestamp: json['timestamp'] == null - ? null - : DateTime.parse(json['timestamp'] as String), + timestamp: DateTime.parse(json['timestamp'] as String), totalCount: (json['totalCount'] as num).toInt(), + nameMatches: (json['nameMatches'] as List?) + ?.map((e) => e as String) + .toList(), sdkLibraryHits: (json['sdkLibraryHits'] as List?) ?.map((e) => SdkLibraryHit.fromJson(e as Map)) .toList(), @@ -90,7 +91,10 @@ PackageSearchResult _$PackageSearchResultFromJson(Map json) => ); Map _$PackageSearchResultToJson(PackageSearchResult instance) { - final val = {}; + final val = { + 'timestamp': instance.timestamp.toIso8601String(), + 'totalCount': instance.totalCount, + }; void writeNotNull(String key, dynamic value) { if (value != null) { @@ -98,8 +102,7 @@ Map _$PackageSearchResultToJson(PackageSearchResult instance) { } } - writeNotNull('timestamp', instance.timestamp?.toIso8601String()); - val['totalCount'] = instance.totalCount; + writeNotNull('nameMatches', instance.nameMatches); val['sdkLibraryHits'] = instance.sdkLibraryHits.map((e) => e.toJson()).toList(); val['packageHits'] = instance.packageHits.map((e) => e.toJson()).toList(); diff --git a/app/lib/service/entrypoint/search_index.dart b/app/lib/service/entrypoint/search_index.dart index 916ee0eff6..c8c3245d61 100644 --- a/app/lib/service/entrypoint/search_index.dart +++ b/app/lib/service/entrypoint/search_index.dart @@ -131,7 +131,9 @@ class IsolateSearchIndex implements SearchIndex { } catch (e, st) { _logger.warning('Failed to search index.', e, st); } - return PackageSearchResult.empty( - errorMessage: 'Failed to process request.'); + return PackageSearchResult.error( + errorMessage: 'Failed to process request.', + statusCode: 500, + ); } } diff --git a/app/test/frontend/handlers/listing_test.dart b/app/test/frontend/handlers/listing_test.dart index 6d791b43bd..a9ba36d497 100644 --- a/app/test/frontend/handlers/listing_test.dart +++ b/app/test/frontend/handlers/listing_test.dart @@ -77,8 +77,9 @@ void main() { SearchClient(MockClient((_) async => throw Exception()))); await nameTracker.reloadFromDatastore(); final content = await expectHtmlResponse( - await issueGet('/packages?q=oxyge'), - status: 500); + await issueGet('/packages?q=oxyge'), + status: 503, + ); expect(content, contains('oxygen is awesome')); }); diff --git a/app/test/search/handlers_test.dart b/app/test/search/handlers_test.dart index ef413fff5b..c6b163319f 100644 --- a/app/test/search/handlers_test.dart +++ b/app/test/search/handlers_test.dart @@ -33,6 +33,7 @@ void main() { await expectJsonResponse(await issueGet('/search?q=oxygen'), body: { 'timestamp': isNotNull, 'totalCount': 1, + 'nameMatches': ['oxygen'], 'sdkLibraryHits': [], 'packageHits': [ {'package': 'oxygen', 'score': isPositive}, diff --git a/app/test/search/mem_index_test.dart b/app/test/search/mem_index_test.dart index cd673950ec..8ab9314343 100644 --- a/app/test/search/mem_index_test.dart +++ b/app/test/search/mem_index_test.dart @@ -91,6 +91,7 @@ server.dart adds a small, prescriptive server (PicoServer) that can be configure expect(json.decode(json.encode(result)), { 'timestamp': isNotNull, 'totalCount': 1, + 'nameMatches': ['async'], 'sdkLibraryHits': [], 'packageHits': [ {'package': 'async', 'score': closeTo(0.65, 0.01)}, @@ -594,7 +595,7 @@ server.dart adds a small, prescriptive server (PicoServer) that can be configure description: 'def xyz', maxPoints: 100, grantedPoints: 0, - tags: ['sdk:dart'], + tags: ['sdk:dart', 'sdk:flutter'], ), PackageDocument( package: 'def', @@ -623,11 +624,12 @@ server.dart adds a small, prescriptive server (PicoServer) that can be configure (index.search(ServiceSearchQuery.parse(query: 'abc'))).toJson(), { 'timestamp': isNotEmpty, 'totalCount': 2, + 'nameMatches': ['abc'], 'sdkLibraryHits': [], 'packageHits': [ - // `abc` is first, despite its lower score - {'package': 'abc', 'score': closeTo(0.48, 0.01)}, + // `abc` is at its natural place {'package': 'def', 'score': closeTo(0.69, 0.01)}, + {'package': 'abc', 'score': closeTo(0.48, 0.01)}, ] }); // exact name match with tags @@ -637,10 +639,25 @@ server.dart adds a small, prescriptive server (PicoServer) that can be configure { 'timestamp': isNotEmpty, 'totalCount': 2, + 'nameMatches': ['abc'], 'sdkLibraryHits': [], 'packageHits': [ - // `abc` is first, despite its lower score + // `abc` is at its natural place + {'package': 'def', 'score': closeTo(0.69, 0.01)}, {'package': 'abc', 'score': closeTo(0.48, 0.01)}, + ] + }); + // absent exact name match with tags + expect( + (index.search(ServiceSearchQuery.parse(query: 'abc -sdk:flutter'))) + .toJson(), + { + 'timestamp': isNotEmpty, + 'totalCount': 1, + 'nameMatches': ['abc'], + 'sdkLibraryHits': [], + 'packageHits': [ + // `abc` is not present in the package list {'package': 'def', 'score': closeTo(0.69, 0.01)}, ] });