[NAE-2129] Inject Plugin JAR into Community Edition #319
Conversation
Introduced annotations (`EntryPoint` and `EntryPointMethod`) and corresponding domain models (`Plugin`, `EntryPoint`, `Method`, and `ListenerFilter`) to support a plugin architecture. These enable defining and managing plugins, their methods, and listener filters for remote invocation.
Introduced a new interface to define plugin registration configuration. It includes methods for retrieving the plugin name, version, and entry points. This provides a foundation for standardizing plugin integration.
Introduced core classes and services to enable a plugin architecture. This includes plugin injection, entry point management, and dynamic meta-class updates for ActionDelegate. The changes set the foundation for extensible plugin support within the system.
Introduced core classes and services to enable a plugin architecture. This includes plugin injection, entry point management, and dynamic meta-class updates for ActionDelegate. The changes set the foundation for extensible plugin support within the system.
The EntryPointLoaderService interface was unused and redundant, so it has been deleted to simplify the codebase. This helps maintain cleaner and more maintainable code.
Replaced the use of EntryPointLoaderService with ApplicationContext to retrieve PluginRegistrationConfiguration beans. This simplifies the process of loading plugins and their entry points, consolidating configuration management and improving maintainability.
Updated the PluginRunner to assign the plugin version from the configuration file instead of using a hardcoded value. This ensures better flexibility and easier version management for plugins.
Introduced PluginHolder as a new field in ActionDelegate and initialized it in the constructor. This change supports enhanced plugin management within the ActionDelegate logic.
- Enhanced PluginInjector with improved meta-class injection logic and documentation. - Updated PluginRegistrationConfiguration to include metadata retrieval.
WalkthroughThis change introduces a dynamic plugin architecture to the application. It adds new domain classes, annotations, and interfaces for plugin definition and registration, along with runtime injection and method invocation mechanisms. New components facilitate the discovery, registration, and dynamic method invocation of plugin entry points and their methods during application startup. Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as PluginRunner
participant Context as ApplicationContext
participant Injector as PluginInjector
participant Holder as PluginHolder
participant Service as PluginServiceImpl
participant Bean as EntryPoint Bean
Startup->>Context: getBeansOfType(PluginRegistrationConfiguration)
Startup->>Injector: inject(plugin)
Injector->>Holder: Dynamically add PluginMeta with EntryPointMeta(s)
Note over Holder: EntryPointMeta methods delegate to PluginService
sequenceDiagram
participant Client as Application Code
participant Holder as PluginHolder
participant Service as PluginServiceImpl
participant Bean as EntryPoint Bean
Client->>Holder: PluginHolder.<plugin>.<entryPoint>.<method>(args)
Holder->>Service: call(pluginId, entryPoint, method, args)
Service->>Bean: invoke method reflectively
Bean-->>Service: return result
Service-->>Holder: return result
Holder-->>Client: return result
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (14)
nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/ListenerFilter.java (4)
12-13: Grammar: Improve class Javadoc.
The Javadoc reads "This filter define, which listener will call methods of this plugin." Consider changing to "This filter defines which listeners will invoke methods of this plugin."
17-18: Consider Lombok’s @NoArgsConstructor.
You manually defined a no-argument constructor; using Lombok’s@NoArgsConstructorwould align with existing Lombok usage and reduce boilerplate.
20-22: Enforce non-null on required fields.
SinceeventTypeanddispatchMethodare mandatory for this filter, annotate them with Lombok’s@NonNullto automatically generate null checks.
27-31: Validate builder parameters.
The@Builderconstructor doesn’t enforce non-null constraints. Consider annotating its parameters with@NonNullor adding explicit checks to prevent invalid instantiation.application-engine/src/main/groovy/com/netgrif/application/engine/plugin/meta/PluginMeta.groovy (1)
4-5: Add class-level Javadoc for PluginMeta.
Include a brief description explaining that this meta-class serves as a dynamic container for injected plugin entry points and their methods at runtime.application-engine/src/main/groovy/com/netgrif/application/engine/plugin/meta/PluginHolder.groovy (1)
4-5: Add documentation for PluginHolder.
This class is used byActionDelegateto hold dynamically injected plugin metadata. A concise Javadoc will improve readability and maintainability.application-engine/src/main/groovy/com/netgrif/application/engine/plugin/meta/EntryPointMeta.groovy (1)
3-6: Grammar: Refine class Javadoc.
Remove the unnecessary comma in "Class, that has modified meta class" and rephrase for clarity, e.g., "Class that has a modified meta class and is injected into …".application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1)
199-224: Suggest inline instantiation for thePluginholder
Rather than assigningthis.Plugin = new PluginHolder()ininit(), you can instantiatePluginat declaration to guarantee it's always non-null and cut boilerplate.
Apply this diff within the selected lines:@@ - PluginHolder Plugin + PluginHolder Plugin = new PluginHolder() @@ - this.Plugin = new PluginHolder() + // inline instantiation removes need for explicit init assignmentnae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/annotations/EntryPointMethod.java (1)
15-19: Consider validation for the listeners array.The annotation design is sound with appropriate targeting and retention. However, consider adding validation to ensure that when
listenersis specified, it contains valid configurations.You might want to add validation annotations or documentation about expected behavior when the listeners array is empty vs. when it contains filters.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/service/PluginService.java (1)
6-8: Consider adding parameter validation and security documentation.The interface design is clean and appropriate for dynamic plugin invocation. However, consider the following:
- Parameter validation: String-based identifiers could lead to runtime errors if invalid
- Security implications: Dynamic method invocation should be carefully controlled to prevent unauthorized access
Consider adding validation methods or builder patterns to ensure valid plugin/entry point/method combinations before invocation. Also, document security considerations for implementations.
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/config/PluginRegistrationConfiguration.java (1)
7-16: Consider documenting expected behavior for Map return values.The interface design is clean and provides a good contract for plugin registration. Consider adding documentation about:
- Null handling: Whether methods can return null or should return empty Maps
- Required keys: Any expected keys in the metadata Map
- Entry point validation: Expected format/constraints for entry point keys
Adding JavaDoc comments with examples would improve the interface's usability:
public interface PluginRegistrationConfiguration { + /** + * @return the plugin name, must not be null or empty + */ String getPluginName(); + /** + * @return the plugin version, must not be null or empty + */ String getVersion(); + /** + * @return map of entry point identifiers to EntryPoint objects, never null + */ Map<String, EntryPoint> getEntryPoints(); + /** + * @return plugin metadata as key-value pairs, never null but may be empty + */ Map<String, String> getMetadata(); }nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Method.java (1)
11-15: Improve javadoc comment formatting for better documentation.Consider using proper
@linkannotation for the referenced annotation class to enable IDE navigation and documentation generation.- * and are annotated with {@code com.netgrif.application.engine.adapter.spring.plugin.annotations.EntryPointMethod} annotations. These methods + * and are annotated with {@link com.netgrif.application.engine.adapter.spring.plugin.annotations.EntryPointMethod} annotations. These methodsapplication-engine/src/main/java/com/netgrif/application/engine/PluginServiceImpl.java (2)
40-42: Consider renaming method to better reflect its purposeThe method name
findMethodWithSuperClassParamsis misleading. It actually finds methods with compatible parameter types (usingisAssignableFrom), not just superclass parameters.-private Method findMethodWithSuperClassParams(Object bean, String methodToExecute, Class<?>[] requestParamTypes, - NoSuchMethodException caughtException) +private Method findMethodWithCompatibleParams(Object bean, String methodToExecute, Class<?>[] requestParamTypes, + NoSuchMethodException caughtException)
40-80: Consider adding support for varargs methodsThe current implementation doesn't handle varargs methods, which could limit plugin functionality.
Would you like me to provide an implementation that handles varargs methods?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy(3 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/plugin/PluginInjector.groovy(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/plugin/meta/EntryPointMeta.groovy(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/plugin/meta/PluginHolder.groovy(1 hunks)application-engine/src/main/groovy/com/netgrif/application/engine/plugin/meta/PluginMeta.groovy(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/PluginServiceImpl.java(1 hunks)application-engine/src/main/java/com/netgrif/application/engine/startup/runner/PluginRunner.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/EntryPoint.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/ListenerFilter.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Method.java(1 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/annotations/EntryPoint.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/annotations/EntryPointMethod.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/annotations/ListenerFilter.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/config/PluginRegistrationConfiguration.java(1 hunks)nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/service/PluginService.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: task-list-completed
- GitHub Check: task-list-completed
- GitHub Check: Build
- GitHub Check: task-list-completed
🔇 Additional comments (9)
application-engine/src/main/groovy/com/netgrif/application/engine/petrinet/domain/dataset/logic/action/ActionDelegate.groovy (1)
55-55: ImportPluginHolderfor plugin DSL support
The newPluginfield relies onPluginHolder. The import is correct and necessary for the dynamic plugin injection.nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/plugin/annotations/EntryPoint.java (1)
11-28: Well-designed annotation following Spring patterns.The annotation is properly structured with correct use of
@AliasForfor value/name aliasing, appropriate retention policy, and clear documentation. The integration with Spring's@Serviceannotation is a good design choice.nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/EntryPoint.java (1)
15-40: Well-designed domain class with proper serialization support.The implementation correctly:
- Implements
Serializablewith explicitserialVersionUID- Uses Lombok annotations for boilerplate reduction
- Initializes the
methodsmap in the default constructor to prevent NPE- Provides a builder pattern constructor for flexible object creation
nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Method.java (1)
16-38: Consistent and well-implemented domain class.The implementation follows the same solid patterns as
EntryPoint:
- Proper serialization support with explicit
serialVersionUID- Defensive programming with list initialization in default constructor
- Clean builder pattern usage with Lombok annotations
nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/Plugin.java (1)
11-57: Comprehensive plugin domain model with excellent design patterns.The implementation demonstrates:
- Complete plugin metadata model with network service ports (REST/gRPC)
- Proper serialization support and defensive initialization
- Flexible activation control via the
activeboolean flag- Consistent with other domain classes in the plugin package
The design supports both standalone plugins and those exposing network services, providing a solid foundation for the plugin architecture.
application-engine/src/main/groovy/com/netgrif/application/engine/plugin/PluginInjector.groovy (1)
22-40: Well-structured component with clear separation of concerns.The design appropriately separates the public interface (
inject) from the implementation details (updateMetaClasses), and the delegation toPluginServicemaintains good architectural boundaries.application-engine/src/main/java/com/netgrif/application/engine/startup/runner/PluginRunner.java (1)
22-26: Verify the default plugin enabling behavior aligns with security requirements.The
matchIfMissing = truemeans plugins are enabled by default when the property is not set. This could be a security concern in production environments where plugin functionality should be explicitly enabled.Consider whether plugins should be disabled by default for security:
@ConditionalOnProperty( value = "nae.plugin.enabled", havingValue = "true", - matchIfMissing = true + matchIfMissing = false )Please verify this aligns with your security and deployment requirements.
application-engine/src/main/java/com/netgrif/application/engine/PluginServiceImpl.java (2)
31-38: Method resolution strategy looks goodThe two-phase approach (exact match followed by compatible match) is a good design pattern for flexible method resolution.
25-26: Add null safety checksThe code could throw
NullPointerExceptionin two scenarios:
- If any element in
argsis null,Object::getClasswill fail- If the bean is not found in the application context
Add null safety checks:
-Class<?>[] paramTypesFromRequest = Arrays.stream(args).map(Object::getClass).toArray(Class[]::new); -Object bean = applicationContext.getBean(entryPoint); +Class<?>[] paramTypesFromRequest = Arrays.stream(args) + .map(arg -> arg != null ? arg.getClass() : null) + .toArray(Class[]::new); +Object bean = applicationContext.getBean(entryPoint); +if (bean == null) { + throw new IllegalArgumentException("Entry point bean not found: " + entryPoint); +}Likely an incorrect or invalid review comment.
...in/java/com/netgrif/application/engine/adapter/spring/plugin/annotations/ListenerFilter.java
Show resolved
Hide resolved
| * Domain class for entry points. Entry points are beans annotated with {@link com.netgrif.plugin.core.annotations.EntryPoint} | ||
| * annotation. These beans contain methods, that can be run from server where the plugin is registered. | ||
| * */ |
There was a problem hiding this comment.
Fix the incorrect package reference in the javadoc comment.
The comment references com.netgrif.plugin.core.annotations.EntryPoint but according to the AI summary, the actual annotation is located at com.netgrif.application.engine.adapter.spring.plugin.annotations.EntryPoint.
- * Domain class for entry points. Entry points are beans annotated with {@link com.netgrif.plugin.core.annotations.EntryPoint}
+ * Domain class for entry points. Entry points are beans annotated with {@link com.netgrif.application.engine.adapter.spring.plugin.annotations.EntryPoint}📝 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.
| * Domain class for entry points. Entry points are beans annotated with {@link com.netgrif.plugin.core.annotations.EntryPoint} | |
| * annotation. These beans contain methods, that can be run from server where the plugin is registered. | |
| * */ | |
| * Domain class for entry points. Entry points are beans annotated with {@link com.netgrif.application.engine.adapter.spring.plugin.annotations.EntryPoint} | |
| * annotation. These beans contain methods, that can be run from server where the plugin is registered. | |
| * */ |
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/plugin/domain/EntryPoint.java
around lines 12 to 14, update the Javadoc comment to correct the package
reference for the EntryPoint annotation. Replace the incorrect package
`com.netgrif.plugin.core.annotations.EntryPoint` with the correct one
`com.netgrif.application.engine.adapter.spring.plugin.annotations.EntryPoint` to
accurately reflect the annotation's location.
| protected void updateMetaClasses(Plugin plugin) { | ||
| def pluginMeta = new PluginMeta() | ||
|
|
||
| plugin.entryPoints.values().each { EntryPoint ep -> | ||
| def epMeta = new EntryPointMeta() | ||
|
|
||
| ep.methods.values().each { Method method -> | ||
| /** | ||
| * Dynamically generated method closure for entry point invocation. | ||
| * | ||
| * @param args variable-length list of Serializable arguments | ||
| * @return the result returned by PluginService.call(...) | ||
| */ | ||
| epMeta.metaClass."${method.name}" = { Serializable... args -> | ||
| pluginService.call(plugin.identifier, ep.name, method.name, args) | ||
| } | ||
| } | ||
|
|
||
| pluginMeta.metaClass."${ep.name}" = epMeta | ||
| } | ||
|
|
||
| PluginHolder.metaClass."${plugin.name}" = pluginMeta | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation and error handling to the dynamic injection mechanism.
While the metaClass approach is appropriate for Groovy, consider these improvements:
- Validation: Verify plugin and entry point data before injection
- Error handling: The dynamic closures lack error handling for method invocation failures
- Security: Using
Serializable... argsmay be overly permissive
protected void updateMetaClasses(Plugin plugin) {
+ if (plugin?.name == null || plugin.entryPoints == null) {
+ log.warn("Skipping injection for invalid plugin: ${plugin}")
+ return
+ }
+
def pluginMeta = new PluginMeta()
plugin.entryPoints.values().each { EntryPoint ep ->
+ if (ep?.name == null || ep.methods == null) {
+ log.warn("Skipping invalid entry point: ${ep}")
+ return
+ }
def epMeta = new EntryPointMeta()
ep.methods.values().each { Method method ->
+ if (method?.name == null) {
+ log.warn("Skipping invalid method: ${method}")
+ return
+ }
epMeta.metaClass."${method.name}" = { Serializable... args ->
+ try {
pluginService.call(plugin.identifier, ep.name, method.name, args)
+ } catch (Exception e) {
+ log.error("Error calling plugin method ${plugin.identifier}.${ep.name}.${method.name}", e)
+ throw 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.
| protected void updateMetaClasses(Plugin plugin) { | |
| def pluginMeta = new PluginMeta() | |
| plugin.entryPoints.values().each { EntryPoint ep -> | |
| def epMeta = new EntryPointMeta() | |
| ep.methods.values().each { Method method -> | |
| /** | |
| * Dynamically generated method closure for entry point invocation. | |
| * | |
| * @param args variable-length list of Serializable arguments | |
| * @return the result returned by PluginService.call(...) | |
| */ | |
| epMeta.metaClass."${method.name}" = { Serializable... args -> | |
| pluginService.call(plugin.identifier, ep.name, method.name, args) | |
| } | |
| } | |
| pluginMeta.metaClass."${ep.name}" = epMeta | |
| } | |
| PluginHolder.metaClass."${plugin.name}" = pluginMeta | |
| } | |
| protected void updateMetaClasses(Plugin plugin) { | |
| // Validation: skip entirely if the plugin is malformed | |
| if (plugin?.name == null || plugin.entryPoints == null) { | |
| log.warn("Skipping injection for invalid plugin: ${plugin}") | |
| return | |
| } | |
| def pluginMeta = new PluginMeta() | |
| plugin.entryPoints.values().each { EntryPoint ep -> | |
| // Validation: skip individual entry point if malformed | |
| if (ep?.name == null || ep.methods == null) { | |
| log.warn("Skipping invalid entry point: ${ep}") | |
| return | |
| } | |
| def epMeta = new EntryPointMeta() | |
| ep.methods.values().each { Method method -> | |
| // Validation: skip individual method if malformed | |
| if (method?.name == null) { | |
| log.warn("Skipping invalid method: ${method}") | |
| return | |
| } | |
| epMeta.metaClass."${method.name}" = { Serializable... args -> | |
| // Error handling around the dynamic call | |
| try { | |
| pluginService.call(plugin.identifier, ep.name, method.name, args) | |
| } catch (Exception e) { | |
| log.error("Error calling plugin method ${plugin.identifier}.${ep.name}.${method.name}", e) | |
| throw e | |
| } | |
| } | |
| } | |
| pluginMeta.metaClass."${ep.name}" = epMeta | |
| } | |
| PluginHolder.metaClass."${plugin.name}" = pluginMeta | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/groovy/com/netgrif/application/engine/plugin/PluginInjector.groovy
between lines 50 and 72, add validation to check that the plugin and its entry
points are not null or empty before injecting metaClasses. Wrap the dynamic
method closures in try-catch blocks to handle and log any exceptions during
pluginService.call invocations. Also, restrict or validate the types of
arguments accepted instead of allowing any Serializable to improve security and
prevent misuse.
| public void run(ApplicationArguments args) throws Exception { | ||
| log.info("Registering plugins."); | ||
| Map<String, Plugin> pluginMap = new HashMap<>(); | ||
|
|
||
| applicationContext.getBeansOfType(PluginRegistrationConfiguration.class).forEach((key, config) -> { | ||
| if (!pluginMap.containsKey(config.getPluginName())) { | ||
| Plugin plugin = Plugin.builder() | ||
| .identifier(config.getPluginName()) | ||
| .name(config.getPluginName()) | ||
| .version(config.getVersion()) | ||
| .description(StringUtils.EMPTY) | ||
| .entryPoints(config.getEntryPoints()) | ||
| .build(); | ||
| pluginMap.put(config.getPluginName(), plugin); | ||
| } | ||
| }); | ||
| pluginMap.values().forEach(pluginInjector::inject); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and improve plugin metadata handling.
The current implementation has several areas for improvement:
- Error handling: No exception handling during plugin collection or injection
- Plugin metadata: Using plugin name as identifier and setting empty description
- Validation: No validation of plugin configurations
@Override
public void run(ApplicationArguments args) throws Exception {
log.info("Registering plugins.");
Map<String, Plugin> pluginMap = new HashMap<>();
applicationContext.getBeansOfType(PluginRegistrationConfiguration.class).forEach((key, config) -> {
- if (!pluginMap.containsKey(config.getPluginName())) {
- Plugin plugin = Plugin.builder()
- .identifier(config.getPluginName())
- .name(config.getPluginName())
- .version(config.getVersion())
- .description(StringUtils.EMPTY)
- .entryPoints(config.getEntryPoints())
- .build();
- pluginMap.put(config.getPluginName(), plugin);
- }
+ try {
+ if (StringUtils.isBlank(config.getPluginName())) {
+ log.warn("Skipping plugin registration with blank name: {}", key);
+ return;
+ }
+
+ if (!pluginMap.containsKey(config.getPluginName())) {
+ Plugin plugin = Plugin.builder()
+ .identifier(config.getPluginName()) // Consider using a separate identifier
+ .name(config.getPluginName())
+ .version(config.getVersion())
+ .description(config.getDescription() != null ? config.getDescription() : StringUtils.EMPTY)
+ .entryPoints(config.getEntryPoints())
+ .active(true) // Set explicit active state
+ .build();
+ pluginMap.put(config.getPluginName(), plugin);
+ log.debug("Registered plugin: {}", config.getPluginName());
+ }
+ } catch (Exception e) {
+ log.error("Error registering plugin configuration: {}", key, e);
+ }
});
- pluginMap.values().forEach(pluginInjector::inject);
+
+ pluginMap.values().forEach(plugin -> {
+ try {
+ pluginInjector.inject(plugin);
+ log.debug("Injected plugin: {}", plugin.getName());
+ } catch (Exception e) {
+ log.error("Error injecting plugin: {}", plugin.getName(), e);
+ }
+ });
+
+ log.info("Plugin registration completed. Registered {} plugins.", pluginMap.size());
}📝 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 void run(ApplicationArguments args) throws Exception { | |
| log.info("Registering plugins."); | |
| Map<String, Plugin> pluginMap = new HashMap<>(); | |
| applicationContext.getBeansOfType(PluginRegistrationConfiguration.class).forEach((key, config) -> { | |
| if (!pluginMap.containsKey(config.getPluginName())) { | |
| Plugin plugin = Plugin.builder() | |
| .identifier(config.getPluginName()) | |
| .name(config.getPluginName()) | |
| .version(config.getVersion()) | |
| .description(StringUtils.EMPTY) | |
| .entryPoints(config.getEntryPoints()) | |
| .build(); | |
| pluginMap.put(config.getPluginName(), plugin); | |
| } | |
| }); | |
| pluginMap.values().forEach(pluginInjector::inject); | |
| } | |
| @Override | |
| public void run(ApplicationArguments args) throws Exception { | |
| log.info("Registering plugins."); | |
| Map<String, Plugin> pluginMap = new HashMap<>(); | |
| applicationContext.getBeansOfType(PluginRegistrationConfiguration.class).forEach((key, config) -> { | |
| try { | |
| if (StringUtils.isBlank(config.getPluginName())) { | |
| log.warn("Skipping plugin registration with blank name: {}", key); | |
| return; | |
| } | |
| if (!pluginMap.containsKey(config.getPluginName())) { | |
| Plugin plugin = Plugin.builder() | |
| .identifier(config.getPluginName()) // Consider using a separate identifier | |
| .name(config.getPluginName()) | |
| .version(config.getVersion()) | |
| .description(config.getDescription() != null ? config.getDescription() : StringUtils.EMPTY) | |
| .entryPoints(config.getEntryPoints()) | |
| .active(true) // Set explicit active state | |
| .build(); | |
| pluginMap.put(config.getPluginName(), plugin); | |
| log.debug("Registered plugin: {}", config.getPluginName()); | |
| } | |
| } catch (Exception e) { | |
| log.error("Error registering plugin configuration: {}", key, e); | |
| } | |
| }); | |
| pluginMap.values().forEach(plugin -> { | |
| try { | |
| pluginInjector.inject(plugin); | |
| log.debug("Injected plugin: {}", plugin.getName()); | |
| } catch (Exception e) { | |
| log.error("Error injecting plugin: {}", plugin.getName(), e); | |
| } | |
| }); | |
| log.info("Plugin registration completed. Registered {} plugins.", pluginMap.size()); | |
| } |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/startup/runner/PluginRunner.java
between lines 39 and 56, add try-catch blocks around the plugin collection and
injection logic to handle and log exceptions gracefully. Replace using the
plugin name as the identifier with a unique identifier if available, and
populate the description field with meaningful metadata from the plugin
configuration. Also, add validation checks for plugin configurations before
creating Plugin objects to ensure required fields are present and valid,
skipping or logging invalid entries.
| throw new IllegalArgumentException(String.format("Method %s is ambiguous for the param types %s", | ||
| methodToExecute, Arrays.toString(requestParamTypes))); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve ambiguous method error message
When multiple methods match, include information about which methods were found to help with debugging.
-throw new IllegalArgumentException(String.format("Method %s is ambiguous for the param types %s",
- methodToExecute, Arrays.toString(requestParamTypes)));
+throw new IllegalArgumentException(String.format(
+ "Method %s is ambiguous for param types %s. Found: %s and %s",
+ methodToExecute, Arrays.toString(requestParamTypes),
+ methodToInvoke.toString(), method.toString()));Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/PluginServiceImpl.java
around lines 69 to 70, the error message for ambiguous methods only states the
method and parameter types but lacks details on which methods were found. Modify
the exception message to include a list or description of all matching methods
that caused the ambiguity, so debugging is easier by knowing exactly which
methods conflicted.
| Class<?>[] paramTypesFromRequest = Arrays.stream(args).map(Object::getClass).toArray(Class[]::new); | ||
| Object bean = applicationContext.getBean(entryPoint); | ||
| Method methodToInvoke = findMethod(bean, method, paramTypesFromRequest); | ||
| return methodToInvoke.invoke(bean, Arrays.stream(args).toArray()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant array conversion
The args parameter is already a Serializable[] array. Converting it to an array again is unnecessary.
-return methodToInvoke.invoke(bean, Arrays.stream(args).toArray());
+return methodToInvoke.invoke(bean, args);📝 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.
| return methodToInvoke.invoke(bean, Arrays.stream(args).toArray()); | |
| - return methodToInvoke.invoke(bean, Arrays.stream(args).toArray()); | |
| + return methodToInvoke.invoke(bean, args); |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/PluginServiceImpl.java
at line 28, remove the redundant conversion of the args parameter to an array.
Since args is already a Serializable[] array, pass it directly to
methodToInvoke.invoke without wrapping it in Arrays.stream(args).toArray().
| public Object call(String pluginId, String entryPoint, String method, Serializable... args) throws InvocationTargetException, IllegalAccessException, NoSuchMethodException { | ||
| log.info("Executing entry point [{}] with method [{}]...", entryPoint, method); |
There was a problem hiding this comment.
Use or remove the unused pluginId parameter
The pluginId parameter is never used in the method implementation. Either use it (e.g., in logging or validation) or remove it from the method signature.
Consider including it in the log message:
-log.info("Executing entry point [{}] with method [{}]...", entryPoint, method);
+log.info("Executing plugin [{}] entry point [{}] with method [{}]...", pluginId, entryPoint, method);📝 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 Object call(String pluginId, String entryPoint, String method, Serializable... args) throws InvocationTargetException, IllegalAccessException, NoSuchMethodException { | |
| log.info("Executing entry point [{}] with method [{}]...", entryPoint, method); | |
| public Object call(String pluginId, String entryPoint, String method, Serializable... args) throws InvocationTargetException, IllegalAccessException, NoSuchMethodException { | |
| log.info("Executing plugin [{}] entry point [{}] with method [{}]...", pluginId, entryPoint, method); |
🤖 Prompt for AI Agents
In
application-engine/src/main/java/com/netgrif/application/engine/PluginServiceImpl.java
around lines 23 to 24, the method call has an unused parameter pluginId. To fix
this, either remove the pluginId parameter from the method signature if it is
unnecessary, or incorporate it into the method logic, such as including it in
the log message to provide more context about which plugin is being executed.
- Introduced @target and @retention annotations for ListenerFilter definition. - Ensured compatibility with method-level usage and set retention policy at runtime.
Description
Implements NAE-2129
Dependencies
Third party dependencies
No new dependencies were introduced
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>
Checklist:
Summary by CodeRabbit
New Features
Documentation