feature: union static deploy #118
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughThe changes introduce a Maven Mojo Changes
Sequence Diagram(s)sequenceDiagram
participant MavenBuild as Maven Build
participant KouplelessBaseIntegrateBizMojo
participant ArkConfigHolder
participant Constants
participant Resources
MavenBuild->>KouplelessBaseIntegrateBizMojo: Execute Mojo
KouplelessBaseIntegrateBizMojo->>ArkConfigHolder: Init Configurations
ArkConfigHolder->>Constants: Retrieve Config Paths
Constants-->>ArkConfigHolder: Config Paths
ArkConfigHolder-->>KouplelessBaseIntegrateBizMojo: Configurations Initialized
KouplelessBaseIntegrateBizMojo->>Resources: Copy Files and Directories
Resources-->>KouplelessBaseIntegrateBizMojo: Files Integrated
KouplelessBaseIntegrateBizMojo-->>MavenBuild: Mojo Execution Complete
Assessment against linked issues
Poem
Tip You can get early access to new featuresEnable the 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseBuildPrePackageMojo.java (2 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojo.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/common/JarFileUtils.java (2 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/constant/Constants.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/ArkConfigHolder.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/KouplelessIntegrateBizConfig.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/MavenDependencyAdapterMapping.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/utils/ParseUtils.java (1 hunks)
Files skipped from review due to trivial changes (3)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseBuildPrePackageMojo.java
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/constant/Constants.java
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/MavenDependencyAdapterMapping.java
Comment on lines
29
to
42
| @Getter | ||
| public class KouplelessIntegrateBizConfig { | ||
| private Set<String> fileURLs = new HashSet<>(); | ||
|
|
||
| private Set<String> localDirURLs = new HashSet<>(); | ||
|
|
||
| public void addFileURLs(Set<String> urls) { | ||
| fileURLs.addAll(urls); | ||
| } | ||
|
|
||
| public void addLocalDirURLs(Set<String> urls) { | ||
| localDirURLs.addAll(urls); | ||
| } | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Good use of Java Collections and Lombok for simplicity in KouplelessIntegrateBizConfig.
The use of HashSet ensures uniqueness and @Getter from Lombok simplifies the boilerplate code. However, consider adding thread safety if concurrent modifications are expected.
- private Set<String> fileURLs = new HashSet<>();
- private Set<String> localDirURLs = new HashSet<>();
+ private Set<String> fileURLs = Collections.synchronizedSet(new HashSet<>());
+ private Set<String> localDirURLs = Collections.synchronizedSet(new HashSet<>());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.
Suggested change
| @Getter | |
| public class KouplelessIntegrateBizConfig { | |
| private Set<String> fileURLs = new HashSet<>(); | |
| private Set<String> localDirURLs = new HashSet<>(); | |
| public void addFileURLs(Set<String> urls) { | |
| fileURLs.addAll(urls); | |
| } | |
| public void addLocalDirURLs(Set<String> urls) { | |
| localDirURLs.addAll(urls); | |
| } | |
| } | |
| @Getter | |
| public class KouplelessIntegrateBizConfig { | |
| private Set<String> fileURLs = Collections.synchronizedSet(new HashSet<>()); | |
| private Set<String> localDirURLs = Collections.synchronizedSet(new HashSet<>()); | |
| public void addFileURLs(Set<String> urls) { | |
| fileURLs.addAll(urls); | |
| } | |
| public void addLocalDirURLs(Set<String> urls) { | |
| localDirURLs.addAll(urls); | |
| } | |
| } |
Comment on lines
+34
to
+47
| public static Set<String> getSetValues(Properties prop, String confKey) { | ||
| if (null == prop) { | ||
| return newHashSet(); | ||
| } | ||
| String[] values = StringUtils.split(prop.getProperty(confKey), COMMA_SPLIT); | ||
| return values == null ? newHashSet() : newHashSet(values); | ||
| } | ||
|
|
||
| public static Set<String> getStringSet(Map<String, Object> yaml, String confKey) { | ||
| if (null != yaml && yaml.containsKey(confKey) && null != yaml.get(confKey)) { | ||
| return newHashSet((List<String>) yaml.get(confKey)); | ||
| } | ||
| return newHashSet(); | ||
| } |
Contributor
There was a problem hiding this comment.
Review of ParseUtils methods for extracting sets from properties and maps.
The methods correctly handle null inputs and use efficient libraries for string manipulation and set creation. Ensure that the type casting in getStringSet is safe and consider adding generics to avoid unchecked casting.
- public static Set<String> getStringSet(Map<String, Object> yaml, String confKey) {
+ public static Set<String> getStringSet(Map<String, Object> yaml, String confKey) {
+ // Ensure type safety with a check before casting
+ if (!(yaml.get(confKey) instanceof List)) throw new IllegalArgumentException("Expected a List type for key: " + confKey);
return newHashSet((List<String>) yaml.get(confKey));
}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.
Suggested change
| public static Set<String> getSetValues(Properties prop, String confKey) { | |
| if (null == prop) { | |
| return newHashSet(); | |
| } | |
| String[] values = StringUtils.split(prop.getProperty(confKey), COMMA_SPLIT); | |
| return values == null ? newHashSet() : newHashSet(values); | |
| } | |
| public static Set<String> getStringSet(Map<String, Object> yaml, String confKey) { | |
| if (null != yaml && yaml.containsKey(confKey) && null != yaml.get(confKey)) { | |
| return newHashSet((List<String>) yaml.get(confKey)); | |
| } | |
| return newHashSet(); | |
| } | |
| public static Set<String> getStringSet(Map<String, Object> yaml, String confKey) { | |
| // Ensure type safety with a check before casting | |
| if (!(yaml.get(confKey) instanceof List)) throw new IllegalArgumentException("Expected a List type for key: " + confKey); | |
| return newHashSet((List<String>) yaml.get(confKey)); | |
| } |
Comment on lines
+75
to
+84
| @SneakyThrows | ||
| public static Map<String, Object> getMainAttributes(String bizUrl) { | ||
| try (JarFile jarFile = new JarFile(bizUrl)) { | ||
| Manifest manifest = jarFile.getManifest(); | ||
| Preconditions.checkState(manifest != null, "Manifest file not found in the JAR."); | ||
| Map<String, Object> result = new HashMap<>(); | ||
| manifest.getMainAttributes().forEach((k, v) -> result.put(k.toString(), v)); | ||
| return result; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
New method getMainAttributes in JarFileUtils needs improvements in exception handling.
The method uses Preconditions.checkState from Guava, which is good for state validation. However, consider enhancing the error message to include more context about the failure.
- Preconditions.checkState(manifest != null, "Manifest file not found in the JAR.");
+ Preconditions.checkState(manifest != null, "Manifest file not found in JAR: " + bizUrl);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.
Suggested change
| @SneakyThrows | |
| public static Map<String, Object> getMainAttributes(String bizUrl) { | |
| try (JarFile jarFile = new JarFile(bizUrl)) { | |
| Manifest manifest = jarFile.getManifest(); | |
| Preconditions.checkState(manifest != null, "Manifest file not found in the JAR."); | |
| Map<String, Object> result = new HashMap<>(); | |
| manifest.getMainAttributes().forEach((k, v) -> result.put(k.toString(), v)); | |
| return result; | |
| } | |
| } | |
| @SneakyThrows | |
| public static Map<String, Object> getMainAttributes(String bizUrl) { | |
| try (JarFile jarFile = new JarFile(bizUrl)) { | |
| Manifest manifest = jarFile.getManifest(); | |
| Preconditions.checkState(manifest != null, "Manifest file not found in JAR: " + bizUrl); | |
| Map<String, Object> result = new HashMap<>(); | |
| manifest.getMainAttributes().forEach((k, v) -> result.put(k.toString(), v)); | |
| return result; | |
| } | |
| } |
Comment on lines
42
to
105
| public static Properties getArkProperties(String baseDir) { | ||
| return arkProperties == null ? initArkProperties(baseDir) : arkProperties; | ||
| } | ||
|
|
||
| public static Map<String, Object> getArkYaml(String baseDir) { | ||
| return arkYaml == null ? initArkYaml(baseDir) : arkYaml; | ||
| } | ||
|
|
||
| @SneakyThrows | ||
| private static Map<String, Object> initArkYaml(String baseDir) { | ||
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | ||
| + ARK_YML_FILE; | ||
| File configFile = new File(configPath); | ||
| if (!configFile.exists()) { | ||
| System.out.printf( | ||
| "koupleless-base-build-plugin: extension-config %s not found, will not config it", | ||
| configPath); | ||
| return newHashMap(); | ||
| } | ||
|
|
||
| System.out.printf( | ||
| "koupleless-base-build-plugin: find extension-config %s and will config it\n", | ||
| configPath); | ||
|
|
||
| try (FileInputStream fis = new FileInputStream(configPath)) { | ||
| Yaml yaml = new Yaml(); | ||
| arkYaml = yaml.load(fis); | ||
| return arkYaml; | ||
| } catch (IOException ex) { | ||
| System.out.printf( | ||
| String.format("failed to parse excludes artifacts from %s.", configPath), ex); | ||
| throw ex; | ||
| } | ||
| } | ||
|
|
||
| @SneakyThrows | ||
| private static Properties initArkProperties(String baseDir) { | ||
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | ||
| + ARK_PROPERTIES_FILE; | ||
| File configFile = new File(configPath); | ||
| if (!configFile.exists()) { | ||
| System.out.printf( | ||
| "koupleless-base-build-plugin: extension-config %s not found, will not config it%n\n", | ||
| configPath); | ||
| return new Properties(); | ||
| } | ||
|
|
||
| System.out.printf( | ||
| "koupleless-base-build-plugin: find extension-config %s and will config it", | ||
| configPath); | ||
|
|
||
| Properties prop = new Properties(); | ||
| try (FileInputStream fis = new FileInputStream(configPath)) { | ||
| prop.load(fis); | ||
| arkProperties = prop; | ||
| return prop; | ||
| } catch (IOException ex) { | ||
| System.out.printf( | ||
| String.format("koupleless-base-build-plugin: failed to read extension-config %s.", | ||
| configPath), | ||
| ex); | ||
| throw ex; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Review of configuration loading methods in ArkConfigHolder.
Methods use System.out.printf for logging, which is not suitable for production. Replace with a proper logging framework. Additionally, consider catching specific exceptions rather than using SneakyThrows.
- System.out.printf("...");
+ logger.info("..."); // Use a logging framework like SLF4J
- @SneakyThrows
+ try {
+ ...
+ } catch (SpecificException ex) {
+ logger.error("Error message", ex);
+ throw new CustomException("Error message", ex);
+ }Committable suggestion was skipped due to low confidence.
Comment on lines
67
to
166
| /** {@inheritDoc} */ | ||
| @Override | ||
| public void execute() { | ||
| try { | ||
| initKouplelessIntegrateBizConfig(); | ||
| integrateBizToResource(); | ||
| } catch (Throwable t) { | ||
| getLog().error(t); | ||
| throw new RuntimeException(t); | ||
| } | ||
| } | ||
|
|
||
| private void integrateBizToResource() throws IOException { | ||
| copyFilesToResource(); | ||
| copyLocalDirsToResource(); | ||
| } | ||
|
|
||
| private void copyFilesToResource() throws IOException { | ||
| File targetDir = new File(outputDirectory, SOFA_ARK_MODULE); | ||
| for (String urlStr : kouplelessIntegrateBizConfig.getFileURLs()) { | ||
| File targetFile = new File(targetDir, getFullRevision(urlStr)); | ||
| if (!targetFile.exists()) { | ||
| URL url = new URL(urlStr); | ||
| try (InputStream inputStream = url.openStream()) { | ||
| FileUtils.copyInputStreamToFile(inputStream, targetFile); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private String getFullRevision(String bizUrl) { | ||
| // file:///xxx/{name}-{version}-ark-biz.jar | ||
| if (bizUrl.startsWith(FILE_PREFIX)) { | ||
| return StringUtils.substringAfterLast(bizUrl, File.separator); | ||
| } | ||
|
|
||
| // https://xxx/{name}-{version}.jar | ||
| if (bizUrl.startsWith(HTTP_PREFIX) || bizUrl.startsWith(HTTPS_PREFIX)) { | ||
| return StringUtils.substringAfterLast(bizUrl, "/"); | ||
| } | ||
|
|
||
| return "unknownBizName-unknownBizVersion.jar"; | ||
| } | ||
|
|
||
| private void copyLocalDirsToResource() throws IOException { | ||
| File targetDir = new File(outputDirectory, SOFA_ARK_MODULE); | ||
| for (String dirPath : kouplelessIntegrateBizConfig.getLocalDirURLs()) { | ||
| for (File bizSourceFile : getBizFileFromLocalFileSystem(dirPath)) { | ||
| File targetFile = new File(targetDir, bizSourceFile.getName()); | ||
| if (!targetFile.exists()) { | ||
| FileUtils.copyFileToDirectory(bizSourceFile, targetDir); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @SneakyThrows | ||
| public List<File> getBizFileFromLocalFileSystem(String absoluteBizDirPath) { | ||
| List<File> bizFiles = new ArrayList<>(); | ||
| Files.walkFileTree(new File(absoluteBizDirPath).toPath(), new SimpleFileVisitor<Path>() { | ||
| @Override | ||
| public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { | ||
| Path absolutePath = file.toAbsolutePath(); | ||
| if (absolutePath.toString().endsWith(".jar")) { | ||
| Map<String, Object> attributes = JarFileUtils | ||
| .getMainAttributes(absolutePath.toString()); | ||
| if (attributes.containsKey("Ark-Biz-Name")) { | ||
| getLog().info(String.format("Found biz jar file: %s", absolutePath)); | ||
| bizFiles.add(file.toFile()); | ||
| } | ||
| } | ||
|
|
||
| return FileVisitResult.CONTINUE; | ||
| } | ||
| }); | ||
|
|
||
| return bizFiles; | ||
| } | ||
|
|
||
| private void initKouplelessIntegrateBizConfig() { | ||
| readFromArkPropertiesFile(); | ||
| readFromArkYamlFile(); | ||
| } | ||
|
|
||
| private void readFromArkYamlFile() { | ||
| Map<String, Object> arkYaml = ArkConfigHolder.getArkYaml(baseDir.getAbsolutePath()); | ||
| kouplelessIntegrateBizConfig | ||
| .addFileURLs(ParseUtils.getStringSet(arkYaml, EXTENSION_INTEGRATE_LOCAL_URLS)); | ||
| kouplelessIntegrateBizConfig | ||
| .addLocalDirURLs(ParseUtils.getStringSet(arkYaml, EXTENSION_INTEGRATE_LOCAL_DIRS)); | ||
| } | ||
|
|
||
| private void readFromArkPropertiesFile() { | ||
| Properties arkProperties = ArkConfigHolder.getArkProperties(baseDir.getAbsolutePath()); | ||
| kouplelessIntegrateBizConfig | ||
| .addFileURLs(ParseUtils.getSetValues(arkProperties, EXTENSION_INTEGRATE_LOCAL_URLS)); | ||
| kouplelessIntegrateBizConfig.addLocalDirURLs( | ||
| ParseUtils.getSetValues(arkProperties, EXTENSION_INTEGRATE_LOCAL_DIRS)); | ||
|
|
||
| } |
Contributor
There was a problem hiding this comment.
Review of execute and configuration methods in KouplelessBaseIntegrateBizMojo.
The methods are well-structured and follow Maven's guidelines for Mojos. However, improve exception handling by not rethrowing RuntimeException without context.
- throw new RuntimeException(t);
+ throw new RuntimeException("Error during integration of business artifacts", t);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.
Suggested change
| /** {@inheritDoc} */ | |
| @Override | |
| public void execute() { | |
| try { | |
| initKouplelessIntegrateBizConfig(); | |
| integrateBizToResource(); | |
| } catch (Throwable t) { | |
| getLog().error(t); | |
| throw new RuntimeException(t); | |
| } | |
| } | |
| private void integrateBizToResource() throws IOException { | |
| copyFilesToResource(); | |
| copyLocalDirsToResource(); | |
| } | |
| private void copyFilesToResource() throws IOException { | |
| File targetDir = new File(outputDirectory, SOFA_ARK_MODULE); | |
| for (String urlStr : kouplelessIntegrateBizConfig.getFileURLs()) { | |
| File targetFile = new File(targetDir, getFullRevision(urlStr)); | |
| if (!targetFile.exists()) { | |
| URL url = new URL(urlStr); | |
| try (InputStream inputStream = url.openStream()) { | |
| FileUtils.copyInputStreamToFile(inputStream, targetFile); | |
| } | |
| } | |
| } | |
| } | |
| private String getFullRevision(String bizUrl) { | |
| // file:///xxx/{name}-{version}-ark-biz.jar | |
| if (bizUrl.startsWith(FILE_PREFIX)) { | |
| return StringUtils.substringAfterLast(bizUrl, File.separator); | |
| } | |
| // https://xxx/{name}-{version}.jar | |
| if (bizUrl.startsWith(HTTP_PREFIX) || bizUrl.startsWith(HTTPS_PREFIX)) { | |
| return StringUtils.substringAfterLast(bizUrl, "/"); | |
| } | |
| return "unknownBizName-unknownBizVersion.jar"; | |
| } | |
| private void copyLocalDirsToResource() throws IOException { | |
| File targetDir = new File(outputDirectory, SOFA_ARK_MODULE); | |
| for (String dirPath : kouplelessIntegrateBizConfig.getLocalDirURLs()) { | |
| for (File bizSourceFile : getBizFileFromLocalFileSystem(dirPath)) { | |
| File targetFile = new File(targetDir, bizSourceFile.getName()); | |
| if (!targetFile.exists()) { | |
| FileUtils.copyFileToDirectory(bizSourceFile, targetDir); | |
| } | |
| } | |
| } | |
| } | |
| @SneakyThrows | |
| public List<File> getBizFileFromLocalFileSystem(String absoluteBizDirPath) { | |
| List<File> bizFiles = new ArrayList<>(); | |
| Files.walkFileTree(new File(absoluteBizDirPath).toPath(), new SimpleFileVisitor<Path>() { | |
| @Override | |
| public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { | |
| Path absolutePath = file.toAbsolutePath(); | |
| if (absolutePath.toString().endsWith(".jar")) { | |
| Map<String, Object> attributes = JarFileUtils | |
| .getMainAttributes(absolutePath.toString()); | |
| if (attributes.containsKey("Ark-Biz-Name")) { | |
| getLog().info(String.format("Found biz jar file: %s", absolutePath)); | |
| bizFiles.add(file.toFile()); | |
| } | |
| } | |
| return FileVisitResult.CONTINUE; | |
| } | |
| }); | |
| return bizFiles; | |
| } | |
| private void initKouplelessIntegrateBizConfig() { | |
| readFromArkPropertiesFile(); | |
| readFromArkYamlFile(); | |
| } | |
| private void readFromArkYamlFile() { | |
| Map<String, Object> arkYaml = ArkConfigHolder.getArkYaml(baseDir.getAbsolutePath()); | |
| kouplelessIntegrateBizConfig | |
| .addFileURLs(ParseUtils.getStringSet(arkYaml, EXTENSION_INTEGRATE_LOCAL_URLS)); | |
| kouplelessIntegrateBizConfig | |
| .addLocalDirURLs(ParseUtils.getStringSet(arkYaml, EXTENSION_INTEGRATE_LOCAL_DIRS)); | |
| } | |
| private void readFromArkPropertiesFile() { | |
| Properties arkProperties = ArkConfigHolder.getArkProperties(baseDir.getAbsolutePath()); | |
| kouplelessIntegrateBizConfig | |
| .addFileURLs(ParseUtils.getSetValues(arkProperties, EXTENSION_INTEGRATE_LOCAL_URLS)); | |
| kouplelessIntegrateBizConfig.addLocalDirURLs( | |
| ParseUtils.getSetValues(arkProperties, EXTENSION_INTEGRATE_LOCAL_DIRS)); | |
| } | |
| /** {@inheritDoc} */ | |
| @Override | |
| public void execute() { | |
| try { | |
| initKouplelessIntegrateBizConfig(); | |
| integrateBizToResource(); | |
| } catch (Throwable t) { | |
| getLog().error(t); | |
| throw new RuntimeException("Error during integration of business artifacts", t); | |
| } | |
| } | |
| private void integrateBizToResource() throws IOException { | |
| copyFilesToResource(); | |
| copyLocalDirsToResource(); | |
| } | |
| private void copyFilesToResource() throws IOException { | |
| File targetDir = new File(outputDirectory, SOFA_ARK_MODULE); | |
| for (String urlStr : kouplelessIntegrateBizConfig.getFileURLs()) { | |
| File targetFile = new File(targetDir, getFullRevision(urlStr)); | |
| if (!targetFile.exists()) { | |
| URL url = new URL(urlStr); | |
| try (InputStream inputStream = url.openStream()) { | |
| FileUtils.copyInputStreamToFile(inputStream, targetFile); | |
| } | |
| } | |
| } | |
| } | |
| private String getFullRevision(String bizUrl) { | |
| // file:///xxx/{name}-{version}-ark-biz.jar | |
| if (bizUrl.startsWith(FILE_PREFIX)) { | |
| return StringUtils.substringAfterLast(bizUrl, File.separator); | |
| } | |
| // https://xxx/{name}-{version}.jar | |
| if (bizUrl.startsWith(HTTP_PREFIX) || bizUrl.startsWith(HTTPS_PREFIX)) { | |
| return StringUtils.substringAfterLast(bizUrl, "/"); | |
| } | |
| return "unknownBizName-unknownBizVersion.jar"; | |
| } | |
| private void copyLocalDirsToResource() throws IOException { | |
| File targetDir = new File(outputDirectory, SOFA_ARK_MODULE); | |
| for (String dirPath : kouplelessIntegrateBizConfig.getLocalDirURLs()) { | |
| for (File bizSourceFile : getBizFileFromLocalFileSystem(dirPath)) { | |
| File targetFile = new File(targetDir, bizSourceFile.getName()); | |
| if (!targetFile.exists()) { | |
| FileUtils.copyFileToDirectory(bizSourceFile, targetDir); | |
| } | |
| } | |
| } | |
| } | |
| @SneakyThrows | |
| public List<File> getBizFileFromLocalFileSystem(String absoluteBizDirPath) { | |
| List<File> bizFiles = new ArrayList<>(); | |
| Files.walkFileTree(new File(absoluteBizDirPath).toPath(), new SimpleFileVisitor<Path>() { | |
| @Override | |
| public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { | |
| Path absolutePath = file.toAbsolutePath(); | |
| if (absolutePath.toString().endsWith(".jar")) { | |
| Map<String, Object> attributes = JarFileUtils | |
| .getMainAttributes(absolutePath.toString()); | |
| if (attributes.containsKey("Ark-Biz-Name")) { | |
| getLog().info(String.format("Found biz jar file: %s", absolutePath)); | |
| bizFiles.add(file.toFile()); | |
| } | |
| } | |
| return FileVisitResult.CONTINUE; | |
| } | |
| }); | |
| return bizFiles; | |
| } | |
| private void initKouplelessIntegrateBizConfig() { | |
| readFromArkPropertiesFile(); | |
| readFromArkYamlFile(); | |
| } | |
| private void readFromArkYamlFile() { | |
| Map<String, Object> arkYaml = ArkConfigHolder.getArkYaml(baseDir.getAbsolutePath()); | |
| kouplelessIntegrateBizConfig | |
| .addFileURLs(ParseUtils.getStringSet(arkYaml, EXTENSION_INTEGRATE_LOCAL_URLS)); | |
| kouplelessIntegrateBizConfig | |
| .addLocalDirURLs(ParseUtils.getStringSet(arkYaml, EXTENSION_INTEGRATE_LOCAL_DIRS)); | |
| } | |
| private void readFromArkPropertiesFile() { | |
| Properties arkProperties = ArkConfigHolder.getArkProperties(baseDir.getAbsolutePath()); | |
| kouplelessIntegrateBizConfig | |
| .addFileURLs(ParseUtils.getSetValues(arkProperties, EXTENSION_INTEGRATE_LOCAL_URLS)); | |
| kouplelessIntegrateBizConfig.addLocalDirURLs( | |
| ParseUtils.getSetValues(arkProperties, EXTENSION_INTEGRATE_LOCAL_DIRS)); | |
| } |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/resourcesToCopy/biz2-bootstrap-0.0.1-SNAPSHOT-ark-biz.jaris excluded by!**/*.jarkoupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/resourcesToCopy/staticDeployDir/biz1-bootstrap-0.0.1-SNAPSHOT-ark-biz.jaris excluded by!**/*.jar
Files selected for processing (8)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojo.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/constant/Constants.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/KouplelessIntegrateBizConfig.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/utils/OSUtils.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojoTest.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/java/com/alipay/sofa/koupleless/utils/MockUtils.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/ark.properties (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/ark.yml (1 hunks)
Files skipped from review due to trivial changes (2)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/java/com/alipay/sofa/koupleless/utils/MockUtils.java
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/ark.properties
Files skipped from review as they are similar to previous changes (3)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojo.java
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/constant/Constants.java
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/KouplelessIntegrateBizConfig.java
Additional context used
yamllint
koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/ark.yml
[error] 5-5:
Additional comments not posted (2)
koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/utils/OSUtils.java (1)
31-37: Validate OS-specific file protocol handling.The method
getLocalFileProtocolPrefixcorrectly handles file protocol prefixes for different operating systems. However, consider adding more detailed OS checks and handling edge cases or less common OS names.koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojoTest.java (1)
41-49: Ensure comprehensive testing for configuration initialization.The test
testInitKouplelessIntegrateBizConfigchecks the size of file URLs and local directories. Consider adding assertions for the actual content of these configurations to ensure they are loaded correctly.
Comment on lines
1
to
5
| integrateBizURLs: | ||
| - file:///aaa.jar | ||
| integrateLocalDirs: | ||
| - /aaa | ||
| - /bbb No newline at end of file |
Contributor
There was a problem hiding this comment.
Ensure consistent indentation in YAML files.
The static analysis tool flagged an error related to indentation at line 5. Proper indentation is crucial for the correct parsing of YAML files.
integrateLocalDirs:
- - /aaa
- - /bbb
+ - /aaa
+ - /bbbCommittable 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.
Suggested change
| integrateBizURLs: | |
| - file:///aaa.jar | |
| integrateLocalDirs: | |
| - /aaa | |
| - /bbb | |
| integrateBizURLs: | |
| - file:///aaa.jar | |
| integrateLocalDirs: | |
| - /aaa | |
| - /bbb |
Tools
yamllint
[error] 5-5:
Comment on lines
51
to
66
| @Test | ||
| public void testIntegrateBizToResource() throws Exception { | ||
| KouplelessBaseIntegrateBizMojo mojo = new KouplelessBaseIntegrateBizMojo(); | ||
| mojo.outputDirectory = getResourceAsFile("mockOutputDirectory"); | ||
| FileUtils.cleanDirectory(mojo.outputDirectory); | ||
|
|
||
| String bizFileURL = OSUtils.getLocalFileProtocolPrefix() + getResourceAsFile( | ||
| "resourcesToCopy/biz2-bootstrap-0.0.1-SNAPSHOT-ark-biz.jar"); | ||
| String bizDir = getResourceAsFile("resourcesToCopy/staticDeployDir").getAbsolutePath(); | ||
| mojo.kouplelessIntegrateBizConfig = KouplelessIntegrateBizConfig.builder() | ||
| .fileURLs(Sets.newHashSet(bizFileURL)).localDirs(Sets.newHashSet(bizDir)).build(); | ||
| mojo.integrateBizToResource(); | ||
|
|
||
| File targetDir = new File(mojo.outputDirectory,SOFA_ARK_MODULE); | ||
| assertEquals(2, targetDir.listFiles().length); | ||
| } |
Contributor
There was a problem hiding this comment.
Validate resource integration functionality.
The test testIntegrateBizToResource checks the number of files in the target directory. It's important to also verify the integrity and correctness of the files themselves, not just their count.
// Suggestion to add file content verification
assertEquals("Expected file content", FileUtils.readFileToString(new File(targetDir, "expectedFileName"), StandardCharsets.UTF_8));
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/constant/Constants.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojoTest.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/bootstrap.properties (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/bootstrap.yml (1 hunks)
Files skipped from review due to trivial changes (1)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/bootstrap.properties
Files skipped from review as they are similar to previous changes (2)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/constant/Constants.java
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojoTest.java
Additional context used
yamllint
koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/bootstrap.yml
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
Additional comments not posted (4)
koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/test/resources/mockBaseDir/conf/ark/bootstrap.yml (4)
1-1: Ensure consistency in configuration keys.The configuration key
integrateBizURLsis consistent with the naming conventions used in the rest of the project.
2-2: Verify the validity of the file URL.The file URL
file:///aaa.jaris likely a placeholder. Ensure that it is replaced with a valid file path in the actual configuration.
3-3: Ensure consistency in configuration keys.The configuration key
integrateLocalDirsis consistent with the naming conventions used in the rest of the project.
4-5: Verify the validity of the directory paths.The directory paths
/aaaand/bbbare likely placeholders. Ensure that they are replaced with valid directory paths in the actual configuration.Tools
yamllint
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
| - file:///aaa.jar | ||
| integrateLocalDirs: | ||
| - /aaa | ||
| - /bbb No newline at end of file |
Contributor
There was a problem hiding this comment.
Add a new line character at the end of the file.
The file should end with a new line character to comply with POSIX standards.
- /bbb
+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.
Suggested change
| - /bbb | |
| - /bbb | |
Tools
yamllint
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
lvjing2
reviewed
Jul 11, 2024
...c/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojo.java
Outdated
Show resolved
Hide resolved
lvjing2
reviewed
Jul 11, 2024
...plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/ArkConfigHolder.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojo.java (1 hunks)
- koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/model/ArkConfigHolder.java (1 hunks)
Additional comments not posted (6)
koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/KouplelessBaseIntegrateBizMojo.java (6)
79-81: LGTM!The method is well-structured and follows good practices.
84-94: LGTM!The method is well-structured and follows good practices.
97-110: LGTM!The method is well-structured and handles different URL protocols appropriately.
112-121: LGTM!The method is well-structured and follows good practices.
147-150: LGTM!The method is well-structured and follows good practices.
152-165: LGTM!The methods are well-structured and follow good practices.
Comment on lines
+45
to
+47
| public static Properties getArkProperties(String baseDir) { | ||
| return arkProperties == null ? initArkProperties(baseDir) : arkProperties; | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid using SneakyThrows and improve logging.
Using SneakyThrows is not recommended for production code. Instead, handle exceptions explicitly and use a proper logging framework.
- @SneakyThrows
private static Properties initArkProperties(String baseDir) {
String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator
+ ARK_PROPERTIES_FILE;
File configFile = new File(configPath);
if (!configFile.exists()) {
log.info(String.format(
"koupleless-base-build-plugin: extension-config %s not found, will not config it",
configPath));
return new Properties();
}
log.info(String.format(
"koupleless-base-build-plugin: find extension-config %s and will config it",
configPath));
Properties prop = new Properties();
try (FileInputStream fis = new FileInputStream(configPath)) {
prop.load(fis);
arkProperties = prop;
return prop;
} catch (IOException ex) {
log.error(String.format(
"koupleless-base-build-plugin: failed to read extension-config from %s.",
configPath), ex);
throw new RuntimeException("Failed to read extension-config", ex);
}
}Committable suggestion was skipped due to low confidence.
Comment on lines
+49
to
+51
| public static Map<String, Object> getArkYaml(String baseDir) { | ||
| return arkYaml == null ? initArkYaml(baseDir) : arkYaml; | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid using SneakyThrows and improve logging.
Using SneakyThrows is not recommended for production code. Instead, handle exceptions explicitly and use a proper logging framework.
- @SneakyThrows
private static Map<String, Object> initArkYaml(String baseDir) {
String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator
+ ARK_YML_FILE;
File configFile = new File(configPath);
if (!configFile.exists()) {
log.info(String.format(
"koupleless-base-build-plugin: extension-config %s not found, will not config it",
configPath));
return newHashMap();
}
log.info(String.format(
"koupleless-base-build-plugin: find extension-config %s and will config it",
configPath));
try (FileInputStream fis = new FileInputStream(configPath)) {
Yaml yaml = new Yaml();
arkYaml = yaml.load(fis);
return arkYaml;
} catch (IOException ex) {
log.error(String.format("failed to parse excludes artifacts from %s.", configPath), ex);
throw new RuntimeException("Failed to parse excludes artifacts", ex);
}
}Committable suggestion was skipped due to low confidence.
Comment on lines
+53
to
+77
| @SneakyThrows | ||
| private static Map<String, Object> initArkYaml(String baseDir) { | ||
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | ||
| + ARK_YML_FILE; | ||
| File configFile = new File(configPath); | ||
| if (!configFile.exists()) { | ||
| log.info(String.format( | ||
| "koupleless-base-build-plugin: extension-config %s not found, will not config it", | ||
| configPath)); | ||
| return newHashMap(); | ||
| } | ||
|
|
||
| log.info(String.format( | ||
| "koupleless-base-build-plugin: find extension-config %s and will config it", | ||
| configPath)); | ||
|
|
||
| try (FileInputStream fis = new FileInputStream(configPath)) { | ||
| Yaml yaml = new Yaml(); | ||
| arkYaml = yaml.load(fis); | ||
| return arkYaml; | ||
| } catch (IOException ex) { | ||
| log.error(String.format("failed to parse excludes artifacts from %s.", configPath), ex); | ||
| throw ex; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Improve exception handling and logging.
Explicitly handle exceptions and use a proper logging framework.
- @SneakyThrows
private static Map<String, Object> initArkYaml(String baseDir) {
String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator
+ ARK_YML_FILE;
File configFile = new File(configPath);
if (!configFile.exists()) {
log.info(String.format(
"koupleless-base-build-plugin: extension-config %s not found, will not config it",
configPath));
return newHashMap();
}
log.info(String.format(
"koupleless-base-build-plugin: find extension-config %s and will config it",
configPath));
try (FileInputStream fis = new FileInputStream(configPath)) {
Yaml yaml = new Yaml();
arkYaml = yaml.load(fis);
return arkYaml;
} catch (IOException ex) {
log.error(String.format("failed to parse excludes artifacts from %s.", configPath), ex);
throw new RuntimeException("Failed to parse excludes artifacts", ex);
}
}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.
Suggested change
| @SneakyThrows | |
| private static Map<String, Object> initArkYaml(String baseDir) { | |
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | |
| + ARK_YML_FILE; | |
| File configFile = new File(configPath); | |
| if (!configFile.exists()) { | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: extension-config %s not found, will not config it", | |
| configPath)); | |
| return newHashMap(); | |
| } | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: find extension-config %s and will config it", | |
| configPath)); | |
| try (FileInputStream fis = new FileInputStream(configPath)) { | |
| Yaml yaml = new Yaml(); | |
| arkYaml = yaml.load(fis); | |
| return arkYaml; | |
| } catch (IOException ex) { | |
| log.error(String.format("failed to parse excludes artifacts from %s.", configPath), ex); | |
| throw ex; | |
| } | |
| } | |
| private static Map<String, Object> initArkYaml(String baseDir) { | |
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | |
| + ARK_YML_FILE; | |
| File configFile = new File(configPath); | |
| if (!configFile.exists()) { | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: extension-config %s not found, will not config it", | |
| configPath)); | |
| return newHashMap(); | |
| } | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: find extension-config %s and will config it", | |
| configPath)); | |
| try (FileInputStream fis = new FileInputStream(configPath)) { | |
| Yaml yaml = new Yaml(); | |
| arkYaml = yaml.load(fis); | |
| return arkYaml; | |
| } catch (IOException ex) { | |
| log.error(String.format("failed to parse excludes artifacts from %s.", configPath), ex); | |
| throw new RuntimeException("Failed to parse excludes artifacts", ex); | |
| } | |
| } |
Comment on lines
+79
to
+106
| @SneakyThrows | ||
| private static Properties initArkProperties(String baseDir) { | ||
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | ||
| + ARK_PROPERTIES_FILE; | ||
| File configFile = new File(configPath); | ||
| if (!configFile.exists()) { | ||
| log.info(String.format( | ||
| "koupleless-base-build-plugin: extension-config %s not found, will not config it", | ||
| configPath)); | ||
| return new Properties(); | ||
| } | ||
|
|
||
| log.info(String.format( | ||
| "koupleless-base-build-plugin: find extension-config %s and will config it", | ||
| configPath)); | ||
|
|
||
| Properties prop = new Properties(); | ||
| try (FileInputStream fis = new FileInputStream(configPath)) { | ||
| prop.load(fis); | ||
| arkProperties = prop; | ||
| return prop; | ||
| } catch (IOException ex) { | ||
| log.error(String.format( | ||
| "koupleless-base-build-plugin: failed to read extension-config from %s.", | ||
| configPath), ex); | ||
| throw ex; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Improve exception handling and logging.
Explicitly handle exceptions and use a proper logging framework.
- @SneakyThrows
private static Properties initArkProperties(String baseDir) {
String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator
+ ARK_PROPERTIES_FILE;
File configFile = new File(configPath);
if (!configFile.exists()) {
log.info(String.format(
"koupleless-base-build-plugin: extension-config %s not found, will not config it",
configPath));
return new Properties();
}
log.info(String.format(
"koupleless-base-build-plugin: find extension-config %s and will config it",
configPath));
Properties prop = new Properties();
try (FileInputStream fis = new FileInputStream(configPath)) {
prop.load(fis);
arkProperties = prop;
return prop;
} catch (IOException ex) {
log.error(String.format(
"koupleless-base-build-plugin: failed to read extension-config from %s.",
configPath), ex);
throw new RuntimeException("Failed to read extension-config", ex);
}
}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.
Suggested change
| @SneakyThrows | |
| private static Properties initArkProperties(String baseDir) { | |
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | |
| + ARK_PROPERTIES_FILE; | |
| File configFile = new File(configPath); | |
| if (!configFile.exists()) { | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: extension-config %s not found, will not config it", | |
| configPath)); | |
| return new Properties(); | |
| } | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: find extension-config %s and will config it", | |
| configPath)); | |
| Properties prop = new Properties(); | |
| try (FileInputStream fis = new FileInputStream(configPath)) { | |
| prop.load(fis); | |
| arkProperties = prop; | |
| return prop; | |
| } catch (IOException ex) { | |
| log.error(String.format( | |
| "koupleless-base-build-plugin: failed to read extension-config from %s.", | |
| configPath), ex); | |
| throw ex; | |
| } | |
| } | |
| private static Properties initArkProperties(String baseDir) { | |
| String configPath = baseDir + File.separator + ARK_CONF_BASE_DIR + File.separator | |
| + ARK_PROPERTIES_FILE; | |
| File configFile = new File(configPath); | |
| if (!configFile.exists()) { | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: extension-config %s not found, will not config it", | |
| configPath)); | |
| return new Properties(); | |
| } | |
| log.info(String.format( | |
| "koupleless-base-build-plugin: find extension-config %s and will config it", | |
| configPath)); | |
| Properties prop = new Properties(); | |
| try (FileInputStream fis = new FileInputStream(configPath)) { | |
| prop.load(fis); | |
| arkProperties = prop; | |
| return prop; | |
| } catch (IOException ex) { | |
| log.error(String.format( | |
| "koupleless-base-build-plugin: failed to read extension-config from %s.", | |
| configPath), ex); | |
| throw new RuntimeException("Failed to read extension-config", ex); | |
| } | |
| } |
Comment on lines
+125
to
+145
| public List<File> getBizFileFromLocalFileSystem(String absoluteBizDirPath) { | ||
| List<File> bizFiles = new ArrayList<>(); | ||
| Files.walkFileTree(new File(absoluteBizDirPath).toPath(), new SimpleFileVisitor<Path>() { | ||
| @Override | ||
| public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { | ||
| Path absolutePath = file.toAbsolutePath(); | ||
| if (absolutePath.toString().endsWith(".jar")) { | ||
| Map<String, Object> attributes = JarFileUtils | ||
| .getMainAttributes(absolutePath.toString()); | ||
| if (attributes.containsKey("Ark-Biz-Name")) { | ||
| getLog().info(String.format("Found biz jar file: %s", absolutePath)); | ||
| bizFiles.add(file.toFile()); | ||
| } | ||
| } | ||
|
|
||
| return FileVisitResult.CONTINUE; | ||
| } | ||
| }); | ||
|
|
||
| return bizFiles; | ||
| } |
Contributor
There was a problem hiding this comment.
Improve exception handling and logging.
Explicitly handle exceptions and use a proper logging framework.
- @SneakyThrows
public List<File> getBizFileFromLocalFileSystem(String absoluteBizDirPath) {
List<File> bizFiles = new ArrayList<>();
Files.walkFileTree(new File(absoluteBizDirPath).toPath(), new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
Path absolutePath = file.toAbsolutePath();
if (absolutePath.toString().endsWith(".jar")) {
Map<String, Object> attributes = JarFileUtils
.getMainAttributes(absolutePath.toString());
if (attributes.containsKey("Ark-Biz-Name")) {
getLog().info(String.format("Found biz jar file: %s", absolutePath));
bizFiles.add(file.toFile());
}
}
return FileVisitResult.CONTINUE;
}
});
return bizFiles;
}Committable suggestion was skipped due to low confidence.
Comment on lines
+69
to
+75
| public void execute() { | ||
| try { | ||
| initKouplelessIntegrateBizConfig(); | ||
| integrateBizToResource(); | ||
| } catch (Throwable t) { | ||
| getLog().error(t); | ||
| throw new RuntimeException(t); |
Contributor
There was a problem hiding this comment.
Improve exception handling.
Provide more context when rethrowing exceptions.
- throw new RuntimeException(t);
+ throw new RuntimeException("Error during integration of business artifacts", t);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.
Suggested change
| public void execute() { | |
| try { | |
| initKouplelessIntegrateBizConfig(); | |
| integrateBizToResource(); | |
| } catch (Throwable t) { | |
| getLog().error(t); | |
| throw new RuntimeException(t); | |
| public void execute() { | |
| try { | |
| initKouplelessIntegrateBizConfig(); | |
| integrateBizToResource(); | |
| } catch (Throwable t) { | |
| getLog().error(t); | |
| throw new RuntimeException("Error during integration of business artifacts", t); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To fix koupleless/koupleless#221
使用方式
bootstrap.properties:
bootstrap.yml
<goal>integrate-biz</goal>：Summary by CodeRabbit
New Features
Bug Fixes
MavenDependencyAdapterMappingclass.