Optimize package lookup database queries#497
Merged
Merged
Conversation
WalkthroughRemote package exploration now uses paginated repository queries, ordered hydration, asynchronous stale-result handling, and incremental grid/list loading. Image caching is split into memory and disk tiers with virtualized image rendering. OAS synchronization and plugin scan failure handling were also tightened. ChangesRemote Package Paging
Image Caching
Task Validation and Failure Handling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ExploreController
participant ExploreService
participant RemotePackageRepository
participant ExploreUI
ExploreController->>ExploreService: request paged remote packages
ExploreService->>RemotePackageRepository: query matching package IDs
ExploreService->>RemotePackageRepository: hydrate bundles and targets
ExploreService-->>ExploreController: return ordered package page
ExploreController->>ExploreUI: render or append package results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java (1)
336-351: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlways reset state for an accepted first-page search result.
This comparison misses changed searches whose total and first page match but later pages differ, retaining stale loaded pages. It also returns false for the initial empty result, leaving
loadedPackagePartitionsuninitialized.Remove this guard or include initialized state and the displayed criteria/search generation in the comparison.
Also applies to: 448-462
🤖 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/explore/controllers/ExploreController.java` around lines 336 - 351, Update displayPackages and its corresponding logic around the additional referenced range so every accepted first-page search result resets pagination state, including an initial empty result. Do not rely only on total count and first-page contents; include initialized state plus the active search criteria or generation when deciding whether a refresh is needed, or remove the guard while preserving any necessary non-search refresh behavior.
🧹 Nitpick comments (1)
owlplug-client/src/main/java/com/owlplug/explore/services/ExploreService.java (1)
120-135: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftUse an actual ID projection for the first query.
findAll(filterSpec, pageable)hydrates completeRemotePackageentities—and their eager tags—before extracting IDs, then hydrates those packages again. A repository ID-page query would better achieve this PR’s lookup optimization.🤖 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/explore/services/ExploreService.java` around lines 120 - 135, Update getRemotePackages to obtain the paginated IDs through a repository projection query rather than remotePackageRepository.findAll(filterSpec, pageable), avoiding full RemotePackage hydration and eager tags in the first query. Add or use a repository method that applies the filter specification and pageable while selecting only IDs, then preserve the existing second-stage entity hydration using those IDs.
🤖 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/explore/controllers/ExploreController.java`:
- Around line 366-371: Update the subsequent-page completion flow in
ExploreController around the searchSequence callback to refresh the total result
count from the newly fetched page before deciding whether more pages should
load. Ensure pagination uses the latest count after inserts or deletions, while
preserving the existing loadedRemotePackages, nextDbPage, and onLoaded updates.
In
`@owlplug-client/src/main/java/com/owlplug/explore/services/ExploreService.java`:
- Around line 129-135: Update getRemotePackages to create or augment the
Pageable with a deterministic sort before calling
remotePackageRepository.findAll: preserve the caller’s requested primary sort
and append RemotePackage ID as the stable tie-breaker, then use that pageable
for pagination.
In `@owlplug-client/src/main/java/com/owlplug/explore/tasks/SourceSyncTask.java`:
- Around line 202-203: Update the skip-reason expression in SourceSyncTask’s
package skip log to include both predicates: report “no bundles” when bundles
are absent and “unresolved plugin type” when the type is unresolved, combining
both reasons when both conditions apply. Preserve the existing package, version,
and debug logging context.
---
Outside diff comments:
In
`@owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java`:
- Around line 336-351: Update displayPackages and its corresponding logic around
the additional referenced range so every accepted first-page search result
resets pagination state, including an initial empty result. Do not rely only on
total count and first-page contents; include initialized state plus the active
search criteria or generation when deciding whether a refresh is needed, or
remove the guard while preserving any necessary non-search refresh behavior.
---
Nitpick comments:
In
`@owlplug-client/src/main/java/com/owlplug/explore/services/ExploreService.java`:
- Around line 120-135: Update getRemotePackages to obtain the paginated IDs
through a repository projection query rather than
remotePackageRepository.findAll(filterSpec, pageable), avoiding full
RemotePackage hydration and eager tags in the first query. Add or use a
repository method that applies the filter specification and pageable while
selecting only IDs, then preserve the existing second-stage entity hydration
using those IDs.
🪄 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: 14f3786c-a87b-41be-8229-6dd0dde7642f
📒 Files selected for processing (7)
owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.javaowlplug-client/src/main/java/com/owlplug/explore/model/PackageBundle.javaowlplug-client/src/main/java/com/owlplug/explore/model/RemotePackage.javaowlplug-client/src/main/java/com/owlplug/explore/repositories/RemotePackageRepository.javaowlplug-client/src/main/java/com/owlplug/explore/services/ExploreService.javaowlplug-client/src/main/java/com/owlplug/explore/tasks/SourceSyncTask.javaowlplug-client/src/main/java/com/owlplug/plugin/tasks/PluginScanTask.java
DropSnorz
force-pushed
the
chore/package-query-opt
branch
from
July 15, 2026 18:49
349581a to
f14876e
Compare
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java (1)
460-471: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude the search criteria in refresh detection.
Different filters can produce the same total and identical first 100 IDs while differing on later pages. This method then retains the old tail, and subsequent pagination appends new results to that stale list.
Track the criteria applied to
loadedRemotePackagesand force a refresh when they change, or always replace the results for a completed first-page search.🤖 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/explore/controllers/ExploreController.java` around lines 460 - 471, Update shouldRefreshPackages to include the active search criteria in refresh detection, comparing them with the criteria used to populate loadedRemotePackages and returning true when they differ. Ensure the criteria are stored or updated whenever the loaded results are replaced, so identical totals and first-page IDs cannot preserve a stale tail.
🤖 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/core/components/ImageCache.java`:
- Around line 140-155: Update lookupDiskCache to check whether ImageIO.read
returns a null BufferedImage before calling SwingFXUtils.toFXImage; remove the
invalid URL entry from the disk cache and return null so the caller can fetch a
fresh image.
- Around line 169-182: Update fetchFromUrl so failed Image loads are evicted
from the memory cache for both synchronous and asynchronous fetches. Add an
error listener that removes the URL only when getMemoryCache().get(url) still
references fetchedImage, preserving newer replacements; keep successful
disk-cache persistence unchanged.
In
`@owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java`:
- Around line 165-169: Update the search and pagination flows using
searchSequence to capture the controller’s current search generation before
queuing each FX.run callback, then recheck that generation inside both callbacks
before mutating UI or pagination state. Ensure stale first-page and
load-next-page results return without applying changes, while current results
retain existing behavior.
- Around line 342-350: Update the new-search flow in ExploreController around
searchSequence and fetchNextDbPage so fetchingNextDbPage remains disabled while
the page-zero request is pending, then is re-enabled on both success and
failure. Reset nextDbPage when starting the search, before the asynchronous
request begins, while preserving the existing displayPackages behavior after
page zero succeeds.
---
Outside diff comments:
In
`@owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java`:
- Around line 460-471: Update shouldRefreshPackages to include the active search
criteria in refresh detection, comparing them with the criteria used to populate
loadedRemotePackages and returning true when they differ. Ensure the criteria
are stored or updated whenever the loaded results are replaced, so identical
totals and first-page IDs cannot preserve a stale tail.
🪄 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: fc686892-5479-4f20-9f3d-a2b5eafdf687
📒 Files selected for processing (13)
owlplug-client/src/main/java/com/owlplug/OwlPlug.javaowlplug-client/src/main/java/com/owlplug/core/components/ImageCache.javaowlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.javaowlplug-client/src/main/java/com/owlplug/explore/model/PackageBundle.javaowlplug-client/src/main/java/com/owlplug/explore/model/RemotePackage.javaowlplug-client/src/main/java/com/owlplug/explore/repositories/RemotePackageRepository.javaowlplug-client/src/main/java/com/owlplug/explore/services/ExploreService.javaowlplug-client/src/main/java/com/owlplug/explore/tasks/SourceSyncTask.javaowlplug-client/src/main/java/com/owlplug/explore/ui/PackageListRowCellFactory.javaowlplug-client/src/main/java/com/owlplug/plugin/tasks/PluginScanTask.javaowlplug-client/src/main/java/com/owlplug/plugin/ui/PluginListCellFactory.javaowlplug-client/src/main/resources/fxml/explore/ExploreView.fxmlowlplug-client/src/main/resources/owlplug.css
🚧 Files skipped from review as they are similar to previous changes (6)
- owlplug-client/src/main/java/com/owlplug/plugin/tasks/PluginScanTask.java
- owlplug-client/src/main/java/com/owlplug/explore/model/RemotePackage.java
- owlplug-client/src/main/java/com/owlplug/explore/model/PackageBundle.java
- owlplug-client/src/main/java/com/owlplug/explore/services/ExploreService.java
- owlplug-client/src/main/java/com/owlplug/explore/tasks/SourceSyncTask.java
- owlplug-client/src/main/java/com/owlplug/explore/repositories/RemotePackageRepository.java
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/core/ui/VirtualizedImageView.java`:
- Around line 88-99: Update the async completion chain in VirtualizedImageView
so imageCache.getAsync(url) uses whenComplete instead of thenAccept, ensuring
setLoading(false) runs for both successful and exceptional fetches. Preserve the
currentUrl stale-result guard and only update imageView when a valid, non-error
image is returned.
🪄 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: 967bee83-5923-4039-bf0b-72366c629f80
📒 Files selected for processing (7)
owlplug-client/src/main/java/com/owlplug/OwlPlug.javaowlplug-client/src/main/java/com/owlplug/core/components/ImageCache.javaowlplug-client/src/main/java/com/owlplug/core/ui/VirtualizedImageView.javaowlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.javaowlplug-client/src/main/java/com/owlplug/explore/ui/PackageListRowCellFactory.javaowlplug-client/src/main/java/com/owlplug/explore/ui/PackageListRowView.javaowlplug-client/src/main/resources/owlplug.css
🚧 Files skipped from review as they are similar to previous changes (4)
- owlplug-client/src/main/java/com/owlplug/explore/ui/PackageListRowCellFactory.java
- owlplug-client/src/main/java/com/owlplug/OwlPlug.java
- owlplug-client/src/main/java/com/owlplug/explore/controllers/ExploreController.java
- owlplug-client/src/main/java/com/owlplug/core/components/ImageCache.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relates to #495