Skip to content

Conversation

tuplle
Copy link
Member

@tuplle tuplle commented Sep 9, 2025

Description

Move the method for resolving a class of Spring bean as a utility method for broader usage. Use this method in a module service bean injection.

Fixes NAE-2204

Dependencies

No new dependencies were introduced.

How Has Been This Tested?

Tested by manually launching the engine.

Test Configuration

Name Tested on
OS Linux
Runtime OpenJDK 21
Dependency Manager Maven 3
Framework version
Run parameters
Other configuration

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes have been checked, personally or remotely, with @machacjozef
  • I have commented my code, particularly in hard-to-understand areas
  • I have resolved all conflicts with the target branch of the PR
  • I have updated and synced my code with the target branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes:
    • Lint test
    • Unit tests
    • Integration tests
  • I have checked my contribution with code analysis tools:
  • I have made corresponding changes to the documentation:
    • Developer documentation
    • User Guides
    • Migration Guides

Summary by CodeRabbit

  • Refactor

    • Improved class-resolution and proxy handling across the system, increasing reliability of module service detection and startup ordering.
  • Chores

    • Added a shared adapter to centralize reflection utilities, reducing duplication and aligning behavior across modules.

…xyed by Spring

- Moved `resolveClass` and `indexOfClass` methods to a new `ReflectionUtils` utility class for reuse and better code organization.
- Replaced old implementations with calls to `ReflectionUtils` in affected classes.
- Removed redundant code from `ApplicationRunnerOrderResolver` to streamline functionality.
@tuplle tuplle self-assigned this Sep 9, 2025
@tuplle tuplle added bugfix A change that fixes a bug improvement A change that improves on an existing feature labels Sep 9, 2025
Copy link

coderabbitai bot commented Sep 9, 2025

Walkthrough

Refactors class-resolution logic to a new shared utility NaeReflectionUtils, updates ApplicationRunner sorting and module service grouping to use it, and adds the new adapter module as a Maven dependency. Removes duplicate resolveClass and indexOfClass implementations from ApplicationRunnerOrderResolver.

Changes

Cohort / File(s) Summary
Shared reflection utility (new)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/NaeReflectionUtils.java
Adds final utility class with public static methods: resolveClass(T) (unwraps Spring proxies via AopProxyUtils) and indexOfClass(List, Class) (finds first list element matching a class).
Startup runner ordering refactor
application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerOrderResolver.java, application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerExecutor.java
Replaces local resolveClass/indexOfClass implementations and imports with NaeReflectionUtils equivalents; removes the removed methods from ApplicationRunnerOrderResolver; Executor now statically imports NaeReflectionUtils.resolveClass.
Module service annotation resolution
application-engine/src/main/groovy/com/netgrif/application/engine/integration/modules/ModuleServiceInjector.groovy
Uses NaeReflectionUtils.resolveClass to obtain the service class before reading @ModuleService annotations; adds NaeReflectionUtils and AnnotatedElementUtils imports.
Build/dependency update
application-engine/pom.xml
Adds dependency: com.netgrif:nae-spring-core-adapter:${project.version}.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as Application
  participant Exec as ApplicationRunnerExecutor
  participant Resolver as ApplicationRunnerOrderResolver
  participant Util as NaeReflectionUtils

  App->>Exec: run()
  Exec->>Resolver: sort(runners)
  loop For each runner
    Resolver->>Util: resolveClass(runner)
    Util-->>Resolver: Class
  end
  Resolver->>Util: indexOfClass(runners, targetClass)
  Util-->>Resolver: index
  Resolver-->>Exec: sorted runners
  Exec-->>App: execute in order
Loading
sequenceDiagram
  autonumber
  participant Injector as ModuleServiceInjector
  participant Util as NaeReflectionUtils

  loop For each service bean
    Injector->>Util: resolveClass(bean)
    Util-->>Injector: Class
    Injector->>Injector: read @ModuleService annotations from Class
    Injector-->>Injector: group by module
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately describes the primary issue the PR fixes — inability to resolve annotations on Spring‑proxied Plugin EntryPoint classes — and includes the tracking ID (NAE-2204); this aligns with the changes that centralize class-resolution into NaeReflectionUtils and apply it to module/service injection. It is specific to the main change and concise enough for a reviewer scanning history to understand the purpose.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added the Medium label Sep 9, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e1bf989 and 5097102.

📒 Files selected for processing (4)
  • application-engine/src/main/groovy/com/netgrif/application/engine/integration/modules/ModuleServiceInjector.groovy (2 hunks)
  • application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerExecutor.java (1 hunks)
  • application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerOrderResolver.java (1 hunks)
  • nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/ReflectionUtils.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerExecutor.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/ReflectionUtils.java (1)
  • ReflectionUtils (7-50)
application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerOrderResolver.java (1)
nae-spring-core-adapter/src/main/java/com/netgrif/application/engine/adapter/spring/utils/ReflectionUtils.java (1)
  • ReflectionUtils (7-50)
⏰ 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). (4)
  • GitHub Check: Test
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
🔇 Additional comments (1)
application-engine/src/main/java/com/netgrif/application/engine/startup/ApplicationRunnerOrderResolver.java (1)

17-19: Good centralization of proxy-unwrapping logic.

Static-importing shared utils reduces duplication and keeps behavior consistent across components.

…xyed by Spring

- Renamed `ReflectionUtils` to `NaeReflectionUtils` and adjusted related imports for accuracy and consistency.
- Enhanced `resolveClass` with null checks and proxy target class handling for better reliability.
- Updated impacted classes to utilize the refactored utility methods, ensuring improved readability and maintainability.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e39b992 and d541edd.

📒 Files selected for processing (1)
  • application-engine/pom.xml (1 hunks)
⏰ 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). (4)
  • GitHub Check: Test
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
  • GitHub Check: task-list-completed
🔇 Additional comments (1)
application-engine/pom.xml (1)

138-142: New dependency added — reconcile with PR description and ensure availability

application-engine/pom.xml (lines 138–142) adds com.netgrif:nae-spring-core-adapter at ${project.version}, contradicting the PR note "No new dependencies were introduced." Verification attempt failed (search returned "No files were searched"), so I could not confirm if this is an internal reactor module or an external artifact.

  • If internal: add the module to the top-level or include it in this PR so the reactor builds.
  • If external: ensure com.netgrif:nae-spring-core-adapter at the resolved version (POM uses ${project.version}; confirm that version is published and resolvable from your repositories).
  • Update the PR description to accurately state that a new dependency was added.

@machacjozef machacjozef merged commit cb03c1f into release/7.0.0-rev7 Sep 10, 2025
6 of 7 checks passed
@machacjozef machacjozef deleted the NAE-2204 branch September 10, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix A change that fixes a bug improvement A change that improves on an existing feature Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants