Add methods for remove specific properties and property removal #2
Conversation
Introduced methods to customize JAR manifests, remove specific properties, and handle dependency trees. Enhanced documentation for added functionalities like Spring factories generation, manifest building, and flattened dependencies. These changes improve JAR handling flexibility and maintain build configurability.
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Maven as Maven Build
participant BuildModuleMojo as BuildModuleMojo
participant JAR as Built JAR
participant TempJAR as Temp JAR
Maven->>BuildModuleMojo: execute()
BuildModuleMojo->>BuildModuleMojo: generateSpringFactoriesToOutputDir()
BuildModuleMojo->>BuildModuleMojo: createJarWithCustomManifest()
BuildModuleMojo->>BuildModuleMojo: removeProperties()
BuildModuleMojo->>JAR: Read JAR entries
BuildModuleMojo->>TempJAR: Write entries excluding property files
BuildModuleMojo->>BuildModuleMojo: Replace original JAR with Temp JAR
Possibly related PRs
Suggested labels
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Docstrings generation was requested by @machacjozef. * #2 (comment) The following files were modified: * `src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java`
|
Note Generated docstrings for this pull request at #3 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java (2)
347-349:⚠️ Potential issueCompile-time failure:
List#getFirst()is not available
java.util.Listhas nogetFirst()method – the code will not compile.
Fix by accessing the first element withget(0)orstream().findFirst().- putIfNotNull(customAttributes, "Netgrif-License", project.getLicenses().getFirst()); + putIfNotNull(customAttributes, "Netgrif-License", project.getLicenses().get(0));
423-427:⚠️ Potential issueNull-pointer risk when
hostAppis not suppliedThe
else if (hostApp.isValid())branch dereferenceshostAppwithout a null-check, which will throw an NPE whenhostAppis null andnaeVersionis blank.- } else if (hostApp.isValid()) { + } else if (hostApp != null && hostApp.isValid()) {
🧹 Nitpick comments (4)
src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java (4)
284-297: Jar entry metadata is lost during copy
new JarEntry(name)recreates the entry but drops timestamps, compression method, extra fields, etc. Use the copy-constructor so all attributes are preserved:- jarOut.putNextEntry(new JarEntry(name)); + jarOut.putNextEntry(new JarEntry(entry));Optionally copy/set the time explicitly if you need deterministic builds.
589-595: Use UTF-8 when writing the imports file
PrintWriterwithout charset uses the platform default, yielding non-deterministic builds across machines. Specify UTF-8 explicitly:-importsFile, "org.springframework.boot.autoconfigure.AutoConfiguration.imports"); - try (PrintWriter writer = new PrintWriter(new FileWriter(importsFile))) { +importsFile, "org.springframework.boot.autoconfigure.AutoConfiguration.imports"); + try (PrintWriter writer = + new PrintWriter(Files.newBufferedWriter(importsFile.toPath(), + StandardCharsets.UTF_8))) {Add
import java.nio.charset.StandardCharsets;.
675-680:File.renameTois non-portable; preferFiles.movewithATOMIC_MOVE
renameTocan silently fail across filesystems. Using NIO provides better reliability and atomicity.- if (!tempJar.renameTo(jarFile)) { - throw new IOException("Could not rename temp JAR to original name!"); - } + try { + Files.move(tempJar.toPath(), jarFile.toPath(), + StandardCopyOption.REPLACE_EXISTING, + StandardCopyOption.ATOMIC_MOVE); + } catch (IOException ex) { + throw new IOException("Could not replace original JAR with temporary copy", ex); + }Remember to add the required imports:
import java.nio.file.Files; import java.nio.file.StandardCopyOption;
683-686: Method name could be clearer
isRootOrClasspathalso accepts any path starting with"BOOT-INF/classes/", not only classpath-root entries. Consider renaming toisRootOrBootClasspath(or similar) to better reflect intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java(11 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
Previously, the code assumed the presence of a manifest in JAR files, leading to a NullPointerException if it was absent. This fix provides a default manifest when none is found, ensuring robust handling of such cases.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java (1)
642-646: Fixed manifest handling to prevent null manifest issues.This implementation correctly handles the case where the source JAR lacks a manifest by creating a new one if needed. This addresses the previous review comment about potential null manifest issues.
🧹 Nitpick comments (2)
src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java (2)
600-684: Consider adding summary logging for property removal.While the implementation correctly removes specified property files, it would be helpful to add summary logging at the end of the method to indicate how many files were removed in total.
if (!tempJar.renameTo(jarFile)) { throw new IOException("Could not rename temp JAR to original name!"); } + log().info("Property files removal completed successfully"); }
545-597: Consider handling directory creation failure.The method doesn't verify if
springMetaInf.mkdirs()succeeds. Add error handling to ensure the directory is created before proceeding.File springMetaInf = new File(outputDir, "META-INF/spring"); if (!springMetaInf.exists()) { - springMetaInf.mkdirs(); + if (!springMetaInf.mkdirs()) { + throw new IOException("Failed to create directory: " + springMetaInf.getAbsolutePath()); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java(11 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
🔇 Additional comments (9)
src/main/java/com/netgrif/maven/plugin/module/BuildModuleMojo.java (9)
51-58: Nice addition of a logging accessor method.Adding a dedicated
log()method provides consistent access to the logger throughout the class and makes future logger implementation changes easier.
135-136: Good addition of the worker profile flag.The
forceEnableWorkerProfileparameter with clear documentation provides a configurable way to control whether worker-specific properties are included in the final JAR.
178-178: Well-placed call to the new property removal method.The
removeProperties()call is appropriately sequenced after Spring factories generation and manifest customization, ensuring all necessary processing completes before file cleanup.
226-267: Well-structured manifest handling with proper error management.The
createJarWithCustomManifest()method has clear execution paths based on thecustomManifestOutputJarflag, good error handling, and appropriate logging.
686-689: Good helper method for path checking.The
isRootOrClasspath()method provides a clean abstraction for checking if a file is at the root level or in the BOOT-INF/classes directory, making the property removal logic more readable.
301-391: Thorough manifest enrichment with project metadata.The
buildCustomManifest()method comprehensively enriches the manifest with various project metadata, enhancing jar identification and traceability. The formatting of developer information is particularly well-handled.
431-457: Well-implemented recursive dependency search.The
findDependencymethod effectively searches through the dependency tree using a clear recursive approach with proper null checking and early returns.
459-478: Clean implementation of dependency flattening.The
flattenDependenciesmethod efficiently collects all dependencies from a hierarchical structure using a recursive approach and proper null safety.
480-509: Well-documented descriptor builder with proper exclusion handling.The
separateDescriptormethod is now better documented and handles exclusions correctly, with appropriate debug logging.
Description
Introduced methods to customize JAR manifests, remove specific properties, and handle dependency trees. Enhanced documentation for added functionalities like Spring factories generation, manifest building, and flattened dependencies. These changes improve JAR handling flexibility and maintain build configurability.
Dependencies
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>
Checklist:
Summary by CodeRabbit
application.propertiesand worker-specific configs) from the built JAR to streamline deployments.