Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
36 changes: 27 additions & 9 deletions app/lib/search/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@ import 'package:meta/meta.dart';
// ignore: implementation_imports
import 'package:pana/src/dartdoc/pub_dartdoc_data.dart';
import 'package:pool/pool.dart';
import 'package:pub_dev/publisher/backend.dart';

import 'package:pub_dev/search/search_client.dart';
import 'package:pub_dev/service/download_counts/backend.dart';
import 'package:pub_dev/shared/popularity_storage.dart';
import 'package:pub_dev/shared/redis_cache.dart';
import 'package:pub_dev/shared/utils.dart';
import 'package:retry/retry.dart';

import '../../publisher/backend.dart';
import '../../service/download_counts/backend.dart';
import '../../service/topics/models.dart';
import '../../shared/popularity_storage.dart';
import '../../shared/redis_cache.dart';
import '../../shared/utils.dart';
import '../package/backend.dart';
import '../package/model_properties.dart';
import '../package/models.dart';
Expand All @@ -48,6 +46,7 @@ import 'dart_sdk_mem_index.dart';
import 'flutter_sdk_mem_index.dart';
import 'models.dart';
import 'result_combiner.dart';
import 'search_client.dart';
import 'search_service.dart';
import 'text_utils.dart';

Expand Down Expand Up @@ -549,9 +548,28 @@ SearchForm? canonicalizeSearchForm(SearchForm form) {
}
if (newTags != null) {
return form.change(query: query.change(tagsPredicate: newTags).toString());
} else {
return null;
}

final newQueryText = form.parsedQuery.text?.split(' ').map((p) {
if (p.startsWith('#') && p.length > 1) {
final topic = p.substring(1);
// Checking the surface format, and skipping the change if the
// text would be an invalid topic.
if (!isValidTopicFormat(topic)) {
return p;
}
// NOTE: We don't know if this topic exists or spelled correctly.
// We should consider restricting the updates to existing
// topics only (TBD).
return 'topic:$topic';
}
return p;
}).join(' ');
if (newQueryText != form.parsedQuery.text) {
return form.change(query: newQueryText);
}

return null;
}

/// Creates the index-related API data structure from the extracted dartdoc data.
Expand Down
2 changes: 0 additions & 2 deletions app/lib/service/topics/models.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import 'dart:io';

import 'package:json_annotation/json_annotation.dart';
import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;
import 'package:pub_dev/frontend/static_files.dart';
import 'package:yaml/yaml.dart';
Expand Down Expand Up @@ -92,7 +91,6 @@ class CanonicalTopicFileContent {
}

/// True, if [topic] is formatted like a valid topic.
@visibleForTesting
bool isValidTopicFormat(String topic) =>
RegExp(r'^[a-z0-9-]{2,32}$').hasMatch(topic) &&
!topic.contains('--') &&
Expand Down
14 changes: 14 additions & 0 deletions app/test/frontend/handlers/redirects_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -117,5 +117,19 @@ void main() {
'https://api.flutter.dev/',
);
});

testWithProfile('search canonicalization: topic name', fn: () async {
await expectRedirectResponse(
await issueGet('/packages?q=topic%3Awidgets'),
'/packages?q=topic%3Awidget',
);
});

testWithProfile('search canonicalization: topic shortcut', fn: () async {
await expectRedirectResponse(
await issueGet('/packages?q=%23hash'),
'/packages?q=topic%3Ahash',
);
});
});
}
7 changes: 7 additions & 0 deletions app/test/search/backend_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,12 @@ void main() {
'topic:widget abc',
);
});

test('query with topic shortcut `#`', () {
expect(
canonicalizeSearchForm(_parse('#widget #testing'))?.query,
'topic:widget topic:testing',
);
Copy link
Member

Choose a reason for hiding this comment

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

Can we have a test case where I search for "#hash" -- and would that work if I actually want to search for a keyword that starts with # ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

# is by default removed from the content at indexing and from queries (used as a separator).

The end result of this change is that #hash search from https://pub.dev/packages?q=%23hash gets redirected to https://pub.dev/packages?q=topic%3Ahash and yeah, we can add a test for that.

});
});
}
Loading