Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
10 changes: 9 additions & 1 deletion lib/src/command/outdated.dart
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,22 @@ Consider using the Dart 2.19 sdk to migrate to null safety.''');
),
};
// Add all dependencies from the lockfile.
final idsToAnalyze = <PackageRef>[];
for (final id in [
...currentPackages,
...upgradablePackages,
...resolvablePackages,
]) {
if (!visited.add(id.name)) continue;
rows.add(await analyzeDependency(id.toRef()));
idsToAnalyze.add(id.toRef());
}
await cache.prefetchAdvisoriesAndStatus([
...currentPackages,
...upgradablePackages,
...resolvablePackages,
]);

rows.addAll(await Future.wait(idsToAnalyze.map(analyzeDependency)));

if (!includeDevDependencies) {
rows.removeWhere((r) => r.kind == _DependencyKind.dev);
Expand Down
6 changes: 6 additions & 0 deletions lib/src/solver/report.dart
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,12 @@ $contentHashesDocumentationUrl
final names = _newLockFile.packages.keys.toList();
names.remove(_rootPubspec.name);
names.sort();
if (_type != SolveType.downgrade) {
await _cache.prefetchAdvisoriesAndStatus(
names.map((name) => _newLockFile.packages[name]).nonNulls,
);
}

var changes = 0;
for (final name in names) {
changes += await _reportPackage(name, output) ? 1 : 0;
Expand Down
24 changes: 14 additions & 10 deletions lib/src/source/hosted.dart
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,12 @@ class HostedSource extends CachedSource {
final stat = io.File(advisoriesCachePath).statSync();

if (stat.type == io.FileSystemEntityType.file) {
if (advisoriesUpdated.isAfter(stat.modified)) {
// To tolerate timezone differences and clock skew between the local
// machine and the pub.dev server, we add a 24-hour buffer to
// stat.modified.
if (advisoriesUpdated.isAfter(
stat.modified.add(const Duration(hours: 24)),
)) {
tryDeleteEntry(advisoriesCachePath);
return null;
}
Expand All @@ -792,14 +797,7 @@ class HostedSource extends CachedSource {
final parsedCacheAdvisoriesUpdated = DateTime.parse(
cachedAdvisoriesUpdated,
);
final advisoriesUpdated =
(await status(id.toRef(), id.version, cache)).advisoriesUpdated;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here we were missing the maxAge


if (
// We could not obtain the timestamp of latest advisory update.
advisoriesUpdated == null ||
// The cached entry is too old.
advisoriesUpdated.isAfter(parsedCacheAdvisoriesUpdated)) {
if (advisoriesUpdated.isAfter(parsedCacheAdvisoriesUpdated)) {
tryDeleteEntry(advisoriesCachePath);
} else {
return _extractAdvisoryDetailsForPackage(doc, id.toRef().name);
Expand Down Expand Up @@ -828,6 +826,9 @@ class HostedSource extends CachedSource {
final Map<PackageRef, (DateTime, List<HostedVersionInfo>)> _responseCache =
{};

/// An in-memory cache to store the futures of fetched security advisories.
final Map<PackageId, Future<List<Advisory>?>> _advisoriesCache = {};

/// If a cached version listing response for [ref] exists on disk and is less
/// than [maxAge] old it is parsed and returned.
///
Expand Down Expand Up @@ -1097,7 +1098,10 @@ class HostedSource extends CachedSource {
SystemCache cache,
Duration? maxAge,
) {
return _getAdvisories(id, cache, maxAge);
return _advisoriesCache.putIfAbsent(
id,
() => _getAdvisories(id, cache, maxAge),
);
}

@override
Expand Down
24 changes: 24 additions & 0 deletions lib/src/system_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,30 @@ Consider setting the `PUB_CACHE` variable manually.
return pubspec;
}

/// Prefetches the status and advisories for [packages] in parallel.
Future<void> prefetchAdvisoriesAndStatus(Iterable<PackageId> packages) async {
final futures = <Future<Object?>>[];
for (final id in packages) {
futures.add(
id.source.status(
id.toRef(),
id.version,
this,
maxAge: const Duration(days: 3),
),
);
final advisoriesFuture = id.source.getAdvisoriesForPackageVersion(
id,
this,
const Duration(days: 3),
);
if (advisoriesFuture != null) {
futures.add(advisoriesFuture);
}
}
await Future.wait(futures);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we have concurrency limits here? probably that exists in fetch and schedule logic, just wanted to check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Really good point. Shared the pool with the ratelimitedscheduler

}

/// Get the IDs of all versions that match [ref].
///
/// Note that this does *not* require the packages to be downloaded locally,
Expand Down
32 changes: 32 additions & 0 deletions test/get/hosted/advisory_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -489,4 +489,36 @@ Future<void> main() async {
File(p.join(d.sandbox, appPath, 'pubspec.lock')).deleteSync();
await pubGet(args: ['--offline']);
});

test('subsequent pub get reads advisories and status from cache '
'without network calls', () async {
final server = await servePackages();
server.serve('foo', '1.2.3');

await d.dir(appPath, [
d.pubspec({
'name': 'app',
'dependencies': {'foo': '^1.0.0'},
}),
]).create();

server.addAdvisory(
advisoryId: '123',
displayUrl: 'https://github.com/advisories/123',
affectedPackages: [
AffectedPackage(name: 'foo', versions: ['1.2.3']),
],
);

// First fetch: populates the cache
await pubGet();

// Configure server to fail on any subsequent HTTP requests
server.serveErrors();

// Second fetch: should read everything from cache and make 0 network
// requests (even with a stable lockfile, pub get still validates
// security advisories).
await pubGet();
});
}
Loading