Copilot commented on code in PR #16042:
URL: https://github.com/apache/grails-core/pull/16042#discussion_r3664439766
##########
grails-core/src/main/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformation.groovy:
##########
@@ -211,153 +231,391 @@ class GlobalGrailsClassInjectorTransformation
implements ASTTransformation, Comp
return targetDirectory
}
+ /**
+ * Adds the compiled class to the {@code META-INF/grails.factories} entry
for the supplied type
+ * when it is a concrete subtype of that type. Existing generated entries
and matching
+ * project-source entries are preserved, and the resulting factory file is
written to the
+ * compilation target directory.
+ *
+ * @param classNode the class being compiled
+ * @param superType the factory interface or superclass whose
implementations are registered
+ * @param compilationTargetDirectory the compilation output directory
containing the factory file
+ * @return {@code true} when {@code classNode} is a non-abstract subtype
of {@code superType} and
+ * was registered; {@code false} otherwise
+ */
static boolean updateGrailsFactoriesWithType(ClassNode classNode,
ClassNode superType, File compilationTargetDirectory) {
- FactoriesFileWriter.updateFactoriesWithType(classNode, superType,
compilationTargetDirectory,
- 'META-INF/grails.factories',
['src/main/resources/META-INF/grails.factories'])
+ FactoriesFileWriter.updateFactoriesWithType(
+ classNode,
+ superType,
+ compilationTargetDirectory,
+ 'META-INF/grails.factories',
+ ['src/main/resources/META-INF/grails.factories']
+ )
}
- static LinkedHashSet<String> pendingPluginClasses = []
- static Collection<String> pluginExcludes = []
+ private static boolean updateGrailsFactoriesWithTypes(ClassNode classNode,
Collection<ClassNode> superTypes, File compilationTargetDirectory) {
+ superTypes.any {
+ updateGrailsFactoriesWithType(classNode, it,
compilationTargetDirectory)
+ }
+ }
- protected static void generatePluginXml(ClassNode pluginClassNode, String
pluginVersion, Set<String> transformedClasses, File pluginXmlFile) {
+ /**
+ * Creates or updates the generated {@code META-INF/grails-plugin.xml}
descriptor and carries
+ * forward artefact classes collected during compilation.
+ *
+ * @param pluginClassNode the compiled plugin descriptor class, or {@code
null} when none was found
+ * @param pluginVersion the plugin version, or {@code null} when no
concrete plugin descriptor
+ * is being generated
+ * @param transformedClassNames the artefact classes transformed in the
current source unit
+ * @param pluginXmlFile the generated plugin descriptor file
+ */
+ protected static void generatePluginXml(
+ @Nullable ClassNode pluginClassNode,
+ @Nullable String pluginVersion,
+ Set<String> transformedClassNames,
+ File pluginXmlFile
+ ) {
+ // first check if plugin.xml exists
+ pluginXmlFile.parentFile.mkdirs()
def pluginXmlExists = pluginXmlFile.exists()
- LinkedHashSet<String> pluginClasses = []
- pluginClasses.addAll(transformedClasses)
- pluginClasses.addAll(pendingPluginClasses)
-
- // if the class being transformed is a *GrailsPlugin class then if it
doesn't exist create it
- if (pluginClassNode && !pluginClassNode.isAbstract()) {
+ def pluginClasses = [] as LinkedHashSet<String>
+ pluginClasses.addAll(transformedClassNames)
+ pluginClasses.addAll(pendingPluginClassNames)
+
+ // Create or update grails-plugin.xml when a concrete plugin class is
present; otherwise,
+ // update an existing descriptor or defer resource names until the
descriptor is compiled.
+ if (pluginClassNode && !pluginClassNode.abstract) {
+ if (!pluginVersion) {
+ throw new IllegalStateException(
+ "Unable to generate '${pluginXmlFile}' because plugin
class '${pluginClassNode.name}' " +
+ 'does not define a plugin version.'
+ )
+ }
if (!pluginXmlExists) {
+ // The plugin descriptor is being compiled for the first time.
writePluginXml(pluginClassNode, pluginVersion, pluginXmlFile,
pluginClasses)
} else {
- // otherwise if the file does exist, update it with the plugin
name
+ // Refresh the existing descriptor with the current plugin
metadata and resources.
updatePluginXml(pluginClassNode, pluginVersion, pluginXmlFile,
pluginClasses)
}
} else if (pluginXmlExists) {
- // if the class isn't the *GrailsPlugin class then only update the
plugin.xml if it already exists
+ // Add resources from this source unit to the existing descriptor.
updatePluginXml(null, pluginVersion, pluginXmlFile, pluginClasses)
} else {
- // otherwise add it to a list of pending classes to populated when
the plugin.xml is created
- pendingPluginClasses.addAll(transformedClasses)
+ // Defer these resource names until a source unit compiles the
plugin descriptor.
+ pendingPluginClassNames.addAll(transformedClassNames)
}
}
+ /**
+ * Writes a new plugin descriptor from the plugin class metadata and
supplied artefact classes.
+ *
+ * @param pluginClassNode the plugin descriptor class
+ * @param pluginVersion the required plugin version when {@code
pluginClassNode} is present
+ * @param pluginXml the output descriptor file
+ * @param artefactClassNames artefact class names to include as resources
+ */
@CompileDynamic
- static void writePluginXml(ClassNode pluginClassNode, String
pluginVersion, File pluginXml, Collection<String> artefactClasses) {
+ static void writePluginXml(
+ @Nullable ClassNode pluginClassNode,
+ String pluginVersion,
+ File pluginXml,
+ Collection<String> artefactClassNames
+ ) {
if (pluginClassNode) {
- PluginAstReader pluginAstReader = new PluginAstReader()
- def info = pluginAstReader.readPluginInfo(pluginClassNode)
-
+ def pluginInfo = new
PluginAstReader().readPluginInfo(pluginClassNode)
pluginXml.withWriter(StandardCharsets.UTF_8.name()) { Writer
writer ->
- def mkp = new MarkupBuilder(writer)
+ def markupBuilder = new MarkupBuilder(writer)
def pluginName =
GrailsNameUtils.getLogicalPropertyName(pluginClassNode.name, 'GrailsPlugin')
-
- def pluginProperties = info.getProperties()
- def excludes = pluginProperties.get('pluginExcludes')
- if (excludes instanceof List) {
- pluginExcludes.clear()
- pluginExcludes.addAll(excludes)
+ def pluginProperties = pluginInfo.properties
+ def pluginExcludes = pluginProperties.get('pluginExcludes')
+ if (pluginExcludes instanceof List) {
+ pluginExcludePatterns.clear()
+ pluginExcludePatterns.addAll(pluginExcludes)
}
- def grailsVersion = pluginProperties['grailsVersion'] ?:
getClass().getPackage().getImplementationVersion() + ' > *'
- mkp.plugin(name: pluginName, version: pluginVersion,
grailsVersion: grailsVersion) {
+ // if the plugin class doesn't define a grailsVersion, use the
version of the grails-core jar
+ def grailsVersion = pluginProperties['grailsVersion'] ?:
+
GlobalGrailsClassInjectorTransformation.package.implementationVersion + ' > *'
+
+ markupBuilder.plugin(name: pluginName, version: pluginVersion,
grailsVersion: grailsVersion) {
type(pluginClassNode.name)
- for (entry in pluginProperties) {
+ for (def entry : pluginProperties) {
delegate."$entry.key"(entry.value)
}
- // if there are pending classes to add to the plugin.xml
add those
- if (artefactClasses) {
- def antPathMatcher = new AntPathMatcher()
+ // if there are pending class names to add to the
plugin.xml - add them as resources
+ if (artefactClassNames) {
resources {
- for (String cn in artefactClasses) {
- if (!pluginExcludes.any() { String exc ->
antPathMatcher.match(exc, cn.replace('.', '/')) }) {
- resource(cn)
+ for (def artefactClassName : artefactClassNames) {
+ if
(!isResourceExcludedByPlugin(artefactClassName)) {
+ resource(artefactClassName)
}
}
}
}
}
}
- pendingPluginClasses.clear()
+ pendingPluginClassNames.clear()
}
}
- @CompileDynamic
- static void updatePluginXml(ClassNode pluginClassNode, String
pluginVersion, File pluginXmlFile, Collection<String> artefactClasses) {
- if (!artefactClasses) return
-
+ /**
+ * Updates an existing plugin descriptor with plugin metadata and newly
discovered artefact
+ * resources. If the descriptor cannot be read or written, it is recreated.
+ *
+ * @param pluginClassNode the plugin descriptor class, or {@code null}
when only resources are updated
+ * @param pluginVersion the plugin version, or {@code null} when only
resources are updated
+ * @param pluginXmlFile the existing plugin descriptor file
+ * @param artefactClassNames artefact class names to add as resources
+ */
+ static void updatePluginXml(
+ @Nullable ClassNode pluginClassNode,
+ @Nullable String pluginVersion,
+ File pluginXmlFile,
+ Collection<String> artefactClassNames
+ ) {
+ if (!artefactClassNames) return
try {
- XmlSlurper xmlSlurper = IOUtils.createXmlSlurper()
-
- def pluginXml = xmlSlurper.parse(pluginXmlFile)
+ def pluginXml = IOUtils.createXmlSlurper().parse(pluginXmlFile)
if (pluginClassNode) {
- def pluginName =
GrailsNameUtils.getLogicalPropertyName(pluginClassNode.name, 'GrailsPlugin')
- pluginXml.@name = pluginName
- pluginXml.@version = pluginVersion
- pluginXml.type = pluginClassNode.name
-
- PluginAstReader pluginAstReader = new PluginAstReader()
- def info = pluginAstReader.readPluginInfo(pluginClassNode)
-
- def pluginProperties = info.getProperties()
- def grailsVersion = pluginProperties['grailsVersion'] ?:
getClass().getPackage().getImplementationVersion() + ' > *'
- pluginXml.@grailsVersion = grailsVersion
- for (entry in pluginProperties) {
- pluginXml."$entry.key" = entry.value
+ def pluginProperties =
writePluginXmlProperties(pluginClassNode, pluginVersion, pluginXml)
+ def pluginExcludes = pluginProperties.get('pluginExcludes')
+ if (pluginExcludes instanceof List) {
+ pluginExcludePatterns.clear()
+ pluginExcludePatterns.addAll(pluginExcludes as
List<String>)
}
+ }
+ writePluginXmlResources(pluginXml, artefactClassNames)
+ handleExcludes(pluginXml)
- def excludes = pluginProperties.get('pluginExcludes')
- if (excludes instanceof List) {
- pluginExcludes.clear()
- pluginExcludes.addAll(excludes)
- }
+ pluginXmlFile.withWriter(StandardCharsets.UTF_8.name()) {
+ createMarkup(pluginXml).writeTo(it)
}
- def resources = pluginXml.resources
+ pendingPluginClassNames.clear()
- for (String cn in artefactClasses) {
- if (!resources.resource.find { it.text() == cn }) {
- resources.appendNode {
- resource(cn)
- }
+ } catch (IOException | ParserConfigurationException | SAXException e) {
+ // Invalid or unreadable description; recreate it
+ log.warn('Failed to update existing file {}. Recreating it
instead...', pluginXmlFile.absolutePath, e)
+ writePluginXml(pluginClassNode, pluginVersion, pluginXmlFile,
artefactClassNames)
+ }
Review Comment:
In updatePluginXml, the catch branch logs that the descriptor is being
recreated, but calls writePluginXml(...) even when pluginClassNode is null.
writePluginXml is a no-op in that case, so a malformed plugin.xml remains
malformed and pendingPluginClassNames is not cleared. This can leave plugin
discovery broken and can cause pending resources to leak into later
compilations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
