Skip to content

Commit

Permalink
Merge pull request #19798 Do not share resolvable artifacts between v…
Browse files Browse the repository at this point in the history
…erified and non-verified resolve contexts
  • Loading branch information
bot-gradle committed Feb 2, 2022
2 parents b20b3c5 + 72500b3 commit 88ab9b6
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,6 @@ private File maybeFetchSignatureFile(ModuleComponentIdentifier moduleComponentId
@Override
public void resolveArtifacts(ComponentResolveMetadata component, ConfigurationMetadata variant, BuildableComponentArtifactsResolveResult result) {
delegate.resolveArtifacts(component, variant, result);
if (result.hasResult() && result.isSuccessful()) {
for (ComponentArtifactMetadata artifact : variant.getArtifacts()) {
if (isExternalArtifactId(artifact.getId()) && !operation.wasAlreadyProcessed((ModuleComponentArtifactIdentifier) artifact.getId(), getId())) {
resolveArtifact(artifact, component.getSources(), new DefaultBuildableArtifactResolveResult());
}
}
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public ComponentResolvers create(String resolveContextName,
moduleComponentRepository = startParameterResolutionOverride.overrideModuleVersionRepository(moduleComponentRepository);
moduleComponentRepository = new CachingModuleComponentRepository(moduleComponentRepository, cacheProvider.getPersistentCaches(), cachePolicy, timeProvider, componentMetadataProcessor, listener);
}
moduleComponentRepository = cacheProvider.getResolvedArtifactCaches().provideResolvedArtifactCache(moduleComponentRepository);
moduleComponentRepository = cacheProvider.getResolvedArtifactCaches().provideResolvedArtifactCache(moduleComponentRepository, resolutionStrategy.isDependencyVerificationEnabled());

if (baseRepository.isDynamicResolveMode()) {
moduleComponentRepository = new IvyDynamicResolveModuleComponentRepository(moduleComponentRepository);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

public interface ArtifactVerificationOperation {
void onArtifact(ArtifactKind kind, ModuleComponentArtifactIdentifier artifact, File mainFile, Factory<File> signatureFile, String repositoryName, String repositoryId);
boolean wasAlreadyProcessed(ModuleComponentArtifactIdentifier artifact, String repositoryId);

enum ArtifactKind {
METADATA,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.common.collect.Queues;
import com.google.common.collect.Sets;
import org.gradle.api.artifacts.component.ComponentArtifactIdentifier;
import org.gradle.api.artifacts.component.ModuleComponentIdentifier;
import org.gradle.api.artifacts.result.ResolvedArtifactResult;
import org.gradle.api.artifacts.result.ResolvedVariantResult;
import org.gradle.api.artifacts.verification.DependencyVerificationMode;
Expand Down Expand Up @@ -70,7 +69,7 @@ public class ChecksumAndSignatureVerificationOverride implements DependencyVerif
private final ChecksumService checksumService;
private final SignatureVerificationService signatureVerificationService;
private final DependencyVerificationMode verificationMode;
private final Set<String> verificationQueries = Sets.newConcurrentHashSet();
private final Set<VerificationQuery> verificationQueries = Sets.newConcurrentHashSet();
private final Deque<VerificationEvent> verificationEvents = Queues.newArrayDeque();
private final AtomicBoolean closed = new AtomicBoolean();
private final AtomicBoolean hasFatalFailure = new AtomicBoolean();
Expand Down Expand Up @@ -108,19 +107,14 @@ private List<URI> keyServers() {

@Override
public void onArtifact(ArtifactKind kind, ModuleComponentArtifactIdentifier artifact, File mainFile, Factory<File> signatureFile, String repositoryName, String repositoryId) {
if (verificationQueries.add(getVerificationQuery(artifact, repositoryId))) {
if (verificationQueries.add(new VerificationQuery(artifact, repositoryId))) {
VerificationEvent event = new VerificationEvent(kind, artifact, mainFile, signatureFile, repositoryName);
synchronized (verificationEvents) {
verificationEvents.add(event);
}
}
}

@Override
public boolean wasAlreadyProcessed(ModuleComponentArtifactIdentifier artifact, String repositoryId) {
return verificationQueries.contains(getVerificationQuery(artifact, repositoryId));
}

private void verifyConcurrently() {
hasFatalFailure.set(false);
synchronized (verificationEvents) {
Expand Down Expand Up @@ -230,9 +224,50 @@ public void stop() {
signatureVerificationService.stop();
}

private String getVerificationQuery(ModuleComponentArtifactIdentifier artifactIdentifier, String repositoryId) {
ModuleComponentIdentifier componentIdentifier = artifactIdentifier.getComponentIdentifier();
return componentIdentifier.getVersion() + ":" + componentIdentifier.getModule() + ":" + componentIdentifier.getGroup() + ":" + artifactIdentifier.getFileName() + ":" + repositoryId;
private static class VerificationQuery {
private final ModuleComponentArtifactIdentifier artifact;
private final String repositoryId;
private final int hashCode;

public VerificationQuery(ModuleComponentArtifactIdentifier artifact, String repositoryId) {
this.artifact = artifact;
this.repositoryId = repositoryId;
this.hashCode = precomputeHashCode(artifact, repositoryId);
}

private int precomputeHashCode(ModuleComponentArtifactIdentifier artifact, String repositoryId) {
int hashCode = artifact.getComponentIdentifier().hashCode();
hashCode = 31 * hashCode + artifact.getFileName().hashCode();
hashCode = 31 * hashCode + repositoryId.hashCode();
return hashCode;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}

VerificationQuery that = (VerificationQuery) o;
if (hashCode != that.hashCode) {
return false;
}
if (!artifact.getComponentIdentifier().equals(that.artifact.getComponentIdentifier())) {
return false;
}
if (!artifact.getFileName().equals(that.artifact.getFileName())) {
return false;
}
return repositoryId.equals(that.repositoryId);
}

@Override
public int hashCode() {
return hashCode;
}
}

private static class VerificationEvent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,13 +437,6 @@ public void onArtifact(ArtifactKind kind, ModuleComponentArtifactIdentifier id,
}
}

@Override
public boolean wasAlreadyProcessed(ModuleComponentArtifactIdentifier artifact, String repositoryId) {
// Since writing to a file is done rarely and this is called only to avoid resolving
// artifacts again when already cached, there is not much penalty to not do any check here
return false;
}

private void addPgp(ModuleComponentArtifactIdentifier id, ArtifactKind kind, File mainFile, Factory<File> signatureFile) {
PgpEntry entry = new PgpEntry(id, kind, mainFile, signatureFile);
synchronized (entriesToBeWritten) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,23 @@ public class ResolvedArtifactCaches implements Stoppable {
private final static Logger LOG = Logging.getLogger(ResolvedArtifactCaches.class);

private final Map<String, Map<ComponentArtifactIdentifier, ResolvableArtifact>> cachePerRepo = new MapMaker().makeMap();
private final Map<String, Map<ComponentArtifactIdentifier, ResolvableArtifact>> cachePerRepoWithVerification = new MapMaker().makeMap();

/**
* For a remote repository, the only thing required is a resolved artifact cache.
* The rest of the in-memory caching is handled by the CachingModuleComponentRepository.
*/
public ModuleComponentRepository provideResolvedArtifactCache(ModuleComponentRepository input) {
Map<ComponentArtifactIdentifier, ResolvableArtifact> caches = getResolvedArtifactCache(input);
public ModuleComponentRepository provideResolvedArtifactCache(ModuleComponentRepository input, boolean withVerification) {
Map<ComponentArtifactIdentifier, ResolvableArtifact> caches = getResolvedArtifactCache(withVerification ? cachePerRepoWithVerification : cachePerRepo, input);
return new ResolvedArtifactCacheProvidingModuleComponentRepository(caches, input);
}

private Map<ComponentArtifactIdentifier, ResolvableArtifact> getResolvedArtifactCache(ModuleComponentRepository input) {
Map<ComponentArtifactIdentifier, ResolvableArtifact> resolvedArtifactCache = cachePerRepo.get(input.getId());
private Map<ComponentArtifactIdentifier, ResolvableArtifact> getResolvedArtifactCache(Map<String, Map<ComponentArtifactIdentifier, ResolvableArtifact>> cache, ModuleComponentRepository input) {
Map<ComponentArtifactIdentifier, ResolvableArtifact> resolvedArtifactCache = cache.get(input.getId());
if (resolvedArtifactCache == null) {
LOG.debug("Creating new in-memory cache for repo '{}' [{}].", input.getName(), input.getId());
resolvedArtifactCache = Maps.newConcurrentMap();
cachePerRepo.put(input.getId(), resolvedArtifactCache);
cache.put(input.getId(), resolvedArtifactCache);
} else {
LOG.debug("Reusing in-memory cache for repo '{}' [{}].", input.getName(), input.getId());
}
Expand All @@ -61,6 +62,7 @@ private Map<ComponentArtifactIdentifier, ResolvableArtifact> getResolvedArtifact
@Override
public void stop() {
cachePerRepo.clear();
cachePerRepoWithVerification.clear();
}

private static class ResolvedArtifactCacheProvidingModuleComponentRepository extends BaseModuleComponentRepository {
Expand Down

0 comments on commit 88ab9b6

Please sign in to comment.