Skip to content

deviceFilters: stop computing six Pinot queries to answer one - #1679

Open
ivan-flamingo wants to merge 2 commits into
mainfrom
hotfix/permanent-la
Open

deviceFilters: stop computing six Pinot queries to answer one#1679
ivan-flamingo wants to merge 2 commits into
mainfrom
hotfix/permanent-la

Conversation

@ivan-flamingo

@ivan-flamingo ivan-flamingo commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What this fix does not do:

Collapse six facets into one Pinot query when six facets are genuinely requested.

What this fix do:

After the PR, the cost is one query per selected facet:

Caller Document selects Pinot queries before after PR
Onboarding auto-detect filteredCount 6 1
Dashboard / customers overview (shared GET_DEVICE_COUNTS_QUERY) statuses, organizationIds, filteredCount 6 3
Device filter UI all six 6 6

Merging the facets that are requested into a single Pinot query is a separate axis, and is limited by the self-exclusion faceting: when filters are active, each facet query has a different WHERE clause (each facet drops the filter on its own field), so only the unfiltered case could be merged.

Summary by CodeRabbit

Performance

  • Device filter queries now calculate only the facets requested, reducing unnecessary processing.
  • Supports selective retrieval of counts, statuses, device types, operating systems, organizations, and tags.

Compatibility

  • Existing requests continue to return all device-filter facets by default.
  • Field selections, aliases, and fragments are supported without errors.
  • Unrequested facets are safely omitted from selective responses.

@ivan-flamingo ivan-flamingo self-assigned this Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b2708018-d902-4bb5-a1b4-fd067f697df8

📥 Commits

Reviewing files that changed from the base of the PR and between a4ec242 and 102c856.

📒 Files selected for processing (3)
  • openframe-api-lib/src/main/java/com/openframe/api/config/DeviceFilterExecutorConfig.java
  • openframe-api-lib/src/main/java/com/openframe/api/dto/device/DeviceFilterFacet.java
  • openframe-api-lib/src/main/java/com/openframe/api/service/DeviceFilterService.java
🚧 Files skipped from review as they are similar to previous changes (3)
  • openframe-api-lib/src/main/java/com/openframe/api/config/DeviceFilterExecutorConfig.java
  • openframe-api-lib/src/main/java/com/openframe/api/dto/device/DeviceFilterFacet.java
  • openframe-api-lib/src/main/java/com/openframe/api/service/DeviceFilterService.java

📝 Walkthrough

Walkthrough

DeviceDataFetcher detects selected device-filter facets from GraphQL queries. DeviceFilterService executes only requested facet queries through a dedicated executor. Tests cover selection handling, compatibility defaults, aliases, fragments, and schema mappings.

Changes

Device-filter facet selection

Layer / File(s) Summary
Facet contract and executor configuration
openframe-api-lib/src/main/java/com/openframe/api/dto/device/DeviceFilterFacet.java, openframe-api-lib/src/main/java/com/openframe/api/config/DeviceFilterExecutorConfig.java
Defines six device-filter facets with GraphQL field mappings and configures the dedicated executor.
Selective facet service execution
openframe-api-lib/src/main/java/com/openframe/api/service/DeviceFilterService.java, openframe-api-lib/src/test/java/com/openframe/api/service/DeviceFilterServiceTest.java
Adds requested-facet selection, executor-backed queries, null futures for omitted facets, and compatibility defaults.
GraphQL selection wiring and validation
openframe-api-service-core/src/main/java/com/openframe/api/datafetcher/DeviceDataFetcher.java, openframe-api-service-core/src/test/java/com/openframe/api/datafetcher/DeviceFiltersSelectionSetTest.java
Maps GraphQL selections, fragments, and aliases to requested facets and validates production schema mappings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GraphQL as DeviceDataFetcher
  participant Service as DeviceFilterService
  participant Executor as DeviceFilterFacetExecutor
  participant Repository as DeviceFilterRepository
  GraphQL->>Service: requested DeviceFilterFacet values
  Service->>Executor: submit selected facet queries
  Executor->>Repository: execute repository queries
  Repository-->>Service: facet results
  Service-->>GraphQL: DeviceFilters result
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: reducing unnecessary Pinot queries for device filter facets.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/permanent-la

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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
`@openframe-api-lib/src/main/java/com/openframe/api/dto/device/DeviceFilterFacet.java`:
- Around line 26-27: Make the DeviceFilterFacet.ALL collection unmodifiable
while preserving its complete EnumSet contents, so callers cannot mutate the
shared default used by the single-argument service overload and null-selection
fallback.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 63a5f53a-30a0-4349-af08-a88599c0a2a4

📥 Commits

Reviewing files that changed from the base of the PR and between 70e80a4 and 6a1f9e5.

📒 Files selected for processing (6)
  • openframe-api-lib/src/main/java/com/openframe/api/config/DeviceFilterExecutorConfig.java
  • openframe-api-lib/src/main/java/com/openframe/api/dto/device/DeviceFilterFacet.java
  • openframe-api-lib/src/main/java/com/openframe/api/service/DeviceFilterService.java
  • openframe-api-lib/src/test/java/com/openframe/api/service/DeviceFilterServiceTest.java
  • openframe-api-service-core/src/main/java/com/openframe/api/datafetcher/DeviceDataFetcher.java
  • openframe-api-service-core/src/test/java/com/openframe/api/datafetcher/DeviceFiltersSelectionSetTest.java

@oleksandrd-flamingo
oleksandrd-flamingo force-pushed the hotfix/permanent-la branch 2 times, most recently from ecf64c4 to a4ec242 Compare August 7, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants