Refactor Source.

sdk/lib/_internal/pub/lib/src/source.dart

Index: sdk/lib/_internal/pub/lib/src/source.dart
diff --git a/sdk/lib/_internal/pub/lib/src/source.dart b/sdk/lib/_internal/pub/lib/src/source.dart
index 06181c5197a5ee46803b9e9ac5e5fbc074b48a5b..b68fe8a210f4e9b5e8753fa39ff26ab9518de411 100644
--- a/sdk/lib/_internal/pub/lib/src/source.dart
+++ b/sdk/lib/_internal/pub/lib/src/source.dart
@@ -6,20 +6,20 @@ library pub.source;
 
 import 'dart:async';
 
-import 'package:path/path.dart' as path;
-import 'package:stack_trace/stack_trace.dart';
-
-import 'io.dart';
 import 'package.dart';
 import 'pubspec.dart';
 import 'system_cache.dart';
-import 'utils.dart';
 import 'version.dart';
 
 /// A source from which to get packages.
 ///
-/// Each source has many packages that it looks up using [PackageId]s. The
-/// source is responsible for getting these packages into the package cache.
+/// Each source has many packages that it looks up using [PackageId]s. Sources
+/// that inherit this directly (currently just [PathSource]) are *uncached*
+/// sources. They deliver a package directly to the package that depends on it.
+///
+/// Other sources are *cached* sources. These extend [CachedSource]. When a
+/// package needs a dependency from a cached source, it is first installed in
+/// the [SystemCache] and then acquired from there.
 abstract class Source {
   /// The name of the source. Should be lower-case, suitable for use in a
   /// filename, and unique accross all sources.
@@ -28,27 +28,16 @@ abstract class Source {
   /// Whether or not this source is the default source.
   bool get isDefault => systemCache.sources.defaultSource == this;
 
-  /// Whether this source's packages should be cached in Pub's global cache
-  /// directory.
-  ///
-  /// A source should be cached if it requires network access to retrieve
-  /// packages. It doesn't need to be cached if all packages are available
-  /// locally.
-  bool get shouldCache;
-
   /// The system cache with which this source is registered.
   SystemCache get systemCache {
     assert(_systemCache != null);
     return _systemCache;
   }
 
-  /// The system cache variable. Set by [_bind].
-  SystemCache _systemCache;
-
-  /// The root directory of this source's cache within the system cache.
+  /// The system cache variable.
   ///
-  /// This shouldn't be overridden by subclasses.
-  String get systemCacheRoot => path.join(systemCache.rootDir, name);
+  /// Set by [_bind].
+  SystemCache _systemCache;
 
   /// Records the system cache to which this source belongs.
   ///
@@ -62,7 +51,7 @@ abstract class Source {
   /// Get the list of all versions that exist for the package described by
   /// [description]. [name] is the expected name of the package.
   ///
-  /// Note that this does *not* require the package to be downloaded locally,
+  /// Note that this does *not* require the packages to be downloaded locally,
   /// which is the point. This is used during version resolution to determine
   /// which package versions are available to be downloaded (or already
   /// downloaded).
@@ -71,176 +60,44 @@ abstract class Source {
   /// uses [describe] to get that version.
   Future<List<Version>> getVersions(String name, description) {
     var id = new PackageId(name, this.name, Version.none, description);
-    return describeUncached(id).then((pubspec) => [pubspec.version]);
+    return describe(id).then((pubspec) => [pubspec.version]);
   }
 
   /// Loads the (possibly remote) pubspec for the package version identified by
   /// [id]. This may be called for packages that have not yet been downloaded
   /// during the version resolution process.
   ///
-  /// If the package has been downloaded to the system cache, the cached pubspec
-  /// will be used. Otherwise, it delegates to host-specific lookup behavior.
-  ///
-  /// For cached sources, by default this uses [downloadToSystemCache] to get
-  /// the pubspec. There is no default implementation for non-cached sources;
-  /// they must implement it manually.
+  /// Sources should not override this. Instead, they implement [onDescribe].
   Future<Pubspec> describe(PackageId id) {
     if (id.isRoot) throw new ArgumentError("Cannot describe the root package.");
     if (id.source != name) {
       throw new ArgumentError("Package $id does not use source $name.");
     }
 
-    // Try to get it from the system cache first.
-    if (shouldCache) {
-      return systemCacheDirectory(id).then((packageDir) {
-        if (!fileExists(path.join(packageDir, "pubspec.yaml"))) {
-          return describeUncached(id);
-        }
-
-        return new Pubspec.load(packageDir, _systemCache.sources,
-            expectedName: id.name);
-      });
-    }
-
-    // Not cached, so get it from the source.
-    return describeUncached(id);
+    // Delegate to the overridden one.
+    return onDescribe(id);
   }
 
-  /// Loads the pubspec for the package version identified by [id] which is not
-  /// already in the system cache.
-  ///
-  /// For cached sources, by default this uses [downloadToSystemCache] to get
-  /// the pubspec. There is no default implementation for non-cached sources;
-  /// they must implement it manually.
+  /// Loads the (possibly remote) pubspec for the package version identified by
+  /// [id]. This may be called for packages that have not yet been downloaded
+  /// during the version resolution process.
   ///
-  /// This method is effectively protected. Derived classes may override it,
-  /// but external code should not call it. Call [describe()] instead.
-  Future<Pubspec> describeUncached(PackageId id) {
-    if (!shouldCache) {
-      throw new UnimplementedError(
-          "Source $name must implement describeUncached(id).");
-    }
-    return downloadToSystemCache(id).then((package) => package.pubspec);
-  }
+  /// This method is effectively protected: subclasses must implement it, but
+  /// external code should not call this. Instead, call [describe].
+  Future<Pubspec> onDescribe(PackageId id);
 
   /// Gets the package identified by [id] and places it at [path].
   ///
-  /// Returns a [Future] that completes when the operation finishes. The
-  /// [Future] should resolve to true if the package was found in the source
-  /// and false if it wasn't. For all other error conditions, it should complete
-  /// with an exception.
-  ///
-  /// [path] is guaranteed not to exist, and its parent directory is guaranteed
-  /// to exist.
-  ///
-  /// Note that [path] may be deleted. If re-getting a package that has already
-  /// been gotten would be costly or impossible, [downloadToSystemCache]
-  /// should be implemented instead of [get].
-  ///
-  /// This doesn't need to be implemented if [downloadToSystemCache] is
-  /// implemented.
-  Future<bool> get(PackageId id, String path) {
-    throw new UnimplementedError("Either get() or downloadToSystemCache() must "
-        "be implemented for source $name.");
-  }
-
-  /// Determines if the package with [id] is already downloaded to the system
-  /// cache.
-  ///
-  /// This should only be called for sources with [shouldCache] set to true.
-  /// Completes to true if the package is in the cache and appears to be
-  /// uncorrupted.
-  Future<bool> isInSystemCache(PackageId id) {
-    return systemCacheDirectory(id).then((packageDir) {
-      return dirExists(packageDir) && !_isCachedPackageCorrupted(packageDir);
-    });
-  }
-
-  /// Downloads the package identified by [id] to the system cache.
-  ///
-  /// This is only called for sources with [shouldCache] set to true. By
-  /// default, this uses [systemCacheDirectory] and [get].
-  ///
-  /// If [force] is `true`, then the package is downloaded even if it already
-  /// exists in the cache. The previous one will be deleted.
-  Future<Package> downloadToSystemCache(PackageId id, {bool force}) {
-    if (force == null) force = false;
-
-    var packageDir;
-    return systemCacheDirectory(id).then((p) {
-      packageDir = p;
-
-      // See if it's already cached.
-      if (dirExists(packageDir)) {
-        if (force || _isCachedPackageCorrupted(packageDir)) {
-          // Wipe it out and re-install it.
-          deleteEntry(packageDir);
-        } else {
-          // Already downloaded.
-          return true;
-        }
-      }
+  /// Returns a [Future] that completes when the operation finishes. [path] is
+  /// guaranteed not to exist, and its parent directory is guaranteed to exist.
+  Future get(PackageId id, String path);
 
-      ensureDir(path.dirname(packageDir));
-      return get(id, packageDir);
-    }).then((found) {
-      if (!found) fail('Package $id not found.');
-      return new Package.load(id.name, packageDir, systemCache.sources);
-    });
-  }
-
-  /// Since pub generates symlinks that point into the system cache (in
-  /// particular, targeting the "lib" directories of cached packages), it's
-  /// possible to accidentally break cached packages if something traverses
-  /// that symlink.
-  ///
-  /// This tries to determine if the cached package at [packageDir] has been
-  /// corrupted. The heuristics are it is corrupted if any of the following are
-  /// true:
+  /// Returns the directory where this package can (or could) be found locally.
   ///
-  ///   * It has an empty "lib" directory.
-  ///   * It has no pubspec.
-  bool _isCachedPackageCorrupted(String packageDir) {
-    if (!fileExists(path.join(packageDir, "pubspec.yaml"))) return true;
-
-    var libDir = path.join(packageDir, "lib");
-    if (dirExists(libDir)) return listDir(libDir).length == 0;
-
-    // If we got here, it's OK.
-    return false;
-  }
-
-  /// Returns the directory where this package can be found locally. If this is
-  /// a cached source, it will be in the system cache. Otherwise, it will
-  /// depend on the source.
-  Future<String> getDirectory(PackageId id) {
-    if (shouldCache) return systemCacheDirectory(id);
-    throw new UnimplementedError("Source $name must implement this.");
-  }
-
-  /// Returns the directory in the system cache that the package identified by
-  /// [id] should be downloaded to. This should return a path to a subdirectory
-  /// of [systemCacheRoot].
-  ///
-  /// This doesn't need to be implemented if [shouldCache] is false.
-  Future<String> systemCacheDirectory(PackageId id) {
-    return new Future.error(
-        "systemCacheDirectory() must be implemented if shouldCache is true.",
-        new Chain.current());
-  }
-
-  /// Reinstalls all packages that have been previously installed into the
-  /// system cache by this source.
-  ///
-  /// Returns a [Pair] whose first element is the number of packages
-  /// successfully repaired and the second is the number of failures.
-  Future<Pair<int, int>> repairCachedPackages() {
-    if (shouldCache) {
-      throw new UnimplementedError("Source $name must implement this.");
-    }
-    throw new UnsupportedError("Cannot call repairCachedPackages() on an "
-        "uncached source.");
-  }
+  /// If the source is cached, this will be a path in the system cache. In that
+  /// case, this will return a directory even if the package has not been
+  /// installed into the cache yet.
+  Future<String> getDirectory(PackageId id);
 
   /// When a [Pubspec] or [LockFile] is parsed, it reads in the description for
   /// each dependency. It is up to the dependency's [Source] to determine how
@@ -259,9 +116,7 @@ abstract class Source {
   /// [fromLockFile] is true when the description comes from a [LockFile], to
   /// allow the source to use lockfile-specific descriptions via [resolveId].
   dynamic parseDescription(String containingPath, description,
-                           {bool fromLockFile: false}) {
-    return description;
-  }
+                           {bool fromLockFile: false});
 
   /// When a [LockFile] is serialized, it uses this method to get the
   /// [description] in the right format.
@@ -284,10 +139,7 @@ abstract class Source {
   /// Returns whether or not [description1] describes the same package as
   /// [description2] for this source. This method should be light-weight. It
   /// doesn't need to validate that either package exists.
-  ///
-  /// By default, just uses regular equality.
-  bool descriptionsEqual(description1, description2) =>
-    description1 == description2;
+  bool descriptionsEqual(description1, description2);
 
   /// For some sources, [PackageId]s can point to different chunks of code at
   /// different times. This takes such an [id] and returns a future that
@@ -309,15 +161,6 @@ abstract class Source {
   /// By default, this just returns [id].
   Future<PackageId> resolveId(PackageId id) => new Future.value(id);
 
-  /// Returns the [Package]s that have been downloaded to the system cache.
-  List<Package> getCachedPackages() {
-    if (shouldCache) {
-      throw new UnimplementedError("Source $name must implement this.");
-    }
-    throw new UnsupportedError("Cannot call getCachedPackages() on an "
-        "uncached source.");
-  }
-
   /// Returns the source's name.
   String toString() => name;
 }