diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.facets/src/com/google/cloud/tools/eclipse/appengine/facets/AppEngineSdkClasspathContainer.java b/plugins/com.google.cloud.tools.eclipse.appengine.facets/src/com/google/cloud/tools/eclipse/appengine/facets/AppEngineSdkClasspathContainer.java index 0e4283fefd..92cd881540 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.facets/src/com/google/cloud/tools/eclipse/appengine/facets/AppEngineSdkClasspathContainer.java +++ b/plugins/com.google.cloud.tools.eclipse.appengine.facets/src/com/google/cloud/tools/eclipse/appengine/facets/AppEngineSdkClasspathContainer.java @@ -8,17 +8,14 @@ import org.eclipse.jdt.core.IClasspathEntry; import org.eclipse.jdt.core.JavaCore; +import com.google.cloud.tools.appengine.cloudsdk.CloudSdk; import com.google.cloud.tools.eclipse.sdk.CloudSdkProvider; public final class AppEngineSdkClasspathContainer implements IClasspathContainer { - // TODO should be changed once app-tools-lib can provide the directory - // https://github.com/GoogleCloudPlatform/app-tools-lib-for-java/issues/134 - private static final String SDK_JAR = "platform/google_appengine/google/appengine/tools/java/lib/appengine-tools-api.jar"; + private static final CloudSdk CLOUD_SDK = new CloudSdkProvider().getCloudSdk(); public static final String CONTAINER_ID = "AppEngineSDK"; - private File cloudSdkLocation = new CloudSdkProvider().getCloudSdkLocation(); - @Override public IPath getPath() { return new Path(AppEngineSdkClasspathContainer.CONTAINER_ID); @@ -36,8 +33,8 @@ public String getDescription() { @Override public IClasspathEntry[] getClasspathEntries() { - if (cloudSdkLocation != null) { - File jarFile = cloudSdkLocation.toPath().resolve(SDK_JAR).toFile(); + if (CLOUD_SDK.getJavaAppEngineSdkPath() != null) { + File jarFile = CLOUD_SDK.getJavaToolsJar().toFile(); if (jarFile.exists()) { String appEngineToolsApiJar = jarFile.getPath(); IClasspathEntry appEngineToolsEntry = diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ServletClasspathProvider.java b/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ServletClasspathProvider.java index 55374f6916..a732eae0e4 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ServletClasspathProvider.java +++ b/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ServletClasspathProvider.java @@ -30,7 +30,7 @@ public IClasspathEntry[] resolveClasspathContainer(IProject project, IRuntime ru @Override public IClasspathEntry[] resolveClasspathContainer(IRuntime runtime) { - java.nio.file.Path cloudSdkPath = new CloudSdkProvider(null).getCloudSdkLocation().toPath(); + java.nio.file.Path cloudSdkPath = new CloudSdkProvider().getCloudSdk().getJavaAppEngineSdkPath(); if (cloudSdkPath == null) { return new IClasspathEntry[0]; }; diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ui/CloudSdkRuntimeWizardFragment.java b/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ui/CloudSdkRuntimeWizardFragment.java index 8f219ebe2f..502d347aba 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ui/CloudSdkRuntimeWizardFragment.java +++ b/plugins/com.google.cloud.tools.eclipse.appengine.localserver/src/com/google/cloud/tools/eclipse/appengine/localserver/ui/CloudSdkRuntimeWizardFragment.java @@ -146,7 +146,7 @@ public void widgetSelected(SelectionEvent event) { } }); - File location = new CloudSdkProvider(null).getCloudSdkLocation(); + File location = new CloudSdkProvider().getCloudSdk().getJavaAppEngineSdkPath().toFile(); if (location != null) { dirTextBox.setText(location.toString()); } diff --git a/plugins/com.google.cloud.tools.eclipse.appengine.newproject/src/com/google/cloud/tools/eclipse/appengine/newproject/AppEngineFacet.java b/plugins/com.google.cloud.tools.eclipse.appengine.newproject/src/com/google/cloud/tools/eclipse/appengine/newproject/AppEngineFacet.java index e5ff870769..071f434497 100644 --- a/plugins/com.google.cloud.tools.eclipse.appengine.newproject/src/com/google/cloud/tools/eclipse/appengine/newproject/AppEngineFacet.java +++ b/plugins/com.google.cloud.tools.eclipse.appengine.newproject/src/com/google/cloud/tools/eclipse/appengine/newproject/AppEngineFacet.java @@ -49,7 +49,7 @@ public static void installAppEngineRuntime(IFacetedProject project, IProgressMon IRuntimeWorkingCopy appEngineRuntimeWorkingCopy = appEngineRuntimeType.createRuntime(null, monitor); - File sdkLocation = new CloudSdkProvider(null).getCloudSdkLocation(); + File sdkLocation = new CloudSdkProvider().getCloudSdk().getJavaAppEngineSdkPath().toFile(); if (sdkLocation != null) { IPath sdkPath = Path.fromOSString(sdkLocation.getAbsolutePath()); appEngineRuntimeWorkingCopy.setLocation(sdkPath); diff --git a/plugins/com.google.cloud.tools.eclipse.sdk.test/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkProviderTest.java b/plugins/com.google.cloud.tools.eclipse.sdk.test/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkProviderTest.java index e4ba498731..d88fd38455 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk.test/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkProviderTest.java +++ b/plugins/com.google.cloud.tools.eclipse.sdk.test/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkProviderTest.java @@ -32,12 +32,10 @@ import java.nio.file.Path; public class CloudSdkProviderTest { - - private IPreferenceStore preferences; @Before public void setUp() { - preferences = new MockPreferences(); + //preferences = new MockPreferences(); } /** Verify that the preference overrides PathResolver. */ @@ -46,10 +44,9 @@ public void testSetPreferenceInvalid() throws Exception { // A path that almost certainly does not contain the SDK File root = File.listRoots()[0]; - CloudSdk.Builder builder = new CloudSdkProvider(preferences).createBuilder(null); + CloudSdk instance = new CloudSdkProvider().getCloudSdk(); // todo we shouldn't need reflection here; use visible for testing if we must - assertEquals(root.toPath(), ReflectionUtil.getField(builder, "sdkPath", Path.class)); - CloudSdk instance = builder.build(); + assertEquals(root.toPath(), ReflectionUtil.getField(instance, "sdkPath", Path.class)); assertEquals(root.toPath(), ReflectionUtil.invoke(instance, "getSdkPath", Path.class)); try { instance.validate(); diff --git a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferencePage.java b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferencePage.java index 886dfac81d..90ff124c5b 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferencePage.java +++ b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPreferencePage.java @@ -39,6 +39,8 @@ import java.io.File; import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.Path; +import java.nio.file.Paths; import java.text.MessageFormat; import java.util.logging.Level; import java.util.logging.Logger; @@ -101,7 +103,7 @@ public void createFieldEditors() { addField(sdkLocation); } - protected boolean validateSdk(File location) { + protected boolean validateSdk(Path location) { try { CloudSdk sdk = new CloudSdk.Builder().sdkPath(location).build(); sdk.validate(); @@ -141,7 +143,7 @@ protected boolean doCheckState() { if (!super.doCheckState()) { return false; } - return getStringValue().isEmpty() || validateSdk(new File(getStringValue())); + return getStringValue().isEmpty() || validateSdk(Paths.get(getStringValue())); } } } diff --git a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPrompter.java b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPrompter.java index f1846ee5dc..76ca9ab95e 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPrompter.java +++ b/plugins/com.google.cloud.tools.eclipse.sdk.ui/src/com/google/cloud/tools/eclipse/sdk/ui/preferences/CloudSdkPrompter.java @@ -33,6 +33,8 @@ * found. Must be called from the SWT UI Thread. */ public class CloudSdkPrompter { + + private static CloudSdk CLOUD_SDK = new CloudSdkProvider().getCloudSdk(); /** * Return the Cloud SDK. If it cannot be found, prompt the user to specify its location. @@ -59,13 +61,12 @@ public static CloudSdk getCloudSdk(Shell shell) { * @return the Cloud SDK, or {@code null} if unspecified */ public static CloudSdk getCloudSdk(IShellProvider shellProvider) { - CloudSdkProvider cloudSdkProvider = new CloudSdkProvider(null); - CloudSdk sdk = cloudSdkProvider.getCloudSdk(); - if (sdk != null) { - return sdk; + // Not sure I understand what's going on here... + if (CLOUD_SDK != null) { + return CLOUD_SDK; } if (promptForSdk(shellProvider)) { - return cloudSdkProvider.getCloudSdk(); + return CLOUD_SDK; } return null; } @@ -95,13 +96,13 @@ public static File getCloudSdkLocation(Shell shell) { * @return the Cloud SDK location, or {@code null} if unspecified */ public static File getCloudSdkLocation(IShellProvider shellProvider) { - CloudSdkProvider cloudSdkProvider = new CloudSdkProvider(null); - File location = cloudSdkProvider.getCloudSdkLocation(); + CloudSdk cloudSdk = new CloudSdkProvider().getCloudSdk(); + File location = cloudSdk.getJavaAppEngineSdkPath().toFile(); if (location != null) { return location; } if (promptForSdk(shellProvider)) { - return cloudSdkProvider.getCloudSdkLocation(); + return cloudSdk.getJavaAppEngineSdkPath().toFile(); } return null; } diff --git a/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/CloudSdkProvider.java b/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/CloudSdkProvider.java index b7c26c1096..b2b4c48065 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/CloudSdkProvider.java +++ b/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/CloudSdkProvider.java @@ -26,6 +26,7 @@ import java.io.File; import java.nio.file.Path; +import java.nio.file.Paths; /** * Utility to find the Google Cloud SDK either at locations configured by the user or in standard @@ -33,19 +34,7 @@ */ public class CloudSdkProvider extends ContextFunction { - private final IPreferenceStore preferences; - - public CloudSdkProvider(IPreferenceStore preferences) { - // todo drop the null check and use the no args constructor instead - if (preferences == null) { - preferences = PreferenceInitializer.getPreferenceStore(); - } - this.preferences = preferences; - } - - public CloudSdkProvider() { - this(PreferenceInitializer.getPreferenceStore()); - } + private final IPreferenceStore preferences = PreferenceInitializer.getPreferenceStore(); /** * Return the {@link CloudSdk} instance from the configured or discovered Cloud SDK. @@ -53,53 +42,17 @@ public CloudSdkProvider() { * @return the configured {@link CloudSdk} or {@code null} if no SDK could be found */ public CloudSdk getCloudSdk() { - return createBuilder(null).build(); - } - - /** - * Return the location of the configured or discovered Cloud SDK. - * - * @return the configured location or {@code null} if the SDK could not be found - */ - public File getCloudSdkLocation() { - return resolveSdkLocation(); - } - - /** - * Return a {@link CloudSdk.Builder} instance, suitable for creating a {@link CloudSdk} instance. - * {@code location}, if not null, is used as the location to the Cloud SDK, otherwise the - * configured or discovered Cloud SDK will be used instead. - * - * @param location if not {@code null}, overrides the default location for the SDK; can be a - * {@linkplain String}, {@linkplain File}, or {@linkplain Path}. - * @return a builder, or {@code null} if the Google Cloud SDK cannot be located - */ - public CloudSdk.Builder createBuilder(File location) { - // perhaps should try to be cleverer in case location references the .../bin/gcloud - if (location == null || !location.exists()) { - location = resolveSdkLocation(); - } - if (location == null || !location.exists()) { - return null; - } - return new CloudSdk.Builder().sdkPath(location); - } - - /** - * Attempt to resolve the Google Cloud SDK from the configured location or try to discover its - * location. - * - * @return the location, or {@code null} if not found - */ - private File resolveSdkLocation() { - String value = preferences.getString(PreferenceConstants.CLOUDSDK_PATH); - if (value != null && !value.isEmpty()) { - return new File(value); - } - Path discovered = PathResolver.INSTANCE.getCloudSdkPath(); - if (discovered != null) { - return discovered.toFile(); + CloudSdk.Builder sdkBuilder = new CloudSdk.Builder(); + + String configuredPath = preferences.getString(PreferenceConstants.CLOUDSDK_PATH); + + // Let's use the configured path, if there is one. Otherwise, the lib will try to discover the path. + if (configuredPath != null && !configuredPath.isEmpty()) { + // TODO(joaomartins): What happens when the provided path is invalid? Tools lib isn't + // calling validate(). + sdkBuilder.sdkPath(Paths.get(configuredPath)); } - return null; + + return sdkBuilder.build(); } } diff --git a/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkContextFunction.java b/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkContextFunction.java index 964b43c9fe..facf9322e7 100644 --- a/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkContextFunction.java +++ b/plugins/com.google.cloud.tools.eclipse.sdk/src/com/google/cloud/tools/eclipse/sdk/internal/CloudSdkContextFunction.java @@ -61,11 +61,9 @@ public Object compute(IEclipseContext context, String contextKey) { Object path = context.get(PreferenceConstants.CLOUDSDK_PATH); File location = toFile(path); - CloudSdk.Builder builder = new CloudSdkProvider(null).createBuilder(location); - if (builder == null) { - return NOT_A_VALUE; - } - CloudSdk instance = builder.build(); + // TODO(joaomartins): When can the SDK location in the context be different from the one in the + // preference store? + CloudSdk instance = new CloudSdkProvider().getCloudSdk(); try { instance.validate(); return instance;