Skip to content

Commit

Permalink
Refactor FeatureListGraph
Browse files Browse the repository at this point in the history
  • Loading branch information
@cherylking
cherylking committed Jan 23, 2024
1 parent ab4ffb2 commit 920dafd
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 135 deletions.
21 changes: 7 additions & 14 deletions ...iberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2020, 2023 IBM Corporation and others.
* Copyright (c) 2020, 2024 IBM Corporation and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
Expand All @@ -25,8 +25,6 @@
import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime;
import io.openliberty.tools.langserver.lemminx.models.feature.*;
import io.openliberty.tools.langserver.lemminx.services.FeatureService;
import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager;
import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace;
import io.openliberty.tools.langserver.lemminx.services.SettingsService;
import io.openliberty.tools.langserver.lemminx.util.*;

Expand Down Expand Up @@ -87,18 +85,13 @@ private Hover getFeatureDescription(String featureName, DOMDocument domDocument)
}

private Hover getHoverFeatureDescription(String featureName, DOMDocument document) {
LibertyWorkspace ws = LibertyProjectsManager.getInstance().getWorkspaceFolder(document.getDocumentURI());
FeatureListGraph featureGraph = null;
if (ws == null) {
LOGGER.warning("Could not get workspace for: "+document.getDocumentURI() + ". Using cached feature list for hover.");
featureGraph = FeatureService.getInstance().getDefaultFeatureList();
} else {
featureGraph = ws.getFeatureListGraph();
}

FeatureListNode flNode = featureGraph.get(featureName);
// Choosing to use the default feature list to get the full enables/enabled by information to display, as quite often the generated
// feature list will only be a subset of the default one. If the feature is not found in the default feature list, this code will
// default to the original description only which is available from the downloaded features.json file.
FeatureListGraph featureGraph = FeatureService.getInstance().getDefaultFeatureList();
FeatureListNode flNode = featureGraph.getFeatureListNode(featureName);
if (flNode == null) {
LOGGER.warning("Could not get full description for feature: "+featureName+" from cached feature list.");
LOGGER.warning("Could not get full description for feature: "+featureName+" from cached feature list. Using description from features.json file.");
return getFeatureDescription(featureName, document);
}

Expand Down
11 changes: 6 additions & 5 deletions ...liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.jsonrpc.CancelChecker;

import io.openliberty.tools.langserver.lemminx.data.ConfigElementNode;
import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph;
import io.openliberty.tools.langserver.lemminx.data.FeatureListNode;
import io.openliberty.tools.langserver.lemminx.services.FeatureService;
import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager;
import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace;
Expand Down Expand Up @@ -73,14 +73,15 @@ public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeAction
featureGraph = ws.getFeatureListGraph();
}

