Copilot commented on code in PR #4058:
URL:
https://github.com/apache/incubator-kie-kogito-runtimes/pull/4058#discussion_r2351697066
##########
springboot/integration-tests/pom.xml:
##########
@@ -33,6 +33,7 @@
<properties>
<sonar.exclusions>**/*</sonar.exclusions>
<java.module.name>org.kie.kogito.springboot.starter.tests</java.module.name>
+ <maven.compiler.release>17</maven.compiler.release>
Review Comment:
[nitpick] Consider using a property reference like
`${maven.compiler.release}` instead of hardcoding the Java version, to maintain
consistency with the parent POM and allow easier version management across the
project.
```suggestion
```
##########
kogito-codegen-modules/kogito-codegen-manager/src/main/java/org/kie/kogito/codegen/manager/GenerateModelHelper.java:
##########
@@ -20,28 +20,108 @@
import java.io.File;
import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import org.drools.codegen.common.GeneratedFile;
import org.kie.kogito.codegen.api.context.KogitoBuildContext;
import org.kie.kogito.codegen.core.ApplicationGenerator;
import org.kie.kogito.codegen.core.utils.ApplicationGeneratorDiscovery;
+import org.kie.kogito.codegen.manager.processes.PersistenceGenerationHelper;
+import org.kie.kogito.codegen.manager.util.CodeGenManagerUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import static org.drools.codegen.common.GeneratedFileType.COMPILED_CLASS;
+import static
org.kie.efesto.common.api.constants.Constants.INDEXFILE_DIRECTORY_PROPERTY;
import static org.kie.kogito.codegen.manager.CompilerHelper.RESOURCES;
import static org.kie.kogito.codegen.manager.CompilerHelper.SOURCES;
public class GenerateModelHelper {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(GenerateModelHelper.class);
+
private GenerateModelHelper() {
}
- public static Map<String, Collection<GeneratedFile>>
generateModelFiles(KogitoBuildContext kogitoBuildContext, boolean
generatePartial) {
- ApplicationGenerator appGen =
ApplicationGeneratorDiscovery.discover(kogitoBuildContext);
+ public record GenerateModelInfo(ClassLoader projectClassLoader,
+ KogitoBuildContext kogitoBuildContext,
+ boolean onDemand,
+ boolean generatePartial,
+ Map<String, String> properties,
+ File outputDirectory,
+ List<String> runtimeClassPathElements,
+ File baseDir,
+ String javaSourceEncoding,
+ String javaVersion,
+ String schemaVersion,
+ boolean keepSources) {
+
+ public GenerateModelInfo(ClassLoader projectClassLoader,
KogitoBuildContext kogitoBuildContext, BuilderManager.BuildInfo buildInfo) {
+ this(projectClassLoader, kogitoBuildContext, buildInfo.onDemand(),
buildInfo.generatePartial(), buildInfo.properties(),
+ buildInfo.outputDirectory().toFile(),
+ buildInfo.runtimeClassPathElements(),
+ buildInfo.projectBaseAbsolutePath().toFile(),
+ buildInfo.javaSourceEncoding(),
+ buildInfo.javaVersion(),
+ buildInfo.jsonSchemaVersion(),
+ buildInfo.keepSources());
+ }
+ }
+
+ public record GenerateModelFilesInfo(KogitoBuildContext kogitoBuildContext,
+ boolean generatePartial) {
+
+ public GenerateModelFilesInfo(GenerateModelInfo generateModelInfo) {
+ this(generateModelInfo.kogitoBuildContext,
+ generateModelInfo.generatePartial);
+ }
+ }
+
+ public static void generateModel(GenerateModelInfo generateModelInfo) {
+ Map<String, Collection<GeneratedFile>> generatedModelFiles;
+ if (generateModelInfo.onDemand) {
+ LOGGER.info("On-Demand Mode is On. Use mvn compile
kogito:scaffold");
+ generatedModelFiles = new HashMap<>();
+ } else {
+ generatedModelFiles = generateModelFiles(new
GenerateModelFilesInfo(generateModelInfo));
+ }
+ if (generateModelInfo.outputDirectory == null) {
+ throw new IllegalStateException("outputDirectory is null");
Review Comment:
The error message should be more descriptive. Consider: 'Output directory
cannot be null. Please ensure the project build output directory is properly
configured.'
```suggestion
throw new IllegalStateException("Output directory cannot be
null. Please ensure the project build output directory is properly
configured.");
```
##########
kogito-maven-plugin/src/main/java/org/kie/kogito/maven/plugin/AbstractKieMojo.java:
##########
@@ -19,89 +19,124 @@
package org.kie.kogito.maven.plugin;
import java.io.File;
-import java.nio.file.Path;
+import java.io.IOException;
+import java.net.URI;
import java.util.Map;
+import java.util.Set;
+import org.apache.maven.artifact.DependencyResolutionRequiredException;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.plugins.annotations.Component;
import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.project.MavenProject;
-import org.drools.codegen.common.GeneratedFileWriter;
+import org.kie.kogito.codegen.manager.BuilderManager;
import org.kie.kogito.codegen.manager.util.CodeGenManagerUtil;
import org.kie.kogito.maven.plugin.util.MojoUtil;
public abstract class AbstractKieMojo extends AbstractMojo {
- protected static final GeneratedFileWriter.Builder
GENERATED_FILE_WRITER_BUILDER = GeneratedFileWriter.builder("kogito",
"kogito.codegen.resources.directory", "kogito.codegen.sources.directory");
+ @Parameter(property = "kogito.codegen.decisions")
+ protected String generateDecisions;
- @Parameter(required = true, defaultValue = "${project.basedir}")
- protected File projectBaseDir;
+ /**
+ * Partial generation can be used when reprocessing a pre-compiled project
+ * for faster code-generation. It only generates code for rules and
processes,
+ * and does not generate extra meta-classes (etc. Application).
+ * Use only when doing recompilation and for development purposes
+ */
+ @Parameter(property = "kogito.codegen.partial", defaultValue = "false")
+ protected boolean generatePartial;
- @Parameter
- protected Map<String, String> properties;
+ @Parameter(property = "kogito.codegen.predictions")
+ protected String generatePredictions;
- @Parameter(required = true, defaultValue = "${project}")
- protected MavenProject project;
+ @Parameter(property = "kogito.codegen.processes")
+ protected String generateProcesses;
- @Parameter(required = true, defaultValue =
"${project.build.sourceEncoding}")
- protected String projectSourceEncoding;
+ @Parameter(property = "kogito.codegen.rules")
+ protected String generateRules;
- @Parameter(required = true, defaultValue =
"${project.build.outputDirectory}")
- protected File outputDirectory;
+ @Parameter
+ protected Map<String, String> properties;
- @Parameter(required = true, defaultValue = "${project.basedir}")
- protected File baseDir;
+ @Parameter(defaultValue = "17", property = "maven.compiler.release")
+ protected String mavenCompilerJavaVersion;
@Parameter(property = "kogito.codegen.persistence")
protected boolean persistence;
- @Parameter(property = "kogito.codegen.rules")
- protected String generateRules;
+ @Parameter(required = true, defaultValue = "${project.basedir}")
+ protected File projectBaseDir;
- @Parameter(property = "kogito.codegen.processes")
- protected String generateProcesses;
+ @Parameter(required = true, defaultValue =
"${project.build.outputDirectory}")
+ protected File projectBuildOutputDirectory;
- @Parameter(property = "kogito.codegen.decisions")
- protected String generateDecisions;
+ @Parameter(required = true, defaultValue =
"${project.build.sourceEncoding}")
+ protected String projectSourceEncoding;
- @Parameter(property = "kogito.codegen.predictions")
- protected String generatePredictions;
+ @Parameter(property = "kogito.jsonSchema.version", required = false)
//TODO double check this required false
Review Comment:
Remove the TODO comment or address it. If this parameter should remain
optional, document why; if it needs validation, implement proper checks.
```suggestion
/**
* Optional: Allows users to specify a custom JSON Schema version for
code generation.
* If not set, the default version will be used.
*/
@Parameter(property = "kogito.jsonSchema.version", required = false)
```
##########
kogito-codegen-modules/kogito-codegen-manager/src/main/java/org/kie/kogito/codegen/manager/util/CodeGenManagerUtil.java:
##########
@@ -158,7 +197,8 @@ static Predicate<Class<?>>
classSubTypeAvailabilityResolver(ClassLoader projectC
.anyMatch(c -> !c.isInterface() &&
!Modifier.isAbstract(c.getModifiers()));
}
- public static boolean isClassNameInUrlClassLoader(URL[] urls, String
className) {
+ public static boolean isClassNameInUrlClassLoader(Set<URI> uris, String
className) throws MalformedURLException {
+ URL[] urls = convertURIsToURLs(uris);
try (URLClassLoader cl = new URLClassLoader(urls)) {
cl.loadClass(className);
return true;
Review Comment:
Creating a new URLClassLoader for each class lookup is inefficient. Consider
caching the URLClassLoader or using a different approach for class existence
checking, especially if this method is called frequently.
##########
kogito-maven-plugin/src/main/java/org/kie/kogito/maven/plugin/GenerateModelMojo.java:
##########
@@ -15,171 +15,25 @@
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
+ *
*/
-package org.kie.kogito.maven.plugin;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.Map;
+package org.kie.kogito.maven.plugin;
-import org.apache.maven.execution.MavenSession;
-import org.apache.maven.plugin.MojoExecution;
import org.apache.maven.plugin.MojoExecutionException;
-import org.apache.maven.plugin.MojoFailureException;
-import org.apache.maven.plugins.annotations.Component;
import org.apache.maven.plugins.annotations.LifecyclePhase;
import org.apache.maven.plugins.annotations.Mojo;
-import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.plugins.annotations.ResolutionScope;
-import org.apache.maven.project.MavenProject;
-import org.drools.codegen.common.GeneratedFile;
-import org.kie.kogito.KogitoGAV;
-import org.kie.kogito.codegen.api.context.KogitoBuildContext;
-import org.kie.kogito.codegen.manager.CompilerHelper;
-import org.kie.kogito.codegen.manager.GenerateModelHelper;
-import org.kie.kogito.codegen.manager.processes.PersistenceGenerationHelper;
-import org.kie.kogito.codegen.manager.util.CodeGenManagerUtil;
-import org.kie.kogito.maven.plugin.util.MojoUtil;
-
-import static
org.kie.efesto.common.api.constants.Constants.INDEXFILE_DIRECTORY_PROPERTY;
-import static org.kie.kogito.codegen.manager.CompilerHelper.RESOURCES;
-import static org.kie.kogito.codegen.manager.CompilerHelper.SOURCES;
-import static
org.kie.kogito.codegen.manager.util.CodeGenManagerUtil.discoverKogitoRuntimeContext;
@Mojo(name = "generateModel",
requiresDependencyResolution = ResolutionScope.COMPILE_PLUS_RUNTIME,
- requiresProject = true,
defaultPhase = LifecyclePhase.COMPILE,
threadSafe = true)
public class GenerateModelMojo extends AbstractKieMojo {
- /**
- * Partial generation can be used when reprocessing a pre-compiled project
- * for faster code-generation. It only generates code for rules and
processes,
- * and does not generate extra meta-classes (etc. Application).
- * Use only when doing recompilation and for development purposes
- */
- @Parameter(property = "kogito.codegen.partial", defaultValue = "false")
- private boolean generatePartial;
-
- @Parameter(property = "kogito.codegen.ondemand", defaultValue = "false")
- private boolean onDemand;
-
- @Parameter(property = "kogito.sources.keep", defaultValue = "false")
- private boolean keepSources;
-
- @Parameter(property = "kogito.jsonSchema.version", required = false)
- String schemaVersion;
-
- @Parameter(defaultValue = "${mojoExecution}")
- private MojoExecution mojoExecution;
-
- /**
- * The <code>maven-compiler-plugin</code> version to use.
- * Default to <b>3.10.2</b>
- */
- @Parameter(defaultValue = "3.10.2", property = "version.compiler.plugin")
- private String compilerPluginVersion;
-
- @Parameter(defaultValue = "17", property = "maven.compiler.release")
- private String compilerSourceJavaVersion;
-
- @Parameter(defaultValue = "17", property = "maven.compiler.release")
- private String compilerTargetJavaVersion;
-
- @Component
- private MavenProject mavenProject;
-
- @Component
- private MavenSession mavenSession;
-
@Override
- public void execute() throws MojoExecutionException, MojoFailureException {
- getLog().debug("Compiler Target Java Version:" +
compilerTargetJavaVersion);
- getLog().debug("Compiler Source Java Version:" +
compilerSourceJavaVersion);
- getLog().debug("Compiler Source Encoding:" + projectSourceEncoding);
- getLog().debug("Targeting directory: " + outputDirectory);
-
- boolean indexFileDirectorySet = false;
- if (outputDirectory == null) {
- throw new MojoExecutionException("${project.build.directory} is
null");
- }
- if (System.getProperty(INDEXFILE_DIRECTORY_PROPERTY) == null) {
- System.setProperty(INDEXFILE_DIRECTORY_PROPERTY,
outputDirectory.toString());
- indexFileDirectorySet = true;
- }
- addCompileSourceRoots();
- Map<String, Collection<GeneratedFile>> generatedModelFiles;
- ClassLoader projectClassLoader = projectClassLoader();
- KogitoBuildContext kogitoBuildContext =
getKogitoBuildContext(projectClassLoader);
- if (isOnDemand()) {
- getLog().info("On-Demand Mode is On. Use mvn compile
kogito:scaffold");
- generatedModelFiles = new HashMap<>();
- } else {
- generatedModelFiles = generateModel(kogitoBuildContext);
- }
- if (indexFileDirectorySet) {
- System.clearProperty(INDEXFILE_DIRECTORY_PROPERTY);
- }
-
- // Compile and write model files
- compileAndDump(generatedModelFiles, projectClassLoader);
-
- Map<String, Collection<GeneratedFile>> generatedPersistenceFiles =
generatePersistence(kogitoBuildContext, projectClassLoader);
-
- compileAndDump(generatedPersistenceFiles, projectClassLoader);
-
- if (!keepSources) {
- CodeGenManagerUtil.deleteDrlFiles(outputDirectory.toPath());
- }
- }
-
- KogitoBuildContext getKogitoBuildContext(ClassLoader projectClassLoader) {
- return discoverKogitoRuntimeContext(projectClassLoader,
- projectBaseDir.toPath(),
- new KogitoGAV(project.getGroupId(),
- project.getArtifactId(),
- project.getVersion()),
- new CodeGenManagerUtil.ProjectParameters(discoverFramework(),
- generateDecisions,
- generatePredictions,
- generateProcesses,
- generateRules,
- persistence),
- className -> MojoUtil.hasClassOnClasspath(project, className));
- }
-
- protected boolean isOnDemand() {
- return onDemand;
- }
-
- protected void addCompileSourceRoots() {
-
project.addCompileSourceRoot(getGeneratedFileWriter().getScaffoldedSourcesDir().toString());
- }
-
- protected Map<String, Collection<GeneratedFile>>
generateModel(KogitoBuildContext kogitoBuildContext) {
- setSystemProperties(properties);
- return GenerateModelHelper.generateModelFiles(kogitoBuildContext,
generatePartial);
- }
-
- protected Map<String, Collection<GeneratedFile>>
generatePersistence(KogitoBuildContext kogitoBuildContext, ClassLoader
projectClassloader) {
- return
PersistenceGenerationHelper.generatePersistenceFiles(kogitoBuildContext,
projectClassloader, schemaVersion);
- }
-
- protected void compileAndDump(Map<String, Collection<GeneratedFile>>
generatedFiles, ClassLoader classloader) throws MojoExecutionException {
- try {
- // Compile and write files
-
CompilerHelper.compileAndDumpGeneratedSources(generatedFiles.get(SOURCES),
- classloader,
- project.getRuntimeClasspathElements(),
- baseDir,
- projectSourceEncoding,
- compilerSourceJavaVersion,
- compilerTargetJavaVersion);
- // Dump resources
- CompilerHelper.dumpResources(generatedFiles.get(RESOURCES),
baseDir);
- } catch (Exception e) {
- throw new MojoExecutionException("Error during processing model
classes: " + e.getMessage(), e);
- }
+ public void execute() throws MojoExecutionException {
+ getLog().info("execute");
Review Comment:
The log message 'execute' is not informative. Consider using a more
descriptive message like 'Starting Kogito model generation' or remove the
redundant logging since buildProject() already logs appropriately.
```suggestion
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
