[v2] Container view types#413
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## anilm3/v2 #413 +/- ##
=============================================
+ Coverage 85.64% 85.76% +0.11%
=============================================
Files 176 176
Lines 9015 9088 +73
Branches 3842 3860 +18
=============================================
+ Hits 7721 7794 +73
+ Misses 494 492 -2
- Partials 800 802 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Benchmarks clang-pgoBenchmark execution time: 2025-06-09 14:32:10 Comparing candidate commit c8eba08 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. |
da3a16a to
ac364f7
Compare
Benchmarks clangBenchmark execution time: 2025-06-09 14:15:43 Comparing candidate commit c8eba08 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 0 metrics, 0 unstable metrics. scenario:global-benchmark.random.clang
|
Benchmarks gccBenchmark execution time: 2025-06-09 14:18:55 Comparing candidate commit c8eba08 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 0 metrics, 0 unstable metrics. scenario:global-benchmark.random.gcc
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two new container view types, array_view and map_view, and updates various modules to use them instead of the more generic object_view when a map or array is expected. Key changes include:
- Refactored function signatures and test cases to replace object_view with map_view/array_view.
- Adjusted internal loops and helper functions to iterate using the new container view types.
- Updated conversion routines in configuration and argument retrieval code to leverage the new views.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/processor/fingerprint_test.cpp | Removed tests for invalid container types and adjusted expectations in fingerprint tests. |
| tests/unit/object_view_test.cpp | Updated key path tests and added comprehensive tests for array_view and map_view. |
| src/processor/fingerprint.hpp / cpp | Refactored fingerprint evaluation functions to use map_view instead of object_view and updated iteration patterns. |
| src/object_store.hpp / cpp | Changed insert functions to accept map_view, with added type checks. |
| src/object.hpp | Introduced array_view and map_view classes along with cast and iteration functionality. |
| src/configuration/common/raw_configuration.cpp | Modified conversion operators to build on new container view types. |
| src/argument_retriever.hpp | Expanded type conversions to support array_view and map_view. |
Comments suppressed due to low confidence (2)
src/processor/fingerprint.cpp:597
- Remove the leftover commented code that duplicates header extraction since the ranged-for loop already provides clear header access.
// const auto [key, child] = headers.value.at(i);
tests/unit/processor/fingerprint_test.cpp:140
- Since tests for invalid query or header types have been removed in favor of new container view types, consider adding equivalent tests that verify map_view and array_view enforce valid type constraints and throw or report errors appropriately.
TEST(TestHttpEndpointFingerprint, InvalidQueryType)
| array_view(const detail::object *o) | ||
| { | ||
| if (o == nullptr || o->type != object_type::array) { | ||
| throw std::invalid_argument("array_view initialised with null or incompatible type"); | ||
| } | ||
| data_ = o->via.array.ptr; | ||
| size_ = o->via.array.size; | ||
| } | ||
| // NOLINTNEXTLINE(google-explicit-constructor, hicpp-explicit-conversions) | ||
| array_view(const detail::object &o) : array_view(&o) {} | ||
| // NOLINTNEXTLINE(google-explicit-constructor, hicpp-explicit-conversions) | ||
| array_view(object_view o) : array_view(o.ptr()) {} | ||
| // NOLINTNEXTLINE(google-explicit-constructor, hicpp-explicit-conversions) | ||
| array_view(const owned_object &ow); | ||
| // NOLINTNEXTLINE(google-explicit-constructor, hicpp-explicit-conversions) | ||
| array_view(const borrowed_object &ow); |
There was a problem hiding this comment.
Repetitive... maybe using some concept for types convertible to detail::object *? If not std::convertible_to because you don't want to make it directly convertible, some other method to expose the pointer.
There was a problem hiding this comment.
I'll address this in the next PR.
* Object view: read only abstraction to ddwaf_object (#341) * [v2] Remove mingw builds (#381) * Writable objects: owned and borrowed object and object limits removal (#378, #382) * [v2] Refactor and improve object types (#387) * [v2] Update unit tests to use new abstractions (#389) * [v2] Update `raw_configuration` type to use `object_view` (#390) * [v2] Remove remaining uses of ddwaf_object in `src` and `tests/unit` (#391) * [v2] JWT Decoding Processor (#401) * [v2] First iteration of object layout changes (#394) * Split context data insertion from evaluation (#407) * [v2] Second iteration of object layout changes (#408) * [v2] Add new fingerprint and object view tests (#414) * Reenable attribute collector unit test (#415) * [v2] Container view types (#413) * Exclude assertions from coverage (#416) * [v2] Use allocators internally instead of malloc/free and stop generating zero-terminated strings (#418) * Add memory resource to owned and borrowed objects (#428) * [v2] Propagate allocators from context (#420) * [v2] Update interface and expose allocators (#427) * Refactor evaluation stages out of the context (#442) * Subcontext: replace ephemerals with a new scope with user-defined lifetime derived from the context (#443) * Validator: Add support for testing subcontexts and attributes (#451) * [v2] Pass allocator to context and subcontext eval and add new allocators (#452) * Update logger to avoid dependencies on ddwaf.h (#453) * [v2] Return DDWAF_MATCH when there are events, attributes or actions (#455) * [v2] Cleanup: remove exclusion namespace and some redundant references (#456)
This PR introduces
array_viewandmap_viewas new container view types to better reflect the required underlying type in cases where this is explicitly needed.The semantics of these view types do not resemble those of STL containers, instead, they follow the semantics of
object_viewitself for consistency acrossdetail::objectwrappers.