From e01d439b2bfa0e03a207be894a52d08a959be1ba Mon Sep 17 00:00:00 2001 From: nsingh Date: Mon, 9 Jan 2017 15:00:35 -0800 Subject: [PATCH] Removed CF target caching Clients should be cached and reused when possible, but not the targets as the external targets configuration (e.g. CLI config) can change while the language server is running. --- .../{CFTargets.java => CFTargetsFactory.java} | 20 +++--- .../cloudfoundry/client/CFClientTest.java | 4 +- .../yaml/AbstractCFHintsProvider.java | 52 ++++++++++++++++ .../ManifestYamlCFBuildpacksProvider.java | 45 +++++++------- .../yaml/ManifestYamlCFServicesProvider.java | 42 ++++++------- .../yaml/ManifestYamlLanguageServer.java | 62 ++++--------------- 6 files changed, 122 insertions(+), 103 deletions(-) rename vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/{CFTargets.java => CFTargetsFactory.java} (86%) create mode 100644 vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java diff --git a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargets.java b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargetsFactory.java similarity index 86% rename from vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargets.java rename to vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargetsFactory.java index afc445ea3..4b0c923e9 100644 --- a/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargets.java +++ b/vscode-extensions/commons/commons-cf/src/main/java/org/springframework/ide/vscode/commons/cloudfoundry/client/cftarget/CFTargetsFactory.java @@ -21,16 +21,21 @@ import org.springframework.ide.vscode.commons.cloudfoundry.client.v2.DefaultClou * Creates targets given a client parameters factory and a client factory. * */ -public class CFTargets { +public class CFTargetsFactory { - private final CFClientParamsFactory paramsFactory; private final CloudFoundryClientFactory clientFactory; + private final CFClientParamsFactory paramsFactory; - public CFTargets(CFClientParamsFactory paramsFactory, CloudFoundryClientFactory clientFactory) { - this.paramsFactory = paramsFactory; + public CFTargetsFactory(CFClientParamsFactory paramsFactory, CloudFoundryClientFactory clientFactory) { this.clientFactory = clientFactory; + this.paramsFactory = paramsFactory; } + /** + * + * @return up-to-date list of CF targets. + * @throws Exception + */ public List getTargets() throws Exception { List allParams = paramsFactory.getParams(); List targets = new ArrayList<>(); @@ -59,10 +64,11 @@ public class CFTargets { } } - public static CFTargets createDefaultV2Targets() { - CFClientParamsFactory paramsFactory = CFClientParamsFactory.INSTANCE; + public static CFTargetsFactory createDefaultV2TargetsFactory() { CloudFoundryClientFactory clientFactory = DefaultCloudFoundryClientFactoryV2.INSTANCE; - return new CFTargets(paramsFactory, clientFactory); + CFClientParamsFactory paramsFactory = CFClientParamsFactory.INSTANCE; + + return new CFTargetsFactory(paramsFactory, clientFactory); } } diff --git a/vscode-extensions/commons/commons-cf/src/test/java/org/springframework/ide/vscode/commons/cloudfoundry/client/CFClientTest.java b/vscode-extensions/commons/commons-cf/src/test/java/org/springframework/ide/vscode/commons/cloudfoundry/client/CFClientTest.java index a1a4e1a95..1fcdaf2d2 100644 --- a/vscode-extensions/commons/commons-cf/src/test/java/org/springframework/ide/vscode/commons/cloudfoundry/client/CFClientTest.java +++ b/vscode-extensions/commons/commons-cf/src/test/java/org/springframework/ide/vscode/commons/cloudfoundry/client/CFClientTest.java @@ -18,7 +18,7 @@ import org.junit.Ignore; import org.junit.Test; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFClientParamsFactory; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTarget; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTargets; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTargetsFactory; import org.springframework.ide.vscode.commons.cloudfoundry.client.v2.CloudFoundryClientFactory; import org.springframework.ide.vscode.commons.cloudfoundry.client.v2.DefaultCloudFoundryClientFactoryV2; @@ -31,7 +31,7 @@ public class CFClientTest { CFClientParamsFactory paramsFactory = CFClientParamsFactory.INSTANCE; CloudFoundryClientFactory clientFactory = DefaultCloudFoundryClientFactoryV2.INSTANCE; - CFTargets targets = new CFTargets(paramsFactory, clientFactory); + CFTargetsFactory targets = new CFTargetsFactory(paramsFactory, clientFactory); CFTarget target = targets.getTargets().get(0); List buildPacks = target.getBuildpacks(); diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java new file mode 100644 index 000000000..6f3a61efc --- /dev/null +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java @@ -0,0 +1,52 @@ +/******************************************************************************* + * 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.Collection; +import java.util.Collections; +import java.util.List; + +import javax.inject.Provider; + +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTarget; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTargetsFactory; +import org.springframework.ide.vscode.commons.yaml.schema.YValueHint; + +import com.google.common.collect.Lists; + +public abstract class AbstractCFHintsProvider implements Provider> { + + private final CFTargetsFactory targetsFactory; + + public AbstractCFHintsProvider(CFTargetsFactory targetsFactory) { + this.targetsFactory = targetsFactory; + } + + /** + * + * @return non-null list of targets. May be empty if no targets available. + * @throws Exception + * if error when creating the targets + */ + protected List getUpdatedCFTargets() throws Exception { + if (targetsFactory != null) { + // Do not cache the targets. They may change (e.g. if using cf + // CLI config, if CLI targets another API or org/space, the list + // of targets fetched from the factory also + // needs to be up-to-date) + List updatedTargets = targetsFactory.getTargets(); + if (updatedTargets != null) { + Lists.newArrayList(updatedTargets.iterator()); + } + } + return Collections.emptyList(); + } +} diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java index 4b113b7a4..0533734b6 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java @@ -16,48 +16,45 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; -import javax.inject.Provider; - import org.springframework.ide.vscode.commons.cloudfoundry.client.CFBuildpack; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTarget; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTargetsFactory; import org.springframework.ide.vscode.commons.yaml.schema.BasicYValueHint; import org.springframework.ide.vscode.commons.yaml.schema.YValueHint; -public class ManifestYamlCFBuildpacksProvider implements Provider> { +public class ManifestYamlCFBuildpacksProvider extends AbstractCFHintsProvider { - private final List targets; private static final Logger logger = Logger.getLogger(ManifestYamlCFBuildpacksProvider.class.getName()); - - public ManifestYamlCFBuildpacksProvider(List targets) { - this.targets = targets; + public ManifestYamlCFBuildpacksProvider(CFTargetsFactory targetsFactory) { + super(targetsFactory); } - @Override public Collection get() { List hints = new ArrayList<>(); - - if (targets != null) { + try { + // Do not cache the targets. They may change (e.g. if using cf + // CLI config, if CLI targets another API or org/space, the list + // of targets fetched from the factory also + // needs to be up-to-date) + List targets = getUpdatedCFTargets(); for (CFTarget cfTarget : targets) { - - List buildpacks; - try { - buildpacks = cfTarget.getBuildpacks(); - if (buildpacks != null) { - for (CFBuildpack buildpack : buildpacks) { - String name = buildpack.getName(); - String label = getBuildpackLabel(cfTarget, buildpack); - YValueHint hint = new BasicYValueHint(name, label); - if (!hints.contains(hint)) { - hints.add(hint); - } + + List buildpacks = cfTarget.getBuildpacks(); + if (buildpacks != null) { + for (CFBuildpack buildpack : buildpacks) { + String name = buildpack.getName(); + String label = getBuildpackLabel(cfTarget, buildpack); + YValueHint hint = new BasicYValueHint(name, label); + if (!hints.contains(hint)) { + hints.add(hint); } } - } catch (Exception e) { - logger.log(Level.SEVERE, e.getMessage(), e); } } + } catch (Exception e) { + logger.log(Level.SEVERE, e.getMessage(), e); } return hints; } diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java index 19119ce86..bcc45eb90 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java @@ -16,47 +16,47 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; -import javax.inject.Provider; - import org.springframework.ide.vscode.commons.cloudfoundry.client.CFServiceInstance; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTarget; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTargetsFactory; import org.springframework.ide.vscode.commons.yaml.schema.BasicYValueHint; import org.springframework.ide.vscode.commons.yaml.schema.YValueHint; -public class ManifestYamlCFServicesProvider implements Provider> { +public class ManifestYamlCFServicesProvider extends AbstractCFHintsProvider { - private final List targets; private static final Logger logger = Logger.getLogger(ManifestYamlCFServicesProvider.class.getName()); - public ManifestYamlCFServicesProvider(List targets) { - this.targets = targets; + public ManifestYamlCFServicesProvider(CFTargetsFactory targetsFactory) { + super(targetsFactory); } @Override public Collection get() { List hints = new ArrayList<>(); + + try { + + List targets = getUpdatedCFTargets(); - if (targets != null) { for (CFTarget cfTarget : targets) { - - try { - List services = cfTarget.getClientRequests().getServices(); - if (services != null) { - for (CFServiceInstance service : services) { - String name = service.getName(); - String label = getServiceLabel(cfTarget, service); - YValueHint hint = new BasicYValueHint(name, label); - if (!hints.contains(hint)) { - hints.add(hint); - } + List services = cfTarget.getClientRequests().getServices(); + if (services != null) { + for (CFServiceInstance service : services) { + String name = service.getName(); + String label = getServiceLabel(cfTarget, service); + YValueHint hint = new BasicYValueHint(name, label); + if (!hints.contains(hint)) { + hints.add(hint); } - } - } catch (Exception e) { - logger.log(Level.SEVERE, e.getMessage(), e); + } } } + + } catch (Exception e) { + logger.log(Level.SEVERE, e.getMessage(), e); } + return hints; } diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlLanguageServer.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlLanguageServer.java index 4afb45945..2c6027516 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlLanguageServer.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlLanguageServer.java @@ -1,17 +1,13 @@ package org.springframework.ide.vscode.manifest.yaml; import java.util.Collection; -import java.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; import javax.inject.Provider; import org.eclipse.lsp4j.CompletionOptions; import org.eclipse.lsp4j.ServerCapabilities; import org.eclipse.lsp4j.TextDocumentSyncKind; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTarget; -import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTargets; +import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTargetsFactory; import org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngine; import org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter; import org.springframework.ide.vscode.commons.languageserver.hover.HoverInfoProvider; @@ -33,27 +29,21 @@ import org.springframework.ide.vscode.commons.yaml.schema.YamlSchema; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureProvider; import org.yaml.snakeyaml.Yaml; -import com.google.common.collect.ImmutableList; - public class ManifestYamlLanguageServer extends SimpleLanguageServer { - private static final Provider> NO_PROVIDER = () -> ImmutableList.of(); - private static Logger logger = Logger.getLogger(ManifestYamlLanguageServer.class.getName()); private Yaml yaml = new Yaml(); private YamlSchema schema; - private CFTargets cfTargets; + private CFTargetsFactory cfTargetsFactory; public ManifestYamlLanguageServer() { SimpleTextDocumentService documents = getTextDocumentService(); YamlASTProvider parser = new YamlParser(yaml); - - CFTargets cfTargets = getCFTargets(); - - Provider> buildPacksProvider = getBuildpacksProvider(cfTargets); - Provider> servicesProvider = getServicesProvider(cfTargets); + + Provider> buildPacksProvider = getBuildpacksProvider(); + Provider> servicesProvider = getServicesProvider(); schema = new ManifestYmlSchema(buildPacksProvider, servicesProvider); @@ -87,45 +77,19 @@ public class ManifestYamlLanguageServer extends SimpleLanguageServer { documents.onHover(hoverEngine ::getHover); } - private CFTargets getCFTargets() { - // TODO: probably shouldn't be cached as targets can change during a language server session - if (cfTargets == null) { - cfTargets = CFTargets.createDefaultV2Targets(); + private CFTargetsFactory getCFTargetsFactory() { + if (cfTargetsFactory == null) { + cfTargetsFactory = CFTargetsFactory.createDefaultV2TargetsFactory(); } - return cfTargets; + return cfTargetsFactory; } - private Provider> getBuildpacksProvider(CFTargets targets) { - - try { - if (targets != null) { - List cfTargets = targets.getTargets(); - if (cfTargets != null && !cfTargets.isEmpty()) {; - return new ManifestYamlCFBuildpacksProvider(cfTargets); - } - } - } catch (Exception e) { - logger.log(Level.SEVERE, e.getMessage(), e); - } - - return NO_PROVIDER; + private Provider> getBuildpacksProvider() { + return new ManifestYamlCFBuildpacksProvider(getCFTargetsFactory()); } - private Provider> getServicesProvider(CFTargets targets) { - - try { - if (targets != null) { - List cfTargets = targets.getTargets(); - if (cfTargets != null && !cfTargets.isEmpty()) { - return new ManifestYamlCFServicesProvider(cfTargets); - } - } - - } catch (Exception e) { - logger.log(Level.SEVERE, e.getMessage(), e); - } - - return null; + private Provider> getServicesProvider() { + return new ManifestYamlCFServicesProvider(getCFTargetsFactory()); } @Override