Skip to content

Commit 1535dde

Browse files
laeubilaeubi
committed
Refresh targets in the background
A target refresh operation is currently run in the UI thread as it probably in the past where considered a fast operation. At least with P2 Updatesites this is no longer the case and we can'T really make any valid assumptions on how other implementation might perform. This now schedule the work into a background job and also cleanup the code to reflect current code base. In addition to that, if a job is already running, the user is asked to cancel it before perform other actions. Fix #1523
1 parent 0d51f20 commit 1535dde

File tree

4 files changed

+150
-101
lines changed

4 files changed

+150
-101
lines changed

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/Messages.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
public class Messages extends NLS {
1919
private static final String BUNDLE_NAME = "org.eclipse.pde.internal.ui.shared.target.messages"; //$NON-NLS-1$
2020

21+
public static String UpdateTargetJob_RefreshingTarget;
22+
public static String UpdateTargetJob_RefreshJobName;
23+
2124
public static String AddBundleContainerSelectionPage_1;
2225
public static String AddBundleContainerSelectionPage_10;
2326
public static String AddBundleContainerSelectionPage_2;
@@ -148,13 +151,17 @@ public class Messages extends NLS {
148151
public static String TargetLocationsGroup_refresh;
149152
public static String TargetLocationsGroup_1;
150153

154+
public static String TargetLocationsGroup_group_refresh;
155+
156+
public static String TargetLocationsGroup_operation_in_progress;
157+
158+
public static String TargetLocationsGroup_operation_running;
159+
151160
public static String TargetLocationsGroup_TargetUpdateErrorDialog;
152161
public static String TargetStatus_NoActiveTargetPlatformStatus;
153162
public static String TargetStatus_TargetStatusDefaultString;
154163
public static String TargetStatus_UnresolvedTarget;
155164
public static String TargetStatus_UnresolvedTargetStatus;
156-
public static String UpdateTargetJob_TargetUpdateFailedStatus;
157-
public static String UpdateTargetJob_TargetUpdateSuccessStatus;
158165
public static String UpdateTargetJob_UpdateJobName;
159166
public static String UpdateTargetJob_UpdatingTarget;
160167
public static String EditTargetContainerPage_Add_Title;

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/TargetLocationsGroup.java‎

Lines changed: 70 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,19 @@
1717

1818
import static org.eclipse.swt.events.SelectionListener.widgetSelectedAdapter;
1919

20-
import java.util.Collections;
21-
import java.util.List;
2220
import java.util.Objects;
2321

2422
import org.eclipse.core.runtime.CoreException;
2523
import org.eclipse.core.runtime.ICoreRunnable;
2624
import org.eclipse.core.runtime.IStatus;
2725
import org.eclipse.core.runtime.ListenerList;
28-
import org.eclipse.core.runtime.NullProgressMonitor;
2926
import org.eclipse.core.runtime.jobs.IJobChangeEvent;
30-
import org.eclipse.core.runtime.jobs.IJobFunction;
3127
import org.eclipse.core.runtime.jobs.Job;
3228
import org.eclipse.core.runtime.jobs.JobChangeAdapter;
3329
import org.eclipse.jface.action.Action;
3430
import org.eclipse.jface.action.MenuManager;
3531
import org.eclipse.jface.dialogs.ErrorDialog;
32+
import org.eclipse.jface.dialogs.MessageDialog;
3633
import org.eclipse.jface.viewers.AbstractTreeViewer;
3734
import org.eclipse.jface.viewers.ITreeSelection;
3835
import org.eclipse.jface.viewers.TreePath;
@@ -121,6 +118,7 @@ static DeleteButtonState computeState(boolean canRemove, boolean canEnable, bool
121118
private final ListenerList<ITargetChangedListener> fChangeListeners = new ListenerList<>();
122119
private final ListenerList<ITargetChangedListener> fReloadListeners = new ListenerList<>();
123120
private static final TargetLocationHandlerAdapter ADAPTER = new TargetLocationHandlerAdapter();
121+
private Job targetLocationJob;
124122

125123
/**
126124
* Creates this part using the form toolkit and adds it to the given
@@ -483,47 +481,60 @@ private void handleUpdate() {
483481
fUpdateButton.setEnabled(false);
484482
return;
485483
}
486-
List<IJobFunction> updateActions = Collections
487-
.singletonList(monitor -> log(ADAPTER.update(fTarget, selection.getPaths(), monitor)));
488-
JobChangeAdapter listener = new JobChangeAdapter() {
489-
@Override
490-
public void done(final IJobChangeEvent event) {
491-
UIJob job = UIJob.create(Messages.UpdateTargetJob_UpdateJobName, monitor -> {
492-
IStatus result = event.getJob().getResult();
493-
if (!result.isOK()) {
494-
if (!fTreeViewer.getControl().isDisposed()) {
495-
ErrorDialog.openError(fTreeViewer.getTree().getShell(),
496-
Messages.TargetLocationsGroup_TargetUpdateErrorDialog, result.getMessage(), result);
497-
}
498-
} else if (result.getCode() != ITargetLocationHandler.STATUS_CODE_NO_CHANGE) {
499-
// Update was successful and changed the target, if
500-
// dialog/editor still open, update it
501-
if (!fTreeViewer.getControl().isDisposed()) {
502-
contentsChanged(true);
503-
fTreeViewer.refresh(true);
504-
updateButtons();
505-
}
484+
if (checkJob()) {
485+
JobChangeAdapter listener = new JobChangeAdapter() {
486+
@Override
487+
public void done(final IJobChangeEvent event) {
488+
UIJob job = UIJob.create(Messages.UpdateTargetJob_UpdateJobName, monitor -> {
489+
targetLocationJob = null;
490+
IStatus result = event.getJob().getResult();
491+
log(result);
492+
if (!result.isOK()) {
493+
if (!fTreeViewer.getControl().isDisposed()) {
494+
ErrorDialog.openError(fTreeViewer.getTree().getShell(),
495+
Messages.TargetLocationsGroup_TargetUpdateErrorDialog, result.getMessage(),
496+
result);
497+
}
498+
} else if (result.getCode() != ITargetLocationHandler.STATUS_CODE_NO_CHANGE) {
499+
// Update was successful and changed the target, if
500+
// dialog/editor still open, update it
501+
if (!fTreeViewer.getControl().isDisposed()) {
502+
contentsChanged(true);
503+
fTreeViewer.refresh(true);
504+
updateButtons();
505+
}
506506

507-
// If the target is the current platform, run a load
508-
// job for the user
509-
try {
510-
ITargetPlatformService service = PDECore.getDefault()
511-
.acquireService(ITargetPlatformService.class);
512-
if (service != null) {
513-
ITargetHandle currentTarget = service.getWorkspaceTargetHandle();
514-
if (fTarget.getHandle().equals(currentTarget))
515-
LoadTargetDefinitionJob.load(fTarget);
507+
// If the target is the current platform, run a load
508+
// job for the user
509+
try {
510+
ITargetPlatformService service = PDECore.getDefault()
511+
.acquireService(ITargetPlatformService.class);
512+
if (service != null) {
513+
ITargetHandle currentTarget = service.getWorkspaceTargetHandle();
514+
if (fTarget.getHandle().equals(currentTarget))
515+
LoadTargetDefinitionJob.load(fTarget);
516+
}
517+
} catch (CoreException e) {
518+
// do nothing if we could not set the current
519+
// target.
516520
}
517-
} catch (CoreException e) {
518-
// do nothing if we could not set the current
519-
// target.
520521
}
521-
}
522-
});
523-
job.schedule();
524-
}
525-
};
526-
UpdateTargetJob.update(updateActions, listener);
522+
});
523+
job.schedule();
524+
}
525+
};
526+
targetLocationJob = UpdateTargetJob.update(fTarget, ADAPTER, selection.getPaths(), listener,
527+
targetLocationJob);
528+
}
529+
}
530+
531+
private boolean checkJob() {
532+
Job job = targetLocationJob;
533+
if (job == null) {
534+
return true;
535+
}
536+
return MessageDialog.openQuestion(fTreeViewer.getControl().getShell(),
537+
Messages.TargetLocationsGroup_operation_in_progress, Messages.TargetLocationsGroup_operation_running);
527538
}
528539

529540
private void updateButtons() {
@@ -562,16 +573,28 @@ private void updateButtons() {
562573
case ENABLE -> fRemoveButton.setText(Messages.BundleContainerTable_Btn_Text_Enable);
563574
case TOGGLE -> fRemoveButton.setText(Messages.BundleContainerTable_Btn_Text_Toggle);
564575
default -> fRemoveButton.setText(Messages.BundleContainerTable_Btn_Text_Remove);
565-
}
576+
}
566577
fRemoveButton.setEnabled(state != DeleteButtonState.NONE);
567578
fRemoveButton.setData(BUTTON_STATE, state);
568579
}
569580

570581
private void handleReload() {
571-
log(ADAPTER.reload(fTarget, fTarget.getTargetLocations(), new NullProgressMonitor()));
572-
Job job = UIJob.create("Refreshing...", (ICoreRunnable) monitor -> contentsReload()); //$NON-NLS-1$
573-
job.schedule();
574-
582+
if (checkJob()) {
583+
targetLocationJob = UpdateTargetJob.refresh(fTarget, ADAPTER, new JobChangeAdapter() {
584+
@Override
585+
public void done(IJobChangeEvent event) {
586+
IStatus result = event.getResult();
587+
log(result);
588+
if (result.isOK()) {
589+
Job job = UIJob.create(Messages.TargetLocationsGroup_group_refresh, (ICoreRunnable) monitor -> {
590+
targetLocationJob = null;
591+
contentsReload();
592+
});
593+
job.schedule();
594+
}
595+
}
596+
}, targetLocationJob);
597+
}
575598
}
576599

577600
private void toggleCollapse() {

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/UpdateTargetJob.java‎

Lines changed: 66 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,14 @@
1616
*******************************************************************************/
1717
package org.eclipse.pde.internal.ui.shared.target;
1818

19-
import java.util.List;
20-
import java.util.Map;
21-
import java.util.Set;
22-
2319
import org.eclipse.core.runtime.IProgressMonitor;
2420
import org.eclipse.core.runtime.IStatus;
25-
import org.eclipse.core.runtime.MultiStatus;
26-
import org.eclipse.core.runtime.Status;
2721
import org.eclipse.core.runtime.SubMonitor;
2822
import org.eclipse.core.runtime.jobs.IJobChangeListener;
2923
import org.eclipse.core.runtime.jobs.IJobFunction;
3024
import org.eclipse.core.runtime.jobs.Job;
25+
import org.eclipse.jface.viewers.TreePath;
3126
import org.eclipse.pde.core.target.ITargetDefinition;
32-
import org.eclipse.pde.core.target.ITargetLocation;
33-
import org.eclipse.pde.internal.core.PDECore;
3427
import org.eclipse.pde.ui.target.ITargetLocationHandler;
3528

3629
/**
@@ -44,72 +37,95 @@
4437
*/
4538
public class UpdateTargetJob extends Job {
4639

47-
public static final String JOB_FAMILY_ID = "UpdateTargetJob"; //$NON-NLS-1$
40+
private final IJobFunction action;
41+
42+
private boolean update;
4843

49-
private final List<IJobFunction> toUpdate;
44+
private Job cancelJob;
5045

5146
/**
52-
* Schedules a new update job that will update all target locations in the
53-
* provided map. A target's selected children can be added as a set to the
54-
* values of the map so that only certain portions of the target location
55-
* get updated.
47+
* Schedules a new update job that will update all selected children of the
48+
* target location get updated.
5649
* <p>
57-
* If all calls to {@link IJobFunction#run(IProgressMonitor)} return an OK
58-
* status with {@link ITargetLocationHandler#STATUS_CODE_NO_CHANGE}, the
59-
* returned status will also have that status code, indicating that no
60-
* changes were made to the target.
50+
* If no changes are performed an OK status with
51+
* {@link ITargetLocationHandler#STATUS_CODE_NO_CHANGE}, the returned status
52+
* will also have that status code, indicating that no changes were made to
53+
* the target.
6154
* </p>
6255
*
63-
* @param updateActions
64-
* maps {@link ITargetLocation}s to the {@link Set} of selected
65-
* children items that should be updated. The sets may be empty,
66-
* but not <code>null</code>
56+
* @param definition
57+
* the target definition to update
58+
* @param handler
59+
* the handler
60+
* @param treePaths
61+
* the actual path to refresh
6762
* @param listener
6863
* job change listener that will be added to the created job, can
6964
* be <code>null</code>
65+
* @param cancelJob
66+
* a job to cancel when the new one is run, can be
67+
* <code>null</code>
68+
* @return the job created and scheduled
7069
*/
71-
public static void update(List<IJobFunction> updateActions, IJobChangeListener listener) {
72-
Job.getJobManager().cancel(JOB_FAMILY_ID);
73-
Job job = new UpdateTargetJob(updateActions);
70+
public static Job update(ITargetDefinition definition, ITargetLocationHandler handler, TreePath[] treePaths,
71+
IJobChangeListener listener, Job cancelJob) {
72+
Job job = new UpdateTargetJob(cancelJob, monitor -> handler.update(definition, treePaths, monitor), true);
7473
job.setUser(true);
7574
if (listener != null) {
7675
job.addJobChangeListener(listener);
7776
}
7877
job.schedule();
78+
return job;
7979
}
8080

8181
/**
82-
* Use {@link #update(ITargetDefinition, Map, IJobChangeListener)} instead
82+
* Schedules a new update job that will refresh all target locations in the
83+
* provided target.
84+
*
85+
* @param handler
86+
* the handler
87+
* @param definition
88+
* the target definition to refresh
89+
* @param listener
90+
* job change listener that will be added to the created job, can
91+
* be <code>null</code>
92+
* @param cancelJob
93+
* a job to cancel when the new one is run, can be
94+
* <code>null</code>
95+
* @return the job created and scheduled
8396
*/
84-
private UpdateTargetJob(List<IJobFunction> updateActions) {
85-
super(Messages.UpdateTargetJob_UpdateJobName);
86-
this.toUpdate = updateActions;
97+
public static Job refresh(ITargetDefinition definition, ITargetLocationHandler handler,
98+
IJobChangeListener listener, Job cancelJob) {
99+
Job job = new UpdateTargetJob(cancelJob,
100+
monitor -> handler.reload(definition, definition.getTargetLocations(), monitor),
101+
false);
102+
job.setUser(true);
103+
if (listener != null) {
104+
job.addJobChangeListener(listener);
105+
}
106+
job.schedule();
107+
return job;
108+
}
109+
110+
private UpdateTargetJob(Job cancelJob, IJobFunction updateActions, boolean update) {
111+
super(update ? Messages.UpdateTargetJob_UpdateJobName : Messages.UpdateTargetJob_RefreshJobName);
112+
this.cancelJob = cancelJob;
113+
this.action = updateActions;
114+
this.update = update;
87115
}
88116

89117
@Override
90118
protected IStatus run(IProgressMonitor monitor) {
91-
SubMonitor progress = SubMonitor.convert(monitor, Messages.UpdateTargetJob_UpdatingTarget,
92-
toUpdate.size() * 100);
93-
MultiStatus errors = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.UpdateTargetJob_TargetUpdateFailedStatus,
94-
null);
95-
boolean noChange = true;
96-
for (IJobFunction action : toUpdate) {
97-
IStatus result = action.run(progress.split(100));
98-
if (result.isOK() && result.getCode() != ITargetLocationHandler.STATUS_CODE_NO_CHANGE) {
99-
noChange = false;
100-
} else if (!result.isOK()) {
101-
noChange = false;
102-
errors.add(result);
119+
SubMonitor progress = SubMonitor.convert(monitor,
120+
update ? Messages.UpdateTargetJob_UpdatingTarget : Messages.UpdateTargetJob_RefreshingTarget, 100);
121+
if (cancelJob != null) {
122+
cancelJob.cancel();
123+
try {
124+
cancelJob.join();
125+
} catch (InterruptedException e) {
126+
// we tried our best
103127
}
104128
}
105-
progress.done();
106-
if (noChange) {
107-
return new Status(IStatus.OK, PDECore.PLUGIN_ID, ITargetLocationHandler.STATUS_CODE_NO_CHANGE,
108-
Messages.UpdateTargetJob_TargetUpdateSuccessStatus, null);
109-
} else if (!errors.isOK()) {
110-
return errors;
111-
} else {
112-
return Status.OK_STATUS;
113-
}
129+
return action.run(progress);
114130
}
115131
}

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/messages.properties‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,18 @@ TargetContentsGroup_resolveCancelled=Resolve Cancelled
138138
TargetLocationsGroup_update=Looks for newer versions of the target contents and updates them
139139
TargetLocationsGroup_refresh=Clears the cached target data and resolves the target, looking for newer versions of any artifacts
140140
TargetLocationsGroup_1=&Show location content
141+
TargetLocationsGroup_group_refresh=Refreshing...
142+
TargetLocationsGroup_operation_in_progress=Target operation in progress
143+
TargetLocationsGroup_operation_running=A target editor operation is currently running, do you want to cancel it?
141144
TargetLocationsGroup_TargetUpdateErrorDialog=Problems Updating Target Definition
142145
TargetStatus_NoActiveTargetPlatformStatus=No active target platform
143146
TargetStatus_TargetStatusDefaultString=Target Platform
144147
TargetStatus_UnresolvedTarget=Unresolved ''{0}''
145148
TargetStatus_UnresolvedTargetStatus=Target platform is not loaded
146-
UpdateTargetJob_TargetUpdateFailedStatus=The target definition did not update successfully
147-
UpdateTargetJob_TargetUpdateSuccessStatus=Target definition update completed successfully
148149
UpdateTargetJob_UpdateJobName=Update Target Definition
150+
UpdateTargetJob_RefreshJobName=Refresh Target Definition
149151
UpdateTargetJob_UpdatingTarget=Updating Target
152+
UpdateTargetJob_RefreshingTarget=Refreshing Target
150153
EditTargetContainerPage_Add_Title=Create target reference
151154
EditTargetContainerPage_Edit_Title=Edit target reference
152155
EditTargetContainerPage_Message=Please enter a URI to the target to be referenced

0 commit comments

Comments
 (0)