From b281a066312230b17ba361ddd8fb498bc28389ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Istv=C3=A1n=20So=C3=B3s?= Date: Wed, 15 Nov 2023 10:03:57 +0100 Subject: [PATCH] Hide invalid popularity scores. (#7182) --- CHANGELOG.md | 1 + app/lib/fake/backend/fake_popularity.dart | 2 +- .../templates/views/pkg/labeled_scores.dart | 8 +++++++- .../templates/views/pkg/score_tab.dart | 8 ++++++++ app/lib/search/backend.dart | 6 ++---- app/lib/search/models.dart | 13 ++++++++++++ app/lib/shared/popularity_storage.dart | 20 ++++++++++++++++++- 7 files changed, 51 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03bd684682..d16a797882 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) + * Hiding invalid popularity scores. ## `20231109t095900-all` * Resyncing all `SecurityAdvisory` entities to get `syncTime` fields backfilled. diff --git a/app/lib/fake/backend/fake_popularity.dart b/app/lib/fake/backend/fake_popularity.dart index 1492bcf98c..145d5737f5 100644 --- a/app/lib/fake/backend/fake_popularity.dart +++ b/app/lib/fake/backend/fake_popularity.dart @@ -19,5 +19,5 @@ Future generateFakePopularityValues() async { values[p.name!] = value; } // ignore: invalid_use_of_visible_for_testing_member - popularityStorage.updateValues(values); + popularityStorage.updateValues(values, invalid: false); } diff --git a/app/lib/frontend/templates/views/pkg/labeled_scores.dart b/app/lib/frontend/templates/views/pkg/labeled_scores.dart index 3f0a425eb9..14b8d62171 100644 --- a/app/lib/frontend/templates/views/pkg/labeled_scores.dart +++ b/app/lib/frontend/templates/views/pkg/labeled_scores.dart @@ -2,6 +2,8 @@ // 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_dev/shared/popularity_storage.dart'; + import '../../../dom/dom.dart' as d; d.Node labeledScoresNode({ @@ -24,7 +26,11 @@ d.Node labeledScoresNode({ ), d.div( classes: ['packages-score', 'packages-score-popularity'], - child: _labeledScore('popularity', popularity, sign: '%'), + child: _labeledScore( + 'popularity', + popularityStorage.isInvalid ? null : popularity, + sign: popularityStorage.isInvalid ? '' : '%', + ), ), ], ); diff --git a/app/lib/frontend/templates/views/pkg/score_tab.dart b/app/lib/frontend/templates/views/pkg/score_tab.dart index 1c8325f414..2eee90506f 100644 --- a/app/lib/frontend/templates/views/pkg/score_tab.dart +++ b/app/lib/frontend/templates/views/pkg/score_tab.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'package:pana/models.dart'; +import 'package:pub_dev/shared/popularity_storage.dart'; import '../../../../scorecard/models.dart' hide ReportStatus; import '../../../../shared/urls.dart' as urls; @@ -243,6 +244,13 @@ d.Node _likeKeyFigureNode(int? likeCount) { } d.Node _popularityKeyFigureNode(double? popularity) { + if (popularityStorage.isInvalid) { + return _keyFigureNode( + value: '--', + supplemental: '', + label: 'popularity', + ); + } return _keyFigureNode( value: formatScore(popularity), supplemental: '%', diff --git a/app/lib/search/backend.dart b/app/lib/search/backend.dart index af6def178d..554532bda5 100644 --- a/app/lib/search/backend.dart +++ b/app/lib/search/backend.dart @@ -169,6 +169,7 @@ class SearchBackend { return; } snapshot.updateLikeScores(); + snapshot.updatePopularityScores(); // first complete snapshot, uploading it await _snapshotStorage.uploadDataAsJsonMap(snapshot.toJson()); @@ -199,12 +200,9 @@ class SearchBackend { if (claim.valid && lastUploadedSnapshotTimestamp != snapshot.updated) { // Updates the normalized like score across all the packages. snapshot.updateLikeScores(); - // Updates all popularity values to the currently cached one, otherwise // only updated package would have been on their new values. - for (final d in snapshot.documents!.values) { - d.popularityScore = popularityStorage.lookup(d.package); - } + snapshot.updatePopularityScores(); await _snapshotStorage.uploadDataAsJsonMap(snapshot.toJson()); lastUploadedSnapshotTimestamp = snapshot.updated!; diff --git a/app/lib/search/models.dart b/app/lib/search/models.dart index 527db1f2ed..bf20e059d4 100644 --- a/app/lib/search/models.dart +++ b/app/lib/search/models.dart @@ -4,6 +4,7 @@ import 'package:clock/clock.dart'; import 'package:json_annotation/json_annotation.dart'; +import 'package:pub_dev/shared/popularity_storage.dart'; import 'search_service.dart'; @@ -38,6 +39,18 @@ class SearchSnapshot { documents!.values.updateLikeScores(); } + /// Updates all popularity values to the currently cached one, otherwise + /// only updated package would have been on their new values. + void updatePopularityScores() { + for (final d in documents!.values) { + if (popularityStorage.isInvalid) { + d.popularityScore = d.likeScore; + } else { + d.popularityScore = popularityStorage.lookup(d.package); + } + } + } + Map toJson() => _$SearchSnapshotToJson(this); } diff --git a/app/lib/shared/popularity_storage.dart b/app/lib/shared/popularity_storage.dart index 97aabf7144..726c2d7e37 100644 --- a/app/lib/shared/popularity_storage.dart +++ b/app/lib/shared/popularity_storage.dart @@ -30,6 +30,7 @@ PopularityStorage get popularityStorage => class PopularityStorage { late CachedValue<_PopularityData> _popularity; late final _PopularityLoader _loader; + bool? _invalid; PopularityStorage(Bucket bucket) { _loader = _PopularityLoader(bucket); @@ -41,6 +42,10 @@ class PopularityStorage { ); } + bool get isInvalid => + _invalid ?? + (!_popularity.isAvailable || (_popularity.value?.isInvalid ?? true)); + DateTime? get lastFetched => _popularity.lastUpdated; String? get dateRange => _popularity.value?.dateRange; int get count => _popularity.value?.values.length ?? 0; @@ -60,7 +65,13 @@ class PopularityStorage { // Updates popularity scores to fixed values, useful for testing. @visibleForTesting - void updateValues(Map values) { + void updateValues( + Map values, { + bool? invalid, + }) { + if (invalid != null) { + _invalid = invalid; + } // ignore: invalid_use_of_visible_for_testing_member _popularity.setValue( _PopularityData(values: values, first: clock.now(), last: clock.now())); @@ -131,6 +142,13 @@ class _PopularityData { String get dateRange => '${first?.toIso8601String()} - ${last?.toIso8601String()}'; + + /// Indicates that the data has no time range, or that the range is not + /// covering a full day, or that it is more than a month old. + late final isInvalid = first == null || + last == null || + last!.difference(first!).inDays < 1 || + clock.now().difference(last!).inDays > 30; } class _Entry implements Comparable<_Entry> {