Skip to content

Commit

Permalink
Platform completion fix (#318)
Browse files Browse the repository at this point in the history 
* correcting copyrights

Signed-off-by: Arun Venmany <[email protected]>

* fixing platform completion issue for repeating same platform and added platform completion filtering check for conflicting platforms

Signed-off-by: Arun Venmany <[email protected]>

* fixing platform completion test comments

Signed-off-by: Arun Venmany <[email protected]>

---------

Signed-off-by: Arun Venmany <[email protected]>
  • Loading branch information
@arunvenmany-ibm
arunvenmany-ibm authored Oct 23, 2024
1 parent 2cebdcd commit dee90d7
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 21 deletions.
24 changes: 6 additions & 18 deletions ...y/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;

import org.eclipse.lemminx.commons.BadLocationException;
import org.eclipse.lemminx.dom.DOMDocument;
Expand Down Expand Up @@ -78,24 +79,10 @@ public void onXMLContent(ICompletionRequest request, ICompletionResponse respons
String currentPlatformName = platformTextNode != null ? platformTextNode.getTextContent() : "";
String currentPlatformNameWithoutVersion = LibertyUtils.stripVersion(currentPlatformName);
// collect existing platforms
List<String> existingPlatforms = new ArrayList<>();
if (parentElement.getParentNode() != null
&& parentElement.getParentNode().getNodeName().equals(LibertyConstants.FEATURE_MANAGER_ELEMENT)) {
DOMNode featureMgrNode = parentElement.getParentNode();
List<DOMNode> features = featureMgrNode.getChildren();
for (DOMNode featureNode : features) {
DOMNode featureTextNode = (DOMNode) featureNode.getChildNodes().item(0);
// skip nodes that do not have any text value (ie. comments)
if (featureNode.getNodeName().equals(LibertyConstants.PLATFORM_ELEMENT) && featureTextNode != null) {
String platformName = featureTextNode.getTextContent();
String platformWithoutVersion = LibertyUtils.stripVersion(platformName);
if (!platformWithoutVersion.equalsIgnoreCase(currentPlatformNameWithoutVersion)) {
existingPlatforms.add(platformWithoutVersion.toLowerCase());
}
}
}
}
this.buildPlatformCompletionItems(request, response, parentElement, existingPlatforms);
List<String> existingPlatforms = FeatureService.getInstance()
.collectExistingPlatforms(request.getXMLDocument(),currentPlatformNameWithoutVersion);
List<String> existingPlatformsWithoutVersion = existingPlatforms.stream().map(LibertyUtils::stripVersion).collect(Collectors.toList());
this.buildPlatformCompletionItems(request, response, parentElement, existingPlatformsWithoutVersion);
}
}

Expand All @@ -120,6 +107,7 @@ private void buildPlatformCompletionItems(ICompletionRequest request, ICompletio
request.getXMLDocument().getDocumentURI());
platforms.stream().filter(it->(Objects.isNull(platformName) || it.contains(platformName)))
.filter(p -> !existingPlatforms.contains(LibertyUtils.stripVersion(p).toLowerCase()))
.filter(p -> !existingPlatforms.contains(LibertyConstants.conflictingPlatforms.get(LibertyUtils.stripVersion(p).toLowerCase())))
.forEach(platformItem -> {
Range range = XMLPositionUtility.createRange(parentElement.getStartTagCloseOffset() + 1,
parentElement.getEndTagOpenOffset(), request.getXMLDocument());
Expand Down
5 changes: 4 additions & 1 deletion ...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 @@ -697,6 +697,7 @@ private void addRequiredFeatureNames(Feature feature, Set<String> requiredFeatur
public List<String> collectExistingPlatforms(DOMDocument document, String currentPlatformName) {
List<String> includedPlatforms = new ArrayList<>();
List<DOMNode> nodes = document.getDocumentElement().getChildren();
boolean alreadyIgnoredOnce = false;
DOMNode featureManager = null;

for (DOMNode node : nodes) {
Expand All @@ -716,7 +717,9 @@ public List<String> collectExistingPlatforms(DOMDocument document, String curren
if (platformNode.getNodeName().equals(LibertyConstants.PLATFORM_ELEMENT) && platformTextNode != null) {
String platformName = platformTextNode.getTextContent();
String platformNameLowerCase = platformName.toLowerCase();
if ((!platformNameLowerCase.equalsIgnoreCase(currentPlatformName))) {
if (platformNameLowerCase.equalsIgnoreCase(currentPlatformName) && !alreadyIgnoredOnce) {
alreadyIgnoredOnce = true;
} else {
includedPlatforms.add(platformNameLowerCase);
}
}
Expand Down
44 changes: 42 additions & 2 deletions lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,21 @@ public void testPlatformCompletionItem() throws BadLocationException {
" </featureManager>", //
"</server>" //
);
//here result should be 5 because it should show only jakartaee related completion as javaee is already added
XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 5);
//here result should be 2 because it should show only one for CDATA and one for <-
// since ja is entered and javaee-8.0 is included, jakartaee should not be shown because its conflicting with javaee
XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 2);

serverXML = String.join(newLine, //
"<server description=\"Sample Liberty server\">", //
" <featureManager>", //
" <platform>jakartaee-9.0</platform>", //
" <platform>ja|</platform>", //
" </featureManager>", //
"</server>" //
);
//here result should be 2 because it should show only one for CDATA and one for <-
// since ja is entered and jakartaee-8.0 is included, javaee should not be shown because its conflicting with jakartaee
XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 2);

serverXML = String.join(newLine, //
"<server description=\"Sample Liberty server\">", //
Expand All @@ -157,6 +170,33 @@ public void testPlatformCompletionItem() throws BadLocationException {
);
//here result should be 8 because it should show only jakartaee and javaee related completion as microprofile is already added
XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 8);

// repeating same platform to see for any issues
serverXML = String.join(newLine, //
"<server description=\"Sample Liberty server\">", //
" <featureManager>", //
" <platform>jakartaee-9.0</platform>", //
" <platform>jakartaee|</platform>", //
" </featureManager>", //
"</server>" //
);
//here result should be 2 because it should show only one for CDATA and one for <-
// since jakartaee is entered and jakartaee-9.0 is included, javaee should not be shown because its conflicting with jakartaee
XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 2);

// repeating same platform to see for any issues
serverXML = String.join(newLine, //
"<server description=\"Sample Liberty server\">", //
" <featureManager>", //
" <platform>jakartaee-9.0</platform>", //
" <platform>jakartaee-9.0</platform>", //
" <platform>jakartaee|</platform>", //
" </featureManager>", //
"</server>" //
);
//here result should be 2 because it should show only one for CDATA and one for <-
// since jakartaee is entered and jakartaee-9.0 is included, javaee should not be shown because its conflicting with jakartaee
XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 2);
}

// Tests the feature completion for same feature repetition
Expand Down

0 comments on commit dee90d7

Please sign in to comment.