From d31ac324985629ef6e406fb8d5f96536382c51dd Mon Sep 17 00:00:00 2001 From: nsingh Date: Mon, 30 Jan 2017 15:18:09 -0800 Subject: [PATCH 1/2] Changes to the CF target and client caching Cache client and target based on a CF client params key that does not include the credentials. Also added logic to refresh a target if error encountered during last call on target. --- .../client/cftarget/CFTarget.java | 53 +++-- .../client/cftarget/CFTargetCache.java | 16 +- .../client/cftarget/ClientParamsCacheKey.java | 81 ++++++++ .../client/v2/CFClientProvider.java | 143 ++++++++++++++ .../client/v2/CloudFoundryClientCache.java | 183 ++++-------------- .../client/v2/DefaultClientRequestsV2.java | 13 +- 6 files changed, 321 insertions(+), 168 deletions(-) create mode 100644 vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/ClientParamsCacheKey.java create mode 100644 vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CFClientProvider.java diff --git a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTarget.java b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTarget.java index 6d35acd19..e87be091f 100644 --- a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTarget.java +++ b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTarget.java @@ -11,7 +11,7 @@ package org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget; import java.util.List; -import java.util.concurrent.ExecutionException; +import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; import org.springframework.ide.vscode.commons.cloudfoundry.client.CFBuildpack; @@ -39,19 +39,20 @@ public class CFTarget { */ private final LoadingCache> buildpacksCache; private final LoadingCache> servicesCache; + private Throwable lastCFFailure; public CFTarget(String targetName, CFClientParams params, ClientRequests requests) { this.params = params; this.requests = requests; this.targetName = targetName; CacheLoader> servicesLoader = new CacheLoader>() { - + @Override public List load(String key) throws Exception { - /* - * Ignore the key. Not used. + /* Cache of services does not use keys, as the whole cache + * gets wiped clean on any new call to CF. */ - return requests.getServices(); + return runAndCheckForFailure(() -> requests.getServices()); } }; this.servicesCache = CacheBuilder.newBuilder() @@ -61,26 +62,57 @@ public class CFTarget { @Override public List load(String key) throws Exception { - /* - * Ignore the key. Not used. + /* Cache does not use keys, as the whole cache + * gets wiped clean on any new call to CF. */ - return requests.getBuildpacks(); + return runAndCheckForFailure(() -> requests.getBuildpacks()); } }; this.buildpacksCache = CacheBuilder.newBuilder() .expireAfterAccess(CFTargetCache.TARGET_EXPIRATION, TimeUnit.HOURS).build(buildpacksLoader); } + + protected T runAndCheckForFailure(Callable callable) throws Exception { + this.lastCFFailure = null; + try { + return callable.call(); + } catch (Exception e) { + if (isAcceptableCFFailure(e)) { + this.lastCFFailure = e; + } + throw e; + } + } + + public boolean hasCFFailure() { + return this.lastCFFailure != null; + } + + protected boolean isAcceptableCFFailure(Throwable e) { + // TODO: this is too broad. Be more specific: e.g. look for IOException, etc.. + return e != null; + } public CFClientParams getParams() { return params; } public List getBuildpacks() throws Exception { - return this.buildpacksCache.get(getName()); + // Use the target name as the "key" , since Guava cache doesn't allow null keys + // However, the key is not really used when fetching buildpacks, as we are not caching + // buildpacks per target here. This class only represents ONE target, so it will only + // ever have one key + String key = getName(); + return this.buildpacksCache.get(key); } public List getServices() throws Exception { - return this.servicesCache.get(getName()); + /* services don't use keys, as they get wiped clean on each refresh + * . That said, the cache doesn't allow a null key, so use the target name as the "key" + * + */ + String key = getName(); + return this.servicesCache.get(key); } public ClientRequests getClientRequests() { @@ -95,5 +127,4 @@ public class CFTarget { public String toString() { return "CFClientTarget [params=" + params + ", targetName=" + targetName + "]"; } - } diff --git a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargetCache.java b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargetCache.java index 3bbe48630..d50434b5a 100644 --- a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargetCache.java +++ b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargetCache.java @@ -27,7 +27,7 @@ public class CFTargetCache { private final ClientParamsProvider paramsProvider; private final CloudFoundryClientFactory clientFactory; private final ClientTimeouts timeouts; - private final LoadingCache cache; + private final LoadingCache cache; public static final long SERVICES_EXPIRATION = 10; public static final long TARGET_EXPIRATION = 1; @@ -39,11 +39,11 @@ public class CFTargetCache { this.paramsProvider = paramsProvider; this.clientFactory = clientFactory; this.timeouts = timeouts; - CacheLoader loader = new CacheLoader() { + CacheLoader loader = new CacheLoader() { @Override - public CFTarget load(CFClientParams params) throws Exception { - return create(params); + public CFTarget load(ClientParamsCacheKey params) throws Exception { + return create(params.fullParams); } }; @@ -63,8 +63,14 @@ public class CFTargetCache { List targets = new ArrayList<>(); if (allParams != null) { for (CFClientParams params : allParams) { - CFTarget target = cache.get(params); + ClientParamsCacheKey key = ClientParamsCacheKey.from(params); + CFTarget target = cache.get(key); if (target != null) { + // If any CF errors occurred in the target, refresh once + if (target.hasCFFailure()) { + cache.refresh(key); + target = cache.get(key); + } targets.add(target); } } diff --git a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/ClientParamsCacheKey.java b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/ClientParamsCacheKey.java new file mode 100644 index 000000000..8dcec91f3 --- /dev/null +++ b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/ClientParamsCacheKey.java @@ -0,0 +1,81 @@ +/******************************************************************************* + * Copyright (c) 2017 Pivotal, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget; + +/** + * + * A key for CF targets that uses {@link CFClientParams} values, EXCEPT for + * credentials, as the key value. + * + */ +public class ClientParamsCacheKey { + + public final CFClientParams fullParams; + + /** + * Use static API to create: {@link #from(CFClientParams)} + * @param fullParams + */ + private ClientParamsCacheKey(CFClientParams fullParams) { + this.fullParams = fullParams; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((fullParams.getApiUrl() == null) ? 0 : fullParams.getApiUrl().hashCode()); + result = prime * result + ((fullParams.getOrgName() == null) ? 0 : fullParams.getOrgName().hashCode()); + result = prime * result + (fullParams.skipSslValidation() ? 1231 : 1237); + result = prime * result + ((fullParams.getSpaceName() == null) ? 0 : fullParams.getSpaceName().hashCode()); + result = prime * result + ((fullParams.getUsername() == null) ? 0 : fullParams.getUsername().hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + ClientParamsCacheKey other = (ClientParamsCacheKey) obj; + if (fullParams.getApiUrl() == null) { + if (other.fullParams.getApiUrl() != null) + return false; + } else if (!fullParams.getApiUrl().equals(other.fullParams.getApiUrl())) + return false; + if (fullParams.getOrgName() == null) { + if (other.fullParams.getOrgName() != null) + return false; + } else if (!fullParams.getOrgName().equals(other.fullParams.getOrgName())) + return false; + if (fullParams.skipSslValidation() != other.fullParams.skipSslValidation()) + return false; + if (fullParams.getSpaceName() == null) { + if (other.fullParams.getSpaceName() != null) + return false; + } else if (!fullParams.getSpaceName().equals(other.fullParams.getSpaceName())) + return false; + if (fullParams.getUsername() == null) { + if (other.fullParams.getUsername() != null) + return false; + } else if (!fullParams.getUsername().equals(other.fullParams.getUsername())) + return false; + return true; + } + + public static ClientParamsCacheKey from(CFClientParams params) { + return new ClientParamsCacheKey(params); + } + +} diff --git a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CFClientProvider.java b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CFClientProvider.java new file mode 100644 index 000000000..4443045d9 --- /dev/null +++ b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CFClientProvider.java @@ -0,0 +1,143 @@ +/******************************************************************************* + * Copyright (c) 2017 Pivotal, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.commons.cloudfoundry.client.v2; + +import java.time.Duration; +import java.util.Optional; + +import org.cloudfoundry.client.CloudFoundryClient; +import org.cloudfoundry.reactor.ConnectionContext; +import org.cloudfoundry.reactor.DefaultConnectionContext; +import org.cloudfoundry.reactor.ProxyConfiguration; +import org.cloudfoundry.reactor.TokenProvider; +import org.cloudfoundry.reactor.client.ReactorCloudFoundryClient; +import org.cloudfoundry.reactor.doppler.ReactorDopplerClient; +import org.cloudfoundry.reactor.tokenprovider.OneTimePasscodeTokenProvider; +import org.cloudfoundry.reactor.tokenprovider.PasswordGrantTokenProvider; +import org.cloudfoundry.reactor.tokenprovider.RefreshTokenGrantTokenProvider; +import org.cloudfoundry.reactor.uaa.ReactorUaaClient; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFClientParams; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFCredentials; + +public class CFClientProvider { + + final ConnectionContext connection; + final TokenProvider tokenProvider; + + //Note the three client objects below are 'stateless' wrappers and it would be + // fine to recreate as needed instead of store them + + final CloudFoundryClient client; + final ReactorUaaClient uaaClient; + final ReactorDopplerClient doppler; + + private ProxyConfiguration getProxy(String host) { + // TODO: enable proxy support for vscode. The code below retrieves proxy via Eclipse proxy service thus + // not applicable when this is used outside of Eclipse +// try { +// if (StringUtils.hasText(host)) { +// URL url = new URL("https://"+host); +// // In certain cases, the activator would have stopped and the plugin may +// // no longer be available. Usually onl happens on shutdown. +// BootDashActivator plugin = BootDashActivator.getDefault(); +// if (plugin != null) { +// IProxyService proxyService = plugin.getProxyService(); +// if (proxyService != null) { +// IProxyData[] selectedProxies = proxyService.select(url.toURI()); +// +// // No proxy configured or not found +// if (selectedProxies == null || selectedProxies.length == 0) { +// return null; +// } +// +// IProxyData data = selectedProxies[0]; +// int proxyPort = data.getPort(); +// String proxyHost = data.getHost(); +// String user = data.getUserId(); +// String password = data.getPassword(); +// if (proxyHost!=null) { +// return ProxyConfiguration.builder() +// .host(proxyHost) +// .port(proxyPort==-1?Optional.empty():Optional.of(proxyPort)) +// .username(Optional.ofNullable(user)) +// .password(Optional.ofNullable(password)) +// .build(); +//// return proxyHost != null ? new HttpProxyConfiguration(proxyHost, proxyPort, +//// data.isRequiresAuthentication(), user, password) : null; +// } +// } +// } +// } +// } catch (Exception e) { +// Log.log(e); +// } + return null; + } + + public CFClientProvider(CFClientParams params) { + long sslTimeout = Long.getLong("sts.bootdash.cf.client.ssl.handshake.timeout", 60); //TODO: make a preference for this? + Optional keepAlive = getBooleanSystemProp("http.keepAlive"); + CloudFoundryClientCache.debug("cf client keepAlive = "+keepAlive); + connection = DefaultConnectionContext.builder() + .proxyConfiguration(Optional.ofNullable(getProxy(params.getHost()))) + .apiHost(params.getHost()) + .sslHandshakeTimeout(Duration.ofSeconds(sslTimeout)) + .keepAlive(keepAlive) + .skipSslValidation(params.skipSslValidation()) + .build(); + + tokenProvider = createTokenProvider(params); + + client = ReactorCloudFoundryClient.builder() + .connectionContext(connection) + .tokenProvider(tokenProvider) + .build(); + + uaaClient = ReactorUaaClient.builder() + .connectionContext(connection) + .tokenProvider(tokenProvider) + .build(); + + doppler = ReactorDopplerClient.builder() + .connectionContext(connection) + .tokenProvider(tokenProvider) + .build(); + } + + private TokenProvider createTokenProvider(CFClientParams params) { + CFCredentials creds = params.getCredentials(); + switch (creds.getType()) { + case PASSWORD: + return PasswordGrantTokenProvider.builder() + .username(params.getUsername()) + .password(creds.getSecret()) + .build(); + case REFRESH_TOKEN: + return RefreshTokenGrantTokenProvider.builder() + .token(creds.getSecret()) + .build(); + case TEMPORARY_CODE: + return OneTimePasscodeTokenProvider.builder() + .passcode(creds.getSecret()) + .build(); + default: + throw new IllegalStateException("BUG! Missing switch case?"); + } + } + + private Optional getBooleanSystemProp(String name) { + String str = System.getProperty(name); + if (str!=null) { + return Optional.of(Boolean.valueOf(str)); + } + return Optional.empty(); + } + } \ No newline at end of file diff --git a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CloudFoundryClientCache.java b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CloudFoundryClientCache.java index 67fb5b131..76825745f 100644 --- a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CloudFoundryClientCache.java +++ b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/CloudFoundryClientCache.java @@ -10,183 +10,66 @@ *******************************************************************************/ package org.springframework.ide.vscode.commons.cloudfoundry.client.v2; -import java.time.Duration; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; +import java.util.concurrent.TimeUnit; -import org.cloudfoundry.client.CloudFoundryClient; -import org.cloudfoundry.reactor.ConnectionContext; -import org.cloudfoundry.reactor.DefaultConnectionContext; -import org.cloudfoundry.reactor.ProxyConfiguration; -import org.cloudfoundry.reactor.TokenProvider; -import org.cloudfoundry.reactor.client.ReactorCloudFoundryClient; -import org.cloudfoundry.reactor.doppler.ReactorDopplerClient; -import org.cloudfoundry.reactor.tokenprovider.OneTimePasscodeTokenProvider; -import org.cloudfoundry.reactor.tokenprovider.PasswordGrantTokenProvider; -import org.cloudfoundry.reactor.tokenprovider.RefreshTokenGrantTokenProvider; -import org.cloudfoundry.reactor.uaa.ReactorUaaClient; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFClientParams; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFCredentials; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.ClientParamsCacheKey; + +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; /** * TODO: Remove this class when the 'thread leak bug' in V2 client is fixed. * - * At the moment each time {@link SpringCloudFoundryClient} is create a threadpool - * is created by the client and it is never cleaned up. The only way we have - * to mitigate this leak is to try and create as few clients as possible. + * At the moment each time {@link SpringCloudFoundryClient} is create a + * threadpool is created by the client and it is never cleaned up. The only way + * we have to mitigate this leak is to try and create as few clients as + * possible. *

* So we have a permanent cache of clients here that is reused. *

- * When the bug is fixed then this should no longer be necessary and we can removed this cache - * and just create the client as needed. + * When the bug is fixed then this should no longer be necessary and we can + * removed this cache and just create the client as needed. * * @author Kris De Volder */ public class CloudFoundryClientCache { - public class CFClientProvider { - - final ConnectionContext connection; - final TokenProvider tokenProvider; - - //Note the three client objects below are 'stateless' wrappers and it would be - // fine to recreate as needed instead of store them - - final CloudFoundryClient client; - final ReactorUaaClient uaaClient; - final ReactorDopplerClient doppler; - - private ProxyConfiguration getProxy(String host) { - // TODO: enable proxy support for vscode. The code below retrieves proxy via Eclipse proxy service thus - // not applicable when this is used outside of Eclipse -// try { -// if (StringUtils.hasText(host)) { -// URL url = new URL("https://"+host); -// // In certain cases, the activator would have stopped and the plugin may -// // no longer be available. Usually onl happens on shutdown. -// BootDashActivator plugin = BootDashActivator.getDefault(); -// if (plugin != null) { -// IProxyService proxyService = plugin.getProxyService(); -// if (proxyService != null) { -// IProxyData[] selectedProxies = proxyService.select(url.toURI()); -// -// // No proxy configured or not found -// if (selectedProxies == null || selectedProxies.length == 0) { -// return null; -// } -// -// IProxyData data = selectedProxies[0]; -// int proxyPort = data.getPort(); -// String proxyHost = data.getHost(); -// String user = data.getUserId(); -// String password = data.getPassword(); -// if (proxyHost!=null) { -// return ProxyConfiguration.builder() -// .host(proxyHost) -// .port(proxyPort==-1?Optional.empty():Optional.of(proxyPort)) -// .username(Optional.ofNullable(user)) -// .password(Optional.ofNullable(password)) -// .build(); -//// return proxyHost != null ? new HttpProxyConfiguration(proxyHost, proxyPort, -//// data.isRequiresAuthentication(), user, password) : null; -// } -// } -// } -// } -// } catch (Exception e) { -// Log.log(e); -// } - return null; - } - - public CFClientProvider(CFClientParams params) { - long sslTimeout = Long.getLong("sts.bootdash.cf.client.ssl.handshake.timeout", 60); //TODO: make a preference for this? - Optional keepAlive = getBooleanSystemProp("http.keepAlive"); - debug("cf client keepAlive = "+keepAlive); - connection = DefaultConnectionContext.builder() - .proxyConfiguration(Optional.ofNullable(getProxy(params.getHost()))) - .apiHost(params.getHost()) - .sslHandshakeTimeout(Duration.ofSeconds(sslTimeout)) - .keepAlive(keepAlive) - .skipSslValidation(params.skipSslValidation()) - .build(); - - tokenProvider = createTokenProvider(params); - - client = ReactorCloudFoundryClient.builder() - .connectionContext(connection) - .tokenProvider(tokenProvider) - .build(); - - uaaClient = ReactorUaaClient.builder() - .connectionContext(connection) - .tokenProvider(tokenProvider) - .build(); - - doppler = ReactorDopplerClient.builder() - .connectionContext(connection) - .tokenProvider(tokenProvider) - .build(); - } - - private TokenProvider createTokenProvider(CFClientParams params) { - CFCredentials creds = params.getCredentials(); - switch (creds.getType()) { - case PASSWORD: - return PasswordGrantTokenProvider.builder() - .username(params.getUsername()) - .password(creds.getSecret()) - .build(); - case REFRESH_TOKEN: - return RefreshTokenGrantTokenProvider.builder() - .token(creds.getSecret()) - .build(); - case TEMPORARY_CODE: - return OneTimePasscodeTokenProvider.builder() - .passcode(creds.getSecret()) - .build(); - default: - throw new IllegalStateException("BUG! Missing switch case?"); - } - } - - private Optional getBooleanSystemProp(String name) { - String str = System.getProperty(name); - if (str!=null) { - return Optional.of(Boolean.valueOf(str)); - } - return Optional.empty(); - } - } - private static final boolean DEBUG = false; + public static final long EXPIRATION = 1; - private static void debug(String string) { + static void debug(String string) { if (DEBUG) { System.out.println(string); } } - - private Map cache = new HashMap<>(); + private final LoadingCache cache; private int clientCount = 0; - public synchronized CFClientProvider getOrCreate(CFClientParams params) { - CFClientProvider client = cache.get(params); - if (client==null) { - clientCount++; - debug("Creating client ["+clientCount+"]: "+params); - cache.put(params, client = create(params)); - } else { - debug("Reusing client ["+clientCount+"]: "+params); - } - return client; + public CloudFoundryClientCache() { + CacheLoader loader = new CacheLoader() { + + @Override + public CFClientProvider load(ClientParamsCacheKey params) throws Exception { + clientCount++; + debug("Creating client [" + clientCount + "]: " + params); + return create(params.fullParams); + } + + }; + cache = CacheBuilder.newBuilder().initialCapacity(1).expireAfterAccess(EXPIRATION, TimeUnit.HOURS) + .build(loader); + + } + + public synchronized CFClientProvider getOrCreate(CFClientParams params) throws Exception { + return cache.get(ClientParamsCacheKey.from(params)); } protected CFClientProvider create(CFClientParams params) { return new CFClientProvider(params); } - } diff --git a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/DefaultClientRequestsV2.java b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/DefaultClientRequestsV2.java index 8fef04792..b541f3c8d 100644 --- a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/DefaultClientRequestsV2.java +++ b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/v2/DefaultClientRequestsV2.java @@ -22,7 +22,6 @@ import org.springframework.ide.vscode.commons.cloudfoundry.client.CFServiceInsta import org.springframework.ide.vscode.commons.cloudfoundry.client.ClientRequests; import org.springframework.ide.vscode.commons.cloudfoundry.client.ClientTimeouts; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFClientParams; -import org.springframework.ide.vscode.commons.cloudfoundry.client.v2.CloudFoundryClientCache.CFClientProvider; import org.springframework.ide.vscode.commons.util.ExceptionUtil; import com.google.common.collect.ImmutableList; @@ -45,7 +44,7 @@ public class DefaultClientRequestsV2 implements ClientRequests { private final ClientTimeouts timeouts; public DefaultClientRequestsV2(CloudFoundryClientCache clients, CFClientParams params, ClientTimeouts timeouts) { - CFClientProvider provider = clients.getOrCreate(params); + CFClientProvider provider = getFromCache(clients, params); this._client = provider.client; this._operations = DefaultCloudFoundryOperations.builder() @@ -61,6 +60,16 @@ public class DefaultClientRequestsV2 implements ClientRequests { this.timeouts = timeouts != null ? timeouts : ClientTimeouts.DEFAULT_TIMEOUTS; } + private CFClientProvider getFromCache(CloudFoundryClientCache clients, CFClientParams params) { + CFClientProvider provider = null; + try { + provider = clients.getOrCreate(params); + } catch (Exception e) { + throw new IllegalArgumentException("Failed to create a v2 CF Java client using params: " + params, e); + } + return provider; + } + @Override public List getServices() throws Exception { return ReactorUtils.get(timeouts.getServicesTimeout(), CancelationTokens.NULL, From c69801e188a1702436ba3aeb8e246ceb53f9d2eb Mon Sep 17 00:00:00 2001 From: nsingh Date: Mon, 30 Jan 2017 15:47:31 -0800 Subject: [PATCH 2/2] Removed alternate CF mock tests Now that mockito tests run, the alternate way of mocking CF is no longer needed. Copied over the extra tests from the alternate implementation into the editor tests --- .../manifest/yaml/BasicCfClientHarness.java | 166 ------------------ .../yaml/ManifestYamlEditorCFBasicTest.java | 128 -------------- .../manifest/yaml/ManifestYamlEditorTest.java | 68 ++++++- 3 files changed, 59 insertions(+), 303 deletions(-) delete mode 100644 vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/BasicCfClientHarness.java delete mode 100644 vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorCFBasicTest.java diff --git a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/BasicCfClientHarness.java b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/BasicCfClientHarness.java deleted file mode 100644 index 8eb1301cc..000000000 --- a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/BasicCfClientHarness.java +++ /dev/null @@ -1,166 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2017 Pivotal, Inc. - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the Eclipse Public License v1.0 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * Pivotal, Inc. - initial API and implementation - *******************************************************************************/ -package org.springframework.ide.vscode.manifest.yaml; - -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.ExecutionException; - -import org.springframework.ide.vscode.commons.cloudfoundry.client.CFBuildpack; -import org.springframework.ide.vscode.commons.cloudfoundry.client.CFEntities; -import org.springframework.ide.vscode.commons.cloudfoundry.client.CFServiceInstance; -import org.springframework.ide.vscode.commons.cloudfoundry.client.ClientRequests; -import org.springframework.ide.vscode.commons.cloudfoundry.client.ClientTimeouts; -import org.springframework.ide.vscode.commons.cloudfoundry.client.CloudFoundryClientFactory; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFClientParams; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFCredentials; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.ClientParamsProvider; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.NoTargetsException; - -import com.google.common.collect.ImmutableList; - -/** - * An alternative to using mockito for mocking the CF client, as the mockito - * version of the CF client seems to fail when run on OpenJDK (which is used for - * the sts4 concourse ci build). - *

- * This also allows additional testing of the general manifest-yaml vscode - * framework, as the framework should be able to take any client factory, - * including the basic one below, and still produce correct results for content - * assist as well as reconcile - * - */ -public class BasicCfClientHarness { - - private BasicCFClientFactory clientFactory = new BasicCFClientFactory(); - - private ClientParamsProvider paramsProvider = new BasicClientParamsProvider(ImmutableList.of(DEFAULT_PARAMS)); - - public static CFClientParams DEFAULT_PARAMS = new CFClientParams("test.io", "testuser", - CFCredentials.fromRefreshToken("refreshtoken"), false); - - public BasicCFClientFactory getBasicClientFactory() { - return clientFactory; - } - - public ClientParamsProvider getParamsProvider() { - return paramsProvider; - } - - public void addServiceInstances(String... serviceInstances) { - List services = null; - if (serviceInstances != null) { - services = new ArrayList(); - for (String name : serviceInstances) { - services.add(createServiceInstance(name)); - } - } - getBasicClientFactory().getExistingClientInHarness().setServices(services); - } - - public void addBuildpacks(String... buildpacks) { - - // Allow testing of null condition when vscode asks for buildpacks from - // the client and - // client returns null instead of empty list - List asList = null; - if (buildpacks != null) { - asList = new ArrayList(); - for (String name : buildpacks) { - asList.add(CFEntities.createBuildpack(name)); - } - } - - getBasicClientFactory().getExistingClientInHarness().setBuildPacks(asList); - } - - protected CFServiceInstance createServiceInstance(String name) { - return CFEntities.createServiceInstance(name, null, null, null, null, null); - } - - //////////////////////////////////////////////// - // "Mocked" CF client types - - public static final class BasicCFClientFactory implements CloudFoundryClientFactory { - - /* - * Create the client "ahead of time" so that it can be configured before - * the language server is tested - */ - private BasicClientRequests preexistingClient = new BasicClientRequests(DEFAULT_PARAMS, new ClientTimeouts()); - - @Override - public ClientRequests getClient(CFClientParams params, ClientTimeouts timeouts) throws Exception { - return this.preexistingClient; - } - - /** - * Convenient non-framework method to fetch the existing client so that - * values can be set to simulate values from CF. - * - * @return - */ - public BasicClientRequests getExistingClientInHarness() { - return this.preexistingClient; - } - } - - static class BasicClientParamsProvider implements ClientParamsProvider { - - private List params; - - public BasicClientParamsProvider(List defaultParams) { - this.params = defaultParams; - } - - public void setParams(List params) { - this.params = params; - } - - @Override - public List getParams() throws NoTargetsException, ExecutionException { - return this.params; - } - - } - - static class BasicClientRequests implements ClientRequests { - - private List buildpacks; - private List serviceInstances; - private CFClientParams params; - private ClientTimeouts timeouts; - - public BasicClientRequests(CFClientParams params, ClientTimeouts timeouts) { - this.params = params; - this.timeouts = timeouts; - } - - @Override - public List getBuildpacks() throws Exception { - return this.buildpacks; - } - - @Override - public List getServices() throws Exception { - return this.serviceInstances; - } - - public void setBuildPacks(List buildpacks) { - this.buildpacks = buildpacks; - } - - public void setServices(List serviceInstances) { - this.serviceInstances = serviceInstances; - } - } - -} diff --git a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorCFBasicTest.java b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorCFBasicTest.java deleted file mode 100644 index e913530fd..000000000 --- a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorCFBasicTest.java +++ /dev/null @@ -1,128 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2017 Pivotal, Inc. - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the Eclipse Public License v1.0 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * Pivotal, Inc. - initial API and implementation - *******************************************************************************/ -package org.springframework.ide.vscode.manifest.yaml; - -import static org.junit.Assert.assertEquals; - -import org.eclipse.lsp4j.Diagnostic; -import org.eclipse.lsp4j.DiagnosticSeverity; -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Test; -import org.springframework.ide.vscode.languageserver.testharness.Editor; -import org.springframework.ide.vscode.languageserver.testharness.LanguageServerHarness; - -/** - * Basic CF tests for services and buildpacks, using a basic CF client that - * requires no actual CF connection. - * - *

- * This is an alternative to using mockito, and also tests that the - * vscode-manifest framework can take any clients, including the basic one used - * in this test, and still function as expected for CF content assist and - * reconcile - * - */ -public class ManifestYamlEditorCFBasicTest { - LanguageServerHarness harness; - BasicCfClientHarness basicCfClientHarness = new BasicCfClientHarness(); - - @Before - public void setup() throws Exception { - harness = new LanguageServerHarness(() -> new ManifestYamlLanguageServer( - basicCfClientHarness.getBasicClientFactory(), basicCfClientHarness.getParamsProvider())); - harness.intialize(null); - } - - @Test - public void contentAssistBuildpack() throws Exception { - basicCfClientHarness.addBuildpacks("java_buildpack"); - assertContainsCompletions("buildpack: <*>", "buildpack: java_buildpack<*>"); - } - - @Test - public void contentAssistDoesNotContainBuildpack() throws Exception { - basicCfClientHarness.addBuildpacks("java_buildpack"); - assertDoesNotContainCompletions("buildpack: <*>", "buildpack: wrong_buildpack<*>"); - } - - @Test - public void contentAssistServices() throws Exception { - basicCfClientHarness.addServiceInstances("mysql"); - assertContainsCompletions("services:\n" + " - <*>", "mysql"); - } - - @Test - public void contentAssistDoesNotContainServices() throws Exception { - basicCfClientHarness.addServiceInstances("mysql"); - assertDoesNotContainCompletions("services:\n" + " - <*>", "wrongsql"); - } - - @Test - public void contentAssistDoesNotContainServicesEmptyServices() throws Exception { - basicCfClientHarness.addServiceInstances(/*no services*/); - assertDoesNotContainCompletions("services:\n" + " - <*>", "mysql"); - } - - @Test - public void reconcileCFService() throws Exception { - basicCfClientHarness.addServiceInstances("myservice"); - Editor editor = harness.newEditor("applications:\n" // - + "- name: foo\n" // - + " services:\n" // - + " - myservice\n" // - ); - // Should have no problems - editor.assertProblems(/* none */); - } - - @Test - public void reconcileShowsWarningOnUnknownService() throws Exception { - basicCfClientHarness.addServiceInstances("myservice"); - Editor editor = harness.newEditor("applications:\n" // - + "- name: foo\n" // - + " services:\n" // - + " - bad-service\n" // - - ); - editor.assertProblems("bad-service|There is no service instance called"); - - Diagnostic problem = editor.assertProblem("bad-service"); - assertEquals(DiagnosticSeverity.Warning, problem.getSeverity()); - } - - @Test - public void reconcileShowsWarningOnEmptyServices() throws Exception { - // Add empty list of services - basicCfClientHarness.addServiceInstances(); - Editor editor = harness.newEditor("applications:\n" // - + "- name: foo\n" // - + " services:\n" // - + " - bad-service\n");// - editor.assertProblems("bad-service|There is no service instance called"); - - Diagnostic problem = editor.assertProblem("bad-service"); - assertEquals(DiagnosticSeverity.Warning, problem.getSeverity()); - } - - ////////////////////////////////////////////////////////////////////////////// - - private void assertContainsCompletions(String textBefore, String... textAfter) throws Exception { - Editor editor = harness.newEditor(textBefore); - editor.assertContainsCompletions(textAfter); - } - - private void assertDoesNotContainCompletions(String textBefore, String... notToBeFound) throws Exception { - Editor editor = harness.newEditor(textBefore); - editor.assertDoesNotContainCompletions(notToBeFound); - } - -} diff --git a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java index c645971b5..4444cf279 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java +++ b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java @@ -12,30 +12,25 @@ package org.springframework.ide.vscode.manifest.yaml; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.*; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.when; import java.io.IOException; -import java.util.List; import org.eclipse.lsp4j.CompletionItem; import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.DiagnosticSeverity; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.mockito.Mockito; +import org.springframework.ide.vscode.commons.cloudfoundry.client.CFBuildpack; import org.springframework.ide.vscode.commons.cloudfoundry.client.CFServiceInstance; import org.springframework.ide.vscode.commons.cloudfoundry.client.ClientRequests; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFClientParams; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFCredentials; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.ClientParamsProvider; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.NoTargetsException; -import org.springframework.ide.vscode.commons.util.CollectionUtil; import org.springframework.ide.vscode.languageserver.testharness.Editor; import org.springframework.ide.vscode.languageserver.testharness.LanguageServerHarness; -import static org.springframework.ide.vscode.languageserver.testharness.TestAsserts.*; - import com.google.common.collect.ImmutableList; public class ManifestYamlEditorTest { @@ -896,6 +891,51 @@ public class ManifestYamlEditorTest { //query string should match the 'filter text' otherwise vscode will filter the item and it will be gone! assertEquals("something", completion.getFilterText()); } + + @Test + public void serviceContentAssistEmptyServices() throws Exception { + ClientRequests cfClient = cloudfoundry.client; + when(cfClient.getServices()).thenReturn(ImmutableList.of()); + assertDoesNotContainCompletions("services:\n" + " - <*>", "mysql"); + } + + @Test + public void serviceContentAssistDoesNotContainServices() throws Exception { + ClientRequests cfClient = cloudfoundry.client; + CFServiceInstance service = Mockito.mock(CFServiceInstance.class); + when(service.getName()).thenReturn("mysql"); + when(cfClient.getServices()).thenReturn(ImmutableList.of(service)); + assertDoesNotContainCompletions("services:\n" + " - <*>", "wrongsql"); + } + + @Test + public void serviceContentAssist() throws Exception { + ClientRequests cfClient = cloudfoundry.client; + CFServiceInstance service = Mockito.mock(CFServiceInstance.class); + when(service.getName()).thenReturn("mysql"); + when(cfClient.getServices()).thenReturn(ImmutableList.of(service)); + + assertContainsCompletions("services:\n" + " - <*>", "mysql"); + } + + @Test + public void buildpackContentAssist() throws Exception { + ClientRequests cfClient = cloudfoundry.client; + CFBuildpack buildPack = Mockito.mock(CFBuildpack.class); + when(buildPack.getName()).thenReturn("java_buildpack"); + when(cfClient.getBuildpacks()).thenReturn(ImmutableList.of(buildPack)); + + assertContainsCompletions("buildpack: <*>", "buildpack: java_buildpack<*>"); + } + + @Test + public void buildpackContentAssistDoesNotContainCompletion() throws Exception { + ClientRequests cfClient = cloudfoundry.client; + CFBuildpack buildPack = Mockito.mock(CFBuildpack.class); + when(buildPack.getName()).thenReturn("java_buildpack"); + when(cfClient.getBuildpacks()).thenReturn(ImmutableList.of(buildPack)); + assertDoesNotContainCompletions("buildpack: <*>", "buildpack: wrong_buildpack<*>"); + } ////////////////////////////////////////////////////////////////////////////// @@ -903,5 +943,15 @@ public class ManifestYamlEditorTest { Editor editor = harness.newEditor(textBefore); editor.assertCompletions(textAfter); } + + private void assertDoesNotContainCompletions(String textBefore, String... notToBeFound) throws Exception { + Editor editor = harness.newEditor(textBefore); + editor.assertDoesNotContainCompletions(notToBeFound); + } + + private void assertContainsCompletions(String textBefore, String... textAfter) throws Exception { + Editor editor = harness.newEditor(textBefore); + editor.assertContainsCompletions(textAfter); + } }