Skip to content

Commit

Permalink
Fix IProgressMonitor usage #335
Browse files Browse the repository at this point in the history
passing a monitor to two callees requires it to be splitted.

#335

Also fixed forgotten monitor.done() in performProvisioningPlan and
improved tests.
  • Loading branch information
EcljpseB0T authored and HannesWell committed Nov 5, 2023
1 parent 2ed3a71 commit e31c371
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ public RawMirrorRequest(IArtifactDescriptor sourceDescriptor, IArtifactDescripto

@Override
public void perform(IArtifactRepository sourceRepository, IProgressMonitor monitor) {
monitor.subTask(NLS.bind(Messages.downloading, getArtifactKey().getId()));
SubMonitor subMon = SubMonitor.convert(monitor, NLS.bind(Messages.downloading, getArtifactKey().getId()), 1);
setSourceRepository(sourceRepository);
// Do we already have the descriptor in the target?
if (target.contains(targetDescriptor)) {
setResult(new Status(IStatus.INFO, Activator.ID, NLS.bind(Messages.mirror_alreadyExists, targetDescriptor, target)));
setResult(Status.info(NLS.bind(Messages.mirror_alreadyExists, targetDescriptor, target)));
return;
}
// Does the source actually have the descriptor?
if (!source.contains(getArtifactDescriptor())) {
setResult(new Status(IStatus.ERROR, Activator.ID, NLS.bind(Messages.artifact_not_found, getArtifactKey())));
return;
}
IStatus status = transfer(targetDescriptor, sourceDescriptor, monitor);
IStatus status = transfer(targetDescriptor, sourceDescriptor, subMon.newChild(1));

// if ok, cancelled or transfer has already been done with the canonical form return with status set
if (status.getSeverity() == IStatus.CANCEL) {
Expand Down Expand Up @@ -88,6 +88,7 @@ public IArtifactDescriptor getArtifactDescriptor() {
// Perform the mirror operation without any processing steps
@Override
protected IStatus getArtifact(IArtifactDescriptor artifactDescriptor, OutputStream destination, IProgressMonitor monitor) {
SubMonitor subMon = SubMonitor.convert(monitor, 2);
if (SimpleArtifactRepository.CHECKSUMS_ENABLED) {
Collection<ChecksumVerifier> steps = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor,
IArtifactDescriptor.DOWNLOAD_CHECKSUM, Collections.emptySet());
Expand All @@ -98,9 +99,9 @@ protected IStatus getArtifact(IArtifactDescriptor artifactDescriptor, OutputStre
ProcessingStep[] stepArray = steps.toArray(new ProcessingStep[steps.size()]);
// TODO should probably be using createAndLink here
ProcessingStepHandler handler = new ProcessingStepHandler();
destination = handler.link(stepArray, destination, monitor);
destination = handler.link(stepArray, destination, subMon.split(1));
}

return getSourceRepository().getRawArtifact(artifactDescriptor, destination, monitor);
subMon.setWorkRemaining(1);
return getSourceRepository().getRawArtifact(artifactDescriptor, destination, subMon.split(1));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ private boolean doRemoveArtifact(IArtifactDescriptor descriptor) {
}

protected IStatus downloadArtifact(IArtifactDescriptor descriptor, OutputStream destination, IProgressMonitor monitor) {
monitor = IProgressMonitor.nullSafe(monitor);
SubMonitor subMon = SubMonitor.convert(monitor, 2);
if (isFolderBased(descriptor)) {
File artifactFolder = getArtifactFile(descriptor);
if (artifactFolder == null) {
Expand Down Expand Up @@ -655,8 +655,8 @@ protected IStatus downloadArtifact(IArtifactDescriptor descriptor, OutputStream
URI baseLocation = getLocation(descriptor);
if (baseLocation == null)
return new Status(IStatus.ERROR, Activator.ID, NLS.bind(Messages.no_location, descriptor));
URI mirrorLocation = getMirror(baseLocation, monitor);
IStatus status = downloadArtifact(mirrorLocation, destination, monitor);
URI mirrorLocation = getMirror(baseLocation, subMon.split(1));
IStatus status = downloadArtifact(mirrorLocation, destination, subMon.split(1));
IStatus result = reportStatus(descriptor, destination, status);
// if the original download went reasonably but the reportStatus found some issues
// (e..g, in the processing steps/validators) then mark the mirror as bad and return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public IStatus perform(IProfile iprofile, IPhaseSet phases, Operand[] operands,
Profile profile = profileRegistry.validate(iprofile);

profileRegistry.lockProfile(profile);
SubMonitor subMon = SubMonitor.convert(monitor, 3);
try {
eventBus.publishEvent(new BeginOperationEvent(profile, phaseSet, operands, this));
if (DebugHelper.DEBUG_ENGINE)
Expand All @@ -91,17 +92,17 @@ public IStatus perform(IProfile iprofile, IPhaseSet phases, Operand[] operands,
}
context.setProperty(ProvisioningContext.CHECK_AUTHORITIES, property);
}

MultiStatus result = phaseSet.perform(session, operands, monitor);
MultiStatus result = phaseSet.perform(session, operands, subMon.split(1));
if (result.isOK() || result.matches(IStatus.INFO | IStatus.WARNING)) {
if (DebugHelper.DEBUG_ENGINE)
DebugHelper.debug(ENGINE, "Preparing to commit engine operation for profile=" + profile.getProfileId()); //$NON-NLS-1$
result.merge(session.prepare(monitor));
result.merge(session.prepare(subMon.split(1)));
}
subMon.setWorkRemaining(1);
if (result.matches(IStatus.ERROR | IStatus.CANCEL)) {
if (DebugHelper.DEBUG_ENGINE)
DebugHelper.debug(ENGINE, "Rolling back engine operation for profile=" + profile.getProfileId() + ". Reason was: " + result.toString()); //$NON-NLS-1$ //$NON-NLS-2$
IStatus status = session.rollback(monitor, result.getSeverity());
IStatus status = session.rollback(subMon.split(1), result.getSeverity());
if (status.matches(IStatus.ERROR))
LogHelper.log(status);
eventBus.publishEvent(new RollbackOperationEvent(profile, phaseSet, operands, this, result));
Expand All @@ -110,7 +111,7 @@ public IStatus perform(IProfile iprofile, IPhaseSet phases, Operand[] operands,
DebugHelper.debug(ENGINE, "Committing engine operation for profile=" + profile.getProfileId()); //$NON-NLS-1$
if (profile.isChanged())
profileRegistry.updateProfile(profile);
IStatus status = session.commit(monitor);
IStatus status = session.commit(subMon.split(1));
if (status.matches(IStatus.ERROR))
LogHelper.log(status);
eventBus.publishEvent(new CommitOperationEvent(profile, phaseSet, operands, this));
Expand All @@ -121,6 +122,7 @@ public IStatus perform(IProfile iprofile, IPhaseSet phases, Operand[] operands,
} finally {
profileRegistry.unlockProfile(profile);
profile.setChanged(false);
monitor.done();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* resolution status to be reported to a client to determine whether the operation
* should proceed. Each phase of the operation can be performed synchronously or in
* the background as a job. To perform the operation synchronously:
*
*
* <pre>
* IStatus result = op.resolveModal(monitor);
* if (result.isOK())
Expand All @@ -37,10 +37,10 @@
* // interpret the result
* }
* </pre>
*
*
* To perform the resolution synchronously and the provisioning job in the
* background:
*
*
* <pre>
* IStatus status = op.resolveModal(monitor);
* if (status.isOK()) {
Expand All @@ -50,9 +50,9 @@
* // interpret the result
* }
* </pre>
*
*
* To resolve in the background and perform the job when it is complete:
*
*
* <pre>
* ProvisioningJob job = op.getResolveJob(monitor);
* job.addJobChangeListener(new JobChangeAdapter() {
Expand All @@ -65,18 +65,18 @@
* }
* });
* job.schedule();
*
*
* </pre>
*
* In general, it is expected that clients create a new ProfileChangeOperation if
*
* In general, it is expected that clients create a new ProfileChangeOperation if
* the resolution result of the current operation is not satisfactory. However,
* subclasses may prescribe a different life cycle where appropriate.
*
* When retrieving the resolution and provisioning jobs managed by this operation,
*
* When retrieving the resolution and provisioning jobs managed by this operation,
* a client may supply a progress monitor to be used by the job. When the job is
* run by the platform job manager, both the platform job manager progress indicator
* run by the platform job manager, both the platform job manager progress indicator
* and the monitor supplied by the client will be updated.
*
*
* @noextend This class is not intended to be subclassed by clients.
* @since 2.0
*/
Expand All @@ -93,7 +93,7 @@ public abstract class ProfileChangeOperation implements IProfileChangeJob {
* Create an operation using the provided provisioning session.
* Unless otherwise specified by the client, the operation is
* performed on the currently running profile.
*
*
* @param session the provisioning session providing the services
*/
protected ProfileChangeOperation(ProvisioningSession session) {
Expand All @@ -105,20 +105,20 @@ protected ProfileChangeOperation(ProvisioningSession session) {
/**
* Resolve the operation in the current thread using the specified progress
* monitor. Return a status describing the result of the resolution.
*
*
* @param monitor the progress monitor to use
* @return a status describing the resolution results
*/
public final IStatus resolveModal(IProgressMonitor monitor) {
if (monitor == null)
monitor = new NullProgressMonitor();
SubMonitor subMon = SubMonitor.convert(monitor, 2);
prepareToResolve();
makeResolveJob(monitor);
makeResolveJob(subMon.split(1));
if (job != null) {
IStatus status = job.runModal(monitor);
IStatus status = job.runModal(subMon.split(1));
if (status.getSeverity() == IStatus.CANCEL)
return Status.CANCEL_STATUS;
}
monitor.done();
// For anything other than cancellation, we examine the artifacts of the resolution and come
// up with an overall summary.
return getResolutionResult();
Expand All @@ -135,11 +135,11 @@ public void setProfileId(String id) {

/**
* Return a job that can be used to resolve this operation in the background.
*
*
* @param monitor a progress monitor that should be used to report the job's progress in addition
* to the standard job progress reporting. Can be <code>null</code>. If provided, this monitor
* to the standard job progress reporting. Can be <code>null</code>. If provided, this monitor
* will be called from a background thread.
*
*
* @return a job that can be scheduled to perform the provisioning operation.
*/
public final ProvisioningJob getResolveJob(IProgressMonitor monitor) {
Expand Down Expand Up @@ -178,11 +178,11 @@ void makeResolveJob(IProgressMonitor monitor) {

/**
* Compute the profile change request for this operation, adding any relevant intermediate status
* to the supplied status.
*
* to the supplied status.
*
* @param status a multi-status to be used to add relevant status. If a profile change request cannot
* be computed for any reason, a status should be added to explain the problem.
*
*
* @param monitor the progress monitor to use for computing the profile change request
*/
protected abstract void computeProfileChangeRequest(MultiStatus status, IProgressMonitor monitor);
Expand All @@ -193,14 +193,14 @@ private void createPlannerResolutionJob() {

/**
* Return an appropriate name for the resolution job.
*
*
* @return the resolution job name.
*/
protected abstract String getResolveJobName();

/**
* Return an appropriate name for the provisioning job.
*
*
* @return the provisioning job name.
*/
protected abstract String getProvisioningJobName();
Expand All @@ -209,7 +209,7 @@ private void createPlannerResolutionJob() {
* Return a status indicating the result of resolving this
* operation. A <code>null</code> return indicates that
* resolving has not occurred yet.
*
*
* @return the status of the resolution, or <code>null</code>
* if resolution has not yet occurred.
*/
Expand All @@ -231,7 +231,7 @@ public IStatus getResolutionResult() {
/**
* Return a string that can be used to describe the results of the resolution
* to a client.
*
*
* @return a string describing the resolution details, or <code>null</code> if the
* operation has not been resolved.
*/
Expand All @@ -250,9 +250,9 @@ public String getResolutionDetails() {
/**
* Return a string that describes the specific resolution results
* related to the supplied {@link IInstallableUnit}.
*
*
* @param iu the IInstallableUnit for which resolution details are requested
*
*
* @return a string describing the results for the installable unit, or <code>null</code> if
* there are no specific results available for the installable unit.
*/
Expand All @@ -265,12 +265,12 @@ public String getResolutionDetails(IInstallableUnit iu) {

/**
* Return the provisioning plan obtained by resolving the receiver.
*
*
* @return the provisioning plan. This may be <code>null</code> if the operation
* has not been resolved, or if a plan could not be obtained when attempting to
* resolve. If the plan is null and the operation has been resolved, then the
* resolution result will explain the problem.
*
*
* @see #hasResolved()
* @see #getResolutionResult()
*/
Expand All @@ -282,12 +282,12 @@ public IProvisioningPlan getProvisioningPlan() {

/**
* Return the profile change request that describes the receiver.
*
*
* @return the profile change request. This may be <code>null</code> if the operation
* has not been resolved, or if a profile change request could not be assembled given
* the operation's state. If the profile change request is null and the operation has
* been resolved, the the resolution result will explain the problem.
*
*
* @see #hasResolved()
* @see #getResolutionResult()
* @since 2.1
Expand All @@ -299,22 +299,22 @@ public IProfileChangeRequest getProfileChangeRequest() {
}

/**
* Return a provisioning job that can be used to perform the resolved operation. The job is
* Return a provisioning job that can be used to perform the resolved operation. The job is
* created using the default values associated with a new job. It is up to clients to configure
* the priority of the job and set any appropriate properties, such as
* {@link Job#setUser(boolean)},
* {@link Job#setUser(boolean)},
* {@link Job#setSystem(boolean)}, or {@link Job#setProperty(QualifiedName, Object)},
* before scheduling it.
*
*
* @param monitor a progress monitor that should be used to report the job's progress in addition
* to the standard job progress reporting. Can be <code>null</code>. If provided, this monitor
* to the standard job progress reporting. Can be <code>null</code>. If provided, this monitor
* will be called from a background thread.
*
* @return a job that can be used to perform the provisioning operation. This may be <code>null</code>
*
* @return a job that can be used to perform the provisioning operation. This may be <code>null</code>
* if the operation has not been resolved, or if a plan could not be obtained when attempting to
* resolve. If the job is null and the operation has been resolved, then the resolution result
* resolve. If the job is null and the operation has been resolved, then the resolution result
* will explain the problem.
*
*
* @see #hasResolved()
* @see #getResolutionResult()
*/
Expand All @@ -337,7 +337,7 @@ public ProvisioningJob getProvisioningJob(IProgressMonitor monitor) {
* Set the provisioning context that should be used to resolve and perform the provisioning for
* the operation. This must be set before an attempt is made to resolve the operation
* for it to have any effect.
*
*
* @param context the provisioning context.
*/
public void setProvisioningContext(ProvisioningContext context) {
Expand All @@ -349,7 +349,7 @@ public void setProvisioningContext(ProvisioningContext context) {
/**
* Get the provisioning context that will be used to resolve and perform the provisioning for
* the operation.
*
*
* @return the provisioning context
*/
public ProvisioningContext getProvisioningContext() {
Expand All @@ -367,7 +367,7 @@ public String getProfileId() {
* change request, provisioning plan, or resolution result. It is possible that this
* method return <code>false</code> while resolution is taking place if it is performed
* in the background.
*
*
* @return <code>true</code> if the operation has been resolved, <code>false</code>
* if it has not resolved.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ public IStatus runModal(IProgressMonitor monitor) {
IStatus status = Status.OK_STATUS;
if (task == null)
task = getName();
monitor.beginTask(task, 1000);
try {
status = getSession().performProvisioningPlan(plan, phaseSet, provisioningContext, SubMonitor.convert(monitor, 1000));
SubMonitor subMonitor = SubMonitor.convert(monitor, task, 1000);
status = getSession().performProvisioningPlan(plan, phaseSet, provisioningContext, subMonitor);
} finally {
monitor.done();
}
Expand Down
Loading

0 comments on commit e31c371

Please sign in to comment.