[LFXV2-775] Add support for tags_all parameter with AND logic #25
+379
−202
Conversation
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
Implemented a new tags_all query parameter that works alongside the existing tags parameter to support both OR and AND tag filtering logic: - tags: Filters with OR logic (matches resources with ANY of the tags) - tags_all: Filters with AND logic (matches resources with ALL of the tags) - Both parameters can be used together in a single query Changes: - Updated API design to include tags_all parameter - Modified OpenSearch template to handle tags_all in must clause - Updated mock implementation with two-pass filtering - Added TagsAll field to SearchCriteria domain model - Updated all converter functions to map tags_all from payload - Added comprehensive test coverage for both parameters - Regenerated Goa API code 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <[email protected]>
WalkthroughAdds AND-logic tag filtering by introducing a new TagsAll field propagated from API payloads into search criteria, updating OpenSearch template and mock searcher to apply TagsAll (AND) alongside existing Tags (OR), expanding tests, and bumping the Helm chart version. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Converter
participant Searcher
participant Backend
Client->>API: GET /query/resources?tags_all=active,security
API->>Converter: payloadToCriteria(payload)
Converter->>Converter: criteria.TagsAll = payload.TagsAll
API->>Searcher: QueryResources(criteria)
Searcher->>Searcher: Apply OR filter (criteria.Tags)
alt TagsAll present
Searcher->>Searcher: Apply AND filter (criteria.TagsAll)
end
Searcher->>Backend: Execute query (rendered with should + must clauses)
Backend-->>Searcher: Results
Searcher-->>API: Filtered resources
API-->>Client: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (10)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
Comment |
…service into andrest50/tags-all
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for a new tags_all query parameter to enable AND logic filtering alongside the existing tags parameter (OR logic). The implementation spans API design, domain model, OpenSearch query generation, mock implementation, and comprehensive test coverage. Changes include updating the Goa design file, regenerating API code, modifying the OpenSearch query template to use the must clause for AND logic, implementing two-pass filtering in the mock searcher, and adding test cases for OR-only, AND-only, and combined scenarios.
- Added
TagsAllfield toSearchCriteriadomain model for AND logic filtering - Updated OpenSearch query template to include
tags_allinmustclause - Modified mock implementation with separate filtering passes for OR and AND tag logic
- Regenerated Goa API code with new
tags_allparameter support across all layers
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| design/query-svc.go | Added tags_all attribute definition with AND logic semantics and examples |
| internal/domain/model/search_criteria.go | Added TagsAll field to SearchCriteria struct with documentation |
| internal/infrastructure/opensearch/template.go | Added tags_all terms to must clause for AND logic in OpenSearch query |
| internal/infrastructure/mock/resource_searcher.go | Implemented two-pass filtering for tags (OR) and tags_all (AND) logic |
| internal/infrastructure/opensearch/searcher_test.go | Added test cases for tags_all (AND), tags (OR), and combined scenarios |
| internal/infrastructure/mock/resource_searcher_test.go | Added comprehensive test coverage for new filtering logic |
| cmd/service/converters.go | Updated converter functions to map tags_all from payload to criteria |
| gen/query_svc/service.go | Regenerated payload structures with TagsAll field |
| gen/http/query_svc/server/types.go | Regenerated payload constructors with tagsAll parameter |
| gen/http/query_svc/server/encode_decode.go | Added decoding support for tags_all query parameter |
| gen/http/query_svc/client/encode_decode.go | Added encoding support for tags_all query parameter |
| gen/http/query_svc/client/cli.go | Added CLI flag handling for tags-all parameter |
| gen/http/openapi3.yaml | Updated OpenAPI 3.0 specification with tags_all parameter documentation |
| gen/http/openapi.yaml | Updated Swagger 2.0 specification with tags_all parameter documentation |
| gen/http/openapi3.json | Regenerated JSON representation of OpenAPI 3.0 spec |
| gen/http/openapi.json | Regenerated JSON representation of Swagger 2.0 spec |
| gen/http/cli/lfx_v2_query_service/cli.go | Updated CLI usage examples with tags-all parameter |
Comments suppressed due to low confidence (1)
internal/infrastructure/opensearch/template.go:69
- The
minimum_should_matchandshouldfields are placed incorrectly in the JSON structure. Line 66 closes themustarray with], and then lines 67-83 attempt to addminimum_should_matchandshouldas siblings tomust. However, the comma on line 67 is placed after the closing bracket of themustarray, which would create invalid JSON. These fields should be properly positioned as siblings tomustwithin theboolobject. The correct structure should have the comma before line 67 removed and these fields should be at the same indentation level as themustarray (inside theboolobject but not inside themustarray).
]
{{- if .Tags }},
"minimum_should_match": 1,
"should": [
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Andres Tobon <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit 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/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/encode_decode.gois excluded by!**/gen/**gen/http/query_svc/client/types.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/client.gois excluded by!**/gen/**gen/query_svc/service.gois excluded by!**/gen/**
📒 Files selected for processing (1)
design/query-svc.go(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
design/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep Goa API design specifications in the design/ directory
Files:
design/query-svc.go
🧠 Learnings (2)
📚 Learning: 2025-09-10T17:40:14.123Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-query-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T17:40:14.123Z
Learning: Applies to cmd/query_svc/query_svc.go : Keep the service implementation that connects Goa to domain logic in cmd/query_svc/query_svc.go
Applied to files:
design/query-svc.go
📚 Learning: 2025-09-10T17:40:14.123Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-query-service PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-10T17:40:14.123Z
Learning: Applies to internal/service/resource_search.go : Implement use case orchestration for resource search in internal/service/resource_search.go
Applied to files:
design/query-svc.go
⏰ 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). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (4)
design/query-svc.go (4)
53-58: LGTM! Clear API contract for OR vs AND tag filtering.The descriptions explicitly state the semantic difference between
tags(OR logic) andtags_all(AND logic), and the examples appropriately demonstrate multiple tag values for each parameter.
80-80: LGTM! HTTP parameter mapping is correct.The
tags_allparameter is properly mapped for the query-resources endpoint.
117-122: LGTM! Consistent implementation across endpoints.The
tagsandtags_allattributes for query-resources-count are identical to those in query-resources, ensuring consistent API behavior.
146-146: LGTM! HTTP parameter mapping is correct.The
tags_allparameter is properly mapped for the query-resources-count endpoint, consistent with the query-resources endpoint.
Fixed a bug where the query-resources-count endpoint would panic with
a nil pointer dereference when returning BadRequest errors. The issue
was caused by an inconsistency in the API design where the endpoint
was using dsl.ErrorResult (which generates *goa.ServiceError) instead
of the custom BadRequestError type used by other endpoints.
The generated error handling code tried to cast the error to
*goa.ServiceError, but the service was returning *querysvc.BadRequestError,
causing the cast to fail and leaving the error pointer as nil.
Changes:
- Removed the redundant dsl.Error("BadRequest", dsl.ErrorResult, ...)
declaration from query-resources-count endpoint
- This makes it consistent with other endpoints which use the
service-level BadRequestError type defined at line 22
- Regenerated Goa code to fix the error handling
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Signed-off-by: Andres Tobon <[email protected]>
andrest50
force-pushed
the
andrest50/tags-all
branch
from
5f743b5 to
5141b4f
Compare
mauriciozanettisalomao
approved these changes
Nov 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
tags_allquery parameter that works alongside the existingtagsparameter to support both OR and AND tag filtering logictags: Filters with OR logic (matches resources with ANY of the tags)tags_all: Filters with AND logic (matches resources with ALL of the tags)Changes
tags_allparameter for both/query/resourcesand/query/resources/countendpointstags_allinmustclause (AND logic) andtagsinshouldclause (OR logic)TagsAllfield toSearchCriteriadomain modeltags_allfrom payloadExample Usage
?tags=active&tags=public(matches resources with active OR public)?tags_all=governance&tags_all=security(matches resources with governance AND security)?tags=active&tags_all=governance&tags_all=security(matches resources with active AND (governance AND security))Test Plan
Ticket
LFXV2-775
🤖 Generated with Claude Code