From 19605fabcab1c469e15053048a5866067afa1290 Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Sat, 21 Aug 2021 21:52:26 +0200 Subject: [PATCH] Prevent endless download of artifact sources/javadoc (#252) + fix total-work computation in classpath-update + minor clean up --- .../m2e/jdt/internal/BuildPathManager.java | 13 ++--- .../m2e/jdt/internal/DownloadSourcesJob.java | 47 ++++++++----------- 2 files changed, 27 insertions(+), 33 deletions(-) diff --git a/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/BuildPathManager.java b/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/BuildPathManager.java index e91cc7d9e0..63498373f7 100644 --- a/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/BuildPathManager.java +++ b/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/BuildPathManager.java @@ -332,16 +332,13 @@ private void configureAttachedSourcesAndJavadoc(IMavenProjectFacade facade, Prop if(aKey != null) { // maybe we should try to find artifactKey little harder here? boolean isSnapshot = aKey.getVersion().endsWith("-SNAPSHOT"); // We should update a sources/javadoc jar for a snapshot in case they're already downloaded. - File plainFile = desc.getPath() != null ? desc.getPath().toFile() : null; + File jarFile = desc.getPath() != null ? desc.getPath().toFile() : null; File srcFile = srcPath != null ? srcPath.toFile() : null; boolean downloadSources = (srcPath == null && mavenConfiguration.isDownloadSources()) - || (plainFile != null && plainFile.canRead() && srcFile != null && srcFile.canRead() && isSnapshot - && srcFile.lastModified() < plainFile.lastModified()); + || (isSnapshot && isLastModifiedBefore(srcFile, jarFile)); File javaDocFile = javaDocUrl != null ? getAttachedArtifactFile(aKey, CLASSIFIER_JAVADOC) : null; boolean downloadJavaDoc = (javaDocUrl == null && mavenConfiguration.isDownloadJavaDoc()) - || (plainFile != null && plainFile.canRead() && javaDocFile != null && javaDocFile.canRead() && isSnapshot - && javaDocFile.lastModified() < plainFile.lastModified()); - + || (isSnapshot && isLastModifiedBefore(javaDocFile, jarFile)); scheduleDownload(facade.getProject(), facade.getMavenProject(monitor), aKey, downloadSources, downloadJavaDoc); } @@ -349,6 +346,10 @@ private void configureAttachedSourcesAndJavadoc(IMavenProjectFacade facade, Prop } } + private static boolean isLastModifiedBefore(File file, File ref) { + return ref != null && ref.canRead() && file != null && file.canRead() && file.lastModified() < ref.lastModified(); + } + private static final String ARTIFACT_TYPE_JAR = "jar"; private boolean isUnavailable(ArtifactKey a, List repositories) throws CoreException { diff --git a/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/DownloadSourcesJob.java b/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/DownloadSourcesJob.java index 635bd896cc..213bf5d269 100644 --- a/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/DownloadSourcesJob.java +++ b/org.eclipse.m2e.jdt/src/org/eclipse/m2e/jdt/internal/DownloadSourcesJob.java @@ -14,7 +14,6 @@ package org.eclipse.m2e.jdt.internal; import java.io.File; -import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -58,6 +57,7 @@ * * @author igor */ +@SuppressWarnings("restriction") class DownloadSourcesJob extends Job implements IBackgroundProcessingQueue { private static Logger log = LoggerFactory.getLogger(DownloadSourcesJob.class); @@ -137,6 +137,8 @@ public boolean isNotEmpty() { private final BlockingQueue queue = new LinkedBlockingQueue<>(); + private Set requests = new HashSet<>(); + private final Set toUpdateMavenProjects = new HashSet<>(); private final Map toUpdateAttachments = new HashMap<>(); @@ -163,6 +165,7 @@ public IStatus run(IProgressMonitor monitor) { if(!status.isOK()) { // or maybe just log and ignore? queue.clear(); + requests.clear(); toUpdateAttachments.clear(); toUpdateMavenProjects.clear(); return status; @@ -173,6 +176,7 @@ public IStatus run(IProgressMonitor monitor) { } if(monitor.isCanceled()) { queue.clear(); + requests.clear(); toUpdateAttachments.clear(); toUpdateMavenProjects.clear(); return Status.CANCEL_STATUS; @@ -186,18 +190,16 @@ public IStatus run(IProgressMonitor monitor) { toUpdateAttachments.clear(); toUpdateMavenProjects.clear(); } + requests.clear(); // retain all elements in queue (in an efficient manner) + requests.addAll(queue); // queue might not be empty anymore (filled by updateClasspath) subMonitor.done(); - if(monitor.isCanceled()) { - return Status.CANCEL_STATUS; - } - return Status.OK_STATUS; + return monitor.isCanceled() ? Status.CANCEL_STATUS : Status.OK_STATUS; } private static void updateClasspath(BuildPathManager manager, Set toUpdateMavenProjects, Map toUpdateAttachments, IProgressMonitor monitor) { - SubMonitor updateMonitor = SubMonitor.convert(monitor, - 1 + toUpdateMavenProjects.size() + toUpdateMavenProjects.size()); - updateMonitor.setTaskName(Messages.DownloadSourcesJob_job_associateWithClasspath); + SubMonitor updateMonitor = SubMonitor.convert(monitor, Messages.DownloadSourcesJob_job_associateWithClasspath, + 1 + toUpdateMavenProjects.size() + toUpdateAttachments.size()); ISchedulingRule schedulingRule = ResourcesPlugin.getWorkspace().getRuleFactory().buildRule(); getJobManager().beginRule(schedulingRule, updateMonitor.split(1)); try { @@ -219,8 +221,6 @@ private static void updateClasspath(BuildPathManager manager, Set toUp } IStatus downloadFilesAndPopulateToUpdate(DownloadRequest request, IProgressMonitor monitor) { - final List exceptions = new ArrayList<>(); - SubMonitor requestMonitor = SubMonitor.convert(monitor, 33); try { if(request.artifact != null) { @@ -247,17 +247,13 @@ IStatus downloadFilesAndPopulateToUpdate(DownloadRequest request, IProgressMonit toUpdateAttachments.put(request.fragment, files); } } + return Status.OK_STATUS; } catch(CoreException ex) { - exceptions.add(ex.getStatus()); - } - requestMonitor.done(); - - if(!exceptions.isEmpty()) { - IStatus[] problems = exceptions.toArray(new IStatus[exceptions.size()]); - return new MultiStatus(MavenJdtPlugin.PLUGIN_ID, -1, problems, "Could not download sources or javadoc", null); + return new MultiStatus(MavenJdtPlugin.PLUGIN_ID, -1, new IStatus[] {ex.getStatus()}, + "Could not download sources or javadoc", null); + } finally { + requestMonitor.done(); } - - return Status.OK_STATUS; } private Attachments downloadMaven(IMavenProjectFacade projectFacade, ArtifactKey artifact, boolean downloadSources, @@ -339,17 +335,14 @@ private File download(ArtifactKey artifact, List repositorie private void scheduleDownload(IProject project, IPackageFragmentRoot fragment, ArtifactKey artifact, boolean downloadSources, boolean downloadJavadoc) { - addDownloadRequest(project, fragment, artifact, downloadSources, downloadJavadoc); - - schedule(SCHEDULE_INTERVAL); - } - - public void addDownloadRequest(IProject project, IPackageFragmentRoot fragment, ArtifactKey artifact, - boolean downloadSources, boolean downloadJavadoc) { if(project == null || !project.isAccessible()) { return; } - queue.add(new DownloadRequest(project, fragment, artifact, downloadSources, downloadJavadoc)); + DownloadRequest request = new DownloadRequest(project, fragment, artifact, downloadSources, downloadJavadoc); + if(requests.add(request)) { // guard against new requests that are/will be already downloaded in this run to prevent endless loops + queue.add(request); + schedule(SCHEDULE_INTERVAL); + } } /**