FeatureListNode flNode = featureGraph.get(diagnostic.getSource());
if (flNode == null) {
ConfigElementNode configElement = featureGraph.getConfigElementNode(diagnostic.getSource());
if (configElement == null) {
LOGGER.warning("Could not add quick fix for missing feature for config element due to missing information in the feature list: "+diagnostic.getSource());
return;
}

// getAllEnabledBy would return all transitive features but typically offers too much
Set<String> featureCandidates = flNode.getEnabledBy();
// This is getting features that directly enable the config element and not all transitive features
// as that would typically be too much to display/offer in a quick fix.
Set<String> featureCandidates = configElement.getEnabledBy();
if (featureCandidates.isEmpty()) {
return;
}
Expand Down
21 changes: 21 additions & 0 deletions ...liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/ConfigElementNode.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*******************************************************************************
* Copyright (c) 2024 IBM Corporation and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* IBM Corporation - initial API and implementation
*******************************************************************************/
package io.openliberty.tools.langserver.lemminx.data;

// Class to represent a config element in a feature list xml
public class ConfigElementNode extends Node {

public ConfigElementNode(String nodeName) {
super(nodeName);
}
}
107 changes: 31 additions & 76 deletions ...-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListGraph.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,62 +21,68 @@

public class FeatureListGraph {
private String runtime = "";
private Map<String, FeatureListNode> nodes;
private Map<String, FeatureListNode> featureNodes;
private Map<String, ConfigElementNode> configElementNodes;
private Map<String, Node> nodes;
private Map<String, Set<String>> enabledByCache;
private Map<String, Set<String>> enabledByCacheLowerCase; // storing in lower case to enable diagnostics with configured features
private Map<String, Set<String>> enablesCache;


public FeatureListGraph() {
nodes = new HashMap<String, FeatureListNode>();
nodes = new HashMap<String, Node>();
featureNodes = new HashMap<String, FeatureListNode>();
configElementNodes = new HashMap<String, ConfigElementNode>();
enabledByCacheLowerCase = new HashMap<String, Set<String>>();
enabledByCache = new HashMap<String, Set<String>>();
enablesCache = new HashMap<String, Set<String>>();
}

public FeatureListNode addFeature(String nodeName) {
if (nodes.containsKey(nodeName)) {
return nodes.get(nodeName);
if (featureNodes.containsKey(nodeName)) {
return featureNodes.get(nodeName);
}
FeatureListNode node = new FeatureListNode(nodeName);
featureNodes.put(nodeName, node);
nodes.put(nodeName, node);
return node;
}

public FeatureListNode addFeature(String nodeName, String description) {
if (nodes.containsKey(nodeName)) {
FeatureListNode node = nodes.get(nodeName);
if (featureNodes.containsKey(nodeName)) {
FeatureListNode node = featureNodes.get(nodeName);
if (node.getDescription().isEmpty()) {
node.setDescription(description);
}
return node;
}
FeatureListNode node = new FeatureListNode(nodeName, description);
featureNodes.put(nodeName, node);
nodes.put(nodeName, node);
return node;
}

public FeatureListNode addConfigElement(String nodeName) {
if (nodes.containsKey(nodeName)) {
return nodes.get(nodeName);
public ConfigElementNode addConfigElement(String nodeName) {
if (configElementNodes.containsKey(nodeName)) {
return configElementNodes.get(nodeName);
}
FeatureListNode node = new FeatureListNode(nodeName);
ConfigElementNode node = new ConfigElementNode(nodeName);
configElementNodes.put(nodeName, node);
nodes.put(nodeName, node);
return node;
}

public FeatureListNode get(String nodeName) {
return nodes.get(nodeName);
public FeatureListNode getFeatureListNode(String nodeName) {
return featureNodes.get(nodeName);
}

public ConfigElementNode getConfigElementNode(String nodeName) {
return configElementNodes.get(nodeName);
}

public boolean isEmpty() {
return nodes.isEmpty();
return configElementNodes.isEmpty() && featureNodes.isEmpty();
}

public boolean isConfigElement(String featureListNode) {
if (!nodes.containsKey(featureListNode)) {
return false;
}
return nodes.get(featureListNode).isConfigElement();
public boolean isConfigElement(String name) {
return configElementNodes.containsKey(name);
}

public void setRuntime(String runtime) {
Expand Down Expand Up @@ -113,12 +119,13 @@ public Set<String> getAllEnabledBy(String elementName, boolean lowerCase) {
if (enabledByCache.containsKey(elementName)) {
return enabledByCache.get(elementName);
}

if (!nodes.containsKey(elementName)) {
return null;
}

// Implements a breadth-first-search on parent nodes
Set<String> allEnabledBy = new HashSet<String>(nodes.get(elementName).getEnabledBy());
Set<String> allEnabledBy = new HashSet<String>(nodes.get(elementName).getEnabledBy());;
Deque<String> queue = new ArrayDeque<String>(allEnabledBy);
Set<String> visited = new HashSet<String>();
while (!queue.isEmpty()) {
Expand Down Expand Up @@ -149,57 +156,5 @@ private Set<String> addToEnabledByCache(String configElement, Set<String> allEna

return lowerCase ? lowercaseEnabledBy : originalcaseEnabledBy;
}

/**
* Returns the set of supported features or config elements for a given feature.
* @param feature
* @return
*/
public Set<String> getAllEnables(String feature) {
if (enablesCache.containsKey(feature)) {
return enablesCache.get(feature);
}
if (!nodes.containsKey(feature)) {
return null;
}
// Implements a breadth-first-search on child nodes
Set<String> allEnables = new HashSet<String>(nodes.get(feature).getEnables());
Deque<String> queue = new ArrayDeque<String>(allEnables);
Set<String> visited = new HashSet<String>();
while (!queue.isEmpty()) {
String node = queue.getFirst();
queue.removeFirst();
if (visited.contains(node)) {
continue;
}
Set<String> enablers = nodes.get(node).getEnables();
visited.add(node);
allEnables.addAll(enablers);
queue.addAll(enablers);
}
enablesCache.put(feature, allEnables);
return allEnables;
}

/** Will be useful for future features **/

// public Set<String> getAllConfigElements(String feature) {
// Set<String> configElements = new HashSet<String>();
// for (String node : getAllEnables(feature)) {
// if (isConfigElement(node)) {
// configElements.add(node);
// }
// }
// return configElements;
// }

// public Set<String> getAllEnabledFeatures(String feature) {
// Set<String> enabledFeatures = new HashSet<String>();
// for (String node : getAllEnables(feature)) {
// if (!isConfigElement(node)) {
// enabledFeatures.add(node);
// }
// }
// return enabledFeatures;
// }

}
48 changes: 15 additions & 33 deletions ...x-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/FeatureListNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,20 @@
import java.util.Set;


// Class to represent a feature OR config element in a feature list xml
public class FeatureListNode {
protected String nodeName;
protected String description; // only used for features
protected Set<String> enabledBy;
protected Set<String> enables;
// Class to represent a feature in a feature list xml
public class FeatureListNode extends Node {
protected String description;
protected Set<String> enablesFeatures;
protected Set<String> enablesConfigElements;

public FeatureListNode(String nodeName) {
enabledBy = new HashSet<String>();
enables = new HashSet<String>();
this.nodeName = nodeName;
super(nodeName);
enablesFeatures = new HashSet<String>();
enablesConfigElements = new HashSet<String>();
}

public FeatureListNode(String nodeName, String description) {
enabledBy = new HashSet<String>();
enables = new HashSet<String>();
this.nodeName = nodeName;
this(nodeName);
this.description = description;
}

Expand All @@ -43,35 +40,20 @@ public void setDescription(String description) {
public String getDescription() {
return this.description == null ? "" : this.description;
}

public void addEnabledBy(String nodeName) {
enabledBy.add(nodeName);
}

public void addEnables(String nodeName) {
enables.add(nodeName);
}

public Set<String> getEnabledBy() {
return enabledBy;
public void addEnablesFeature(String nodeName) {
enablesFeatures.add(nodeName);
}

public Set<String> getEnables() {
return enables;
public void addEnablesConfigElement(String nodeName) {
enablesConfigElements.add(nodeName);
}

public Set<String> getEnablesFeatures() {
Set<String> enablesFeatures = new HashSet<String>();
for (String next: enables) {
if (next.contains("-")) {
enablesFeatures.add(next);
}
}
return enablesFeatures;
}

// based on a heuristic that features use major versions and config elements don't use '.'
public boolean isConfigElement() {
return this.nodeName.indexOf('.') == -1;
public Set<String> getEnablesConfigElements() {
return enablesConfigElements;
}
}
34 changes: 34 additions & 0 deletions lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/data/Node.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*******************************************************************************
* Copyright (c) 2024 IBM Corporation and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* IBM Corporation - initial API and implementation
*******************************************************************************/
package io.openliberty.tools.langserver.lemminx.data;

import java.util.HashSet;
import java.util.Set;

public class Node {
protected String nodeName;
protected Set<String> enabledBy;

public Node(String nodeName) {
enabledBy = new HashSet<String>();
this.nodeName = nodeName;
}

public void addEnabledBy(String nodeName) {
enabledBy.add(nodeName);
}

public Set<String> getEnabledBy() {
return enabledBy;
}
}
7 changes: 4 additions & 3 deletions ...iberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.google.gson.Gson;
import com.google.gson.JsonParseException;

import io.openliberty.tools.langserver.lemminx.data.ConfigElementNode;
import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph;
import io.openliberty.tools.langserver.lemminx.data.FeatureListNode;
import io.openliberty.tools.langserver.lemminx.models.feature.Feature;
Expand Down Expand Up @@ -499,14 +500,14 @@ public List<Feature> readFeaturesFromFeatureListFile(List<Feature> installedFeat
for (String enabledFeature : enables) {
FeatureListNode feature = featureListGraph.addFeature(enabledFeature);
feature.addEnabledBy(currentFeature);
currentFeatureNode.addEnables(enabledFeature);
currentFeatureNode.addEnablesFeature(enabledFeature);
}
}
if (configElements != null) {
for (String configElement : configElements) {
FeatureListNode configNode = featureListGraph.addConfigElement(configElement);
ConfigElementNode configNode = featureListGraph.addConfigElement(configElement);
configNode.addEnabledBy(currentFeature);
currentFeatureNode.addEnables(configElement);
currentFeatureNode.addEnablesConfigElement(configElement);
}
}
}
Expand Down
Loading

0 comments on commit 920dafd

Please sign in to comment.