[LFXV2-919] Add date range filtering support to query service endpoints#29
[LFXV2-919] Add date range filtering support to query service endpoints#29
Conversation
- Add date_field, date_from, and date_to query parameters - Support both ISO 8601 datetime and date-only formats - Auto-convert date-only to start/end of day UTC - Auto-prefix date fields with "data." to scope to data object - Add comprehensive test coverage with 17 test cases - Update both query-resources and query-resources-count endpoints 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
WalkthroughThis pull request adds date range filtering capabilities to the Resource Search API. It introduces three new parameters ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Converter as Payload Converter
participant Domain as Domain Model
participant Template as OpenSearch Template
participant Search as OpenSearch
Client->>Converter: POST /query-resources<br/>(date_field, date_from, date_to)
Converter->>Converter: parseDateFilter()<br/>(normalize & validate)
Converter->>Converter: Validate date_field required<br/>if date_from/date_to present
Converter->>Domain: Create SearchCriteria<br/>(DateField, DateFrom, DateTo)
Domain->>Template: Render query with<br/>date filter block
Template->>Template: If DateField set:<br/>append range clause
Template->>Search: Execute query with<br/>gte/lte constraints
Search-->>Client: Return filtered results
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cmd/service/converters_test.go (1)
690-817: Good integration tests for date filtering.The tests verify the complete flow from payload to criteria, including the
data.prefix behavior and date normalization. Consider adding a test case for whenDateFieldis provided but bothDateFromandDateToare nil—this scenario is valid but not explicitly tested.🔎 Optional: Additional edge case test
{ name: "date_field provided but no date bounds", payload: &querysvc.QueryResourcesPayload{ DateField: stringPtr("updated_at"), }, expectError: false, checkCriteria: func(t *testing.T, c model.SearchCriteria) { assert.NotNil(t, c.DateField) assert.Equal(t, "data.updated_at", *c.DateField) assert.Nil(t, c.DateFrom) assert.Nil(t, c.DateTo) }, },cmd/service/converters.go (2)
96-121: Consider extracting duplicate date handling logic.The date filtering logic is duplicated across three functions:
payloadToCriteria,payloadToCountPublicCriteria, andpayloadToCountAggregationCriteria. This could be extracted into a helper function to improve maintainability.🔎 Proposed refactor to extract common date handling
+// applyDateFilters parses and applies date filtering parameters to the criteria. +// Returns an error if date format is invalid. +func applyDateFilters(criteria *model.SearchCriteria, dateField, dateFrom, dateTo *string) error { + if dateField == nil { + return nil + } + + // Auto-prefix with "data." to scope to data object + prefixedField := "data." + *dateField + criteria.DateField = &prefixedField + + // Parse and normalize date_from + if dateFrom != nil { + normalizedFrom, err := parseDateFilter(*dateFrom, false) + if err != nil { + return fmt.Errorf("invalid date_from: %w", err) + } + criteria.DateFrom = &normalizedFrom + } + + // Parse and normalize date_to + if dateTo != nil { + normalizedTo, err := parseDateFilter(*dateTo, true) + if err != nil { + return fmt.Errorf("invalid date_to: %w", err) + } + criteria.DateTo = &normalizedTo + } + + return nil +}Then use it in each converter function:
if err := applyDateFilters(&criteria, p.DateField, p.DateFrom, p.DateTo); err != nil { return criteria, wrapError(ctx, err) }Also applies to: 171-194, 225-248
97-100: Consider validating DateField to prevent unexpected nested field access.The
DateFieldvalue is directly concatenated without validation. A malicious or erroneous input like"foo.bar.baz"would result in"data.foo.bar.baz", potentially accessing unintended nested fields. Consider validating thatDateFieldcontains only alphanumeric characters and underscores.🔎 Proposed validation
+import "regexp" + +// validDateFieldPattern matches simple field names (alphanumeric and underscores) +var validDateFieldPattern = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`) + // Handle date filtering parameters if p.DateField != nil { + if !validDateFieldPattern.MatchString(*p.DateField) { + return criteria, fmt.Errorf("invalid date_field: must contain only alphanumeric characters and underscores") + } // Auto-prefix with "data." to scope to data object prefixedField := "data." + *p.DateField criteria.DateField = &prefixedField
📜 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 (8)
CLAUDE.mdREADME.mdcmd/service/converters.gocmd/service/converters_test.gocmd/service/service.godesign/query-svc.gointernal/domain/model/search_criteria.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.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.go
design/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep Goa API design specifications in the design/ directory
Files:
design/query-svc.go
🧬 Code graph analysis (1)
cmd/service/converters.go (2)
gen/query_svc/service.go (1)
QueryResourcesCountPayload(110-135)internal/domain/model/search_criteria.go (1)
SearchCriteria(7-44)
⏰ 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: Agent
- GitHub Check: CodeQL analysis (go)
🔇 Additional comments (9)
internal/domain/model/search_criteria.go (1)
38-43: LGTM!The new date-range fields are well-documented and follow the existing conventions of the struct with optional pointer types.
design/query-svc.go (2)
59-67: LGTM!The API design for date filtering is well-documented with clear format examples. The validation of date formats is appropriately handled at the service layer (in
parseDateFilter), which provides better error messages than pattern-based validation.
135-143: Consistent implementation across endpoints.Date filtering parameters are correctly mirrored between
query-resourcesandquery-resources-countendpoints, ensuring a consistent API surface.README.md (1)
245-283: Excellent documentation!The Date Range Filtering section provides comprehensive examples covering all use cases (date-only, ISO 8601, open-ended, and combined filters) with clear format notes explaining the conversion behavior.
internal/infrastructure/opensearch/template.go (1)
66-80: LGTM!The template logic correctly handles all three date range scenarios (from-only, to-only, and both) with proper comma placement and JSON structure. The conditional
{{- if and .DateFrom .DateTo }},{{- end }}on line 73 ensures valid JSON when both bounds are present.CLAUDE.md (1)
113-155: LGTM!The developer documentation clearly explains the date range filtering feature with implementation details pointing to specific code locations. This is valuable for maintainability and onboarding.
cmd/service/service.go (1)
79-86: LGTM!Error handling is properly added for the payload conversion functions. The pattern is consistent with how
QueryResourceshandles errors frompayloadToCriteria.cmd/service/converters_test.go (1)
593-688: Comprehensive date parsing tests!The test cases thoroughly cover ISO 8601 formats, date-only conversions, empty strings, and various invalid format scenarios. The milliseconds truncation test (line 665-670) is a nice edge case to catch.
cmd/service/converters.go (1)
19-53: Well-implemented date parsing logic.The function handles both ISO 8601 and date-only formats correctly, with clear error messages. The comment on line 45 explaining the 23:59:59 choice is helpful for future maintainers.
- Document date_field, date_from, and date_to parameters in README - Add usage examples with both ISO 8601 and date-only formats - Include date format conversion notes and behavior details - Update CLAUDE.md with implementation details and examples - Add references to relevant source files for maintainability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
andrest50
force-pushed
the
andrest50/query-date-range
branch
from
af2c8e1 to
682615f
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds date range filtering capabilities to the query service endpoints (/query/resources and /query/resources/count), enabling clients to filter resources by date fields within the data object. The implementation supports both ISO 8601 datetime and date-only formats with automatic conversion to start/end of day UTC.
Key Changes:
- Added three new query parameters:
date_field,date_from, anddate_toto support flexible date range filtering - Implemented date parsing logic with dual format support (ISO 8601 datetime and date-only YYYY-MM-DD)
- Extended OpenSearch query templates with range query support for date filtering
- Added comprehensive test coverage with 17 test cases covering various date formats and edge cases
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| design/query-svc.go | Added date_field, date_from, and date_to attributes to query-resources and query-resources-count endpoints |
| internal/domain/model/search_criteria.go | Extended SearchCriteria struct with DateField, DateFrom, and DateTo fields |
| internal/infrastructure/opensearch/template.go | Added range query template logic for date filtering with gte/lte operators |
| cmd/service/converters.go | Implemented parseDateFilter function with dual format support and integrated date filtering into payload-to-criteria converters |
| cmd/service/service.go | Updated error handling for converter functions that now return errors |
| cmd/service/converters_test.go | Added 10 tests for date parsing and 7 tests for payload-to-criteria conversion with date filters |
| gen/* (multiple files) | Auto-generated Goa framework files reflecting the new API parameters |
| README.md | Added comprehensive documentation with usage examples and format notes |
| CLAUDE.md | Added implementation details and API feature documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
217-221: Clarify whetherdate_fieldis required when date range parameters are provided.The parameter documentation states that
date_fieldis "used with date_from and/or date_to," but does not explicitly indicate whetherdate_fieldis mandatory whendate_fromordate_toare supplied. This could cause confusion for API consumers.Recommend adding a note clarifying the requirement: e.g., "Required when
date_fromand/ordate_toare provided."
📜 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 selected for processing (2)
CLAUDE.mdREADME.md
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🔇 Additional comments (3)
README.md (3)
217-218: Verify backward compatibility fortagsparameter changes.The documentation now lists both
tags(OR logic) andtags_all(AND logic) as separate parameters. The AI summary indicates that the "single tags filter" was replaced with these two parameters. Clarify whether:
- The old
tagsparameter is still supported (new feature addingtags_all)- The old
tagsparameter has been removed (breaking change requiring migration)If this is a breaking change, the README should include a deprecation note or migration guide for existing API consumers.
246-283: Date range filtering documentation is clear and comprehensive.The new section provides well-structured examples covering date-only, ISO 8601, open-ended, and combined filtering scenarios. The format notes correctly explain UTC normalization semantics, inclusive behavior, and the automatic "data." prefix scoping. One minor enhancement: the date-only end-of-day conversion documents
T23:59:59Zbut does not mention microsecond/nanosecond precision for edge cases at day boundaries (e.g., if a resource timestamp includes fractional seconds). This is unlikely to cause issues in practice but could be clarified for completeness.
188-194: Authentication configuration documentation is well-organized.The AUTH_SOURCE environment variable section clearly documents the choice between "mock" and "jwt" modes, along with associated JWKS, JWT_AUDIENCE, and mock principal configuration. This aligns well with the description at lines 73–79 in the architecture section.
Validate that date_field is required when using date_from or date_to to prevent silent failures where date filtering is ignored. This ensures users receive clear error messages when providing incomplete date filter parameters. Changes: - Add validation in payloadToCriteria to check date_field requirement - Add validation in payloadToCountPublicCriteria for consistency - Add validation in payloadToCountAggregationCriteria for consistency - Add 3 test cases to verify validation behavior - Return 400 Bad Request with clear error message for invalid input All tests passing (10 date filter tests total). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/service/converters.go (1)
178-206: Consider extracting duplicated date filter logic into a helper function.The date validation and normalization logic is duplicated across
payloadToCriteria,payloadToCountPublicCriteria, andpayloadToCountAggregationCriteria(~30 lines each). This increases maintenance burden and risks divergence.Additionally, error handling is inconsistent:
payloadToCriteriauseswrapErrorand logs viaslog, while the count functions return rawfmt.Errorfwithout logging.🔎 Proposed helper to reduce duplication
// applyDateFilters validates and normalizes date filter parameters onto the criteria. // Returns an error if date_from/date_to are provided without date_field. func applyDateFilters(dateField, dateFrom, dateTo *string, criteria *model.SearchCriteria) error { if (dateFrom != nil || dateTo != nil) && dateField == nil { return fmt.Errorf("date_field is required when using date_from or date_to") } if dateField == nil { return nil } prefixedField := "data." + *dateField criteria.DateField = &prefixedField if dateFrom != nil { normalizedFrom, err := parseDateFilter(*dateFrom, false) if err != nil { return fmt.Errorf("invalid date_from: %w", err) } criteria.DateFrom = &normalizedFrom } if dateTo != nil { normalizedTo, err := parseDateFilter(*dateTo, true) if err != nil { return fmt.Errorf("invalid date_to: %w", err) } criteria.DateTo = &normalizedTo } return nil }Then in each converter:
if err := applyDateFilters(p.DateField, p.DateFrom, p.DateTo, &criteria); err != nil { slog.ErrorContext(ctx, "invalid date filter parameters", "error", err) return criteria, wrapError(ctx, err) }Also applies to: 237-265
📜 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 selected for processing (2)
cmd/service/converters.gocmd/service/converters_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*_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.go
🧬 Code graph analysis (2)
cmd/service/converters_test.go (3)
cmd/service/service.go (1)
NewQuerySvc(161-173)gen/query_svc/service.go (1)
QueryResourcesPayload(151-180)internal/domain/model/search_criteria.go (1)
SearchCriteria(7-44)
cmd/service/converters.go (2)
gen/query_svc/service.go (1)
QueryResourcesCountPayload(110-135)internal/domain/model/search_criteria.go (1)
SearchCriteria(7-44)
🔇 Additional comments (4)
cmd/service/converters.go (1)
96-128: Date filter validation and handling looks correct.The validation ensures
date_fieldis required whendate_fromordate_toare provided, preventing silent filter failures. The auto-prefixing withdata.and normalization logic are implemented correctly.cmd/service/converters_test.go (3)
593-688: Comprehensive test coverage for date parsing.The
TestParseDateFilterfunction covers key scenarios: ISO 8601 datetime, date-only formats (start/end of day), empty input, invalid formats, partial datetime, and milliseconds truncation. The error message verification (assert.Contains(t, err.Error(), "invalid date format")) ensures meaningful error feedback.
690-839: Good integration test coverage for date filter validation.The tests effectively cover the date filter integration including:
- ISO 8601 and date-only format normalization
- Open-ended ranges (only
date_fromordate_to)- Invalid format handling
- Validation that
date_fieldis required (lines 797-818)The
checkCriteriacallback pattern keeps assertions focused and readable.
841-844: Standard helper for test readability.The
stringPtrhelper follows common Go testing patterns for creating pointer values in test fixtures.
…service into andrest50/query-date-range
Summary
This PR adds date range filtering support to the query service endpoints, allowing clients to filter resources by date fields within the
dataobject.Ticket
https://linuxfoundation.atlassian.net/browse/LFXV2-919
Changes
New Query Parameters
Added three new query parameters to both
/query/resourcesand/query/resources/countendpoints:date_field(string, optional): Specifies the date field to filter on (automatically prefixed with"data.")date_from(string, optional): Start date for range filter (inclusive, usesgteoperator)date_to(string, optional): End date for range filter (inclusive, useslteoperator)Date Format Support
The implementation supports two date formats:
2025-01-10T15:30:00Z(time used as provided)2025-01-10(automatically converted to start/end of day UTC)date_fromwith date-only →2025-01-10T00:00:00Z(start of day)date_towith date-only →2025-01-10T23:59:59Z(end of day)Implementation Details
Modified Files:
design/query-svc.go: Added new query parameters to Goa API designinternal/domain/model/search_criteria.go: Added DateField, DateFrom, DateTo fieldsinternal/infrastructure/opensearch/template.go: Added range query template logiccmd/service/converters.go: ImplementedparseDateFilter()function with dual format supportcmd/service/service.go: Updated error handling for converter functionscmd/service/converters_test.go: Added 17 comprehensive test casesREADME.md: Added documentation with usage examplesCLAUDE.md: Added implementation details and referencesGenerated Files:
Usage Examples
Date range with date-only format:
Date range with ISO 8601 format:
Open-ended range (only start date):
Combined with other filters:
Test Coverage
TestParseDateFilter)TestPayloadToCriteriaWithDateFilters)OpenSearch Query Translation
The date filters are translated into OpenSearch range queries:
{ "range": { "data.updated_at": { "gte": "2025-01-10T00:00:00Z", "lte": "2025-01-28T23:59:59Z" } } }Future Enhancements
This implementation provides a simple single date range filter. Future enhancements could include:
🤖 Generated with Claude Code