-
Notifications
You must be signed in to change notification settings - Fork 6
[NAE-2302] Check for process plugin dependencies on deploy #398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/7.0.0-rev9
Are you sure you want to change the base?
Conversation
- added parsing of pluginDependencies from action to Importer.java - added pluginDependencies attribute to PetriNet.java
WalkthroughThe changes introduce plugin dependency extraction from PetriNet action definitions during the import process. A regex pattern identifies plugin references within action definitions, collects unique plugin names, and stores them in the PetriNet domain object. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested labels
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
- removed unused imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.java(4 hunks)nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-29T17:19:18.300Z
Learnt from: tuplle
Repo: netgrif/application-engine PR: 331
File: application-engine/src/main/java/com/netgrif/application/engine/elastic/service/ElasticPetriNetService.java:45-46
Timestamp: 2025-07-29T17:19:18.300Z
Learning: In ElasticPetriNetService class, petriNetService is properly initialized using Lazy setter injection rather than constructor injection. This pattern with Lazy Autowired setter methods is commonly used in Spring to resolve circular dependencies and is a valid alternative to constructor injection.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java
📚 Learning: 2025-07-31T23:40:46.499Z
Learnt from: tuplle
Repo: netgrif/application-engine PR: 334
File: application-engine/src/main/java/com/netgrif/application/engine/petrinet/service/PetriNetService.java:204-214
Timestamp: 2025-07-31T23:40:46.499Z
Learning: In the PetriNetService.importPetriNet method, existingNet.getVersion() cannot be null because all existing nets in the system were deployed through processes that ensure every net always has a version assigned.
Applied to files:
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
application-engine/src/main/java/com/netgrif/application/engine/importer/service/Importer.java
Show resolved
Hide resolved
| @Getter | ||
| @Setter | ||
| private Set<String> pluginDependencies; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Plugin dependency field looks good; consider defensive copy in copy constructor
The new pluginDependencies field and its initialization in the default constructor are fine and align with the new Importer behavior.
In the copy constructor, though, this.pluginDependencies = petriNet.getPluginDependencies(); reuses the same mutable Set instance, which can lead to surprising shared mutations between the original and the copy, and it does not guard against a null value.
Consider defensively copying and null‑guarding here:
- this.pluginDependencies = petriNet.getPluginDependencies();
+ Set<String> sourcePlugins = petriNet.getPluginDependencies();
+ this.pluginDependencies = sourcePlugins != null
+ ? new HashSet<>(sourcePlugins)
+ : new HashSet<>();This keeps the field non‑null and avoids accidental cross‑net coupling via a shared set.
Also applies to: 164-165, 200-200
🤖 Prompt for AI Agents
In
nae-object-library/src/main/java/com/netgrif/application/engine/objects/petrinet/domain/PetriNet.java
around lines 137-139 (and also update similar assignments at 164-165 and 200),
the copy constructor assigns the pluginDependencies reference directly from the
source object, risking shared mutable state and possible NPE; change these
assignments to create a defensive copy and null-guard the source (e.g., if
source.getPluginDependencies() is null, assign a new empty Set, otherwise assign
a new Set constructed from the source) so the copy has its own non-null
collection instance.
| return tags; | ||
| } | ||
|
|
||
| @Transactional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for @Transactional. This annotations have been removed in another task on the pull request.
There is no database interaction in the method
Description
Implements NAE-2302
Test Configuration
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.