feat: Update provider

[NeaguGeorgiana23]

This PR

• adds GetHooks function to the FeatureProvider interface
• Adds GetHooks function to all classes that inherit from FeatureProvider (InMemoryProvider, NoopProvider and MockFeatureProvider)

Related Issues

Fixes #78

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: efaab86f-c227-4e2d-a2d6-11854dc48ff7

📥 Commits

Reviewing files that changed from the base of the PR and between a3f7c58 and d82aeeb.

📒 Files selected for processing (3)

• openfeature/provider.h
• test/BUILD
• test/mocks/mock_feature_provider.h

💤 Files with no reviewable changes (1)

• test/BUILD

🚧 Files skipped from review as they are similar to previous changes (1)

• test/mocks/mock_feature_provider.h

---

📝 Walkthrough

Walkthrough

Adds the evaluation_options Bazel target and extends FeatureProvider with GetHooks(). No-op and in-memory providers return empty hook collections. The mock provider and Bazel dependencies are updated.

Changes

Provider Hooks

Layer / File(s)	Summary
Hook contracts and evaluation options openfeature/BUILD, openfeature/provider.h	Adds the evaluation_options C++ library target and the default FeatureProvider::GetHooks() contract.
Provider hook implementations openfeature/noop_provider.*, openfeature/memory_provider/*, openfeature/BUILD	Adds GetHooks() overrides that return empty vectors and updates provider dependencies.
Mock provider integration test/BUILD, test/mocks/mock_feature_provider.h	Adds the GeneralHook dependency and mocks GetHooks() const.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• open-feature/cpp-sdk#118: Introduces the GeneralHook API used by this provider hook contract.
• open-feature/cpp-sdk#115: Introduces the hook_hints dependency used by evaluation_options.
• open-feature/cpp-sdk#123: Modifies the same provider hook contract and implementations.

Suggested reviewers: oxddr

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR adds hook accessors but does not show hook registration or participation in the flag evaluation lifecycle required by issue #78.	Implement provider hook execution and lifecycle integration, or narrow issue #78 to the provider hook accessor API.
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.37% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title relates to the provider changes but does not identify the primary GetHooks or provider hooks feature.	Use a specific title such as feat: Add provider hooks interface.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly identifies the GetHooks changes and links them to provider hooks issue #78.
Out of Scope Changes check	✅ Passed	The code changes support the provider hooks API through interface updates, implementations, mocks, and Bazel dependencies.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

openfeature/provider.h (1)

28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Provide a default implementation for GetHooks to avoid breaking existing providers.

Adding a new pure virtual method (= 0) to FeatureProvider is a breaking change that will cause compilation failures for any existing third-party provider implementations. Since providing hooks is typically optional for a provider, consider providing a default implementation that returns an empty vector. This aligns with the approach taken for Init and Shutdown and preserves backward compatibility.

♻️ Proposed fix

-  virtual std::vector<std::shared_ptr<BaseHook>> GetHooks() const = 0;
+  virtual std::vector<std::shared_ptr<BaseHook>> GetHooks() const {
+    return {};
+  }

🤖 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 `@openfeature/provider.h` at line 28, Update FeatureProvider::GetHooks to
provide a default implementation returning an empty vector instead of declaring
it pure virtual, matching the optional behavior of Init and Shutdown while
preserving existing provider compatibility.

openfeature/flag_evaluation_details.h (1)

25-26: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Add an rvalue-reference constructor overload for ResolutionDetails<T>. Consider adding an overload that takes ResolutionDetails<T>&& to allow moving the resolution details instead of always copying them. Flag evaluations occur frequently, and bypassing the copy of heap-allocated structures like std::string and std::unordered_map (inside FlagMetadata) will significantly reduce allocation overhead.
(Note: For this move to be fully effective, ensure that ResolutionDetails<T> also supports move semantics in the future by ensuring it doesn't unnecessarily define a destructor that suppresses implicit move operations).

• openfeature/flag_evaluation_details.h#L25-L26: Add the declaration for the rvalue overload.

  FlagEvaluationDetails(std::string flag_key,
                        const ResolutionDetails<T>& resolution_details);

  FlagEvaluationDetails(std::string flag_key,
                        ResolutionDetails<T>&& resolution_details);

• openfeature/flag_evaluation_details.cpp#L25-L29: Provide the corresponding implementation.

template <typename T>
FlagEvaluationDetails<T>::FlagEvaluationDetails(
    std::string flag_key, const ResolutionDetails<T>& resolution_details)
    : ResolutionDetails<T>(resolution_details),
      flag_key_(std::move(flag_key)) {}

template <typename T>
FlagEvaluationDetails<T>::FlagEvaluationDetails(
    std::string flag_key, ResolutionDetails<T>&& resolution_details)
    : ResolutionDetails<T>(std::move(resolution_details)),
      flag_key_(std::move(flag_key)) {}

🤖 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 `@openfeature/flag_evaluation_details.h` around lines 25 - 26, Add an
rvalue-reference constructor overload to FlagEvaluationDetails<T> in
openfeature/flag_evaluation_details.h at lines 25-26, accepting
ResolutionDetails<T>&& alongside the existing const-reference overload.
Implement the matching overload in openfeature/flag_evaluation_details.cpp at
lines 25-29, move-constructing the ResolutionDetails<T> base and moving
flag_key; ensure ResolutionDetails<T> retains implicit move support by avoiding
an unnecessary destructor.

🤖 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 `@openfeature/flag_evaluation_details.cpp`:
- Around line 3-6: Add an explicit <utility> include alongside the existing
standard-library headers in flag_evaluation_details.cpp so the file directly
provides std::move without relying on transitive includes.

In `@openfeature/hook.h`:
- Around line 34-38: Update the virtual Before method in Hook to accept const
HookContext<T>&, enforcing the immutable hook-context contract and matching the
signatures of After, Error, and Finally. Update the corresponding Before
override in hook_test.cpp so it uses the same const-reference parameter and
continues to satisfy the interface.

---

Nitpick comments:
In `@openfeature/flag_evaluation_details.h`:
- Around line 25-26: Add an rvalue-reference constructor overload to
FlagEvaluationDetails<T> in openfeature/flag_evaluation_details.h at lines
25-26, accepting ResolutionDetails<T>&& alongside the existing const-reference
overload. Implement the matching overload in
openfeature/flag_evaluation_details.cpp at lines 25-29, move-constructing the
ResolutionDetails<T> base and moving flag_key; ensure ResolutionDetails<T>
retains implicit move support by avoiding an unnecessary destructor.

In `@openfeature/provider.h`:
- Line 28: Update FeatureProvider::GetHooks to provide a default implementation
returning an empty vector instead of declaring it pure virtual, matching the
optional behavior of Init and Shutdown while preserving existing provider
compatibility.

🪄 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 Plus

Run ID: 6c117f5a-2248-4b2e-93be-1e737eabbfd8

📥 Commits

Reviewing files that changed from the base of the PR and between b7e65b5 and 6942c4e.

📒 Files selected for processing (12)

• openfeature/BUILD
• openfeature/base_hook.h
• openfeature/evaluation_options.h
• openfeature/flag_evaluation_details.cpp
• openfeature/flag_evaluation_details.h
• openfeature/hook.cpp
• openfeature/hook.h
• openfeature/provider.h
• test/BUILD
• test/evaluation_options_test.cpp
• test/flag_evaluation_details_test.cpp
• test/hook_test.cpp

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

openfeature/BUILD (2)

91-100: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Define the base_hook Bazel target before using it.

All four dependencies require a target named base_hook. Add a header-only cc_library for base_hook.h in openfeature/BUILD.

• openfeature/BUILD#L91-L100: define base_hook before evaluation_options consumes it.
• openfeature/BUILD#L237-L237: keep noop_provider dependent on the new target.
• openfeature/BUILD#L286-L286: keep provider dependent on the new target.
• openfeature/memory_provider/BUILD#L24-L24: keep the absolute dependency after the target exists. (raw.githubusercontent.com)

🤖 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 `@openfeature/BUILD` around lines 91 - 100, Define a header-only cc_library
target named base_hook for base_hook.h before evaluation_options in
openfeature/BUILD, so its dependency is available. Preserve the existing
dependency on the new target at openfeature/BUILD:237-237 for noop_provider,
openfeature/BUILD:286-286 for provider, and
openfeature/memory_provider/BUILD:24-24; these sites require no direct changes.

Source: MCP tools

---

33-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Derive GeneralHook from BaseHook.

Hook<T> currently inherits from GeneralHook, but provider hooks and EvaluationOptions store hooks as std::shared_ptr<BaseHook>. Make GeneralHook inherit from BaseHook, include openfeature/base_hook.h, and add :base_hook to the general_hook Bazel target so concrete hooks are usable through the base hook interface.

🤖 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 `@openfeature/BUILD` around lines 33 - 42, Update the GeneralHook class to
inherit from BaseHook, include openfeature/base_hook.h, and initialize the
BaseHook subobject via :base_hook. Add the :base_hook dependency to the
general_hook Bazel target so Hook<T>, provider hooks, and EvaluationOptions can
use concrete hooks through std::shared_ptr<BaseHook>.

Source: MCP tools

🤖 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 `@openfeature/noop_provider.h`:
- Around line 27-29: Update FeatureProvider::GetHooks() to provide a non-pure
default implementation that returns an empty vector of hooks, while keeping
existing overrides valid so external providers remain source-compatible.

---

Outside diff comments:
In `@openfeature/BUILD`:
- Around line 91-100: Define a header-only cc_library target named base_hook for
base_hook.h before evaluation_options in openfeature/BUILD, so its dependency is
available. Preserve the existing dependency on the new target at
openfeature/BUILD:237-237 for noop_provider, openfeature/BUILD:286-286 for
provider, and openfeature/memory_provider/BUILD:24-24; these sites require no
direct changes.
- Around line 33-42: Update the GeneralHook class to inherit from BaseHook,
include openfeature/base_hook.h, and initialize the BaseHook subobject via
:base_hook. Add the :base_hook dependency to the general_hook Bazel target so
Hook<T>, provider hooks, and EvaluationOptions can use concrete hooks through
std::shared_ptr<BaseHook>.

🪄 Autofix

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 Plus

Run ID: 574fdea7-6b79-4103-bf6c-cbe427b0f503

📥 Commits

Reviewing files that changed from the base of the PR and between 6942c4e and a283a19.

📒 Files selected for processing (8)

• openfeature/BUILD
• openfeature/memory_provider/BUILD
• openfeature/memory_provider/in_memory_provider.cpp
• openfeature/memory_provider/in_memory_provider.h
• openfeature/noop_provider.cpp
• openfeature/noop_provider.h
• test/BUILD
• test/mocks/mock_feature_provider.h

🚧 Files skipped from review as they are similar to previous changes (1)

• test/BUILD

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@openfeature/provider.h`:
- Line 28: Update FeatureProvider::GetHooks so existing subclasses remain
instantiable by providing a default empty implementation instead of making the
method pure virtual. Preserve the current return type and const contract, and
avoid changing provider behavior for implementations that do override GetHooks.

🪄 Autofix

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 Plus

Run ID: 1ae2d828-9e52-4a56-9df7-eb90a90b3391

📥 Commits

Reviewing files that changed from the base of the PR and between a283a19 and a3f7c58.

📒 Files selected for processing (11)

• openfeature/BUILD
• openfeature/evaluation_options.h
• openfeature/memory_provider/BUILD
• openfeature/memory_provider/in_memory_provider.cpp
• openfeature/memory_provider/in_memory_provider.h
• openfeature/noop_provider.cpp
• openfeature/noop_provider.h
• openfeature/provider.h
• test/BUILD
• test/evaluation_options_test.cpp
• test/mocks/mock_feature_provider.h

🚧 Files skipped from review as they are similar to previous changes (6)

• openfeature/memory_provider/in_memory_provider.h
• openfeature/noop_provider.cpp
• openfeature/noop_provider.h
• test/mocks/mock_feature_provider.h
• openfeature/evaluation_options.h
• openfeature/memory_provider/in_memory_provider.cpp

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds provider-level hook support and verifies EvaluationOptions storage semantics via new unit tests.

Changes:

• Extends FeatureProvider with a GetHooks() API returning GeneralHook instances.
• Implements GetHooks() for NoopProvider, InMemoryProvider, and updates the provider mock accordingly.
• Introduces a new evaluation_options_test Bazel target with tests for hooks and hook hints.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/mocks/mock_feature_provider.h	Updates mock provider interface to include GetHooks()
test/evaluation_options_test.cpp	Adds unit tests for EvaluationOptions hooks and hook hints behavior
test/BUILD	Registers new test target and required deps
openfeature/provider.h	Adds GetHooks() pure virtual API to provider interface
openfeature/noop_provider.h / .cpp	Implements GetHooks() for the noop provider
openfeature/memory_provider/in_memory_provider.h / .cpp	Implements GetHooks() for in-memory provider
openfeature/memory_provider/BUILD	Adds general_hook dep for provider build
openfeature/BUILD	Adds general_hook deps to affected libraries

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[m-olko]

You are adding here evaluation_options_test.cpp which you deleted in earlier PR. Other than that, LGTM

[NeaguGeorgiana23]

You are adding here evaluation_options_test.cpp which you deleted in earlier PR. Other than that, LGTM

Thanks for pointing that out. I overlooked that.