feat: initial implementation of gradle plugin #73 #1005
Conversation
WalkthroughThis pull request introduces a comprehensive set of new classes and functionalities within the Changes
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Outside diff range, codebase verification and nitpick comments (3)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (1)
49-203: Well-structured Gradle task class with clear responsibilities.The
ResolveMainClassNameclass is well-implemented with clear methods for setting and getting properties, and for resolving the main class name. The use of Gradle'sDefaultTaskand properties is appropriate, and the task-specific annotations are correctly applied.
- The method
resolveAndStoreMainClassNameeffectively handles file operations which are crucial for the task's functionality.- The use of
@DisableCachingByDefaultis justified as the task's output is not worth caching due to its dynamic nature based on the project's state.Consider adding error handling for file operations and classpath manipulations to enhance robustness.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java (1)
37-254: Well-structured Gradle task class for creating bootable JAR files.The
ArkJarclass is well-implemented with clear methods for configuring the JAR task and handling file operations. The use of Gradle'sJarclass and theBootArchiveinterface is appropriate, and the task-specific methods are correctly applied.
- The methods
copyandcreateCopyActioneffectively handle the JAR creation process, which is crucial for the task's functionality.- The use of properties like
mainClassandclasspathis well thought out, allowing for dynamic configuration based on the project's state.Consider adding more detailed error handling for file operations and JAR creation to enhance robustness.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (1)
47-221: Comprehensive Gradle task configuration class with robust functionality.The
ArkPluginActionclass is well-implemented with methods that comprehensively configure Gradle tasks and settings for the SofaArk framework. The use of theAction<Project>interface is appropriate, and the methods are well-organized and documented.
- The methods cover a broad range of configurations, from JAR task setup to compiler argument adjustments, which are crucial for the framework's functionality.
- The UTF-8 encoding setup and the handling of compiler arguments are particularly noteworthy for ensuring compatibility and performance.
Consider adding more detailed logging or debugging information to help trace the configuration steps during project builds.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkCopyAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/BootArchive.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/DefaultTimeZoneOffset.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/JarTypeFileSpec.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/LoaderZipEntries.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ZipCompression.java (1 hunks)
Files skipped from review due to trivial changes (1)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ZipCompression.java
Additional comments not posted (12)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/DefaultTimeZoneOffset.java (1)
23-51: Well-implemented utility class for time zone offset handling.The
DefaultTimeZoneOffsetclass is well-designed, using a singleton pattern for easy reuse across the project. The methods are clear and effectively documented, providing robust functionality for time manipulation relative to the default time zone.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/BootArchive.java (4)
32-38: Well-implemented method for retrieving the main class.The use of
@InputandProperty<String>is appropriate and follows Gradle best practices for task inputs.
40-45: Flexible method for specifying unpack requirements.The use of varargs for specifying Ant-style patterns is flexible and user-friendly.
47-52: Enhanced flexibility with spec-based unpacking.Allowing the use of
Spec<FileTreeElement>provides advanced users more control over unpacking conditions.
55-61: Properly annotated method for classpath retrieval.The use of
@Optionaland@Classpathannotations is correct and aligns with Gradle's requirements for task classpaths.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (3)
54-66: Well-implemented static initialization block.The static block correctly initializes the
DEFAULT_LAUNCHER_CLASSESwith an immutable set, which is a good practice for thread safety and to prevent accidental modifications.
86-96: Constructor initializes class fields effectively.The constructor of
ArkArchiveSupportis well-structured and initializes multiple class fields. It also sets up the manifest and parses Git information, which are crucial for the functionality of this class.Consider verifying the robustness of the
JGitParser.parse(gitDic)method in handling different edge cases or potential exceptions.
99-113: Effective use of Java Map methods in manifest configuration.The method
configureManifestusesputIfAbsentandcomputeIfAbsenteffectively to ensure that manifest entries are not inadvertently overwritten. This approach maintains the integrity of existing manifest data while allowing new data to be added conditionally.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkCopyAction.java (4)
115-124: Review of execute MethodThe
executemethod inArkCopyActionclass handles the core functionality of copying actions. Here are some observations:
- Error Handling: The method catches
IOExceptionand wraps it in aGradleException, which is a good practice as it provides a clear message about the failure context.- WorkResult Usage: The method returns a
WorkResultobject, which is appropriate for tasks in Gradle that can either succeed or fail.This method is implemented correctly and follows good error handling practices.
126-149: Review of writeArchive MethodsThere are overloaded
writeArchivemethods designed to handle the writing of the archive with detailed error handling and resource management:
- Resource Management: The use of try-with-resources or finally blocks to ensure streams are closed is commendable.
- Separation of Concerns: The method that accepts an
OutputStreamas a parameter allows for better testing and reusability by decoupling the file writing logic from stream management.These methods are well-implemented with proper resource management and error handling.
151-165: Review of closeQuietly MethodThe
closeQuietlymethod is used to close streams without throwing exceptions. This is a utility method often used to simplify stream handling in finally blocks. However, consider the following:
- Silent Failure: The method catches and ignores
IOException. While this is typical for a closeQuietly method, it's essential to ensure that this silent failure is acceptable in all use cases wherecloseQuietlyis used.This method is generally well-implemented for its intended use, but ensure that ignoring exceptions is acceptable in all cases where this method is used.
183-209: Review of InputStreamEntryWriter ClassThe
InputStreamEntryWriterclass is an implementation of theEntryWriterinterface, designed to write data from anInputStreamto anOutputStream. Observations:
- Resource Management: The class takes care of closing the
InputStreamif thecloseflag is true, which is good practice.- Buffer Size: The buffer size is set to 32KB, which is a reasonable default for file I/O operations.
This class is correctly implemented and follows good practices for I/O operations.
| class JarTypeFileSpec implements Spec<File> { | ||
|
|
||
| private static final Set<String> EXCLUDED_JAR_TYPES = Collections.singleton("dependencies-starter"); | ||
|
|
||
| @Override | ||
| public boolean isSatisfiedBy(File file) { | ||
| try (JarFile jar = new JarFile(file)) { | ||
| String jarType = jar.getManifest().getMainAttributes().getValue("Spring-Boot-Jar-Type"); | ||
| if (jarType != null && EXCLUDED_JAR_TYPES.contains(jarType)) { | ||
| return false; | ||
| } | ||
| } | ||
| catch (Exception ex) { | ||
| // Continue | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Overall implementation looks good, but consider enhancing exception handling.
The class JarTypeFileSpec effectively filters files based on the jar type. However, the exception handling in the isSatisfiedBy method is minimal and could potentially suppress important errors that should be addressed or logged.
Consider logging the exception or rethrowing it as a specific type that could be handled upstream.
| public class SofaArkGradlePlugin implements Plugin<Project> { | ||
|
|
||
| public static final String ARK_VERSION = "2.2.14"; | ||
| public static final String ARK_BIZ_CONFIGURATION_NAME = "bizJar"; | ||
| public static final String ARK_CONFIGURATION_NAME = "arkJar"; | ||
| public static final String ARK_BIZ_TASK_NAME = "bizJar"; | ||
| public static final String ARK_TASK_NAME = "arkJar"; | ||
| public static final String DEVELOPMENT_ONLY_CONFIGURATION_NAME = "developmentOnly"; | ||
| public static final String PRODUCTION_RUNTIME_CLASSPATH_CONFIGURATION_NAME = "productionRuntimeClasspath"; | ||
|
|
||
| @Override | ||
| public void apply(Project project) { | ||
| verifyGradleVersion(); | ||
| createAndSetExtension(project); | ||
| registerPluginActions(project); | ||
| } | ||
|
|
||
| private void verifyGradleVersion() { | ||
| GradleVersion currentVersion = GradleVersion.current(); | ||
| if (currentVersion.compareTo(GradleVersion.version("6.8")) < 0) { | ||
| throw new GradleException("Spring Boot plugin requires Gradle 6.8.+ " | ||
| + "The current version is " + currentVersion); | ||
| } | ||
| } | ||
|
|
||
| private void createAndSetExtension(Project project) { | ||
| SofaArkGradlePluginExtension arkExtension = project.getExtensions().create("arkConfig", SofaArkGradlePluginExtension.class, project); | ||
|
|
||
| } | ||
|
|
||
| private void registerPluginActions(Project project) { | ||
| ArkPluginAction arkAction = new ArkPluginAction(); | ||
| arkAction.execute(project); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Solid implementation with a suggestion for enhanced error handling.
The SofaArkGradlePlugin class is well-structured and effectively sets up the necessary configurations and actions for the SofaArk framework within a Gradle environment. The enforcement of a minimum Gradle version is crucial and well-handled.
Consider enhancing the error message in the verifyGradleVersion method to provide more detailed guidance on how to resolve the version issue, possibly including a link to documentation or support resources.
| /** | ||
| * Sets the classpath to include in the archive. The given {@code classpath} is | ||
| * evaluated as per {@link Project#files(Object...)}. | ||
| * @param classpath the classpath | ||
| * @since 2.0.7 | ||
| */ | ||
| void setClasspath(Object classpath); | ||
|
|
||
| /** | ||
| * Sets the classpath to include in the archive. | ||
| * @param classpath the classpath | ||
| * @since 2.0.7 | ||
| */ | ||
| void setClasspath(FileCollection classpath); |
There was a problem hiding this comment.
Clarify usage of overloaded setClasspath methods.
While the flexibility of overloading the setClasspath method is beneficial, it could lead to confusion. Consider enhancing the documentation or providing usage examples to clarify when to use each variant.
| private void buildArkManifest(){ | ||
| this.arkManifest.getMainAttributes().putValue("Manifest-Version", "1.0"); | ||
| this.arkManifest.getMainAttributes().putValue("Main-Class", this.loaderMainClass); | ||
| this.arkManifest.getMainAttributes().putValue("Start-Class", this.loaderMainClass); | ||
| this.arkManifest.getMainAttributes().putValue("Sofa-Ark-Version",this.arkVersion); | ||
| this.arkManifest.getMainAttributes().putValue("Ark-Container-Root","SOFA-ARK/container/"); | ||
| this.arkManifest.getMainAttributes().putValue("build-time", | ||
| new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ").format(new Date())); | ||
|
|
||
| if (gitInfo != null) { | ||
| this.arkManifest.getMainAttributes().putValue("remote-origin-url", gitInfo.getRepository()); | ||
| this.arkManifest.getMainAttributes().putValue("commit-branch", gitInfo.getBranchName()); | ||
| this.arkManifest.getMainAttributes().putValue("commit-id", gitInfo.getLastCommitId()); | ||
| this.arkManifest.getMainAttributes().putValue("commit-user-name", gitInfo.getLastCommitUser()); | ||
| this.arkManifest.getMainAttributes() | ||
| .putValue("commit-user-email", gitInfo.getLastCommitEmail()); | ||
| this.arkManifest.getMainAttributes().putValue("COMMIT_TIME", gitInfo.getLastCommitDateTime()); | ||
| this.arkManifest.getMainAttributes().putValue("COMMIT_TIMESTAMP", | ||
| String.valueOf(gitInfo.getLastCommitTime())); | ||
| this.arkManifest.getMainAttributes().putValue("build-user", gitInfo.getBuildUser()); | ||
| this.arkManifest.getMainAttributes().putValue("build-email", gitInfo.getBuildEmail()); | ||
| } |
There was a problem hiding this comment.
Comprehensive manifest building with a suggestion for date handling.
The method buildArkManifest effectively includes detailed build and version information in the manifest. However, consider specifying a locale or timezone in SimpleDateFormat to avoid potential inconsistencies in time representation across different environments.
Consider using SimpleDateFormat with a specific timezone setting to ensure consistent date formatting:
new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ", Locale.US).setTimeZone(TimeZone.getTimeZone("UTC"));| public CopyAction createCopyAction(Jar jar) throws IOException { | ||
| return createCopyAction(jar, null); | ||
| } | ||
|
|
||
| public CopyAction createCopyAction(Jar jar, String layerToolsLocation) throws IOException { | ||
| File bizOutput = getTargetFile(jar, this.arkExtension.getBizClassifier().get()); | ||
| File arkOutput = getTargetFile(jar, this.arkExtension.getArkClassifier().get()); | ||
|
|
||
| System.out.println("bizOutput:"+bizOutput.getAbsolutePath()); | ||
| System.out.println("arkOutput:"+arkOutput.getAbsolutePath()); | ||
|
|
||
| Manifest manifest = jar.getManifest(); | ||
| boolean preserveFileTimestamps = jar.isPreserveFileTimestamps(); | ||
| Integer dirMode = getDirMode(jar); | ||
| Integer fileMode = getFileMode(jar); | ||
| boolean includeDefaultLoader = isUsingDefaultLoader(jar); | ||
| Spec<FileTreeElement> requiresUnpack = this.requiresUnpack.getAsSpec(); | ||
| Spec<FileTreeElement> exclusions = this.exclusions.getAsExcludeSpec(); | ||
| Spec<FileCopyDetails> librarySpec = this.librarySpec; | ||
| Function<FileCopyDetails, ZipCompression> compressionResolver = this.compressionResolver; | ||
| String encoding = jar.getMetadataCharset(); | ||
|
|
||
| CopyAction action = new ArkBizCopyAction(bizOutput,arkOutput, manifest, preserveFileTimestamps, dirMode, fileMode, | ||
| includeDefaultLoader, requiresUnpack, exclusions, librarySpec, | ||
| compressionResolver, encoding, this.arkManifest); | ||
|
|
||
|
|
||
| return jar.isReproducibleFileOrder() ? new ReproducibleOrderingCopyAction(action) : action; | ||
| } |
There was a problem hiding this comment.
Complex method handling file copy actions effectively.
The createCopyAction method is well-implemented, handling various configurations and conditions to set up the appropriate CopyAction. Due to its complexity, consider adding more inline comments to explain the logic behind each configuration step, especially around the conditions that affect the choice of CopyAction.
Add more inline comments to improve readability and maintainability of the method, especially explaining the conditions and their impact on the behavior of the method.
| class LoaderZipEntries { | ||
|
|
||
| private final Long entryTime; | ||
|
|
||
| private final int dirMode; | ||
|
|
||
| private final int fileMode; | ||
|
|
||
| LoaderZipEntries(Long entryTime, int dirMode, int fileMode) { | ||
| this.entryTime = entryTime; | ||
| this.dirMode = dirMode; | ||
| this.fileMode = fileMode; | ||
| } | ||
|
|
||
| WrittenEntries writeTo(ZipArchiveOutputStream out) throws IOException { | ||
| WrittenEntries written = new WrittenEntries(); | ||
| try (ZipInputStream loaderJar = new ZipInputStream( | ||
| getClass().getResourceAsStream("/META-INF/loader/spring-boot-loader.jar"))) { | ||
| ZipEntry entry = loaderJar.getNextEntry(); | ||
| while (entry != null) { | ||
| if (entry.isDirectory() && !entry.getName().equals("META-INF/")) { | ||
| writeDirectory(new ZipArchiveEntry(entry), out); | ||
| written.addDirectory(entry); | ||
| } | ||
| else if (entry.getName().endsWith(".class")) { | ||
| writeClass(new ZipArchiveEntry(entry), loaderJar, out); | ||
| written.addFile(entry); | ||
| } | ||
| entry = loaderJar.getNextEntry(); | ||
| } | ||
| } | ||
| return written; | ||
| } | ||
|
|
||
| private void writeDirectory(ZipArchiveEntry entry, ZipArchiveOutputStream out) throws IOException { | ||
| prepareEntry(entry, this.dirMode); | ||
| out.putArchiveEntry(entry); | ||
| out.closeArchiveEntry(); | ||
| } | ||
|
|
||
| private void writeClass(ZipArchiveEntry entry, ZipInputStream in, ZipArchiveOutputStream out) throws IOException { | ||
| prepareEntry(entry, this.fileMode); | ||
| out.putArchiveEntry(entry); | ||
| copy(in, out); | ||
| out.closeArchiveEntry(); | ||
| } | ||
|
|
||
| private void prepareEntry(ZipArchiveEntry entry, int unixMode) { | ||
| if (this.entryTime != null) { | ||
| entry.setTime(DefaultTimeZoneOffset.INSTANCE.removeFrom(this.entryTime)); | ||
| } | ||
| entry.setUnixMode(unixMode); | ||
| } | ||
|
|
||
| private void copy(InputStream in, OutputStream out) throws IOException { | ||
| System.out.println("55555555555555555555555555555"); | ||
| StreamUtils.copy(in, out); | ||
| } | ||
|
|
||
| /** | ||
| * Tracks entries that have been written. | ||
| */ | ||
| static class WrittenEntries { | ||
|
|
||
| private final Set<String> directories = new LinkedHashSet<>(); | ||
|
|
||
| private final Set<String> files = new LinkedHashSet<>(); | ||
|
|
||
| private void addDirectory(ZipEntry entry) { | ||
| this.directories.add(entry.getName()); | ||
| } | ||
|
|
||
| private void addFile(ZipEntry entry) { | ||
| this.files.add(entry.getName()); | ||
| } | ||
|
|
||
| boolean isWrittenDirectory(FileTreeElement element) { | ||
| String path = element.getRelativePath().getPathString(); | ||
| if (element.isDirectory() && !path.endsWith(("/"))) { | ||
| path += "/"; | ||
| } | ||
| return this.directories.contains(path); | ||
| } | ||
|
|
||
| Set<String> getFiles() { | ||
| return this.files; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Review of LoaderZipEntries class
The LoaderZipEntries class is generally well-implemented and uses Java's try-with-resources effectively. However, it contains debug print statements (e.g., line 93) that should be removed for production code. Additionally, the exception handling could be improved to provide more specific error messages or handle specific cases.
Remove debug print statements:
- System.out.println("55555555555555555555555555555");Improve exception handling by adding specific catch blocks or error messages where applicable.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class LoaderZipEntries { | |
| private final Long entryTime; | |
| private final int dirMode; | |
| private final int fileMode; | |
| LoaderZipEntries(Long entryTime, int dirMode, int fileMode) { | |
| this.entryTime = entryTime; | |
| this.dirMode = dirMode; | |
| this.fileMode = fileMode; | |
| } | |
| WrittenEntries writeTo(ZipArchiveOutputStream out) throws IOException { | |
| WrittenEntries written = new WrittenEntries(); | |
| try (ZipInputStream loaderJar = new ZipInputStream( | |
| getClass().getResourceAsStream("/META-INF/loader/spring-boot-loader.jar"))) { | |
| ZipEntry entry = loaderJar.getNextEntry(); | |
| while (entry != null) { | |
| if (entry.isDirectory() && !entry.getName().equals("META-INF/")) { | |
| writeDirectory(new ZipArchiveEntry(entry), out); | |
| written.addDirectory(entry); | |
| } | |
| else if (entry.getName().endsWith(".class")) { | |
| writeClass(new ZipArchiveEntry(entry), loaderJar, out); | |
| written.addFile(entry); | |
| } | |
| entry = loaderJar.getNextEntry(); | |
| } | |
| } | |
| return written; | |
| } | |
| private void writeDirectory(ZipArchiveEntry entry, ZipArchiveOutputStream out) throws IOException { | |
| prepareEntry(entry, this.dirMode); | |
| out.putArchiveEntry(entry); | |
| out.closeArchiveEntry(); | |
| } | |
| private void writeClass(ZipArchiveEntry entry, ZipInputStream in, ZipArchiveOutputStream out) throws IOException { | |
| prepareEntry(entry, this.fileMode); | |
| out.putArchiveEntry(entry); | |
| copy(in, out); | |
| out.closeArchiveEntry(); | |
| } | |
| private void prepareEntry(ZipArchiveEntry entry, int unixMode) { | |
| if (this.entryTime != null) { | |
| entry.setTime(DefaultTimeZoneOffset.INSTANCE.removeFrom(this.entryTime)); | |
| } | |
| entry.setUnixMode(unixMode); | |
| } | |
| private void copy(InputStream in, OutputStream out) throws IOException { | |
| System.out.println("55555555555555555555555555555"); | |
| StreamUtils.copy(in, out); | |
| } | |
| /** | |
| * Tracks entries that have been written. | |
| */ | |
| static class WrittenEntries { | |
| private final Set<String> directories = new LinkedHashSet<>(); | |
| private final Set<String> files = new LinkedHashSet<>(); | |
| private void addDirectory(ZipEntry entry) { | |
| this.directories.add(entry.getName()); | |
| } | |
| private void addFile(ZipEntry entry) { | |
| this.files.add(entry.getName()); | |
| } | |
| boolean isWrittenDirectory(FileTreeElement element) { | |
| String path = element.getRelativePath().getPathString(); | |
| if (element.isDirectory() && !path.endsWith(("/"))) { | |
| path += "/"; | |
| } | |
| return this.directories.contains(path); | |
| } | |
| Set<String> getFiles() { | |
| return this.files; | |
| } | |
| } | |
| } | |
| class LoaderZipEntries { | |
| private final Long entryTime; | |
| private final int dirMode; | |
| private final int fileMode; | |
| LoaderZipEntries(Long entryTime, int dirMode, int fileMode) { | |
| this.entryTime = entryTime; | |
| this.dirMode = dirMode; | |
| this.fileMode = fileMode; | |
| } | |
| WrittenEntries writeTo(ZipArchiveOutputStream out) throws IOException { | |
| WrittenEntries written = new WrittenEntries(); | |
| try (ZipInputStream loaderJar = new ZipInputStream( | |
| getClass().getResourceAsStream("/META-INF/loader/spring-boot-loader.jar"))) { | |
| ZipEntry entry = loaderJar.getNextEntry(); | |
| while (entry != null) { | |
| if (entry.isDirectory() && !entry.getName().equals("META-INF/")) { | |
| writeDirectory(new ZipArchiveEntry(entry), out); | |
| written.addDirectory(entry); | |
| } | |
| else if (entry.getName().endsWith(".class")) { | |
| writeClass(new ZipArchiveEntry(entry), loaderJar, out); | |
| written.addFile(entry); | |
| } | |
| entry = loaderJar.getNextEntry(); | |
| } | |
| } | |
| return written; | |
| } | |
| private void writeDirectory(ZipArchiveEntry entry, ZipArchiveOutputStream out) throws IOException { | |
| prepareEntry(entry, this.dirMode); | |
| out.putArchiveEntry(entry); | |
| out.closeArchiveEntry(); | |
| } | |
| private void writeClass(ZipArchiveEntry entry, ZipInputStream in, ZipArchiveOutputStream out) throws IOException { | |
| prepareEntry(entry, this.fileMode); | |
| out.putArchiveEntry(entry); | |
| copy(in, out); | |
| out.closeArchiveEntry(); | |
| } | |
| private void prepareEntry(ZipArchiveEntry entry, int unixMode) { | |
| if (this.entryTime != null) { | |
| entry.setTime(DefaultTimeZoneOffset.INSTANCE.removeFrom(this.entryTime)); | |
| } | |
| entry.setUnixMode(unixMode); | |
| } | |
| private void copy(InputStream in, OutputStream out) throws IOException { | |
| StreamUtils.copy(in, out); | |
| } | |
| /** | |
| * Tracks entries that have been written. | |
| */ | |
| static class WrittenEntries { | |
| private final Set<String> directories = new LinkedHashSet<>(); | |
| private final Set<String> files = new LinkedHashSet<>(); | |
| private void addDirectory(ZipEntry entry) { | |
| this.directories.add(entry.getName()); | |
| } | |
| private void addFile(ZipEntry entry) { | |
| this.files.add(entry.getName()); | |
| } | |
| boolean isWrittenDirectory(FileTreeElement element) { | |
| String path = element.getRelativePath().getPathString(); | |
| if (element.isDirectory() && !path.endsWith(("/"))) { | |
| path += "/"; | |
| } | |
| return this.directories.contains(path); | |
| } | |
| Set<String> getFiles() { | |
| return this.files; | |
| } | |
| } | |
| } |
| this.out.write(data, 0, data.length); | ||
| this.out.closeArchiveEntry(); | ||
| } | ||
|
|
||
| private void prepareStoredEntry(FileCopyDetails details, ZipArchiveEntry archiveEntry) throws IOException { | ||
| prepareStoredEntry(details.open(), archiveEntry); | ||
| if (ArkBizCopyAction.this.requiresUnpack.isSatisfiedBy(details)) { | ||
| archiveEntry.setComment("UNPACK:" + FileUtils.sha1Hash(details.getFile())); | ||
| } | ||
| } | ||
|
|
||
| private void prepareStoredEntry(InputStream input, ZipArchiveEntry archiveEntry) throws IOException { | ||
| new CrcAndSize(input).setUpStoredEntry(archiveEntry); | ||
| } | ||
|
|
||
| private Long getTime() { | ||
| return getTime(null); | ||
| } | ||
|
|
||
| private Long getTime(FileCopyDetails details) { | ||
| if (!ArkBizCopyAction.this.preserveFileTimestamps) { | ||
| return CONSTANT_TIME_FOR_ZIP_ENTRIES; | ||
| } | ||
| if (details != null) { | ||
| return details.getLastModified(); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private int getDirMode() { | ||
| return (ArkBizCopyAction.this.dirMode != null) ? ArkBizCopyAction.this.dirMode | ||
| : UnixStat.DIR_FLAG | UnixStat.DEFAULT_DIR_PERM; | ||
| } | ||
|
|
||
| private int getFileMode() { | ||
| return (ArkBizCopyAction.this.fileMode != null) ? ArkBizCopyAction.this.fileMode | ||
| : UnixStat.FILE_FLAG | UnixStat.DEFAULT_FILE_PERM; | ||
| } | ||
|
|
||
| private int getFileMode(FileCopyDetails details) { | ||
| return (ArkBizCopyAction.this.fileMode != null) ? ArkBizCopyAction.this.fileMode | ||
| : UnixStat.FILE_FLAG | getPermissions(details); | ||
| } | ||
|
|
||
| private int getPermissions(FileCopyDetails details) { | ||
| if (GradleVersion.current().compareTo(GradleVersion.version("8.3")) >= 0) { | ||
| try { | ||
| Object permissions = details.getClass().getMethod("getPermissions").invoke(details); | ||
| return (int) permissions.getClass().getMethod("toUnixNumeric").invoke(permissions); | ||
| } | ||
| catch (Exception ex) { | ||
| throw new GradleException("Failed to get permissions", ex); | ||
| } | ||
| } | ||
| return details.getMode(); | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Data holder for CRC and Size. | ||
| */ | ||
| private static class CrcAndSize { | ||
|
|
||
| private static final int BUFFER_SIZE = 32 * 1024; | ||
|
|
||
| private final CRC32 crc = new CRC32(); | ||
|
|
||
| private long size; | ||
|
|
||
| CrcAndSize(InputStream inputStream) throws IOException { | ||
| try { | ||
| load(inputStream); | ||
| } | ||
| finally { | ||
| inputStream.close(); | ||
| } | ||
| } | ||
|
|
||
| private void load(InputStream inputStream) throws IOException { | ||
| byte[] buffer = new byte[BUFFER_SIZE]; | ||
| int bytesRead; | ||
| while ((bytesRead = inputStream.read(buffer)) != -1) { | ||
| this.crc.update(buffer, 0, bytesRead); | ||
| this.size += bytesRead; | ||
| } | ||
| } | ||
|
|
||
| void setUpStoredEntry(ZipArchiveEntry entry) { | ||
| entry.setSize(this.size); | ||
| entry.setCompressedSize(this.size); | ||
| entry.setCrc(this.crc.getValue()); | ||
| entry.setMethod(ZipEntry.STORED); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Review of ArkBizCopyAction class
The ArkBizCopyAction class is complex and handles many aspects of file copying and ZIP handling effectively using Java's try-with-resources and functional interfaces. However, it contains debug print statements (e.g., lines 126, 148, 151) that should be removed for production code. The class could also benefit from additional comments or refactoring for better readability and maintainability.
Remove debug print statements:
- System.out.println("``````````````````````````````````````````````````````````````````````````````````");
- System.out.println("111111111111111111111111111111111");
- System.out.println("2222222222222222222222222222");Add more comments or refactor for clarity, especially around complex logic sections.
Committable suggestion was skipped due to low confidence.
| public class ArkCopyAction implements CopyAction { | ||
|
|
||
| static final long CONSTANT_TIME_FOR_ZIP_ENTRIES = OffsetDateTime.of(1980, 2, 1, 0, 0, 0, 0, ZoneOffset.UTC) | ||
| .toInstant() | ||
| .toEpochMilli(); | ||
|
|
||
| private final File output; | ||
|
|
||
| private final Manifest manifest; | ||
|
|
||
| private final boolean preserveFileTimestamps; | ||
|
|
||
| private final Integer dirMode; | ||
|
|
||
| private final Integer fileMode; | ||
|
|
||
| private final boolean includeDefaultLoader; | ||
|
|
||
| private final Spec<FileTreeElement> requiresUnpack; | ||
|
|
||
| private final Spec<FileTreeElement> exclusions; | ||
|
|
||
| private final Spec<FileCopyDetails> librarySpec; | ||
|
|
||
| private final Function<FileCopyDetails, ZipCompression> compressionResolver; | ||
|
|
||
| private final String encoding; | ||
|
|
||
| private final JarOutputStream jarOutput; | ||
|
|
||
| private final File arkOutput; | ||
|
|
||
|
|
||
|
|
||
| ArkCopyAction(File output,File arkOutput, Manifest manifest, boolean preserveFileTimestamps, Integer dirMode, Integer fileMode, | ||
| boolean includeDefaultLoader, Spec<FileTreeElement> requiresUnpack, | ||
| Spec<FileTreeElement> exclusions, Spec<FileCopyDetails> librarySpec, | ||
| Function<FileCopyDetails, ZipCompression> compressionResolver, String encoding | ||
| ) throws IOException { | ||
| this.output = output; | ||
| this.arkOutput = arkOutput; | ||
| this.manifest = manifest; | ||
| this.preserveFileTimestamps = preserveFileTimestamps; | ||
| this.dirMode = dirMode; | ||
| this.fileMode = fileMode; | ||
| this.includeDefaultLoader = includeDefaultLoader; | ||
| this.requiresUnpack = requiresUnpack; | ||
| this.exclusions = exclusions; | ||
| this.librarySpec = librarySpec; | ||
| this.compressionResolver = compressionResolver; | ||
| this.encoding = encoding; | ||
| FileOutputStream fileOutputStream = new FileOutputStream(this.output); | ||
| this.jarOutput = new JarOutputStream(fileOutputStream); | ||
| } |
There was a problem hiding this comment.
Review of ArkCopyAction Constructor
The constructor of ArkCopyAction is well-structured and initializes several fields based on the parameters it receives. However, there are a few points to consider:
- Exception Handling: The constructor throws
IOException, which is appropriate given that it deals with file operations. Ensure that callers of this constructor handle this exception properly. - Parameter Validation: There is no explicit validation of the parameters (e.g., checking if any are
null). Adding null checks could prevent potentialNullPointerExceptionsduring runtime. - Resource Management: The constructor initializes
JarOutputStreamonFileOutputStream. It's crucial to ensure that these streams are closed properly in case of exceptions to avoid resource leaks.
Consider adding parameter validation and enhancing exception handling.
| /** | ||
| * Internal process used to copy {@link FileCopyDetails file details} to the zip file. | ||
| */ | ||
| private class Processor { | ||
|
|
||
| private final ZipArchiveOutputStream out; | ||
|
|
||
| private final JarArchiveOutputStream jarOutput; | ||
|
|
||
| private File arkFile; | ||
|
|
||
|
|
||
| private LoaderZipEntries.WrittenEntries writtenLoaderEntries; | ||
|
|
||
| private final Set<String> writtenDirectories = new LinkedHashSet<>(); | ||
|
|
||
| private final Set<String> writtenLibraries = new LinkedHashSet<>(); | ||
|
|
||
|
|
||
| Processor(ZipArchiveOutputStream out, JarArchiveOutputStream outputStream) throws IOException { | ||
| this.out = out; | ||
| jarOutput = outputStream; | ||
| } | ||
|
|
||
| void process(FileCopyDetails details) { | ||
| if(details.getName().contains("sofa-ark-all")){ | ||
| this.arkFile = details.getFile(); | ||
| return; | ||
| } | ||
| if (skipProcessing(details)) { | ||
| return; | ||
| } | ||
| try { | ||
| //writeLoaderEntriesIfNecessary(details); | ||
| if (details.isDirectory()) { | ||
| processDirectory(details); | ||
| } | ||
| else { | ||
| processFile(details); | ||
| } | ||
| } | ||
| catch (IOException ex) { | ||
| throw new GradleException("Failed to add " + details + " to " + ArkCopyAction.this.output, ex); | ||
| } | ||
| } |
There was a problem hiding this comment.
Review of Processor Class
The Processor class is a crucial component within ArkCopyAction, handling the actual file and directory processing for the archive. Key points:
- Complexity: The class is quite complex and handles multiple responsibilities, including file and directory processing, and special cases like handling
sofa-ark-allfiles. - Error Handling: Proper exception handling is in place, with specific errors wrapped in
GradleExceptionproviding context about what failed.
Consider refactoring to reduce complexity and improve maintainability, possibly by breaking down responsibilities into smaller, more focused methods or classes.
| System.out.println("||"+entry.getName()); | ||
| if (entry.getName().contains("sofa-ark-archive") | ||
| || entry.getName().contains("sofa-ark-spi") | ||
| || entry.getName().contains("sofa-ark-common")) { | ||
|
|
||
| JarInputStream inputStream = new JarInputStream(new BufferedInputStream( | ||
| jarFileSource.getInputStream(entry))); | ||
| writeLoaderClasses(inputStream, jarFileSource); | ||
| } | ||
| } | ||
| } catch (NullPointerException exception){ | ||
| throw new RuntimeException("No sofa-ark-all file find, please configure it"); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| private void writeLoaderClasses(JarInputStream jarInputStream, JarFile jarFileSource) throws IOException { | ||
| JarEntry entry; | ||
| while ((entry = jarInputStream.getNextJarEntry()) != null) { | ||
| if (entry.getName().endsWith(".class") | ||
| && (entry.getName().contains("com/alipay/sofa/ark/spi/archive") | ||
| || entry.getName().contains("com/alipay/sofa/ark/loader") | ||
| || entry.getName().contains("com/alipay/sofa/ark/bootstrap") | ||
| || entry.getName().contains("com/alipay/sofa/ark/common/util/StringUtils") | ||
| || entry.getName().contains("com/alipay/sofa/ark/common/util/AssertUtils") || entry | ||
| .getName().contains("com/alipay/sofa/ark/spi/constant"))) { | ||
|
|
||
|
|
||
| // byte[] buffer = new byte[BUFFER_SIZE]; | ||
| // int bytesRead; | ||
| // while ((bytesRead = this.inputStream.read(buffer)) != -1) { | ||
| // outputStream.write(buffer, 0, bytesRead); | ||
| // } | ||
|
|
||
| ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(entry.getName()); | ||
| prepareEntry(zipArchiveEntry, entry.getName(), getTime(), getFileMode()); | ||
| this.out.putArchiveEntry(zipArchiveEntry); | ||
|
|
||
| byte[] bytes = new byte[1024]; | ||
| int length; | ||
| while ((length = jarInputStream.read(bytes)) >= 0) { | ||
| this.out.write(bytes, 0, length); | ||
| } | ||
| // this.out.write(entry.); | ||
| this.out.closeArchiveEntry(); | ||
|
|
||
|
|
||
| } | ||
| } | ||
| jarInputStream.close(); | ||
| } | ||
|
|
||
| private void writeEntry(JarEntry entry, EntryWriter entryWriter, JarFile jarFileSource) throws IOException { | ||
| System.out.println(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>"+entry.getName()); | ||
|
|
||
| JarArchiveEntry newEntry = new JarArchiveEntry(entry.getName()); | ||
| newEntry.setSize(entry.getSize()); | ||
| newEntry.setTime(entry.getTime()); | ||
|
|
||
| jarOutput.putArchiveEntry(newEntry); | ||
|
|
||
| // | ||
| try (InputStream is = jarFileSource.getInputStream(entry)) { | ||
| IOUtils.copy(is, jarOutput); | ||
| } | ||
|
|
||
| jarOutput.closeArchiveEntry(); | ||
|
|
||
|
|
||
| // String parent = entry.getName(); | ||
| // if (parent.endsWith("/")) { | ||
| // parent = parent.substring(0, parent.length() - 1); | ||
| // } | ||
| // if (parent.lastIndexOf("/") != -1) { | ||
| // parent = parent.substring(0, parent.lastIndexOf("/") + 1); | ||
| // if (parent.length() > 0) { | ||
| // writeEntry(new JarEntry(parent), null); | ||
| // } | ||
| // } | ||
| // | ||
| // | ||
| // this.jarOutput.putNextEntry(entry); | ||
| // if (entryWriter != null) { | ||
| // entryWriter.write(this.jarOutput); | ||
| // } | ||
| // this.jarOutput.closeEntry(); | ||
|
|
||
| } | ||
|
|
||
|
|
||
|
|
||
| private void writeArkBizMark() throws IOException { | ||
| String str = "a mark file included in sofa-ark module."; | ||
| String name = "com/alipay/sofa/ark/biz/mark"; | ||
| ZipArchiveEntry entry = new ZipArchiveEntry(name); | ||
| prepareEntry(entry, name, getTime(), getFileMode()); | ||
| this.out.putArchiveEntry(entry); | ||
| this.out.write(str.getBytes()); | ||
| this.out.closeArchiveEntry(); | ||
| } |
There was a problem hiding this comment.
Review of writeArkBizMark Method
The writeArkBizMark method adds a specific entry to the archive, which seems to be a marker for the sofa-ark module. Observations:
- Hardcoded Values: The method contains hardcoded strings for both the entry name and the content. These could be extracted as constants or configurable properties to enhance flexibility and maintainability.
- Single Responsibility: The method is focused and handles only one specific task, which is good for maintainability.
Consider extracting hardcoded values to constants or configuration settings.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkCopyAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/LoaderZipEntries.java (1 hunks)
Files skipped from review due to trivial changes (1)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java
Files skipped from review as they are similar to previous changes (4)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkCopyAction.java
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/LoaderZipEntries.java
There was a problem hiding this comment.
Actionable comments posted: 31
🧹 Outside diff range and nitpick comments (14)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/Nullable.java (1)
28-34: LGTM: Annotation declaration and meta-annotations are well-defined.The
Nullableannotation is correctly declared with appropriate meta-annotations. It's designed to be used on methods, parameters, and fields, with runtime retention for reflection access. The use of@Nonnull(when = When.MAYBE)effectively indicates that the annotated element might be non-null under certain conditions.Consider adding a brief Javadoc comment to explain the purpose and usage of this annotation. For example:
/** * Indicates that the annotated element can be null under some circumstances. * This annotation can be used on methods, parameters, and fields to provide * nullability information for static analysis tools and to improve code readability. */ @Target({ElementType.METHOD, ElementType.PARAMETER, ElementType.FIELD}) // ... (rest of the annotations) public @interface Nullable { }sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1)
53-54: Remove unused variable or utilize the extension instance.The variable
arkExtensionis assigned but not used afterward. If you don't need to reference the extension instance in your plugin code, you can omit the variable assignment.Apply this diff to simplify the code:
- SofaArkGradlePluginExtension arkExtension = project.getExtensions().create("arkConfig", SofaArkGradlePluginExtension.class, project); + project.getExtensions().create("arkConfig", SofaArkGradlePluginExtension.class, project);sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (3)
49-50: Re-evaluate the decision to disable task cachingThe
@DisableCachingByDefault(because = "Not worth caching")annotation disables build caching for theResolveMainClassNametask. While this might be appropriate for tasks with negligible execution time, enabling caching can improve build performance in larger projects or when the task's outputs are reused.Consider whether the benefits of caching outweigh the overhead in your specific use case.
176-182: EnsureJavaApplicationplugin is applied before accessing its propertiesIn the
getJavaApplicationMainClassmethod, you attempt to obtain theJavaApplicationextension from the project. If the Java Application plugin is not applied, this method will returnnull.To prevent unexpected
nullvalues, consider checking if the plugin is applied:if (!project.getPlugins().hasPlugin("application")) { return null; }Alternatively, document that the Java Application plugin must be applied for this functionality.
52-52: Clarify the constant's purpose with an explanatory commentThe constant
SPRING_BOOT_APPLICATION_CLASS_NAMEholds the class name for Spring Boot's@SpringBootApplication. Adding a comment explaining its use can improve code readability.For example:
// Class name of Spring Boot's @SpringBootApplication to assist in main class detection private static final String SPRING_BOOT_APPLICATION_CLASS_NAME = "org.springframework.boot.autoconfigure.SpringBootApplication";sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java (1)
218-230: Simplify thefromCallToandcallTomethodsThe helper methods
fromCallToandcallToadd extra layers of abstraction, which might reduce code readability. Consider replacing them with lambda expressions for clarity.Apply this diff to simplify the code:
private void configureBootInfSpec(CopySpec bootInfSpec) { - bootInfSpec.into("", fromCallTo(this::classpathDirectories)); + bootInfSpec.into("", spec -> spec.from(this.classpathDirectories())); - bootInfSpec.into("lib", fromCallTo(this::classpathFiles)).eachFile(this.support::excludeNonZipFiles); + bootInfSpec.into("lib", spec -> spec.from(this.classpathFiles())).eachFile(this.support::excludeNonZipFiles); } - private static <T> Action<CopySpec> fromCallTo(Callable<T> callable) { - return (spec) -> spec.from(callTo(callable)); - } - private static <T> Callable<T> callTo(Callable<T> callable) { - return callable; - }This change reduces indirection and may improve readability.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (3)
220-225: RenameisTruemethod toisNonEmptyStringfor clarityThe
isTruemethod at lines 220-225 checks if an object is a non-emptyString. The method nameisTruemight be misleading, as it suggests a general truth value check. Consider renaming the method toisNonEmptyStringto clearly convey its purpose.Apply this diff to rename the method:
- private Boolean isTrue(Object object){ + private Boolean isNonEmptyString(Object object){ if (object instanceof String) { return !((String) object).isEmpty(); } return false; }
227-233: Enhance themaybemethod for better readabilityThe
maybemethod concatenates strings with a hyphen, which can be unclear. To improve readability, consider renaming the method toappendWithHyphenIfNotEmptyand adding documentation to explain its behavior.Apply this diff to rename the method and add documentation:
+ /** + * Appends the value to the prefix with a hyphen if the value is not empty. + * + * @param prefix the current string + * @param value the value to append + * @return the combined string + */ - private String maybe(String prefix, String value) { + private String appendWithHyphenIfNotEmpty(String prefix, String value) { if (isNonEmptyString(value)) { return isNonEmptyString(prefix) ? "-".concat(value) : value; } else { return ""; } }
246-256: Handle exceptions more specifically ingetModemethodIn the
getModemethod, a generalExceptionis caught and rethrown as aGradleException. Catching broad exceptions can obscure the root cause of issues. Consider catching more specific exceptions, such asNoSuchMethodExceptionandIllegalAccessException, to improve error handling and debugging.Apply this diff to catch specific exceptions:
try { Object filePermissions = ((Property<Object>) copySpec.getClass().getMethod(methodName).invoke(copySpec)) .getOrNull(); return (filePermissions != null) ? (int) filePermissions.getClass().getMethod("toUnixNumeric").invoke(filePermissions) : null; } - catch (Exception ex) { + catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException ex) { throw new GradleException("Failed to get permissions", ex); }sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/MainClassFinder.java (3)
360-361: UseLinkedHashSetto maintain annotation orderCurrently, annotations are stored in a
HashSet, which does not preserve the order of insertion. If the order of annotations is significant, consider using aLinkedHashSetto maintain the order.Update the constructor in
MainClassas follows:- this.annotationNames = Collections.unmodifiableSet(new HashSet<>(annotationNames)); + this.annotationNames = Collections.unmodifiableSet(new LinkedHashSet<>(annotationNames));
251-261: Log exceptions increateClassDescriptorfor better diagnosticsIn the
createClassDescriptormethod, if anIOExceptionoccurs, it is caught and ignored, returningnull. This can make debugging difficult if classes fail to load.Consider logging the exception using a proper logging framework:
catch (IOException ex) { + // Log the exception using the appropriate logging framework + logger.warn("Failed to read class descriptor", ex); return null; }Ensure that a logger (e.g.,
private static final Logger logger = LoggerFactory.getLogger(MainClassFinder.class);) is available in the class.
289-289: Update ASM API version to support newer Java class filesThe code uses
Opcodes.ASM7, which supports class files up to Java 11. To ensure compatibility with newer Java versions (Java 12 and above), consider updating toOpcodes.ASM9or the latest ASM version.Modify the super constructor to use the latest ASM API version:
- super(Opcodes.ASM7); + super(Opcodes.ASM9);Ensure that the ASM library dependency is updated to the corresponding version that supports
Opcodes.ASM9.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1)
263-265: Remove or implement the emptywriteConfigmethodThe
writeConfigmethod is currently empty, which may lead to confusion.If this method is intended for future use, consider adding a
// TODOcomment to indicate that implementation is pending:// TODO: Implement the writeConfig methodOtherwise, if it's unnecessary, consider removing it to keep the code clean.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/LoaderZipEntries.java (1)
117-117: Simplify condition by removing unnecessary parenthesesIn the condition
!path.endsWith(("/")), the parentheses around the string"/"are unnecessary. Removing them enhances code readability.Apply this diff:
- if (element.isDirectory() && !path.endsWith(("/"))) { + if (element.isDirectory() && !path.endsWith("/")) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
- sofa-ark-parent/support/ark-gradle-plugin/build.gradle (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/settings.gradle (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/LoaderZipEntries.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/MainClassFinder.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/Nullable.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/StringUtils.java (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- sofa-ark-parent/support/ark-gradle-plugin/build.gradle
- sofa-ark-parent/support/ark-gradle-plugin/settings.gradle
🧰 Additional context used
🔇 Additional comments (14)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/Nullable.java (3)
1-17: LGTM: License header and package declaration are correct.The Apache License 2.0 header is properly included, and the package declaration follows Java naming conventions.
19-26: LGTM: Import statements are appropriate and well-organized.All imported classes are used in the annotation declaration, and there are no unused or redundant imports. The use of
javax.annotationclasses indicates good integration with existing annotation processing tools.
1-34: Overall assessment: Well-implementedNullableannotation.This new
Nullableannotation is a valuable addition to the SofaArk framework. It follows Java annotation best practices and integrates well with existing annotation processing tools. The annotation will be useful for static analysis and improving code readability by providing explicit nullability information.The implementation is clean, concise, and correctly structured. With the suggested minor documentation improvement, this file will be in excellent shape.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/StringUtils.java (3)
3-7: LGTM: Import statements are appropriate and concise.The import statements are well-organized and include only the necessary classes for the functionality provided in this utility class.
23-25: LGTM: Concise and well-implemented convenience method.The
collectionToCommaDelimitedStringmethod is a clear and useful wrapper for the more generalcollectionToDelimitedStringmethod. The use of the@Nullableannotation is a good practice for null safety.
27-29: LGTM: Well-designed overloaded method with sensible defaults.This overload of
collectionToDelimitedStringprovides a good balance between flexibility and convenience, allowing users to specify a delimiter while using sensible defaults for prefix and suffix.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (2)
29-39: Consider making constantsstaticThe constants defined between lines 29-39 should be made
staticsince they do not depend on instance state. This change would improve memory efficiency for the class.
58-90: Code implementation is correct and follows best practicesThe methods and properties are well-defined, adhering to Gradle plugin development conventions. The use of abstract methods for property definitions allows for flexible configuration in subclasses.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (1)
126-138: Handle the case when no main class is foundIn the
resolveMainClassNamemethod, if no main class is found, the method returns an empty string (""). Depending on how this value is used later, it might be more appropriate to returnnullor throw an exception to explicitly indicate that the main class could not be resolved.Consider updating the method to reflect this:
- Return
nullif no main class is found.- Or, throw an exception with a clear message.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (4)
157-177: Clarify Configuration Attributes for Development and Runtime ClasspathsThe method
configureDevelopmentOnlyConfigurationsets up configurations for development-only and production runtime classpaths. Ensure that the attributes are set correctly to avoid unexpected behavior during dependency resolution.Please double-check the configuration attributes and their implications on dependency resolution.
125-132: Ensuring UTF-8 Encoding in CompilationThe
configureUtf8Encodingmethod sets the encoding to UTF-8 if not already specified. This is good practice to ensure consistent character encoding.
134-141: Adding Compiler Argument for Method Parameter NamesThe
configureParametersCompilerArgmethod adds the-parameterscompiler argument to retain parameter names at runtime. This is beneficial for frameworks that rely on reflection.
97-98:⚠️ Potential issueMissing Import for
ResolveMainClassNameThe class
ResolveMainClassNameis used but not imported. This will cause a compilation issue.Add the appropriate import statement for
ResolveMainClassName:+ import org.springframework.boot.gradle.tasks.application.ResolveMainClassName;Ensure that
ResolveMainClassNameis available in your project's dependencies or replace it with the correct class.Likely invalid or redundant comment.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1)
247-250: Ensure proper closure ofFileInputStreaminwriteBizJarWhile
try-with-resourcesis used, confirm that theFileInputStreamis correctly closed to prevent resource leaks.Run the following script to check for any unclosed streams:
If the script returns any matches, ensure those streams are properly managed.
| static class CollectionUtils { | ||
| public static boolean isEmpty(@Nullable Collection<?> collection) { | ||
| return (collection == null || collection.isEmpty()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider moving isEmpty method to the main class.
The isEmpty method is well-implemented and uses the @Nullable annotation correctly. However, nesting it within a static CollectionUtils class might be unnecessary and could lead to confusion.
Consider moving the isEmpty method directly into the StringUtils class:
- static class CollectionUtils {
- public static boolean isEmpty(@Nullable Collection<?> collection) {
- return (collection == null || collection.isEmpty());
- }
- }
+ public static boolean isEmpty(@Nullable Collection<?> collection) {
+ return (collection == null || collection.isEmpty());
+ }This change would simplify the class structure and make the method more directly accessible. If you prefer to keep collection-related utilities separate, consider creating a standalone CollectionUtils class instead of nesting it within StringUtils.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static class CollectionUtils { | |
| public static boolean isEmpty(@Nullable Collection<?> collection) { | |
| return (collection == null || collection.isEmpty()); | |
| } | |
| } | |
| public static boolean isEmpty(@Nullable Collection<?> collection) { | |
| return (collection == null || collection.isEmpty()); | |
| } |
| static int copyTo(InputStream in, OutputStream out) throws IOException { | ||
| int byteCount = 0; | ||
|
|
||
| int bytesRead; | ||
| for(byte[] buffer = new byte[4096]; (bytesRead = in.read(buffer)) != -1; byteCount += bytesRead) { | ||
| out.write(buffer, 0, bytesRead); | ||
| } | ||
|
|
||
| out.flush(); | ||
| return byteCount; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving robustness and clarify method visibility.
The copyTo method efficiently copies data between streams. However, consider the following improvements:
- Add null checks for input parameters to prevent NullPointerExceptions.
- The method is package-private. Is this intentional, or should it be public for wider use?
Here's a suggested improvement:
- static int copyTo(InputStream in, OutputStream out) throws IOException {
+ public static int copyTo(InputStream in, OutputStream out) throws IOException {
+ if (in == null || out == null) {
+ throw new IllegalArgumentException("Input and output streams cannot be null");
+ }
int byteCount = 0;
int bytesRead;
for(byte[] buffer = new byte[4096]; (bytesRead = in.read(buffer)) != -1; byteCount += bytesRead) {
out.write(buffer, 0, bytesRead);
}
out.flush();
return byteCount;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static int copyTo(InputStream in, OutputStream out) throws IOException { | |
| int byteCount = 0; | |
| int bytesRead; | |
| for(byte[] buffer = new byte[4096]; (bytesRead = in.read(buffer)) != -1; byteCount += bytesRead) { | |
| out.write(buffer, 0, bytesRead); | |
| } | |
| out.flush(); | |
| return byteCount; | |
| } | |
| public static int copyTo(InputStream in, OutputStream out) throws IOException { | |
| if (in == null || out == null) { | |
| throw new IllegalArgumentException("Input and output streams cannot be null"); | |
| } | |
| int byteCount = 0; | |
| int bytesRead; | |
| for(byte[] buffer = new byte[4096]; (bytesRead = in.read(buffer)) != -1; byteCount += bytesRead) { | |
| out.write(buffer, 0, bytesRead); | |
| } | |
| out.flush(); | |
| return byteCount; | |
| } |
| public static String collectionToDelimitedString( | ||
| @Nullable Collection<?> coll, String delim, String prefix, String suffix) { | ||
|
|
||
| if (CollectionUtils.isEmpty(coll)) { | ||
| return ""; | ||
| } | ||
|
|
||
| int totalLength = coll.size() * (prefix.length() + suffix.length()) + (coll.size() - 1) * delim.length(); | ||
| for (Object element : coll) { | ||
| totalLength += String.valueOf(element).length(); | ||
| } | ||
|
|
||
| StringBuilder sb = new StringBuilder(totalLength); | ||
| Iterator<?> it = coll.iterator(); | ||
| while (it.hasNext()) { | ||
| sb.append(prefix).append(it.next()).append(suffix); | ||
| if (it.hasNext()) { | ||
| sb.append(delim); | ||
| } | ||
| } | ||
| return sb.toString(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
LGTM: Efficient and well-implemented string conversion method.
The collectionToDelimitedString method is well-designed and efficiently implemented. It handles edge cases, uses StringBuilder for performance, and delegates null checks to a utility method.
A minor optimization suggestion:
Consider using StringBuilder.append(CharSequence, int, int) to avoid creating substring objects:
StringBuilder sb = new StringBuilder(totalLength);
Iterator<?> it = coll.iterator();
while (it.hasNext()) {
- sb.append(prefix).append(it.next()).append(suffix);
+ sb.append(prefix);
+ String element = String.valueOf(it.next());
+ sb.append(element, 0, element.length());
+ sb.append(suffix);
if (it.hasNext()) {
sb.append(delim);
}
}This change can potentially reduce object creation and improve performance, especially for large collections.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static String collectionToDelimitedString( | |
| @Nullable Collection<?> coll, String delim, String prefix, String suffix) { | |
| if (CollectionUtils.isEmpty(coll)) { | |
| return ""; | |
| } | |
| int totalLength = coll.size() * (prefix.length() + suffix.length()) + (coll.size() - 1) * delim.length(); | |
| for (Object element : coll) { | |
| totalLength += String.valueOf(element).length(); | |
| } | |
| StringBuilder sb = new StringBuilder(totalLength); | |
| Iterator<?> it = coll.iterator(); | |
| while (it.hasNext()) { | |
| sb.append(prefix).append(it.next()).append(suffix); | |
| if (it.hasNext()) { | |
| sb.append(delim); | |
| } | |
| } | |
| return sb.toString(); | |
| } | |
| public static String collectionToDelimitedString( | |
| @Nullable Collection<?> coll, String delim, String prefix, String suffix) { | |
| if (CollectionUtils.isEmpty(coll)) { | |
| return ""; | |
| } | |
| int totalLength = coll.size() * (prefix.length() + suffix.length()) + (coll.size() - 1) * delim.length(); | |
| for (Object element : coll) { | |
| totalLength += String.valueOf(element).length(); | |
| } | |
| StringBuilder sb = new StringBuilder(totalLength); | |
| Iterator<?> it = coll.iterator(); | |
| while (it.hasNext()) { | |
| sb.append(prefix); | |
| String element = String.valueOf(it.next()); | |
| sb.append(element, 0, element.length()); | |
| sb.append(suffix); | |
| if (it.hasNext()) { | |
| sb.append(delim); | |
| } | |
| } | |
| return sb.toString(); | |
| } |
| throw new GradleException("Spring Boot plugin requires Gradle 6.8.+ " | ||
| + "The current version is " + currentVersion); |
There was a problem hiding this comment.
Correct the exception message to reference the Sofa Ark Gradle plugin.
The exception message currently refers to the "Spring Boot plugin," which is incorrect. It should reference the "Sofa Ark Gradle plugin" to accurately reflect the context.
Apply this diff to fix the error message:
- throw new GradleException("Spring Boot plugin requires Gradle 6.8.+ "
+ throw new GradleException("Sofa Ark Gradle plugin requires Gradle 6.8.+ "📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw new GradleException("Spring Boot plugin requires Gradle 6.8.+ " | |
| + "The current version is " + currentVersion); | |
| throw new GradleException("Sofa Ark Gradle plugin requires Gradle 6.8.+ " | |
| + "The current version is " + currentVersion); |
| @TaskAction | ||
| void resolveAndStoreMainClassName() throws IOException { | ||
| File outputFile = this.outputFile.getAsFile().get(); | ||
| outputFile.getParentFile().mkdirs(); |
There was a problem hiding this comment.
Check the result of mkdirs() to handle potential directory creation failures
The call to outputFile.getParentFile().mkdirs(); creates the parent directories if they do not exist. However, if directory creation fails due to permission issues or other I/O errors, the subsequent file write operation could throw an exception.
Consider checking the return value of mkdirs() and handling any failures appropriately, possibly by throwing an exception with a clear error message.
| private void writeBootstrapEntry() throws IOException { | ||
| try (JarFile jarFileSource = new JarFile(this.arkFile)){ | ||
| Enumeration<JarEntry> entries = jarFileSource.entries(); | ||
| while (entries.hasMoreElements()) { | ||
| JarEntry entry = entries.nextElement(); | ||
| if (entry.getName().contains("sofa-ark-archive") | ||
| || entry.getName().contains("sofa-ark-spi") | ||
| || entry.getName().contains("sofa-ark-common")) { | ||
|
|
||
| JarInputStream inputStream = new JarInputStream(new BufferedInputStream( | ||
| jarFileSource.getInputStream(entry))); | ||
| writeLoaderClasses(inputStream, jarFileSource); | ||
| } | ||
| } | ||
| } catch (NullPointerException exception){ | ||
| throw new RuntimeException("No sofa-ark-all file find, please configure it"); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor writeBootstrapEntry for efficiency and clarity
The logic within writeBootstrapEntry can be optimized for better performance and readability.
Consider the following improvements:
- Combine conditions or use a collection to manage the entry names to check.
- Avoid nested streams if possible.
- Ensure all resources are properly closed.
Here's an example of refactored code:
private void writeBootstrapEntry() throws IOException {
List<String> entryNames = Arrays.asList(
"sofa-ark-archive",
"sofa-ark-spi",
"sofa-ark-common"
);
try (JarFile jarFileSource = new JarFile(this.arkFile)) {
Enumeration<JarEntry> entries = jarFileSource.entries();
while (entries.hasMoreElements()) {
JarEntry entry = entries.nextElement();
if (entryNames.stream().anyMatch(entry.getName()::contains)) {
try (JarInputStream inputStream = new JarInputStream(
new BufferedInputStream(jarFileSource.getInputStream(entry)))) {
writeLoaderClasses(inputStream, jarFileSource);
}
}
}
} catch (IOException exception) {
throw new RuntimeException("Error processing ark file", exception);
}
}| try (JarFile jarFileSource = new JarFile(this.arkFile)){ | ||
| Enumeration<JarEntry> entries = jarFileSource.entries(); | ||
| while (entries.hasMoreElements()) { | ||
| JarEntry entry = entries.nextElement(); | ||
| if (entry.getName().contains("sofa-ark-archive") | ||
| || entry.getName().contains("sofa-ark-spi") | ||
| || entry.getName().contains("sofa-ark-common")) { | ||
|
|
||
| JarInputStream inputStream = new JarInputStream(new BufferedInputStream( | ||
| jarFileSource.getInputStream(entry))); | ||
| writeLoaderClasses(inputStream, jarFileSource); | ||
| } | ||
| } | ||
| } catch (NullPointerException exception){ | ||
| throw new RuntimeException("No sofa-ark-all file find, please configure it"); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Improve resource handling in writeBootstrapEntry
In the writeBootstrapEntry method, the JarInputStream is opened inside a loop but may not be properly closed for each iteration, which can lead to resource leaks.
Ensure that JarInputStream is closed after processing each entry. Modify the code as follows:
if (entry.getName().contains("sofa-ark-archive")
|| entry.getName().contains("sofa-ark-spi")
|| entry.getName().contains("sofa-ark-common")) {
- JarInputStream inputStream = new JarInputStream(new BufferedInputStream(
+ try (JarInputStream inputStream = new JarInputStream(new BufferedInputStream(
jarFileSource.getInputStream(entry)))) {
writeLoaderClasses(inputStream, jarFileSource);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try (JarFile jarFileSource = new JarFile(this.arkFile)){ | |
| Enumeration<JarEntry> entries = jarFileSource.entries(); | |
| while (entries.hasMoreElements()) { | |
| JarEntry entry = entries.nextElement(); | |
| if (entry.getName().contains("sofa-ark-archive") | |
| || entry.getName().contains("sofa-ark-spi") | |
| || entry.getName().contains("sofa-ark-common")) { | |
| JarInputStream inputStream = new JarInputStream(new BufferedInputStream( | |
| jarFileSource.getInputStream(entry))); | |
| writeLoaderClasses(inputStream, jarFileSource); | |
| } | |
| } | |
| } catch (NullPointerException exception){ | |
| throw new RuntimeException("No sofa-ark-all file find, please configure it"); | |
| } | |
| } | |
| try (JarFile jarFileSource = new JarFile(this.arkFile)){ | |
| Enumeration<JarEntry> entries = jarFileSource.entries(); | |
| while (entries.hasMoreElements()) { | |
| JarEntry entry = entries.nextElement(); | |
| if (entry.getName().contains("sofa-ark-archive") | |
| || entry.getName().contains("sofa-ark-spi") | |
| || entry.getName().contains("sofa-ark-common")) { | |
| try (JarInputStream inputStream = new JarInputStream(new BufferedInputStream( | |
| jarFileSource.getInputStream(entry)))) { | |
| writeLoaderClasses(inputStream, jarFileSource); | |
| } | |
| } | |
| } | |
| } catch (NullPointerException exception){ | |
| throw new RuntimeException("No sofa-ark-all file find, please configure it"); | |
| } | |
| } |
| if (GradleVersion.current().compareTo(GradleVersion.version("8.3")) >= 0) { | ||
| try { | ||
| Object permissions = details.getClass().getMethod("getPermissions").invoke(details); | ||
| return (int) permissions.getClass().getMethod("toUnixNumeric").invoke(permissions); | ||
| } | ||
| catch (Exception ex) { | ||
| throw new GradleException("Failed to get permissions", ex); | ||
| } | ||
| } | ||
| return details.getMode(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential reflection exceptions and ensure compatibility
The code uses reflection to access getPermissions and toUnixNumeric methods based on the Gradle version. This approach might fail if the methods change in future Gradle releases.
Consider handling possible exceptions more gracefully and providing a fallback:
try {
Object permissions = details.getClass().getMethod("getPermissions").invoke(details);
return (int) permissions.getClass().getMethod("toUnixNumeric").invoke(permissions);
}
catch (Exception ex) {
- throw new GradleException("Failed to get permissions", ex);
+ // Fall back to default file permissions if reflection fails
+ return UnixStat.FILE_FLAG | UnixStat.DEFAULT_FILE_PERM;
}Additionally, consider checking method availability to ensure compatibility with future Gradle versions.
Committable suggestion was skipped due to low confidence.
| } | ||
|
|
||
| private void copy(InputStream in, OutputStream out) throws IOException { | ||
| StringUtils.copyTo(in, out); |
There was a problem hiding this comment.
Use appropriate utility for copying streams and ensure necessary imports
The method StringUtils.copyTo(in, out); may not be suitable for copying data between streams. Consider using IOUtils.copy(in, out); from Apache Commons IO for this purpose. Additionally, ensure that the IOUtils class is properly imported.
Apply this diff to update the copy method and import statement:
+import org.apache.commons.io.IOUtils;
...
private void copy(InputStream in, OutputStream out) throws IOException {
- StringUtils.copyTo(in, out);
+ IOUtils.copy(in, out);
}Committable suggestion was skipped due to low confidence.
| try (ZipInputStream loaderJar = new ZipInputStream( | ||
| getClass().getResourceAsStream("/META-INF/loader/spring-boot-loader.jar"))) { | ||
| ZipEntry entry = loaderJar.getNextEntry(); |
There was a problem hiding this comment.
Handle potential NullPointerException when resource is missing
If the resource /META-INF/loader/spring-boot-loader.jar is not found, getResourceAsStream returns null, leading to a NullPointerException when initializing ZipInputStream. It's important to check for null and handle this scenario appropriately to prevent runtime exceptions.
Apply this diff to handle the potential null InputStream:
try (InputStream resourceStream = getClass().getResourceAsStream("/META-INF/loader/spring-boot-loader.jar")) {
+ if (resourceStream == null) {
+ throw new IOException("Resource /META-INF/loader/spring-boot-loader.jar not found.");
+ }
try (ZipInputStream loaderJar = new ZipInputStream(resourceStream)) {
ZipEntry entry = loaderJar.getNextEntry();
while (entry != null) {
// existing logic
entry = loaderJar.getNextEntry();
}
}
}Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Outside diff range and nitpick comments (3)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1)
26-26: Add class-level documentation.Since this is the main entry point for the Gradle plugin, consider adding comprehensive JavaDoc documentation that includes:
- Purpose and responsibilities of the plugin
- Configuration options available through arkConfig
- Example usage in build.gradle
+/** + * Gradle plugin for SofaArk framework that enables building Biz and Ark packages. + * <p> + * Usage in build.gradle: + * <pre> + * plugins { + * id 'com.alipay.sofa.ark' + * } + * + * arkConfig { + * // configuration options + * } + * </pre> + */ public class SofaArkGradlePlugin implements Plugin<Project> {sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1)
Line range hint
27-102: Add Javadoc comments to public methods and the classIncluding Javadoc comments for the class and its public methods enhances code readability and provides valuable documentation for users and maintainers. This is especially important for plugin extensions, as other developers may need to understand how to interact with these properties and what each configuration option does.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1)
423-430: Enhance the content of the mark file for clarityThe mark file written in
writeArkBizMark()contains a generic message. Providing more specific information can be helpful for future maintenance and understanding the purpose of the file.Consider including details such as version information or a descriptive message:
private void writeArkBizMark() throws IOException { - String info = "a mark file included in sofa-ark module."; + String info = "This is a marker file indicating the presence of a SOFA-Ark business module."; String name = "com/alipay/sofa/ark/biz/mark"; ZipArchiveEntry entry = new ZipArchiveEntry(name); prepareEntry(entry, name, getTime(), getFileMode()); this.out.putArchiveEntry(entry); byte[] data = info.getBytes(StandardCharsets.UTF_8); this.out.write(data, 0, data.length); this.out.closeArchiveEntry(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1 hunks)
🧰 Additional context used
🔇 Additional comments (9)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (2)
23-23: Remove unused internal Gradle import.The import of
org.gradle.internal.impldep.org.bouncycastle.pqc.crypto.newhope.NHSecretKeyProcessor.PartyUBuilderappears to be unused and references an internal Gradle class.
44-45: Fix the plugin name in error message.The error message incorrectly references "Spring Boot plugin" instead of "Sofa Ark Gradle plugin".
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (5)
90-90: Inconsistent Task Name ReferenceThe assemble task depends on
"arkJar", but the task is registered with the nameSofaArkGradlePlugin.ARK_BIZ_TASK_NAME. This inconsistency can cause aTaskNotFoundException.
176-179: Optimize Configuration TimingConfiguring additional metadata locations after project evaluation may be inefficient.
232-237: Hard-Coded Reference tospring-boot-configuration-processorThe method
hasConfigurationProcessorOnClasspathspecifically checks forspring-boot-configuration-processor, which might not be appropriate for a Sofa Ark plugin.
242-243: Missing Import forStringUtils
StringUtils.collectionToCommaDelimitedStringis used without importingStringUtils, resulting in a compilation error.
107-107:⚠️ Potential issueUndefined Class
JarTypeFileSpec
JarTypeFileSpecused in.filter(new JarTypeFileSpec())is not defined or imported, leading to a compilation error.Ensure
JarTypeFileSpecis defined or imported. If it's part of your project, add the import statement:+ import com.alipay.sofa.ark.plugin.utils.JarTypeFileSpec;If it doesn't exist, implement it or replace it with an appropriate alternative.
Likely invalid or redundant comment.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (2)
191-194: Confirm Accurate Spring Boot Version RetrievalThe
determineSpringBootVersionmethod uses the implementation version of the current class's package, which may not accurately reflect the Spring Boot version if not set.Consider specifying the Spring Boot version explicitly or verifying that the implementation version is correctly populated during the build process.
266-279:⚠️ Potential issueEnsure Compatibility with Gradle Versions Below 8.3
The method
getModeuses reflection to access methods that may not exist in Gradle versions below 8.3. This could lead to runtime exceptions in environments using older Gradle versions.Run the following script to verify compatibility with different Gradle versions:
Ensure that your build script or plugin accounts for environments with older Gradle versions.
|
|
||
| public class SofaArkGradlePlugin implements Plugin<Project> { | ||
|
|
||
| public static final String ARK_VERSION = "2.2.14"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider externalizing the ARK_VERSION constant.
Hardcoding the version number makes it difficult to maintain and update. Consider moving this to a gradle.properties file or using version catalogs for better version management.
Example using gradle.properties:
- public static final String ARK_VERSION = "2.2.14";
+ public static final String ARK_VERSION = project.findProperty("sofaArkVersion") ?: "2.2.14";Committable suggestion was skipped due to low confidence.
| private void registerPluginActions(Project project) { | ||
| ArkPluginAction arkAction = new ArkPluginAction(); | ||
| arkAction.execute(project); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider dependency injection for ArkPluginAction.
The current implementation creates a tight coupling with the ArkPluginAction class. Consider using dependency injection to improve testability and flexibility.
- private void registerPluginActions(Project project) {
- ArkPluginAction arkAction = new ArkPluginAction();
+ private void registerPluginActions(Project project) {
+ ArkPluginAction arkAction = createPluginAction();
arkAction.execute(project);
}
+
+ // Protected for testing
+ protected ArkPluginAction createPluginAction() {
+ return new ArkPluginAction();
+ }Also, there's an extra whitespace in the method declaration and before the new keyword.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void registerPluginActions(Project project) { | |
| ArkPluginAction arkAction = new ArkPluginAction(); | |
| arkAction.execute(project); | |
| } | |
| private void registerPluginActions(Project project) { | |
| ArkPluginAction arkAction = createPluginAction(); | |
| arkAction.execute(project); | |
| } | |
| // Protected for testing | |
| protected ArkPluginAction createPluginAction() { | |
| return new ArkPluginAction(); | |
| } |
| private final Property<String> mainClass; | ||
|
|
||
| public SofaArkGradlePluginExtension(Project project){ | ||
|
|
||
| this.mainClass = project.getObjects().property(String.class); | ||
|
|
||
| getPriority().convention(project.provider(() -> PRIORITY)); | ||
| getArkClassifier().convention(project.provider(() -> ARK_CLASSIFIER)); | ||
| getFinalName().convention(project.provider(() -> FINAL_NAME)); | ||
| getBizName().convention(project.provider(() -> BIZ_NAME)); | ||
| getBizClassifier().convention(project.provider(() -> BIZ_CLASSIFIER)); | ||
|
|
||
| getBizVersion().convention(project.provider(() -> ARK_JAR_PLUGIN_VERSION)); | ||
| getArkVersion().convention(project.provider(() -> ARK_JAR_PLUGIN_VERSION)); | ||
| getWebContextPath().convention(project.provider(()-> WEB_CONTEXT_PATH)); | ||
|
|
||
| getOutputDirectory().convention(project.getLayout().getBuildDirectory().dir("libs")); | ||
| } | ||
|
|
||
| public Property<String> getMainClass() { | ||
| return this.mainClass; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider declaring getMainClass() as an abstract method for consistency
The mainClass property is initialized directly in the constructor, and getMainClass() is a concrete method returning this property. In contrast, all other properties are declared as abstract methods and utilize Gradle's lazy property mechanism with conventions set in the constructor. For consistency and to align with Gradle best practices, consider declaring getMainClass() as an abstract method like the others.
Suggested refactor:
Remove the mainClass field and its initialization. Declare getMainClass() as an abstract method and set its convention in the constructor if needed.
- private final Property<String> mainClass;
public SofaArkGradlePluginExtension(Project project){
- this.mainClass = project.getObjects().property(String.class);
// Set conventions for properties
+ getMainClass().convention(project.provider(() -> /* default value if any */));
// Existing code...
}
- public Property<String> getMainClass() {
- return this.mainClass;
- }
+ abstract public Property<String> getMainClass();Committable suggestion was skipped due to low confidence.
| Configuration runtimeClasspath = project.getConfigurations().getByName("runtimeClasspath"); | ||
| Configuration sofaArkConfig = project.getConfigurations().maybeCreate("sofaArkConfig"); | ||
| Dependency arkDependency = project.getDependencies().create(SofaArkGradlePlugin.ARK_BOOTSTRAP+arkConfig.getArkVersion().get()); | ||
| ((ModuleDependency) arkDependency).setTransitive(false); |
There was a problem hiding this comment.
Ensure Safe Casting to ModuleDependency
Casting arkDependency to ModuleDependency without verification may lead to a ClassCastException if the dependency is not of the expected type.
Before casting, verify that arkDependency is an instance of ModuleDependency:
if (arkDependency instanceof ModuleDependency) {
((ModuleDependency) arkDependency).setTransitive(false);
} else {
// Handle the scenario where arkDependency is not a ModuleDependency
}This ensures type safety and prevents potential runtime exceptions.
| for (String exclude : arkConfig.getExcludes().get()) { | ||
| String[] parts = exclude.split(":"); | ||
| if (parts.length == 2) { | ||
| Map<String, String> excludeMap = new HashMap<>(); | ||
| excludeMap.put("group", parts[0]); | ||
| excludeMap.put("module", parts[1]); | ||
| configuration.exclude(excludeMap); | ||
| } |
There was a problem hiding this comment.
Handle Invalid Exclusion Patterns
In applyExclusions, exclusions with an unexpected format are silently ignored, which may lead to configuration issues.
Add logging to notify about invalid exclusion patterns:
else {
project.getLogger().warn("Invalid exclusion pattern: {}", exclude);
}This aids in debugging and ensures all intended exclusions are applied.
| private void closeQuietly(OutputStream outputStream) { | ||
| try { | ||
| outputStream.close(); | ||
| } | ||
| catch (IOException ex) { | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid empty catch blocks; handle exceptions appropriately
The catch block in closeQuietly is empty, which can suppress exceptions silently and make debugging difficult. It's advisable to at least log the exception or document why it's safe to ignore it.
Consider handling the exception as follows:
private void closeQuietly(OutputStream outputStream) {
try {
outputStream.close();
}
- catch (IOException ex) {
- }
+ catch (IOException ex) {
+ // Optionally log the exception
+ // logger.warn("Failed to close OutputStream", ex);
+ // If logging is not desired, add a comment explaining why
+ // Ignoring IOException on close as it's not critical
+ }
}If you choose to ignore the exception intentionally, adding a comment clarifies the reasoning.
Committable suggestion was skipped due to low confidence.
| Processor1(ZipArchiveOutputStream out) throws IOException { | ||
| this.out = out; | ||
| this.arkFile = conFile.get(0).getAbsolutePath(); | ||
| } |
There was a problem hiding this comment.
Add null or empty check for conFile to prevent IndexOutOfBoundsException
In the constructor of Processor1, the code assumes that conFile is not null or empty when accessing conFile.get(0). If conFile is empty, this will result in an IndexOutOfBoundsException.
Consider adding a check to ensure conFile is not null or empty before accessing it:
Processor1(ZipArchiveOutputStream out) throws IOException {
this.out = out;
- this.arkFile = conFile.get(0).getAbsolutePath();
+ if (conFile != null && !conFile.isEmpty()) {
+ this.arkFile = conFile.get(0).getAbsolutePath();
+ } else {
+ throw new IllegalArgumentException("Container file list is null or empty. Please configure it properly.");
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Processor1(ZipArchiveOutputStream out) throws IOException { | |
| this.out = out; | |
| this.arkFile = conFile.get(0).getAbsolutePath(); | |
| } | |
| Processor1(ZipArchiveOutputStream out) throws IOException { | |
| this.out = out; | |
| if (conFile != null && !conFile.isEmpty()) { | |
| this.arkFile = conFile.get(0).getAbsolutePath(); | |
| } else { | |
| throw new IllegalArgumentException("Container file list is null or empty. Please configure it properly."); | |
| } | |
| } |
| void writeFiles(List<File> files, String path){ | ||
| for(File file : files){ | ||
| try( FileInputStream fileInputStream = new FileInputStream(file)) { | ||
| ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(path + file.getName()); | ||
| writeEntry(fileInputStream, zipArchiveEntry); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Check for null or empty file lists in writeFiles method
The method writeFiles assumes that the files list is not null or empty. If files is null, it will cause a NullPointerException. If it's empty, the method does nothing, which might not be the intended behavior.
Add a null and empty check for the files list:
void writeFiles(List<File> files, String path){
+ if (files == null || files.isEmpty()) {
+ throw new IllegalArgumentException("File list is null or empty. Cannot write files.");
+ }
for(File file : files){
try(FileInputStream fileInputStream = new FileInputStream(file)) {
ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(path + file.getName());
writeEntry(fileInputStream, zipArchiveEntry);
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void writeFiles(List<File> files, String path){ | |
| for(File file : files){ | |
| try( FileInputStream fileInputStream = new FileInputStream(file)) { | |
| ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(path + file.getName()); | |
| writeEntry(fileInputStream, zipArchiveEntry); | |
| } catch (IOException e) { | |
| throw new RuntimeException(e); | |
| } | |
| } | |
| } | |
| void writeFiles(List<File> files, String path){ | |
| if (files == null || files.isEmpty()) { | |
| throw new IllegalArgumentException("File list is null or empty. Cannot write files."); | |
| } | |
| for(File file : files){ | |
| try( FileInputStream fileInputStream = new FileInputStream(file)) { | |
| ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(path + file.getName()); | |
| writeEntry(fileInputStream, zipArchiveEntry); | |
| } catch (IOException e) { | |
| throw new RuntimeException(e); | |
| } | |
| } | |
| } |
|
|
||
| private void writeClass(ZipArchiveEntry entry, ZipInputStream in, ZipArchiveOutputStream out) throws IOException { | ||
| out.putArchiveEntry(entry); | ||
| StringUtils.copyTo(in, out); |
There was a problem hiding this comment.
Import StringUtils or replace with appropriate utility method
In line 289, StringUtils.copyTo(in, out); is used, but there is no import statement for StringUtils. This may lead to a compilation error if StringUtils is not defined or imported correctly.
If StringUtils is a custom utility class, ensure it is imported:
+ import com.alipay.sofa.ark.common.util.StringUtils;Alternatively, if you intended to use a standard utility, consider replacing it with IOUtils.copy() from Apache Commons IO:
- StringUtils.copyTo(in, out);
+ IOUtils.copy(in, out);And add the necessary import:
+ import org.apache.commons.io.IOUtils;Committable suggestion was skipped due to low confidence.
| private void writeLoaderClasses(JarInputStream jarInputStream, JarFile jarFileSource) throws IOException { | ||
| JarEntry entry; | ||
| while ((entry = jarInputStream.getNextJarEntry()) != null) { | ||
| if (entry.getName().endsWith(".class") | ||
| && (entry.getName().contains("com/alipay/sofa/ark/spi/archive") | ||
| || entry.getName().contains("com/alipay/sofa/ark/loader") | ||
| || entry.getName().contains("com/alipay/sofa/ark/bootstrap") | ||
| || entry.getName().contains("com/alipay/sofa/ark/common/util/StringUtils") | ||
| || entry.getName().contains("com/alipay/sofa/ark/common/util/AssertUtils") || entry | ||
| .getName().contains("com/alipay/sofa/ark/spi/constant"))) { | ||
|
|
||
| ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(entry.getName()); | ||
| this.out.putArchiveEntry(zipArchiveEntry); | ||
|
|
||
| byte[] bytes = new byte[1024]; | ||
| int length; | ||
| while ((length = jarInputStream.read(bytes)) >= 0) { | ||
| this.out.write(bytes, 0, length); | ||
| } | ||
| this.out.closeArchiveEntry(); | ||
|
|
||
|
|
||
| } | ||
| } | ||
| jarInputStream.close(); | ||
| } |
There was a problem hiding this comment.
Close JarInputStream properly to prevent resource leaks
In the writeLoaderClasses method, the JarInputStream is closed inside the method. However, since it's passed as a parameter, it's generally the caller's responsibility to close it.
Avoid closing JarInputStream inside the method or document that the method takes ownership of the stream.
Alternatively, remove the explicit close statement if the stream is managed elsewhere:
private void writeLoaderClasses(JarInputStream jarInputStream, JarFile jarFileSource) throws IOException {
// processing entries
}
- jarInputStream.close();
}Ensure that JarInputStream is properly closed in the calling method using try-with-resources.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void writeLoaderClasses(JarInputStream jarInputStream, JarFile jarFileSource) throws IOException { | |
| JarEntry entry; | |
| while ((entry = jarInputStream.getNextJarEntry()) != null) { | |
| if (entry.getName().endsWith(".class") | |
| && (entry.getName().contains("com/alipay/sofa/ark/spi/archive") | |
| || entry.getName().contains("com/alipay/sofa/ark/loader") | |
| || entry.getName().contains("com/alipay/sofa/ark/bootstrap") | |
| || entry.getName().contains("com/alipay/sofa/ark/common/util/StringUtils") | |
| || entry.getName().contains("com/alipay/sofa/ark/common/util/AssertUtils") || entry | |
| .getName().contains("com/alipay/sofa/ark/spi/constant"))) { | |
| ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(entry.getName()); | |
| this.out.putArchiveEntry(zipArchiveEntry); | |
| byte[] bytes = new byte[1024]; | |
| int length; | |
| while ((length = jarInputStream.read(bytes)) >= 0) { | |
| this.out.write(bytes, 0, length); | |
| } | |
| this.out.closeArchiveEntry(); | |
| } | |
| } | |
| jarInputStream.close(); | |
| } | |
| private void writeLoaderClasses(JarInputStream jarInputStream, JarFile jarFileSource) throws IOException { | |
| JarEntry entry; | |
| while ((entry = jarInputStream.getNextJarEntry()) != null) { | |
| if (entry.getName().endsWith(".class") | |
| && (entry.getName().contains("com/alipay/sofa/ark/spi/archive") | |
| || entry.getName().contains("com/alipay/sofa/ark/loader") | |
| || entry.getName().contains("com/alipay/sofa/ark/bootstrap") | |
| || entry.getName().contains("com/alipay/sofa/ark/common/util/StringUtils") | |
| || entry.getName().contains("com/alipay/sofa/ark/common/util/AssertUtils") || entry | |
| .getName().contains("com/alipay/sofa/ark/spi/constant"))) { | |
| ZipArchiveEntry zipArchiveEntry = new ZipArchiveEntry(entry.getName()); | |
| this.out.putArchiveEntry(zipArchiveEntry); | |
| byte[] bytes = new byte[1024]; | |
| int length; | |
| while ((length = jarInputStream.read(bytes)) >= 0) { | |
| this.out.write(bytes, 0, length); | |
| } | |
| this.out.closeArchiveEntry(); | |
| } | |
| } | |
| } |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1005 +/- ##
============================================
+ Coverage 78.21% 78.36% +0.14%
Complexity 874 874
============================================
Files 171 171
Lines 7052 7052
Branches 1036 1036
============================================
+ Hits 5516 5526 +10
+ Misses 1005 994 -11
- Partials 531 532 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (8)
sofa-ark-parent/support/ark-gradle-plugin/README.md (2)
1-11: Consider providing English documentation for better accessibility.The documentation is currently only in Chinese. Consider providing an English version or making it bilingual to improve accessibility for the international community.
1-11: Add missing essential documentation sections.Consider adding these important sections to complete the documentation:
- Project Status/Roadmap
- Contributing Guidelines
- License Information
- Changelog/Release Notes
- Support and Issue Reporting
This documentation serves as the entry point for developers. A well-structured README is crucial for adoption and maintenance of the Gradle plugin.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/JarTypeFileSpec.java (1)
25-28: Add class-level documentation.Add Javadoc to explain the purpose of this class and its role in the Gradle plugin. This will help maintainers understand its significance in filtering Spring Boot dependency starters.
+/** + * Gradle file specification that filters JAR files based on their manifest attributes. + * Specifically excludes JARs with certain Spring Boot jar types defined in {@link #EXCLUDED_JAR_TYPES}. + */ class JarTypeFileSpec implements Spec<File> {sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/StringUtils.java (1)
72-76: Consider architectural implications before refactoring CollectionUtils.While moving
isEmptyto the main class would simplify the structure, consider these architectural points first:
- The method might be used by other plugin components that expect collection-related utilities to be grouped.
- If more collection utilities are planned for the plugin, a separate
CollectionUtilsclass might be justified.Consider creating a utility package (e.g.,
com.alipay.sofa.ark.plugin.util) to organize utility classes if more are planned. This would provide better separation of concerns and maintainability as the plugin grows.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1)
25-25: Add class-level JavaDoc for the plugin classConsider adding JavaDoc comments to the
SofaArkGradlePluginclass to improve code readability and maintainability. Providing documentation helps other developers understand the purpose and usage of the plugin.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java (2)
36-36: Add class-level JavaDoc for better documentationAdding JavaDoc comments to the
ArkJarclass will improve code readability and maintainability by providing clear documentation of its purpose and usage.
217-229: Simplify utility methods by inliningcallToThe
callTomethod simply returns the providedCallable. You can simplify the code by inlining this method withinfromCallTo.Apply this diff to simplify the methods:
- private static <T> Callable<T> callTo(Callable<T> callable) { - return callable; - } private static <T> Action<CopySpec> fromCallTo(Callable<T> callable) { - return (spec) -> spec.from(callTo(callable)); + return (spec) -> spec.from(callable); }sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/MainClassFinder.java (1)
331-340: AnnotateMainClassCallbackwith@FunctionalInterfaceSince
MainClassCallbackis an interface with a single abstract methoddoWith, it can be annotated with@FunctionalInterface. This annotation improves code readability and indicates that the interface is intended to be implemented by a lambda expression or method reference.Apply this diff to annotate the interface:
+@FunctionalInterface interface MainClassCallback<T> { /** * Handle the specified main class. * @param mainClass the main class * @return a non-null value if processing should end or {@code null} to continue */ T doWith(MainClass mainClass); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
- sofa-ark-parent/support/ark-gradle-plugin/README.md (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/BootArchive.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/DefaultTimeZoneOffset.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/JarTypeFileSpec.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/LoaderZipEntries.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/MainClassFinder.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/StringUtils.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ZipCompression.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkBizCopyAction.java
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java
🧰 Additional context used
🔇 Additional comments (13)
sofa-ark-parent/support/ark-gradle-plugin/README.md (1)
6-9:⚠️ Potential issueExpand the usage section with detailed instructions.
The usage section needs significant enhancement. Consider adding:
- Prerequisites and system requirements
- Step-by-step installation instructions
- Detailed usage examples with code snippets
- Clear explanation of the "pending application" status for remote repository integration
- Version compatibility information
- Migration guide from the Maven plugin
Let's check if there are any example usages in the codebase:
#!/bin/bash # Look for example configurations or test cases that could be used as documentation examples fd -e gradle -e groovy --exec rg -l 'arkConfig'sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ZipCompression.java (1)
21-33: Clean and well-structured enum implementation!The enum effectively maps to Java's ZipEntry compression methods with clear documentation.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/JarTypeFileSpec.java (1)
25-43: Verify integration with Spring Boot jar types.The class assumes specific Spring Boot manifest attributes. Let's verify this assumption against the Spring Boot documentation and codebase.
✅ Verification successful
Let me gather more information about this manifest attribute usage and related Spring Boot dependencies.
This will help us understand:
- The context of "dependencies-starter" usage
- Other Spring Boot manifest attributes being used
- Similar manifest attribute checks
- Spring Boot related configuration files
Let me check the test file that mentions "dependencies-starter" to understand its usage context and look for any related Spring Boot configurations.
Let me check the Spring Boot integration modules to understand how this manifest attribute is used in the context of the project.
Based on the investigation so far, I can now provide a final response about the Spring Boot jar type verification concern.
The
Spring-Boot-Jar-Typemanifest attribute appears to be project-specific and not a standard Spring Boot attributeThe investigation reveals that:
- The
Spring-Boot-Jar-Typemanifest attribute is only used inJarTypeFileSpec.javato exclude jars with type "dependencies-starter"- This is not a standard Spring Boot manifest attribute, as it's not found in Spring Boot's documentation or codebase
- The attribute appears to be specific to SOFA Ark's build tooling for managing dependencies
The code is correctly implementing a project-specific jar filtering mechanism and doesn't make incorrect assumptions about Spring Boot's manifest attributes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for Spring-Boot-Jar-Type usage in dependencies # Expected: Find documentation or usage of this manifest attribute # Look for any Spring Boot related configuration or documentation files fd -e md -e properties -e yaml -e xml . | xargs rg -l "Spring-Boot-Jar-Type" # Look for similar manifest attribute usage in Java files rg -t java "getMainAttributes\(\)\.getValue\(.*Boot.*\)"Length of output: 331
Script:
#!/bin/bash # Search for dependencies-starter related configurations and code rg -g '!*.class' -l "dependencies-starter" # Look for Spring Boot manifest attribute definitions or configurations rg -g '!*.class' "Boot.*Type" -A 2 -B 2 # Check if there are any other manifest attribute checks in the codebase ast-grep --pattern 'getManifest().getMainAttributes().getValue($STR)' # Look for any Spring Boot plugin or configuration files fd "spring-boot" -t fLength of output: 1176
Script:
#!/bin/bash # Check the test file content cat "sofa-ark-parent/support/ark-maven-plugin/src/test/java/com/alipay/sofa/ark/boot/mojo/ModuleSlimStrategyTest.java" # Look for any Spring Boot related configurations in build files fd -e gradle -e xml . | xargs rg -l "spring-boot" # Search for any documentation about jar types or manifest attributes fd "README|CONTRIBUTING" -e md | xargs rg -l -i "manifest|jar.*type"Length of output: 20992
Script:
#!/bin/bash # Check Spring Boot integration related files cat "sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/pom.xml" # Look for any manifest-related code in Spring Boot integration rg -g '!*.class' -t java "manifest|jar.*type" -A 2 -B 2 "sofa-ark-parent/support/ark-springboot-integration/" # Check if there are any configuration properties related to jar types fd -e properties -e yaml . "sofa-ark-parent/support/ark-springboot-integration/" | xargs catLength of output: 6414
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/DefaultTimeZoneOffset.java (1)
46-48: 🛠️ Refactor suggestionVerify timestamp handling for edge cases
The current implementation might have issues with:
- Extreme timestamps that could cause long overflow
- Daylight savings time transitions
- Dates before 1970 (negative timestamps)
Let's verify the usage context to understand the potential impact:
Consider adding bounds checking and handling edge cases:
long removeFrom(long time) { + // Handle potential overflow + long offset = this.defaultTimeZone.getOffset(time); + if (time > 0 && offset > 0 && time > Long.MAX_VALUE - offset) { + throw new ArithmeticException("Timestamp too large for offset adjustment"); + } + if (time < 0 && offset < 0 && time < Long.MIN_VALUE - offset) { + throw new ArithmeticException("Timestamp too small for offset adjustment"); + } return time - this.defaultTimeZone.getOffset(time); }sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/StringUtils.java (2)
39-68: Verify delimiter usage patterns in the plugin.The string conversion methods are well-implemented. Let's verify their usage patterns in the plugin context to ensure consistent delimiter handling.
✅ Verification successful
Let me generate additional scripts to check for actual usage of these string conversion methods in the codebase.
Based on the search results, I can now provide a final response as we have sufficient information about the usage patterns of these string conversion methods.
String conversion methods are used consistently and appropriately
The implementation is not only well-designed but also consistently used across the codebase. The search results show that:
- The comma-delimited version is used in specific contexts where comma separation is the standard format:
- Spring Boot configuration processor metadata locations
- Tomcat TLD skip patterns configuration
- No alternative or inconsistent collection-to-string conversion patterns were found (no competing stream().collect(Collectors.joining()) or StringBuilder patterns)
- The delimiter usage is standardized and matches the expected use cases in the Gradle plugin context
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent delimiter usage across the plugin echo "Checking delimiter patterns in method calls..." rg "collectionToDelimitedString\(.*\)" --type java -A 2 echo "Checking for potential inconsistent delimiter usage..." rg "String.*delim.*=.*[\"'].*[\"']" --type javaLength of output: 1266
Script:
#!/bin/bash # Search for usages of collectionToCommaDelimitedString and any string concatenation with collections echo "Checking usages of collection string conversion methods..." rg "collectionToCommaDelimitedString|collectionToDelimitedString" --type java -B 2 -A 2 echo "Checking for collection joining patterns..." ast-grep --pattern 'String $_ = $collection.stream().collect(Collectors.joining($_))' echo "Checking for StringBuilder operations with collections..." ast-grep --pattern '$_.append($collection.stream())'Length of output: 5379
27-37: Verify stream handling in the Gradle plugin context.While the implementation is correct, let's ensure it's used safely in the plugin context.
✅ Verification successful
Let me gather more context about the stream handling in the calling classes to ensure proper resource management.
This will help us verify if the streams are properly managed in the calling code.
Based on the analysis of the shell script results, I can now provide a final response regarding the stream handling in the copyTo method:
Stream handling is properly managed by calling code
The
copyTomethod is used in a safe manner throughout the codebase:
In
ArkBizCopyAction, the streams are managed with try-with-resources blocks:try (FileInputStream fileInputStream = new FileInputStream(file)) { writeEntry(fileInputStream, zipArchiveEntry); }In
LoaderZipEntries, the streams are also properly closed:try (ZipInputStream loaderJar = new ZipInputStream(...)) { // copyTo usage }The method is correctly implemented as an internal utility method that expects the caller to handle stream lifecycle, which is consistently done throughout the codebase using try-with-resources blocks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for proper stream handling patterns # Look for try-with-resources usage with this method # Search for copyTo method usage echo "Searching for copyTo method usage patterns..." rg -A 10 "copyTo\(.*\)" --type java # Look for potential resource leaks echo "Checking for proper stream handling..." ast-grep --pattern 'try ($_) { $$$ copyTo($_, $_) $$$ }'Length of output: 6144
Script:
#!/bin/bash # Check the implementation of classes that use copyTo echo "Checking ArkBizCopyAction implementation..." rg -A 20 "class ArkBizCopyAction" --type java echo "\nChecking LoaderZipEntries implementation..." rg -A 20 "class LoaderZipEntries" --type java # Look for try-with-resources or stream closing patterns echo "\nChecking stream handling patterns..." ast-grep --pattern 'try ($$$) { $$$ }'Length of output: 67900
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1)
34-38: Initialization logic looks goodThe
applymethod correctly verifies the Gradle version, sets up the extension, and registers plugin actions. The sequence ensures that the plugin is properly configured.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/BootArchive.java (1)
29-84: Interface definition is clear and well-structuredThe
BootArchiveinterface is well-designed, with appropriately defined methods and annotations that align with Gradle's task configuration practices.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (4)
119-119: Check the result ofmkdirs()to handle potential directory creation failuresThe call to
outputFile.getParentFile().mkdirs();may fail due to I/O errors or permission issues, which can cause subsequent file operations to throw exceptions.
143-144: Log exceptions when finding the main class for better debuggingIn the
findMainClassmethod, when anIOExceptionoccurs, it is caught and the method returnsnullsilently, which can make debugging difficult.
165-168: Prevent possibleNullPointerExceptionwhen accessingspringBootExtensionThe
springBootExtensionmay benullif theSofaArkGradlePluginExtensionis not applied to the project. Accessing it without checking can cause aNullPointerException.
196-196: Include the caught exception as the cause in theRuntimeExceptionWhen catching the
IOException, the re-thrownRuntimeExceptiondoes not include the original exception as its cause, which can hinder debugging.sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (1)
49-66: Well-structured implementation of theexecutemethodThe
executemethod effectively sets up the project by invoking configuration methods, ensuring that tasks and settings are properly initialized.
| # 配置 | ||
| `sofa-ark-gradle-plugin` 使用 arkConfig 来进行配置。 |
There was a problem hiding this comment.
Enhance the configuration section with detailed examples.
The configuration section is too brief. Consider adding:
- Complete list of available configuration options in
arkConfig - Example configurations for common use cases
- Default values for each configuration option
Would you like me to help create a comprehensive configuration documentation section?
| 参考`sofa-ark-plugin-gradle-plugin`的本地发布和引入。 | ||
| 使用Gradle刷新后，如果一切正常，会在IDEA右侧Gradle任务列表中出现arkJar，具体如下： Tasks > build > arkJar，点击arkJar执行，会在指定的outputDirectory中输出ark包和biz包。 |
There was a problem hiding this comment.
💡 Codebase verification
Based on the gathered information, I can now provide a comprehensive response about the build instructions.
Enhance the build instructions with more details and configuration options.
The current documentation should be expanded to include:
-
Configuration options through
arkConfig:arkVersion: Version of SOFAArk to usebizName: Name of the business modulebizClassifier: Classifier for the biz package (defaults to "ark-biz")arkClassifier: Classifier for the ark package (defaults to "ark-executable")excludes: Dependencies to excludeexcludeGroupIds: Group IDs to excludeexcludeArtifactIds: Artifact IDs to exclude
-
Task outputs:
- The
arkJartask generates two artifacts:- An executable ark package (with classifier "ark-executable")
- A business module package (with classifier "ark-biz")
- Output files are placed in the project's build directory
- The
-
Task dependencies:
- The task is added to the build group
- It depends on the project's classes and resources being compiled
-
Example configuration:
arkConfig {
arkVersion = "2.2.14"
bizName = "example-biz"
excludes = ["group:artifact"]
}🔗 Analysis chain
Enhance the build instructions with more details.
The build instructions should include:
- Expected output artifacts and their locations
- Description of the
arkJartask and its dependencies - Available configuration options for output directory
- Troubleshooting guide for common issues
Let's check the actual implementation to ensure documentation accuracy:
Let me try a different approach to find the Gradle task and configuration details.
Let me check the README files to understand the documented behavior and configuration options.
Let me try one final search to find the configuration options and task details.
Let me check the ArkJar task implementation and configuration options.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for the arkJar task implementation
ast-grep --pattern 'class $_ implements Task {
$$$
arkJar($$$) {
$$$
}
$$$
}'
Length of output: 47
Script:
#!/bin/bash
# Search for arkJar task definition in Gradle files
rg -l "task.*arkJar" --type groovy --type gradle
# Search for configuration and task details
rg "arkJar|outputDirectory|ark" --type groovy --type gradle -A 5
# Look for any documentation about the task
fd "README" -e md -x rg -l "arkJar|ark.*plugin"
Length of output: 875
Script:
#!/bin/bash
# Check content of relevant README files
echo "=== Main README ==="
cat README.md | rg -A 5 "arkJar|gradle.*plugin"
echo -e "\n=== Gradle Plugin README ==="
cat sofa-ark-parent/support/ark-gradle-plugin/README.md | rg -A 10 "arkJar|gradle.*plugin|配置项"
# Look for any test files that might document the behavior
rg -l "arkJar" --type-add 'test:*{Test,Spec}.*' --type test
Length of output: 677
Script:
#!/bin/bash
# Search for arkConfig configuration options in source files
rg "arkConfig" --type groovy --type java -B 2 -A 5
# Look for any source files that define the task
fd -e java -e groovy --exec rg -l "class.*ArkJar|task.*arkJar"
# Check for any examples in test resources
fd -e gradle -t f -x rg -l "arkConfig"
Length of output: 8871
Script:
#!/bin/bash
# Check the ArkJar task implementation
cat sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkJar.java
# Check the extension class for configuration options
rg -A 10 "class SofaArkGradlePluginExtension" --type java
# Look for any example configurations in test resources
fd -t f -e gradle -x rg -A 5 "arkConfig \{"
Length of output: 9987
| public enum ZipCompression { | ||
|
|
||
| /** | ||
| * The entry should be {@link ZipEntry#STORED} in the archive. | ||
| */ | ||
| STORED, | ||
|
|
||
| /** | ||
| * The entry should be {@link ZipEntry#DEFLATED} in the archive. | ||
| */ | ||
| DEFLATED | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding utility method for ZipEntry integration.
To improve usability and encapsulation, consider adding a method to convert the enum to the corresponding ZipEntry compression value.
Here's a suggested implementation:
public enum ZipCompression {
/**
* The entry should be {@link ZipEntry#STORED} in the archive.
*/
STORED,
/**
* The entry should be {@link ZipEntry#DEFLATED} in the archive.
*/
DEFLATED;
+
+ /**
+ * Returns the corresponding ZipEntry compression method value.
+ * @return the compression method value as defined in {@link ZipEntry}
+ */
+ public int getZipEntryMethod() {
+ return this == STORED ? ZipEntry.STORED : ZipEntry.DEFLATED;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public enum ZipCompression { | |
| /** | |
| * The entry should be {@link ZipEntry#STORED} in the archive. | |
| */ | |
| STORED, | |
| /** | |
| * The entry should be {@link ZipEntry#DEFLATED} in the archive. | |
| */ | |
| DEFLATED | |
| } | |
| public enum ZipCompression { | |
| /** | |
| * The entry should be {@link ZipEntry#STORED} in the archive. | |
| */ | |
| STORED, | |
| /** | |
| * The entry should be {@link ZipEntry#DEFLATED} in the archive. | |
| */ | |
| DEFLATED; | |
| /** | |
| * Returns the corresponding ZipEntry compression method value. | |
| * @return the compression method value as defined in {@link ZipEntry} | |
| */ | |
| public int getZipEntryMethod() { | |
| return this == STORED ? ZipEntry.STORED : ZipEntry.DEFLATED; | |
| } | |
| } |
|
|
||
| class JarTypeFileSpec implements Spec<File> { | ||
|
|
||
| private static final Set<String> EXCLUDED_JAR_TYPES = Collections.singleton("dependencies-starter"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making EXCLUDED_JAR_TYPES more flexible.
The hardcoded singleton set limits extensibility. Consider making it configurable through the Gradle plugin extension to allow users to customize excluded jar types.
-private static final Set<String> EXCLUDED_JAR_TYPES = Collections.singleton("dependencies-starter");
+private final Set<String> excludedJarTypes;
+
+public JarTypeFileSpec(Set<String> excludedJarTypes) {
+ this.excludedJarTypes = excludedJarTypes != null ?
+ Collections.unmodifiableSet(new HashSet<>(excludedJarTypes)) :
+ Collections.singleton("dependencies-starter");
+}Committable suggestion was skipped due to low confidence.
| @Override | ||
| public boolean isSatisfiedBy(File file) { | ||
| try (JarFile jar = new JarFile(file)) { | ||
| String jarType = jar.getManifest().getMainAttributes().getValue("Spring-Boot-Jar-Type"); | ||
| if (jarType != null && EXCLUDED_JAR_TYPES.contains(jarType)) { | ||
| return false; | ||
| } | ||
| } | ||
| catch (Exception ex) { | ||
| // Continue | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Enhance robustness of manifest attribute checking.
The current implementation has several potential issues:
- No null check for manifest
- No validation of file existence or JAR format
- Magic string for manifest attribute key
Consider this improved implementation:
+private static final String SPRING_BOOT_JAR_TYPE = "Spring-Boot-Jar-Type";
@Override
public boolean isSatisfiedBy(File file) {
+ if (file == null || !file.exists() || !file.isFile()) {
+ return false;
+ }
try (JarFile jar = new JarFile(file)) {
- String jarType = jar.getManifest().getMainAttributes().getValue("Spring-Boot-Jar-Type");
+ var manifest = jar.getManifest();
+ if (manifest == null) {
+ return true;
+ }
+ String jarType = manifest.getMainAttributes().getValue(SPRING_BOOT_JAR_TYPE);
if (jarType != null && EXCLUDED_JAR_TYPES.contains(jarType)) {
return false;
}
}
catch (Exception ex) {
- // Continue
+ // Invalid JAR file or IO error, exclude it
+ return false;
}
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public boolean isSatisfiedBy(File file) { | |
| try (JarFile jar = new JarFile(file)) { | |
| String jarType = jar.getManifest().getMainAttributes().getValue("Spring-Boot-Jar-Type"); | |
| if (jarType != null && EXCLUDED_JAR_TYPES.contains(jarType)) { | |
| return false; | |
| } | |
| } | |
| catch (Exception ex) { | |
| // Continue | |
| } | |
| return true; | |
| } | |
| private static final String SPRING_BOOT_JAR_TYPE = "Spring-Boot-Jar-Type"; | |
| @Override | |
| public boolean isSatisfiedBy(File file) { | |
| if (file == null || !file.exists() || !file.isFile()) { | |
| return false; | |
| } | |
| try (JarFile jar = new JarFile(file)) { | |
| var manifest = jar.getManifest(); | |
| if (manifest == null) { | |
| return true; | |
| } | |
| String jarType = manifest.getMainAttributes().getValue(SPRING_BOOT_JAR_TYPE); | |
| if (jarType != null && EXCLUDED_JAR_TYPES.contains(jarType)) { | |
| return false; | |
| } | |
| } | |
| catch (Exception ex) { | |
| // Invalid JAR file or IO error, exclude it | |
| return false; | |
| } | |
| return true; | |
| } |
| /** | ||
| * Returns the fully-qualified name of the application's main class. | ||
| * @return the fully-qualified name of the application's main class | ||
| * @since 2.4.0 | ||
| */ | ||
| @Input | ||
| Property<String> getMainClass(); |
There was a problem hiding this comment.
Verify the accuracy of the @since tags in Javadoc comments
The methods getMainClass() and setClasspath() have @since tags with versions (2.4.0 and 2.0.7) that may not reflect the actual versioning of this new interface. Please ensure that the @since annotations accurately represent when these methods were introduced.
Also applies to: 69-82
| * Internal utility used to copy entries from the {@code spring-boot-loader.jar}. | ||
| * | ||
| * @author Andy Wilkinson | ||
| * @author Phillip Webb | ||
| * @author Scott Frederick | ||
| */ |
There was a problem hiding this comment.
Verify license compliance and proper attribution.
The class documentation and the authors listed (Andy Wilkinson, Phillip Webb, Scott Frederick) suggest that this code may have been copied from the Spring Boot project. Please ensure that the code is used in compliance with its original license and that proper attribution is provided according to the license terms.
|
|
||
| private void prepareEntry(ZipArchiveEntry entry, int unixMode) { | ||
| if (this.entryTime != null) { | ||
| entry.setTime(DefaultTimeZoneOffset.INSTANCE.removeFrom(this.entryTime)); |
There was a problem hiding this comment.
DefaultTimeZoneOffset is undefined or missing an import.
The class DefaultTimeZoneOffset used in entry.setTime(DefaultTimeZoneOffset.INSTANCE.removeFrom(this.entryTime)); is neither defined in this file nor imported from any library. This will lead to a compilation error. Ensure that DefaultTimeZoneOffset is properly defined or the appropriate import statement is added.
Apply this diff to address the issue if DefaultTimeZoneOffset is part of the project or a library:
+import org.springframework.boot.loader.tools.DefaultTimeZoneOffset;If DefaultTimeZoneOffset is not available, consider handling the time adjustment differently or implementing the necessary functionality.
Committable suggestion was skipped due to low confidence.
| Configuration developmentOnly = project.getConfigurations() | ||
| .create(SofaArkGradlePlugin.DEVELOPMENT_ONLY_CONFIGURATION_NAME); | ||
| developmentOnly | ||
| .setDescription("Configuration for development-only dependencies such as Spring Boot's DevTools."); |
There was a problem hiding this comment.
Avoid hard-coded references to Spring Boot components in configuration descriptions
The description for the developmentOnly configuration mentions "Spring Boot's DevTools". Since this plugin is for Sofa Ark, referencing Spring Boot-specific components may not be appropriate.
Consider updating the description to be more general:
- .setDescription("Configuration for development-only dependencies such as Spring Boot's DevTools.");
+ .setDescription("Configuration for development-only dependencies.");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .setDescription("Configuration for development-only dependencies such as Spring Boot's DevTools."); | |
| .setDescription("Configuration for development-only dependencies."); |
| private static ClassDescriptor createClassDescriptor(InputStream inputStream) { | ||
| try { | ||
| ClassReader classReader = new ClassReader(inputStream); | ||
| ClassDescriptor classDescriptor = new ClassDescriptor(); | ||
| classReader.accept(classDescriptor, ClassReader.SKIP_CODE); | ||
| return classDescriptor; | ||
| } | ||
| catch (IOException ex) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle potential non-IOException exceptions in createClassDescriptor
The createClassDescriptor method currently catches only IOException, but methods like ClassReader's constructor and accept can throw other exceptions such as IllegalArgumentException or ArrayIndexOutOfBoundsException when processing malformed or corrupted class files. To prevent the application from crashing due to unhandled exceptions, consider catching Exception instead.
Apply this diff to handle all exceptions:
private static ClassDescriptor createClassDescriptor(InputStream inputStream) {
try {
ClassReader classReader = new ClassReader(inputStream);
ClassDescriptor classDescriptor = new ClassDescriptor();
classReader.accept(classDescriptor, ClassReader.SKIP_CODE);
return classDescriptor;
- }
- catch (IOException ex) {
+ } catch (Exception ex) {
return null;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static ClassDescriptor createClassDescriptor(InputStream inputStream) { | |
| try { | |
| ClassReader classReader = new ClassReader(inputStream); | |
| ClassDescriptor classDescriptor = new ClassDescriptor(); | |
| classReader.accept(classDescriptor, ClassReader.SKIP_CODE); | |
| return classDescriptor; | |
| } | |
| catch (IOException ex) { | |
| return null; | |
| } | |
| } | |
| private static ClassDescriptor createClassDescriptor(InputStream inputStream) { | |
| try { | |
| ClassReader classReader = new ClassReader(inputStream); | |
| ClassDescriptor classDescriptor = new ClassDescriptor(); | |
| classReader.accept(classDescriptor, ClassReader.SKIP_CODE); | |
| return classDescriptor; | |
| } catch (Exception ex) { | |
| return null; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (2)
198-219: Unused parameter layerToolsLocation in createCopyAction method
The parameter layerToolsLocation in the createCopyAction(Jar jar, String layerToolsLocation) method is not used within the method implementation. Consider removing it if it's unnecessary or implement its usage if it's intended for future functionality.
222-238: Use Gradle logging instead of System.out.println
Using System.out.println for logging is not recommended in Gradle plugins. It's better to use Gradle's logging API to allow users to control the logging level and integrate with Gradle's logging system.
Apply this diff to replace System.out.println with jar.getProject().getLogger().lifecycle:
if (!outputDir.exists()) {
boolean created = outputDir.mkdirs();
if (!created) {
throw new GradleException("Failed to create output directory: " + outputDir.getAbsolutePath());
}
- System.out.println("Created output directory: " + outputDir.getAbsolutePath());
+ jar.getProject().getLogger().lifecycle("Created output directory: " + outputDir.getAbsolutePath());
}
...
- System.out.println("Target file will be created at: " + targetFile.getAbsolutePath());
+ jar.getProject().getLogger().lifecycle("Target file will be created at: " + targetFile.getAbsolutePath());📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java
🔇 Additional comments (8)
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePlugin.java (3)
33-38: LGTM! Well-structured plugin implementation.
The apply method follows a clear and logical sequence of operations:
- Version verification
- Extension creation
- Plugin action registration
52-55: 🛠️ Refactor suggestion
Clean up formatting and improve testability.
- Remove extra spaces:
- private void registerPluginActions(Project project) {
- ArkPluginAction arkAction = new ArkPluginAction();
+ private void registerPluginActions(Project project) {
+ ArkPluginAction arkAction = new ArkPluginAction();- Consider improving testability by introducing a factory method:
+ // Protected for testing
+ protected ArkPluginAction createPluginAction() {
+ return new ArkPluginAction();
+ }
private void registerPluginActions(Project project) {
- ArkPluginAction arkAction = new ArkPluginAction();
+ ArkPluginAction arkAction = createPluginAction();
arkAction.execute(project);
}Likely invalid or redundant comment.
40-46:
Fix incorrect error message in version verification.
The error message incorrectly references the "Spring Boot plugin" instead of "Sofa Ark Gradle plugin".
Apply this fix:
- throw new GradleException("Spring Boot plugin requires Gradle 6.8.+ "
+ throw new GradleException("Sofa Ark Gradle plugin requires Gradle 6.8.+ "
+ "The current version is " + currentVersion);Likely invalid or redundant comment.
sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (5)
57-107: Constructor and initialization are properly implemented
The ArkArchiveSupport constructor correctly initializes all required fields, including handling the Git information and building the initial Ark manifest.
110-125: configureBizManifest method correctly sets manifest attributes
The method accurately populates the manifest with essential attributes for the Biz application, ensuring proper configuration.
127-149: Ensure consistency in adding Git information to manifest
While the buildArkManifest method effectively adds necessary attributes, consider refactoring to eliminate duplicate code when adding Git information to the manifest, as previously suggested.
252-257: Consider renaming isTrue to isNonEmptyString for clarity
The method isTrue(Object object) checks if an object is a non-empty string. Renaming it to isNonEmptyString(Object object) would enhance readability and better reflect its functionality.
337-355: Handle exceptions appropriately in isSofaArk method
The catch block for the IOException is currently empty, which can make debugging difficult if an exception occurs. Swallowing exceptions may hide underlying issues.
| import org.gradle.api.GradleException; | ||
| import org.gradle.api.Plugin; | ||
| import org.gradle.api.Project; | ||
| import org.gradle.internal.impldep.org.bouncycastle.pqc.crypto.newhope.NHSecretKeyProcessor.PartyUBuilder; |
There was a problem hiding this comment.
Remove unnecessary internal Gradle dependency.
The import of org.gradle.internal.impldep.org.bouncycastle.pqc.crypto.newhope.NHSecretKeyProcessor.PartyUBuilder appears to be unused and references an internal Gradle class. Internal Gradle APIs (org.gradle.internal.*) are not part of the public API contract and may change without notice.
Remove this import line as it's not being used in the implementation.
| public static final String ARK_VERSION = "2.2.14"; | ||
| public static final String ARK_BIZ_TASK_NAME = "arkJar"; | ||
| public static final String DEVELOPMENT_ONLY_CONFIGURATION_NAME = "developmentOnly"; | ||
| public static final String PRODUCTION_RUNTIME_CLASSPATH_CONFIGURATION_NAME = "productionRuntimeClasspath"; | ||
| public static final String ARK_BOOTSTRAP = "com.alipay.sofa:sofa-ark-all:"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve constants management and encapsulation.
-
The
ARK_VERSIONshould be externalized to support easier version management:- Consider using Gradle's version catalog feature
- Or move it to
gradle.properties - Or use a version management plugin
-
Consider reducing the visibility of these constants to package-private since they appear to be used only internally:
- public static final String ARK_VERSION = "2.2.14";
- public static final String ARK_BIZ_TASK_NAME = "arkJar";
- public static final String DEVELOPMENT_ONLY_CONFIGURATION_NAME = "developmentOnly";
- public static final String PRODUCTION_RUNTIME_CLASSPATH_CONFIGURATION_NAME = "productionRuntimeClasspath";
- public static final String ARK_BOOTSTRAP = "com.alipay.sofa:sofa-ark-all:";
+ static final String ARK_VERSION = "2.2.14";
+ static final String ARK_BIZ_TASK_NAME = "arkJar";
+ static final String DEVELOPMENT_ONLY_CONFIGURATION_NAME = "developmentOnly";
+ static final String PRODUCTION_RUNTIME_CLASSPATH_CONFIGURATION_NAME = "productionRuntimeClasspath";
+ static final String ARK_BOOTSTRAP = "com.alipay.sofa:sofa-ark-all:";Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java
| if (!created) { | ||
| throw new GradleException("Failed to create output directory: " + outputDir.getAbsolutePath()); | ||
| } | ||
| System.out.println("Created output directory: " + outputDir.getAbsolutePath()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a logging framework instead of System.out.println
Using System.out.println for logging is not recommended in production code. Consider using a logging framework like SLF4J or Log4j for better control over logging levels and outputs, which aligns with best practices and enhances maintainability.
Apply this diff to replace System.out.println with a logging framework:
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class ArkArchiveSupport {
+ private static final Logger logger = LoggerFactory.getLogger(ArkArchiveSupport.class);
...
if (!outputDir.exists()) {
boolean created = outputDir.mkdirs();
if (!created) {
throw new GradleException("Failed to create output directory: " + outputDir.getAbsolutePath());
}
- System.out.println("Created output directory: " + outputDir.getAbsolutePath());
+ logger.info("Created output directory: {}", outputDir.getAbsolutePath());
}
File targetFile = new File(outputDir, getArkBizName(jar, classifier));
- System.out.println("Target file will be created at: " + targetFile.getAbsolutePath());
+ logger.info("Target file will be created at: {}", targetFile.getAbsolutePath());Also applies to: 226-226
There was a problem hiding this comment.
ark-gradle-plugin 的能力如下：
- 支持打包：ark-biz.jar 和 ark-executable.jar
- 支持参数配置：priority, finalName, bizName, bizVersion, mainClass, outputDirectory, arkClassifier, excludes, excludeArtifactIds, excludeGroupIds, denyImportPackages, denyImportClasses, denyImportResources, injectPluginDependencies, injectPluginExportPackages, webContextPath 字段
- 不支持参数配置：declaredMode, packExcludesUrl, packExcludesConfig, baseDir, mainClass, baseDependencyParentIdentity, skip, includes, includeGroupIds, includeArtifactIds ( 及不常用字段：requiresUnpack, packageProvided, skipArkExecutable, keepArkBizJar, disableGitInfo, gitDirectory）
整体上，具备 ark-biz.jar 打包能力
后续 TODO:
- 支持 declaredMode 等其它必要参数
- 复用 sofa-ark 自身的 JarWriter、ModuleSlimStrategy 等代码做代码打包、模块瘦身等
- 一些方法的兼容性和重命名（详见代码中的 TODO 部分）
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
sofa-ark-parent/support/ark-gradle-plugin/build.gradle (2)
15-30: Enhance publishing configuration for production readiness
Consider the following improvements:
- Add artifact signing for security
- Configure distribution repositories beyond mavenLocal()
- Add POM metadata (description, developers, SCM info)
Example configuration:
signing {
sign publishing.publications.maven
}
publishing {
publications {
maven(MavenPublication) {
pom {
name = 'SofaArk Gradle Plugin'
description = 'Gradle plugin for building SofaArk Biz and Ark packages'
url = 'https://github.com/sofastack/sofa-ark'
licenses {
license {
name = 'The Apache License, Version 2.0'
url = 'http://www.apache.org/licenses/LICENSE-2.0.txt'
}
}
}
}
}
}53-55: Enhance test configuration
The test setup needs additional configuration:
- Add test dependencies (JUnit, Mockito, etc.)
- Configure test logging
- Add test sets for integration tests
Add these test dependencies:
dependencies {
testImplementation 'org.junit.jupiter:junit-jupiter:5.9.2'
testImplementation 'org.mockito:mockito-core:5.3.1'
testImplementation gradleTestKit()
}📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- sofa-ark-parent/support/ark-gradle-plugin/build.gradle (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java (1 hunks)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ResolveMainClassName.java
🚧 Files skipped from review as they are similar to previous changes (3)
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkArchiveSupport.java
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/ArkPluginAction.java
- sofa-ark-parent/support/ark-gradle-plugin/src/main/java/com/alipay/sofa/ark/plugin/SofaArkGradlePluginExtension.java
🔇 Additional comments (3)
sofa-ark-parent/support/ark-gradle-plugin/build.gradle (3)
1-5: LGTM: Plugin configuration is appropriate
The selected plugins provide the necessary functionality for developing and publishing a Gradle plugin.
47-51: Improve dependency version management
Consider the following recommendations:
- Use version catalogs or a central version management system
- Verify compatibility between ASM 9.4 and SofaArk core
- Add constraints to prevent version conflicts
#!/bin/bash
# Check for version conflicts with SofaArk core
rg "org\.ow2\.asm:asm:"
rg "commons-compress:"32-39: Revise plugin naming for clarity
The plugin name 'DependenciesPlugin' doesn't accurately reflect its purpose of building Ark packages. Consider renaming to 'SofaArkPlugin' or 'ArkBuildPlugin'.
#!/bin/bash
# Verify the implementation class exists and is properly implemented
ast-grep --pattern 'class SofaArkGradlePlugin implements Plugin<Project> {
$$$
}'| ext { | ||
| arkGradlePluginId = 'sofa-ark-gradle-plugin' | ||
| } | ||
|
|
||
| group = 'com.alipay.sofa' | ||
| version = '1.0.0' | ||
| sourceCompatibility = '1.8' |
There was a problem hiding this comment.
💡 Codebase verification
Version and Java compatibility settings should align with parent POM
The hardcoded version '1.0.0' and Java 1.8 compatibility in the Gradle plugin should align with the parent project configuration:
- The parent POM defines
java.version=1.8, so the current Java compatibility is correct - Version should be synchronized with
sofa.ark.version.old(2.2.14) from the parent POM
- Update
version = '1.0.0'to match the parent project version - Keep
sourceCompatibility = '1.8'as it aligns with the project's Java requirements
🔗 Analysis chain
Consider using dynamic version management
The version is currently hardcoded to '1.0.0'. Consider:
- Using project-wide version management
- Aligning the version with SofaArk's versioning scheme
Review Java compatibility requirements
Java 1.8 compatibility is set. Consider upgrading to a more recent Java version if there are no specific compatibility requirements with older systems.
Let's search for Java version requirements and version management patterns in build configuration files to better understand the context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check SofaArk's Java compatibility requirements
rg -l "sourceCompatibility|targetCompatibility" | xargs cat
Length of output: 85
Script:
#!/bin/bash
# Check Java version requirements across the project
rg -i "java|jdk|sourcecompat|targetcompat" "*.gradle" "*.properties" "pom.xml" -A 2 -B 2
# Check if there's a parent build file or properties file that might define versions
fd -e gradle -e properties "^(gradle|sofa)" --type f
Length of output: 2816
* add check for auto slim config in sofa-ark-maven-plugin (#1009) (cherry picked from commit 74f7ede) * support blank in properties for slim config @lvjing2 (#1010) * add check for auto slim config in sofa-ark-maven-plugin * support blank in properties for slim config (cherry picked from commit 88fb8e2) * enable handle AfterBizStopEvent (cherry picked from commit f957314) * add clean up event handlers (cherry picked from commit 3b8c05e) * fix test (cherry picked from commit 5d66962) * simply unregister event handlers (#1014) (cherry picked from commit 6eef785) * enable not clean biz instance when stop biz failed (#1013) * simply unregister event handlers * try to fix stop failed * not remove biz install when biz stop failed * add test * rebase master (cherry picked from commit a7bc1ca) * add commit for biz state (#1015) * add comments for UNRESOLVED * add commit for biz state (cherry picked from commit 9ce12e6) * feat: support multiple version module as activated (#1018) * feat: support multiple version module as activated (cherry picked from commit 7c40ec7) * feat:baseDependencyParentIdentity_without_version (#1019) (cherry picked from commit f3b88fc) * gradle implementation for the Ark plugin (#1012) * gradle implementation for the Ark plugin --------- Co-authored-by: pmupkin <[email protected]> Co-authored-by: gaosaroma <[email protected]> (cherry picked from commit 0a41e92) * feat: initial implementation of gradle plugin #73 (#1005) * feat: initial implementation of ark gradle plugin (cherry picked from commit 30663b3) * init log space before log reinit (#1020) (cherry picked from commit 53fa805) * feat: 模块瘦身检查 && 允许不自动排除和基座相同的依赖 (#1017) * 优化显示 * fix as bizStateRecords * 更新显示 * feat: add 'build failed when exclude different base dependency'; allow not to exclude same dependency by base-starter automatically * build failed when exclude dependencies not in base * rm all gradle * fix merge * fix ut --------- Co-authored-by: leo james <[email protected]> (cherry picked from commit a99756a) --------- Co-authored-by: Lipeng <[email protected]> Co-authored-by: pmupkin <[email protected]>
Motivation
Related issue: #73
Enable SofaArk to support building Biz and Ark packages in the Gradle environment. Subsequent implementations, documentation, testing, and optimizations will be carried out in this PR.
Summary by CodeRabbit
New Features
ArkArchiveSupport,ArkBizCopyAction,ArkPluginAction,ArkJar,LoaderZipEntries,MainClassFinder,ResolveMainClassName, andDefaultTimeZoneOffset.Documentation
sofa-ark-gradle-plugin, outlining configuration and usage instructions.Bug Fixes