From e0334149c71fc128caa17823d42b8dc7533c8e11 Mon Sep 17 00:00:00 2001 From: fedejeanne Date: Wed, 10 Apr 2024 17:05:36 +0200 Subject: [PATCH] Rename method in SmartImportJob and add getCurrentImportJob - Rename "getImportJob" to "createOrGetConfiguredImportJob" to better express its behavior and also change its JavaDoc. - Add a new method "getCurrentImportJob" that merely retrieves the current job (without creating/configuring anything). - Extract private method in SmartImportTests - Remove calls to waitForJobs(...) in SmartImportTests and join the SmartImportJob instead. Fixes: https://github.com/eclipse-platform/eclipse.platform.ui/issues/1427 Fixes: https://github.com/eclipse-platform/eclipse.platform.ui/issues/1807 --- .../SmartImportRootWizardPage.java | 22 +++++----- .../datatransfer/SmartImportWizard.java | 43 ++++++++++++++----- .../tests/datatransfer/SmartImportTests.java | 15 +++---- 3 files changed, 50 insertions(+), 30 deletions(-) diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportRootWizardPage.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportRootWizardPage.java index 0578dcc10c3..09c7bf71e96 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportRootWizardPage.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportRootWizardPage.java @@ -180,7 +180,7 @@ private class FolderForProjectsLabelProvider extends CellLabelProvider implement public String getText(Object o) { File file = (File) o; Path filePath = file.toPath(); - Path rootPath = getWizard().getImportJob().getRoot().toPath(); + Path rootPath = getWizard().createOrGetConfiguredImportJob().getRoot().toPath(); if (filePath.startsWith(rootPath)) { Path parent = rootPath.getParent(); if (parent != null) { @@ -738,9 +738,9 @@ protected void validatePage() { @Override public boolean isPageComplete() { - return sourceIsValid() && getWizard().getImportJob() != null - && (getWizard().getImportJob().getDirectoriesToImport() == null - || !getWizard().getImportJob().getDirectoriesToImport().isEmpty()); + return sourceIsValid() && getWizard().createOrGetConfiguredImportJob() != null + && (getWizard().createOrGetConfiguredImportJob().getDirectoriesToImport() == null + || !getWizard().createOrGetConfiguredImportJob().getDirectoriesToImport().isEmpty()); } private boolean sourceIsValid() { @@ -785,10 +785,10 @@ public Set getSelectedWorkingSets() { } private void proposalsSelectionChanged() { - if (getWizard().getImportJob() != null) { + if (getWizard().createOrGetConfiguredImportJob() != null) { if (potentialProjects.size() == 1 && potentialProjects.values().iterator().next().isEmpty()) { - getWizard().getImportJob().setDirectoriesToImport(null); - getWizard().getImportJob().setExcludedDirectories(null); + getWizard().createOrGetConfiguredImportJob().setDirectoriesToImport(null); + getWizard().createOrGetConfiguredImportJob().setExcludedDirectories(null); selectionSummary.setText(NLS.bind(DataTransferMessages.SmartImportProposals_selectionSummary, directoriesToImport.size(), 1)); @@ -797,8 +797,8 @@ private void proposalsSelectionChanged() { for (File directory : this.directoriesToImport) { excludedDirectories.remove(directory); } - getWizard().getImportJob().setDirectoriesToImport(directoriesToImport); - getWizard().getImportJob().setExcludedDirectories(excludedDirectories); + getWizard().createOrGetConfiguredImportJob().setDirectoriesToImport(directoriesToImport); + getWizard().createOrGetConfiguredImportJob().setExcludedDirectories(excludedDirectories); selectionSummary.setText(NLS.bind(DataTransferMessages.SmartImportProposals_selectionSummary, directoriesToImport.size(), potentialProjects.size())); @@ -837,7 +837,7 @@ private void refreshProposals() { TreeItem computingItem = new TreeItem(tree.getTree(), SWT.DEFAULT); computingItem .setText(NLS.bind(DataTransferMessages.SmartImportJob_inspecting, selection.getAbsolutePath())); - final SmartImportJob importJob = getWizard().getImportJob(); + final SmartImportJob importJob = getWizard().createOrGetConfiguredImportJob(); refreshProposalsJob = new Job( NLS.bind(DataTransferMessages.SmartImportJob_inspecting, selection.getAbsolutePath())) { @Override @@ -879,7 +879,7 @@ public void done(IJobChangeEvent event) { IStatus result = event.getResult(); if (!control.isDisposed() && result.isOK()) { computingItem.dispose(); - if (sourceIsValid() && getWizard().getImportJob() == importJob) { + if (sourceIsValid() && getWizard().createOrGetConfiguredImportJob() == importJob) { proposalsUpdated(); } tree.getTree().setEnabled(potentialProjects.size() > 1); diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportWizard.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportWizard.java index bf5f63c43a2..f2d06dd5687 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportWizard.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/wizards/datatransfer/SmartImportWizard.java @@ -241,7 +241,7 @@ public boolean performFinish() { System.arraycopy(previousProposals, 0, newProposals, 1, previousProposals.length); getDialogSettings().put(SmartImportRootWizardPage.IMPORTED_SOURCES, newProposals); } - SmartImportJob job = getImportJob(); + SmartImportJob job = createOrGetConfiguredImportJob(); boolean runInBackground = WorkbenchPlugin.getDefault().getPreferenceStore() .getBoolean(IPreferenceConstants.RUN_IN_BACKGROUND); job.setProperty(IProgressConstants.PROPERTY_IN_DIALOG, runInBackground); @@ -250,12 +250,20 @@ public boolean performFinish() { } /** - * Get the import job that will be processed by this wizard. Can be null (if - * provided directory is invalid). + * Get or create the import job that will be processed by this wizard. Can be + * null (if the provided directory is invalid). * - * @return the import job + * IMPORTANT: If there was already a job but it didn't match + * the current configuration of the wizard then a new job will be created and + * returned. + * + * Regardless of whether or not the job was created, some configurations will be + * applied to it. + * + * @return the (newly created) import job + * @see #getCurrentImportJob() */ - public SmartImportJob getImportJob() { + public SmartImportJob createOrGetConfiguredImportJob() { final File root = this.projectRootPage.getSelectedRoot(); if (root == null) { return null; @@ -270,16 +278,29 @@ public SmartImportJob getImportJob() { } else { return null; } + if (this.easymportJob == null || !matchesPage(this.easymportJob, this.projectRootPage)) { this.easymportJob = new SmartImportJob(this.directoryToImport, projectRootPage.getSelectedWorkingSets(), projectRootPage.isConfigureProjects(), projectRootPage.isDetectNestedProject()); } - if (this.easymportJob != null) { - // always update working set on request as the job isn't updated on - // WS change automatically - this.easymportJob.setWorkingSets(projectRootPage.getSelectedWorkingSets()); - this.easymportJob.setCloseProjectsAfterImport(projectRootPage.isCloseProjectsAfterImport()); - } + + // always update working set on request as the job isn't updated on + // WS change automatically + this.easymportJob.setWorkingSets(projectRootPage.getSelectedWorkingSets()); + this.easymportJob.setCloseProjectsAfterImport(projectRootPage.isCloseProjectsAfterImport()); + + return this.easymportJob; + } + + /** + * Gets the current import job but it will not create it if it's + * null. If you need to create the job based on the current + * configuration of the wizard then you can call {@link #createOrGetConfiguredImportJob()} + * + * @return The current import job (it might be null). + * @see #createOrGetConfiguredImportJob() + */ + public SmartImportJob getCurrentImportJob() { return this.easymportJob; } diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/datatransfer/SmartImportTests.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/datatransfer/SmartImportTests.java index 41f738123f2..3325d0e92fd 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/datatransfer/SmartImportTests.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/datatransfer/SmartImportTests.java @@ -150,10 +150,7 @@ private void proceedSmartImportWizard(SmartImportWizard wizard, Consumer okButton.isEnabled(), -1); - wizard.performFinish(); - waitForJobs(100, 1000); // give the job framework time to schedule the job - wizard.getImportJob().join(); - waitForJobs(100, 5000); // give some time for asynchronous workspace jobs to complete + finishWizard(wizard); } finally { if (!dialog.getShell().isDisposed()) { dialog.close(); @@ -442,10 +439,7 @@ public void testChangedWorkingSets() throws Exception { combo.notifyListeners(SWT.Selection, e); processEvents(); processEventsUntil(() -> okButton.isEnabled(), -1); - wizard.performFinish(); - waitForJobs(100, 1000); // give the job framework time to schedule the job - wizard.getImportJob().join(); - waitForJobs(100, 5000); // give some time for asynchronous workspace jobs to complete + finishWizard(wizard); assertEquals("WorkingSet2 should be selected", Collections.singleton(workingSet2), page.getSelectedWorkingSets()); assertEquals("Projects were not added to working set", 1, workingSet2.getElements().length); @@ -459,6 +453,11 @@ public void testChangedWorkingSets() throws Exception { } } + private void finishWizard(SmartImportWizard wizard) throws InterruptedException { + wizard.performFinish(); + wizard.getCurrentImportJob().join(); + } + /** * Bug 559600 - [SmartImport] Label provider throws exception if results contain * filesystem root