Bug 413867 - Concurrent artifact resolution using same repo session (data) in combination with enhanced local repo can fail sporadically
Fixed interaction between DefaultArtifactResolver and DefaultUpdateCheckManager to ensure download requests for unavailable artifacts are not suppressed unless a resolution error has been cached
diff --git a/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java b/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java
index 4b0dd2f..536a70e 100644
--- a/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java
+++ b/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java
@@ -594,7 +594,7 @@
new UpdateCheck<Artifact, ArtifactTransferException>();
check.setItem( artifact );
check.setFile( download.getFile() );
- check.setFileValid( !download.isExistenceCheck() );
+ check.setFileValid( false );
check.setRepository( group.repository );
check.setPolicy( policy.getUpdatePolicy() );
item.updateCheck = check;
diff --git a/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java b/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
index fafaed6..51ba4d8 100644
--- a/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
+++ b/aether-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
@@ -114,7 +114,7 @@
{
if ( logger.isDebugEnabled() )
{
- logger.debug( "Skipped remote update check for " + check.getItem()
+ logger.debug( "Skipped remote request for " + check.getItem()
+ ", locally installed artifact up-to-date." );
}
@@ -142,14 +142,18 @@
String error = getError( props, dataKey );
long lastUpdated;
- if ( fileExists )
+ if ( error == null )
{
- lastUpdated = artifactFile.lastModified();
- }
- else if ( error == null )
- {
- // this is the first attempt ever
- lastUpdated = 0;
+ if ( fileExists )
+ {
+ // last update was successful
+ lastUpdated = artifactFile.lastModified();
+ }
+ else
+ {
+ // this is the first attempt ever
+ lastUpdated = 0;
+ }
}
else if ( error.length() <= 0 )
{
@@ -163,11 +167,15 @@
lastUpdated = getLastUpdated( props, transferKey );
}
- if ( isAlreadyUpdated( session.getData(), updateKey ) )
+ if ( lastUpdated == 0 )
+ {
+ check.setRequired( true );
+ }
+ else if ( isAlreadyUpdated( session.getData(), updateKey ) )
{
if ( logger.isDebugEnabled() )
{
- logger.debug( "Skipped remote update check for " + check.getItem()
+ logger.debug( "Skipped remote request for " + check.getItem()
+ ", already updated during this session." );
}
@@ -177,10 +185,6 @@
check.setException( newException( error, artifact, repository ) );
}
}
- else if ( lastUpdated == 0 )
- {
- check.setRequired( true );
- }
else if ( isUpdatedRequired( session, lastUpdated, check.getPolicy() ) )
{
check.setRequired( true );
@@ -189,8 +193,7 @@
{
if ( logger.isDebugEnabled() )
{
- logger.debug( "Skipped remote update check for " + check.getItem()
- + ", locally cached artifact up-to-date." );
+ logger.debug( "Skipped remote request for " + check.getItem() + ", locally cached artifact up-to-date." );
}
check.setRequired( false );
@@ -248,7 +251,7 @@
{
if ( logger.isDebugEnabled() )
{
- logger.debug( "Skipped remote update check for " + check.getItem()
+ logger.debug( "Skipped remote request for " + check.getItem()
+ ", locally installed metadata up-to-date." );
}
@@ -301,11 +304,15 @@
lastUpdated = getLastUpdated( props, transferKey );
}
- if ( isAlreadyUpdated( session.getData(), updateKey ) )
+ if ( lastUpdated == 0 )
+ {
+ check.setRequired( true );
+ }
+ else if ( isAlreadyUpdated( session.getData(), updateKey ) )
{
if ( logger.isDebugEnabled() )
{
- logger.debug( "Skipped remote update check for " + check.getItem()
+ logger.debug( "Skipped remote request for " + check.getItem()
+ ", already updated during this session." );
}
@@ -315,10 +322,6 @@
check.setException( newException( error, metadata, repository ) );
}
}
- else if ( lastUpdated == 0 )
- {
- check.setRequired( true );
- }
else if ( isUpdatedRequired( session, lastUpdated, check.getPolicy() ) )
{
check.setRequired( true );
@@ -327,8 +330,7 @@
{
if ( logger.isDebugEnabled() )
{
- logger.debug( "Skipped remote update check for " + check.getItem()
- + ", locally cached metadata up-to-date." );
+ logger.debug( "Skipped remote request for " + check.getItem() + ", locally cached metadata up-to-date." );
}
check.setRequired( false );
diff --git a/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java b/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java
index c67c9df..0b884a2 100644
--- a/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java
+++ b/aether-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManagerTest.java
@@ -69,7 +69,8 @@
TestFileUtils.writeString( artifactFile, "artifact" );
session = TestUtils.newSession();
- repository = new RemoteRepository.Builder( "id", "default", TestFileUtils.createTempDir().toURI().toURL().toString() ).build();
+ repository =
+ new RemoteRepository.Builder( "id", "default", TestFileUtils.createTempDir().toURI().toURL().toString() ).build();
manager = new DefaultUpdateCheckManager().setUpdatePolicyAnalyzer( new DefaultUpdatePolicyAnalyzer() );
metadata =
new DefaultMetadata( "gid", "aid", "ver", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT,
@@ -334,9 +335,18 @@
manager.checkMetadata( session, check );
assertEquals( true, check.isRequired() );
+ // first touch, without exception
manager.touchMetadata( session, check );
- // second check in same session
+ // another check in same session
+ manager.checkMetadata( session, check );
+ assertEquals( true, check.isRequired() );
+
+ // another touch, with exception
+ check.setException( new MetadataNotFoundException( check.getItem(), check.getRepository() ) );
+ manager.touchMetadata( session, check );
+
+ // another check in same session
manager.checkMetadata( session, check );
assertEquals( false, check.isRequired() );
}
@@ -646,9 +656,18 @@
manager.checkArtifact( session, check );
assertEquals( true, check.isRequired() );
+ // first touch, without exception
manager.touchArtifact( session, check );
- // second check in same session
+ // another check in same session
+ manager.checkArtifact( session, check );
+ assertEquals( true, check.isRequired() );
+
+ // another touch, with exception
+ check.setException( new ArtifactNotFoundException( check.getItem(), check.getRepository() ) );
+ manager.touchArtifact( session, check );
+
+ // another check in same session
manager.checkArtifact( session, check );
assertEquals( false, check.isRequired() );
}
