Remap DAW plugin resolver to components#499
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughPlugin lookup associations now target ChangesComponent lookup and navigation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectInfoController
participant PluginsController
participant PluginTableController
participant PluginTreeViewController
ProjectInfoController->>PluginsController: Select component by ID
PluginsController->>PluginTableController: Delegate for table tab
PluginsController->>PluginTreeViewController: Delegate for tree tab
PluginTableController-->>PluginsController: Select component or parent plugin row
PluginTreeViewController-->>PluginsController: Select component or parent plugin node
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
owlplug-client/src/main/java/com/owlplug/project/services/PluginLookupService.java (1)
57-71: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrevent potential
NullPointerExceptionand optimize the query.If
projectPlugin.getName()isnull,PluginComponentRepository.nameContainswill throw aNullPointerExceptionwhen calling.toLowerCase(). Additionally, if a generic search matches many components,findAllwithout pagination will load all matches into memory just to retrieve the first one.Consider adding a guard for missing names and using
PageRequest.of(0, 1)to limit the query result to a single element.♻️ Proposed refactor
- Specification<PluginComponent> baseSpec = PluginComponentRepository.nameContains(projectPlugin.getName()) - .and(PluginComponentRepository.hasFormat(projectPlugin.getFormat())); - - // Prefer an active (successfully scanned, platform-compatible) component. - // Fall back to inactive/ghost components only if no active match exists. - List<PluginComponent> components = pluginComponentRepository.findAll( - baseSpec.and(PluginComponentRepository.isActive(true))); - - if (components.isEmpty()) { - components = pluginComponentRepository.findAll(baseSpec); - } - - if (!components.isEmpty()) { - lookup.setComponent(components.get(0)); - lookup.setResult(LookupResult.FOUND); - } + String pluginName = projectPlugin.getName(); + if (pluginName == null || pluginName.isBlank()) { + return lookup; + } + + Specification<PluginComponent> baseSpec = PluginComponentRepository.nameContains(pluginName) + .and(PluginComponentRepository.hasFormat(projectPlugin.getFormat())); + + // Prefer an active (successfully scanned, platform-compatible) component. + // Fall back to inactive/ghost components only if no active match exists. + org.springframework.data.domain.Pageable limit = org.springframework.data.domain.PageRequest.of(0, 1); + List<PluginComponent> components = pluginComponentRepository.findAll( + baseSpec.and(PluginComponentRepository.isActive(true)), limit).getContent(); + + if (components.isEmpty()) { + components = pluginComponentRepository.findAll(baseSpec, limit).getContent(); + } + + if (!components.isEmpty()) { + lookup.setComponent(components.get(0)); + lookup.setResult(LookupResult.FOUND); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@owlplug-client/src/main/java/com/owlplug/project/services/PluginLookupService.java` around lines 57 - 71, Update the lookup flow around projectPlugin.getName() and PluginComponentRepository.nameContains to handle a null or missing plugin name without invoking the specification or throwing; return the existing not-found result for that case. For valid names, replace the unbounded findAll calls in the active and fallback searches with a PageRequest.of(0, 1)-limited query while preserving the preference for active components and selecting the first match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@owlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTreeViewController.java`:
- Around line 338-363: Update selectComponentById and selectPluginById to expand
each matched item's collapsed ancestors before selecting it, then select the
TreeItem via the selection model's select(item) overload rather than converting
it to a row index. Preserve the existing component lookup and fallback behavior
while ensuring hidden items become visible and selection scrolling works
correctly.
---
Nitpick comments:
In
`@owlplug-client/src/main/java/com/owlplug/project/services/PluginLookupService.java`:
- Around line 57-71: Update the lookup flow around projectPlugin.getName() and
PluginComponentRepository.nameContains to handle a null or missing plugin name
without invoking the specification or throwing; return the existing not-found
result for that case. For valid names, replace the unbounded findAll calls in
the active and fallback searches with a PageRequest.of(0, 1)-limited query while
preserving the preference for active components and selecting the first match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6a0f1ac1-ba0c-454e-bc44-7de3c9f465bb
⛔ Files ignored due to path filters (1)
owlplug-client/src/main/resources/icons/error-red-16.pngis excluded by!**/*.png,!**/*.png
📒 Files selected for processing (14)
owlplug-client/src/main/java/com/owlplug/core/components/ApplicationDefaults.javaowlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTableController.javaowlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginTreeViewController.javaowlplug-client/src/main/java/com/owlplug/plugin/controllers/PluginsController.javaowlplug-client/src/main/java/com/owlplug/plugin/model/Plugin.javaowlplug-client/src/main/java/com/owlplug/plugin/model/PluginComponent.javaowlplug-client/src/main/java/com/owlplug/plugin/repositories/PluginComponentRepository.javaowlplug-client/src/main/java/com/owlplug/plugin/repositories/PluginRepository.javaowlplug-client/src/main/java/com/owlplug/plugin/services/PluginService.javaowlplug-client/src/main/java/com/owlplug/project/controllers/ProjectInfoController.javaowlplug-client/src/main/java/com/owlplug/project/model/DawPluginLookup.javaowlplug-client/src/main/java/com/owlplug/project/services/PluginLookupService.javaowlplug-client/src/main/java/com/owlplug/project/ui/ProjectTreeCell.javaowlplug-client/src/main/resources/owlplug.css
💤 Files with no reviewable changes (4)
- owlplug-client/src/main/java/com/owlplug/core/components/ApplicationDefaults.java
- owlplug-client/src/main/java/com/owlplug/plugin/repositories/PluginRepository.java
- owlplug-client/src/main/java/com/owlplug/plugin/services/PluginService.java
- owlplug-client/src/main/java/com/owlplug/plugin/model/Plugin.java
8e4ddd2 to
2b3fbbb
Compare
No description provided.