feat(audit-logs): improve audit logs search - #52
Conversation
📝 WalkthroughWalkthrough
ChangesAudit Log Text Search Filter
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
persistence/src/main/kotlin/com/alleslocker/backend/persistence/auditlog/specification/AuditLogSpecification.kt (2)
23-45: Consider performance implications and input validation for text search.The implementation uses
LIKEwith leading wildcards (%...%) across multiple fields via a LEFT JOIN, which has several performance characteristics to consider:
- Index usage: Leading wildcard patterns prevent database index usage, requiring full table scans on large audit log tables
- Join overhead: The LEFT JOIN to
UserEntityadds query complexity- Multiple LIKE operations: The OR predicate evaluates up to four LIKE operations per row
- DISTINCT overhead: While correctly skipped for count queries,
distinct(true)adds result deduplication costAdditionally, there's no length validation on
searchText, which could allow very long inputs that amplify these performance costs.Recommendations:
- Add validation in the DTO or use case to limit
searchTextlength (e.g., max 100 characters)- Monitor query performance on production-scale data
- If search becomes a bottleneck, consider:
- Database full-text search features (PostgreSQL
tsvector, MySQL FULLTEXT)- Dedicated search solutions (Elasticsearch, OpenSearch)
- Computed search columns with appropriate indexes
🤖 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 `@persistence/src/main/kotlin/com/alleslocker/backend/persistence/auditlog/specification/AuditLogSpecification.kt` around lines 23 - 45, Add length validation on the searchText parameter in the AuditLogSpecification filtering logic to prevent very long inputs from amplifying performance costs. Modify the condition where searchText is processed (in the takeIf or let block) to limit the maximum length to a reasonable value such as 100 characters. This validation can be applied either by chaining an additional takeIf condition that checks the length, or by validating at the start of the let block where the pattern is created and the LIKE queries are built. This will help mitigate the performance impact of the leading wildcard LIKE operations across multiple fields in the LEFT JOIN.
26-32: Add integration tests to verify pagination count behavior with searchText filter.The count query detection correctly checks for
Longtypes (bothjava.lang.Longand primitivelong), which is the standard pattern Spring Data JPA uses when executing count queries during pagination. The conditional application ofdistinct(true)is correct—it's only applied to content queries, not count queries, preventing count inaccuracies.However, there are no existing tests for
getAllAuditLogsPagedwith the searchText filter. Consider adding integration tests to verify:
totalElementscount is accurate when usingsearchTextwith the LEFT join onperformedBy- The distinct behavior doesn't affect the count query results
- Pagination works correctly across multiple pages with filtered results
Test suggestion
`@Test` fun `should return correct total count with searchText filter`() { // Given: Create audit logs with and without users, some matching search text // When: Filter with searchText and request page // Then: Verify totalElements count matches actual distinct results }🤖 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 `@persistence/src/main/kotlin/com/alleslocker/backend/persistence/auditlog/specification/AuditLogSpecification.kt` around lines 26 - 32, Add integration tests for the `getAllAuditLogsPaged` method in the AuditLogSpecification class to verify pagination behavior with the searchText filter. Create test cases that verify: (1) the totalElements count is accurate when filtering with searchText and the LEFT join on performedBy, (2) the distinct behavior applied in the conditional check does not negatively affect count query results, and (3) pagination correctly works across multiple pages when searchText filter is applied. These tests should validate that the count query detection logic (checking for Long and primitive long types) and the conditional application of distinct(true) work together correctly to produce accurate pagination results.
🤖 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
`@persistence/src/main/kotlin/com/alleslocker/backend/persistence/auditlog/specification/AuditLogSpecification.kt`:
- Line 34: The search text is being directly interpolated into the LIKE pattern
without escaping SQL wildcard characters, which will cause `%` and `_`
characters in user input to be treated as wildcards. In the line where pattern
is assigned with `"%${searchText.lowercase()}%"`, you need to escape special
characters in the searchText before interpolation by replacing `%` with `\%`,
`_` with `\_`, and `\` with `\\`. Then use the three-parameter overload of
CriteriaBuilder.like() that accepts an escape character parameter (typically
`\`) when building the query condition. Apply this same escaping logic to all
other occurrences where search text is used to build LIKE patterns in this
specification class.
---
Nitpick comments:
In
`@persistence/src/main/kotlin/com/alleslocker/backend/persistence/auditlog/specification/AuditLogSpecification.kt`:
- Around line 23-45: Add length validation on the searchText parameter in the
AuditLogSpecification filtering logic to prevent very long inputs from
amplifying performance costs. Modify the condition where searchText is processed
(in the takeIf or let block) to limit the maximum length to a reasonable value
such as 100 characters. This validation can be applied either by chaining an
additional takeIf condition that checks the length, or by validating at the
start of the let block where the pattern is created and the LIKE queries are
built. This will help mitigate the performance impact of the leading wildcard
LIKE operations across multiple fields in the LEFT JOIN.
- Around line 26-32: Add integration tests for the `getAllAuditLogsPaged` method
in the AuditLogSpecification class to verify pagination behavior with the
searchText filter. Create test cases that verify: (1) the totalElements count is
accurate when filtering with searchText and the LEFT join on performedBy, (2)
the distinct behavior applied in the conditional check does not negatively
affect count query results, and (3) pagination correctly works across multiple
pages when searchText filter is applied. These tests should validate that the
count query detection logic (checking for Long and primitive long types) and the
conditional application of distinct(true) work together correctly to produce
accurate pagination results.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea4d0ffb-c95e-4e93-b7b1-46cc95270efc
📒 Files selected for processing (2)
application/src/main/kotlin/com/alleslocker/backend/application/auditlog/dto/filter/AuditLogFilterDto.ktpersistence/src/main/kotlin/com/alleslocker/backend/persistence/auditlog/specification/AuditLogSpecification.kt
Summary by CodeRabbit