-
Notifications
You must be signed in to change notification settings - Fork 52
Prompt the user if unpublished changes detected during launch #2047
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
Changes from all commits
911889e
03ab310
0fc7a39
23a28f8
350b10f
280c8a8
1bdef74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| import com.google.cloud.tools.eclipse.appengine.localserver.Messages; | ||
| import com.google.cloud.tools.eclipse.appengine.localserver.PreferencesInitializer; | ||
| import com.google.cloud.tools.eclipse.appengine.localserver.ui.LocalAppEngineConsole; | ||
| import com.google.cloud.tools.eclipse.appengine.localserver.ui.StaleResourcesStatusHandler; | ||
| import com.google.cloud.tools.eclipse.ui.util.MessageConsoleUtilities; | ||
| import com.google.cloud.tools.eclipse.ui.util.WorkbenchUtil; | ||
| import com.google.cloud.tools.eclipse.usagetracker.AnalyticsEvents; | ||
|
|
@@ -50,13 +51,16 @@ | |
| import java.util.logging.Logger; | ||
| import org.eclipse.core.resources.IMarkerDelta; | ||
| import org.eclipse.core.resources.IProject; | ||
| import org.eclipse.core.resources.ResourcesPlugin; | ||
| import org.eclipse.core.runtime.CoreException; | ||
| import org.eclipse.core.runtime.IPath; | ||
| import org.eclipse.core.runtime.IProgressMonitor; | ||
| import org.eclipse.core.runtime.IStatus; | ||
| import org.eclipse.core.runtime.MultiStatus; | ||
| import org.eclipse.core.runtime.Platform; | ||
| import org.eclipse.core.runtime.Status; | ||
| import org.eclipse.core.runtime.SubMonitor; | ||
| import org.eclipse.core.runtime.jobs.Job; | ||
| import org.eclipse.debug.core.DebugEvent; | ||
| import org.eclipse.debug.core.DebugException; | ||
| import org.eclipse.debug.core.DebugPlugin; | ||
|
|
@@ -65,6 +69,7 @@ | |
| import org.eclipse.debug.core.ILaunchConfigurationType; | ||
| import org.eclipse.debug.core.ILaunchManager; | ||
| import org.eclipse.debug.core.ILaunchesListener2; | ||
| import org.eclipse.debug.core.IStatusHandler; | ||
| import org.eclipse.debug.core.model.DebugElement; | ||
| import org.eclipse.debug.core.model.IBreakpoint; | ||
| import org.eclipse.debug.core.model.IDebugTarget; | ||
|
|
@@ -82,6 +87,7 @@ | |
| import org.eclipse.wst.server.core.IModule; | ||
| import org.eclipse.wst.server.core.IServer; | ||
| import org.eclipse.wst.server.core.IServerListener; | ||
| import org.eclipse.wst.server.core.ServerCore; | ||
| import org.eclipse.wst.server.core.ServerEvent; | ||
| import org.eclipse.wst.server.core.ServerUtil; | ||
|
|
||
|
|
@@ -128,6 +134,65 @@ public ILaunch getLaunch(ILaunchConfiguration configuration, String mode) throws | |
| return super.getLaunch(configuration, mode); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean finalLaunchCheck(ILaunchConfiguration configuration, String mode, | ||
| IProgressMonitor monitor) throws CoreException { | ||
| SubMonitor progress = SubMonitor.convert(monitor, 40); | ||
| if (!super.finalLaunchCheck(configuration, mode, progress.newChild(20))) { | ||
| return false; | ||
| } | ||
|
|
||
| // If we're auto-publishing before launch, check if there may be stale | ||
| // resources not yet published. See | ||
| // https://github.com/GoogleCloudPlatform/google-cloud-eclipse/issues/1832 | ||
| if (ServerCore.isAutoPublishing() && ResourcesPlugin.getWorkspace().isAutoBuilding()) { | ||
| // Must wait for any current autobuild to complete so resource changes are triggered | ||
| // and WTP will kick off ResourceChangeJobs. Note that there may be builds | ||
| // pending that are unrelated to our resource changes, so simply checking | ||
| // <code>JobManager.find(FAMILY_AUTO_BUILD).length > 0</code> produces too many | ||
| // false positives. | ||
| try { | ||
| Job.getJobManager().join(ResourcesPlugin.FAMILY_AUTO_BUILD, progress.newChild(20)); | ||
| } catch (InterruptedException ex) { | ||
| /* ignore */ | ||
| } | ||
| IServer server = ServerUtil.getServer(configuration); | ||
| if (server.shouldPublish() || hasPendingChangesToPublish()) { | ||
| IStatusHandler prompter = DebugPlugin.getDefault().getStatusHandler(promptStatus); | ||
| if (prompter != null) { | ||
| Object continueLaunch = prompter | ||
| .handleStatus(StaleResourcesStatusHandler.CONTINUE_LAUNCH_REQUEST, configuration); | ||
| if (!(Boolean) continueLaunch) { | ||
| // cancel the launch so Server.StartJob won't raise an error dialog, since the | ||
| // server won't have been started | ||
| monitor.setCanceled(true); | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Check if there are pending changes to be published: this is a nasty condition that can occur if | ||
| * the user saves changes as part of launching the server. | ||
| */ | ||
| private boolean hasPendingChangesToPublish() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just wondering if this method can be called before some ResourceChangeJob is actually scheduled?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. It may not… It should be safe to join on the AUTOBUILD job.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. However, I'm fine with the current status if that turns out too complex or not really worth.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After experimenting, we need to join on the auto build job to ensure the workspace's resource changes happen, so that we see that there are pending events for publishing. |
||
| Job[] serverJobs = Job.getJobManager().find(ServerUtil.SERVER_JOB_FAMILY); | ||
| Job currentJob = Job.getJobManager().currentJob(); | ||
| for (Job job : serverJobs) { | ||
| // Launching from Server#start() means this will be running within a | ||
| // Server.StartJob. All other jobs should be ResourceChangeJob or | ||
| // PublishJob, both of which indicate unpublished changes. | ||
| if (job != currentJob) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| void checkConflictingLaunches(ILaunchConfigurationType launchConfigType, | ||
| DefaultRunConfiguration runConfig, ILaunch[] launches) throws CoreException { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /* | ||
| * Copyright 2017 Google Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.cloud.tools.eclipse.appengine.localserver.ui; | ||
|
|
||
| import com.google.cloud.tools.eclipse.appengine.localserver.Messages; | ||
| import org.eclipse.core.runtime.CoreException; | ||
| import org.eclipse.core.runtime.IStatus; | ||
| import org.eclipse.core.runtime.Status; | ||
| import org.eclipse.debug.core.ILaunchConfiguration; | ||
| import org.eclipse.debug.core.IStatusHandler; | ||
| import org.eclipse.debug.internal.ui.DebugUIPlugin; | ||
| import org.eclipse.debug.ui.DebugUITools; | ||
| import org.eclipse.jface.dialogs.MessageDialog; | ||
| import org.eclipse.swt.widgets.Shell; | ||
|
|
||
| /** | ||
| * A simple prompter for the Platform/Debug framework to prompt the user to confirm whether the | ||
| * launch should continue when possibly stale resources have been found. | ||
| */ | ||
| public class StaleResourcesStatusHandler implements IStatusHandler { | ||
| /** | ||
| * The error code indicating that there may be stale resources. Used with the plugin ID to | ||
| * uniquely identify this prompter. | ||
| */ | ||
| static final int CONFIRM_LAUNCH_CODE = 255; | ||
|
|
||
| /** | ||
| * A specially crafted status message that is pass into the Debug Prompter class to obtain our | ||
| * confirmation prompter. | ||
| */ | ||
| public static final IStatus CONTINUE_LAUNCH_REQUEST = new Status(IStatus.INFO, | ||
| "com.google.cloud.tools.eclipse.appengine.localserver", CONFIRM_LAUNCH_CODE, "", null); | ||
|
|
||
| @Override | ||
| public Object handleStatus(IStatus status, Object source) throws CoreException { | ||
| if (source instanceof ILaunchConfiguration) { | ||
| ILaunchConfiguration config = (ILaunchConfiguration) source; | ||
| if (DebugUITools.isPrivate(config)) { | ||
| return Boolean.FALSE; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does a "private" config mean? So, if it's private, does this stop launching?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I gather, it's used for launches that aren't visible to the user. Like Ant builds or Maven builds. We could conceivably usr launches for our staging, etc. The advantage being that it's tracked and managed by the launch info. |
||
| } | ||
| } | ||
|
|
||
| Shell activeShell = DebugUIPlugin.getShell(); | ||
| // should consider using MessageDialogWithToggle? | ||
| return MessageDialog.openQuestion(activeShell, Messages.getString("STALE_RESOURCES_DETECTED"), | ||
| Messages.getString("STALE_RESOURCES_LAUNCH_CONFIRMATION")); | ||
| } | ||
| } | ||
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.
a comment for "255" might be helpful