From 806dd1b84068a94facdb5a036f523c47526c93ab Mon Sep 17 00:00:00 2001 From: Octavia Togami Date: Mon, 29 Mar 2021 00:48:52 -0700 Subject: [PATCH] Fix name comparision in GroovyXmlUtil (#373) Nodes can also have groovy.xml.QNames, which need to be compared using their matches(Object) method. --- .../net/fabricmc/loom/util/GroovyXmlUtil.java | 17 ++++- .../fabricmc/loom/GroovyXmlUtilTest.groovy | 74 +++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 src/test/groovy/net/fabricmc/loom/GroovyXmlUtilTest.groovy diff --git a/src/main/java/net/fabricmc/loom/util/GroovyXmlUtil.java b/src/main/java/net/fabricmc/loom/util/GroovyXmlUtil.java index c07fa64..801cc11 100644 --- a/src/main/java/net/fabricmc/loom/util/GroovyXmlUtil.java +++ b/src/main/java/net/fabricmc/loom/util/GroovyXmlUtil.java @@ -29,13 +29,14 @@ import java.util.Optional; import java.util.stream.Stream; import groovy.util.Node; +import groovy.xml.QName; public final class GroovyXmlUtil { private GroovyXmlUtil() { } public static Node getOrCreateNode(Node parent, String name) { for (Object object : parent.children()) { - if (object instanceof Node && name.equals(((Node) object).name())) { + if (object instanceof Node && isSameName(((Node) object).name(), name)) { return (Node) object; } } @@ -45,7 +46,7 @@ public final class GroovyXmlUtil { public static Optional getNode(Node parent, String name) { for (Object object : parent.children()) { - if (object instanceof Node && name.equals(((Node) object).name())) { + if (object instanceof Node && isSameName(((Node) object).name(), name)) { return Optional.of((Node) object); } } @@ -53,6 +54,18 @@ public final class GroovyXmlUtil { return Optional.empty(); } + private static boolean isSameName(Object nodeName, String givenName) { + if (nodeName instanceof String) { + return nodeName.equals(givenName); + } + + if (nodeName instanceof QName) { + return ((QName) nodeName).matches(givenName); + } + + throw new UnsupportedOperationException("Cannot determine if " + nodeName.getClass() + " is the same as a String"); + } + public static Stream childrenNodesStream(Node node) { //noinspection unchecked return (Stream) (Stream) (((List) node.children()).stream().filter((i) -> i instanceof Node)); diff --git a/src/test/groovy/net/fabricmc/loom/GroovyXmlUtilTest.groovy b/src/test/groovy/net/fabricmc/loom/GroovyXmlUtilTest.groovy new file mode 100644 index 0000000..a379b72 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/GroovyXmlUtilTest.groovy @@ -0,0 +1,74 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2016, 2017, 2018 FabricMC + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package net.fabricmc.loom + +import groovy.xml.QName +import net.fabricmc.loom.util.GroovyXmlUtil +import spock.lang.Specification + +class GroovyXmlUtilTest extends Specification { + def "getOrCreateNode finds existing node"() { + when: + def xmlTree = new XmlParser().parseText(text) + def existingNode = xmlTree[innerName] + def actualNode = GroovyXmlUtil.getOrCreateNode(xmlTree, innerName) + + then: + existingNode.text() == actualNode.text() + + where: + innerName | text + "bar" | "inner content to ensure correct" + "dependencies" | "inner content to ensure correct" + } + + def "getOrCreateNode creates a node if needed"() { + when: + def xmlTree = new XmlParser().parseText(text) + def actualNode = GroovyXmlUtil.getOrCreateNode(xmlTree, innerName) + + then: + xmlTree[QName.valueOf(actualNode.name().toString())] != null + + where: + innerName | text + "bar" | "" + "dependencies" | "" + } + + def "getNode finds existing node"() { + when: + def xmlTree = new XmlParser().parseText(text) + def actualNode = GroovyXmlUtil.getNode(xmlTree, innerName) + + then: + actualNode.isPresent() + + where: + innerName | text + "bar" | "inner content to ensure correct" + "dependencies" | "inner content to ensure correct" + } +}