Skip to content

Commit

Permalink
Refresh targets in the background
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@laeubi
laeubi committed Dec 20, 2024
1 parent 0d51f20 commit 511067d
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 92 deletions.
11 changes: 11 additions & 0 deletions repository/.project
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<projectDescription>
<name>repository</name>
<comment></comment>
<projects>
</projects>
<buildSpec>
</buildSpec>
<natures>
</natures>
</projectDescription>
11 changes: 9 additions & 2 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/Messages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
public class Messages extends NLS {
private static final String BUNDLE_NAME = "org.eclipse.pde.internal.ui.shared.target.messages"; //$NON-NLS-1$

public static String UpdateTargetJob_RefreshingTarget;
public static String UpdateTargetJob_RefreshJobName;

public static String AddBundleContainerSelectionPage_1;
public static String AddBundleContainerSelectionPage_10;
public static String AddBundleContainerSelectionPage_2;
Expand Down Expand Up @@ -148,13 +151,17 @@ public class Messages extends NLS {
public static String TargetLocationsGroup_refresh;
public static String TargetLocationsGroup_1;

public static String TargetLocationsGroup_group_refresh;

public static String TargetLocationsGroup_operation_in_progress;

public static String TargetLocationsGroup_operation_running;

public static String TargetLocationsGroup_TargetUpdateErrorDialog;
public static String TargetStatus_NoActiveTargetPlatformStatus;
public static String TargetStatus_TargetStatusDefaultString;
public static String TargetStatus_UnresolvedTarget;
public static String TargetStatus_UnresolvedTargetStatus;
public static String UpdateTargetJob_TargetUpdateFailedStatus;
public static String UpdateTargetJob_TargetUpdateSuccessStatus;
public static String UpdateTargetJob_UpdateJobName;
public static String UpdateTargetJob_UpdatingTarget;
public static String EditTargetContainerPage_Add_Title;
Expand Down
117 changes: 70 additions & 47 deletions ...rg.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/TargetLocationsGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,19 @@

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

import java.util.Collections;
import java.util.List;
import java.util.Objects;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.ICoreRunnable;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.ListenerList;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.jobs.IJobChangeEvent;
import org.eclipse.core.runtime.jobs.IJobFunction;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.core.runtime.jobs.JobChangeAdapter;
import org.eclipse.jface.action.Action;
import org.eclipse.jface.action.MenuManager;
import org.eclipse.jface.dialogs.ErrorDialog;
import org.eclipse.jface.dialogs.MessageDialog;
import org.eclipse.jface.viewers.AbstractTreeViewer;
import org.eclipse.jface.viewers.ITreeSelection;
import org.eclipse.jface.viewers.TreePath;
Expand Down Expand Up @@ -121,6 +118,7 @@ static DeleteButtonState computeState(boolean canRemove, boolean canEnable, bool
private final ListenerList<ITargetChangedListener> fChangeListeners = new ListenerList<>();
private final ListenerList<ITargetChangedListener> fReloadListeners = new ListenerList<>();
private static final TargetLocationHandlerAdapter ADAPTER = new TargetLocationHandlerAdapter();
private Job targetLocationJob;

/**
* Creates this part using the form toolkit and adds it to the given
Expand Down Expand Up @@ -483,47 +481,60 @@ private void handleUpdate() {
fUpdateButton.setEnabled(false);
return;
}
List<IJobFunction> updateActions = Collections
.singletonList(monitor -> log(ADAPTER.update(fTarget, selection.getPaths(), monitor)));
JobChangeAdapter listener = new JobChangeAdapter() {
@Override
public void done(final IJobChangeEvent event) {
UIJob job = UIJob.create(Messages.UpdateTargetJob_UpdateJobName, monitor -> {
IStatus result = event.getJob().getResult();
if (!result.isOK()) {
if (!fTreeViewer.getControl().isDisposed()) {
ErrorDialog.openError(fTreeViewer.getTree().getShell(),
Messages.TargetLocationsGroup_TargetUpdateErrorDialog, result.getMessage(), result);
}
} else if (result.getCode() != ITargetLocationHandler.STATUS_CODE_NO_CHANGE) {
// Update was successful and changed the target, if
// dialog/editor still open, update it
if (!fTreeViewer.getControl().isDisposed()) {
contentsChanged(true);
fTreeViewer.refresh(true);
updateButtons();
}
if (checkJob()) {
JobChangeAdapter listener = new JobChangeAdapter() {
@Override
public void done(final IJobChangeEvent event) {
UIJob job = UIJob.create(Messages.UpdateTargetJob_UpdateJobName, monitor -> {
targetLocationJob = null;
IStatus result = event.getJob().getResult();
log(result);
if (!result.isOK()) {
if (!fTreeViewer.getControl().isDisposed()) {
ErrorDialog.openError(fTreeViewer.getTree().getShell(),
Messages.TargetLocationsGroup_TargetUpdateErrorDialog, result.getMessage(),
result);
}
} else if (result.getCode() != ITargetLocationHandler.STATUS_CODE_NO_CHANGE) {
// Update was successful and changed the target, if
// dialog/editor still open, update it
if (!fTreeViewer.getControl().isDisposed()) {
contentsChanged(true);
fTreeViewer.refresh(true);
updateButtons();
}

// If the target is the current platform, run a load
// job for the user
try {
ITargetPlatformService service = PDECore.getDefault()
.acquireService(ITargetPlatformService.class);
if (service != null) {
ITargetHandle currentTarget = service.getWorkspaceTargetHandle();
if (fTarget.getHandle().equals(currentTarget))
LoadTargetDefinitionJob.load(fTarget);
// If the target is the current platform, run a load
// job for the user
try {
ITargetPlatformService service = PDECore.getDefault()
.acquireService(ITargetPlatformService.class);
if (service != null) {
ITargetHandle currentTarget = service.getWorkspaceTargetHandle();
if (fTarget.getHandle().equals(currentTarget))
LoadTargetDefinitionJob.load(fTarget);
}
} catch (CoreException e) {
// do nothing if we could not set the current
// target.
}
} catch (CoreException e) {
// do nothing if we could not set the current
// target.
}
}
});
job.schedule();
}
};
UpdateTargetJob.update(updateActions, listener);
});
job.schedule();
}
};
targetLocationJob = UpdateTargetJob.update(fTarget, ADAPTER, selection.getPaths(), listener,
targetLocationJob);
}
}

private boolean checkJob() {
Job job = targetLocationJob;
if (job == null) {
return true;
}
return MessageDialog.openQuestion(fTreeViewer.getControl().getShell(),
Messages.TargetLocationsGroup_operation_in_progress, Messages.TargetLocationsGroup_operation_running);
}

private void updateButtons() {
Expand Down Expand Up @@ -562,16 +573,28 @@ private void updateButtons() {
case ENABLE -> fRemoveButton.setText(Messages.BundleContainerTable_Btn_Text_Enable);
case TOGGLE -> fRemoveButton.setText(Messages.BundleContainerTable_Btn_Text_Toggle);
default -> fRemoveButton.setText(Messages.BundleContainerTable_Btn_Text_Remove);
}
}
fRemoveButton.setEnabled(state != DeleteButtonState.NONE);
fRemoveButton.setData(BUTTON_STATE, state);
}

private void handleReload() {
log(ADAPTER.reload(fTarget, fTarget.getTargetLocations(), new NullProgressMonitor()));
Job job = UIJob.create("Refreshing...", (ICoreRunnable) monitor -> contentsReload()); //$NON-NLS-1$
job.schedule();

if (checkJob()) {
targetLocationJob = UpdateTargetJob.refresh(fTarget, ADAPTER, new JobChangeAdapter() {
@Override
public void done(IJobChangeEvent event) {
IStatus result = event.getResult();
log(result);
if (result.isOK()) {
Job job = UIJob.create(Messages.TargetLocationsGroup_group_refresh, (ICoreRunnable) monitor -> {
targetLocationJob = null;
contentsReload();
});
job.schedule();
}
}
}, targetLocationJob);
}
}

private void toggleCollapse() {
Expand Down
99 changes: 58 additions & 41 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/UpdateTargetJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,18 @@
*******************************************************************************/
package org.eclipse.pde.internal.ui.shared.target;

import java.util.List;
import java.util.Map;
import java.util.Set;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.MultiStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.SubMonitor;
import org.eclipse.core.runtime.jobs.IJobChangeListener;
import org.eclipse.core.runtime.jobs.IJobFunction;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.jface.viewers.TreePath;
import org.eclipse.pde.core.target.ITargetDefinition;
import org.eclipse.pde.core.target.ITargetLocation;
import org.eclipse.pde.internal.core.PDECore;
import org.eclipse.pde.ui.target.ITargetLocationHandler;

/**
Expand All @@ -44,72 +41,92 @@
*/
public class UpdateTargetJob extends Job {

public static final String JOB_FAMILY_ID = "UpdateTargetJob"; //$NON-NLS-1$
private final IJobFunction action;

private final List<IJobFunction> toUpdate;
private boolean update;

private Job cancelJob;

/**
* Schedules a new update job that will update all target locations in the
* provided map. A target's selected children can be added as a set to the
* values of the map so that only certain portions of the target location
* get updated.
* Schedules a new update job that will update all selected children of the
* target location get updated.
* <p>
* If all calls to {@link IJobFunction#run(IProgressMonitor)} return an OK
* status with {@link ITargetLocationHandler#STATUS_CODE_NO_CHANGE}, the
* returned status will also have that status code, indicating that no
* changes were made to the target.
* If no changes are performed an OK status with
* {@link ITargetLocationHandler#STATUS_CODE_NO_CHANGE}, the returned status
* will also have that status code, indicating that no changes were made to
* the target.
* </p>
*
* @param treePaths
*
* @param updateActions
* maps {@link ITargetLocation}s to the {@link Set} of selected
* children items that should be updated. The sets may be empty,
* but not <code>null</code>
* @param listener
* job change listener that will be added to the created job, can
* be <code>null</code>
* @param cancelJob
* @return the job created and scheduled
*/
public static Job update(ITargetDefinition definition, ITargetLocationHandler handler, TreePath[] treePaths,
IJobChangeListener listener, Job cancelJob) {
Job job = new UpdateTargetJob(cancelJob, monitor -> handler.update(definition, treePaths, monitor), true);
job.setUser(true);
if (listener != null) {
job.addJobChangeListener(listener);
}
job.schedule();
return job;
}

/**
* Schedules a new update job that will refresh all target locations in the
* provided target.
*
* @param definition
* the target definition to refresh
* @param listener
* job change listener that will be added to the created job, can
* be <code>null</code>
* @param cancelJob
* @return the job created and scheduled
*/
public static void update(List<IJobFunction> updateActions, IJobChangeListener listener) {
Job.getJobManager().cancel(JOB_FAMILY_ID);
Job job = new UpdateTargetJob(updateActions);
public static Job refresh(ITargetDefinition definition, ITargetLocationHandler handler,
IJobChangeListener listener, Job cancelJob) {
Job job = new UpdateTargetJob(cancelJob,
monitor -> handler.reload(definition, definition.getTargetLocations(), monitor),
false);
job.setUser(true);
if (listener != null) {
job.addJobChangeListener(listener);
}
job.schedule();
return job;
}

/**
* Use {@link #update(ITargetDefinition, Map, IJobChangeListener)} instead
*/
private UpdateTargetJob(List<IJobFunction> updateActions) {
super(Messages.UpdateTargetJob_UpdateJobName);
this.toUpdate = updateActions;
private UpdateTargetJob(Job cancelJob, IJobFunction updateActions, boolean update) {
super(update ? Messages.UpdateTargetJob_UpdateJobName : Messages.UpdateTargetJob_RefreshJobName);
this.cancelJob = cancelJob;
this.action = updateActions;
this.update = update;
}

@Override
protected IStatus run(IProgressMonitor monitor) {
SubMonitor progress = SubMonitor.convert(monitor, Messages.UpdateTargetJob_UpdatingTarget,
toUpdate.size() * 100);
MultiStatus errors = new MultiStatus(PDECore.PLUGIN_ID, 0, Messages.UpdateTargetJob_TargetUpdateFailedStatus,
null);
boolean noChange = true;
for (IJobFunction action : toUpdate) {
IStatus result = action.run(progress.split(100));
if (result.isOK() && result.getCode() != ITargetLocationHandler.STATUS_CODE_NO_CHANGE) {
noChange = false;
} else if (!result.isOK()) {
noChange = false;
errors.add(result);
SubMonitor progress = SubMonitor.convert(monitor,
update ? Messages.UpdateTargetJob_UpdatingTarget : Messages.UpdateTargetJob_RefreshingTarget, 100);
if (cancelJob != null) {
cancelJob.cancel();
try {
cancelJob.join();
} catch (InterruptedException e) {
// we tried our best
}
}
progress.done();
if (noChange) {
return new Status(IStatus.OK, PDECore.PLUGIN_ID, ITargetLocationHandler.STATUS_CODE_NO_CHANGE,
Messages.UpdateTargetJob_TargetUpdateSuccessStatus, null);
} else if (!errors.isOK()) {
return errors;
} else {
return Status.OK_STATUS;
}
return action.run(progress);
}
}
7 changes: 5 additions & 2 deletions ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/shared/target/messages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,18 @@ TargetContentsGroup_resolveCancelled=Resolve Cancelled
TargetLocationsGroup_update=Looks for newer versions of the target contents and updates them
TargetLocationsGroup_refresh=Clears the cached target data and resolves the target, looking for newer versions of any artifacts
TargetLocationsGroup_1=&Show location content
TargetLocationsGroup_group_refresh=Refreshing...
TargetLocationsGroup_operation_in_progress=Target operation in progress
TargetLocationsGroup_operation_running=A target editor operation is currently running, do you want to cancel it?
TargetLocationsGroup_TargetUpdateErrorDialog=Problems Updating Target Definition
TargetStatus_NoActiveTargetPlatformStatus=No active target platform
TargetStatus_TargetStatusDefaultString=Target Platform
TargetStatus_UnresolvedTarget=Unresolved ''{0}''
TargetStatus_UnresolvedTargetStatus=Target platform is not loaded
UpdateTargetJob_TargetUpdateFailedStatus=The target definition did not update successfully
UpdateTargetJob_TargetUpdateSuccessStatus=Target definition update completed successfully
UpdateTargetJob_UpdateJobName=Update Target Definition
UpdateTargetJob_RefreshJobName=Refresh Target Definition
UpdateTargetJob_UpdatingTarget=Updating Target
UpdateTargetJob_RefreshingTarget=Refreshing Target
EditTargetContainerPage_Add_Title=Create target reference
EditTargetContainerPage_Edit_Title=Edit target reference
EditTargetContainerPage_Message=Please enter a URI to the target to be referenced
Expand Down

0 comments on commit 511067d

Please sign in to comment.