Skip to content

Commit

Permalink
Hide invalid popularity scores. (dart-lang#7182)
Browse files Browse the repository at this point in the history
  • Loading branch information
isoos authored Nov 15, 2023
1 parent 9398ad7 commit b281a06
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion app/lib/fake/backend/fake_popularity.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ Future<void> generateFakePopularityValues() async {
values[p.name!] = value;
}
// ignore: invalid_use_of_visible_for_testing_member
popularityStorage.updateValues(values);
popularityStorage.updateValues(values, invalid: false);
}
8 changes: 7 additions & 1 deletion app/lib/frontend/templates/views/pkg/labeled_scores.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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 ? '' : '%',
),
),
],
);
Expand Down
8 changes: 8 additions & 0 deletions app/lib/frontend/templates/views/pkg/score_tab.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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: '%',
Expand Down
6 changes: 2 additions & 4 deletions app/lib/search/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ class SearchBackend {
return;
}
snapshot.updateLikeScores();
snapshot.updatePopularityScores();

// first complete snapshot, uploading it
await _snapshotStorage.uploadDataAsJsonMap(snapshot.toJson());
Expand Down Expand Up @@ -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!;
Expand Down
13 changes: 13 additions & 0 deletions app/lib/search/models.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<String, dynamic> toJson() => _$SearchSnapshotToJson(this);
}

Expand Down
20 changes: 19 additions & 1 deletion app/lib/shared/popularity_storage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -60,7 +65,13 @@ class PopularityStorage {

// Updates popularity scores to fixed values, useful for testing.
@visibleForTesting
void updateValues(Map<String, double> values) {
void updateValues(
Map<String, double> 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()));
Expand Down Expand Up @@ -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> {
Expand Down

0 comments on commit b281a06

Please sign in to comment.