Use the cached pubspec if possible for describing hosted packages.

BUG=https://code.google.com/p/dart/issues/detail?id=9027

Review URL: https://codereview.chromium.org//14241005

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge@21777 260f80e4-7a28-3924-810f-c04153c831b5

lib/src/hosted_source.dart

@@ -49,11 +49,16 @@ class HostedSource extends Source {
   /// Downloads and parses the pubspec for a specific version of a package that
   /// is available from the site.
   Future<Pubspec> describe(PackageId id) {
+    // Request it from the server.
     var url = _makeVersionUrl(id, (server, package, version) =>
         "$server/packages/$package/versions/$version.yaml");
 
     log.io("Describe package at $url.");
     return httpClient.read(url).then((yaml) {
+      // TODO(rnystrom): After this is pulled down, we could place it in
+      // a secondary cache of just pubspecs. This would let us have a
+      // persistent cache for pubspecs for packages that haven't actually
+      // been installed.
       return new Pubspec.parse(null, yaml, systemCache.sources);
     }).catchError((ex) {
       var parsed = _parseDescription(id.description);
@@ -122,11 +127,11 @@ class HostedSource extends Source {
     var cacheDir = path.join(systemCacheRoot,
                              _getSourceDirectory(_defaultUrl));
     if (!dirExists(cacheDir)) return [];
-  
+
     return listDir(path.join(cacheDir)).map((entry) =>
         new Package.load(null, entry, systemCache.sources)).toList();
   }
-  
+
   /// When an error occurs trying to read something about [package] from [url],
   /// this tries to translate into a more user friendly error message. Always
   /// throws an error, either the original one or a better one.

lib/src/package.dart

@@ -146,7 +146,7 @@ class PackageId implements Comparable<PackageId> {
   }
 
   /// Returns the pubspec for this package.
-  Future<Pubspec> describe() => source.describe(this);
+  Future<Pubspec> describe() => source.systemCache.describe(this);
 
   /// Returns a future that completes to the resovled [PackageId] for this id.
   Future<PackageId> get resolved => source.resolveId(this);

lib/src/system_cache.dart

@@ -70,13 +70,16 @@ class SystemCache {
     // Try to get it from the system cache first.
     if (id.source.shouldCache) {
       return id.systemCacheDirectory.then((packageDir) {
-        if (!dirExists(packageDir)) return id.describe();
+        if (!fileExists(path.join(packageDir, "pubspec.yaml"))) {
+          return id.source.describe(id);
+        }
+
         return new Pubspec.load(id.name, packageDir, sources);
       });
     }
 
     // Not cached, so get it from the source.
-    return id.describe();
+    return id.source.describe(id);
   }
 
   /// Ensures that the package identified by [id] is installed to the cache,

test/install/hosted/cached_pubspec_test.dart

+// Copyright (c) 2012, 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.
+
+library pub_tests;
+
+import 'package:pathos/path.dart' as path;
+import 'package:scheduled_test/scheduled_test.dart';
+
+import '../../../../pub/io.dart';
+import '../../descriptor.dart' as d;
+import '../../test_pub.dart';
+
+main() {
+  initConfig();
+
+  integration('does not request a pubspec for a cached package', () {
+    servePackages([packageMap("foo", "1.2.3")]);
+
+    d.appDir([dependencyMap("foo", "1.2.3")]).create();
+
+    // Run install once so it gets cached.
+    schedulePub(args: ['install'],
+        output: new RegExp("Dependencies installed!\$"));
+
+    // Clear the cache. We don't care about anything that was served during
+    // the initial install.
+    getRequestedPaths();
+
+    d.cacheDir({"foo": "1.2.3"}).validate();
+    d.packagesDir({"foo": "1.2.3"}).validate();
+
+    // Run the solver again now that it's cached.
+    schedulePub(args: ['install'],
+        output: new RegExp("Dependencies installed!\$"));
+
+    // The update should not have requested the pubspec since it's installed
+    // locally already.
+    getRequestedPaths().then((paths) {
+      expect(paths, isNot(contains("packages/foo/versions/1.2.3.yaml")));
+    });
+  });
+}

test/test_pub.dart

@@ -57,6 +57,10 @@ bool get runningOnBuildbot =>
 /// The current [HttpServer] created using [serve].
 var _server;
 
+/// The list of paths that have been requested from the server since the last
+/// call to [getRequestedPaths].
+final _requestedPaths = <String>[];
+
 /// The cached value for [_portCompleter].
 Completer<int> _portCompleterCache;
 
@@ -73,6 +77,16 @@ Completer<int> get _portCompleter {
 /// A future that will complete to the port used for the current server.
 Future<int> get port => _portCompleter.future;
 
+/// Gets the list of paths that have been requested from the server since the
+/// last time this was called (or since the server was first spun up).
+Future<List<String>> getRequestedPaths() {
+  return schedule(() {
+    var paths = _requestedPaths.toList();
+    _requestedPaths.clear();
+    return paths;
+  });
+}
+
 /// Creates an HTTP server to serve [contents] as static files. This server will
 /// exist only for the duration of the pub run.
 ///
@@ -88,6 +102,10 @@ void serve([List<d.Descriptor> contents]) {
           var response = request.response;
           try {
             var path = request.uri.path.replaceFirst("/", "");
+
+            if (_requestedPaths == null) _requestedPaths = <String>[];
+            _requestedPaths.add(path);
+
             response.persistentConnection = false;
             var stream = baseDir.load(path);
 

test/version_solver_test.dart

@@ -888,6 +888,10 @@ class MockSource extends Source {
 
   MockSource(this.name);
 
+  Future<String> systemCacheDirectory(PackageId id) {
+    return new Future.value('${id.name}-${id.version}');
+  }
+
   Future<List<Version>> getVersions(String name, String description) {
     return new Future.sync(() {
       // Make sure the solver doesn't request the same thing twice.