Skip to content

feat: add a new filter function that returns remainders additionally#2019

Merged
frewilhelm merged 3 commits into
open-component-model:mainfrom
frewilhelm:add-filter-out
Mar 19, 2026
Merged

feat: add a new filter function that returns remainders additionally#2019
frewilhelm merged 3 commits into
open-component-model:mainfrom
frewilhelm:add-filter-out

Conversation

@frewilhelm

@frewilhelm frewilhelm commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

Adds a new function FilterWithRemainder() that also returns out-filtered configurations, so the caller can decide what to do with it, e.g. logging the out-filtered ones. As it basically does the same thing as the function Filter() previously, we adjusted the function to use FilterWithRemainder() and redirect the out-filtered ones to the void (For more details check out the conversation).

Which issue(s) this PR fixes

Related and required for #1998

Summary by CodeRabbit

  • New Features

    • Configuration filtering now returns both matching and non-matching sets in a single operation, making it easier to separate and process relevant entries without additional passes.
  • Tests

    • Added tests covering empty options, partial matches, and full matches to validate filtered and remainder outputs and ensure correct behavior across scenarios.

Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm frewilhelm requested a review from a team as a code owner March 19, 2026 09:19
@coderabbitai

coderabbitai Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4b3ed62c-042d-4063-961d-59b7c6386713

📥 Commits

Reviewing files that changed from the base of the PR and between 7e1d121 and d16bb27.

📒 Files selected for processing (1)
  • bindings/go/configuration/generic/v1/spec/filter.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • bindings/go/configuration/generic/v1/spec/filter.go

📝 Walkthrough

Walkthrough

Added FilterWithRemainder to split a Config into filtered (entries whose type is in FilterOptions.ConfigTypes) and remainder (other entries). Filter now delegates to FilterWithRemainder. Tests added to validate empty, partial, and complete matches.

Changes

Cohort / File(s) Summary
Filter implementation & tests
bindings/go/configuration/generic/v1/spec/filter.go, bindings/go/configuration/generic/v1/spec/filter_test.go
Introduced FilterWithRemainder(config, options) (*Config, *Config, error) which returns filtered and remainder configs with nil-checks; refactored Filter to call it and return only filtered; added TestFilterWithRemainder covering empty, partial, and full matches.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hop through configs, sorting with glee,

filtered on left, remainder on thee,
types matched and split, neat as a seam,
a little refactor — a rabbit's dream! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and clearly describes the main change: introducing a new filter function that returns remainder configurations in addition to filtered results.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added kind/feature new feature, enhancement, improvement, extension size/s Small labels Mar 19, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
bindings/go/configuration/generic/v1/spec/filter_test.go (1)

116-167: Add nil-input test cases to lock API contract.

Please add cases for options == nil (treated as empty options) and config == nil (returns error) so panic regressions are caught.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bindings/go/configuration/generic/v1/spec/filter_test.go` around lines 116 -
167, Update TestFilterWithRemainder to include two additional table-driven
cases: one where options == nil (treat as empty options) using a non-nil Config
with entries and assert that filtered is nil and remainder contains all entries,
and one where config == nil (pass nil as the first arg to FilterWithRemainder
and options non-nil or nil) and assert that an error is returned. Reference the
existing TestFilterWithRemainder table, the FilterWithRemainder function, and
types Config and FilterOptions when adding these cases so the test verifies
nil-options behave like empty FilterOptions and nil-config produces an error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bindings/go/configuration/generic/v1/spec/filter.go`:
- Around line 18-23: Add nil guards at the start of the exported function
FilterWithRemainder: validate that the input pointers config and options are not
nil and return a clear error (and nils for the two *Config return values) if
either is nil, to avoid dereferencing panics in lines that access config.Type,
config.Configurations, or options.ConfigTypes; also treat a nil
options.ConfigTypes as an empty slice (or let slices.Contains handle it) but do
not assume options itself is non-nil. Ensure the error message clearly names the
offending parameter and reference the function FilterWithRemainder in your
change.

---

Nitpick comments:
In `@bindings/go/configuration/generic/v1/spec/filter_test.go`:
- Around line 116-167: Update TestFilterWithRemainder to include two additional
table-driven cases: one where options == nil (treat as empty options) using a
non-nil Config with entries and assert that filtered is nil and remainder
contains all entries, and one where config == nil (pass nil as the first arg to
FilterWithRemainder and options non-nil or nil) and assert that an error is
returned. Reference the existing TestFilterWithRemainder table, the
FilterWithRemainder function, and types Config and FilterOptions when adding
these cases so the test verifies nil-options behave like empty FilterOptions and
nil-config produces an error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 568d25c4-6cd2-4973-8a20-e48b47ef56d3

📥 Commits

Reviewing files that changed from the base of the PR and between 78d10d8 and 7e1d121.

📒 Files selected for processing (2)
  • bindings/go/configuration/generic/v1/spec/filter.go
  • bindings/go/configuration/generic/v1/spec/filter_test.go

Comment thread bindings/go/configuration/generic/v1/spec/filter.go
Signed-off-by: Frederic Wilhelm <frederic.wilhelm@sap.com>
@frewilhelm

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@frewilhelm frewilhelm enabled auto-merge (squash) March 19, 2026 10:50
@frewilhelm frewilhelm merged commit 2275859 into open-component-model:main Mar 19, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature new feature, enhancement, improvement, extension size/s Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants