-
Notifications
You must be signed in to change notification settings - Fork 408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Download java sources lazily for Maven projects #986
Conversation
* @throws JavaModelException | ||
* @throws CoreException | ||
*/ | ||
default void getSource(IClassFile classFile, IProgressMonitor monitor) throws CoreException { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
||
/** | ||
* @author Fred Bricon | ||
* | ||
*/ | ||
public class MavenBuildSupport implements IBuildSupport { | ||
|
||
private static final int MAX_WAIT_MILLIS = 3000; | ||
private static Cache<String, Boolean> cache = CacheBuilder.newBuilder().maximumSize(100).expireAfterWrite(1, TimeUnit.HOURS).build(); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@@ -76,4 +78,17 @@ default void refresh(IResource resource, CHANGE_TYPE changeType, IProgressMonito | |||
} | |||
} | |||
|
|||
/** | |||
* download source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're actually discovering AND attaching the source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please update the javadoc
IClasspathManager buildpathManager = MavenJdtPlugin.getDefault().getBuildpathManager(); | ||
buildpathManager.scheduleDownload(fragment, true, true); | ||
waitForDownload(monitor); | ||
buildpathManager.scheduleDownload(project, true, true); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@fbricon I have updated the PR. |
} | ||
} | ||
|
||
private void waitForDownload(IProgressMonitor monitor) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use JobHelpers.waitForJobs(IJobMatcher)
IClasspathManager buildpathManager = MavenJdtPlugin.getDefault().getBuildpathManager(); | ||
buildpathManager.scheduleDownload(fragment, true, true); | ||
waitForDownload(monitor); | ||
downloadRequestsCache.put(path.toString(), true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move before buildpathManager.scheduleDownload(fragment, true, true); else you might get simultaneous multiple download attempts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some tests checking source downloads, timeouts
IJavaProject javaProject = JavaCore.create(project); | ||
IType type = javaProject.findType("org.apache.commons.lang3.StringUtils"); | ||
IClassFile classFile = ((BinaryType) type).getClassFile(); | ||
assertTrue(classFile.getBuffer() == null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assertNull
waitForJobs(DownloadSourcesJobMatcher.INSTANCE, WAIT_FOR_DOWNLOAD_SOURCE_JOB_MILLIS); | ||
} | ||
|
||
public static void waitForDownloadSourcesJobsTest() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, this class shouldn't have test specific methods. I'd rather prefer you added
public static void waitForDownloadSourcesJobs(int maxWaitMillis)
@@ -76,4 +78,17 @@ default void refresh(IResource resource, CHANGE_TYPE changeType, IProgressMonito | |||
} | |||
} | |||
|
|||
/** | |||
* discover source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discover the source for classFile and attach it to the project it belongs to.
* - a class file | ||
* @param monitor | ||
* - a progress monitor | ||
* @throws JavaModelException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JavaModelException is not in the signature
Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
@fbricon I have updated the PR. |
Fixes #979
Requires redhat-developer/vscode-java#870
Signed-off-by: Snjezana Peco snjezana.peco@redhat.com