From 717ab82836fea66729a6f939564508324bfa86e5 Mon Sep 17 00:00:00 2001 From: nbashirbello Date: Wed, 8 Feb 2017 14:29:52 -0500 Subject: [PATCH] Auto-open browser on deploy (#1332) * Cleaned up StandardDeployJob and added step to auto-open the deployed app in a browser * Moved StandardDeployJob.DeployOutput and StandardDeployJob#parseDeployJsonOutput into AppEngineDeployUtil * Minor clean up * When creating DeployOutput check for a valid version object * Removed duplicate version check * Minor changes per PR comments * Separated StdOutLineListener for the deploy and staging process * Created a utility function for lauching a web browser with a specified url. * Utilizing WorkbenchUtil#openInBrowser * Minor changes per PR comments * Created a private constructor for AppEngineDeployOutput so that the only way to create an instance is on a successful call to AppEngineDeployOutput#parse * Changed variable name * Nit * Created a VersionNotFoundException to be thrown when we cannot determine the version of the deployed application * Created a convience method WorkbenchUtil#openInBrowser(IWorkbench workbench, String urlPath) * Updated javadoc for WorkbenchUtil#openInBrowserInUiThread and WorkbenchUtil#openInBrowser --- .../deploy/AppEngineDeployOutputTest.java | 97 +++++++++ .../META-INF/MANIFEST.MF | 7 +- .../deploy/AppEngineDeployOutput.java | 68 ++++++ .../deploy/VersionNotFoundException.java | 30 +++ .../appengine/deploy/messages.properties | 3 + .../deploy/standard/StandardDeployJob.java | 196 +++++++++++++----- ...gineServerLaunchConfigurationDelegate.java | 29 +-- .../META-INF/MANIFEST.MF | 3 +- .../preferences/AnalyticsOptInArea.java | 12 +- .../META-INF/MANIFEST.MF | 3 +- ...essageConsoleWriterOutputLineListener.java | 2 +- .../preferences/CloudSdkPreferenceArea.java | 17 +- .../sdk/ui/preferences/messages.properties | 2 - .../META-INF/MANIFEST.MF | 1 + .../tools/eclipse/ui/util/WorkbenchUtil.java | 77 +++++++ .../console/BrowserSupportBasedHyperlink.java | 25 +-- .../tools/eclipse/ui/util/messages.properties | 2 +- 17 files changed, 439 insertions(+), 135 deletions(-) create mode 100644 plugins/com.google.cloud.tools.eclipse.appengine.deploy.test/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutputTest.java create mode 100644 plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutput.java create mode 100644 plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/VersionNotFoundException.java diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.deploy.test/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutputTest.java b/plugins/com.google.cloud.tools.eclipse.appengine.deploy.test/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutputTest.java new file mode 100644 index 0000000000..d651ded2b9 --- /dev/null +++ b/plugins/com.google.cloud.tools.eclipse.appengine.deploy.test/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutputTest.java @@ -0,0 +1,97 @@ +/* + * 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.deploy; + +import com.google.gson.JsonParseException; +import org.junit.Assert; +import org.junit.Test; + +/** + * Unit tests for {@link AppEngineDeployOutput} + */ +public class AppEngineDeployOutputTest { + @Test + public void testDeployOutputJsonParsingOneVersion() { + String jsonOutput = + "{\n" + + " \"configs\": [],\n" + + " \"versions\": [\n" + + " {\n" + + " \"id\": \"20160429t112518\",\n" + + " \"last_deployed_time\": null,\n" + + " \"project\": \"some-project\",\n" + + " \"service\": \"default\",\n" + + " \"traffic_split\": null,\n" + + " \"version\": null\n" + + " }\n" + + " ]\n" + + "}\n"; + + AppEngineDeployOutput deployOutput = + AppEngineDeployOutput.parse(jsonOutput); + Assert.assertEquals("20160429t112518", deployOutput.getVersion()); + Assert.assertEquals("default", deployOutput.getService()); + } + + @Test + public void testDeployOutputJsonParsingTwoVersions() { + String jsonOutput = + "{\n" + + " \"configs\": [],\n" + + " \"versions\": [\n" + + " {\n" + + " \"id\": \"20160429t112518\",\n" + + " \"last_deployed_time\": null,\n" + + " \"project\": \"some-project\",\n" + + " \"service\": \"default\",\n" + + " \"traffic_split\": null,\n" + + " \"version\": null\n" + + " },\n" + + " {\n" + + " \"id\": \"20160429t112518\",\n" + + " \"last_deployed_time\": null,\n" + + " \"project\": \"some-project\",\n" + + " \"service\": \"default\",\n" + + " \"traffic_split\": null,\n" + + " \"version\": null\n" + + " }\n" + + " ]\n" + + "}\n"; + + try { + AppEngineDeployOutput.parse(jsonOutput); + Assert.fail("Failure to throw exception when parsing deploy output with more that one version entry"); + } catch (JsonParseException e) { + // Success! Should throw a JsonParseException. + } + } + + @Test + public void testDeployOutputJsonParsingOldFormat() { + String jsonOutput = + "{\n" + + " \"default\": \"https://springboot-maven-project.appspot.com\"\n" + + "}\n"; + + try { + AppEngineDeployOutput.parse(jsonOutput); + Assert.fail("Failure to throw exception when parsing deploy output in old format"); + } catch (JsonParseException e) { + // Success! Should throw a JsonParseException. + } + } +} diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/META-INF/MANIFEST.MF b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/META-INF/MANIFEST.MF index ccb54f91dd..8ade2b59b8 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/META-INF/MANIFEST.MF +++ b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/META-INF/MANIFEST.MF @@ -7,14 +7,17 @@ Bundle-Vendor: %providerName Bundle-Localization: plugin Bundle-RequiredExecutionEnvironment: JavaSE-1.7 Bundle-ActivationPolicy: lazy -Require-Bundle: com.google.cloud.tools.appengine;bundle-version="0.2.6", +Require-Bundle: com.google.cloud.tools.appengine, com.google.cloud.tools.eclipse.util, + com.google.gson, com.google.guava;bundle-version="[20.0.0,21.0.0)", org.eclipse.core.commands, org.eclipse.core.jobs, org.eclipse.core.resources, org.eclipse.equinox.common, org.eclipse.jst.j2ee.ui, + org.eclipse.swt, + org.eclipse.ui.workbench, org.eclipse.wst.common.modulecore, org.eclipse.wst.common.project.facet.core, org.eclipse.wst.server.core @@ -24,6 +27,8 @@ Export-Package: com.google.cloud.tools.eclipse.appengine.deploy, Import-Package: com.google.api.client.auth.oauth2, com.google.cloud.tools.eclipse.login, com.google.cloud.tools.eclipse.sdk, + com.google.cloud.tools.eclipse.sdk.ui, + com.google.cloud.tools.eclipse.ui.util, org.eclipse.core.runtime;bundle-symbolic-name:="org.eclipse.core.runtime", org.eclipse.core.runtime.preferences;version="3.3.0", org.osgi.framework;version="1.8.0", diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutput.java b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutput.java new file mode 100644 index 0000000000..6d5bb9d2f1 --- /dev/null +++ b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/AppEngineDeployOutput.java @@ -0,0 +1,68 @@ +/* + * 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.deploy; + +import com.google.gson.Gson; +import com.google.gson.JsonParseException; +import java.util.List; + +/** + * Holds de-serialized JSON output of gcloud app deploy. Don't change the field names + * because Gson uses them for automatic de-serialization. + */ +// TODO: move into appengine-plugins-core +// TODO expand to include other Version attributes +public class AppEngineDeployOutput { + private AppEngineDeployOutput() { + } + + private static class Version { + String id; + String service; + } + + private List versions; + + /** + * @return version, can be null + */ + public String getVersion() { + return versions.get(0).id; + } + + /** + * @return service, can be null + */ + public String getService() { + return versions.get(0).service; + } + + /** + * Parse the raw JSON output of the deployment. + * + * @return the output of gcloud app deploy + * @throws JsonParseException if unable to extract the deploy output information needed + */ + public static AppEngineDeployOutput parse(String jsonOutput) throws JsonParseException { + AppEngineDeployOutput deployOutput = new Gson().fromJson(jsonOutput, AppEngineDeployOutput.class); + if (deployOutput == null + || deployOutput.versions == null || deployOutput.versions.size() != 1) { + throw new JsonParseException("Cannot get app version: unexpected gcloud JSON output format"); + } + return deployOutput; + } +} diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/VersionNotFoundException.java b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/VersionNotFoundException.java new file mode 100644 index 0000000000..2179b3ed42 --- /dev/null +++ b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/VersionNotFoundException.java @@ -0,0 +1,30 @@ +/* + * 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.deploy; + +/** + * Thrown if the version of a deployed App Engine application cannot be determined. + */ +public class VersionNotFoundException extends Exception { + + private static final long serialVersionUID = 1L; + + public VersionNotFoundException(String message, Throwable cause) { + super(message, cause); + } + +} diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/messages.properties b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/messages.properties index fcd8354687..98af88581a 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/messages.properties +++ b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/messages.properties @@ -13,3 +13,6 @@ deploy.job.staging.failed=Staging failed. Check the error message in the Console deploy.job.deploy.failed=Deploy failed. Check the error message in the Console View deploy.failed.error.message=Deploy failed. cloudsdk.process.failed=Process exited with error code {0} +save.credential.failed=Error temporarily saving credential +browser.launch.failed=Error launching deployed app in browser +browser.launch.title=App Engine Deploy - {0} \ No newline at end of file diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/standard/StandardDeployJob.java b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/standard/StandardDeployJob.java index 8d5c4bd781..ffd51eb42a 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/standard/StandardDeployJob.java +++ b/plugins/com.google.cloud.tools.eclipse.appengine.deploy/src/com/google/cloud/tools/eclipse/appengine/deploy/standard/StandardDeployJob.java @@ -22,14 +22,18 @@ import com.google.cloud.tools.appengine.cloudsdk.process.ProcessExitListener; import com.google.cloud.tools.appengine.cloudsdk.process.ProcessOutputLineListener; import com.google.cloud.tools.appengine.cloudsdk.process.ProcessStartListener; +import com.google.cloud.tools.eclipse.appengine.deploy.AppEngineDeployOutput; import com.google.cloud.tools.eclipse.appengine.deploy.AppEngineProjectDeployer; import com.google.cloud.tools.eclipse.appengine.deploy.Messages; +import com.google.cloud.tools.eclipse.appengine.deploy.VersionNotFoundException; import com.google.cloud.tools.eclipse.login.CredentialHelper; import com.google.cloud.tools.eclipse.sdk.CollectingLineListener; +import com.google.cloud.tools.eclipse.ui.util.WorkbenchUtil; import com.google.cloud.tools.eclipse.util.CloudToolsInfo; import com.google.cloud.tools.eclipse.util.status.StatusUtil; import com.google.common.base.Joiner; import com.google.common.base.Predicate; +import com.google.gson.JsonParseException; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -43,6 +47,7 @@ import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.OperationCanceledException; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.SubMonitor; @@ -54,6 +59,7 @@ *
  • export exploded WAR
    • *
  • stage project for deploy
    • *
  • deploy staged project
    • + *
  • launch the deployed app in browser
    • * * It uses a work directory where it will create separate directories for the exploded WAR and the * staging results. @@ -74,7 +80,8 @@ public class StandardDeployJob extends WorkspaceJob { private IProject project; private Credential credential; protected IPath workDirectoryParent; - private ProcessOutputLineListener stdoutLineListener; + private ProcessOutputLineListener stagingStdoutLineListener; + private ProcessOutputLineListener deployStdoutLineListener; private ProcessOutputLineListener stderrLineListener; private DefaultDeployConfiguration deployConfiguration; private CollectingLineListener errorCollectingLineListener; @@ -82,16 +89,18 @@ public class StandardDeployJob extends WorkspaceJob { public StandardDeployJob(IProject project, Credential credential, IPath workDirectoryParent, - ProcessOutputLineListener stdoutLineListener, + ProcessOutputLineListener stagingStdoutLineListener, ProcessOutputLineListener stderrLineListener, DefaultDeployConfiguration deployConfiguration) { super(Messages.getString("deploy.standard.runnable.name")); //$NON-NLS-1$ this.project = project; this.credential = credential; this.workDirectoryParent = workDirectoryParent; - this.stdoutLineListener = stdoutLineListener; + this.stagingStdoutLineListener = stagingStdoutLineListener; this.stderrLineListener = stderrLineListener; this.deployConfiguration = deployConfiguration; + // TODO: change to StringBuilderProcessOutputLineListener from the appengine-plugins-core + this.deployStdoutLineListener = new StringBuilderProcessOutputLineListener(); errorCollectingLineListener = new CollectingLineListener(new Predicate() { @Override @@ -106,47 +115,30 @@ public boolean apply(String line) { public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException { SubMonitor progress = SubMonitor.convert(monitor, 100); Path credentialFile = null; + try { IPath workDirectory = workDirectoryParent; IPath explodedWarDirectory = workDirectory.append(EXPLODED_WAR_DIRECTORY_NAME); IPath stagingDirectory = workDirectory.append(STAGING_DIRECTORY_NAME); credentialFile = workDirectory.append(CREDENTIAL_FILENAME).toFile().toPath(); - saveCredential(credentialFile, credential); - CloudSdk cloudSdk = getCloudSdk(credentialFile); - try { - getJobManager().beginRule(project, progress); - new ExplodedWarPublisher().publish( - project, explodedWarDirectory, progress.newChild(10)); - new StandardProjectStaging().stage( - explodedWarDirectory, stagingDirectory, cloudSdk, progress.newChild(20)); - } finally { - getJobManager().endRule(project); + IStatus saveStatus = saveCredential(credentialFile, credential); + if (saveStatus != Status.OK_STATUS) { + return saveStatus; } - if (!cloudSdkProcessStatus.isOK()) { - if (cloudSdkProcessStatus == Status.CANCEL_STATUS) { - return cloudSdkProcessStatus; - } - // temporary way of error handling, after #439 is fixed, it'll be cleaner - String errorMessage = - getErrorMessageOrDefault(Messages.getString("deploy.job.staging.failed")); - return StatusUtil.error(getClass(), errorMessage); + IStatus stagingStatus = + stageProject(credentialFile, explodedWarDirectory, stagingDirectory, progress.newChild(30)); + if (stagingStatus != Status.OK_STATUS) { + return stagingStatus; } - new AppEngineProjectDeployer().deploy( - stagingDirectory, cloudSdk, deployConfiguration, progress.newChild(70)); - if (!cloudSdkProcessStatus.isOK() && cloudSdkProcessStatus != Status.CANCEL_STATUS) { - // temporary way of error handling, after #439 is fixed, it'll be cleaner - String errorMessage = - getErrorMessageOrDefault(Messages.getString("deploy.job.deploy.failed")); - return StatusUtil.error(getClass(), errorMessage); + + IStatus deployStatus = deployProject(credentialFile, stagingDirectory, progress.newChild(70)); + if (deployStatus != Status.OK_STATUS) { + return deployStatus; } - return cloudSdkProcessStatus; - } catch (IOException exception) { - throw new CoreException(StatusUtil.error(getClass(), - Messages.getString("save.credential.failed"), - exception)); + return openAppInBrowser(); } finally { if (credentialFile != null) { try { @@ -168,38 +160,101 @@ protected void canceling() { super.canceling(); } - /** - * @return the error message obtained from config.getErrorMessageProvider() or - * defaultMessage - */ - private String getErrorMessageOrDefault(String defaultMessage) { - List messages = errorCollectingLineListener.getCollectedMessages(); - if (!messages.isEmpty()) { - return Joiner.on('\n').join(messages); - } else { - return defaultMessage; + private IStatus saveCredential(Path destination, Credential credential) { + String jsonCredential = new CredentialHelper().toJson(credential); + try { + Files.write(destination, jsonCredential.getBytes(StandardCharsets.UTF_8)); + } catch (IOException ex) { + return StatusUtil.error(getClass(), Messages.getString("save.credential.failed"), ex); } + return Status.OK_STATUS; } - private static void saveCredential(Path destination, Credential credential) throws IOException { - String jsonCredential = new CredentialHelper().toJson(credential); - Files.write(destination, jsonCredential.getBytes(StandardCharsets.UTF_8)); + private IStatus stageProject(Path credentialFile, IPath explodedWarDirectory, IPath stagingDirectory, IProgressMonitor monitor) { + SubMonitor progress = SubMonitor.convert(monitor, 100); + RecordProcessError stagingExitListener = new RecordProcessError(); + CloudSdk cloudSdk = getCloudSdk(credentialFile, stagingStdoutLineListener, stagingExitListener); + + try { + getJobManager().beginRule(project, progress); + new ExplodedWarPublisher().publish(project, explodedWarDirectory, progress.newChild(40)); + new StandardProjectStaging().stage(explodedWarDirectory, stagingDirectory, cloudSdk, progress.newChild(60)); + return stagingExitListener.getExitStatus(); + } catch (CoreException | IllegalArgumentException | OperationCanceledException ex) { + return StatusUtil.error(getClass(), Messages.getString("deploy.job.staging.failed"), ex); + } finally { + getJobManager().endRule(project); + } } - private CloudSdk getCloudSdk(Path credentialFile) { + private IStatus deployProject(Path credentialFile, IPath stagingDirectory, IProgressMonitor monitor) { + RecordProcessError deployExitListener = new RecordProcessError(); + CloudSdk cloudSdk = getCloudSdk(credentialFile, deployStdoutLineListener, deployExitListener); + new AppEngineProjectDeployer().deploy( + stagingDirectory, cloudSdk, deployConfiguration, monitor); + return deployExitListener.getExitStatus(); + } + + private CloudSdk getCloudSdk(Path credentialFile, ProcessOutputLineListener stdoutLineListener, ProcessExitListener processExitListener) { CloudSdk cloudSdk = new CloudSdk.Builder() .addStdOutLineListener(stdoutLineListener) .addStdErrLineListener(stderrLineListener) .addStdErrLineListener(errorCollectingLineListener) .appCommandCredentialFile(credentialFile.toFile()) .startListener(new StoreProcessObjectListener()) - .exitListener(new RecordProcessError()) + .exitListener(processExitListener) .appCommandMetricsEnvironment(CloudToolsInfo.METRICS_NAME) .appCommandMetricsEnvironmentVersion(CloudToolsInfo.getToolsVersion()) + .appCommandOutputFormat("json") .build(); return cloudSdk; } + private IStatus openAppInBrowser() { + final String project = deployConfiguration.getProject(); + String appLocation = null; + if (deployConfiguration.getPromote()) { + appLocation = "https://" + project + ".appspot.com"; + } else { + try { + String version = getDeployedAppVersion(); + appLocation = "https://" + version + "-dot-" + project+ ".appspot.com/"; + } catch (VersionNotFoundException ex) { + return StatusUtil.error(getClass(), Messages.getString("browser.launch.failed"), ex); + } + } + + String browserTitle = Messages.getString("browser.launch.title", project); + WorkbenchUtil.openInBrowserInUiThread(appLocation, null, browserTitle, browserTitle); + return Status.OK_STATUS; + } + + /** + * @return the error message obtained from errorCollectingLineListener() or + * defaultMessage + */ + private String getErrorMessageOrDefault(String defaultMessage) { + // TODO: Check the assumption that if there are error messages during staging collected via + // the errorCollectingLineListener, the staging process will have a non-zero exitcode, + // making it ok to use the same errorCollectingLineListener for the deploy process + List messages = errorCollectingLineListener.getCollectedMessages(); + if (!messages.isEmpty()) { + return Joiner.on('\n').join(messages); + } else { + return defaultMessage; + } + } + + private String getDeployedAppVersion() throws VersionNotFoundException { + try { + String rawDeployOutput = deployStdoutLineListener.toString(); + AppEngineDeployOutput deployOutput = AppEngineDeployOutput.parse(rawDeployOutput); + return deployOutput.getVersion(); + } catch (IndexOutOfBoundsException | JsonParseException ex) { + throw new VersionNotFoundException("Error getting deployed app version", ex); + } + } + private final class StoreProcessObjectListener implements ProcessStartListener { @Override public void onStart(Process proces) { @@ -207,15 +262,46 @@ public void onStart(Process proces) { } } - private final class RecordProcessError implements ProcessExitListener { - // temporary way of error handling, after #439 is fixed, it'll be cleaner + private class RecordProcessError implements ProcessExitListener { + private IStatus status; + @Override public void onExit(int exitCode) { - // if it's cancelled we don't need to record the exit code from the process, it would be the exit code - // that corresponds to the process.destroy() - if (cloudSdkProcessStatus != Status.CANCEL_STATUS && exitCode != 0) { - cloudSdkProcessStatus = StatusUtil.error(this, Messages.getString("cloudsdk.process.failed", exitCode)); + if (cloudSdkProcessStatus == Status.CANCEL_STATUS) { + status = cloudSdkProcessStatus; + } else if (exitCode != 0) { + // temporary way of error handling, after #439 is fixed, it'll be cleaner + String errorMessage = + getErrorMessageOrDefault(Messages.getString("cloudsdk.process.failed", exitCode)); + status = StatusUtil.error(this, errorMessage); + } else { + status = Status.OK_STATUS; } } + + /** + * @return the status on exit of the process or null if the process has not exited. + */ + public IStatus getExitStatus() { + return status; + } + } + + private class StringBuilderProcessOutputLineListener implements ProcessOutputLineListener { + private final StringBuffer buffer = new StringBuffer(); + + public StringBuilderProcessOutputLineListener() { + } + + @Override + public void onOutputLine(String line) { + buffer.append(line); + } + + @Override + public String toString() { + return buffer.toString(); + } } -} + +} \ No newline at end of file diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/server/LocalAppEngineServerLaunchConfigurationDelegate.java b/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/server/LocalAppEngineServerLaunchConfigurationDelegate.java index e3b995f738..5a88ca5035 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/server/LocalAppEngineServerLaunchConfigurationDelegate.java +++ b/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/server/LocalAppEngineServerLaunchConfigurationDelegate.java @@ -24,6 +24,7 @@ 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.ui.util.MessageConsoleUtilities; +import com.google.cloud.tools.eclipse.ui.util.WorkbenchUtil; import com.google.cloud.tools.eclipse.usagetracker.AnalyticsEvents; import com.google.cloud.tools.eclipse.usagetracker.AnalyticsPingManager; import com.google.common.annotations.VisibleForTesting; @@ -169,33 +170,7 @@ protected void openBrowserPage(final IServer server) { if (pageLocation == null) { return; } - final IWorkbench workbench = PlatformUI.getWorkbench(); - - Job openJob = new UIJob(workbench.getDisplay(), "Launching start page") { //$NON-NLS-1$ - - @Override - public IStatus runInUIThread(IProgressMonitor monitor) { - if (server.getServerState() != IServer.STATE_STARTED) { - return Status.CANCEL_STATUS; - } - try { - URL url = new URL(pageLocation); - IWorkbenchBrowserSupport browserSupport = workbench.getBrowserSupport(); - int style = IWorkbenchBrowserSupport.LOCATION_BAR - | IWorkbenchBrowserSupport.NAVIGATION_BAR | IWorkbenchBrowserSupport.STATUS; - browserSupport.createBrowser(style, server.getId(), server.getName(), server.getName()) - .openURL(url); - } catch (PartInitException ex) { - // Unable to use the normal browser support, so punt to the OS - logger.log(Level.WARNING, "Cannot launch a browser", ex); //$NON-NLS-1$ - Program.launch(pageLocation); - } catch (MalformedURLException ex) { - logger.log(Level.SEVERE, "Invalid dev_appserver URL", ex); //$NON-NLS-1$ - } - return Status.OK_STATUS; - } - }; - openJob.schedule(); + WorkbenchUtil.openInBrowserInUiThread(pageLocation, server.getId(), server.getName(), server.getName()); } private void setupDebugTarget(ILaunch launch, int port, diff --git a/plugins/com.google.cloud.tools.eclipse.preferences/META-INF/MANIFEST.MF b/plugins/com.google.cloud.tools.eclipse.preferences/META-INF/MANIFEST.MF index 6391c9ff43..1c65486179 100644 --- a/plugins/com.google.cloud.tools.eclipse.preferences/META-INF/MANIFEST.MF +++ b/plugins/com.google.cloud.tools.eclipse.preferences/META-INF/MANIFEST.MF @@ -12,4 +12,5 @@ Bundle-ActivationPolicy: lazy Bundle-Localization: plugin Require-Bundle: org.eclipse.ui, org.eclipse.core.runtime -Import-Package: org.eclipse.core.resources +Import-Package: com.google.cloud.tools.eclipse.ui.util, + org.eclipse.core.resources diff --git a/plugins/com.google.cloud.tools.eclipse.preferences/src/com/google/cloud/tools/eclipse/preferences/AnalyticsOptInArea.java b/plugins/com.google.cloud.tools.eclipse.preferences/src/com/google/cloud/tools/eclipse/preferences/AnalyticsOptInArea.java index d18d1475a0..3d5a9d15a2 100644 --- a/plugins/com.google.cloud.tools.eclipse.preferences/src/com/google/cloud/tools/eclipse/preferences/AnalyticsOptInArea.java +++ b/plugins/com.google.cloud.tools.eclipse.preferences/src/com/google/cloud/tools/eclipse/preferences/AnalyticsOptInArea.java @@ -17,7 +17,7 @@ package com.google.cloud.tools.eclipse.preferences; import com.google.cloud.tools.eclipse.preferences.areas.PreferenceArea; - +import com.google.cloud.tools.eclipse.ui.util.WorkbenchUtil; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; import org.eclipse.swt.SWT; @@ -75,15 +75,7 @@ public void widgetDefaultSelected(SelectionEvent event) { @Override public void widgetSelected(SelectionEvent event) { // Open a privacy policy web page when the link is clicked. - try { - URL url = new URL(Messages.getString("GOOGLE_PRIVACY_POLICY_URL")); - IWorkbenchBrowserSupport browserSupport = PlatformUI.getWorkbench().getBrowserSupport(); - browserSupport.createBrowser(null).openURL(url); - } catch (MalformedURLException mue) { - logger.log(Level.WARNING, "URL malformed", mue); - } catch (PartInitException pie) { - logger.log(Level.WARNING, "Cannot launch a browser", pie); - } + WorkbenchUtil.openInBrowser(PlatformUI.getWorkbench(), Messages.getString("GOOGLE_PRIVACY_POLICY_URL")); } }); diff --git a/plugins/com.google.cloud.tools.eclipse.sdk.ui/META-INF/MANIFEST.MF b/plugins/com.google.cloud.tools.eclipse.sdk.ui/META-INF/MANIFEST.MF index 65179bd46c..3002b27360 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk.ui/META-INF/MANIFEST.MF +++ b/plugins/com.google.cloud.tools.eclipse.sdk.ui/META-INF/MANIFEST.MF @@ -14,7 +14,8 @@ Require-Bundle: org.eclipse.ui;bundle-version="3.107.0", org.eclipse.e4.core.contexts;bundle-version="1.4.0", com.google.cloud.tools.eclipse.sdk;bundle-version="0.1.0", com.google.cloud.tools.eclipse.preferences -Import-Package: org.eclipse.osgi.util;version="1.1.0", +Import-Package: com.google.cloud.tools.eclipse.ui.util, + org.eclipse.osgi.util;version="1.1.0", org.eclipse.ui.console, org.osgi.framework;version="1.8.0" Export-Package: com.google.cloud.tools.eclipse.sdk.ui, diff --git a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/MessageConsoleWriterOutputLineListener.java b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/MessageConsoleWriterOutputLineListener.java index 48c708de60..268aa6979a 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/MessageConsoleWriterOutputLineListener.java +++ b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/MessageConsoleWriterOutputLineListener.java @@ -34,4 +34,4 @@ public void onOutputLine(String line) { stream.println(line); } } -} +} \ No newline at end of file diff --git a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferenceArea.java b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferenceArea.java index 208a3f80c4..19637ce02d 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferenceArea.java +++ b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferenceArea.java @@ -21,8 +21,10 @@ import com.google.cloud.tools.appengine.cloudsdk.CloudSdk; import com.google.cloud.tools.appengine.cloudsdk.CloudSdkNotFoundException; import com.google.cloud.tools.appengine.cloudsdk.CloudSdkOutOfDateException; +import com.google.cloud.tools.eclipse.preferences.Messages; import com.google.cloud.tools.eclipse.preferences.areas.PreferenceArea; import com.google.cloud.tools.eclipse.sdk.internal.PreferenceConstants; +import com.google.cloud.tools.eclipse.ui.util.WorkbenchUtil; import java.io.File; import java.net.MalformedURLException; import java.net.URL; @@ -47,6 +49,7 @@ import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Link; import org.eclipse.ui.PartInitException; +import org.eclipse.ui.PlatformUI; import org.eclipse.ui.browser.IWorkbenchBrowserSupport; public class CloudSdkPreferenceArea extends PreferenceArea { @@ -128,19 +131,7 @@ public void setStringValue(String value) { } protected void openUrl(String urlText) { - try { - if (getWorkbench() != null) { - URL url = new URL(urlText); - IWorkbenchBrowserSupport browserSupport = getWorkbench().getBrowserSupport(); - browserSupport.createBrowser(null).openURL(url); - } else { - Program.launch(urlText); - } - } catch (MalformedURLException mue) { - logger.log(Level.WARNING, SdkUiMessages.getString("CloudSdkPreferencePage_3"), mue); - } catch (PartInitException pie) { - logger.log(Level.WARNING, SdkUiMessages.getString("CloudSdkPreferencePage_4"), pie); - } + WorkbenchUtil.openInBrowser(PlatformUI.getWorkbench(), urlText); } private static Path getDefaultSdkLocation() { diff --git a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/messages.properties b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/messages.properties index caf7e04644..3c85cf80ac 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/messages.properties +++ b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/messages.properties @@ -1,6 +1,4 @@ CloudSdkRequired=Google Cloud Tools for Eclipse requires the Google Cloud SDK with the App Engine Java components. -CloudSdkPreferencePage_3=URL malformed -CloudSdkPreferencePage_4=Cannot launch a browser SdkLocation=&SDK location: CloudSdkNotFound=Cloud SDK not found in {0} CloudSdkOutOfDate=Installed Cloud SDK is too old. Run `gcloud components update` and try again. diff --git a/plugins/com.google.cloud.tools.eclipse.ui.util/META-INF/MANIFEST.MF b/plugins/com.google.cloud.tools.eclipse.ui.util/META-INF/MANIFEST.MF index 668c328e91..fa17c2b1be 100644 --- a/plugins/com.google.cloud.tools.eclipse.ui.util/META-INF/MANIFEST.MF +++ b/plugins/com.google.cloud.tools.eclipse.ui.util/META-INF/MANIFEST.MF @@ -21,6 +21,7 @@ Import-Package: com.google.cloud.tools.eclipse.ui.util, org.eclipse.core.expressions, org.eclipse.core.resources, org.eclipse.core.runtime;bundle-symbolic-name:="org.eclipse.core.runtime", + org.eclipse.core.runtime.jobs, org.eclipse.jface.action, org.eclipse.jface.dialogs, org.eclipse.jface.resource, diff --git a/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/WorkbenchUtil.java b/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/WorkbenchUtil.java index a55c69d083..5842c28b88 100644 --- a/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/WorkbenchUtil.java +++ b/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/WorkbenchUtil.java @@ -16,14 +16,28 @@ package com.google.cloud.tools.eclipse.ui.util; +import com.google.cloud.tools.eclipse.util.status.StatusUtil; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.logging.Level; +import java.util.logging.Logger; import org.eclipse.core.resources.IFile; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Status; +import org.eclipse.core.runtime.jobs.Job; +import org.eclipse.swt.program.Program; import org.eclipse.ui.IWorkbench; import org.eclipse.ui.IWorkbenchPage; import org.eclipse.ui.IWorkbenchWindow; import org.eclipse.ui.PartInitException; +import org.eclipse.ui.PlatformUI; +import org.eclipse.ui.browser.IWorkbenchBrowserSupport; import org.eclipse.ui.ide.IDE; +import org.eclipse.ui.progress.UIJob; public class WorkbenchUtil { + private static final Logger logger = Logger.getLogger(WorkbenchUtil.class.getName()); /** * Open the specified file in the editor. @@ -43,4 +57,67 @@ public static void openInEditor(IWorkbench workbench, IFile file) { } } + /** + * Opens the specified url in a Web browser instance in a UI thread. + * + * @param urlPath the URL to display + * @param browserId if an instance of a browser with the same id is already opened, it will be + * returned instead of creating a new one. Passing null will create a new instance with a + * generated id. + * @param name a name displayed on the tab of the internal browser + * @param tooltip the text for a tooltip on the name of the internal browser + */ + public static void openInBrowserInUiThread(final String urlPath, final String browserId, + final String name, final String tooltip) { + final IWorkbench workbench = PlatformUI.getWorkbench(); + Job launchBrowserJob = new UIJob(workbench.getDisplay(), name) { + @Override + public IStatus runInUIThread(IProgressMonitor monitor) { + return openInBrowser(workbench, urlPath, browserId, name, tooltip); + } + + }; + launchBrowserJob.schedule(); + } + + /** + * Opens the specified url in a Web browser instance. + * + * @param workbench the current workbench + * @param urlPath the URL to display + * @param browserId if an instance of a browser with the same id is already opened, it will be + * returned instead of creating a new one. Passing null will create a new instance with a + * generated id. + * @param name a name displayed on the tab of the internal browser + * @param tooltip the text for a tooltip on the name of the internal browser + * @return resulting status of the operation + */ + public static IStatus openInBrowser(IWorkbench workbench, String urlPath, String browserId, + String name, String tooltip) { + try { + URL url = new URL(urlPath); + IWorkbenchBrowserSupport browserSupport = workbench.getBrowserSupport(); + int style = IWorkbenchBrowserSupport.LOCATION_BAR + | IWorkbenchBrowserSupport.NAVIGATION_BAR | IWorkbenchBrowserSupport.STATUS; + browserSupport.createBrowser(style, browserId, name, tooltip).openURL(url); + } catch (PartInitException ex) { + // Unable to use the normal browser support, so push to the OS + logger.log(Level.WARNING, "Cannot launch a browser", ex); + Program.launch(urlPath); + } catch (MalformedURLException ex) { + return StatusUtil.error(WorkbenchUtil.class, Messages.getString("invalid.url"), ex); + } + return Status.OK_STATUS; + } + + /** + * Opens the specified url in a Web browser instance. + * + * @param workbench the current workbench + * @param urlPath the URL to display + * @return resulting status of the operation + */ + public static IStatus openInBrowser(IWorkbench workbench, String urlPath) { + return openInBrowser(workbench, urlPath, null, null, null); + } } diff --git a/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/console/BrowserSupportBasedHyperlink.java b/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/console/BrowserSupportBasedHyperlink.java index a7df5e5069..4c407f0830 100644 --- a/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/console/BrowserSupportBasedHyperlink.java +++ b/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/console/BrowserSupportBasedHyperlink.java @@ -16,12 +16,8 @@ package com.google.cloud.tools.eclipse.ui.util.console; -import java.net.MalformedURLException; -import java.net.URL; -import java.util.logging.Level; +import com.google.cloud.tools.eclipse.ui.util.WorkbenchUtil; import java.util.logging.Logger; -import org.eclipse.swt.program.Program; -import org.eclipse.ui.PartInitException; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.browser.IWorkbenchBrowserSupport; import org.eclipse.ui.console.IHyperlink; @@ -49,23 +45,6 @@ public void linkEntered() { @Override public void linkActivated() { - try { - IWorkbenchBrowserSupport browserSupport = PlatformUI.getWorkbench().getBrowserSupport(); - int style = IWorkbenchBrowserSupport.LOCATION_BAR | IWorkbenchBrowserSupport.NAVIGATION_BAR - | IWorkbenchBrowserSupport.STATUS; - String browserId = null; - String title = null; - String tooltip = null; - browserSupport.createBrowser(style, browserId, title, tooltip).openURL(new URL(url)); - - } catch (PartInitException partInitException) { - logger.log(Level.SEVERE, "Cannot open hyperlink using browser support, will try SWT's Program.launch(String)", - partInitException); - if (!Program.launch(url)) { - logger.log(Level.SEVERE, "Cannot open hyperlink using SWT's Program.launch(String)"); - } - } catch (MalformedURLException malformedURLException) { - logger.log(Level.SEVERE, "Cannot open hyperlink", malformedURLException); - } + WorkbenchUtil.openInBrowser(PlatformUI.getWorkbench(), url); } } diff --git a/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/messages.properties b/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/messages.properties index 211b939c5f..ffd64bca8d 100644 --- a/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/messages.properties +++ b/plugins/com.google.cloud.tools.eclipse.ui.util/src/com/google/cloud/tools/eclipse/ui/util/messages.properties @@ -2,4 +2,4 @@ bucket.name.invalid=Invalid bucket name project.id.invalid=Invalid project ID project.id.empty=Empty project ID version.invalid=Version must contain only lower-case letters, numbers, and hyphens -version.reserved=Version uses reserved form +version.reserved=Version uses reserved form \ No newline at end of file