Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name matches at the beginning of search. #8085

Merged
merged 2 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: '),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we detect if the match is exact, and in that case write 'Exact package name match:'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

...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
Loading