Support plugin management and dynamic plugin installation with extensions #984
Conversation
# Conflicts: # pom.xml
WalkthroughThe recent updates to the SOFA Ark framework include a version bump in the Changes
Possibly related PRs
Suggested labels
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: 6
Outside diff range, codebase verification and nitpick comments (3)
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/classloader/ClassLoaderServiceImpl.java (3)
193-210: Consider adding comments for clarity.The method
findExportClassLoaderByBizhas a complex logic that could benefit from additional comments to explain the steps.
264-273: Consider adding comments for clarity.The method
findExportResourceClassLoadersInOrderByBizhas a complex logic that could benefit from additional comments to explain the steps.
275-293: Consider adding comments for clarity.The method
findExportResourcePluginsInOrderByBizhas a complex logic that could benefit from additional comments to explain the steps.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (5 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/ArkServiceContainer.java (2 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (5 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/classloader/AbstractClasspathClassLoader.java (3 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/classloader/ClassLoaderServiceImpl.java (4 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.java (5 hunks)
- sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkClient.java (7 hunks)
- sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkConfigs.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java (3 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/PluginOperation.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/biz/BizFactoryService.java (3 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/classloader/ClassLoaderService.java (3 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/plugin/PluginFactoryService.java (1 hunks)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/main/java/com/alipay/sofa/ark/springboot/listener/ArkApplicationStartListener.java (3 hunks)
Additional comments not posted (45)
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/biz/BizFactoryService.java (2)
66-66: Verify the usage of the new parameterextensionUrls.Ensure that the
extensionUrlsparameter is used correctly in the implementation of this method.Verification successful
The
extensionUrlsparameter is used correctly in the implementation of thecreateBiz(File file, URL[] extensionUrls)method.The parameter is utilized to set the class path for the
BizModelin theBizFactoryServiceImplclass.
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.javaScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `extensionUrls` in the implementation of `createBiz`. # Test: Search for the method implementation. Expect: Correct usage of `extensionUrls`. rg --type java -A 10 $'Biz createBiz(File file, URL[] extensionUrls)'Length of output: 206
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `extensionUrls` in the implementation of `createBiz`. # Test: Search for the method implementation. Expect: Correct usage of `extensionUrls`. rg --type java -A 10 $'Biz createBiz(File file, URL\\[\\] extensionUrls)'Length of output: 72
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `extensionUrls` in the implementation of `createBiz`. # Test: Search for the method implementation by name and include more lines of context. rg --type java -A 20 'createBiz'Length of output: 120461
49-49: Verify the usage of the new parameterextensionUrls.Ensure that the
extensionUrlsparameter is used correctly in the implementation of this method.sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/plugin/PluginFactoryService.java (1)
63-63: Verify the usage of the new parameterextensions.Ensure that the
extensionsparameter is used correctly in the implementation of this method.Verification successful
Let's correct the regex pattern and search for the implementation of the
createPlugin(File file, URL[] extensions)method to verify the usage of theextensionsparameter.
To ensure the correct usage of the
extensionsparameter, we need to trace its usage in thecreatePluginmethod it is passed to.
Let's search for all
createPluginmethod implementations to verify if the method with theextensionsparameter exists and to understand howextensionsis used.
Verify the usage of the new parameter
extensions.The
extensionsparameter is conditionally used in the implementation of thecreatePluginmethod, influencing the method's behavior based on its presence. This confirms that the parameter is utilized correctly.
PluginFactoryServiceImpl.java:
createPlugin(File file, URL[] extensions): CallscreatePlugin(JarPluginArchive jarPluginArchive, URL[] extensions, Set<String> exportPackages>).createPlugin(JarPluginArchive jarPluginArchive, URL[] extensions, Set<String> exportPackages): Usesextensionsto determine the method's behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `extensions` in the implementation of `createPlugin`. # Test: Search for the method implementation. Expect: Correct usage of `extensions`. rg --type java -A 10 $'Plugin createPlugin(File file, URL[] extensions)'Length of output: 218
Script:
#!/bin/bash # Description: Verify the usage of the new parameter `extensions` in the implementation of `createPlugin`. # Test: Search for the method implementation. Expect: Correct usage of `extensions`. rg --type java -A 10 'Plugin createPlugin\(File file, URL\[\] extensions\)'Length of output: 3425
Script:
#!/bin/bash # Description: Verify the usage of the `extensions` parameter in the other `createPlugin` method. # Test: Search for the implementation of the other `createPlugin` method. rg --type java -A 10 'createPlugin\(JarPluginArchive jarPluginArchive, URL\[\] extensions, Set<$_>\)'Length of output: 103
Script:
#!/bin/bash # Description: Search for all `createPlugin` method implementations to verify the usage of the `extensions` parameter. # Test: Search for all `createPlugin` method implementations. rg --type java -A 10 'createPlugin'Length of output: 27591
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/classloader/ClassLoaderService.java (2)
95-95: LGTM!The method
findExportClassLoaderByBiz(Biz biz, String className)is well-defined and follows the existing patterns.
114-114: LGTM!The method
findExportResourceClassLoadersInOrderByBiz(Biz biz, String resourceName)is well-defined and follows the existing patterns.sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkConfigs.java (1)
171-173: LGTM!The method
areAllPluginsVisibleForBiz()is well-defined and follows the existing patterns. The implementation correctly retrieves the boolean value from the system properties.sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/main/java/com/alipay/sofa/ark/springboot/listener/ArkApplicationStartListener.java (2)
76-81: LGTM!The replacement of
startUpArk(event)withSofaArkBootstrap.launch(event.getArgs())unifies the method of launching the SOFA Ark framework, potentially improving maintainability and consistency.
92-103: LGTM!The update to the
isEmbedEnablemethod enhances its efficiency by eliminating unnecessary checks when the system property is already defined. The new logic is correctly implemented.sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/ArkServiceContainer.java (2)
30-30: New Import: PluginFactoryServiceThe
PluginFactoryServicehas been imported, indicating its usage within this class.
96-96: Enhancement: Integration of PluginFactoryServiceThe
startmethod now sets thePluginFactoryServicefor theArkClient, enhancing plugin management capabilities.Ensure that all function calls to
startare updated to accommodate this change.Verification successful
Enhancement: Integration of PluginFactoryService
The
startmethod now sets thePluginFactoryServicefor theArkClient, enhancing plugin management capabilities.Ensure that all function calls to
startare updated to accommodate this change.
- ArkServiceContainer.java: The main
startmethod is updated.- ArkContainer.java: Calls
arkServiceContainer.start().- ArkServiceContainerTest.java: Calls
arkServiceContainer.start()in tests.- PluginDeployServiceImpl.java: Calls
plugin.start().These locations should be verified to ensure they align with the new implementation.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `start` in the codebase. # Test: Search for the function usage. Expect: All occurrences are updated. rg --type java -A 5 $'start()'Length of output: 252226
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.java (3)
117-144: Improvement: Streamlined Logic in getFinalPluginUrlsThe
getFinalPluginUrlsmethod has been streamlined to improve readability and control flow. The exclusion of artifacts and addition of extension URLs are now handled more clearly.
155-161: Enhancement: Overloaded createPlugin MethodThe
createPluginmethod has been overloaded to include a new signature that accepts aFileand an array ofURLextensions. This enhances the flexibility of the method.
Line range hint
170-198:
Improvement: Refined Logic in createEmbedPluginThe
createEmbedPluginmethod has been modified to refine the logic for determining the class loader. The combined check foroverrideExportModeandenableClassIsolationimproves clarity.sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (3)
Line range hint
88-110:
Enhancement: Overloaded createBiz MethodThe
createBizmethod has been overloaded to include a new signature that accepts aBizArchiveand an array ofURLextension URLs. This enhances the flexibility of the method.
126-131: New Method: getMergedBizClassPathThe
getMergedBizClassPathmethod merges thebizArchiveUrlsandextensionUrlsinto a single array using Java Streams. It ensures that if no extension URLs are provided, the originalbizArchiveUrlsare returned unchanged.
133-174: New Method: resolveExportMapIfNecessaryThe
resolveExportMapIfNecessarymethod manages the dependencies of plugins and updates various maps within theBizModel. It constructs a set of plugins based on visibility configurations and specified dependent plugins, enhancing the export functionality.sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java (5)
116-116: LGTM!The constant
DEPENDENT_PLUGINSis appropriately named and defined.
205-205: LGTM!The constant
PLUGIN_CLASS_ISOLATION_ENABLEis appropriately named and defined.
206-206: LGTM!The constant
ALL_PLUGINS_VISIBLE_FOR_BIZis appropriately named and defined.
241-241: LGTM!The constant
CONFIG_INSTALL_PLUGIN_DIRis appropriately named and defined.
243-243: LGTM!The constant
BIZ_EXTENSION_URLSis appropriately named and defined.sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkClient.java (3)
180-183: LGTM!The method signature update to include
extensionUrlsas a parameter is appropriate and necessary for the new functionality.
Line range hint
185-192:
LGTM!The method signature update to include
extensionUrlsas a parameter is appropriate and necessary for the new functionality.
173-177: LGTM!The method signature update to include
extensionUrlsas a parameter is appropriate and necessary for the new functionality.sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (18)
41-41: Import statement forPluginlooks good.The import statement is necessary for the new functionality.
50-50: Import statement forConcurrentHashMaplooks good.The import statement is necessary for the caching mechanisms.
116-116: FielddependentPluginsdeclaration looks good.The field is correctly declared and will help manage plugin dependencies.
236-238: MethodgetDependentPluginsimplementation looks good.The method provides access to the
dependentPluginsfield.
240-243: MethodsetDependentPluginsimplementation looks good.The method allows setting the
dependentPluginsfield.
633-633: FieldexportClassAndClassLoaderMapdeclaration looks good.The field is correctly declared and will help manage class and class loader relationships.
634-634: FieldexportNodeAndClassLoaderMapdeclaration looks good.The field is correctly declared and will help manage node and class loader relationships.
635-635: FieldexportStemAndClassLoaderMapdeclaration looks good.The field is correctly declared and will help manage stem and class loader relationships.
638-638: FieldexportResourceAndClassLoaderMapdeclaration looks good.The field is correctly declared and will help manage resource and class loader relationships.
639-639: FieldexportPrefixStemResourceAndClassLoaderMapdeclaration looks good.The field is correctly declared and will help manage prefix stem resource and class loader relationships.
640-640: FieldexportSuffixStemResourceAndClassLoaderMapdeclaration looks good.The field is correctly declared and will help manage suffix stem resource and class loader relationships.
642-644: MethodgetExportClassAndClassLoaderMapimplementation looks good.The method provides access to the
exportClassAndClassLoaderMapfield.
646-648: MethodgetExportNodeAndClassLoaderMapimplementation looks good.The method provides access to the
exportNodeAndClassLoaderMapfield.
650-652: MethodgetExportStemAndClassLoaderMapimplementation looks good.The method provides access to the
exportStemAndClassLoaderMapfield.
654-656: MethodgetExportResourceAndClassLoaderMapimplementation looks good.The method provides access to the
exportResourceAndClassLoaderMapfield.
658-660: MethodgetExportPrefixStemResourceAndClassLoaderMapimplementation looks good.The method provides access to the
exportPrefixStemResourceAndClassLoaderMapfield.
662-664: MethodgetExportSuffixStemResourceAndClassLoaderMapimplementation looks good.The method provides access to the
exportSuffixStemResourceAndClassLoaderMapfield.
666-684: Commented-out classPluginExportCachelooks good.The commented-out code is likely for future use or reference.
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/classloader/AbstractClasspathClassLoader.java (3)
460-466: MethoddoResolveExportClassimplementation looks good.The changes enhance flexibility by differentiating handling based on the class loader type.
564-569: MethodgetExportResourceimplementation looks good.The changes enhance flexibility by differentiating handling based on the class loader type.
641-646: MethodgetExportResourcesimplementation looks good.The changes enhance flexibility by differentiating handling based on the class loader type.
| // prepare extension urls if necessary | ||
| URL[] extensionUrls = null; | ||
| if (bizOperation.getParameters().get(Constants.BIZ_EXTENSION_URLS) != null) { | ||
| Set<String> extensionLibs = StringUtils.strToSet(bizOperation.getParameters().get(Constants.BIZ_EXTENSION_URLS), Constants.COMMA_SPLIT); | ||
| List<URL> urlsList = new ArrayList<>(); | ||
| if (!extensionLibs.isEmpty()) { | ||
| for (String extension : extensionLibs) { | ||
| URL url = new URL(extension); | ||
| urlsList.add(url); | ||
| } | ||
| } | ||
| extensionUrls = urlsList.toArray(new URL[0]); | ||
| } | ||
| return installBiz(bizFile, extensionUrls, args, envs); |
There was a problem hiding this comment.
Consider adding comments for clarity and handling potential exceptions.
The method doInstallOperation has a complex logic that could benefit from additional comments to explain the steps. Additionally, handle potential exceptions when dealing with URLs.
+ try {
URL url = new URL(extension);
urlsList.add(url);
} catch (MalformedURLException e) {
getLogger().error("Invalid URL: " + extension, 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.
| // prepare extension urls if necessary | |
| URL[] extensionUrls = null; | |
| if (bizOperation.getParameters().get(Constants.BIZ_EXTENSION_URLS) != null) { | |
| Set<String> extensionLibs = StringUtils.strToSet(bizOperation.getParameters().get(Constants.BIZ_EXTENSION_URLS), Constants.COMMA_SPLIT); | |
| List<URL> urlsList = new ArrayList<>(); | |
| if (!extensionLibs.isEmpty()) { | |
| for (String extension : extensionLibs) { | |
| URL url = new URL(extension); | |
| urlsList.add(url); | |
| } | |
| } | |
| extensionUrls = urlsList.toArray(new URL[0]); | |
| } | |
| return installBiz(bizFile, extensionUrls, args, envs); | |
| // prepare extension urls if necessary | |
| URL[] extensionUrls = null; | |
| if (bizOperation.getParameters().get(Constants.BIZ_EXTENSION_URLS) != null) { | |
| Set<String> extensionLibs = StringUtils.strToSet(bizOperation.getParameters().get(Constants.BIZ_EXTENSION_URLS), Constants.COMMA_SPLIT); | |
| List<URL> urlsList = new ArrayList<>(); | |
| if (!extensionLibs.isEmpty()) { | |
| for (String extension : extensionLibs) { | |
| try { | |
| URL url = new URL(extension); | |
| urlsList.add(url); | |
| } catch (MalformedURLException e) { | |
| getLogger().error("Invalid URL: " + extension, e); | |
| } | |
| } | |
| } | |
| extensionUrls = urlsList.toArray(new URL[0]); | |
| } | |
| return installBiz(bizFile, extensionUrls, args, envs); |
| public static ClientResponse installPlugin(PluginOperation pluginOperation) throws Exception { | ||
| AssertUtils.assertNotNull(pluginOperation, "pluginOperation must not be null"); | ||
|
|
||
| // prepare plugin file | ||
| File localFile = pluginOperation.getLocalFile(); | ||
| if (localFile == null && !StringUtils.isEmpty(pluginOperation.getUrl())) { | ||
| URL url = new URL(pluginOperation.getUrl()); | ||
| String pluginDir = ArkConfigs.getStringValue(Constants.CONFIG_INSTALL_PLUGIN_DIR); | ||
| File pluginDirectory = StringUtils.isEmpty(pluginDir) ? FileUtils | ||
| .createTempDir("sofa-ark") : FileUtils.mkdir(pluginDir); | ||
| localFile = new File(pluginDirectory, pluginOperation.getPluginName() + "-" | ||
| + pluginOperation.getPluginVersion() + "-" | ||
| + System.currentTimeMillis()); | ||
| try (InputStream inputStream = url.openStream()) { | ||
| FileUtils.copyInputStreamToFile(inputStream, localFile); | ||
| } | ||
| } | ||
|
|
||
| // prepare extension urls if necessary | ||
| List<String> extensionLibs = pluginOperation.getExtensionLibs(); | ||
| List<URL> urlsList = new ArrayList<>(); | ||
| if (extensionLibs != null && !extensionLibs.isEmpty()) { | ||
| for (String extension : extensionLibs) { | ||
| URL url = new URL(extension); | ||
| urlsList.add(url); | ||
| } | ||
| } | ||
| URL[] extensionUrls = urlsList.toArray(new URL[0]); | ||
|
|
||
| long start = System.currentTimeMillis(); | ||
| SimpleDateFormat sdf = new SimpleDateFormat("HH:mm:ss,SSS"); | ||
| String startDate = sdf.format(new Date(start)); | ||
|
|
||
| // create | ||
| Plugin plugin = pluginFactoryService.createPlugin(localFile, extensionUrls); | ||
| // register | ||
| pluginManagerService.registerPlugin(plugin); | ||
| // start | ||
| ClientResponse response = new ClientResponse(); | ||
| try { | ||
| plugin.start(); | ||
| long end = System.currentTimeMillis(); | ||
| response.setCode(ResponseCode.SUCCESS).setMessage( | ||
| String.format("Install Plugin: %s success, cost: %s ms, started at: %s", | ||
| plugin.getPluginName() + ":" + plugin.getVersion(), end - start, startDate)); | ||
| getLogger().info(response.getMessage()); | ||
| } catch (Throwable throwable) { | ||
| long end = System.currentTimeMillis(); | ||
| response.setCode(ResponseCode.FAILED).setMessage( | ||
| String.format("Install Plugin: %s fail,cost: %s ms, started at: %s", | ||
| plugin.getPluginName() + ":" + plugin.getVersion(), end - start, startDate)); | ||
| getLogger().error(response.getMessage(), throwable); | ||
| throw throwable; | ||
| } | ||
| return response; | ||
| } |
There was a problem hiding this comment.
Consider adding comments for clarity and handling potential exceptions.
The method installPlugin has a complex logic that could benefit from additional comments to explain the steps. Additionally, handle potential exceptions when dealing with URLs.
+ try {
URL url = new URL(extension);
urlsList.add(url);
} catch (MalformedURLException e) {
getLogger().error("Invalid URL: " + extension, 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.
| public static ClientResponse installPlugin(PluginOperation pluginOperation) throws Exception { | |
| AssertUtils.assertNotNull(pluginOperation, "pluginOperation must not be null"); | |
| // prepare plugin file | |
| File localFile = pluginOperation.getLocalFile(); | |
| if (localFile == null && !StringUtils.isEmpty(pluginOperation.getUrl())) { | |
| URL url = new URL(pluginOperation.getUrl()); | |
| String pluginDir = ArkConfigs.getStringValue(Constants.CONFIG_INSTALL_PLUGIN_DIR); | |
| File pluginDirectory = StringUtils.isEmpty(pluginDir) ? FileUtils | |
| .createTempDir("sofa-ark") : FileUtils.mkdir(pluginDir); | |
| localFile = new File(pluginDirectory, pluginOperation.getPluginName() + "-" | |
| + pluginOperation.getPluginVersion() + "-" | |
| + System.currentTimeMillis()); | |
| try (InputStream inputStream = url.openStream()) { | |
| FileUtils.copyInputStreamToFile(inputStream, localFile); | |
| } | |
| } | |
| // prepare extension urls if necessary | |
| List<String> extensionLibs = pluginOperation.getExtensionLibs(); | |
| List<URL> urlsList = new ArrayList<>(); | |
| if (extensionLibs != null && !extensionLibs.isEmpty()) { | |
| for (String extension : extensionLibs) { | |
| URL url = new URL(extension); | |
| urlsList.add(url); | |
| } | |
| } | |
| URL[] extensionUrls = urlsList.toArray(new URL[0]); | |
| long start = System.currentTimeMillis(); | |
| SimpleDateFormat sdf = new SimpleDateFormat("HH:mm:ss,SSS"); | |
| String startDate = sdf.format(new Date(start)); | |
| // create | |
| Plugin plugin = pluginFactoryService.createPlugin(localFile, extensionUrls); | |
| // register | |
| pluginManagerService.registerPlugin(plugin); | |
| // start | |
| ClientResponse response = new ClientResponse(); | |
| try { | |
| plugin.start(); | |
| long end = System.currentTimeMillis(); | |
| response.setCode(ResponseCode.SUCCESS).setMessage( | |
| String.format("Install Plugin: %s success, cost: %s ms, started at: %s", | |
| plugin.getPluginName() + ":" + plugin.getVersion(), end - start, startDate)); | |
| getLogger().info(response.getMessage()); | |
| } catch (Throwable throwable) { | |
| long end = System.currentTimeMillis(); | |
| response.setCode(ResponseCode.FAILED).setMessage( | |
| String.format("Install Plugin: %s fail,cost: %s ms, started at: %s", | |
| plugin.getPluginName() + ":" + plugin.getVersion(), end - start, startDate)); | |
| getLogger().error(response.getMessage(), throwable); | |
| throw throwable; | |
| } | |
| return response; | |
| } | |
| public static ClientResponse installPlugin(PluginOperation pluginOperation) throws Exception { | |
| AssertUtils.assertNotNull(pluginOperation, "pluginOperation must not be null"); | |
| // prepare plugin file | |
| File localFile = pluginOperation.getLocalFile(); | |
| if (localFile == null && !StringUtils.isEmpty(pluginOperation.getUrl())) { | |
| URL url = new URL(pluginOperation.getUrl()); | |
| String pluginDir = ArkConfigs.getStringValue(Constants.CONFIG_INSTALL_PLUGIN_DIR); | |
| File pluginDirectory = StringUtils.isEmpty(pluginDir) ? FileUtils | |
| .createTempDir("sofa-ark") : FileUtils.mkdir(pluginDir); | |
| localFile = new File(pluginDirectory, pluginOperation.getPluginName() + "-" | |
| + pluginOperation.getPluginVersion() + "-" | |
| + System.currentTimeMillis()); | |
| try (InputStream inputStream = url.openStream()) { | |
| FileUtils.copyInputStreamToFile(inputStream, localFile); | |
| } | |
| } | |
| // prepare extension urls if necessary | |
| List<String> extensionLibs = pluginOperation.getExtensionLibs(); | |
| List<URL> urlsList = new ArrayList<>(); | |
| if (extensionLibs != null && !extensionLibs.isEmpty()) { | |
| for (String extension : extensionLibs) { | |
| try { | |
| URL url = new URL(extension); | |
| urlsList.add(url); | |
| } catch (MalformedURLException e) { | |
| getLogger().error("Invalid URL: " + extension, e); | |
| } | |
| } | |
| } | |
| URL[] extensionUrls = urlsList.toArray(new URL[0]); | |
| long start = System.currentTimeMillis(); | |
| SimpleDateFormat sdf = new SimpleDateFormat("HH:mm:ss,SSS"); | |
| String startDate = sdf.format(new Date(start)); | |
| // create | |
| Plugin plugin = pluginFactoryService.createPlugin(localFile, extensionUrls); | |
| // register | |
| pluginManagerService.registerPlugin(plugin); | |
| // start | |
| ClientResponse response = new ClientResponse(); | |
| try { | |
| plugin.start(); | |
| long end = System.currentTimeMillis(); | |
| response.setCode(ResponseCode.SUCCESS).setMessage( | |
| String.format("Install Plugin: %s success, cost: %s ms, started at: %s", | |
| plugin.getPluginName() + ":" + plugin.getVersion(), end - start, startDate)); | |
| getLogger().info(response.getMessage()); | |
| } catch (Throwable throwable) { | |
| long end = System.currentTimeMillis(); | |
| response.setCode(ResponseCode.FAILED).setMessage( | |
| String.format("Install Plugin: %s fail,cost: %s ms, started at: %s", | |
| plugin.getPluginName() + ":" + plugin.getVersion(), end - start, startDate)); | |
| getLogger().error(response.getMessage(), throwable); | |
| throw throwable; | |
| } | |
| return response; | |
| } |
|
|
||
| private String pluginName; | ||
|
|
||
| private String pluginVersion; |
There was a problem hiding this comment.
Consider adding validation for pluginVersion.
Ensure that the pluginVersion is not null or empty.
+ public void setPluginVersion(String pluginVersion) {
+ if (pluginVersion == null || pluginVersion.trim().isEmpty()) {
+ throw new IllegalArgumentException("Plugin version cannot be null or empty");
+ }
+ this.pluginVersion = pluginVersion;
+ }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 String pluginVersion; | |
| private String pluginVersion; | |
| public void setPluginVersion(String pluginVersion) { | |
| if (pluginVersion == null || pluginVersion.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Plugin version cannot be null or empty"); | |
| } | |
| this.pluginVersion = pluginVersion; | |
| } |
|
|
||
| private File localFile; | ||
|
|
||
| private List<String> extensionLibs; |
There was a problem hiding this comment.
Consider adding validation for extensionLibs.
Ensure that the extensionLibs list is not null.
+ public void setExtensionLibs(List<String> extensionLibs) {
+ if (extensionLibs == null) {
+ throw new IllegalArgumentException("Extension libraries list cannot be null");
+ }
+ this.extensionLibs = extensionLibs;
+ }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 List<String> extensionLibs; | |
| private List<String> extensionLibs; | |
| public void setExtensionLibs(List<String> extensionLibs) { | |
| if (extensionLibs == null) { | |
| throw new IllegalArgumentException("Extension libraries list cannot be null"); | |
| } | |
| this.extensionLibs = extensionLibs; | |
| } |
|
|
||
| private OperationType operationType; | ||
|
|
||
| private String url; |
There was a problem hiding this comment.
Consider adding validation for url.
Ensure that the url is a valid URL format.
+ public void setUrl(String url) {
+ try {
+ new URL(url);
+ } catch (MalformedURLException e) {
+ throw new IllegalArgumentException("Invalid URL format");
+ }
+ this.url = url;
+ }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 String url; | |
| private String url; | |
| public void setUrl(String url) { | |
| try { | |
| new URL(url); | |
| } catch (MalformedURLException e) { | |
| throw new IllegalArgumentException("Invalid URL format"); | |
| } | |
| this.url = url; | |
| } |
|
|
||
| public class PluginOperation { | ||
|
|
||
| private String pluginName; |
There was a problem hiding this comment.
Consider adding validation for pluginName.
Ensure that the pluginName is not null or empty.
+ public void setPluginName(String pluginName) {
+ if (pluginName == null || pluginName.trim().isEmpty()) {
+ throw new IllegalArgumentException("Plugin name cannot be null or empty");
+ }
+ this.pluginName = pluginName;
+ }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 String pluginName; | |
| private String pluginName; | |
| public void setPluginName(String pluginName) { | |
| if (pluginName == null || pluginName.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Plugin name cannot be null or empty"); | |
| } | |
| this.pluginName = pluginName; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java
2.biz的dependent-plugins参数通过params传入 3.plugin支持安装的时候传入name 4.BizFactoryServiceImpl#getBizUcp 这里构造biz的ucp的时候 把所有plugin都add进来了 这里应该是只需要加载biz依赖的plugin 5.ArkClient提供一个checkplugin/biz是否存在的接口
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (5 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.java (5 hunks)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/api/ArkClientTest.java (4 hunks)
- sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkClient.java (7 hunks)
- sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ClientResponse.java (3 hunks)
- sofa-ark-parent/core/exception/src/main/java/com/alipay/sofa/ark/exception/ArkLoaderException.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/BizConfig.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/PluginConfig.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/biz/BizFactoryService.java (3 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/plugin/PluginFactoryService.java (2 hunks)
Additional comments not posted (50)
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/PluginConfig.java (6)
22-24: LGTM!The getter method is correctly implemented.
The code changes are approved.
26-28: LGTM!The setter method is correctly implemented.
The code changes are approved.
30-32: LGTM!The getter method is correctly implemented.
The code changes are approved.
34-36: LGTM!The setter method is correctly implemented.
The code changes are approved.
38-40: LGTM!The getter method is correctly implemented.
The code changes are approved.
42-44: LGTM!The setter method is correctly implemented.
The code changes are approved.
sofa-ark-parent/core/exception/src/main/java/com/alipay/sofa/ark/exception/ArkLoaderException.java (3)
27-28: LGTM!The constructor modification improves exception handling by storing the throwable in a private field.
The code changes are approved.
Also applies to: 34-35
41-43: LGTM!The method is correctly implemented to avoid performance issues.
The code changes are approved.
44-46: LGTM!The method is correctly implemented to retrieve the underlying cause of the exception.
The code changes are approved.
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/BizConfig.java (10)
31-33: LGTM!The getter method is correctly implemented.
The code changes are approved.
35-37: LGTM!The setter method is correctly implemented.
The code changes are approved.
39-41: LGTM!The getter method is correctly implemented.
The code changes are approved.
43-45: LGTM!The setter method is correctly implemented.
The code changes are approved.
47-49: LGTM!The getter method is correctly implemented.
The code changes are approved.
51-53: LGTM!The setter method is correctly implemented.
The code changes are approved.
55-57: LGTM!The getter method is correctly implemented.
The code changes are approved.
59-61: LGTM!The setter method is correctly implemented.
The code changes are approved.
63-65: LGTM!The getter method is correctly implemented.
The code changes are approved.
67-69: LGTM!The setter method is correctly implemented.
The code changes are approved.
sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ClientResponse.java (3)
35-35: LGTM!The new private field
pluginInfosis correctly added.The code changes are approved.
64-66: LGTM!The new public method
getPluginInfos()is correctly implemented.The code changes are approved.
68-71: LGTM!The new public method
setPluginInfos(Set<Plugin> pluginInfos)is correctly implemented.The code changes are approved.
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/plugin/PluginFactoryService.java (2)
64-65: LGTM! But verify the method implementation.The new overloaded method
createPlugin(File file, URL[] extensions)is correctly added. Ensure that the method is properly implemented in the corresponding class.Run the following script to verify the method implementation:
Verification successful
Method Implementation Verified
The
createPlugin(File file, URL[] extensions)method is correctly implemented in thePluginFactoryServiceImplclass. The implementation aligns with the expected functionality of handling plugin creation using the provided file and extensions.
- Location:
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.javaScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the new overloaded method `createPlugin(File file, URL[] extensions)`. # Test: Search for the method implementation. Expect: The method should be implemented in the corresponding class. rg --type java -A 10 $'createPlugin(File file, URL[] extensions)'Length of output: 197
Script:
#!/bin/bash # Description: Verify the implementation of the new overloaded method `createPlugin(File file, URL[] extensions)`. # Test: Search for the method implementation. Expect: The method should be implemented in the corresponding class. rg --type java -A 10 'createPlugin\(File file, URL\[\] extensions\)'Length of output: 3618
66-67: LGTM! But verify the method implementation.The new overloaded method
createPlugin(File file, PluginConfig pluginConfig)is correctly added. Ensure that the method is properly implemented in the corresponding class.Run the following script to verify the method implementation:
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/service/biz/BizFactoryService.java (4)
90-91: LGTM! But verify the method implementation.The new overloaded method
createBiz(BizArchive bizArchive, BizConfig bizConfig)is correctly added. Ensure that the method is properly implemented in the corresponding class.Run the following script to verify the method implementation:
50-50: LGTM! But verify the method implementation.The new overloaded method
createBiz(BizArchive bizArchive, URL[] extensionUrls)is correctly added. Ensure that the method is properly implemented in the corresponding class.Run the following script to verify the method implementation:
Verification successful
Method Implementation Verified
The method
createBiz(BizArchive bizArchive, URL[] extensionUrls)is correctly implemented in theBizFactoryServiceImplclass. The implementation sets the extension URLs in aBizConfigobject and utilizes it to create aBizinstance.
- Location:
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.javaScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the new overloaded method `createBiz(BizArchive bizArchive, URL[] extensionUrls)`. # Test: Search for the method implementation. Expect: The method should be implemented in the corresponding class. rg --type java -A 10 $'createBiz(BizArchive bizArchive, URL[] extensionUrls)'Length of output: 230
Script:
#!/bin/bash # Description: Verify the implementation of the new overloaded method `createBiz(BizArchive bizArchive, URL[] extensionUrls)`. # Test: Search for the method implementation. Expect: The method should be implemented in the corresponding class. rg --type java -A 10 $'createBiz(BizArchive bizArchive, URL\\[\\] extensionUrls)'Length of output: 80
Script:
#!/bin/bash # Description: Search for any method named `createBiz` to verify its implementation. # Test: Search for any method named `createBiz`. Expect: Find implementations or variations of the method. rg --type java -A 10 'createBiz'Length of output: 87168
82-83: LGTM! But verify the method implementation.The new overloaded method
createBiz(File file, BizConfig bizConfig)is correctly added. Ensure that the method is properly implemented in the corresponding class.Run the following script to verify the method implementation:
Verification successful
Method Implementation Verified
The
createBiz(File file, BizConfig bizConfig)method is correctly implemented in theBizFactoryServiceImplclass, fulfilling the interface contract. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the new overloaded method `createBiz(File file, BizConfig bizConfig)`. # Test: Search for the method implementation. Expect: The method should be implemented in the corresponding class. rg --type java -A 10 $'createBiz(File file, BizConfig bizConfig)'Length of output: 66
Script:
#!/bin/bash # Find the class implementing the BizFactoryService interface ast-grep --lang java --pattern 'class $_ implements BizFactoryService { $$$ }'Length of output: 80
Script:
#!/bin/bash # Search for references to BizFactoryService to find potential implementing classes or usage rg --type java 'BizFactoryService'Length of output: 8729
Script:
#!/bin/bash # Search for the implementation of the createBiz method in BizFactoryServiceImpl ast-grep --lang java --pattern 'class BizFactoryServiceImpl { $$$ createBiz(File file, BizConfig bizConfig) { $$$ } $$$ }'Length of output: 124
Script:
#!/bin/bash # Search for any implementation of the createBiz(File file, BizConfig bizConfig) method in the codebase rg --type java 'createBiz\(File file, BizConfig bizConfig\)'Length of output: 427
67-67: LGTM! But verify the method implementation.The new overloaded method
createBiz(File file, URL[] extensionUrls)is correctly added. Ensure that the method is properly implemented in the corresponding class.Run the following script to verify the method implementation:
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.java (5)
123-129: LGTM!The function is correctly implemented and utilizes the
extensionsparameter as expected.The code changes are approved.
131-161: LGTM!The function is correctly implemented and utilizes the
pluginConfigparameter as expected.The code changes are approved.
165-175: LGTM!The function is correctly implemented and sets the attributes as expected.
The code changes are approved.
204-235: LGTM!The function is correctly implemented and handles the URL construction as expected.
The code changes are approved.
Line range hint
204-241: LGTM!The function is correctly implemented and handles the export mappings as expected.
The code changes are approved.
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (7)
86-91: LGTM!The function is correctly implemented and utilizes the
extensionUrlsparameter as expected.The code changes are approved.
100-105: LGTM!The function is correctly implemented and utilizes the
extensionUrlsparameter as expected.The code changes are approved.
108-113: LGTM!The function is correctly implemented and utilizes the
bizOperationparameter as expected.The code changes are approved.
116-119: LGTM!The function is correctly implemented and utilizes the
bizConfigparameter as expected.The code changes are approved.
122-160: LGTM!The function is correctly implemented and sets the attributes as expected.
The code changes are approved.
165-175: LGTM!The function is correctly implemented and sets the attributes as expected.
The code changes are approved.
197-201: LGTM!The function is correctly implemented and handles the URL merging as expected.
The code changes are approved.
sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/api/ArkClientTest.java (3)
261-261: LGTM!The function is correctly implemented and utilizes the
BizConfigparameter as expected.The code changes are approved.
310-310: LGTM!The function is correctly implemented and utilizes the
BizConfigparameter as expected.The code changes are approved.
325-325: LGTM!The function is correctly implemented and utilizes the
BizConfigparameter as expected.The code changes are approved.
sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkClient.java (7)
121-123: LGTM!The method is correctly implemented.
The code changes are approved.
125-127: LGTM!The method is correctly implemented.
The code changes are approved.
183-185: LGTM!The refactoring enhances the encapsulation of configuration details related to business installations.
The code changes are approved.
188-193: LGTM!The update ensures that the installation process is more coherent and maintainable.
The code changes are approved.
402-421: LGTM!The update adds robustness to the method by preparing extension URLs if necessary.
The code changes are approved.
445-509: LGTM!The method correctly handles the installation of plugins, including error handling and logging.
The code changes are approved.
511-540: LGTM!The method correctly handles the verification of plugins.
The code changes are approved.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- pom.xml (1 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (6 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/classloader/AbstractClasspathClassLoader.java (3 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.java (5 hunks)
- sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkClient.java (7 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/BizConfig.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/PluginConfig.java (1 hunks)
Files skipped from review due to trivial changes (1)
- pom.xml
Files skipped from review as they are similar to previous changes (2)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/BizConfig.java
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/model/PluginConfig.java
Additional comments not posted (20)
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (8)
83-84: LGTM!The method is correctly implemented.
The code changes are approved.
87-91: LGTM!The method is correctly implemented.
The code changes are approved.
94-97: LGTM!The method is correctly implemented.
The code changes are approved.
100-105: LGTM!The method is correctly implemented.
The code changes are approved.
108-113: LGTM!The method is correctly implemented.
The code changes are approved.
116-119: LGTM!The method is correctly implemented.
The code changes are approved.
Line range hint
122-165: LGTM!The method is correctly implemented and enhances configurability.
The code changes are approved.
170-180: LGTM!The method is correctly implemented.
The code changes are approved.
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.java (3)
124-129: LGTM!The method is correctly implemented.
The code changes are approved.
132-169: LGTM!The method is correctly implemented and enhances configurability.
The code changes are approved.
211-242: LGTM!The method is correctly implemented and the changes improve clarity and functionality.
The code changes are approved.
sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkClient.java (6)
183-184: LGTM!The method is correctly implemented and enhances configurability.
The code changes are approved.
Line range hint
187-214: LGTM!The method is correctly implemented and enhances configurability.
The code changes are approved.
401-421: LGTM!The method is correctly implemented and enhances flexibility.
The code changes are approved.
445-509: LGTM!The method is correctly implemented and enhances flexibility.
The code changes are approved.
511-513: LGTM!The method is correctly implemented.
The code changes are approved.
515-540: LGTM!The method is correctly implemented.
The code changes are approved.
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/classloader/AbstractClasspathClassLoader.java (3)
460-466: LGTM!The conditional logic for
BizClassLoaderandPluginClassLoaderimproves the accuracy of class resolution.The code changes are approved.
564-575: LGTM!The conditional logic for
BizClassLoaderandPluginClassLoaderimproves the accuracy of resource resolution.The code changes are approved.
644-652: LGTM!The conditional logic for
BizClassLoaderandPluginClassLoaderimproves the accuracy of resource resolution.The code changes are approved.
# Conflicts: # pom.xml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java (3 hunks)
Additional comments not posted (5)
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java (5)
116-116: LGTM!The constant
DEPENDENT_PLUGINSis correctly defined and follows the naming conventions.The code changes are approved.
205-205: LGTM!The constant
PLUGIN_CLASS_ISOLATION_ENABLEis correctly defined and follows the naming conventions.The code changes are approved.
206-206: LGTM!The constant
ALL_PLUGINS_VISIBLE_FOR_BIZis correctly defined and follows the naming conventions.The code changes are approved.
241-241: LGTM!The constant
CONFIG_INSTALL_PLUGIN_DIRis correctly defined and follows the naming conventions.The code changes are approved.
243-243: LGTM!The constant
BIZ_EXTENSION_URLSis correctly defined and follows the naming conventions.The code changes are approved.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (6 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/test/TestClassLoader.java (1 hunks)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/api/ArkClientTest.java (4 hunks)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceTest.java (2 hunks)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/classloader/BizClassLoaderTest.java (9 hunks)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/classloader/ClassLoaderHookTest.java (2 hunks)
Additional comments not posted (24)
sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceTest.java (2)
36-36: LGTM!Importing all constants from the
Constantsclass can enhance readability and reduce the need for individual constant imports.
71-71: LGTM!Setting the system property
ALL_PLUGINS_VISIBLE_FOR_BIZtoBoolean.TRUE.toString()likely affects the visibility of plugins for the business context during the test execution. This change could influence the behavior of the plugin registration process, ensuring that all plugins are accessible when the business is created and registered.sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/test/TestClassLoader.java (1)
70-70: LGTM!The code change is approved. Invoking the superclass's method for setting the business model is important for maintaining the integrity of the class hierarchy and ensuring that the
BizModelis properly initialized in the context of the parent class.sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/classloader/ClassLoaderHookTest.java (3)
22-22: LGTM!The new import statements are required to use the
BizManagerServicefrom theArkServiceContainerHolderin the test.Also applies to: 26-26
57-59: LGTM!The code changes are correctly using the
BizManagerServiceto set theBizModelon thebizClassLoader. This allows the test to interact with the business model, which is crucial for validating the behavior of the class loader in the context of the business logic.
Line range hint
1-21: SkippedThe code is unchanged and does not require a review.
Also applies to: 23-25, 27-56, 60-148
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (5)
Line range hint
121-167: LGTM!The code changes in the
createBiz(BizArchive bizArchive, BizConfig bizConfig)method look good:
- The null check on
bizConfigensures that it's not null before using it.- The merging of the
BizArchive's URLs and the extension URLs allows for extending the classpath.- The resolution of the export map based on the dependent plugins specified in the
BizConfigor the manifest provides flexibility in specifying the dependencies.
170-180: LGTM!The
createEmbedMasterBiz(ClassLoader masterClassLoader)method looks good:
- It simplifies the process of creating an embedded master
Bizinstance by encapsulating the setup logic and using predefined attributes.- The attributes are set to reasonable defaults.
- It uses the provided
masterClassLoaderfor theClassPathandClassLoader.
Line range hint
182-199: LGTM!The
prepareBizArchive(File file)method looks good:
- It handles the creation of the appropriate
BizArchivebased on theisEmbedEnableconfiguration.- If embedding is enabled, it unpacks the
Fileand creates anExplodedBizArchive.- If embedding is disabled, it creates a
JarBizArchivefrom theFile.
202-206: LGTM!The
getMergedBizClassPath(URL[] bizArchiveUrls, URL[] extensionUrls)method looks good:
- It handles the merging of the
bizArchiveUrlsandextensionUrlsto create a merged classpath.- If the
extensionUrlsisnullor empty, it returns thebizArchiveUrlsas is, avoiding unnecessary operations.- The use of
Stream.concatprovides a concise way to concatenate the arrays.
209-246: LGTM!The
resolveExportMapIfNecessary(BizModel bizModel, List<String> dependentPlugins)method looks good:
- It handles the resolution of the export map of the
BizModelbased on the dependent plugins.- The behavior is controlled by the
areAllPluginsVisibleForBizconfiguration and thedependentPluginslist.- It populates the various export maps of the
BizModelbased on the export information of each dependent plugin.sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/api/ArkClientTest.java (6)
25-25: LGTM!The newly added import statement for
BizConfigclass is approved.
258-258: LGTM!The updated mocked behavior of
bizFactoryServiceMockto expect aBizConfigobject is approved.
261-261: LGTM!The updated method invocation of
installBizwith aBizConfigobject is approved.
302-302: LGTM!The updated mocked behavior of
bizFactoryServiceMockto expect aBizConfigobject is approved.
310-310: LGTM!The updated method invocation of
installBizwith aBizConfigobject is approved.
325-325: LGTM!The updated method invocation of
installBizwith aBizConfigobject is approved.sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/classloader/BizClassLoaderTest.java (7)
105-106: LGTM!The code changes are approved.
174-179: LGTM!The code changes are approved.
271-273: LGTM!The code changes are approved.
354-361: LGTM!The code changes are approved.
376-377: LGTM!The code changes are approved.
425-425: LGTM!The code changes are approved.
439-441: LGTM!The code changes are approved.
...container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| bizModel.setDependentPlugins(plugins); | ||
| for (Plugin plugin : plugins) { |
There was a problem hiding this comment.
所以每个 plugin，是需要定义自己的 export 内容的，这部分需要在使用文档里说明。
sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkConfigs.java
Outdated
Show resolved
Hide resolved
...container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java
Show resolved
Hide resolved
sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkClient.java
Show resolved
Hide resolved
...re-impl/container/src/test/java/com/alipay/sofa/ark/container/service/api/ArkClientTest.java
Show resolved
Hide resolved
...ain/java/com/alipay/sofa/ark/container/service/classloader/AbstractClasspathClassLoader.java
Show resolved
Hide resolved
...container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java
Show resolved
Hide resolved
...arter/src/main/java/com/alipay/sofa/ark/springboot/listener/ArkApplicationStartListener.java
Outdated
Show resolved
Hide resolved
|
https://www.sofastack.tech/projects/sofa-boot/sofa-ark-class-loader-delegation/ |
...container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java
Show resolved
Hide resolved
...ner/src/main/java/com/alipay/sofa/ark/container/service/plugin/PluginFactoryServiceImpl.java
Show resolved
Hide resolved
# Conflicts: # pom.xml
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (1)
639-639: Typographical Error in CommentThere's a typo in the comment. It should likely be:
-/* export cache and classloader relationship cache */ +/* export resource and classloader relationship cache */
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (5 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java (3 hunks)
Additional comments not posted (8)
sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java (6)
116-116: LGTM: New constant for plugin dependenciesThe addition of
DEPENDENT_PLUGINSconstant aligns well with the PR objective of supporting plugin management. It provides a standardized key for specifying plugin dependencies, which is crucial for the new plugin management features.
244-244: 🛠️ Refactor suggestionSuggestion: Consider a more descriptive name and add documentation
The addition of
BIZ_EXTENSION_URLSconstant supports the installation with extension parameters. However, the name could be more descriptive, and its purpose needs further explanation.Suggestions:
- Consider renaming to something more descriptive, e.g.,
BIZ_EXTENSION_RESOURCE_URLS- Add documentation covering:
- The purpose of these extension URLs
- How to use this configuration option
- Any limitations or considerations when specifying these URLs
Also, ensure that there are proper security measures in place for handling these URLs, such as validation and sanitization.
To ensure proper documentation and security measures, please run the following commands:
#!/bin/bash # Check if documentation for BIZ_EXTENSION_URLS exists grep -r "bizExtensionUrls" docs/ # Check if URL validation is implemented grep -r "validateUrl" src/
207-207: LGTM: New constant for plugin visibility, but needs clarificationThe addition of
ALL_PLUGINS_VISIBLE_FOR_BIZconstant is a good enhancement for plugin management in relation to business logic. However, the implications of this setting need further explanation.Could you please add documentation covering:
- The default behavior if this setting is not specified
- The impact of enabling this setting on the plugin management system
- Any potential security or performance considerations
To ensure proper documentation, please run the following command:
#!/bin/bash # Check if documentation for ALL_PLUGINS_VISIBLE_FOR_BIZ exists grep -r "sofa.ark.all.plugins.visible.for.biz" docs/
242-242: LGTM: New constant for plugin installation directory, consider security implicationsThe addition of
CONFIG_INSTALL_PLUGIN_DIRconstant is crucial for supporting dynamic plugin installation. However, it's important to ensure that this directory is properly secured and managed.Consider adding the following security measures:
- Implement access controls to restrict write permissions to this directory
- Validate and sanitize plugin files before installation
- Implement a mechanism to verify the integrity and authenticity of plugins before installation
Also, please add documentation covering:
- How to configure this directory
- Best practices for managing the plugin installation directory
- Any limitations or considerations when using this feature
To ensure proper security measures and documentation, please run the following commands:
#!/bin/bash # Check if security measures for CONFIG_INSTALL_PLUGIN_DIR are implemented grep -r "validatePlugin" src/ grep -r "verifyPluginIntegrity" src/ # Check if documentation for CONFIG_INSTALL_PLUGIN_DIR exists grep -r "com.alipay.sofa.ark.plugin.dir" docs/
Line range hint
116-244: Summary: New constants enhance plugin management, but require comprehensive documentationThe new constants added to the
Constants.javafile align well with the PR objectives of enhancing plugin management, supporting dynamic installation, and allowing for extension parameters. However, to ensure proper understanding and usage of these new features, it's crucial to provide comprehensive documentation.Action items:
- Add detailed documentation for each new constant, explaining their purpose, usage, and any security implications.
- Consider renaming
BIZ_EXTENSION_URLSto be more descriptive.- Implement and document security measures for plugin installation and URL handling.
- Update the PR description to include information about the new class isolation feature.
These enhancements will greatly improve the usability and security of the new plugin management features.
To ensure all necessary documentation is in place, please run the following command:
#!/bin/bash # Check if documentation for all new constants exists grep -r "dependent-plugins" docs/ grep -r "sofa.ark.plugin.class.isolation.enable" docs/ grep -r "sofa.ark.all.plugins.visible.for.biz" docs/ grep -r "com.alipay.sofa.ark.plugin.dir" docs/ grep -r "bizExtensionUrls" docs/
206-206: LGTM: New constant for plugin class isolation, but needs documentationThe addition of
PLUGIN_CLASS_ISOLATION_ENABLEconstant is a good enhancement for plugin management. However, the concept of plugin class isolation is not mentioned in the PR objectives.Could you please add documentation explaining:
- The purpose of plugin class isolation
- How to use this configuration option
- Its impact on the plugin management system
To ensure proper documentation, please run the following command:
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/model/BizModel.java (2)
41-41: Import Statement ApprovedThe import of
com.alipay.sofa.ark.spi.model.Pluginis necessary for the new plugin management functionality.
635-666: Ensure Thread Safety and Document Usage of Cache FieldsThe addition of caching fields enhances plugin management efficiency. Ensure that interactions with these
ConcurrentHashMapinstances are thread-safe and consider adding comments to document their purpose and usage for better maintainability.
| @@ -111,6 +113,8 @@ public class BizModel implements Biz { | |||
|
|
|||
| private List<BizStateRecord> bizStateRecords = new CopyOnWriteArrayList<>(); | |||
|
|
|||
| private Set<Plugin> dependentPlugins = new HashSet<>(); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider Thread Safety for dependentPlugins
If dependentPlugins may be accessed by multiple threads concurrently, consider using a thread-safe collection like CopyOnWriteArraySet or synchronizing access appropriately to prevent concurrent modification issues.
| public Set<Plugin> getDependentPlugins() { | ||
| return dependentPlugins; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return Unmodifiable Set in getDependentPlugins()
Returning the mutable dependentPlugins set directly allows external code to modify the internal state of BizModel. To preserve encapsulation, consider returning an unmodifiable view of the set.
Example:
public Set<Plugin> getDependentPlugins() {
- return dependentPlugins;
+ return Collections.unmodifiableSet(dependentPlugins);
}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 Set<Plugin> getDependentPlugins() { | |
| return dependentPlugins; | |
| } | |
| public Set<Plugin> getDependentPlugins() { | |
| return Collections.unmodifiableSet(dependentPlugins); | |
| } |
| public BizModel setDependentPlugins(Set<Plugin> dependentPlugins) { | ||
| this.dependentPlugins = dependentPlugins; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make a Defensive Copy in setDependentPlugins()
Assigning the provided set directly to dependentPlugins may lead to unintended modifications if the caller alters the set after passing it. Create a defensive copy to safeguard internal state.
Example:
public BizModel setDependentPlugins(Set<Plugin> dependentPlugins) {
- this.dependentPlugins = dependentPlugins;
+ this.dependentPlugins = new HashSet<>(dependentPlugins);
return this;
}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 BizModel setDependentPlugins(Set<Plugin> dependentPlugins) { | |
| this.dependentPlugins = dependentPlugins; | |
| return this; | |
| } | |
| public BizModel setDependentPlugins(Set<Plugin> dependentPlugins) { | |
| this.dependentPlugins = new HashSet<>(dependentPlugins); | |
| return this; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (3)
sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/main/java/com/alipay/sofa/ark/springboot/listener/ArkApplicationStartListener.java (1)
Line range hint
65-84: Consider simplifying the method structure and revisiting theisEmbedEnable()check.
As per the previous comment, the
isEmbedEnable()check might be redundant. Please review if this check is still necessary.The separate branches for Spring Boot 1 and 2 could potentially be combined, as the only difference is the event name being checked. Consider refactoring to improve readability and maintainability:
@Override public void onApplicationEvent(SpringApplicationEvent event) { try { if (isEmbedEnable()) { ArkConfigs.setEmbedEnable(true); startUpArkEmbed(event); return; } String eventName = event.getClass().getCanonicalName(); if ((isSpringBoot2() && APPLICATION_STARTING_EVENT.equals(eventName)) || (isSpringBoot1() && APPLICATION_STARTED_EVENT.equals(eventName))) { SofaArkBootstrap.launch(event.getArgs()); } } catch (Throwable e) { throw new RuntimeException("Meet exception when determine whether to start SOFAArk!", e); } }This refactoring combines the Spring Boot 1 and 2 checks, reducing code duplication and improving readability.
sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (2)
122-122: Typo in exception messageThe exception message "Archive must be a ark biz!" should use "an" instead of "a".
Apply this diff to correct the message:
-AssertUtils.isTrue(isArkBiz(bizArchive), "Archive must be a ark biz!"); +AssertUtils.isTrue(isArkBiz(bizArchive), "Archive must be an ark biz!");
Line range hint
191-193: Potential issue with deleting the original input fileThe code deletes the original input
fileafter unpacking it. Deleting the input file might not be expected behavior and could cause issues if the file is needed elsewhere or by other processes.Consider avoiding deletion of the original input file or ensure that the deletion is intended and safe.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceImpl.java (6 hunks)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceTest.java (1 hunks)
- sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkConfigs.java (1 hunks)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java (3 hunks)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/main/java/com/alipay/sofa/ark/springboot/listener/ArkApplicationStartListener.java (2 hunks)
Files skipped from review due to trivial changes (1)
- sofa-ark-parent/core-impl/container/src/test/java/com/alipay/sofa/ark/container/service/biz/BizFactoryServiceTest.java
Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/core/spi/src/main/java/com/alipay/sofa/ark/spi/constant/Constants.java
Additional comments not posted (3)
sofa-ark-parent/core/api/src/main/java/com/alipay/sofa/ark/api/ArkConfigs.java (1)
171-173:⚠️ Potential issueConsider adding a default value and documentation for the new method.
The new
isBizSpecifyDependentPluginsEnable()method aligns with the PR objectives for supporting plugin management. However, there are a few concerns:
- The method doesn't specify a default value, which could lead to unexpected behavior if the system property is not set.
- There's no documentation explaining the purpose and usage of this new feature.
- The previous reviewer raised a concern about default values and compatibility issues, which is still relevant.
Consider the following improvements:
- Add a default value to ensure consistent behavior:
public static boolean isBizSpecifyDependentPluginsEnable() { - return Boolean.getBoolean(Constants.BIZ_SPECIFY_DEPENDENT_PLUGINS_ENABLE); + return Boolean.getBoolean(Constants.BIZ_SPECIFY_DEPENDENT_PLUGINS_ENABLE, false); }
- Add JavaDoc comments to explain the purpose and usage of this method.
Could you please clarify if there are any potential compatibility issues with existing systems when introducing this new feature? Also, let's verify the usage of the constant across the codebase:
Verification successful
Improvements Approved
The concerns have been addressed, and the suggested improvements can be implemented:
Add a default value to ensure consistent behavior:
public static boolean isBizSpecifyDependentPluginsEnable() { return Boolean.getBoolean(Constants.BIZ_SPECIFY_DEPENDENT_PLUGINS_ENABLE, false); }Add JavaDoc comments to explain the purpose and usage of this method:
/** * Checks if business-specified dependent plugins are enabled. * * @return {@code true} if enabled; {@code false} otherwise. */ public static boolean isBizSpecifyDependentPluginsEnable() { return Boolean.getBoolean(Constants.BIZ_SPECIFY_DEPENDENT_PLUGINS_ENABLE, false); }No compatibility issues detected with existing systems based on the current usage of the constant.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of BIZ_SPECIFY_DEPENDENT_PLUGINS_ENABLE constant # Test: Search for the constant usage rg --type java "BIZ_SPECIFY_DEPENDENT_PLUGINS_ENABLE"Length of output: 436
sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/main/java/com/alipay/sofa/ark/springboot/listener/ArkApplicationStartListener.java (2)
Line range hint
1-153: Summary of review for ArkApplicationStartListener.javaThe changes in this file, particularly in the
onApplicationEventmethod, align well with the PR objectives of supporting dynamic plugin installation and management. The switch to usingSofaArkBootstrap.launch(event.getArgs())appears to be a step in the right direction.However, there are opportunities for further improvement:
- Removing the unused
startUpArkmethod.- Revisiting the necessity of the
isEmbedEnable()check.- Simplifying the method structure to reduce code duplication and improve readability.
These suggestions aim to enhance the overall code quality and maintainability. Please consider implementing these changes in a future iteration if not addressed in this PR.
76-76: Approve changes and suggest removal of unused method.The changes to use
SofaArkBootstrap.launch(event.getArgs())instead ofstartUpArk(event)look good. This aligns with the PR objectives of supporting dynamic plugin installation and management.However, the
startUpArkmethod is now unused and can be removed to keep the codebase clean.Consider removing the unused
startUpArkmethod:- public void startUpArk(SpringApplicationEvent event) { - if (LAUNCH_CLASSLOADER_NAME.equals(this.getClass().getClassLoader().getClass().getName())) { - SofaArkBootstrap.launch(event.getArgs()); - } - }Let's verify if there are any remaining usages of the
startUpArkmethod in the codebase:Also applies to: 81-81
Verification successful
Verified:
startUpArkMethod is UnusedThe
startUpArkmethod is no longer utilized in the codebase and can be safely removed to maintain code cleanliness.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usages of the startUpArk method # Test: Search for method calls to startUpArk rg --type java 'startUpArk\s*\(' # Test: Search for method definition of startUpArk (in case it's used via reflection) rg --type java 'public\s+void\s+startUpArk\s*\('Length of output: 525
| Plugin plugin = pluginManagerService.getPluginByName(pluginName); | ||
| plugins.add(plugin); |
There was a problem hiding this comment.
Potential NullPointerException when adding plugins
The method getPluginByName(pluginName) may return null if no plugin with the specified name is found. Adding a null plugin to the plugins set can lead to NullPointerException or unexpected behavior later on.
Please add a null check before adding the plugin to the set. For example:
Plugin plugin = pluginManagerService.getPluginByName(pluginName);
if (plugin != null) {
plugins.add(plugin);
} else {
// Handle the case where the plugin is not found
}| @Override | ||
| public Biz createBiz(BizArchive bizArchive, URL[] extensionUrls) throws IOException { | ||
| BizConfig bizConfig = new BizConfig(); | ||
| bizConfig.setExtensionUrls(extensionUrls); | ||
| return createBiz(bizArchive, bizConfig); | ||
| } | ||
|
|
||
| @Override | ||
| public Biz createBiz(File file) throws IOException { | ||
| BizArchive bizArchive = prepareBizArchive(file); | ||
| return createBiz(bizArchive, new BizConfig()); | ||
| } | ||
|
|
||
| @Override | ||
| public Biz createBiz(File file, URL[] extensionUrls) throws IOException { | ||
| BizArchive bizArchive = prepareBizArchive(file); | ||
| BizConfig bizConfig = new BizConfig(); | ||
| bizConfig.setExtensionUrls(extensionUrls); | ||
| return createBiz(bizArchive, bizConfig); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring overloaded createBiz methods to reduce code duplication
The overloaded createBiz methods between lines 85 and 105 have similar implementations where a BizConfig is created, configured, and then passed to another createBiz method. This code duplication can be reduced for better maintainability.
Consider refactoring to eliminate duplication, possibly by introducing helper methods or utilizing default parameter values.
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Summary by CodeRabbit
New Features
createBizmethods allowing for additional parameters and configurations.BizConfigfor more dynamicBizcreation.Bizinstances with new methods.Bug Fixes
Bizcreation process with improved error handling and logging.