diff --git a/CHANGELOG.md b/CHANGELOG.md index e08c17266..eccf2348f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Important changes to data models, configuration, and migrations between each AppEngine version, listed here to ease deployment and troubleshooting. ## Next Release (replace with git tag when deployed) + * Note: Internal `search` instance started accepting request through `POST /search`. ## `20250812t135400-all` * Bump runtimeVersion to `2025.08.12`. diff --git a/app/lib/frontend/handlers/experimental.dart b/app/lib/frontend/handlers/experimental.dart index c96745e0f..a3e476c2e 100644 --- a/app/lib/frontend/handlers/experimental.dart +++ b/app/lib/frontend/handlers/experimental.dart @@ -18,6 +18,7 @@ const _publicFlags = { final _allFlags = { 'dark-as-default', + 'search-post', ..._publicFlags.map((x) => x.name), }; @@ -92,6 +93,8 @@ class ExperimentalFlags { bool get isDarkModeDefault => isEnabled('dark-as-default'); + bool get useSearchPost => isEnabled('search-post'); + bool get showTrending => true; String encodedAsCookie() => _enabled.join(':'); diff --git a/app/lib/search/handlers.dart b/app/lib/search/handlers.dart index 9eff853dc..5f7661cd7 100644 --- a/app/lib/search/handlers.dart +++ b/app/lib/search/handlers.dart @@ -3,9 +3,12 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; +import 'package:_pub_shared/search/search_request_data.dart'; import 'package:logging/logging.dart'; import 'package:shelf/shelf.dart' as shelf; +import 'package:shelf_router/shelf_router.dart'; import '../shared/env_config.dart'; import '../shared/handlers.dart'; @@ -18,28 +21,22 @@ final Duration _slowSearchThreshold = const Duration(milliseconds: 200); /// Handlers for the search service. Future searchServiceHandler(shelf.Request request) async { - final path = request.requestedUri.path; - final handler = { - '/debug': _debugHandler, - '/liveness_check': _livenessCheckHandler, - '/readiness_check': _readinessCheckHandler, - '/search': _searchHandler, - '/robots.txt': rejectRobotsHandler, - }[path]; - - if (handler != null) { - return await handler(request); - } else { - return notFoundHandler(request); - } + final router = Router(notFoundHandler: notFoundHandler) + ..get('/debug', _debugHandler) + ..get('/liveness_check', _livenessCheckHandler) + ..get('/readiness_check', _readinessCheckHandler) + ..get('/search', _searchHandler) + ..post('/search', _searchHandler) + ..get('/robots.txt', rejectRobotsHandler); + return await router.call(request); } -/// Handles /liveness_check requests. +/// Handles GET /liveness_check requests. Future _livenessCheckHandler(shelf.Request request) async { return htmlResponse('OK'); } -/// Handles /readiness_check requests. +/// Handles GET /readiness_check requests. Future _readinessCheckHandler(shelf.Request request) async { if (await searchIndex.isReady()) { return htmlResponse('OK'); @@ -48,13 +45,14 @@ Future _readinessCheckHandler(shelf.Request request) async { } } -/// Handler /debug requests +/// Handler GET /debug requests Future _debugHandler(shelf.Request request) async { final info = await searchIndex.indexInfo(); return debugResponse(info.toJson()); } -/// Handles /search requests. +/// Handles GET /search requests. +/// Handles POST /search requests. Future _searchHandler(shelf.Request request) async { final info = await searchIndex.indexInfo(); if (!info.isReady) { @@ -62,7 +60,13 @@ Future _searchHandler(shelf.Request request) async { status: searchIndexNotReadyCode); } final Stopwatch sw = Stopwatch()..start(); - final query = ServiceSearchQuery.fromServiceUrl(request.requestedUri); + final query = request.method == 'POST' + ? ServiceSearchQuery.fromSearchRequestData( + SearchRequestData.fromJson( + json.decode(await request.readAsString()) as Map, + ), + ) + : ServiceSearchQuery.fromServiceUrl(request.requestedUri); final result = await searchIndex.search(query); final Duration elapsed = sw.elapsed; if (elapsed > _slowSearchThreshold) { diff --git a/app/lib/search/mem_index.dart b/app/lib/search/mem_index.dart index dcea8f20f..4dbec6e02 100644 --- a/app/lib/search/mem_index.dart +++ b/app/lib/search/mem_index.dart @@ -5,6 +5,7 @@ import 'dart:math' as math; import 'package:_pub_shared/search/search_form.dart'; +import 'package:_pub_shared/search/search_request_data.dart'; import 'package:clock/clock.dart'; import 'package:collection/collection.dart'; import 'package:logging/logging.dart'; @@ -287,8 +288,6 @@ class InMemoryPackageIndex { case SearchOrder.updated: indexedHits = _updatedOrderedHits.whereInScores(selectFn); break; - // ignore: deprecated_member_use - case SearchOrder.popularity: case SearchOrder.downloads: indexedHits = _downloadsOrderedHits.whereInScores(selectFn); break; diff --git a/app/lib/search/search_client.dart b/app/lib/search/search_client.dart index a87fa37bb..ced8690ba 100644 --- a/app/lib/search/search_client.dart +++ b/app/lib/search/search_client.dart @@ -8,6 +8,7 @@ import 'dart:convert'; import 'package:_pub_shared/utils/http.dart'; import 'package:clock/clock.dart'; import 'package:gcloud/service_scope.dart' as ss; +import 'package:pub_dev/frontend/request_context.dart'; import '../../../service/rate_limit/rate_limit.dart'; import '../shared/configuration.dart'; @@ -69,8 +70,33 @@ class SearchClient { Future<({int statusCode, String? body})?> doCallHttpServiceEndpoint( {String? prefix}) async { final httpHostPort = prefix ?? activeConfiguration.searchServicePrefix; - final serviceUrl = '$httpHostPort/search$serviceUrlParams'; try { + if (requestContext.experimentalFlags.useSearchPost) { + return await withRetryHttpClient( + (client) async { + final data = query.toSearchRequestData(); + // NOTE: Keeping the query parameter to help investigating logs. + final uri = Uri.parse('$httpHostPort/search').replace( + queryParameters: { + 'q': data.query, + }, + ); + final rs = await client.post( + uri, + headers: { + ...?cloudTraceHeaders(), + 'content-type': 'application/json', + }, + body: json.encode(data.toJson()), + ); + return (statusCode: rs.statusCode, body: rs.body); + }, + client: _httpClient, + retryIf: (e) => (e is UnexpectedStatusException && + e.statusCode == searchIndexNotReadyCode), + ); + } + final serviceUrl = '$httpHostPort/search$serviceUrlParams'; return await httpGetWithRetry( Uri.parse(serviceUrl), client: _httpClient, diff --git a/app/lib/search/search_service.dart b/app/lib/search/search_service.dart index 3277055d9..a08e0db57 100644 --- a/app/lib/search/search_service.dart +++ b/app/lib/search/search_service.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:math' show max; import 'package:_pub_shared/search/search_form.dart'; +import 'package:_pub_shared/search/search_request_data.dart'; import 'package:_pub_shared/search/tags.dart'; import 'package:clock/clock.dart'; import 'package:collection/collection.dart'; @@ -240,6 +241,20 @@ class ServiceSearchQuery { ); } + factory ServiceSearchQuery.fromSearchRequestData(SearchRequestData data) { + final tagsPredicate = TagsPredicate.parseQueryValues(data.tags); + return ServiceSearchQuery.parse( + query: data.query, + tagsPredicate: tagsPredicate, + publisherId: data.publisherId, + order: data.order, + minPoints: data.minPoints, + offset: data.offset ?? 0, + limit: data.limit, + textMatchExtent: data.textMatchExtent, + ); + } + ServiceSearchQuery change({ String? query, TagsPredicate? tagsPredicate, @@ -310,38 +325,19 @@ class ServiceSearchQuery { return QueryValidity.accept(); } -} - -/// The scope (depth) of the text matching. -enum TextMatchExtent { - /// No text search is done. - /// Requests with text queries will return a failure message. - none, - - /// Text search is on package names. - name, - - /// Text search is on package names, descriptions and topic tags. - description, - - /// Text search is on names, descriptions, topic tags and readme content. - readme, - - /// Text search is on names, descriptions, topic tags, readme content and API symbols. - api, - ; - - /// Text search is on package names. - bool shouldMatchName() => index >= name.index; - /// Text search is on package names, descriptions and topic tags. - bool shouldMatchDescription() => index >= description.index; - - /// Text search is on names, descriptions, topic tags and readme content. - bool shouldMatchReadme() => index >= readme.index; - - /// Text search is on names, descriptions, topic tags, readme content and API symbols. - bool shouldMatchApi() => index >= api.index; + SearchRequestData toSearchRequestData() { + return SearchRequestData( + query: query, + tags: tagsPredicate.toQueryParameters(), + publisherId: publisherId, + minPoints: minPoints, + order: order, + offset: offset, + limit: limit, + textMatchExtent: textMatchExtent, + ); + } } class QueryValidity { diff --git a/app/lib/service/entrypoint/search_index.dart b/app/lib/service/entrypoint/search_index.dart index dc22d5666..f9663140f 100644 --- a/app/lib/service/entrypoint/search_index.dart +++ b/app/lib/service/entrypoint/search_index.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; +import 'package:_pub_shared/search/search_request_data.dart'; import 'package:args/args.dart'; import 'package:gcloud/service_scope.dart'; import 'package:logging/logging.dart'; diff --git a/app/test/search/handlers_test.dart b/app/test/search/handlers_test.dart index 29986d85a..c22ac05b3 100644 --- a/app/test/search/handlers_test.dart +++ b/app/test/search/handlers_test.dart @@ -4,6 +4,7 @@ import 'dart:async'; +import 'package:_pub_shared/search/search_request_data.dart'; import 'package:html/parser.dart' as html_parser; import 'package:test/test.dart'; @@ -38,6 +39,21 @@ void main() { {'package': 'oxygen', 'score': isPositive}, ], }); + + await expectJsonResponse( + await issuePost( + '/search', + body: SearchRequestData(query: 'oxygen').toJson(), + ), + body: { + 'timestamp': isNotNull, + 'totalCount': 1, + 'sdkLibraryHits': [], + 'packageHits': [ + {'package': 'oxygen', 'score': isPositive}, + ], + }, + ); }); testWithProfile('Finds text in description or readme', fn: () async { diff --git a/pkg/_pub_shared/build.yaml b/pkg/_pub_shared/build.yaml index 1739d7761..2ac74fded 100644 --- a/pkg/_pub_shared/build.yaml +++ b/pkg/_pub_shared/build.yaml @@ -16,4 +16,6 @@ targets: - 'lib/data/publisher_api.dart' - 'lib/data/task_api.dart' - 'lib/data/task_payload.dart' + - 'lib/search/search_form.dart' + - 'lib/search/search_request_data.dart' - 'lib/utils/flutter_archive.dart' diff --git a/pkg/_pub_shared/lib/search/search_form.dart b/pkg/_pub_shared/lib/search/search_form.dart index bbf8895f7..9083bdc66 100644 --- a/pkg/_pub_shared/lib/search/search_form.dart +++ b/pkg/_pub_shared/lib/search/search_form.dart @@ -49,13 +49,6 @@ enum SearchOrder { /// Search order should be in decreasing last package updated time. updated, - /// Search order should be in decreasing popularity score. - /// WARNING: The value shouldn't be used anymore. - /// - /// TODO: remove in a future release. - @Deprecated('Popularity is no longer used.') - popularity, - /// Search order should be in decreasing download counts. downloads, diff --git a/pkg/_pub_shared/lib/search/search_request_data.dart b/pkg/_pub_shared/lib/search/search_request_data.dart new file mode 100644 index 000000000..667185fed --- /dev/null +++ b/pkg/_pub_shared/lib/search/search_request_data.dart @@ -0,0 +1,67 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:_pub_shared/search/search_form.dart'; +import 'package:json_annotation/json_annotation.dart'; + +part 'search_request_data.g.dart'; + +@JsonSerializable() +class SearchRequestData { + final String? query; + final List? tags; + final String? publisherId; + final int? minPoints; + final SearchOrder? order; + final int? offset; + final int? limit; + final TextMatchExtent? textMatchExtent; + + SearchRequestData({ + this.query, + this.tags, + this.publisherId, + this.minPoints, + this.order, + this.offset, + this.limit, + this.textMatchExtent, + }); + + factory SearchRequestData.fromJson(Map json) => + _$SearchRequestDataFromJson(json); + Map toJson() => _$SearchRequestDataToJson(this); +} + +/// The scope (depth) of the text matching. +enum TextMatchExtent { + /// No text search is done. + /// Requests with text queries will return a failure message. + none, + + /// Text search is on package names. + name, + + /// Text search is on package names, descriptions and topic tags. + description, + + /// Text search is on names, descriptions, topic tags and readme content. + readme, + + /// Text search is on names, descriptions, topic tags, readme content and API symbols. + api, + ; + + /// Text search is on package names. + bool shouldMatchName() => index >= name.index; + + /// Text search is on package names, descriptions and topic tags. + bool shouldMatchDescription() => index >= description.index; + + /// Text search is on names, descriptions, topic tags and readme content. + bool shouldMatchReadme() => index >= readme.index; + + /// Text search is on names, descriptions, topic tags, readme content and API symbols. + bool shouldMatchApi() => index >= api.index; +} diff --git a/pkg/_pub_shared/lib/search/search_request_data.g.dart b/pkg/_pub_shared/lib/search/search_request_data.g.dart new file mode 100644 index 000000000..a430707bb --- /dev/null +++ b/pkg/_pub_shared/lib/search/search_request_data.g.dart @@ -0,0 +1,51 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +part of 'search_request_data.dart'; + +// ************************************************************************** +// JsonSerializableGenerator +// ************************************************************************** + +SearchRequestData _$SearchRequestDataFromJson(Map json) => + SearchRequestData( + query: json['query'] as String?, + tags: (json['tags'] as List?)?.map((e) => e as String).toList(), + publisherId: json['publisherId'] as String?, + minPoints: (json['minPoints'] as num?)?.toInt(), + order: $enumDecodeNullable(_$SearchOrderEnumMap, json['order']), + offset: (json['offset'] as num?)?.toInt(), + limit: (json['limit'] as num?)?.toInt(), + textMatchExtent: $enumDecodeNullable( + _$TextMatchExtentEnumMap, json['textMatchExtent']), + ); + +Map _$SearchRequestDataToJson(SearchRequestData instance) => + { + 'query': instance.query, + 'tags': instance.tags, + 'publisherId': instance.publisherId, + 'minPoints': instance.minPoints, + 'order': _$SearchOrderEnumMap[instance.order], + 'offset': instance.offset, + 'limit': instance.limit, + 'textMatchExtent': _$TextMatchExtentEnumMap[instance.textMatchExtent], + }; + +const _$SearchOrderEnumMap = { + SearchOrder.top: 'top', + SearchOrder.text: 'text', + SearchOrder.created: 'created', + SearchOrder.updated: 'updated', + SearchOrder.downloads: 'downloads', + SearchOrder.like: 'like', + SearchOrder.points: 'points', + SearchOrder.trending: 'trending', +}; + +const _$TextMatchExtentEnumMap = { + TextMatchExtent.none: 'none', + TextMatchExtent.name: 'name', + TextMatchExtent.description: 'description', + TextMatchExtent.readme: 'readme', + TextMatchExtent.api: 'api', +}; diff --git a/pkg/_pub_shared/lib/utils/http.dart b/pkg/_pub_shared/lib/utils/http.dart index 81ecf0efa..fe9f1f2e3 100644 --- a/pkg/_pub_shared/lib/utils/http.dart +++ b/pkg/_pub_shared/lib/utils/http.dart @@ -60,21 +60,48 @@ Future httpGetWithRetry( /// Additional retry conditions (on top of the default ones). /// Note: check for [UnexpectedStatusException] to allow non-200 response status codes. bool Function(Exception e)? retryIf, +}) async { + return await withRetryHttpClient( + (http.Client effectiveClient) async { + var f = effectiveClient.get(uri, headers: headers); + if (perRequestTimeout != null) { + f = f.timeout(perRequestTimeout); + } + final rs = await f; + if (rs.statusCode == 200) { + return responseFn(rs); + } + throw UnexpectedStatusException(rs.statusCode, uri); + }, + client: client, + maxAttempts: maxAttempts, + retryIf: retryIf, + ); +} + +/// Creates a HTTP client and executes a HTTP request(s) with it, +/// making sure that the HTTP resources are freed after the callback +/// finishes. +/// +/// The callback is retried on the transient network errors. +Future withRetryHttpClient( + Future Function(http.Client client) fn, { + int maxAttempts = 3, + + /// The HTTP client to use. + /// + /// Note: when the client is not specified, the inner loop will create a new [http.Client] object on each retry attempt. + http.Client? client, + + /// Additional retry conditions (on top of the default ones). + bool Function(Exception e)? retryIf, }) async { return await retry( () async { final closeClient = client == null; final effectiveClient = client ?? http.Client(); try { - var f = effectiveClient.get(uri, headers: headers); - if (perRequestTimeout != null) { - f = f.timeout(perRequestTimeout); - } - final rs = await f; - if (rs.statusCode == 200) { - return responseFn(rs); - } - throw UnexpectedStatusException(rs.statusCode, uri); + return await fn(effectiveClient); } finally { if (closeClient) { effectiveClient.close();