Skip to content

Commit

Permalink
Name matches at the beginning of search.
Browse files Browse the repository at this point in the history
  • Loading branch information
isoos committed Sep 27, 2024
1 parent 78c59f7 commit 96d2412
Show file tree
Hide file tree
Showing 13 changed files with 138 additions and 73 deletions.
4 changes: 1 addition & 3 deletions app/lib/frontend/handlers/listing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ Future<shelf.Response> _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}');
}
Expand All @@ -93,7 +92,6 @@ Future<shelf.Response> _packagesHandlerHtmlCore(shelf.Request request) async {
searchResult,
links,
searchForm: searchForm,
messageFromBackend: searchResult.errorMessage,
openSections: openSections,
),
status: statusCode,
Expand Down
23 changes: 21 additions & 2 deletions app/lib/frontend/templates/listing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -35,7 +37,6 @@ String renderPkgIndexPage(
SearchResultPage searchResultPage,
PageLinks links, {
required SearchForm searchForm,
String? messageFromBackend,
Set<String>? openSections,
}) {
final topPackages = getSdkDict(null).topSdkPackages;
Expand All @@ -47,8 +48,9 @@ String renderPkgIndexPage(
searchForm: searchForm,
totalCount: searchResultPage.totalCount,
title: topPackages,
messageFromBackend: messageFromBackend,
messageFromBackend: searchResultPage.errorMessage,
),
nameMatches: _nameMatches(searchResultPage.nameMatches),
packageList: packageList(searchResultPage),
pagination: searchResultPage.hasHit ? paginationNode(links) : null,
openSections: openSections,
Expand Down Expand Up @@ -121,3 +123,20 @@ class PageLinks {
return min(fromSymmetry, max(currentPage!, fromCount));
}
}

d.Node? _nameMatches(List<String>? matches) {
if (matches == null || matches.isEmpty) {
return null;
}
final singular = matches.length == 1;
final namePluralized = singular ? 'name' : 'names';
return d.p(children: [
d.b(text: 'Matching package $namePluralized: '),
...matches.expandIndexed((i, name) {
return [
if (i > 0) d.text(', '),
d.code(child: d.a(href: urls.pkgPageUrl(name), text: name)),
];
}),
]);
}
2 changes: 2 additions & 0 deletions app/lib/frontend/templates/views/pkg/index.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>? openSections,
}) {
final innerContent = d.fragment([
listingInfo,
if (nameMatches != null) nameMatches,
packageList,
if (pagination != null) pagination,
d.markdown('Check our help page for details on '
Expand Down
38 changes: 22 additions & 16 deletions app/lib/package/search_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}

Expand Down Expand Up @@ -84,8 +85,10 @@ class SearchAdapter {
Future<PackageSearchResult> _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
Expand All @@ -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<Map<String, PackageView>> _getPackageViewsFromHits(
Expand All @@ -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<String>? nameMatches;

/// The hits from the SDK libraries.
final List<SdkLibraryHit> sdkLibraryHits;

/// The current list of packages on the page.
Expand All @@ -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<SdkLibraryHit>? sdkLibraryHits,
List<PackageView>? packageHits,
this.errorMessage,
this.statusCode,
this.statusCode = 200,
}) : sdkLibraryHits = sdkLibraryHits ?? <SdkLibraryHit>[],
packageHits = packageHits ?? <PackageView>[];

SearchResultPage.empty(this.form, {this.errorMessage, this.statusCode})
: totalCount = 0,
sdkLibraryHits = <SdkLibraryHit>[],
packageHits = [];

bool get hasNoHit => sdkLibraryHits.isEmpty && packageHits.isEmpty;
bool get hasHit => !hasNoHit;
}
19 changes: 7 additions & 12 deletions app/lib/search/mem_index.dart
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ class InMemoryPackageIndex {
packages.removeWhere((x) => !keys.contains(x));
}

List<String>? nameMatches;
late List<PackageHit> packageHits;
switch (query.effectiveOrder ?? SearchOrder.top) {
case SearchOrder.top:
Expand All @@ -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 = <String>[parsedQueryText];
}
packageHits = _rankWithValues(overallScore.getValues());
break;
case SearchOrder.text:
final score = textResults?.pkgScore ?? Score.empty();
Expand Down Expand Up @@ -209,6 +208,7 @@ class InMemoryPackageIndex {
return PackageSearchResult(
timestamp: clock.now().toUtc(),
totalCount: totalCount,
nameMatches: nameMatches,
packageHits: packageHits,
);
}
Expand Down Expand Up @@ -333,16 +333,11 @@ class InMemoryPackageIndex {
return null;
}

List<PackageHit> _rankWithValues(
Map<String, double> values, {
String? priorityPackageName,
}) {
List<PackageHit> _rankWithValues(Map<String, double> 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
Expand Down
5 changes: 1 addition & 4 deletions app/lib/search/result_combiner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
}
Expand Down
20 changes: 13 additions & 7 deletions app/lib/search/search_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down Expand Up @@ -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(
Expand All @@ -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) {
Expand Down
48 changes: 33 additions & 15 deletions app/lib/search/search_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>? nameMatches;
final List<SdkLibraryHit> sdkLibraryHits;
final List<PackageHit> packageHits;

Expand All @@ -317,32 +321,46 @@ class PackageSearchResult {
PackageSearchResult({
required this.timestamp,
required this.totalCount,
this.nameMatches,
List<SdkLibraryHit>? sdkLibraryHits,
List<PackageHit>? packageHits,
this.errorMessage,
this.statusCode,
}) : sdkLibraryHits = sdkLibraryHits ?? <SdkLibraryHit>[],
packageHits = packageHits ?? <PackageHit>[];

PackageSearchResult.empty({this.errorMessage, this.statusCode})
: timestamp = clock.now().toUtc(),
int? statusCode,
}) : packageHits = packageHits ?? <PackageHit>[],
sdkLibraryHits = sdkLibraryHits ?? <SdkLibraryHit>[],
statusCode = statusCode;

PackageSearchResult.error({
required this.errorMessage,
required this.statusCode,
}) : timestamp = clock.now().toUtc(),
totalCount = 0,
nameMatches = null,
sdkLibraryHits = <SdkLibraryHit>[],
packageHits = <PackageHit>[];

factory PackageSearchResult.fromJson(Map<String, dynamic> json) {
return _$PackageSearchResultFromJson({
// TODO: remove fallback in the next release
'errorMessage': json['message'],
...json,
});
}
factory PackageSearchResult.fromJson(Map<String, dynamic> json) =>
_$PackageSearchResultFromJson(json);

Duration get age => clock.now().difference(timestamp!);
Duration get age => clock.now().difference(timestamp);

Map<String, dynamic> toJson() => _$PackageSearchResultToJson(this);

bool get isEmpty => packageHits.isEmpty && sdkLibraryHits.isEmpty;

PackageSearchResult change({
List<SdkLibraryHit>? sdkLibraryHits,
}) {
return PackageSearchResult(
timestamp: timestamp,
totalCount: totalCount,
nameMatches: nameMatches,
sdkLibraryHits: sdkLibraryHits ?? this.sdkLibraryHits,
packageHits: packageHits,
errorMessage: errorMessage,
statusCode: statusCode,
);
}
}

@JsonSerializable(includeIfNull: false, explicitToJson: true)
Expand Down
15 changes: 9 additions & 6 deletions app/lib/search/search_service.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 96d2412

Please sign in to comment.