Simplified caching of artifacts for changing modules, now that we can…

… use sha1 keys to prevent re-download - When cache entry for changing module is expired, remove all cache entries for artifacts of that module - Although simpler, we will now re-download an unchanged artifact for a changing module, if no sha1 key is published
gradle · Dec 27, 2011 · f0b24b6 · f0b24b6
1 parent 04b1976
commit f0b24b6
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 89 deletions.
diff --git a/...rc/main/groovy/org/gradle/api/internal/artifacts/DefaultDependencyManagementServices.java b/...rc/main/groovy/org/gradle/api/internal/artifacts/DefaultDependencyManagementServices.java
@@ -35,7 +35,7 @@
 import org.gradle.api.internal.artifacts.dsl.dependencies.ProjectFinder;
 import org.gradle.api.internal.artifacts.ivyservice.*;
 import org.gradle.api.internal.artifacts.ivyservice.artifactcache.ArtifactResolutionCache;
-import org.gradle.api.internal.artifacts.ivyservice.artifactcache.SingleFileBackedArtifactResolutionCache;
+import org.gradle.api.internal.artifacts.ivyservice.artifactcache.DefaultArtifactResolutionCache;
 import org.gradle.api.internal.artifacts.ivyservice.clientmodule.ClientModuleRegistry;
 import org.gradle.api.internal.artifacts.ivyservice.clientmodule.DefaultClientModuleRegistry;
 import org.gradle.api.internal.artifacts.ivyservice.dynamicversions.ModuleResolutionCache;
@@ -173,16 +173,16 @@ protected ModuleDescriptorCache createModuleDescriptorCache() {
         return new DefaultModuleDescriptorCache(
                 get(ArtifactCacheMetaData.class),
                 get(TimeProvider.class),
-                get(CacheLockingManager.class),
-                get(ArtifactResolutionCache.class));
+                get(CacheLockingManager.class)
+        );
     }
 
     protected ArtifactFileStore createArtifactFileStore() {
         return new DefaultArtifactFileStore(get(ArtifactCacheMetaData.class));
     }
 
     protected ArtifactResolutionCache createArtifactResolutionCache() {
-        return new SingleFileBackedArtifactResolutionCache(
+        return new DefaultArtifactResolutionCache(
                 get(ArtifactCacheMetaData.class),
                 get(TimeProvider.class),
                 get(CacheLockingManager.class)

diff --git a/...gleFileBackedArtifactResolutionCache.java → ...cache/DefaultArtifactResolutionCache.java b/...gleFileBackedArtifactResolutionCache.java → ...cache/DefaultArtifactResolutionCache.java
@@ -28,13 +28,13 @@
 import java.io.File;
 import java.io.Serializable;
 
-public class SingleFileBackedArtifactResolutionCache implements ArtifactResolutionCache {
+public class DefaultArtifactResolutionCache implements ArtifactResolutionCache {
     private final TimeProvider timeProvider;
     private final ArtifactCacheMetaData cacheMetadata;
     private final CacheLockingManager cacheLockingManager;
     private PersistentIndexedCache<RevisionKey, ArtifactResolutionCacheEntry> cache;
 
-    public SingleFileBackedArtifactResolutionCache(ArtifactCacheMetaData cacheMetadata, TimeProvider timeProvider, CacheLockingManager cacheLockingManager) {
+    public DefaultArtifactResolutionCache(ArtifactCacheMetaData cacheMetadata, TimeProvider timeProvider, CacheLockingManager cacheLockingManager) {
         this.timeProvider = timeProvider;
         this.cacheLockingManager = cacheLockingManager;
         this.cacheMetadata = cacheMetadata;

diff --git a/...g/gradle/api/internal/artifacts/ivyservice/ivyresolve/CachingModuleVersionRepository.java b/...g/gradle/api/internal/artifacts/ivyservice/ivyresolve/CachingModuleVersionRepository.java
@@ -17,12 +17,13 @@
 
 import org.apache.ivy.core.module.descriptor.Artifact;
 import org.apache.ivy.core.module.descriptor.DependencyDescriptor;
+import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
 import org.apache.ivy.core.module.id.ModuleRevisionId;
 import org.gradle.api.internal.artifacts.configurations.dynamicversion.CachePolicy;
-import org.gradle.api.internal.artifacts.ivyservice.filestore.ArtifactFileStore;
 import org.gradle.api.internal.artifacts.ivyservice.artifactcache.ArtifactResolutionCache;
 import org.gradle.api.internal.artifacts.ivyservice.dynamicversions.ForceChangeDependencyDescriptor;
 import org.gradle.api.internal.artifacts.ivyservice.dynamicversions.ModuleResolutionCache;
+import org.gradle.api.internal.artifacts.ivyservice.filestore.ArtifactFileStore;
 import org.gradle.api.internal.artifacts.ivyservice.modulecache.ModuleDescriptorCache;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -107,7 +108,7 @@ public CachedModuleLookup lookupModuleInCache(DependencyDescriptor resolvedDepen
         }
         if (cachedModuleDescriptor.isChangingModule() || resolvedDependencyDescriptor.isChanging()) {
             if (cachePolicy.mustRefreshChangingModule(cachedModuleDescriptor.getModuleVersion(), cachedModuleDescriptor.getAgeMillis())) {
-                // TODO:DAZ Move expiring of changing module artifacts into here, once we can rely on sha1 comparison to prevent re-download
+                expireArtifactsForChangingModule(delegate, cachedModuleDescriptor.getModuleDescriptor());
                 LOGGER.debug("Cached meta-data for changing module is expired: will perform fresh resolve of '{}'", resolvedModuleVersionId);
                 return notFound();
             }
@@ -120,6 +121,12 @@ public CachedModuleLookup lookupModuleInCache(DependencyDescriptor resolvedDepen
         return found(cachedModule);
     }
 
+    private void expireArtifactsForChangingModule(ModuleVersionRepository repository, ModuleDescriptor descriptor) {
+        for (Artifact artifact : descriptor.getAllArtifacts()) {
+            artifactResolutionCache.expireCachedArtifactResolution(repository, artifact.getId());
+        }
+    }
+
     public ModuleVersionDescriptor resolveModule(DependencyDescriptor resolvedDependencyDescriptor, DependencyDescriptor requestedDependencyDescriptor) {
         ModuleVersionDescriptor module = delegate.getDependency(ForceChangeDependencyDescriptor.forceChangingFlag(resolvedDependencyDescriptor, true));
 

diff --git a/...n/groovy/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DependencyResolvers.java b/...n/groovy/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/DependencyResolvers.java
diff --git a/...rg/gradle/api/internal/artifacts/ivyservice/modulecache/DefaultModuleDescriptorCache.java b/...rg/gradle/api/internal/artifacts/ivyservice/modulecache/DefaultModuleDescriptorCache.java
@@ -15,12 +15,10 @@
  */
 package org.gradle.api.internal.artifacts.ivyservice.modulecache;
 
-import org.apache.ivy.core.module.descriptor.Artifact;
 import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
 import org.apache.ivy.core.module.id.ModuleRevisionId;
 import org.gradle.api.internal.artifacts.ivyservice.ArtifactCacheMetaData;
 import org.gradle.api.internal.artifacts.ivyservice.CacheLockingManager;
-import org.gradle.api.internal.artifacts.ivyservice.artifactcache.ArtifactResolutionCache;
 import org.gradle.api.internal.artifacts.ivyservice.ivyresolve.ModuleVersionRepository;
 import org.gradle.cache.PersistentIndexedCache;
 import org.gradle.util.TimeProvider;
@@ -37,15 +35,13 @@ public class DefaultModuleDescriptorCache implements ModuleDescriptorCache {
     private final ArtifactCacheMetaData cacheMetadata;
     private final CacheLockingManager cacheLockingManager;
 
-    private final ArtifactResolutionCache artifactResolutionCache;
     private final ModuleDescriptorStore moduleDescriptorStore;
     private PersistentIndexedCache<RevisionKey, ModuleDescriptorCacheEntry> cache;
 
-    public DefaultModuleDescriptorCache(ArtifactCacheMetaData cacheMetadata, TimeProvider timeProvider, CacheLockingManager cacheLockingManager, ArtifactResolutionCache artifactResolutionCache) {
+    public DefaultModuleDescriptorCache(ArtifactCacheMetaData cacheMetadata, TimeProvider timeProvider, CacheLockingManager cacheLockingManager) {
         this.timeProvider = timeProvider;
         this.cacheLockingManager = cacheLockingManager;
         this.cacheMetadata = cacheMetadata;
-        this.artifactResolutionCache = artifactResolutionCache;
 
         // TODO:DAZ inject this
         moduleDescriptorStore = new ModuleDescriptorStore(new ModuleDescriptorFileStore(cacheMetadata));
@@ -81,37 +77,11 @@ public void cacheModuleDescriptor(ModuleVersionRepository repository, ModuleRevi
             getCache().put(createKey(repository, moduleRevisionId), createMissingEntry(isChanging));
         } else {
             LOGGER.debug("Recording module descriptor in cache: {} [changing = {}]", moduleDescriptor.getModuleRevisionId(), isChanging);
-            expireArtifactsForChangingModuleIfRequired(repository, moduleDescriptor, isChanging);
-
-            // TODO:DAZ Cache will already be locked, due to prior call to getCachedModuleDescriptor. This locking should be more explicit
             moduleDescriptorStore.putModuleDescriptor(repository, moduleDescriptor);
             getCache().put(createKey(repository, moduleRevisionId), createEntry(isChanging));
         }
     }
 
-    private void expireArtifactsForChangingModuleIfRequired(ModuleVersionRepository repository, ModuleDescriptor newDescriptor, boolean newDescriptorIsChanging) {
-        // Expire all cached artifacts if either the cached module descriptor was changing, or the new module descriptor is changing
-        CachedModuleDescriptor cachedModuleDescriptor = getCachedModuleDescriptor(repository, newDescriptor.getModuleRevisionId());
-        if (cachedModuleDescriptor == null) {
-            return;
-        }
-        if (cachedModuleDescriptor.isChangingModule() || newDescriptorIsChanging) {
-            ModuleDescriptor oldDescriptor = cachedModuleDescriptor.getModuleDescriptor();
-            // Only do this if the publication date has changed
-            // TODO:DAZ Get rid of this and rely on sha1 files to prevent re-download.
-            // Will then be able to do before resolving the module, rather than waiting until we have the new descriptor to compare
-            if (oldDescriptor.getResolvedPublicationDate().getTime() != newDescriptor.getResolvedPublicationDate().getTime()) {
-                expireArtifacts(repository, oldDescriptor);
-            }
-        }
-    }
-
-    private void expireArtifacts(ModuleVersionRepository repository, ModuleDescriptor descriptor) {
-        for (Artifact artifact : descriptor.getAllArtifacts()) {
-            artifactResolutionCache.expireCachedArtifactResolution(repository, artifact.getId());
-        }
-    }
-
     private RevisionKey createKey(ModuleVersionRepository resolver, ModuleRevisionId moduleRevisionId) {
         return new RevisionKey(resolver, moduleRevisionId);
     }

diff --git a/...rg/gradle/integtests/resolve/ivy/IvyDynamicRevisionRemoteResolutionIntegrationTest.groovy b/...rg/gradle/integtests/resolve/ivy/IvyDynamicRevisionRemoteResolutionIntegrationTest.groovy
@@ -408,9 +408,9 @@ task retrieve(type: Copy) {
         and: "Server handles requests"
         server.resetExpectations()
         // Server will be hit to get updated versions
-        server.expectGetMissing('/repo/group/projectA/1.1/ivy-1.1.xml.sha1')
+        server.expectGet('/repo/group/projectA/1.1/ivy-1.1.xml.sha1', module.sha1File(module.ivyFile))
         server.expectGet('/repo/group/projectA/1.1/ivy-1.1.xml', module.ivyFile)
-        server.expectGetMissing('/repo/group/projectA/1.1/projectA-1.1.jar.sha1')
+        server.expectGet('/repo/group/projectA/1.1/projectA-1.1.jar.sha1', module.sha1File(module.jarFile))
         server.expectGet('/repo/group/projectA/1.1/projectA-1.1.jar', module.jarFile)
         server.expectGet('/repo/group/projectA/1.1/other-1.1.jar', module.moduleDir.file('other-1.1.jar'))
         server.expectGet('/repo/group/projectB/2.0/ivy-2.0.xml', moduleB.ivyFile)
@@ -457,7 +457,6 @@ task retrieve(type: Copy) {
         def jarSnapshot = file('build/projectA-1.1.jar').snapshot()
 
         when:
-        waitOneSecondSoThatPublicationDateWillHaveChanged()
         module.publishWithChangedContent()
 
         and:
@@ -509,7 +508,6 @@ task retrieve(type: Copy) {
         def snapshot = jarFile.snapshot()
 
         when:
-        waitOneSecondSoThatPublicationDateWillHaveChanged();
         module.publishWithChangedContent()
 
         server.resetExpectations()
@@ -575,7 +573,6 @@ task retrieve(type: Copy) {
 
         when: "Module meta-data is changed and artifacts are modified"
         module.artifact([name: 'other'])
-        waitOneSecondSoThatPublicationDateWillHaveChanged()
         module.publishWithChangedContent()
 
         and: "We request 1.1 (changing), with module meta-data cached. No server requests."
@@ -607,13 +604,6 @@ task retrieve(type: Copy) {
         jarFile.assertIsCopyOf(module.jarFile)
     }
 
-    private def waitOneSecondSoThatPublicationDateWillHaveChanged() {
-        // TODO:DAZ Remove this
-        // Ivy checks the publication date to see if it's _really_ changed, won't delete the artifacts if not.
-        // So wait a second to ensure the date will be different.
-        Thread.sleep(1000)
-    }
-
     IvyRepository ivyRepo(def dir = 'ivy-repo') {
         return new IvyRepository(distribution.testFile(dir))
     }

diff --git a/...le/integtests/resolve/maven/MavenSnapshotRemoteDependencyResolutionIntegrationTest.groovy b/...le/integtests/resolve/maven/MavenSnapshotRemoteDependencyResolutionIntegrationTest.groovy
@@ -196,7 +196,6 @@ task retrieve(type: Sync) {
         def nonUniqueJarSnapshot = file('libs/nonunique-1.0-SNAPSHOT.jar').assertIsCopyOf(nonUniqueVersionModule.artifactFile).snapshot()
 
         when: "Republish the snapshots"
-        waitOneSecondSoThatPublicationDateWillHaveChanged()
         uniqueVersionModule.publishWithChangedContent()
         nonUniqueVersionModule.publishWithChangedContent()
 
@@ -213,7 +212,7 @@ task retrieve(type: Sync) {
         when: "Server handles requests"
         expectModuleServed(uniqueVersionModule, '/repo', true)
         expectModuleServed(nonUniqueVersionModule, '/repo', true)
-        
+
         and: "Resolve dependencies with cache expired"
         executer.withArguments("-PnoTimeout")
         run 'retrieve'
@@ -224,13 +223,6 @@ task retrieve(type: Sync) {
         file('libs/nonunique-1.0-SNAPSHOT.jar').assertIsCopyOf(nonUniqueVersionModule.artifactFile).assertHasChangedSince(nonUniqueJarSnapshot);
     }
 
-    private def waitOneSecondSoThatPublicationDateWillHaveChanged() {
-        // TODO:DAZ Remove this
-        // Ivy checks the publication date to see if it's _really_ changed, won't delete the artifacts if not.
-        // So wait a second to ensure the date will be different.
-        Thread.sleep(1000)
-    }
-
     def "does not download snapshot artifacts after expiry when snapshot has not changed"() {
         server.start()
 
@@ -271,9 +263,7 @@ task retrieve(type: Sync) {
 
         when: "Server handles requests"
         server.resetExpectations()
-        server.expectGet('/repo/org/gradle/testproject/1.0-SNAPSHOT/maven-metadata.xml', module.moduleDir.file("maven-metadata.xml"))
-        server.expectGetMissing("/repo/org/gradle/testproject/1.0-SNAPSHOT/${module.pomFile.name}.sha1")
-        server.expectGet("/repo/org/gradle/testproject/1.0-SNAPSHOT/${module.pomFile.name}", module.pomFile)
+        expectReuseModuleArtifacts(module, '/repo')
 
         // Retrieve again with zero timeout should check for updated snapshot
         and: 
@@ -294,10 +284,20 @@ task retrieve(type: Sync) {
         server.expectGet("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/${module.artifactFile.name}", module.artifactFile)
 
         if (sha1requests) {
-            server.expectGetMissing("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/${module.artifactFile.name}.sha1")
+            server.expectGet("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/${module.pomFile.name}.sha1", module.sha1File(module.pomFile))
+            server.expectGet("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/${module.artifactFile.name}.sha1", module.sha1File(module.artifactFile))
         }
     }
 
+    private expectReuseModuleArtifacts(MavenModule module, def prefix) {
+        def moduleName = module.artifactId;
+        server.expectGet("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/maven-metadata.xml", module.moduleDir.file("maven-metadata.xml"))
+        server.expectGet("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/${module.pomFile.name}.sha1", module.sha1File(module.pomFile))
+        // TODO - should only ask for metadata once
+        server.expectGet("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/maven-metadata.xml", module.moduleDir.file("maven-metadata.xml"))
+        server.expectGet("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/${module.artifactFile.name}.sha1", module.sha1File(module.artifactFile))
+    }
+
     private expectModuleMissing(MavenModule module, def prefix) {
         def moduleName = module.artifactId;
         server.expectGetMissing("${prefix}/org/gradle/${moduleName}/1.0-SNAPSHOT/maven-metadata.xml")

diff --git a/...ernal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/MavenRepository.groovy b/...ernal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/MavenRepository.groovy
@@ -187,7 +187,8 @@ class MavenModule {
   <groupId>$groupId</groupId>
   <artifactId>$artifactId</artifactId>
   <packaging>$type</packaging>
-  <version>$version</version>"""
+  <version>$version</version>
+  <description>Published on $publishTimestamp</description>"""
 
         if (parentPomSection) {
            pomFile << "\n$parentPomSection\n"