[LFXV2-924] Add direct field filtering support for data object queries#28
[LFXV2-924] Add direct field filtering support for data object queries#28
Conversation
WalkthroughThis PR introduces filter functionality to the query service by adding a "filters" parameter (field:value format) to query endpoints. Filters are parsed into domain objects, propagated through the criteria conversion pipeline, and rendered in the OpenSearch query template as term queries, enabling field-based filtering in search operations. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Service as Query Service
participant Converter as Criteria Converter
participant Parser as Filter Parser
participant Template as OpenSearch Template
participant Search as OpenSearch Engine
Client->>Service: GET /query/resources?filters=status:active
Service->>Converter: payloadToCriteria(payload with filters)
Converter->>Parser: parseFilters(["status:active"])
Parser-->>Converter: FieldFilter{Field: "data.status", Value: "active"}
Converter->>Converter: Set Criteria.Filters
Converter-->>Service: SearchCriteria with Filters
Service->>Template: Render query with Criteria.Filters
Template->>Template: {{range .Filters}} → term query
Template-->>Search: {"must": [..., {"term": {"data.status": "active"}}]}
Search-->>Service: Results
Service-->>Client: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cmd/service/converters_test.go (1)
636-640: Consider validating against empty filter values.The test case "invalid filter format (empty after colon)" expects success for input
"status:", resulting inFieldFilter{Field: "data.status", Value: ""}.Empty values in OpenSearch term queries typically return no results. Consider whether empty values should be rejected during parsing (similar to missing colons) to provide earlier feedback to API consumers.
cmd/service/converters.go (1)
44-49: Verify intentional difference in error handling strategies.There's an inconsistency in how filter parsing errors are handled:
- payloadToCriteria (lines 44-49): Returns error and empty criteria on parse failure
- payloadToCountPublicCriteria (lines 125-135): Logs error but continues with empty filters
- payloadToCountAggregationCriteria (lines 162-172): Logs error but continues with empty filters
The count endpoints are more lenient, which may be intentional design (allowing partial results even with malformed filters). However, this could lead to confusing behavior where invalid filters are silently ignored in count queries but fail in resource queries.
Consider whether count endpoints should also return errors for invalid filter syntax to maintain consistency and provide clear feedback to API consumers.
Also applies to: 125-135, 162-172
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (10)
gen/http/cli/lfx_v2_query_service/cli.gois excluded by!**/gen/**gen/http/openapi.jsonis excluded by!**/gen/**gen/http/openapi.yamlis excluded by!**/gen/**gen/http/openapi3.jsonis excluded by!**/gen/**gen/http/openapi3.yamlis excluded by!**/gen/**gen/http/query_svc/client/cli.gois excluded by!**/gen/**gen/http/query_svc/client/encode_decode.gois excluded by!**/gen/**gen/http/query_svc/server/encode_decode.gois excluded by!**/gen/**gen/http/query_svc/server/types.gois excluded by!**/gen/**gen/query_svc/service.gois excluded by!**/gen/**
📒 Files selected for processing (6)
cmd/service/converters.gocmd/service/converters_test.godesign/query-svc.gointernal/domain/model/search_criteria.gointernal/infrastructure/opensearch/searcher_test.gointernal/infrastructure/opensearch/template.go
🧰 Additional context used
📓 Path-based instructions (4)
internal/domain/model/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core business entities (e.g., Resource, SearchCriteria, AccessCheck) under internal/domain/model/
Files:
internal/domain/model/search_criteria.go
internal/infrastructure/opensearch/**
📄 CodeRabbit inference engine (CLAUDE.md)
Put OpenSearch implementations for resource search under internal/infrastructure/opensearch/
Files:
internal/infrastructure/opensearch/template.gointernal/infrastructure/opensearch/searcher_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Name Go test files with the *_test.go suffix and keep them alongside implementation files
Files:
cmd/service/converters_test.gointernal/infrastructure/opensearch/searcher_test.go
design/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep Goa API design specifications in the design/ directory
Files:
design/query-svc.go
🧬 Code graph analysis (3)
cmd/service/converters_test.go (1)
internal/domain/model/search_criteria.go (1)
FieldFilter(7-10)
internal/infrastructure/opensearch/searcher_test.go (1)
internal/domain/model/search_criteria.go (2)
SearchCriteria(13-46)FieldFilter(7-10)
cmd/service/converters.go (2)
internal/domain/model/search_criteria.go (2)
FieldFilter(7-10)SearchCriteria(13-46)gen/query_svc/service.go (1)
QueryResourcesPayload(145-168)
⏰ 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). (2)
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Agent
🔇 Additional comments (5)
internal/domain/model/search_criteria.go (1)
6-19: LGTM! Clean domain model additions.The
FieldFilterstruct andFiltersfield are well-defined and integrate cleanly into the existingSearchCriteriastructure. The comment clearly documents the AND logic behavior.internal/infrastructure/opensearch/searcher_test.go (1)
314-345: Good test coverage for filter rendering.The three new test cases effectively validate:
- Single field filter rendering
- Multiple field filters rendering
- Combination of tags_all with filters
Note: Once the OpenSearch template is updated to place filters in the
bool.filterclause (per the critical issue in template.go), verify that these tests still pass and update assertions if needed.cmd/service/converters_test.go (1)
593-676: Comprehensive test coverage for filter parsing.The test suite effectively covers all important parsing scenarios including edge cases. Well done!
cmd/service/converters.go (1)
19-40: Clean filter parsing implementation.The
parseFiltersfunction correctly:
- Handles empty/nil input
- Splits on first colon to support colons in values
- Trims whitespace from field names and values
- Auto-prefixes fields with "data."
- Returns clear error messages for invalid formats
design/query-svc.go (1)
59-61: LGTM! Consistent API design for filters parameter.The
filtersparameter is properly defined across bothquery-resourcesandquery-resources-countendpoints with:
- Clear descriptions of the field:value format
- Documentation of automatic data. prefixing
- Appropriate examples
- Consistent HTTP parameter mappings
Also applies to: 84-84, 127-129, 154-154
There was a problem hiding this comment.
Pull request overview
This PR adds direct field filtering support for resource queries through a new filters query parameter. The implementation enables API consumers to filter resources by any field within the data object using OpenSearch term clauses, with fields automatically prefixed with data. to scope filtering appropriately.
Key Changes:
- Added
filtersparameter acceptingfield:valueformat with automaticdata.prefixing - Implemented filter parsing with AND logic for multiple filters
- Updated OpenSearch query template to render term clauses for filters
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
internal/domain/model/search_criteria.go |
Added FieldFilter struct to represent field:value filter pairs |
cmd/service/converters.go |
Implemented parseFilters() function and integrated filter parsing into payload converter functions |
cmd/service/converters_test.go |
Added comprehensive unit tests for filter parsing covering various edge cases |
internal/infrastructure/opensearch/template.go |
Updated OpenSearch query template to render term clauses for each filter in the must array |
internal/infrastructure/opensearch/searcher_test.go |
Added integration tests verifying query rendering with filters |
gen/query_svc/service.go |
Added Filters field to query payload types with documentation |
gen/http/query_svc/server/types.go |
Updated payload builder functions to include filters parameter |
gen/http/query_svc/server/encode_decode.go |
Added filters query parameter decoding for both endpoints |
gen/http/query_svc/client/encode_decode.go |
Added filters query parameter encoding for client requests |
gen/http/query_svc/client/cli.go |
Updated CLI payload builders to parse filters from JSON input |
gen/http/cli/lfx_v2_query_service/cli.go |
Added CLI flags and usage examples for filters parameter |
design/query-svc.go |
Added filters attribute to API design specification for both endpoints |
gen/http/openapi3.yaml |
Added filters parameter documentation to OpenAPI 3.0 spec |
gen/http/openapi3.json |
Added filters parameter to OpenAPI 3.0 JSON spec |
gen/http/openapi.yaml |
Added filters parameter documentation to OpenAPI 2.0 spec |
gen/http/openapi.json |
Added filters parameter to OpenAPI 2.0 JSON spec |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This commit adds a new 'filters' query parameter that allows filtering on any field within the data object using term clauses. Fields are automatically prefixed with 'data.' to ensure filtering only occurs within the resource data object. Key changes: - Added 'filters' parameter to query-resources and query-resources-count endpoints - Format: 'field:value' (e.g., 'status:active', 'priority:high') - Fields are automatically scoped to 'data.' prefix - All filters use AND logic (all must match) - Added FieldFilter model with Field and Value properties - Updated OpenSearch template to render term clauses for each filter - Added comprehensive tests for filter parsing and query rendering API usage: GET /query/resources?filters=status:active&filters=priority:high&v=1 This generates an OpenSearch query with: - term clause for data.status = "active" - term clause for data.priority = "high" Resources without matching data fields will return no results as the term clauses require exact field matches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
314cfff to
7647096
Compare
- Update payloadToCountPublicCriteria to return errors - Update payloadToCountAggregationCriteria to return errors - Add field name validation to prevent empty field names - Add test coverage for empty field name scenarios - Ensure consistent error propagation across all converter functions All 11 filter parsing tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
…service into andrest50/query-direct-fields
Ticket
https://linuxfoundation.atlassian.net/browse/LFXV2-924
Summary
Added a new
filtersquery parameter that enables direct field filtering on any field within the resource's data object using OpenSearch term clauses. This provides a flexible way to filter resources by their data field values without requiring tags.Changes
API Design
filtersparameter toquery-resourcesandquery-resources-countendpointsfield:value(e.g.,status:active,priority:high)data.to scope filtering to the data objectImplementation
FieldFilterstruct withFieldandValuepropertiesparseFilters()function to parsefield:valueformat and auto-prefix withdata.OpenSearch Query Example
Request:
Generated OpenSearch query:
{ "query": { "bool": { "filter": [ { "term": { "latest": true } }, { "term": { "data.status": "active" } }, { "term": { "data.priority": "high" } } ] } } }Key Features
data.prefixproject.idbecomesdata.project.id)Testing
parseFilters()function covering:Test Plan
🤖 Generated with Claude Code