From 1bdc2ea300a46dd1edcb1e766f1f3708c0a6b172 Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Tue, 29 Mar 2016 12:38:41 -0700 Subject: [PATCH 1/4] Add ability to revalidate external resource requests The argument allows Cache-Control: max-age=0 to be added in cases where it's desirable for artifact repositories or caching proxies to revalidate requests: - Listing version metadata - Resource has exceeded it's cache lifetime on disk, such as dynamic/snapshot dependencies or when --refresh-dependencies is specified --- ...faultExternalResourceArtifactResolver.java | 2 +- .../resolver/MavenMetadataLoader.java | 2 +- ...ultCacheAwareExternalResourceAccessor.java | 15 ++++++----- ...ogressLoggingExternalResourceAccessor.java | 8 +++--- .../DefaultExternalResourceRepository.java | 8 +++--- .../transport/ExternalResourceRepository.java | 7 +++-- .../transport/file/FileResourceConnector.java | 6 ++--- .../transport/file/FileTransport.java | 2 +- .../resolver/MavenVersionListerTest.groovy | 14 +++++----- ...heAwareExternalResourceAccessorTest.groovy | 22 ++++++++-------- ...LoggingExternalResourceAccessorTest.groovy | 26 +++++++++---------- .../RepositoryTransportWagonAdapter.java | 2 +- ...RepositoryTransportWagonAdapterTest.groovy | 6 ++--- .../HttpPluginResolutionServiceClient.java | 2 +- ...tpPluginResolutionServiceClientTest.groovy | 6 ++--- .../transport/http/HttpClientHelper.java | 25 ++++++++++-------- .../transport/http/HttpResourceAccessor.java | 22 +++++----------- .../transport/http/HttpResourceLister.java | 2 +- .../http/HttpClientHelperTest.groovy | 18 ++++++++++++- .../http/HttpResourceListerTest.groovy | 4 +-- .../transport/aws/s3/S3ResourceConnector.java | 4 +-- .../aws/s3/S3ResourceConnectorTest.groovy | 2 +- .../transport/sftp/SftpResourceAccessor.java | 6 ++--- .../DefaultExternalResourceConnector.java | 8 +++--- .../transfer/ExternalResourceAccessor.java | 8 +++--- 25 files changed, 122 insertions(+), 105 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/DefaultExternalResourceArtifactResolver.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/DefaultExternalResourceArtifactResolver.java index aaa671f005d0..dd80f7771194 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/DefaultExternalResourceArtifactResolver.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/DefaultExternalResourceArtifactResolver.java @@ -66,7 +66,7 @@ private boolean staticResourceExists(List patternList, ModuleCo result.attempted(location); LOGGER.debug("Loading {}", location); try { - if (repository.getResourceMetaData(location.getUri()) != null) { + if (repository.getResourceMetaData(location.getUri(), true) != null) { return true; } } catch (Exception e) { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/MavenMetadataLoader.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/MavenMetadataLoader.java index 8570b2a92a3d..75baa422beca 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/MavenMetadataLoader.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/repositories/resolver/MavenMetadataLoader.java @@ -54,7 +54,7 @@ public MavenMetadata load(URI metadataLocation) throws ResourceException { } private void parseMavenMetadataInfo(final URI metadataLocation, final MavenMetadata metadata) { - ExternalResource resource = repository.getResource(metadataLocation); + ExternalResource resource = repository.getResource(metadataLocation, true); if (resource == null) { throw new MissingResourceException(metadataLocation, String.format("Maven meta-data not available: %s", metadataLocation)); } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessor.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessor.java index 040700fc7933..4944f54d51c2 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessor.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessor.java @@ -73,7 +73,7 @@ public LocallyAvailableExternalResource getResource(final URI location, final Re // If we have no caching options, just get the thing directly if (cached == null && (localCandidates == null || localCandidates.isNone())) { - return copyToCache(location, fileStore, delegate.withProgressLogging().getResource(location)); + return copyToCache(location, fileStore, delegate.withProgressLogging().getResource(location, false)); } // We might be able to use a cached/locally available version @@ -81,8 +81,11 @@ public LocallyAvailableExternalResource getResource(final URI location, final Re return new DefaultLocallyAvailableExternalResource(location, new DefaultLocallyAvailableResource(cached.getCachedFile()), cached.getExternalResourceMetaData()); } + // We have a cached version, but it might be out of date, so we tell the upstreams to revalidate too + final boolean revalidate = true; + // Get the metadata first to see if it's there - final ExternalResourceMetaData remoteMetaData = delegate.getResourceMetaData(location); + final ExternalResourceMetaData remoteMetaData = delegate.getResourceMetaData(location, revalidate); if (remoteMetaData == null) { return null; } @@ -112,7 +115,7 @@ public ExternalResourceMetaData create() { HashValue remoteChecksum = remoteMetaData.getSha1(); if (remoteChecksum == null) { - remoteChecksum = getResourceSha1(location); + remoteChecksum = getResourceSha1(location, revalidate); } if (remoteChecksum != null) { @@ -129,13 +132,13 @@ public ExternalResourceMetaData create() { } // All local/cached options failed, get directly - return copyToCache(location, fileStore, delegate.withProgressLogging().getResource(location)); + return copyToCache(location, fileStore, delegate.withProgressLogging().getResource(location, revalidate)); } - private HashValue getResourceSha1(URI location) { + private HashValue getResourceSha1(URI location, boolean revalidate) { try { URI sha1Location = new URI(location.toASCIIString() + ".sha1"); - ExternalResource resource = delegate.getResource(sha1Location); + ExternalResource resource = delegate.getResource(sha1Location, revalidate); if (resource == null) { return null; } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessor.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessor.java index 6263681f822b..c4f9c551bf22 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessor.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessor.java @@ -32,8 +32,8 @@ public ProgressLoggingExternalResourceAccessor(ExternalResourceAccessor delegate this.delegate = delegate; } - public ExternalResourceReadResponse openResource(URI location) { - ExternalResourceReadResponse resource = delegate.openResource(location); + public ExternalResourceReadResponse openResource(URI location, boolean revalidate) { + ExternalResourceReadResponse resource = delegate.openResource(location, revalidate); if (resource != null) { return new ProgressLoggingExternalResource(location, resource); } else { @@ -42,8 +42,8 @@ public ExternalResourceReadResponse openResource(URI location) { } @Nullable - public ExternalResourceMetaData getMetaData(URI location) { - return delegate.getMetaData(location); + public ExternalResourceMetaData getMetaData(URI location, boolean revalidate) { + return delegate.getMetaData(location, revalidate); } private class ProgressLoggingExternalResource implements ExternalResourceReadResponse { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/DefaultExternalResourceRepository.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/DefaultExternalResourceRepository.java index dd6d0a63bd70..ac5b75f8ceff 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/DefaultExternalResourceRepository.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/DefaultExternalResourceRepository.java @@ -59,13 +59,13 @@ public ExternalResourceRepository withProgressLogging() { return new DefaultExternalResourceRepository(name, loggingAccessor, loggingUploader, lister, loggingAccessor, loggingUploader); } - public ExternalResource getResource(URI source) { - ExternalResourceReadResponse response = accessor.openResource(source); + public ExternalResource getResource(URI source, boolean revalidate) { + ExternalResourceReadResponse response = accessor.openResource(source, revalidate); return response == null ? null : new DefaultExternalResource(source, response); } - public ExternalResourceMetaData getResourceMetaData(URI source) { - return accessor.getMetaData(source); + public ExternalResourceMetaData getResourceMetaData(URI source, boolean revalidate) { + return accessor.getMetaData(source, revalidate); } public void put(LocalResource source, URI destination) throws IOException { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/ExternalResourceRepository.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/ExternalResourceRepository.java index da7338b5b3fa..6c566c8206ab 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/ExternalResourceRepository.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/ExternalResourceRepository.java @@ -35,11 +35,13 @@ public interface ExternalResourceRepository { /** * Attempts to fetch the given resource. * + * @param source The location of the resource to obtain + * @param revalidate Ensure the external resource is not stale * @return null if the resource is not found. * @throws ResourceException On failure to fetch resource. */ @Nullable - ExternalResource getResource(URI source) throws ResourceException; + ExternalResource getResource(URI source, boolean revalidate) throws ResourceException; /** * Transfer a resource to the repository @@ -54,11 +56,12 @@ public interface ExternalResourceRepository { * Fetches only the metadata for the result. * * @param source The location of the resource to obtain the metadata for + * @param revalidate Ensure the external resource is not stale * @return The resource metadata, or null if the resource does not exist * @throws ResourceException On failure to fetch resource metadata. */ @Nullable - ExternalResourceMetaData getResourceMetaData(URI source) throws ResourceException; + ExternalResourceMetaData getResourceMetaData(URI source, boolean revalidate) throws ResourceException; /** * Return a listing of child resources names. diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileResourceConnector.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileResourceConnector.java index b687c0b3cc68..2271afb1fc8e 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileResourceConnector.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileResourceConnector.java @@ -71,7 +71,7 @@ public void put(LocalResource source, URI destination) throws IOException { } } - public LocallyAvailableExternalResource getResource(URI uri) { + public LocallyAvailableExternalResource getResource(URI uri, boolean revalidate) { File localFile = getFile(uri); if (!localFile.exists()) { return null; @@ -79,8 +79,8 @@ public LocallyAvailableExternalResource getResource(URI uri) { return new DefaultLocallyAvailableExternalResource(uri, new DefaultLocallyAvailableResource(localFile)); } - public ExternalResourceMetaData getResourceMetaData(URI location) { - ExternalResource resource = getResource(location); + public ExternalResourceMetaData getResourceMetaData(URI location, boolean revalidate) { + ExternalResource resource = getResource(location, revalidate); return resource == null ? null : resource.getMetaData(); } diff --git a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileTransport.java b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileTransport.java index 88de1290d5b4..c38cb686fb10 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileTransport.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/internal/resource/transport/file/FileTransport.java @@ -55,7 +55,7 @@ public NoOpCacheAwareExternalResourceAccessor(FileResourceConnector connector) { } public LocallyAvailableExternalResource getResource(URI source, ResourceFileStore fileStore, @Nullable LocallyAvailableResourceCandidates localCandidates) throws IOException { - return connector.getResource(source); + return connector.getResource(source, false); } } } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/MavenVersionListerTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/MavenVersionListerTest.groovy index f7155e3f116f..67044dad923c 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/MavenVersionListerTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/repositories/resolver/MavenVersionListerTest.groovy @@ -57,7 +57,7 @@ class MavenVersionListerTest extends Specification { result.attempted == [metaDataResource.toString()] and: - 1 * repository.getResource(metaDataResource) >> resource + 1 * repository.getResource(metaDataResource, true) >> resource 1 * resource.withContent(_) >> { Action action -> action.execute(new ByteArrayInputStream(""" @@ -92,7 +92,7 @@ class MavenVersionListerTest extends Specification { result.attempted == [location1.toString(), location2.toString()] and: - 1 * repository.getResource(location1) >> resource1 + 1 * repository.getResource(location1, true) >> resource1 1 * resource1.withContent(_) >> { Action action -> action.execute(new ByteArrayInputStream(""" @@ -103,7 +103,7 @@ class MavenVersionListerTest extends Specification { """.bytes)) } - 1 * repository.getResource(location2) >> resource2 + 1 * repository.getResource(location2, true) >> resource2 1 * resource2.withContent(_) >> { Action action -> action.execute(new ByteArrayInputStream(""" @@ -130,7 +130,7 @@ class MavenVersionListerTest extends Specification { result.attempted == [metaDataResource.toString()] and: - 1 * repository.getResource(metaDataResource) >> resource + 1 * repository.getResource(metaDataResource, true) >> resource 1 * resource.withContent(_) >> { Action action -> action.execute(new ByteArrayInputStream(""" @@ -159,7 +159,7 @@ class MavenVersionListerTest extends Specification { result.attempted == [metaDataResource.toString()] and: - 1 * repository.getResource(metaDataResource) >> null + 1 * repository.getResource(metaDataResource, true) >> null 0 * repository._ } @@ -181,7 +181,7 @@ class MavenVersionListerTest extends Specification { and: 1 * resource.close() - 1 * repository.getResource(metaDataResource) >> resource; + 1 * repository.getResource(metaDataResource, true) >> resource; 1 * resource.withContent(_) >> { Action action -> action.execute(new ByteArrayInputStream("yo".bytes)) } 0 * repository._ } @@ -202,7 +202,7 @@ class MavenVersionListerTest extends Specification { result.attempted == [metaDataResource.toString()] and: - 1 * repository.getResource(metaDataResource) >> { throw failure } + 1 * repository.getResource(metaDataResource, true) >> { throw failure } 0 * repository._ } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessorTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessorTest.groovy index 0a2248d79cc2..d8ba7832e85c 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessorTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/DefaultCacheAwareExternalResourceAccessorTest.groovy @@ -62,7 +62,7 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { 1 * index.lookup("scheme:thing") >> null 1 * localCandidates.isNone() >> true 1 * repository.withProgressLogging() >> progressLoggingRepo - 1 * progressLoggingRepo.getResource(uri) >> null + 1 * progressLoggingRepo.getResource(uri, false) >> null 0 * _._ } @@ -80,7 +80,7 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { and: 1 * index.lookup("scheme:thing") >> null 1 * localCandidates.isNone() >> false - 1 * repository.getResourceMetaData(uri) >> null + 1 * repository.getResourceMetaData(uri, true) >> null 0 * _._ } @@ -103,7 +103,7 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { 1 * index.lookup("scheme:thing") >> null 1 * localCandidates.isNone() >> true 1 * repository.withProgressLogging() >> progressLoggingRepo - 1 * progressLoggingRepo.getResource(uri) >> remoteResource + 1 * progressLoggingRepo.getResource(uri, false) >> remoteResource _ * remoteResource.name >> "remoteResource" 1 * remoteResource.withContent(_) >> { ExternalResource.ContentAction a -> a.execute(new ByteArrayInputStream(), metaData) @@ -167,7 +167,7 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { timeProvider.currentTime >> 24000L cached.cachedAt >> 23999L cached.externalResourceMetaData >> cachedMetaData - 1 * repository.getResourceMetaData(uri) >> remoteMetaData + 1 * repository.getResourceMetaData(uri, true) >> remoteMetaData localCandidates.none >> false remoteMetaData.sha1 >> sha1 remoteMetaData.etag >> null @@ -210,14 +210,14 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { and: 1 * index.lookup("scheme:thing") >> null - 1 * repository.getResourceMetaData(uri) >> remoteMetaData + 1 * repository.getResourceMetaData(uri, true) >> remoteMetaData localCandidates.none >> false remoteMetaData.sha1 >> null remoteMetaData.etag >> null remoteMetaData.lastModified >> null cachedMetaData.etag >> null cachedMetaData.lastModified >> null - 1 * repository.getResource(new URI("scheme:thing.sha1")) >> remoteSha1 + 1 * repository.getResource(new URI("scheme:thing.sha1"), true) >> remoteSha1 1 * remoteSha1.withContent(_) >> { Transformer t -> t.transform(new ByteArrayInputStream(sha1.asZeroPaddedHexString(40).bytes)) } @@ -254,16 +254,16 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { and: 1 * index.lookup("scheme:thing") >> null - 1 * repository.getResourceMetaData(uri) >> remoteMetaData + 1 * repository.getResourceMetaData(uri, true) >> remoteMetaData localCandidates.none >> false remoteMetaData.sha1 >> null remoteMetaData.etag >> null remoteMetaData.lastModified >> null cachedMetaData.etag >> null cachedMetaData.lastModified >> null - 1 * repository.getResource(new URI("scheme:thing.sha1")) >> null + 1 * repository.getResource(new URI("scheme:thing.sha1"), true) >> null 1 * repository.withProgressLogging() >> progressLoggingRepo - 1 * progressLoggingRepo.getResource(uri) >> remoteResource + 1 * progressLoggingRepo.getResource(uri, true) >> remoteResource 1 * remoteResource.withContent(_) >> { ExternalResource.ContentAction a -> a.execute(new ByteArrayInputStream(), remoteMetaData) } @@ -306,7 +306,7 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { timeProvider.currentTime >> 24000L cached.cachedAt >> 23999L cached.externalResourceMetaData >> cachedMetaData - 1 * repository.getResourceMetaData(uri) >> remoteMetaData + 1 * repository.getResourceMetaData(uri, true) >> remoteMetaData localCandidates.none >> false remoteMetaData.sha1 >> sha1 remoteMetaData.etag >> null @@ -317,7 +317,7 @@ class DefaultCacheAwareExternalResourceAccessorTest extends Specification { localCandidate.file >> candidate cached.cachedFile >> cachedFile 1 * repository.withProgressLogging() >> progressLoggingRepo - 1 * progressLoggingRepo.getResource(uri) >> remoteResource + 1 * progressLoggingRepo.getResource(uri, true) >> remoteResource 1 * remoteResource.withContent(_) >> { ExternalResource.ContentAction a -> a.execute(new ByteArrayInputStream(), remoteMetaData) } diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessorTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessorTest.groovy index f3e9cb3f5f96..d7c691cd0608 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessorTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/internal/resource/transfer/ProgressLoggingExternalResourceAccessorTest.groovy @@ -38,10 +38,10 @@ class ProgressLoggingExternalResourceAccessorTest extends Specification { @Unroll def "delegates #method to delegate resource accessor"() { when: - progressLoggerAccessor."$method"(new URI("location")) + progressLoggerAccessor."$method"(new URI("location"), false) then: - 1 * accessor."$method"(new URI("location")) + 1 * accessor."$method"(new URI("location"), false) where: method << ['getMetaData', 'openResource'] @@ -49,10 +49,10 @@ class ProgressLoggingExternalResourceAccessorTest extends Specification { def "getResource returns null when delegate returns null"() { setup: - accessor.openResource(new URI("location")) >> null + accessor.openResource(new URI("location"), false) >> null when: - def loadedResource = progressLoggerAccessor.openResource(new URI("location")) + def loadedResource = progressLoggerAccessor.openResource(new URI("location"), false) then: loadedResource == null @@ -60,10 +60,10 @@ class ProgressLoggingExternalResourceAccessorTest extends Specification { def "wraps response in delegate"() { setup: - accessor.openResource(new URI("location")) >> externalResource + accessor.openResource(new URI("location"), false) >> externalResource when: - def loadedResource = progressLoggerAccessor.openResource(new URI("location")) + def loadedResource = progressLoggerAccessor.openResource(new URI("location"), false) then: loadedResource != null @@ -80,12 +80,12 @@ class ProgressLoggingExternalResourceAccessorTest extends Specification { def "fires progress events as content is read"() { setup: - accessor.openResource(new URI("location")) >> externalResource + accessor.openResource(new URI("location"), false) >> externalResource metaData.getContentLength() >> 4096 externalResource.openStream() >> new ByteArrayInputStream(new byte[4096]) when: - def resource = progressLoggerAccessor.openResource(new URI("location")) + def resource = progressLoggerAccessor.openResource(new URI("location"), false) def inputStream = resource.openStream() inputStream.read() inputStream.read() @@ -108,12 +108,12 @@ class ProgressLoggingExternalResourceAccessorTest extends Specification { def "fires complete event when response closed with partially read stream"() { setup: - accessor.openResource(new URI("location")) >> externalResource + accessor.openResource(new URI("location"), false) >> externalResource metaData.getContentLength() >> 4096 externalResource.openStream() >> new ByteArrayInputStream(new byte[4096]) when: - def resource = progressLoggerAccessor.openResource(new URI("location")) + def resource = progressLoggerAccessor.openResource(new URI("location"), false) def inputStream = resource.openStream() inputStream.read(new byte[1600]) resource.close() @@ -128,12 +128,12 @@ class ProgressLoggingExternalResourceAccessorTest extends Specification { def "no progress events logged for resources smaller 1024 bytes"() { setup: - accessor.openResource(new URI("location")) >> externalResource + accessor.openResource(new URI("location"), false) >> externalResource metaData.getContentLength() >> 1023 externalResource.openStream() >> new ByteArrayInputStream(new byte[1023]) when: - def resource = progressLoggerAccessor.openResource(new URI("location")) + def resource = progressLoggerAccessor.openResource(new URI("location"), false) def inputStream = resource.openStream() inputStream.read(new byte[1024]) resource.close() @@ -144,4 +144,4 @@ class ProgressLoggingExternalResourceAccessorTest extends Specification { 1 * progressLogger.completed() 0 * progressLogger.progress(_) } -} \ No newline at end of file +} diff --git a/subprojects/maven/src/main/java/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapter.java b/subprojects/maven/src/main/java/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapter.java index ff5e473dbf21..64308c2c8fa4 100644 --- a/subprojects/maven/src/main/java/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapter.java +++ b/subprojects/maven/src/main/java/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapter.java @@ -37,7 +37,7 @@ public RepositoryTransportWagonAdapter(RepositoryTransport transport, URI rootUr public boolean getRemoteFile(File destination, String resourceName) throws ResourceException { URI uriForResource = getUriForResource(resourceName); - ExternalResource resource = transport.getRepository().getResource(uriForResource); + ExternalResource resource = transport.getRepository().getResource(uriForResource, false); if (resource == null) { return false; } diff --git a/subprojects/maven/src/test/groovy/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapterTest.groovy b/subprojects/maven/src/test/groovy/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapterTest.groovy index 1428896f3642..c0039388a8b2 100644 --- a/subprojects/maven/src/test/groovy/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapterTest.groovy +++ b/subprojects/maven/src/test/groovy/org/gradle/api/publication/maven/internal/wagon/RepositoryTransportWagonAdapterTest.groovy @@ -39,7 +39,7 @@ class RepositoryTransportWagonAdapterTest extends Specification { delegate.getRemoteFile(null, resourceName) then: - 1 * repositoryTransport.getRepository().getResource({ it.toString() == expected }) + 1 * repositoryTransport.getRepository().getResource({ it.toString() == expected }, false) where: repoUrl | resourceName | expected S3_URI | 'a/b/some.jar' | 's3://somewhere/maven/a/b/some.jar' @@ -52,7 +52,7 @@ class RepositoryTransportWagonAdapterTest extends Specification { RepositoryTransport repositoryTransport = Mock() ExternalResourceRepository externalResourceRepo = Mock() repositoryTransport.getRepository() >> externalResourceRepo - externalResourceRepo.getResource(_) >> Mock(ExternalResource) + externalResourceRepo.getResource(_, false) >> Mock(ExternalResource) RepositoryTransportWagonAdapter delegate = new RepositoryTransportWagonAdapter(repositoryTransport, S3_URI) @@ -65,7 +65,7 @@ class RepositoryTransportWagonAdapterTest extends Specification { RepositoryTransport repositoryTransport = Mock() ExternalResourceRepository externalResourceRepo = Mock() repositoryTransport.getRepository() >> externalResourceRepo - externalResourceRepo.getResource(_) >> null + externalResourceRepo.getResource(_, false) >> null RepositoryTransportWagonAdapter delegate = new RepositoryTransportWagonAdapter(repositoryTransport, S3_URI) diff --git a/subprojects/plugin-use/src/main/java/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClient.java b/subprojects/plugin-use/src/main/java/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClient.java index de72e78cb110..d67563d8a663 100644 --- a/subprojects/plugin-use/src/main/java/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClient.java +++ b/subprojects/plugin-use/src/main/java/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClient.java @@ -83,7 +83,7 @@ private Response request(final String requestUrl, final Class type, fi final URI requestUri = toUri(requestUrl, "plugin request"); try { - HttpResponseResource response = getResourceAccessor().getRawResource(requestUri); + HttpResponseResource response = getResourceAccessor().getRawResource(requestUri, false); try { final int statusCode = response.getStatusCode(); String contentType = response.getContentType(); diff --git a/subprojects/plugin-use/src/test/groovy/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClientTest.groovy b/subprojects/plugin-use/src/test/groovy/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClientTest.groovy index 3ef4819a6a1a..036009c1471b 100644 --- a/subprojects/plugin-use/src/test/groovy/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClientTest.groovy +++ b/subprojects/plugin-use/src/test/groovy/org/gradle/plugin/use/resolve/service/internal/HttpPluginResolutionServiceClientTest.groovy @@ -120,17 +120,17 @@ class HttpPluginResolutionServiceClientTest extends Specification { client.queryPluginMetadata(PLUGIN_PORTAL_URL, true, customRequest) then: - 1 * resourceAccessor.getRawResource(new URI("$PLUGIN_PORTAL_URL/${GradleVersion.current().getVersion()}/plugin/use/foo%2Fbar/1%2F0")) >> Stub(HttpResponseResource) { + 1 * resourceAccessor.getRawResource(new URI("$PLUGIN_PORTAL_URL/${GradleVersion.current().getVersion()}/plugin/use/foo%2Fbar/1%2F0"), false) >> Stub(HttpResponseResource) { getStatusCode() >> 500 getContentType() >> "application/json" openStream() >> new ByteArrayInputStream("{errorCode: 'FOO', message: 'BAR'}".getBytes("utf8")) } - 0 * resourceAccessor.getRawResource(_) + 0 * resourceAccessor.getRawResource(_, false) } private void stubResponse(int statusCode, String jsonResponse = null) { interaction { - resourceAccessor.getRawResource(_) >> Stub(HttpResponseResource) { + resourceAccessor.getRawResource(_, false) >> Stub(HttpResponseResource) { getStatusCode() >> statusCode if (jsonResponse != null) { getContentType() >> "application/json" diff --git a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java index 002c2609478d..ef4e81ef26bc 100644 --- a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java +++ b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java @@ -16,6 +16,7 @@ package org.gradle.internal.resource.transport.http; +import org.apache.http.HttpHeaders; import org.apache.http.HttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpHead; @@ -46,25 +47,27 @@ public HttpClientHelper(HttpSettings settings) { this.settings = settings; } - public HttpResponse performRawHead(String source) { - return performRequest(new HttpHead(source)); + public HttpResponse performRawHead(String source, boolean revalidate) { + return performRequest(new HttpHead(source), revalidate); } - public HttpResponse performHead(String source) { - return processResponse(source, "HEAD", performRawHead(source)); + public HttpResponse performHead(String source, boolean revalidate) { + return processResponse(source, "HEAD", performRawHead(source, revalidate)); } - public HttpResponse performRawGet(String source) { - return performRequest(new HttpGet(source)); + public HttpResponse performRawGet(String source, boolean revalidate) { + return performRequest(new HttpGet(source), revalidate); } - public HttpResponse performGet(String source) { - return processResponse(source, "GET", performRawGet(source)); + public HttpResponse performGet(String source, boolean revalidate) { + return processResponse(source, "GET", performRawGet(source, revalidate)); } - public HttpResponse performRequest(HttpRequestBase request) { + public HttpResponse performRequest(HttpRequestBase request, boolean revalidate) { String method = request.getMethod(); - + if (revalidate) { + request.addHeader(HttpHeaders.CACHE_CONTROL, "max-age=0"); + } HttpResponse response; try { response = executeGetOrHead(request); @@ -77,7 +80,7 @@ public HttpResponse performRequest(HttpRequestBase request) { protected HttpResponse executeGetOrHead(HttpRequestBase method) throws IOException { HttpResponse httpResponse = performHttpRequest(method); - // Consume content for non-successful, responses. This avoids the connection being left open. + // Consume content for non-successful, responses. This avoids the connection being left open. if (!wasSuccessful(httpResponse)) { EntityUtils.consume(httpResponse.getEntity()); return httpResponse; diff --git a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java index 16ac2a7a2744..724e9318d1b0 100644 --- a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java +++ b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java @@ -17,8 +17,6 @@ package org.gradle.internal.resource.transport.http; import org.apache.http.HttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpRequestBase; import org.gradle.api.Nullable; import org.gradle.internal.resource.metadata.ExternalResourceMetaData; import org.gradle.internal.resource.transfer.ExternalResourceAccessor; @@ -43,12 +41,12 @@ public HttpResourceAccessor(HttpClientHelper http) { } @Nullable - public HttpResponseResource openResource(final URI uri) { + public HttpResponseResource openResource(final URI uri, boolean revalidate) { abortOpenResources(); String location = uri.toString(); LOGGER.debug("Constructing external resource: {}", location); - HttpResponse response = http.performGet(location); + HttpResponse response = http.performGet(location, revalidate); if (response != null) { HttpResponseResource resource = wrapResponse(uri, response); return recordOpenGetResource(resource); @@ -61,28 +59,20 @@ public HttpResponseResource openResource(final URI uri) { * Same as #getResource except that it always gives access to the response body, * irrespective of the returned HTTP status code. Never returns {@code null}. */ - public HttpResponseResource getRawResource(final URI uri) { + public HttpResponseResource getRawResource(final URI uri, boolean revalidate) { abortOpenResources(); String location = uri.toString(); LOGGER.debug("Constructing external resource: {}", location); - - HttpRequestBase request = new HttpGet(uri); - HttpResponse response; - try { - response = http.performHttpRequest(request); - } catch (IOException e) { - throw new HttpRequestException(String.format("Could not %s '%s'.", request.getMethod(), request.getURI()), e); - } - + HttpResponse response = http.performGet(location, revalidate); HttpResponseResource resource = wrapResponse(uri, response); return recordOpenGetResource(resource); } - public ExternalResourceMetaData getMetaData(URI uri) { + public ExternalResourceMetaData getMetaData(URI uri, boolean revalidate) { abortOpenResources(); String location = uri.toString(); LOGGER.debug("Constructing external resource metadata: {}", location); - HttpResponse response = http.performHead(location); + HttpResponse response = http.performHead(location, revalidate); return response == null ? null : new HttpResponseResource("HEAD", uri, response).getMetaData(); } diff --git a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceLister.java b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceLister.java index ccdfbc0137c9..879022cd2b19 100644 --- a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceLister.java +++ b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceLister.java @@ -34,7 +34,7 @@ public HttpResourceLister(HttpResourceAccessor accessor) { } public List list(final URI directory) { - final ExternalResourceReadResponse response = accessor.openResource(directory); + final ExternalResourceReadResponse response = accessor.openResource(directory, true); if (response == null) { return null; } diff --git a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy index 2490f10c617f..cf3cabbffe48 100644 --- a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy +++ b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy @@ -36,13 +36,29 @@ class HttpClientHelperTest extends Specification { } when: - client.performRequest(new HttpGet("http://gradle.org")) + client.performRequest(new HttpGet("http://gradle.org"), false) then: HttpRequestException e = thrown() e.cause.message == "ouch" } + def "request with revalidate adds Cache-Control header"() { + def client = new HttpClientHelper(httpSettings) { + @Override + protected HttpResponse executeGetOrHead(HttpRequestBase method) { + return null + } + } + + when: + def request = new HttpGet("http://gradle.org") + client.performRequest(request, true) + + then: + request.getHeaders("Cache-Control")[0].value == "max-age=0" + } + private HttpSettings getHttpSettings() { return Stub(HttpSettings) { getProxySettings() >> Mock(HttpProxySettings) diff --git a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpResourceListerTest.groovy b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpResourceListerTest.groovy index a510f99073e8..eeaf96ea092d 100644 --- a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpResourceListerTest.groovy +++ b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpResourceListerTest.groovy @@ -27,7 +27,7 @@ class HttpResourceListerTest extends Specification { def "consumeExternalResource closes resource after reading into stream"() { setup: - accessorMock.openResource(new URI("http://testrepo/")) >> externalResource; + accessorMock.openResource(new URI("http://testrepo/"), true) >> externalResource; when: lister.list(new URI("http://testrepo/")) then: @@ -39,7 +39,7 @@ class HttpResourceListerTest extends Specification { def "list returns null if HttpAccessor returns null"(){ setup: - accessorMock.openResource(new URI("http://testrepo/")) >> null + accessorMock.openResource(new URI("http://testrepo/"), true) >> null expect: null == lister.list(new URI("http://testrepo")) } diff --git a/subprojects/resources-s3/src/main/java/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnector.java b/subprojects/resources-s3/src/main/java/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnector.java index 33b9d5d52be5..856544c42c90 100644 --- a/subprojects/resources-s3/src/main/java/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnector.java +++ b/subprojects/resources-s3/src/main/java/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnector.java @@ -45,7 +45,7 @@ public List list(URI parent) { return s3Client.list(parent); } - public ExternalResourceReadResponse openResource(URI location) { + public ExternalResourceReadResponse openResource(URI location, boolean revalidate) { LOGGER.debug("Attempting to get resource: {}", location); S3Object s3Object = s3Client.getResource(location); if (s3Object == null) { @@ -54,7 +54,7 @@ public ExternalResourceReadResponse openResource(URI location) { return new S3Resource(s3Object, location); } - public ExternalResourceMetaData getMetaData(URI location) { + public ExternalResourceMetaData getMetaData(URI location, boolean revalidate) { LOGGER.debug("Attempting to get resource metadata: {}", location); S3Object s3Object = s3Client.getMetaData(location); if (s3Object == null) { diff --git a/subprojects/resources-s3/src/test/groovy/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnectorTest.groovy b/subprojects/resources-s3/src/test/groovy/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnectorTest.groovy index 716c6f8c48a1..08bd39983e48 100644 --- a/subprojects/resources-s3/src/test/groovy/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnectorTest.groovy +++ b/subprojects/resources-s3/src/test/groovy/org/gradle/internal/resource/transport/aws/s3/S3ResourceConnectorTest.groovy @@ -40,7 +40,7 @@ class S3ResourceConnectorTest extends Specification { } when: - def s3Resource = new S3ResourceConnector(s3Client).openResource(uri) + def s3Resource = new S3ResourceConnector(s3Client).openResource(uri, false) then: s3Resource != null diff --git a/subprojects/resources-sftp/src/main/java/org/gradle/internal/resource/transport/sftp/SftpResourceAccessor.java b/subprojects/resources-sftp/src/main/java/org/gradle/internal/resource/transport/sftp/SftpResourceAccessor.java index a74b3b94ed20..ff068bc3eb5e 100644 --- a/subprojects/resources-sftp/src/main/java/org/gradle/internal/resource/transport/sftp/SftpResourceAccessor.java +++ b/subprojects/resources-sftp/src/main/java/org/gradle/internal/resource/transport/sftp/SftpResourceAccessor.java @@ -37,7 +37,7 @@ public SftpResourceAccessor(SftpClientFactory sftpClientFactory, PasswordCredent this.credentials = credentials; } - public ExternalResourceMetaData getMetaData(URI uri) { + public ExternalResourceMetaData getMetaData(URI uri, boolean revalidate) { LockableSftpClient sftpClient = sftpClientFactory.createSftpClient(uri, credentials); try { SftpATTRS attributes = sftpClient.getSftpClient().lstat(uri.getPath()); @@ -66,8 +66,8 @@ private ExternalResourceMetaData toMetaData(URI uri, SftpATTRS attributes) { return new DefaultExternalResourceMetaData(uri, lastModified, contentLength); } - public ExternalResourceReadResponse openResource(URI location) { - ExternalResourceMetaData metaData = getMetaData(location); + public ExternalResourceReadResponse openResource(URI location, boolean revalidate) { + ExternalResourceMetaData metaData = getMetaData(location, revalidate); return metaData != null ? new SftpResource(sftpClientFactory, metaData, location, credentials) : null; } } diff --git a/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/DefaultExternalResourceConnector.java b/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/DefaultExternalResourceConnector.java index 1436fc584dbe..ecf5d26cb66b 100644 --- a/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/DefaultExternalResourceConnector.java +++ b/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/DefaultExternalResourceConnector.java @@ -51,16 +51,16 @@ public static ExternalResourceAccessStats getStatistics() { @Nullable @Override - public ExternalResourceReadResponse openResource(URI location) { + public ExternalResourceReadResponse openResource(URI location, boolean revalidate) { STATS.resource(location); - return accessor.openResource(location); + return accessor.openResource(location, revalidate); } @Nullable @Override - public ExternalResourceMetaData getMetaData(URI location) { + public ExternalResourceMetaData getMetaData(URI location, boolean revalidate) { STATS.metadata(location); - return accessor.getMetaData(location); + return accessor.getMetaData(location, revalidate); } @Nullable diff --git a/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/ExternalResourceAccessor.java b/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/ExternalResourceAccessor.java index b5ff2d9f8e4b..58e0931d1036 100644 --- a/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/ExternalResourceAccessor.java +++ b/subprojects/resources/src/main/java/org/gradle/internal/resource/transfer/ExternalResourceAccessor.java @@ -33,11 +33,12 @@ public interface ExternalResourceAccessor { * must throw an {@link ResourceException} to indicate a fatal condition. * * @param location The address of the resource to obtain + * @param revalidate The resource should be revalidated as part of the request * @return The resource if it exists, otherwise null. Caller is responsible for closing the result. * @throws ResourceException If the resource may exist, but not could be obtained for some reason. */ @Nullable - ExternalResourceReadResponse openResource(URI location) throws ResourceException; + ExternalResourceReadResponse openResource(URI location, boolean revalidate) throws ResourceException; /** * Obtains only the metadata about the resource. @@ -48,10 +49,11 @@ public interface ExternalResourceAccessor { * must throw an {@link ResourceException} to indicate a fatal condition. * * @param location The location of the resource to obtain the metadata for + * @param revalidate The resource should be revalidated as part of the request * @return The available metadata, null if the resource doesn't exist * @throws ResourceException If the resource may exist, but not could be obtained for some reason */ @Nullable - ExternalResourceMetaData getMetaData(URI location) throws ResourceException; - + ExternalResourceMetaData getMetaData(URI location, boolean revalidate) throws ResourceException; + } From 356e5e4d6106ee934e4cd5f87245afbabde47ebd Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Mon, 11 Apr 2016 11:28:17 -0700 Subject: [PATCH 2/4] Add integration test to ensure that --refresh-dependencies causes a cached artifact to be retrieved with Cache-Control:max-age=0 --- .../fixtures/server/http/HttpServer.groovy | 24 +++++++++++++++++-- .../server/http/MavenHttpModule.groovy | 7 ++++++ ...ginResolutionCachingIntegrationTest.groovy | 6 ++++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy index 5dc19c2185dc..c3f9409eabef 100755 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy @@ -219,6 +219,13 @@ class HttpServer extends ServerWithExpectations { allow(path, true, ['GET', 'HEAD'], fileHandler(path, srcFile)) } + /** + * Adds a given file at the given URL. The source file can be either a file or a directory. + */ + void allowGetOrHeadWithRevalidate(String path, File srcFile) { + allow(path, true, ['GET', 'HEAD'], revalidateFileHandler(path, srcFile)) + } + /** * Adds a given file at the given URL with the given credentials. The source file can be either a file or a directory. */ @@ -234,17 +241,23 @@ class HttpServer extends ServerWithExpectations { } private Action fileHandler(String path, File srcFile) { - return new SendFileAction(path, srcFile) + return new SendFileAction(path, srcFile, false) + } + + private Action revalidateFileHandler(String path, File srcFile) { + return new SendFileAction(path, srcFile, true) } class SendFileAction extends ActionSupport { private final String path private final File srcFile + private final boolean revalidate - SendFileAction(String path, File srcFile) { + SendFileAction(String path, File srcFile, boolean revalidate) { super("return contents of $srcFile.name") this.srcFile = srcFile this.path = path + this.revalidate = revalidate } void handle(HttpServletRequest request, HttpServletResponse response) { @@ -255,6 +268,13 @@ class HttpServer extends ServerWithExpectations { return; } } + if (revalidate) { + String cacheControl = request.getHeader("Cache-Control") + if (!cacheControl.equals("max-age=0")) { + response.sendError(412, String.format("Precondition Failed: Expected Cache-Control:max-age=0 but was '%s'", cacheControl)); + return + } + } def file if (request.pathInfo == path) { file = srcFile diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/MavenHttpModule.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/MavenHttpModule.groovy index 853eca2d4253..8adbd17b79c6 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/MavenHttpModule.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/MavenHttpModule.groovy @@ -78,6 +78,13 @@ class MavenHttpModule extends DelegatingMavenModule implements return this } + MavenHttpModule revalidate() { + server.allowGetOrHeadMissing(pomPath) + server.allowGetOrHeadMissing(metaDataPath) + server.allowGetOrHeadWithRevalidate(artifactPath, artifactFile) + return this + } + void missing() { server.allowGetOrHeadMissing(pomPath) server.allowGetOrHeadMissing(metaDataPath) diff --git a/subprojects/plugin-use/src/integTest/groovy/org/gradle/plugin/use/resolve/service/PluginResolutionCachingIntegrationTest.groovy b/subprojects/plugin-use/src/integTest/groovy/org/gradle/plugin/use/resolve/service/PluginResolutionCachingIntegrationTest.groovy index 6db02040a1b3..7f0d1e6c4428 100644 --- a/subprojects/plugin-use/src/integTest/groovy/org/gradle/plugin/use/resolve/service/PluginResolutionCachingIntegrationTest.groovy +++ b/subprojects/plugin-use/src/integTest/groovy/org/gradle/plugin/use/resolve/service/PluginResolutionCachingIntegrationTest.groovy @@ -69,7 +69,7 @@ class PluginResolutionCachingIntegrationTest extends AbstractIntegrationSpec { reset() args "--refresh-dependencies" pluginQuery() - moduleResolution() + moduleResolutionWithRevalidate() build() reset() @@ -168,6 +168,10 @@ class PluginResolutionCachingIntegrationTest extends AbstractIntegrationSpec { module.allowAll() } + void moduleResolutionWithRevalidate() { + module.revalidate() + } + void pluginQuery() { service.expectPluginQuery(PLUGIN_ID, VERSION, GROUP, ARTIFACT, VERSION) } From d7ddcf1e3db242c8d9179b2be86960417d8bb9d1 Mon Sep 17 00:00:00 2001 From: Danny Thomas Date: Tue, 12 Apr 2016 09:46:14 -0700 Subject: [PATCH 3/4] Add expectations to CachedChangingModulesIntegrationTest to ensure that revalidation occurs when resolution strategy cache settings expire --- ...achedChangingModulesIntegrationTest.groovy | 48 +++++++++---------- .../server/http/AbstractHttpResource.groovy | 4 ++ .../server/http/HttpDirectoryResource.groovy | 10 ++++ .../fixtures/server/http/HttpResource.groovy | 9 ++++ .../fixtures/server/http/HttpServer.groovy | 14 ++++++ 5 files changed, 61 insertions(+), 24 deletions(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/caching/CachedChangingModulesIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/caching/CachedChangingModulesIntegrationTest.groovy index e134d7eceb7c..b0d8b87f8a52 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/caching/CachedChangingModulesIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/caching/CachedChangingModulesIntegrationTest.groovy @@ -62,9 +62,9 @@ public class CachedChangingModulesIntegrationTest extends AbstractHttpDependency when: server.resetExpectations() - module.metaData.expectGet() - sourceArtifact.expectHead() - module.pom.expectHead() + module.metaData.expectGetRevalidate() + sourceArtifact.expectHeadRevalidate() + module.pom.expectHeadRevalidate() then: run 'retrieve' @@ -72,13 +72,13 @@ public class CachedChangingModulesIntegrationTest extends AbstractHttpDependency module.publishWithChangedContent() server.resetExpectations() - module.metaData.expectGet() - module.pom.sha1.expectGet() - module.pom.expectHead() - module.pom.expectGet() - sourceArtifact.expectHead() - sourceArtifact.expectGet() - sourceArtifact.sha1.expectGet() + module.metaData.expectGetRevalidate() + module.pom.sha1.expectGetRevalidate() + module.pom.expectHeadRevalidate() + module.pom.expectGetRevalidate() + sourceArtifact.expectHeadRevalidate() + sourceArtifact.expectGetRevalidate() + sourceArtifact.sha1.expectGetRevalidate() then: run 'retrieve' @@ -133,8 +133,8 @@ public class CachedChangingModulesIntegrationTest extends AbstractHttpDependency when: server.resetExpectations() module.metaData.expectGetMissing() - sourceArtifact.expectHead() - module.pom.expectHead() + sourceArtifact.expectHeadRevalidate() + module.pom.expectHeadRevalidate() then: run 'retrieve' @@ -143,12 +143,12 @@ public class CachedChangingModulesIntegrationTest extends AbstractHttpDependency server.resetExpectations() module.metaData.expectGetMissing() - module.pom.sha1.expectGet() - module.pom.expectHead() - module.pom.expectGet() - sourceArtifact.expectHead() - sourceArtifact.expectGet() - sourceArtifact.sha1.expectGet() + module.pom.sha1.expectGetRevalidate() + module.pom.expectHeadRevalidate() + module.pom.expectGetRevalidate() + sourceArtifact.expectHeadRevalidate() + sourceArtifact.expectGetRevalidate() + sourceArtifact.sha1.expectGetRevalidate() then: run 'retrieve' @@ -200,7 +200,7 @@ public class CachedChangingModulesIntegrationTest extends AbstractHttpDependency when: server.resetExpectations() - module.ivy.expectHead() + module.ivy.expectHeadRevalidate() module.getArtifact(classifier: 'source').expectHead() then: run 'retrieve' @@ -208,13 +208,13 @@ public class CachedChangingModulesIntegrationTest extends AbstractHttpDependency when: module.publishWithChangedContent() server.resetExpectations() - module.ivy.expectHead() + module.ivy.expectHeadRevalidate() module.getArtifact(classifier: 'source').expectHead() - module.ivy.sha1.expectGet() - module.ivy.expectGet() - module.getArtifact(classifier: 'source').expectGet() - module.getArtifact(classifier: 'source').sha1.expectGet() + module.ivy.sha1.expectGetRevalidate() + module.ivy.expectGetRevalidate() + module.getArtifact(classifier: 'source').expectGetRevalidate() + module.getArtifact(classifier: 'source').sha1.expectGetRevalidate() then: run 'retrieve' diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/AbstractHttpResource.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/AbstractHttpResource.groovy index 4d257a7fce16..c92e3d6c5569 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/AbstractHttpResource.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/AbstractHttpResource.groovy @@ -39,12 +39,16 @@ abstract class AbstractHttpResource implements RemoteResource { abstract void expectGetMissing() + abstract void expectGetRevalidate() + abstract void expectHead() abstract void expectHeadBroken() abstract void expectHeadMissing() + abstract void expectHeadRevalidate() + abstract void expectPut() abstract void expectPutBroken() diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpDirectoryResource.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpDirectoryResource.groovy index 4fee010ace2a..12c2fb862818 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpDirectoryResource.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpDirectoryResource.groovy @@ -48,6 +48,11 @@ class HttpDirectoryResource extends AbstractHttpResource { server.expectGetBroken(path) } + @Override + void expectGetRevalidate() { + server.expectGetRevalidate(path, directory) + } + @Override void expectHead() { throw new UnsupportedOperationException() @@ -63,6 +68,11 @@ class HttpDirectoryResource extends AbstractHttpResource { throw new UnsupportedOperationException() } + @Override + void expectHeadRevalidate() { + throw new UnsupportedOperationException() + } + @Override void expectPut() { throw new UnsupportedOperationException() diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpResource.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpResource.groovy index 391edb79423c..b27e9a9f681b 100644 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpResource.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpResource.groovy @@ -49,6 +49,11 @@ abstract class HttpResource extends AbstractHttpResource { server.expectGetMissing(getPath(), credentials) } + @Override + void expectGetRevalidate() { + server.expectGetRevalidate(getPath(), file) + } + void expectHead() { server.expectHead(getPath(), file) } @@ -61,6 +66,10 @@ abstract class HttpResource extends AbstractHttpResource { server.expectHeadBroken(path) } + void expectHeadRevalidate() { + server.expectHeadRevalidate(path, file) + } + void expectPut(PasswordCredentials credentials) { expectPut(200, credentials) } diff --git a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy index c3f9409eabef..76b60231bd4e 100755 --- a/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy +++ b/subprojects/internal-integ-testing/src/main/groovy/org/gradle/test/fixtures/server/http/HttpServer.groovy @@ -354,6 +354,13 @@ class HttpServer extends ServerWithExpectations { expect(path, false, ['HEAD'], fileHandler(path, srcFile)) } + /** + * Expects one HEAD request for the given URL, asserting that the request is revalidated. + */ + void expectHeadRevalidate(String path, File srcFile) { + expect(path, false, ['HEAD'], revalidateFileHandler(path, srcFile)) + } + /** * Allows one HEAD request for the given URL with http authentication. */ @@ -368,6 +375,13 @@ class HttpServer extends ServerWithExpectations { return expect(path, false, ['GET'], fileHandler(path, srcFile)) } + /** + * Allows one GET request for the given URL, asserting that the request revalidates. Reads the request content from the given file. + */ + HttpResourceInteraction expectGetRevalidate(String path, File srcFile) { + return expect(path, false, ['GET'], revalidateFileHandler(path, srcFile)) + } + /** * Expects one GET request for the given URL, with the given credentials. Reads the request content from the given file. */ From fef98b6fa382d611e6566a18ba496813800ca2f6 Mon Sep 17 00:00:00 2001 From: Cedric Champeau Date: Tue, 4 Oct 2016 11:45:46 +0200 Subject: [PATCH 4/4] Fixed failing integration tests due to `null` being returned instead of a proper http response --- .../transport/http/HttpClientHelper.java | 166 +++++++++++++++++- .../transport/http/HttpResourceAccessor.java | 2 +- .../transport/http/HttpResponseResource.java | 2 +- .../http/AbstractHttpClientTest.groovy | 2 +- .../http/HttpClientHelperTest.groovy | 2 +- 5 files changed, 167 insertions(+), 7 deletions(-) diff --git a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java index 844cb03506de..6847dd52d3e7 100644 --- a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java +++ b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpClientHelper.java @@ -16,13 +16,19 @@ package org.gradle.internal.resource.transport.http; -import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.Header; +import org.apache.http.HeaderIterator; +import org.apache.http.HttpEntity; import org.apache.http.HttpHeaders; +import org.apache.http.ProtocolVersion; +import org.apache.http.StatusLine; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpHead; import org.apache.http.client.methods.HttpRequestBase; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.client.utils.HttpClientUtils; +import org.apache.http.entity.BufferedHttpEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.protocol.BasicHttpContext; @@ -32,6 +38,7 @@ import java.io.Closeable; import java.io.IOException; +import java.util.Locale; /** * Provides some convenience and unified logging. @@ -79,11 +86,12 @@ public CloseableHttpResponse performRequest(HttpRequestBase request, boolean rev } protected CloseableHttpResponse executeGetOrHead(HttpRequestBase method) throws IOException { - CloseableHttpResponse httpResponse = performHttpRequest(method); + final CloseableHttpResponse httpResponse = performHttpRequest(method); // Consume content for non-successful, responses. This avoids the connection being left open. if (!wasSuccessful(httpResponse)) { + CloseableHttpResponse response = new AutoClosedHttpResponse(httpResponse); HttpClientUtils.closeQuietly(httpResponse); - return httpResponse; + return response; } return httpResponse; } @@ -135,4 +143,156 @@ public synchronized void close() throws IOException { client.close(); } } + + private static class AutoClosedHttpResponse implements CloseableHttpResponse { + private final HttpEntity entity; + private final CloseableHttpResponse httpResponse; + + public AutoClosedHttpResponse(CloseableHttpResponse httpResponse) throws IOException { + this.httpResponse = httpResponse; + HttpEntity entity = httpResponse.getEntity(); + this.entity = entity !=null ? new BufferedHttpEntity(entity) : null; + } + + @Override + public void close() throws IOException { + } + + @Override + public StatusLine getStatusLine() { + return httpResponse.getStatusLine(); + } + + @Override + public void setStatusLine(StatusLine statusline) { + throw new UnsupportedOperationException(); + } + + @Override + public void setStatusLine(ProtocolVersion ver, int code) { + throw new UnsupportedOperationException(); + } + + @Override + public void setStatusLine(ProtocolVersion ver, int code, String reason) { + throw new UnsupportedOperationException(); + } + + @Override + public void setStatusCode(int code) throws IllegalStateException { + throw new UnsupportedOperationException(); + } + + @Override + public void setReasonPhrase(String reason) throws IllegalStateException { + throw new UnsupportedOperationException(); + } + + @Override + public HttpEntity getEntity() { + return entity; + } + + @Override + public void setEntity(HttpEntity entity) { + throw new UnsupportedOperationException(); + } + + @Override + public Locale getLocale() { + return httpResponse.getLocale(); + } + + @Override + public void setLocale(Locale loc) { + throw new UnsupportedOperationException(); + } + + @Override + public ProtocolVersion getProtocolVersion() { + return httpResponse.getProtocolVersion(); + } + + @Override + public boolean containsHeader(String name) { + return httpResponse.containsHeader(name); + } + + @Override + public Header[] getHeaders(String name) { + return httpResponse.getHeaders(name); + } + + @Override + public Header getFirstHeader(String name) { + return httpResponse.getFirstHeader(name); + } + + @Override + public Header getLastHeader(String name) { + return httpResponse.getLastHeader(name); + } + + @Override + public Header[] getAllHeaders() { + return httpResponse.getAllHeaders(); + } + + @Override + public void addHeader(Header header) { + throw new UnsupportedOperationException(); + } + + @Override + public void addHeader(String name, String value) { + throw new UnsupportedOperationException(); + } + + @Override + public void setHeader(Header header) { + throw new UnsupportedOperationException(); + } + + @Override + public void setHeader(String name, String value) { + throw new UnsupportedOperationException(); + } + + @Override + public void setHeaders(Header[] headers) { + throw new UnsupportedOperationException(); + } + + @Override + public void removeHeader(Header header) { + throw new UnsupportedOperationException(); + } + + @Override + public void removeHeaders(String name) { + throw new UnsupportedOperationException(); + } + + @Override + public HeaderIterator headerIterator() { + return httpResponse.headerIterator(); + } + + @Override + public HeaderIterator headerIterator(String name) { + return httpResponse.headerIterator(name); + } + + @Override + @SuppressWarnings("deprecation") + public org.apache.http.params.HttpParams getParams() { + return httpResponse.getParams(); + } + + @Override + @SuppressWarnings("deprecation") + public void setParams(org.apache.http.params.HttpParams params) { + throw new UnsupportedOperationException(); + } + } } diff --git a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java index 9d42ba5c5682..548446a7ccd5 100644 --- a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java +++ b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResourceAccessor.java @@ -63,7 +63,7 @@ public HttpResponseResource getRawResource(final URI uri, boolean revalidate) { abortOpenResources(); String location = uri.toString(); LOGGER.debug("Constructing external resource: {}", location); - CloseableHttpResponse response = http.performGet(location, revalidate); + CloseableHttpResponse response = http.performRawGet(location, revalidate); HttpResponseResource resource = wrapResponse(uri, response); return recordOpenGetResource(resource); } diff --git a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResponseResource.java b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResponseResource.java index 28f7a5278c43..cdfb0fd14782 100644 --- a/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResponseResource.java +++ b/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/HttpResponseResource.java @@ -113,7 +113,7 @@ public boolean isLocal() { } public InputStream openStream() throws IOException { - if(wasOpened){ + if (wasOpened) { throw new IOException("Unable to open Stream as it was opened before."); } LOGGER.debug("Attempting to download resource {}.", source); diff --git a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/AbstractHttpClientTest.groovy b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/AbstractHttpClientTest.groovy index 2a248e738bc9..dcd3cf2958e9 100644 --- a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/AbstractHttpClientTest.groovy +++ b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/AbstractHttpClientTest.groovy @@ -48,7 +48,7 @@ abstract class AbstractHttpClientTest extends Specification { def content = mockedHttpResponse.content _ * response.getStatusLine() >> new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 400, "I'm broken") 1 * response.close() - 1 * response.getEntity() >> entity + _ * response.getEntity() >> entity 1 * entity.isStreaming() >> true 1 * entity.content >> content 1 * content.close() diff --git a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy index e81da702b01c..d85c546712e8 100644 --- a/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy +++ b/subprojects/resources-http/src/test/groovy/org/gradle/internal/resource/transport/http/HttpClientHelperTest.groovy @@ -50,7 +50,7 @@ class HttpClientHelperTest extends AbstractHttpClientTest { MockedHttpResponse mockedHttpResponse = mockedHttpResponse() when: - client.performRequest(new HttpGet("http://gradle.org")) + client.performRequest(new HttpGet("http://gradle.org"), false) then: interaction {